Silent-failure classes found and closed

Every defect on this page shares one shape: the operation reported success and changed nothing, or reported a verdict it had not computed. None was caught by a failing test, because in most cases a passing test was asserting the report rather than the result.

The point of writing them down together is the recognition pattern, not the individual fixes. When a check is cheap and its result is never read, when a parameter is accepted and never consulted, when a test asserts an echo string — that is this shape, and it is worth looking twice.

The seven

1. session task done persisted nothing

task done <index> mutated a WorkItem obtained from one read, then wrote a freshly read context over it — three separate objects, the mutation on one, the write from another. It echoed Marked as done: every time regardless.

Every backlog count this tool ever reported was wrong. Its test asserted exit_code == 0 and that the words "Marked as done" appeared, so the bug shipped green.

Fix: complete_work_item_at — read, mutate and write ONE context inside a single transaction. The test now re-reads context.json from disk.

Follow-on: task done was one-way, and hand-editing context.json is forbidden by the session-CLI rule, so a mistyped index was unrecoverable through any sanctioned path. task reopen now exists, and mark_todo clears done_at as well as state — an item that is open but carries a completion stamp is a different kind of wrong.

2. incremental_sync dropped every cross-module CALLS edge

_sync_calls_edges built its local and import name indices from the same filtered module list it was writing edges for. On a partial resync that subset was the only thing from <module> import <name> could resolve against, so any call whose target lived outside the subset resolved to nothing and the edge was never written. No error, no warning — a graph that quietly disagrees with a full sync.

before  incremental_sync(['mod_b']) CALLS edges: []
after   incremental_sync(['mod_b']) CALLS edges: [('mod_b.caller_cross', 'mod_a.helper')]

Fix: context_modules carries the full project's modules as read-only resolution context; written edges stay restricted to the filtered set, and the None default keeps full_sync byte-identical.

3. ConstraintEvaluator accepted a Prolog bridge and never read it

self._prolog had exactly one reference in the module: the assignment. Every satisfaction verdict came from a hardcoded if/elif ladder over ConstraintType, while SPEC-007 Key Principle #1 states without qualification that "Prolog decisions are final and cannot be overridden."

A parameter that is accepted, documented, and never consulted is worse than an absent one — it reads at every call site as though the principle is honoured.

Fix: the ladder is deleted, not shadowed. prolog/constraints.pl decides, cut-free per ADR-0025, reproducing the old decision table exactly — all 24 pre-existing cases pass unchanged, which is the proof it was relocated rather than rewritten.

Gotcha worth remembering: PrologEngine.query_once() returns a dict, never None, even for an ordinary failing goal — janus's {'truth': False} is stripped to {}. So if engine.query_once(goal) is not None: is always true and would make every constraint satisfied. Use bool(list(bridge.query(goal))) — [] on failure. A tree-wide AST scan confirmed no other site carries this assumption, but it is easy to reach for.

4. U:Do had no :Volk node, and the docstring called that intentional

PipelineRunner._register_volks instantiated 9 of 10 built-in volks; DocumentorVolk was never constructed. sync_volks_to_kg derives its rows from status_report(), which iterates self.volks — so a volk absent from that dict can never reach the graph no matter how much traffic names it. 43 CXPPacket and 40 Message nodes did.

Two independent "which volks are dispatchable" tables had diverged: pipeline_router's AGENT_CLASS_MAP listed U:Do; _register_volks did not, and its own docstring recorded "only the 9 built-in volks" as if by design.

Fix + the test that matters: the regression test derives the expected set from AGENT_CLASS_MAP, precisely so it cannot vacuously pass by iterating the incomplete dict it is meant to police.

5. Unlabeled MATCH (n {id}) scanned the whole graph

Generic get_node/update_node/delete_node emitted an unlabeled match. Measured on the live 35,789-node test KG: 35,814 dbHits → 6.

The footgun that labeling exposed: get_node_label had no entry for NodeType.MODULE or NodeType.PACKAGE — the two most common code-structure types in this KG, written by sync.py through string literals that bypass the function entirely. Both fell through to a "Node" default, and nothing carries a :Node label. Once a node_type= parameter made this reachable, passing the natural value "module" would have matched zero rows — a fast wrong answer replacing a slow correct one. A completeness test now fails if any NodeType member lacks a mapping.

6. The test generator could not regenerate

--package did not exist (the flag the backlog quoted errored outright), and analyze_directory returned [] for a nonexistent path with exit 0 — "Generated 0 test files" was a silent success. Generated interface-tests also pinned exact signatures, so adding **kwargs broke ~130 of them.

Fix: fail loudly on a missing source path; the signature assertion now accepts compatible changes and still rejects removed, renamed and newly-required parameters — proven in both directions.

7. resume() re-routed a review pipeline as a feature

Nothing persisted the original work_type, so PipelineRunner.resume() carried a literal "feature" default. A resumed documentation or review pipeline silently took feature's stage order unless the caller happened to pass the right value back in.

Fix: save_stage persists work_type; stored_work_type reads it; resume() prefers stored over default, and an explicit argument still wins over both. Optional throughout, so records written before the change still load.

Infrastructure: the orphaned-waiter leak

Not a silent failure in the codebase, but the same shape in the tooling — and it actively obscured the others.

An agent backgrounds a waiter:

until grep -qE "AST pipeline complete" some.output; do sleep 5; done

The file it watches stops being written and never contains that string. The loop has no exit path. The agent finishes, and nothing is left to kill it.

Measured: six alive at once, the oldest having waited 3 days 4 hours. Two had an empty body (do :; done) burning a core each, which drove load average to 27.6 on a 6C/12T box and left a gate run silent for four minutes — read at the time as a hang. They also keep a finished task rendering as still running, so a completed agent looks busy.

Fix, in two halves because neither closes it alone:

Hook Event Role
no-unbounded-wait.sh PreToolUse(Bash) Refuses to create a loop with no exit path. Denies both shapes and names which fired. Allows timeout-bounded waits, counted loops, and ordinary while read.
reap-orphan-waiters.sh SessionStart Clears what already leaked and anything spawned where the guard does not run. Reaped all six on first run. Only touches a process matching an unbounded shape AND past an age gate; spares young waiters, since a live agent may be seconds from satisfying one.

Polling was never needed: a backgrounded call already notifies on completion and returns its output path.

Both guards test their input with quoted strings masked (hook_bare). That mention-vs-invocation defect has already bitten force-test-observation.sh and no-xdist.sh once each — a command that merely quotes a wait loop must not read as running one — so a test pins it.

The backlog itself was lying

Of the first eight items dispatched from the backlog, three were already fixed — one of them on 2026-07-12, proven by swapping the pre-fix file back in (13 failed vs 111 passed). A builder spent a full run establishing that.

A broader audit found the same rot at scale: 60 requirement ids are claimed by two different specs, because spec-001 numbers its ids by user story and borrows namespaces 002..010 while every other spec numbers by spec. check_req_format.py passes throughout — it validates shape, and never asks whether an id means one thing. Live consequence: kernel/pipeline.py's boot cites REQ-008-020 meaning spec-001/US-008 (audit logging), not spec-008 (the knowledge graph); only its docstring disambiguates.

Standing practice from this: verify a backlog item against the tree before dispatching anyone at it. An audit costs minutes; a builder proving a months-old fix costs a full run and produces nothing.

Recognising the shape

Related: [[testing-standards]], [[decisions]], [[blockers]].