Correct the rebuild plan's manifest shape before it steers the add/commit cut

[gunbai-bot]

docs/plans/scm-demo-cli-rebuild.md carried a paragraph whose stated purpose was to stop the next reader following a dead recipe. It had become one.

It said add and commit are blocked on building a CorpusManifestObject shaped path → semantic_root → authored_source_identity. Both halves are false:

• The object exists — gunbc.scm.object_store declares CorpusManifestObject, CorpusManifestRecord and CorpusManifestEntry, and gunbc.scm.object_table_json codes them.
• Its entry is { path, source } — path to authored source, deliberately not to a semantic root. object_store states the reason: a semantic root is a derived ingestion result, and fusing it into the entry makes a manifest unauthorable for malformed or not-yet-ingested source — precisely the corpus a source-control system must still be able to freeze. Settle what a merge IS before the manifest hardens #10522 §5 ruled the same separation.

A second passage under "Not in scope here" repeated the same wrong shape and is corrected with it.

DESIGN §3 names this failure exactly: while a superseded structure stands, every nearby question keeps being answered in its vocabulary. This paragraph sat directly in front of the add/commit cut, so it was positioned to decide it.

What the corrected boundary now says

The actual pending thing is a workspace-scoped authored-source snapshot authority with three consumers: add updates it, status reads it, commit consumes it. It must survive between invocations; it need not live in the repository document.

Three obligations the old text did not carry:

• status moves with this cut rather than being an unchanged downstream reader. It currently passes empty_proposal() and renders "nothing staged"; leaving that in place would report "nothing staged" while authored-source changes are staged — an observation correct about its input and wrong about the subject asked. Paths must not be translated into StagedRole to preserve the existing renderer.
• Staging carries publication's conditional-write obligation. One add races another; a commit races a newer stage. save_repository does not supply this — it checks encoding, then performs an unconditional Filesystem.Write.
• An established unchanged candidate is not an unavailable or unreadable stage, and staged-versus-unstaged needs a separate working-tree observation, since comparing the stage to its base says nothing about whether the working files changed again.

Two further corrections, after review

Two adjacent sections still instructed the reader to do the opposite of the corrected boundary, and one of them was re-endorsed by this very PR:

• The outcome sketch is unsound, and the correction had certified it. RepositorySave carries three failure arms, so ScmWritePersisted { save: RepositoryFileUnwritable { … } } is constructible — a renderer selecting the outer Persisted arm announces persistence while its own payload says the file was unwritable. ScmWriteRefusedByMint accepts the successful RepositoryCommitMinted arm symmetrically. The outer classification and the inner payload can disagree. Replaced with the governing requirement: a success outcome carries established success evidence for that exact operation; a refusal carries an actual refusal cause; where publication cannot be established, that uncertainty stays an explicit outcome. The sketch is kept visible as the instructive half.
• "add has no staging authority" had the right premise and the wrong branch. What is staged genuinely is not a fact the repository document carries — which is why status moves with this cut. But concluding that add therefore inserts into the object store immediately picked the branch cheapest to build: insertion cannot say which paths are selected for the next commit, and cannot be revised or cleared.
• "add is not blocked" narrowed to the dependency actually removed. Host acquisition, path admission, durable selection and conditional update remain work; a removed dependency is not a finished verb.

Scope

Documentation only; no code.

The commit-root ruling is now received: manifest-rooted repository commits. RepositoryCommit.root is still SemanticNodeObjectRef, and committing a manifest is structurally refused in three places (RepositoryCommitMintRootIsCorpusManifest, RepositoryEncodeCommitRootIsCorpusManifest, RepositoryCommitRootIsCorpusManifest) — those get re-aimed, not deleted, with a semantic root becoming the wrong-kind case. That carrier/codec/consumer transition is a model change to a load-bearing module and is deliberately not in this PR. A settled design is not a completed implementation.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VQ4iThiZ1B9LPB9ePr8qa9

[briansrls]

DOCUMENTATION REVIEW — REQUEST_CHANGES at exact head 2d04d39.

I read the complete current rebuild plan, the complete one-file PR patch, the current PR description, and the relevant exact-head object-store, repository-mint, repository-save and CLI contracts. The PR remains documentation-only. No execution is claimed, credited, or required to close the two findings below. No merge has been performed.

CREDITED

The two manifest-shape corrections are right. A manifest already exists and records path-to-authored-source, not path-to-semantic-root-to-source. The new workspace-scoped staging contract correctly has add update the selection, status read it and commit consume it; cross-invocation persistence does not determine physical document placement. The status consumer must move with the source-staging cut rather than continue reporting the empty role proposal. The conditional stage update, exact snapshot consumption, generation-conditioned clearing, stale-base refusal and unchanged-versus-unavailable distinctions are all accepted. Separate working-tree observation is correctly required before claiming staged-versus-unstaged information.

R1 [P2] — The corrected boundary leaves two adjacent operational instructions prescribing the opposite implementation.

Location: docs/plans/scm-demo-cli-rebuild.md, the newly corrected boundary and the immediately following sections 'The design step, stated so it is not improvised' and 'add has no staging authority, and that is not a wiring gap'.

The latter sections still state:

• add composes store_node with save_repository, and commit uses the same save;
• direct content-addressed object insertion with no staging is 'the honest reading of add for this substrate'.

Those are not the explicitly historical synthetic-node recipe earlier in the document. They are unqualified next-increment instructions under their own headings. The new text says object insertion is not by itself add and save_repository cannot supply conditional publication, while the next recipe still directs exactly those operations. A maintainer following that recipe can persist a source-as-node/object insertion, publish through an unconditional write and leave no durable path-grain staging selection for status or commit. The accepted new paragraphs do not make that route satisfy the contract.

Repair these two operative sections in place, or label them superseded at their headings with an explicit pointer to the current staging/publication contract. Preserve useful historical receipts; no whole-document historical rewrite is requested. Do not implement the new stage or publication actuator in this documentation PR.

Also qualify the new sentence 'add is not blocked': what is removed is the supposed prerequisite to build a nonexistent manifest or solve structural merge. Acquisition, supported source representation, path admission, durable selection and conditional update still require implementation. The current PR body's Scope also still calls the commit-root ruling open; update that to the now-received manifest-root decision with its implementation pending. A ruled design is not a landed carrier/codec change.

R2 [P2] — The new paragraph positively re-endorses a write-outcome sketch whose success arm can contain failure.

The new boundary explicitly says 'ScmWriteOutcome is still the right shape for the verb's result'. The linked sketch has:

ScmWritePersisted { save: RepositorySave }

But RepositorySave includes RepositorySaveRefusedByCodec, RepositoryFileUnwritable and RepositoryWriteByteCountUnrepresentable, not only RepositorySaved. Therefore the proposed shape permits, for example:

ScmWritePersisted {
  save: RepositoryFileUnwritable { path: "repo.json", error: "permission denied" }
}

This is a counterexample to the proposed carrier, not a mutation I executed. The inverse misclassification is also expressible: ScmWriteRefusedByMint carries the whole RepositoryCommitMint, including RepositoryCommitMinted. The outer outcome name is not derived from the inner outcome; the two can disagree.

This defect is not merely incidental legacy text: this PR newly certifies that sketch as correct. A renderer matching the outer Persisted arm can announce persistence for an explicitly failed save. Having one outer coproduct does not remove the contradiction when each payload is the complete result coproduct of a different operation.

Withdraw the endorsement and mark the sketch superseded, or replace it at design-contract scope with the rule that a success arm carries only established success evidence for the exact operation, and refusals carry actual failure causes. Preserve an unestablished publication outcome wherever the realization cannot establish application. The existing RepositorySave success also must not be promoted to proof of conditional staging/publication; its operation writes unconditionally. A final shared result schema or new implementation is not required here. The still-valid idea is to compose the actual operation outcomes without positional ambiguity, not to preserve this particular anemic payload sketch or its maintained arm counts.

SEPARATE SCOPING RULING FOR THE NEXT MODEL CUT — NOT A REQUIREMENT TO ADD CODE HERE

CorpusManifestTarget is justified, but by its consumer and semantic role rather than symmetry. The new mint must accept the proposition 'a corpus manifest is required at this locator' and resolve it in the repository's actual store before retaining the CorpusManifestObjectRef obtained from the found record. The public target is not evidence of object existence, correct kind, source completeness, durable retention or publication admission. Keep it separate from SemanticNodeTarget and AuthoredSourceTarget, and from the sole-constructor manifest reference. Do not widen semantic edges, introduce a Resolved/Unresolved state inside this requirement, or mint a manifest brand from an unchecked locator.

The existing find_corpus_manifest_record / CorpusManifestLookup is the resolution authority. Its total population has FOUR outcomes: CorpusManifestFound, CorpusManifestAbsent, CorpusManifestIsSemanticNode and CorpusManifestIsAuthoredSource. There are three non-success outcomes, not three total outcomes. The mint must consume them all. Add only the target/projections demanded by the actual cut rather than a family of symmetry-only helpers.

A target wrapper introduces no new object kind or hashing rule. If the existing object derivations remain unchanged, the planned commit-root change requires the repository schema transition, not an object identity-rule change. The manifest's source requirements and semantic-node edge requirements remain unchanged. Re-aiming the repository commit does not make a held manifest prove a complete source closure.

DISPOSITION

The direction and the next-cut manifest-target scope are accepted. The documentation is not approved at this head because its newly endorsed result shape and remaining operative recipe contradict that direction. R1 and R2 can close by documentation changes alone. No new observer, structural merger, staged-state implementation, conditional-write implementation or execution run is requested for #10555.

[briansrls]

APPROVE — documentation-only PR #10555 at exact head 10836c2.

R1 and R2 from review 5120884296 are CLOSED at documentation-contract scope. No blocking finding remains from my review. This authorizes landing this exact documentation head under the standing reviewer-approval rule, without waiving repository protections or approving an implementation of staging, commit-root replacement, or conditional publication. I have not performed a merge.

REVIEW SUBJECT

I inspected the entire intervening commit patch and changed-file census, the current affected sections and surrounding remainder of docs/plans/scm-demo-cli-rebuild.md, the current PR description, and the prior review submission. The Git commit record confirms this is a direct child of reviewed 2d04d39. Only the Markdown document changed, so the previously inspected source authorities are preserved. I rechecked the live PR head immediately before submitting this review.

R1 — THE OBSOLETE RECIPES ARE WITHDRAWN WHERE READERS ENCOUNTER THEM

Both formerly operative sections now have explicit SUPERSEDED headings. The store_node/save_repository composition is described as what the plan previously instructed, not the next increment's implementation. The direct-insertion conclusion is explicitly withdrawn, while the useful distinction between stored objects and a pending path-grain selection is retained.

The current contract remains one workspace-scoped pending authored-source snapshot shared by add, status and commit. Cross-invocation persistence is required; repository-document placement is not. The status consumer must move with the cut rather than report an empty role proposal. Stage updates and post-commit clearing are conditioned on their exact observed generations; commit consumes the captured staged snapshot, not a reread of working files. A moved target does not authorize silent rebasing. An unchanged candidate remains distinct from an unavailable stage, and staged-versus-unstaged reporting requires its own working-tree observation.

The 'add is not blocked' claim is now limited to the false prerequisite to build an absent manifest object. Acquisition, path admission, durable selection and conditional updates remain work. The current PR body also records the manifest-root decision as received, with the carrier/codec/consumer transition still pending. That closes the landing-description consistency item.

R2 — THE RETAINED SKETCH IS NOW AN EXPLICIT COUNTEREXAMPLE, NOT THE GOVERNING RESULT CONTRACT

The text immediately introducing the sketch calls it unsound and no longer an instruction. The following explanation names both directions of the mismatch: Persisted carrying a failed RepositorySave and RefusedByMint carrying RepositoryCommitMinted. The governing requirement instead admits only established success evidence for the exact operation into a success outcome, actual failure causes into refusals, and an explicit unestablished outcome where application cannot be determined.

The section expressly states that neither this coproduct nor the old composition survives. It also preserves the distinction between an ordinary successful save and a conditional staging/publication update: narrowing the payload alone would not establish the latter. A replacement result type is appropriately left undesigned; this documentation repair does not need to implement one.

NONBLOCKING RESIDUE, RECORDED RATHER THAN OVERLOOKED

The earlier paragraph still literally says 'ScmWriteOutcome is still the right shape for the verb's result'. That sentence was not removed. I do not endorse it as approval of the retained sketch: the sketch's own explicit supersession, counterexamples, replacement governing requirement and 'What does not survive is this coproduct' statement now settle its disposition. The current PR body likewise rejects it. Reconcile the stray endorsement when next editing this passage; it is editorial residue, not a remaining implementation prescription or a reason to demand another blocking round. This approval does not certify the old carrier as sound.

The staging explanation is also scoped to selection authority: object insertion alone does not record, revise or clear which paths are selected for the next commit. It is not a theorem that an object store can never hold manifests or workspace state, nor does absence from the current repository document prove that no other persistence location is possible.

NEXT-CUT BOUNDARY

The separately accepted CorpusManifestTarget direction remains consumer-derived: a publicly constructible manifest requirement resolved against the actual store before retaining the found record's manifest reference. No unchecked target-to-reference conversion, no observation state inside the target, and no widening of semantic edges is authorized. The existing lookup distinguishes Found from Absent, IsSemanticNode and IsAuthoredSource; all of those outcomes must reach the new mint's appropriate decisions. A held manifest does not by itself prove source completeness or publication admission. None of that implementation is part of this PR.

EVIDENCE AND DISPOSITION

No typecheck, witness, filesystem operation or publication has been independently executed or credited in this review. No execution is required to close these documentation findings; repository-required checks remain separate conditions of landing. No final result schema, staging actuator, manifest-target implementation, conditional writer or structural merge solver is requested here.

Review 5120884296 can be dismissed because its findings are answered by the inspected content, not because a push made its head older. Approved to land this exact head; no further blocking document rewrite is requested.

R1 and R2 are closed by inspected documentation at 10836c2, recorded in APPROVE review 5120893878. The two obsolete recipes are explicitly superseded at their locations; the unsound outcome sketch is expressly rejected and replaced by the exact-operation success/refusal/unestablished-evidence requirement. The lingering earlier endorsement sentence is identified as nonblocking residue, not approved as a schema. The PR description now distinguishes the received manifest-root decision from pending implementation. Dismissal is based on repaired content, not head movement. Approval applies only to this documentation head and does not waive repository checks or authorize any implementation.