feat(calm-hub): add snapshot versions for iterating before publish - #3122
Open
willosborne wants to merge 23 commits into
Open
willosborne wants to merge 23 commits into
willosborne wants to merge 23 commits into
Conversation
Adds SNAPSHOT_VERSION_REGEX/MESSAGE alongside the strict VERSION_REGEX and
swaps the six namespace resource endpoint files (architectures, patterns,
flows, standards, interfaces, and the two namespaces/.../versions/{version}
handlers in the mapping controller) onto it. ADR, timeline, control and
layout endpoints are untouched and keep rejecting -SNAPSHOT: ADR runs on
VersionScheme.NUMERIC, where a suffixed value is an unparseable revision,
not a version.
Updates the pre-existing 400 message assertions in the five opted-in
resource test classes to match the new pattern text; validation strictness
itself is unchanged.
…e content decrement_the_version_count_when_it_deleted_a_version only verified an update call happened with any arguments, so a flipped +1/-1 sign passed silently. find_the_version_under_any_accepted_spelling and delete_one_version_and_leave_the_others only asserted the return value, so a dropped canonicalise call or a dropped version constraint on the Nitrite filter also passed silently, since the mocked find(...) returns the stubbed document regardless of which filter it receives. Now each captures the Bson/Filter passed to the mock and asserts on its rendered content, and a Mongo canonicalisation test mirrors the existing Nitrite one so both backends prove the same behaviour.
Standards and interfaces previously had no update path, which is why the PUT endpoint returned 501 for both types. Snapshots are mutable by design, so each store now has an updateStandardForVersion / updateInterfaceForVersion delegating to the version-document helper's upsertVersion instead of createVersion, with no existing-version rejection.
Reorder mutations passed silently in MongoInterfaceStore and NitriteStandardStore: no test distinguished header write before vs. after the version write, which updateHeaderDetails' own javadoc requires. Adds a never-touch-the-header-on-failed-version-write test per store.
… releases A POST to an existing -SNAPSHOT version now overwrites and returns 200 instead of 409, since a client should never have to know whether the snapshot already exists. A snapshot whose release version is already published still returns 409, since promoting that release would otherwise have to both create and delete the same logical version. The shadow check canonicalises the request's release-version spelling before comparing against the stored versions list, since the store only ever holds canonical spellings (e.g. 100-SNAPSHOT's release version "100" must match a stored "1.0.0"). Also extends updateVersionedResourceInStore's switch to cover standard and interface resources, using Task 6's update-for-version store methods, so overwriting a standard/interface snapshot works the same way as the other three resource types.
…ypes updateVersionedResourceInStore's STANDARD/INTERFACE arms had no test that could reach them: PUT returns 501 for both types before the service is called, and the pre-existing STANDARD/INTERFACE tests only exercise the create branch. The snapshot-overwrite path added in b3a3018 is the only caller that reaches those arms, so add a snapshot-overwrite test per type asserting a 200 and that updateStandardForVersion/updateInterfaceForVersion was called.
Publishing a release now deletes its matching -SNAPSHOT version after the release write succeeds. Not atomic: the release is written first, and a failed snapshot delete is logged and swallowed rather than rolled back, since the release is the outcome the user asked for and a stranded snapshot is recoverable. Adds a delete-one-version method to all five store interfaces (Mongo and Nitrite), delegating to the existing version-document helper's deleteVersion.
…snapshot deleteSnapshotForVersion compared a raw path-param release spelling against the canonically stored snapshot version, so a non-canonical release POST (e.g. "100" for "1.0.0") silently orphaned its snapshot instead of deleting it. Canonicalise before both the presence check and the delete call, matching the existing shadow-check pattern earlier in the same method. Also adds the missing success LOG.info line to the five Nitrite deleteXVersion delegates, matching their neighbouring deleteX methods.
TestMappingControllerResourceShould only exercised these through mocked
store interfaces, so the concrete Mongo and Nitrite implementations had zero
effective coverage. Adds a namespace-missing, a resource-missing, a
successful-delete and an absent-version test for each of the ten
deletePatternVersion/deleteArchitectureVersion/deleteFlowVersion/
deleteStandardVersion/deleteInterfaceVersion implementations.
Also adds a regression test for a non-canonical release spelling ("100")
correctly deleting its canonically-stored snapshot ("1.0.0-SNAPSHOT").
The first version of a resource may now be 1.0.0 or 1.0.0-SNAPSHOT, letting a client iterate a document before its first publish. The rule still applies to the release version, so 2.0.0-SNAPSHOT is rejected as a first version. Threads the requested first version down through createResourceInStore into the five create<Type>ForNamespace store methods (Pattern, Flow, Standard and Interface gain a version parameter; the two Architecture stores already carry it on the Architecture object). Canonicalises the release spelling before comparing it against "1.0.0" so a first version like 100-SNAPSHOT is accepted rather than wrongly refused.
…pshot ordering
Adds AuditRequestFilter.restageAction so a resource layer's provisional
staged action can be corrected once the service knows a write actually
overwrites an existing snapshot. deleteSnapshotForVersion now stages a
DELETE for the removed snapshot, carrying its own version, before the
store call.
Pins ArchitectureTimelineService's existing snapshot-in-position
ordering with a regression test (Semver.parse already accepts the
-SNAPSHOT suffix from an earlier task).
Note: the overwrite->UPDATE restage has no live effect yet. The only
endpoint that can currently write a snapshot (POST .../versions/{version})
never stages a context, so AuditRequestFilter falls back to its
path-based resolution (hardcoded UPDATE for that path) regardless. The
generic POST /calm endpoint, which does stage a context, rejects
snapshot versions outright (CalmDocumentParser#parseCanonicalId still
validates against VERSION_REGEX, not SNAPSHOT_VERSION_REGEX). See the
task report for details.
…w on promotion
CalmDocumentParser#parseCanonicalId validated a $id's version against
VERSION_REGEX, so the generic POST/PUT /calm endpoints rejected every
snapshot version while their path-driven sibling
(POST .../versions/{version}) already accepted it. Switches to
SNAPSHOT_VERSION_REGEX so both agree. validateVersion (used only by the
domain-control $id branches) is untouched: controls stay out of
snapshot scope.
This also makes the generic /calm endpoint's staged CREATE reachable
with a snapshot version, so addNewVersion's restageAction(UPDATE) call
now has a live effect and is exercised through HTTP, not only at the
unit level.
Reverts the DELETE staging added for snapshot deletion during
promotion. AuditRequestFilter supports exactly one recorded row per
request; staging a DELETE there overwrote the release write's own row,
so a promotion was audited solely as "deleted the snapshot" with no
record of the release it published. The release write is the durable
event; keeping its row is the better of the two single-row outcomes
available.
Adds a TestContainers-backed integration test proving the full -SNAPSHOT lifecycle against a real MongoDB: create, overwrite, list, promote (with snapshot deletion), shadowing rejection, and search visibility. Mocked store tests can't catch a failure in the create-then-delete promotion sequence or in versionCount bookkeeping split across header and version documents. Also extends the OpenAPI descriptions on the two snapshot-capable POST endpoints and adds a short snapshot-lifecycle table to the CalmHub API docs.
The numeric /api/calm/namespaces/... endpoints (ArchitectureResource, FlowResource, InterfaceResource, PatternResource, StandardResource) call their stores directly and never reach MappingControllerService, which is the only place the three snapshot rules (idempotent overwrite, shadow 409, promotion delete) are enforced. An earlier task widened the version @pattern on POST and PUT for this API too, letting it durably create a snapshot shadowing a published release with no rules applied. Revert POST/PUT version path params to VERSION_REGEX/VERSION_MESSAGE. GET stays on SNAPSHOT_VERSION_REGEX so snapshots created via the name-based /calm/... API remain readable here.
versions holds canonical spellings, but the overwrite check compared it against the raw request spelling directly, so a request using a different accepted spelling than the one stored (e.g. 200-SNAPSHOT vs. stored 2.0.0-SNAPSHOT, or 100 vs. stored 1.0.0) missed the match. That also broke the release path: POST 100 against a stored 1.0.0 returned 400 instead of the correct 409. Canonicalise the request version before the versions.contains check, matching the shadow check above and deleteSnapshotForVersion below.
TimelineResource and ControlResource already keep the strict VERSION_REGEX (snapshots are scoped to the five namespace resource types only), but that boundary had no test coverage of its own and could regress silently.
SNAPSHOT_VERSION_REGEX restated the -SNAPSHOT literal instead of concatenating ResourceVersion.SNAPSHOT_SUFFIX, the one place that spelling should live. It's a compile-time constant, so concatenating it into the annotation constant expression is legal. Also drops an unused ResourceVersion import in TestCanonicalVersionShould.
Member
Author
willosborne
marked this pull request as ready for review
September 21, 2026 10:04
willosborne
requested review from
grahampacker-ms,
jpgough-ms and
rocketstack-matt
as code owners
September 21, 2026 10:04
willosborne
requested review from
a team,
Thels and
markscott-ms
as code owners
September 21, 2026 10:04
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.







Description
Adds
-SNAPSHOTversions to CalmHub, so architects can iterate on a pattern or architecture before they publish it. Closes part of #3023.A snapshot is a mutable version. Push to it again and it is overwritten. Publish the release version and CalmHub deletes the snapshot.
POST .../versions/1.0.0-SNAPSHOT201, or200if it is already therePOST .../versions/1.0.0-SNAPSHOTwhen1.0.0is published409POST .../versions/1.0.0201, and the snapshot is deletedScope is CalmHub only, and only the five namespace resource types: pattern, architecture, flow, standard and interface. The CLI and UI parts of #3023 need their own issues. The
-SNAPSHOTsuffix is refused on ADR, timeline, layout and domain controls.The issue also asks to remove auto-bump from
calm workspace push. That is already the behaviour, so no change was needed.Decisions worth review
Snapshots work on the name-based
/calm/...API only. The numeric/api/calm/...endpoints call the stores directly and never reachMappingControllerService, where the three snapshot rules live. Sharing a store does not share the rules. A numeric endpoint that took a snapshot would create one that shadows a published release, with no check and no cleanup, so those endpoints keep the strict version pattern onPOSTandPUT. TheirGETaccepts the suffix, so a snapshot stays readable there.Promotion is not atomic, deliberately. The release is written first, then the snapshot is deleted. There is no transaction across the two. If the delete fails, the release is correct, the request still returns
201, and the orphan is logged. The reverse order would let a failed release write destroy work in progress.The permission model is unchanged.
WRITEcovers snapshots. No new scope.Gotchas that are not obvious
VERSION_REGEXmakes both separators optional, so1.0.0,1-0-0,1.00and100are the same version. Any version that comes from a request and is compared against stored data must be canonicalised first. This was got wrong four times while building this.docs/superpowers/specsrecords it;ResourceVersionandCanonicalVersionare the only places that should know about the suffix.Semver.tryParseorder of operations. It replaces-with., which is what makes1-0-0parse. The suffix must come off before that. After it,1.0.0-SNAPSHOTbecomes four segments and collapses to0.0.0, which sorts lowest — the ADR revision 100 failure thatVersionSchemedocuments.Known limitations
AuditRequestFilterstages a single context, so promotion cannot record both the release creation and the snapshot deletion. The release keeps its row; the deletion is not audited. Recording both needs multi-row support.SearchResultcarries no version, and search matches the resource header, whose name and description come from the most recent write. A snapshot's title therefore reaches search results before the work is published.Separate from this PR
While working on
MappingControllerServicewe found a pre-existing bug, which this PR does not change. At line 114 thePUTcaller passes its arguments in the wrong order forupdateVersionedResourceInStore, so aPUTwrites the title as the document body. It comes from1c95ec5aand the existing tests miss it because they useany()matchers. It only affects instances that setallow.put.operations=true. It needs its own issue.Type of Change
Affected Components
cli/)calm/)calm-ai/)calm-hub/)calm-hub-ui/)calm-server/)calm-widgets/)docs/)shared/)calm-plugins/vscode/)Testing
./mvnw test -pl calm-hub— 3108 pass, 0 failures. Up from 3090 onmain.../mvnw -P integration verify— 551 pass, including a newSnapshotLifecycleIntegrationthat runs create, overwrite, publish, and the shadow refusal against MongoDB in TestContainers.Tests were checked by mutation: production code was deliberately broken to confirm each new test fails. This found several tests that passed against broken code and would not have caught a regression.
Checklist
🤖 Generated with Claude Code