test(agents): finish the mutation triage, and record what the score means

28 of 86 guards now caught, from 8 when the campaign started. Four files are
clean: capability, dispatch-policy, thread-status and persona.

The last two additions came from taking the remaining survivors seriously
rather than declaring them acceptable. A malformed parent-report payload is
reachable — the payload is a record of unknown fields and nothing validates
it — so the delivery pass is now shown to survive one, and the guard is caught
when removed.

The orphaned-parent branch beside it turned out to be unreachable, and finding
that out was the useful part: `deleteThread` refuses to delete a thread that
has sub-threads, so no supported operation can produce a report whose parent
is gone. The refusal is what gets asserted. Faking the state to make the guard
look covered would have been the wrong answer.

The plan now records the score and, more importantly, the three kinds of
survivor: unobservable, where removing the guard changes nothing anyone can
see; unreachable, where the store forbids the state; and somebody else's,
where store.test.ts covers the behaviour at record level while individual
field validators can each be disabled with every suite green. Counting
survivors without that split would make the number look worse than it is, and
counting them as covered would make it look better.
This commit is contained in:
yiliang114 2026-09-08 14:11:29 +09:00
parent eba821cd36
commit 192bb98cda
2 changed files with 60 additions and 0 deletions

View file

@ -260,6 +260,24 @@ that has never failed is not evidence.
| `run-workspace-agents-crash.mjs` | A writer SIGKILLed holding the lock: the store stays readable and writes return unaided |
| `fuzz-workspace-agents.mjs` | Random operation sequences against the invariants. 25 seeds × 600 steps — 15,000 operations — with no violation |
How much those checks are worth was measured rather than assumed. Disabling
each of the 86 single-line guards in the subsystem one at a time and re-running
the harness caught 8 at first and 28 now. The remaining survivors fall into
three kinds, and the distinction matters more than the number:
- **Unobservable.** Removing `if (pending.length === 0) return` walks an empty
list to the same end; removing the unset-assignee check re-assigns undefined
over undefined. No assertion can catch these and writing one would be
theatre.
- **Unreachable.** `deliverParentReports` guards a report whose parent thread
is gone, but `deleteThread` refuses to delete a thread that has sub-threads,
so no supported operation produces that state. The refusal is asserted; the
guard behind it stays as defence.
- **Somebody else's.** Most of store.ts's 39 survivors are per-field
validators. store.test.ts covers fail-closed at the record level — checked,
not assumed — but a single field check can be disabled with every suite
still green. That is a real if minor gap.
What none of it covers: the vitest suites, which are larger and still need CI;
partial-write recovery, since the file lock means two writers never touch one
file and killing an idle holder never interrupts a write (`store.test.ts`

View file

@ -43,6 +43,7 @@ export * as view from '${repo}/packages/web-shell/client/components/workspace-ag
export { buildAgentToolConfig, classifyAgentTool, createAgentToolInvocationGuard, THREAD_TOOL_NAMES } from '${repo}/${src}/capability.js';
export { outstandingCloseObligations, acknowledgeCloseObligations } from '${repo}/${src}/thread-status.js';
export { resolveAgentPersona } from '${repo}/${src}/persona.js';
export { deleteThread, enqueueThreadEvent } from '${repo}/${src}/store.js';
export { ToolNames } from '${repo}/packages/core/src/tools/tool-names.js';
`,
);
@ -1756,6 +1757,47 @@ ok(
M.acknowledgeCloseObligations(owesOne, 5, () => true) !== owesOne,
);
console.log('\n17h. a report whose parent cannot go away');
// The `!parent` branch in deliverParentReports guards a state the store
// refuses to create: a thread with sub-threads cannot be deleted, so the
// orphaned-report case is unreachable through any supported operation. That
// refusal is the real guarantee, and it is what gets asserted; the guard
// behind it stays as defence and stays uncatchable, which is the honest
// reading rather than a test that fakes the state to look covered.
const keptParent = await M.createThread(ROOT, { title: 'Cannot be deleted' });
await M.createThread(ROOT, {
title: 'Its child',
parentThreadId: keptParent.id,
});
let deletionRefused = false;
try {
await M.deleteThread(ROOT, keptParent.id);
} catch {
deletionRefused = true;
}
ok(
'a thread with sub-threads cannot be deleted out from under them',
deletionRefused,
);
// The payload, though, is only a record of unknown fields, so a malformed one
// can exist and the delivery pass must not take it personally.
const malformed = await M.createThread(ROOT, { title: 'Malformed report' });
await M.enqueueThreadEvent(ROOT, malformed.id, {
kind: 'parent_report',
payload: { event: 'child_in_review', parentThreadId: 42 },
});
let survivedMalformed = true;
try {
await M.deliverParentReports(ROOT);
} catch {
survivedMalformed = false;
}
ok(
'a report whose parent id is not even a string does not break the pass',
survivedMalformed,
);
console.log('\n18. concurrency');
// Everything above ran one operation at a time, which is the one shape a
// store with a mutation lock is guaranteed to survive. These run together.