[joemooney]

M1 slice 2: holds on the gate ledger, single writer (Refs STORY-B6B1278890774EF993D2EBC327862470-080)

Slice 2 of 3 of the M1 gate ledger. Slice 1 (verdicts on the ledger, with dual write) shipped as #3090. This slice puts holds on the ledger and removes the dual write. Carry-over proof events (AC4) and superseding TASK-1992/TASK-1316/STORY-1375/TASK-2008/STORY-1683 are slice 3, the final slice.

What changes

• AC3, holds: hold, hold-release, release-authorized, release-withdrawn and label-sync events (gate/ledger/hold.rs) replace the .aida/merge-holds/PR-N markers and the .aida/merge-hold-clearances/ files. A release appends an event and never deletes evidence. The ship-held mandate, aida awaiting, the merge-hold-gate label sync and rework auto-release read holds through the ledger fold, with read-through of existing legacy markers.
• Single writer:
  • Verdict writers (write_verdict_object_for_spec, close_verdict_on_merge, the archive writer) are ledger-only. The per-commit archive write and the sidecar migration are deleted.
  • Hold writers write no marker files. local_hold_path, local_clearance_path, clear_hold and superseded_clearance_path are deleted.
  • Per the round-2 ruling, collect_standalone_verdict and adopt_direct_write stay. They append ledger events only.
• Tie-break: the same-round tie now goes to the ledger. Spec-only verdicts also go onto the ledger: through the spec index when it names a change, otherwise to by-spec/<SPEC>.jsonl (ruling (a) accepts this for slice 2).
• Ratchets: the verdict-writer and hold-writer baselines in legacy_gate_writers_do_not_gain_call_sites are 0, and the merge-holds / merge-hold-clearances literal baselines are 0.
• Ruling (d): a collected untyped legacy marker becomes a typed Rework hold (legacy=false).

For the reviewer

• New gate/hold_collect.rs: the reviewer-worktree cleanup in gate/dispatch_* reaches hold collection only through this module. It is registered as a gate kernel in story_1888_gate_paths_tests::GATE_KERNELS, since it reaches outside gate/ only through gate paths.
• Authorization voiding moved out of the fold:
  • A placement through merge_hold_core (append_hold_record) now appends an explicit release-withdrawn before its hold when an auto-release authorization is pending. This is the legacy "placement removes the clearance file" behavior.
  • A bare hold event no longer voids the authorization in ledger::hold::fold. The release path's second pass refuses the changed hold and posts "authorization withdrawn" on the PR (TASK-1757).
  • The earlier blanket void in the fold hid a fail-open in the matrix test: a raw replacement whose pr field changed was appended to the other PR's ledger, so PR 37's hold looked unchanged. The fixture now appends that raw replacement to PR 37's ledger, as the legacy fixture rewrote PR 37's marker.
• task1801_record_refuses_to_rewrite_a_refused_archive_entry: one retained-history read moved from RetainedHistory::beside to RetainedHistory::projected.
• same_sha_recorded_twice_is_idempotent: the original assertions are kept: one commit in the per-sha view, and no manufactured rounds. An "exactly one ledger event" count, added during retargeting, was dropped. A re-record stays its own event, because the phase-3 handshake counts only records written during the review session, so deduplicating re-records would fail a fresh review closed.
• Retargeted reads: tests that read legacy files now read the ledger (current_verdict_body, verdict_events_at, ledger::hold::state). The fail-closed expectations (TASK-1801, bug_775 phase-3) are unchanged. Fixtures that hand-write a legacy file now create its directory first.

Tests run in this seat

env -u AIDA_CONTAINED cargo test --profile agent -p aida-cli-lib --lib -- gate:: drain_gate pr_merge_gate gave 1063 passed. All remaining failures are listed below. Follow-up targeted runs covered the fixed tests (rework_hold_release, gate::ledger, review_verdict, gate::mandate, story_1888, bug_775, task_1460, review_verdict_display), plus wider filters (phase_gates, human::, status_awaiting, pr_claim, lane_liveness, doctor, merge_hold). make check-ci-fast passed.

Not runnable in this sandboxed seat (CI is the gate for these):

• Every test that panics at test_env.rs:93 ("BUG-1642: could not install the hermetic test HOME: Read-only file system"):
  • drain_gate::bug_2139…unresolved_root_refuses_before_queue_or_phase_mutation
  • five story_1917_dispatch_flow_schedule_tests (native_pr_ship_*, native_train_held_member…, r5_dispatch_tick…)
  • story_1698_mandate_tests::{bug_2179_…, task_1790_exclusion_special_files…, task_1951_store_history_walk…}
  • task1801_replacing_validated_verdict_with_fifo_reads_open_inode
  • bug_2123_tests::production_auto_release_reads_the_forge_hold_verdict_and_store
  • five solo_review tests
  • four phase_gates::real_phase_driver_wiring_tests
  • three doctor_cmd tests
• task_2344_q1_failed_rollback_is_fenced…: a dispatch tick lock held by another process in this seat.
• bug_1749_doctor_toon_tests::*: needs a built aida-cli binary.
• a_fresh_refusal_after_a_close_is_not_closed: passed in the first run. It failed in one later run because another test in the same process set AIDA_CONTAINED=1 at the same time. That is a pre-existing race: 46 tests on main set that variable in-process, and slice 1 shipped green with them.

🤖 Generated with Claude Code

[joemooney]

Review: PR-3106 — round 1, RequestChanges at 05a4f4d

Spec	Verdict	Evidence
STORY-…-080 (M1 slice 2)	⚠️ PARTIAL	Holds and verdicts are on the ledger as briefed. CI is red, an uncontained reviewer's verdict is not collected, and the new launcher-armed refusal hold has no test.

Acceptance status (slice 2 scope)

• AC1 ledger events: hold and hold-release carry the head and patch identity. Verified by a_release_appends_and_keeps_the_hold_as_evidence. The mandate-cover and carry-over events are not applicable here because they belong to slice 3.
• AC2 change as the only key / spec index: verified by a_pr_keyed_verdict_write_lands_on_the_change_and_a_spec_keyed_one_on_the_spec. Unbound by-spec entries are accepted under ruling (a).
• AC3 a release appends and never deletes; the mandate and awaiting read the ledger: verified by a_release_appends_and_keeps_the_hold_as_evidence and a_cleared_hold_is_read_from_the_ledger_without_a_marker.
• AC4 carry-over: not applicable because it is slice 3.
• AC5 only uncontained writers append: partial. The contained refusal is tested (a_contained_writer_is_refused_and_leaves_nothing_in_its_worktree). An uncontained standalone reviewer's verdict is no longer collected, and the hand-off file stays in the seat clone.
• AC6 read-through, old writers deleted, ratchet: verified by legacy_markers_are_read_through_until_the_ledger_answers, verdict_and_hold_writers_write_no_legacy_file, the writer ratchet with its baselines at 0, and the merge-holds / merge-hold-clearances literal baselines at 0. Integration tests and agent templates still read the legacy files.
• AC7 lives in gate/: hold_collect is registered in GATE_KERNELS. Unverified until CI's gates shard finishes.
• Brief item 1 (hold readers on the ledger, rework release through the projection): verified in the code and by the retargeted story_1687 and bug_2186 tests.
• Brief item 2 (single writer; collect and adopt append to the ledger only): verified by the code, with the collection gap below.
• Brief item 3 (ledger tie-break; spec-only verdicts on the ledger): verified by the_ledger_answers_a_read_only_where_the_file_knows_less.
• Brief item 4 (ratchets at 0): verified.
• Brief item 5 (fail-closed expectations unchanged): the bug_775 phase-3 and TASK-1801 expectations are untouched in the diff. Their pass/fail is unverified because the lib shards are red.
• Ruling (d) (a collected untyped marker becomes a typed Rework hold): verified by a_collected_untyped_marker_becomes_a_typed_rework_hold.

Findings (complete list; all are in the verdict record)

1. [major][CI] CI (cli-0), CI (lib-0) and CI (lib-1) failed in run 40964841522. These shards were green on main. The job logs cannot be read from the reviewer seat. The likely cli-0 cause is aida-cli/tests/bug_2187_reviewer_record_identity.rs:199 and bug_2201_outside_project_reviewer.rs:332, which read the legacy .aida/review-verdicts/PR-N.json that slice 2 no longer writes.
2. [major] collect_standalone_verdict (gate/review_verdict.rs:2372) collects only from files. An uncontained reviewer's aida review record now writes only to the ledger, so the launcher reports NoReviewVerdict and skips arm_collected_refusal.
3. [major][strict-tier] No test covers arm_collected_refusal, which is the contained reviewer's refusal path now. The tests for the old fallback were deleted. The source-order test still pins only adopt_worktree_hold.
4. [major][templates] Two prompts still read the legacy files: templates/dispatch/take-prompt.md:3 (a rework implementer reads review-verdicts/PR-N*) and aida-review step 6a ("Read back AIDA_REVIEW_VERDICT_FILE"). Stale discipline docs: workflow-patterns.md:2186, git-sync-and-review.md:565/577, machinery-glossary.md:279/312 and docs/autonomous-drain.md:115.
5. [minor] verdicts_for_sha keeps the last event per key in key-sorted order, not the newest by (at, seq).
6. [minor] mandate_ship.rs:542: when the release fails, the code still posts the "released" comment and emits a placed:false event, while the ledger hold stays open.
7. [minor] record_label_state appends a label-sync event even when the state is unchanged, so the ledger grows without bound.
8. [minor] The new contained checks use aida_contained_env().unwrap_or(false), so a malformed AIDA_CONTAINED is treated as uncontained (contrary to BUG-2146).
9. [minor][AC5] The contained hand-off verdict file is left in the seat clone after collection.
10. [cosmetic] The hold lock path moved (merge-hold-locks/PR-N.lock → <ledger>/PR-N.hold.lock). An older binary and this one do not lock each other out during rollout.

Recommendation: fix 1–4 in one rework push. 5–10 are small and can be bundled with them.

[joemooney]

Round-1 rework (ec65dd9): all ruling items in one push

#	Finding	Fix	Test
6 (blocking)	Launcher collect refused as "contained" from an uncontained shell	Cause: aida queue work resolves containment from the [agents] contained knob, and apply_queue_work_contained_env set_var'd AIDA_CONTAINED=1 on the launcher's own process for its reviewer child. The fix is session::set_contained_env_for_children, which keeps the inherited value. The ledger checks session::process_contained(), which answers for this process. A contained parent stays contained.	the_launchers_export_for_its_reviewer_does_not_contain_its_own_collection
2	Uncontained reviewer's ledger-only verdict was not collected	collect_standalone_verdict also takes the change's newest ledger record written after the launch, under the same PR, head and child-grant checks. The caller then arms the refusal hold from it.	an_uncontained_reviewers_ledger_only_refusal_is_collected_and_holds_the_pr
3	arm_collected_refusal untested	Split into place_collected_refusal (the ledger hold) and the label sync	a_contained_reviewers_refusal_is_armed_by_the_launcher_and_its_hand_off_removed
4	Templates and docs read PR-N.json	take-prompt, aida-review step 6a, workflow-patterns, git-sync-and-review, machinery-glossary (plus the generated docs/cli/12-glossary.md) and autonomous-drain now point at aida review verdict PR-N --json or the ledger	check-templates, glossary gate
5a	A failed mandate release still posted "released"	Returns before the note, comment, event and unlabel	— (needs a forge mock; the code path is a plain early return)
5b	Malformed AIDA_CONTAINED read as uncontained	process_contained treats a malformed value as contained (BUG-2146)	same test as item 6
5c	Hand-off file left in the seat clone	Worktree candidates are removed after a successful collection	the item 3 test
5d	verdicts_for_sha ordered by key	Ordered by ledger (at, seq)	verdicts_at_a_commit_keep_the_newest_record_per_key_in_ledger_order
1	Red cli-0, lib-0, lib-1	cli: bug_2187 asserts the ledger, and passes locally. bug_2201 needed no change: it failed on item 6 and passes now. lib: bug_1452, story_1417, story_1655 (isolated), task_1458 x2 (isolated) and the orchestrator_leases forge-seam fixtures were retargeted to ledger state, with no assertion removed. story_1687 auto_release_* raced sibling tests that set AIDA_CONTAINED; in the test binary, process_contained now reads the env only under the env lock.	see below

Local runs (this seat cannot read CI logs):

• Full aida-cli-lib --lib run, then targeted reruns: gate::ledger bug_2149 review_verdict merge_hold rework_hold_release containment pr_merge_gate drain_gate bug_1452 story_1417 forge_seam gave 417 passed and 21 failed.
• Of those 21, 17 are isolated (BUG-1642) tests. Re-run in child mode with a writable TMPDIR, plus the 3 retargeted isolated tests, 19 of 20 pass.
• story_1655 solo_self_approval… fails here only because its handshake anchor resolves to the real primary checkout, which is read-only in this seat. CI runs it.
• The other 4 of the 21 are contained launch refused … not a seat clone the host recorded, caused by this seat clone's own .git layout: compete, session spawn, bug_2166, bug_2171.
• cargo test -p aida-cli --test bug_2187_reviewer_record_identity --test bug_2201_outside_project_reviewer gave 4 passed.
• make check-ci-fast passed.

Accepted per ruling: item 7 (label-sync growth) and item 10 (hold-lock path during rollout).

[joemooney]

Round-2 rework (5fb1323): both red tests fixed

Root cause. The ruling suspected alias keying and the refusal projection. Neither caused these failures. CI run 40976271233 shows both tests failing where they open the legacy .aida/review-verdicts/<KEY>.json file, which slice 2 no longer writes (the writer baselines are 0).

• story_078 …node_alias_review_record_keys_the_verdict_under_the_canonical_id failed with no verdict under the canonical id: []: the legacy directory is empty.
• closed_refusal_does_not_count_as_outstanding panicked at :3684:47, the read_to_string(verdict_path).unwrap() in its fixture setup, before the projection is ever asked.

Deviation from the literal ruling. Keeping these tests byte-identical would mean writing the legacy file again. Slice-2 acceptance forbids that ("the story cannot complete with dual writes"). So I applied the 13:40 rule instead: retarget to the equivalent ledger state, and weaken nothing.

• story_078: the canonical-id file check now points at the gate ledger's by-spec/<CANONICAL>.json index entry. A record keyed by the alias would put STORY-HOME-001 there instead, so the check still proves canonical keying. All other assertions are unchanged: a ledger file names the canonical id, no file anywhere under .aida names the alias, and reading back by the alias still works.
• closed_refusal: the fixture closes the refusal through close_verdict_on_merge, which is the BUG-1529 path and now appends to the ledger, in place of hand-editing the file. The AwaitingMerge assertion is unchanged.

No production code changed.

Validation limits in this seat. This sandbox mounts the cargo build-slot lock directory (/run/user/1000/cargo-slots-1000) read-only, so every cargo command waits forever for a build slot. Neither targeted run could execute here, and the cargo-backed gate in make check-ci-fast (no-default-features check) failed on the same cause. Because that runner stops at the first failure, no other fast gate ran either. Locally verified: rustfmt --check on both files. CI is the gate for both tests.

[joemooney]

Update: the first targeted run eventually got a build slot and finished. cargo test --profile agent -p aida-cli-lib --lib closed_refusal_does_not_count_as_outstanding → 1 passed. The test binary was compiled from the edited fixture; the old fixture panics at :3684:47 before the assertion. The story_078 cli test still has not run locally; CI is the gate for it.

[joemooney]

Review: PR-3106 (round 3, head 5fb1323)

Spec	Verdict	Evidence
STORY-…-080 (M1 slice 2, Refs)	✅ PASS	All slice-2 acceptance items and every round-1 and round-2 ruling item are verified (detail below)

Slice-2 acceptance

1. Holds are on the ledger, and releasing a hold appends an event. Verified by merge_hold_core / story_1687 / bug_2186 tests. rework_hold_release now reads through ledger::project (rework_hold_release.rs:146,654).
2. There is one writer. The ratchet legacy_gate_writers_do_not_gain_call_sites (gate/ledger/tests.rs:1402) checks that every writer in the GONE list (including local_hold_path and local_clearance_path) has 0 call sites. verdict_and_hold_writers_write_no_legacy_file proves it at runtime.
3. newer_of gives a same-round tie to the ledger (review_verdict.rs:967-992). Spec-only records use the by-spec index; story_078 covers this.
4. Writer baselines are 0. write_verdict_atomic is allowed in two places, each with a stated reason: the contained hand-off and backfill-shas. Carry-over moves in slice 3.
5. The fail-closed expectations for TASK-1801 and bug_775 phase-3 are unchanged.

Round-1 items: CI is retargeted. The launcher collect now runs uncontained via session::process_contained and set_contained_env_for_children. A malformed AIDA_CONTAINED value is treated as contained (fail closed). A failed mandate release posts nothing. The hand-off file is removed after collection. verdicts_for_sha orders by ledger (at, seq). Templates and docs now use aida review verdict --json.

Round-2 items: The two red tests failed while reading the legacy fixture file in their own setup. The product was not at fault: the panic at queue_work_tests.rs:3684:47 was read_to_string(verdict_path).unwrap(). They now use ledger state: the by-spec index, and the real close_verdict_on_merge path. The assertions are unchanged. The alias still appears in no file, and reading back by alias still works.

AC4 (carry-over), AC7 subsumes: these belong to slice 3 (final slice) and are not applicable here.

CI: run 40981230917 passed at 5fb1323 (cli-0/1, lib-0/1, contracts, rest, gates). merge-hold-gate is red only because of the round-1 hold label, which this approval addresses. Because the spec is keystone-class, a human must clear the hold.

Non-blocking note: read_verdict_for_sha no longer lazily archives the embedded rounds of pre-TASK-1460 legacy files, because the migration was deleted. This repo already ran it (.archive-migrated is present), so nothing is lost here.

[joemooney]

Finding filing pending (the contained reviewer seat cannot write the AIDA store): minor: legacy verdict files that still hold un-archived embedded rounds lose the per-sha lookup in read_verdict_for_sha now that the TASK-1460 lazy migration is deleted (gate/review_verdict.rs ~1877-1915). Suggested route: slice 3's one-time ledger import. Tag from-review:PR-3106.