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, neverNone, even for an ordinary failing goal — janus's{'truth': False}is stripped to{}. Soif engine.query_once(goal) is not None:is always true and would make every constraint satisfied. Usebool(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
- A parameter accepted, documented, and never read.
refs <attr>returning only the assignment is the tell. - A test whose assertions are all about the report — exit code, echoed string — and none about the resulting state.
- Two independent tables of "what exists", diverging silently (
AGENT_CLASS_MAPvs_register_volks). - A default that is a plausible value rather than an absence (
work_type="feature", label"Node"), so the wrong answer looks like a real one. - A lint or check that runs and whose result nobody reads —
trace-lintfinds real errors and is not in the gate battery. - A comparison that cannot be false (
query_once(...) is not None).
Related: [[testing-standards]], [[decisions]], [[blockers]].