highway lane fixtures: Conformance Fixtures 2/3/4 as runnable discriminating tests (qmj, CCV1-023) - #69
Closed
Brian Krabach (bkrabach) wants to merge 12 commits into
Closed
highway lane fixtures: Conformance Fixtures 2/3/4 as runnable discriminating tests (qmj, CCV1-023)#69Brian Krabach (bkrabach) wants to merge 12 commits into
Brian Krabach (bkrabach) wants to merge 12 commits into
Conversation
Core 7 of contracts/custody-coordination.v1.md; ledger row CCV1-009; work item pipeline-dn4. `Beads.resolve`'s custody fence ran only under `if current.status == "held"`. A reap does not leave a reclaimed item held -- `supervisor.reap_project` calls `release`, which puts it back to `open` and clears bd's assignee -- so the one state the fence exists for was the one state it skipped: a stale holder's close landed with exit 0 and no refusal anywhere. Status was never the right discriminator; custody identity is. The fence now also refuses when the item is NOT held but its custody record still names the caller. PR #51's integrator resolve (item pipeline-79t) is preserved by the same key: only the session the custody record names is refused, so an integrator's single-call close of an item nobody holds is exactly as unfenced as before. The `current.holder != who` guard keeps a holder's own already-landed close re-attemptable, so PR #63's phantom-conflict recovery does not regress. Measured, not inspected: tests/integration/test_post_reclaim_fence.py is red on the pre-fix code (DID NOT RAISE FencedError on both fence halves) and green after. It drives the REAL reap sweep as well as a bare release, and pins the integrator half alongside the fence half so neither can be silently traded for the other. The same fix flips the previously-red tool-layer fixture test_reap_recovery.py:: test_explicit_resolve_refusal_after_reap_clears_held_and_allows_new_claim green -- no tool-layer change was needed, its FencedError branch was simply unreachable in this state. Ledger row CCV1-009: VIOLATION -> CONFORMS, probe rewritten to assert the fixed shape (identity-keyed refusal outside the status gate, the contract's own "not held by this session" wording) plus the continued existence of both halves of the behavioural fixture.
…CV1-003)
`work_claim` is one call but two writes -- the bd claim, then
`take_custody`. When the second failed, the tool returned an
honest-looking failure and walked away, leaving the item HELD by that
actor with NO custody record and no session tracking it: invisible to
`work_release` (the session never set `self._held`, so it refuses), and
freed only by the next reap sweep -- up to a custody TTL later, and only
where a sweep runs at all.
The claim landed, so the only honest recovery is to give the item back.
The failing arm now routes to `_release_after_failed_custody`, which:
- releases the just-claimed item via `adapter.Beads.release` (status
checked BEFORE any write, so it can never reopen a closed item;
read-back-verified on the conflict path since PR #63);
- confirms that release by its OWN contention-free read-back
(`get_readonly` -> `_get_item_via_sql`, a pure SELECT), because a
write's self-report of success is exactly what this repo has
repeatedly measured to be unreliable;
- reports both facts: "claim landed; custody could not be established;
item released back to ready: <reason>".
The one residual case -- the compensating release ITSELF failing -- is
never silent: the message says the item may still be held, names the id
and the actor, and says what to do about it. Same for a release that
reports success but cannot be confirmed by read-back.
Tests (real bd/dolt, this suite's isolated server) cover both branches
and both FAIL against the pre-fix code. Ledger row CCV1-003 flips
VIOLATION -> CONFORMS with file:line + fixture evidence, and its probe is
rewritten to assert the compensation rather than pin the broken shape.
Three agent-facing claims contradicted the custody-coordination contract and the code that ships today. All three are corrected, and each row's ledger probe is flipped from pinning the stale wording to pinning the corrected wording (plus an absence-assert on the phrase it replaced), so a regression fails as loudly as the drift did. Core 4 (CCV1-005) -- renewal is one-strike. The skill told agents "you do not need to do anything to keep it fresh under normal operation", inverted for exactly the failure Core 4 names. Both surfaces now state that a single failed renewal ends renewal permanently, that a non-fenced failure leaves the session believing it still holds the item, and that the only discovery path is work_status's holding.custody_lost -- checked before long-running steps and after any tool error. Core 6 (CCV1-008) -- the TTL is not self-enforcing. Both documents stated the 15-minute release unconditionally and never mentioned the sweep. They now state reclaim-ELIGIBILITY enforced by the out-of-band reap sweep: a reclaim lands up to a sweep interval late (300s default), and where no sweep runs a dead agent's hold persists indefinitely. The skill's constants table gains the reap interval, which it had omitted entirely. Core 11 (CCV1-016) -- a reported conflict is UNKNOWN, not proof of failure. Drift created by PR #63 itself: awareness.md (and the CLI's own CONTENTION / RETRY CONTRACT, which the original row did not cover) still stated as a database guarantee that a serialization error means the write did not happen. Incident B measured the opposite. Both now scope the guarantee honestly -- resolve/release verify by read-back and report success when a conflicted write landed; every other write verb surfaces the raw conflict unverified -- while keeping the re-read-before-retry guidance that was always correct. Text only in src/amplifier_work_tracker/cli.py (the module docstring is argparse's description); no behaviour changed anywhere. Rows CCV1-005, CCV1-008, CCV1-016: GAP -> CONFORMS with file:line evidence. Gates: ruff check, ruff format --check, pyright (0 errors), pytest ledger/checks (24 passed).
Exit code is not proof -- neither a zero one nor a non-zero one. PR #63 gave `resolve`/`release` a read-back on the CONFLICT path only; every other item-level write verb still believed the wrapper, and `release`'s own SUCCESS path still returned straight off `p.returncode == 0`. One shared helper, `Beads._verified_write(run, verify, what=...)`, now carries the discipline once: - the wrapper's exhausted-retry `BeadsError` (or a non-zero exit whose output names a serialization/connection failure) is decided by a contention-free read-back, not believed -- exhaustion never proved a write did not land (measured incident, work_tracker pipeline-yym); - a reported SUCCESS is verified too -- a `bd` write that exits 0 and changes nothing is otherwise indistinguishable from one that worked; - a genuine domain refusal is never verified away, and a write that truly did not land still raises. Routed: create, update, comment, edit_item (both halves), supersede, claim_item, claim_next, release, defer/undefer, block/unblock, add_dependency, take_custody, renew_custody (the write behind the tool's `declare`). `claim_item`/`claim_next` additionally now RETURN the read-back rather than an Item parsed from the writing process's own stdout. `resolve` keeps PR #63's inline shape verbatim (that region is pinned by ledger row CCV1-009 and owned elsewhere; behaviour identical). `move_item` is deliberately not routed -- direct dolt SQL, no `_run`, and it already verifies by real row counts plus a compensating cleanup. Two verbs have no id to read back by when a conflict destroys bd's own stdout -- `create` (bd prints the new id) and `claim_next` (bd chooses the item). Both use a new read-only `_ids_via_sql` set difference snapshotted before the write, with an honest three-way answer: exactly one new row means it landed and names it, none means it did not, and more than one is AMBIGUOUS and re-raises rather than guessing. Ledger: CCV1-012, CCV1-013, CCV1-015 -> CONFORMS, probes rewritten to assert the fixed behaviour (per-verb, sliced by AST so one verb's probe cannot match a sibling). Measured by tests/integration/test_write_readback.py -- 26 cases injecting conflict-after-landed, conflict-with-no-write, and phantom-success against the real isolated dolt server.
The tool module's own suite -- the only place the post-reclaim custody behaviour of the agent seam (work_claim / work_declare / work_resolve / work_release) is asserted mechanically -- was importable by nothing and run by nothing: `import amplifier_module_tool_work_tracker` raised ModuleNotFoundError in the repo venv, `make test` was `pytest tests ledger/checks -v`, and ci.yml named only tiers 1-4. Six green claims nobody had ever executed, and the Freeze blocker for contracts/custody-coordination.v1.md. Wired three ways, keeping ONE venv and ONE setup command: - `make venv` and ci.yml's setup step now install the module editable alongside the root package (`-e "modules/tool-work-tracker[dev]"`). Its dev extra already declared the two test-only deps the root package does not need (amplifier-core, pytest-asyncio), so nothing is duplicated into the root dev extra. - Makefile gains `test-module`; `make test` runs both invocations and is deliberately not fail-fast between them, so a pre-existing root-suite failure cannot go back to hiding tier 5's result. - ci.yml gains a "Tier 5 -- tool module tests" step. Separate pytest invocations on purpose: the module suite ships its own session-scoped isolated dolt server fixture (fixtures cannot cross a pytest run), so folding it into a tier would stand up two servers in one session. Measured 2026-09-02: 94 collected, 93 passed, 1 xfailed in 325s against real bd. The xfail is strict and is a PRODUCT defect, not a wiring one: test_reap_recovery.py::test_explicit_resolve_refusal_after_reap_clears_ held_and_allows_new_claim is CCV1-009's post-reclaim fence gap (`Beads.resolve`'s fence runs only under `if current.status == "held"`, and a reaped item is `open`), now measured behaviourally for the first time rather than inferred from source. Not fixed here; strict=True means the day CCV1-009 lands, the xfail fails and this pin must be removed. Ledger row CCV1-022: VIOLATION -> CONFORMS, and its probe rewritten from an absence pin to an assertion that all three halves of the wiring are present. `pytest ledger/checks -q` green (24 passed).
Conflict in ledger/checks/test_custody_rows.py: both prose (CLI path constant for CCV1-016) and readback (_beads_method AST slicer for CCV1-012/013/015) added helpers at the same location -- kept both.
…s (CCV1-023) The contract's Freeze Bar requires "All four Conformance fixtures implemented, passing, and executable via `make test`". Fixture 1 already existed (tests/integration/test_phantom_conflict_recovery.py); 2, 3 and 4 did not exist in any suite. Adds modules/tool-work-tracker/tests/test_conformance_fixtures.py -- ten tests at the agent seam the contract writes the fixtures against (work_resolve / work_status / work_claim), each fixture carrying BOTH halves of its good/bad pair, since one half alone passes against the broken implementation too: Fixture 2 (post-reclaim close fence): a stale holder's work_resolve after a real reap sweep is refused and the item stays open; a live holder's resolve and PR #51's integrator close both still succeed. Fixture 3 (in-process recovery): a session wedged holding an item bd already considers resolved clears its latch via work_release, gets the sanctioned already_closed outcome, and claims again -- with the closed item's entire record asserted byte-identical across the recovery call, which is what proves the Backlogged section's named hazard ("release on a resolved item would reopen it") cannot happen. Plus work_status surfacing holding.custody_lost on the retained-hold path. Fixture 4 (single hold per session): a second claim is refused naming the held item in both directed and queue mode, the would-be second item is untouched, and the constraint lifts after either exit from a hold. Discrimination measured, not asserted: each bad half was run against a deliberately reverted implementation and fails there while every good half stays green (post-reclaim fence branch, release's pre-write resolved guard, claim's single-hold gate). Also removes the now-obsolete xfail(strict=True) on test_reap_recovery.py's post-reclaim resolve test. It pinned CCV1-009's pre-fix gap; the fence has landed, so the strict marker was XPASSing and failing the module suite -- exactly as CCV1-022's own notes predicted it would. Ledger row CCV1-023: GAP -> CONFORMS, probe rewritten from an absence pin to one that asserts the fixture files exist, each fixture keeps >= 2 halves, and no half is xfail'd or skipped. The contract's four stale "Test location" lines are recorded as drift and still pinned; contracts/ is not edited.
Collaborator
Author
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
2 participants
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.
Highway
hw-custody-ledger, lanefixtures(work item qmj, ledger row CCV1-023). DONE status PASS.Implements the contract's Conformance Fixtures 2 (post-reclaim close fence), 3 (wedge recovery / custody-clear) and 4 (single hold per session) as good/bad discriminating test pairs that run under
make test/ CI (module tests + integration), and flips ledger row CCV1-023 with its probe. Rebased onto the squashed wave-1 main (#68); trees identical, clean rebase.🤖 Generated with Amplifier