fix: the operational layer, exercised end to end across a multi-item run

[thedancingdeveloper]

What this is

tests/test_operational_e2e.py: the operational layer run end to end across a
multi-item run. The queue and its leases, the dependency graph, the holds, the
audit store and the reaper had each been tested alone; all of them at once —
over one real SQLite file, with dependencies, a worker killed mid-item,
retries, a question waiting on a person and real threads competing for one
backlog — had never been run.

Written in the shape of test_agent_loop_e2e.py: real SQLite files on disk,
real threads where concurrency is the point, a real git repository where a
worktree is, and the clock injected — the one thing that must not be real,
because AGENTS.md forbids sleeping in tests and a lease measured against a
sleep is a measurement of the machine's load.

Writing it found five defects and the leak recorded as #206. Every one has a
test here that is red against main and green with the fix. Verified by
stashing the src/ changes and re-running: 8 tests fail, 0 after.

The defects

1. claim scanned one page and stopped. ORDER BY attempts, item_id LIMIT 200 made "does the queue have ready work" and "does the first page have
ready work" the same question. A project whose first page was entirely
dependency-blocked was handed nothing at all — permanently, for every worker —
while the one item that would have unblocked all of it sat one row past the
limit. A stalled fleet with a full queue, and nothing in the queue saying why:
the unattended-operation failure this layer exists to prevent. The scan now
walks the pages, keyset rather than offset because rows are retired to
exhausted inside the loop; CLAIM_SCAN_LIMIT still bounds what is in memory
at once.

2. POST /api/work/{id}/retry called release, not requeue.
WorkQueue.requeue exists for precisely this — "a retry that left the count
alone put the item back to pending and watched it return to exhausted
before any worker saw it, while reporting success" — and the fix went into the
queue and never reached the API, which is the only lever an operator has
over a wedged row. It also cleared last_error, the one record of why the item
stopped, and left the durable attempt position, so the retry resumed into the
verdict it was retrying.

3. Nothing ever cancelled a hold. holds.CANCELLED was declared,
documented as "the item was blocked, retried or requeued out from under the
hold", and written by no code anywhere. So a question stayed open after its
item moved on. Three consequences, all asserted: the operator's inbox showed a
question answering could no longer affect; the item's new owner was refused
a question of its own (already has an unanswered question, about an attempt
that no longer exists); and the abandoned hold went on to expire and be
recorded as "nobody answered", which is a different fact from "somebody
cancelled it" and is not the one that happened.

4. GET /api/holds returned entries with no item id. The inbox — the whole
answer to #103 — listed its questions without saying which item each was about.
Hold.as_dict supplies item_id and attempt; HoldView had no field for
either, so pydantic dropped them without a word. Every question readable, none
answerable: POST /api/work/{item_id}/answer needs exactly the id the list
withheld. AGENTS.md treats the API as a contract and the schema as the
documentation, so this is a contract defect, not a cosmetic one.

5. AuditStore.record_baseline bypassed redaction. It writes a human's
free text (label, notes) straight into the store that has no way to remove
anything and that is retained after maintenance thins the primary. The
module's own reasoning is that the filter sits on the only way in "so nothing
added later can route around it" — and this was a second way in.

6. #206. with self._connect() as conn reads like a closing block and is
not one: a sqlite3 connection's context manager manages a transaction.
WorkQueue, SQLiteCommandJournal and AuthorityStore each left a live
handle on every call, and WorkQueue holds a reference cycle (its graph,
attempt log and holds all carry its bound _connect), so the collector only
found it on a gc pass — in a process meant to run for weeks. Proved by counting
/proc/self/fd entries pointing at the database, which is evidence rather than
an assertion about an implementation. The two journals get a _connection
helper that keeps the with conn commit and adds the close, because both open
an explicit BEGIN IMMEDIATE and depend on it.

Closes #206

What was found SOUND, and is now asserted

Reported because "what holds" is worth as much as "what breaks":

• a killed worker returns its item by doing nothing, and the crash costs an attempt;
• a live lease is not stolen, and a heartbeat holds one indefinitely;
• an item that reliably kills every worker ends exhausted, not cycling;
• the late report of a worker that was only slow is discarded, and so is its heartbeat;
• an item with a prior attempt sorts behind fresh work but is not starved by it;
• a dependency is never claimed before its target, and a failed target is not a finished one;
• a cycle is named as a path; an unresolvable target names the id it could not find; an advisory edge never blocks; a cross-project target resolves against the other project;
• D12 holds: a hold keeps the claim, suspends the lease, survives the worker dying (no lease can lapse, reclaim_dead_workers does not touch it), and answering hands the item back to the worker that asked with a fresh lease;
• an unanswered hold returns the item to blocked and never to ready, with the question preserved;
• a hold notice carries no resume token, and a broken notice hook cannot reach the item;
• fifty items, four real threads, one real SQLite file: no item claimed twice, and six workers racing one expired lease produce exactly one owner;
• deleting the queue database — actually unlinked, with its WAL — changes no audited answer;
• the reaper takes only what nobody came back to, never touches a live claim's session, and a session it cannot kill stays on the list for the next sweep.

Pinned, not fixed — said out loud

Both are named in the test module's docstring rather than left in a commit
message nobody reads:

• /api/work/{id}/retry and /block compare a stored lease_until against
  time.time() rather than queue.now(), unlike /api/holds, which uses the
  queue's clock and says why. Identical in production, so the fixture starts
  its clock at the wall clock rather than paper over the difference.
• holds is keyed on (project, item, attempt, asked_at) with asked_at a
  float, so two questions from one attempt within the same clock tick raise a
  bare sqlite3.IntegrityError out of WorkQueue.hold rather than a
  HoldError. Reachable with an injected clock, effectively unreachable
  against a real one.

One further observation, not fixed because it is a taxonomy change and D8 is
open: an item retired to exhausted by the claim scan gets no disposition
and no reason_kind, and empty means "nobody has finished with it yet" —
which is exactly wrong for the one state that says the harness gave up.

Gates

uv run ruff format ., uv run ruff check ., uv run mypy, uv run pytest
— all pass. No gate is weakened; nothing in EXECUTION_PATH learns about a
project, a vendor or a workload.

🤖 Generated with Claude Code