Promotable by Construction
How a staged change is prevented from becoming one the user can see, cannot accept, and cannot clear.
- Status: Findings, plus one implemented fix (§4). The rest of this page is the evidence base and requirement list for a redesign that has been agreed but not yet written up as an ADR — see The chosen direction.
- Policy it serves: ADR 0009 and ADR 0010. ADR 0010 already states the contract this page is about — "partial promotion is atomic", "a failure before the audit/SACM commit changes nothing" — and the audit below records where the implementation does not yet meet it. The redesign will supersede the operation-staging parts of both.
The defect this exists to make impossible
An MCP client staged one operation: set the text of C2, naming the field
content. C2 is a GSN Context — a SACM ArtifactReference — and a context
has no content; its text is its description. The operation was accepted into
the draft, materialized, and drawn on the canvas as a pending change. Only at
Accept did the library seam refuse it, with writing text on C2 failed and
no reason attached.
The user was then stuck: the change could not be accepted, and the message named neither the group to reject nor the field to correct.
This was not one wrong branch. It is the visible instance of a structural split.
The structural cause: two authorities on one question
"Can this change be applied?" is answered twice, by two bodies of code that never meet until the write.
| Staging asks | Promotion asks | |
|---|---|---|
| What runs | ReviewProposalPatchService over the flat POD model |
PlanProposalFromDiff → ApplyProposalPlanToLibrary → the sacm_adapter seams |
| What it knows | Every element has name, content and description |
An element's fields are decided by its SACM kind |
Verdict on content for a context |
Fine | Refused |
CanStageOperations rehearses only materialization
(src/core/drafts/draft_materializer.cpp). PlanDraftPromotion
(src/core/drafts/draft_promotion_service.h) — the function the Draft Changes
panel calls to decide whether a row is promotable — takes a
core::AssuranceCase and no library document, so it cannot know what the
seams will refuse either. The library is consulted for the first time inside the
accept.
The flat POD model is strictly more permissive than the document. Every gap in this class is that difference, unchecked.
It is not one gap
An audit of every operation an MCP client can stage against the seam that must express it found fourteen reachable instances, including:
UpdateElementTextwithfield: "content"on a Context, Solution or Category — the reported defect.fieldis optional and defaults tocontent, so an agent that omits it hits this too.AddSupportedBy/AddInContextOfnaming a Term or Category as an endpoint.list_termsis an advertised tool, terms appear as ordinary elements, and the library requires every endpoint to be anArgumentAsset.SetUndevelopedon a Solution or Context (not anAssertion), or on an Assumption or Justification (declaration isassumed/axiomatic, which cannot also beneedsSupport).RemoveSupportedByon a relationship with several sources — a shape the application itself builds — because the plan has no retarget verb.RemoveElementwithnode_onlyon a node with children, for the same reason.
And one that is worse than a refusal: UpdateElementText, UpdateElementName
and SetUndeveloped targeting a relationship id are silently dropped.
The planner returns before collecting text writes, the accept reports success,
and the approved change is not in the saved file. The post-promotion semantic
hash check cannot catch it, because the predicted model took the same lossy
path.
The knowledge is copied, not shared
"Which field holds this element's text" is written out in eight places, plus
five more inline restatements elsewhere. The intended single authority already
exists — core::ClaimLikeCarriesStatementAsDescription in
src/core/sacm_model.h — and its own comment warns that duplicate copies of the
kind list "is how they drift apart." That is exactly what happened: the guard
was written for one direction (description on a claim) and never for its
mirror.
The chosen direction
The obvious repair is to make staging ask the library before accepting an
operation — the rehearsal primitive for it already exists and is proven
(PreflightProposalAgainstLibrary serializes the live document, reloads it into
a genuine clone, runs the real apply against the clone, and leaves the
authoritative document byte-identical).
That was rejected in favour of something simpler, because it still guards the gap rather than removing it. The agreed direction removes the second model:
The draft is a real SACM document, not a list of operations against a flat model. An agent edits that document directly. Accept is an atomic replacement of the accepted file by the draft.
Why this is the better answer to the same problem:
- The defect class cannot exist. There is no permissive intermediate model for an unrepresentable change to live in. The library accepts or refuses at the moment the client asks, so anything a client can do is something the application can do — by construction, not by a validator kept in sync.
- Accept cannot fail. No plan, no seam sequence, no partial write, no "which of five groups broke". §2 and §3 below stop being work to do and become situations that cannot arise.
- It deletes machinery rather than adding it. The workspace store, the
materializer, the change-group compiler,
PlanDraftPromotion, the promotion preflight, thecreate_refindirection and identity pinning, and the use ofReviewProposalas a promotion vehicle all exist only to carry operations across the gap being removed. - The MCP surface shrinks with it. Staged-group vocabulary, patch-operation
types and
create_refplaceholders exist because operations are held in escrow and cashed later; against a real document, ids are real when an element is created and every call gets an immediate, accurate answer.
Two properties of the current code make it viable: human edits are already routed into the draft rather than the accepted model, so the accepted file has exactly one writer and cannot drift under a draft in normal use; and the save-then-reload round trip is already relied upon as lossless, with a byte-identical test behind it.
Review model. Accept is all-or-nothing. Selective review is preserved by
inverting the gesture: rather than accepting a subset, the reviewer rejects
individual changes in the draft — reverting an element to the accepted value,
a well-defined document operation — and then accepts what remains. Provenance
for "who changed what" belongs in the draft document as vendor TaggedValues
(clause 8.12, the mechanism already used for GSN identifiers and ACPs), so it
travels with the model instead of in a parallel index that can disagree with it.
Open for the ADR. The audit and undo semantics need real design rather than
transcription: promotion is currently an audited, replayable, undoable command,
and the audit store's premise is that replaying transactions reproduces the
SACM. Baseline drift also needs a deliberate answer — operations can be replayed
onto a new baseline, whole documents cannot — though with one writer to the
accepted file, "the draft is stale; keep it or discard it" is likely sufficient.
The draft file belongs under .af/, not beside the argument, or the project
scanner and version control both pick it up.
Requirements the redesign must satisfy
The findings below were the case for redesigning. They are recorded here as the list the new design has to answer, not as work queued against the old one.
1. Every refusal reaches the client at the time it asks
An unusable request must never be accepted, rendered as pending, and refused later. This is the requirement §4 satisfies locally and the redesign satisfies structurally.
2. Promotion must be atomic
ApplyProposalPlanToLibrary writes elements, relationships, terminology, text,
flags and deletions into the live document in sequence. A refusal on write
k leaves writes 1..k-1 applied. The draft-promotion caller compensates with
a serialize-and-restore snapshot, but that rollback is conditional on the audit
bus being open, and its failure is advisory — nothing stops the partial state
reaching disk on the next successful command. The other two callers of the same
command — accepting a review proposal, and accepting an agent change set — have
no preflight, no snapshot and no rollback at all.
A file-replacement accept satisfies this outright. If any part of the sequenced
applier survives the redesign, atomicity belongs in ApplyProposalCommand
rather than in one caller.
3. No unrecoverable state
Two defects in the current implementation, both found by audit, and both requirements on whatever replaces it:
- The
pending_promotiondeadlock. If a promotion's durable marker survives with a hash matching neither the expected result nor the baseline, the workspace refuses accept, refuses every edit, and refuses discard — while the UI's Discard button stays enabled. The only exit is hand-editing.af/drafts/<key>/workspace.json. It is reachable, and untested. A contributing cause is a real inconsistency: the marker records the predicted model hash, while every consumer compares the produced one, and the two legitimately differ for a promotion containing terminology. Discard must always be available. - A failure must name its group. Mid-plan refusals name a resolved element
id, which has no visible relationship to a draft group. Accept-all compiles
every group into one proposal, so "creating XYZ failed" leaves a user with
five groups and no idea which to reject. Since the compilation already
namespaces each group's
create_refs, the group is known and can be reported.
rebase and export recovery data are promised by ADR 0010 and do not exist;
with accept and edit both blocked, discard is the only exit from NeedsRebase,
and discard destroys the work. Either build them or amend the ADR.
4. Remove ambiguity rather than validate it — implemented
The reported defect had a simpler cause underneath: the operation asks the
client which field to write, when the element's kind already determines it.
UpdateElementName is a separate operation, so for any argument element there
is exactly one text field and only one right answer.
ElementCarriesContent (src/core/sacm_model.h) now states which kinds have a
content at all, beside the sibling rule it completes, and the patch service
uses it:
- An omitted field is resolved from the element's kind rather than defaulted
to
content. The caller asserted nothing, so there is nothing to contradict. - A field that names
contenton a kind that has none is refused, namingdescriptionas the field to use — the mirror of the guard that already refuseddescriptionon a claim.
This is the ninth copy of the rule being deleted, not a tenth being added. Both directions are now covered by tests; before this change the mirror case had none.
What is done and what is not
| State | |
|---|---|
| §4 field-kind rule, both directions, with tests | Implemented |
| §1 every refusal reaches the client when it asks | Redesign |
| §2 atomic accept | Redesign — an atomic file replacement satisfies it |
| §3 discard always available; failures name their group | Redesign |
| Silent drop of relationship-targeted text/name/flag operations | Open defect. A correctness bug, not only a usability one: accept reports success and the change is absent from the saved file. Independent of the redesign, and it should be confirmed fixed by it rather than assumed. |
rebase / export recovery data from ADR 0010 |
Not built; the ADR overstates. Either build them or amend it. |
The draft-document redesign is expected to retire §1–§3 by construction. The silent-drop defect and the ADR 0010 overstatement are separate and survive it.