feat: attempt lifecycle on the MCP surface (start_attempt, capture_candidate, record_review) - #447
Merged
Merged
Conversation
…ndidate, record_review) Refs #409 A host session could create, plan and gate a change but could not start an attempt, capture a candidate or record a review: the contract defined the three operations and types/validation carried their shapes, but no surface exposed them, and every test seeded attempts through the store. Design. The store said everything under attempts/ is create-only; the contract says capture-candidate binds candidate_snapshot_id onto a RUNNING attempt and is repeatable until a proof references the candidate. The contract's own qualifier -- "create-only once finalized" -- resolves it: an attempt is a projection while `running` and history once it is not. The store now allows a write over an existing attempt only while the stored outcome is `running`, pins every identity field (inputs, slice, baseline, started_at, parent/supersedes), enforces the attempt transition table, and compare-and-swaps on the bound candidate (`ifCandidate`). A versioned `revision` on attempts (option a) would let inputs/slice/outcome be rewritten under a valid-looking review; side-car binding records (option b) would give the attempt two candidates and a reader that forgot the helper the wrong one. record-contract.md gains one sentence saying so; no field, enum, code or path was added. Surface. start_attempt captures the baseline and capture_candidate the candidate FROM THE SERVER'S CWD through the same walker the acceptance re-read uses (working-tree.ts `observe`), so freshness parity holds by construction; two passes that disagree record `unstable`. A caller-supplied snapshot/manifest/coverage is refused by name on both. record_review binds candidate_snapshot_id, candidate_digest and input_digest from the stored attempt and candidate and refuses them on the body; remaining_blockers is derived. A clean declared-separate review of an adapter-attested stable candidate finalizes the attempt to needs-human-acceptance. `plan` gains an optional write mode (slices + revision) so the inputs an attempt starts from exist. Derived-field refusal covers all three actions. Readers. lifecycle.ts owns boundCandidate(); gates, acceptance, host-observations, traverse and the MCP re-read resolve the attempt's candidate through it. traverse now advances a running attempt through resume-attempt -> record-observations -> request-review -> address-objections from the records, and stops change-concluded once an accepted approval exists for the bound candidate. Trust unchanged: the registry stays empty, assurance_policy stays verified, no approve/accept path; the only approval in the new test is minted by the existing request_acceptance adapter through the test-only registry seam. Tests: tests/engineering-lifecycle-mcp.test.mjs drives the whole loop over tool calls against a real tmp git repo (git isolation helper first, added to GUARDED_FILES); negatives assert by content. Mutants killed: capture diverging from the re-read walker, re-capture after proof, caller snapshot binding, review taking a caller digest, gate bypassing boundCandidate, store rewriting a finalized attempt.
…uards (review of #447) D1 (high): a schema-valid approval dropped into approvals/ — no stored request, a fixture receipt — made traverse emit stop/change-concluded and made request_acceptance refuse already-accepted. Neither path ran evaluateApprovalReceipt. `judgeApprovals` (lifecycle.ts) is now the one place a file becomes "the candidate is accepted": it reads the stored request the receipt names (readStoredRequest moved here from acceptance.ts, layer 6 -> 4) and runs evaluateApprovalReceipt against the attempt, the CURRENT bound candidate, and the other approvals' nonces. traverse concludes only on a genuine verdict and stops blocked-needs-operator naming a non-genuine file and its codes; acceptance.ts counts only genuine approvals as already-accepted, so a dropped file cannot deny service, and discloses the ignored file (id + codes) in the presentation's limitations and the request_acceptance result. Exactly one genuine approval per candidate still holds. D2 (medium): start_attempt accepted caller-declared brief_digest / plan_digest. `readStoredInputDigests` digests the stored brief.md / plan.md bytes; plan reports through it, start_attempt derives through it and refuses a caller value that differs (naming both) or a change with no stored artifacts; update regenerates brief.md when one is stored; the gate re-reads both and refuses `input-stale` (new blocker code; record-contract.md amended with one row) when the attempt's inputs drifted. No existing code fit: proof-stale is about the candidate tree, attempt-not-ready about record states. D3 (medium): tests pin that a same-context, unstable-candidate, or caller-attested clean review does not finalize; plan write refuses a missing/stale revision and a re-plan of a planned/active change; two racing planners leave one slice set (the change CAS now precedes the slice writes — before, the loser's slices stayed on disk); the walker's unstable detection is driven through an @internal between-walks seam (captureWorkingTreeWithSeams; stripInternal keeps it out of dist d.ts, no MCP argument reaches it). D4 (low): refuseSuppliedSnapshot also refuses tree, capture, candidate, baseline, reread, candidate_reread, working_tree, entries, files; record_review's text and limitations state that reviewer separation is declared, not authenticated. traverse's needs-human-acceptance branch consults the gate and stops blocked-needs-operator with the gate's blockers instead of telling the host to ask a person for refused work. Every change only refuses more. VERIFIED_ACCEPTANCE_INTEGRATIONS stays [], assurance_policy stays verified, no approve path.
test-windows failed on both attempts of the 5cf6c8d run, in tests/state-store.test.mjs:194 ("one more than the largest ticket on the floor"). The test started acquireLock, slept a fixed 100 ms, then read the directory once. On a loaded Windows runner the waiter had not written its ticket yet, so `mine` was undefined. This PR adds lifecycle tests that start many stdio servers in parallel with the other test files, which plausibly adds the delay. The same test passed on main and on 61397f3. The test now polls for the ticket (every 20 ms, up to 2.5 s), and the waiter's timeout rises from 400 ms to 3000 ms so it is still waiting while the test looks. The assertion is unchanged: the ticket is one more than the largest on the floor. core/status.ts is untouched. Not reproduced on Linux: the original and fixed versions both passed 8/8 under 32 busy loops. The fix removes the timing assumption rather than tuning it.
…447) The re-review of 5cf6c8d showed that the sentence 'exactly one genuine approval per candidate still holds' is false when the storage boundary is unprotected. An agent that can write approvals/ can copy a genuine approval under a new id. Both copies then read receipt-replayed, and traverse stops for an operator. request_acceptance discloses the pair and asks the person again, and an accept mints a second genuine approval for the same candidate. Write access to approvals/ already rules out 'verified', so the paragraph now says this is disclosed, not prevented. The change to stop instead of asking again is tracked as a follow-up. Documentation only.
This was referenced Sep 28, 2026
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.
Refs #409
What
A host session can now run the whole supervised loop through
codecarto_changealone:create→plan(with slices) →start_attempt→capture_candidate→record_proof/ingest_observations→record_review→gate→request_acceptance. Before this, the contract named the three lifecycle operations and no surface exposed them; every test seeded attempts through the store. Full write-up:docs/engineering/attempt-lifecycle.md.Design: create-only vs "capture-candidate binds, repeatable"
store.tssaid everything underattempts/is create-only; the contract sayscapture-candidatebindscandidate_snapshot_idonto a running attempt and is repeatable until a proof references it. The contract's own qualifier in § Ownership and layout — "create-only once finalized" — resolves it, and that is what I implemented (option c):running; it pins every identity field (id,change_id,slice_id,inputs,baseline_snapshot_id,started_at,created_at,parent_attempt_id,supersedes_attempt_id→invalid-value), checksrunning → outcomeagainstATTEMPT_OUTCOME_TRANSITIONS(invalid-transition), and compare-and-swaps onifCandidate(the candidate the caller last read;stale-revisionon mismatch). Once the stored outcome has leftrunningno write reaches the file.revisionCAS like change/slice: attempts have norevision(schema amendment), and it would letinputs/slice_id/outcomebe rewritten under a still-valid review or approval — exactly the "attempt re-bound to another candidate or slice, or inputs changed" case the contract's rejection table makes a reader catch.attempt.candidate_snapshot_idis a required field for the review-ready outcomes and every reader, fixture and bundle rule binds to it; a side file would give the attempt two candidates and a reader that forgot the helper the stale one, plus a new record kind/path/bundle rule.invalid-statenaming the candidate and the proof; baseline snapshot is written before the attempt.record-contract.md§ Ownership and layout spelling out therunningexception. No field, enum, error code or path added.Actions (snake_case, additive)
start_attemptchange_id, slice_id, inputs{brief_digest,plan_digest,references}, parent_attempt_id?attempt_id, outcome:"running", baseline_snapshot_id, baseline_digest, attested_by:"adapter", stability, input_digest, limitations— activates aplannedchange /pendingslicecapture_candidatechange_id, attempt_idcandidate_snapshot_id, candidate_digest, attested_by:"adapter", stability, outcome:"running", superseded_candidate_id?, limitationsrecord_reviewchange_id, attempt_id, review{reviewer,objections,summary}review_id, candidate_snapshot_id, candidate_digest, input_digest, remaining_blockers, outcome, finalized— clean declared-separate review of an adapter-attested stable candidate finalizes toneeds-human-acceptanceplangained an optional write mode (slices+revision+acceptance_scenarios) that stores slicespending, movesdraft → planned, writesbrief.md/plan.mdand returns their digests. Withoutslicesit is the read-only brief it was.observe) the acceptance re-read uses — parity by construction. Capture walks twice; disagreement recordsunstable.repository.dirtyistrueon git trees (no git is run; the limitation says so).snapshot/baseline_snapshot/candidate_snapshot/manifest/coverage/repositoryis refused by name on both capture actions (the adapter can always read its own cwd; refusing rather than silently ignoring tells the caller its bytes did not become the record). Thecaller-attested path exists in core for an adapter that cannot read the tree; no MCP action reaches it.ALWAYS_EXCLUDED+ host-declared exclusions. No change/slice field declares exclusions and no host config channel reaches MCP, socaptureScope()returns[]— one function to change when a source exists.record_reviewrefusescandidate_snapshot_id/candidate_digest/input_digest(and the envelope) on the body and binds them from the stored attempt/candidate;remaining_blockersis derived.state,decision,attested_by,outcome,stability,digest,candidate_*,input_digest, …).Readers
lifecycle.tsownsboundCandidate();gates.ts,acceptance.ts,host-observations.ts,traverse.tsand the MCP re-read resolve through it.traverse now emits: no candidate →
resume-attempt; candidate, no proof names it →record-observations; proof, no review of those bytes →request-review; review with open/deferred blockers →address-objections(naming review + blockers; remedy = new attempt); reviewed clean, not finalized →resume-attempt;needs-human-acceptance→request-human-acceptance; accepted approval for the bound candidate →stop/change-concludednaming the approval.capture-candidateadded toTRAVERSE_ACTIONS.Trust: unchanged
VERIFIED_ACCEPTANCE_INTEGRATIONSstays[];assurance_policystaysverified;cooperativeunreachable; no approve/accept path. The only approval in the new test is minted by the existingrequest_acceptanceadapter through the test-only registry seam.Tests
tests/engineering-lifecycle-mcp.test.mjs(git isolation helper first; added toGUARDED_FILES; real tmp git repo,git initonly for the fixture): full loop over tool calls → accepted, exactly one approval, traversechange-concluded; caller snapshot refused on both actions (nothing bound); re-capture repeatable then refused naming candidate + proof; edit-then-revert gives the original digest,ALWAYS_EXCLUDEDapplied and disclosed; capture thenreadWorkingTreeFRESH untouched / STALE after edit (+ gateproof-stalenaming the candidate); forged review digests refused by field, stored review bound to store digests; open blocker doesn't finalize, traverseaddress-objections; derived fields refused ×9 fields ×3 actions; finalized attempt immutable; store pins identity / CAS on candidate. The one observed proof is written the way E05's protected host entry would (MCP proofs areclaimedby contract), and the test says so.Mutants (each reverted after):
ALWAYS_EXCLUDEDcollectSnapshottoo, so the walker mutation alone is masked; noted in limitscandidate_digestboundCandidateGates
build 0 errors ·
npm test1368/1368 ×2 (main 1358; +10) · tarball smoke 12/12 ·git diff --checkclean · leak grep clean ·git status --porcelain .codecartoclean after tests.Review round 2 (5cf6c8d)
judgeApprovals(lifecycle.ts) is the one place an approval file becomes an acceptance: stored request +evaluateApprovalReceiptagainst the CURRENT bound candidate and the other approvals' nonces. traverse concludes only on a genuine one and stopsblocked-needs-operatornaming a non-genuine file + codes;request_acceptancecounts only genuine approvals asalready-acceptedand discloses the ignored file (id + codes) in the presentation limitations and the result. Probe: forged approval → traverseblocked-needs-operator (receipt-unknown-request); request_acceptance proceeds.readStoredInputDigestsdigests storedbrief.md/plan.md;planreports through it,start_attemptderives through it and refuses divergent caller digests (naming both) or a change with no stored artifacts;updateregeneratesbrief.md; the gate refusesinput-stale(new blocker code, one row added to record-contract.md § acceptance requirements). Probe: bogus digests →-32602 inputs.brief_digest "sha256:aaaa…" (brief.md digests to …).captureWorkingTreeWithSeams(@internal, stripped from d.ts) drives the unstable path.tree, capture, candidate, baseline, reread, candidate_reread, working_tree, entries, files;record_reviewdiscloses "reviewer separation is declared by the review's author, not authenticated"; traverse's needs-human-acceptance branch consults the gate and stops with its blockers.ALWAYS_EXCLUDEDmasked bycollectSnapshot, snapshots.ts).npm test1374/1374 ×2; build 0 errors; tarball smoke 12/12; d.ts consumer test passes.Limits
repository.dirtyalwaystrueon git captures (no git run).ALWAYS_EXCLUDEDonly; no host exclusion channel on MCP yet.record_reviewfinalizes only toneeds-human-acceptance; no action emitsready-for-review,failedorblockedyet (the store accepts them via the same running-attempt path).ALWAYS_EXCLUDED-only mutant survived becausecollectSnapshotre-applies the built-in exclusion; walker parity for host-declared exclusions is what the parity test pins.judgeApprovalsjudges against the CURRENT bound candidate only; a genuine approval for a superseded candidate readsreceipt-mismatch(old acceptance is history). A non-genuine file is disclosed and left in place; nothing removes it.planwrite: a crash between the change CAS and the slice writes leaves aplannedchange with no slices, whichplanrefuses to re-plan — operator matter.input-stalecompare is a limitation, not a refusal, when the change has no storedbrief.md/plan.md(store-seeded records); MCP never starts an attempt without them.