From 6ef8022f607b7f4d9e886cb437581b59c10b97ac Mon Sep 17 00:00:00 2001
From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com>
Date: Tue, 25 Aug 2026 01:25:51 -0700
Subject: [PATCH 1/5] =?UTF-8?q?docs(review):=20rule=2019=20=E2=80=94=20whi?=
 =?UTF-8?q?ch=20way=20does=20this=20guard=20fail,=20and=20who=20hears=20it?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Five sites on main independently choose the noisy degraded path and none
cite each other: the #1115 ledger-row ruling, ThreadUserState's
cutoffUnknown CASE arm, resolveBotUserIds' empty-map catch,
reRootOrphanedChains' never-fail-the-delete wrapper, and
BARE_RUNTIME_ARTIFACTS as an observed set. Each argues the principle
locally, which is why the repetition is invisible.

The rule carries an audience clause the bare form lacks. The attempts
cap in agentEventService reads as a counterexample and is the sharpest
confirming instance: the retire pass exists to make exhaustion visible,
transitions to status 'failed' and warns — and list() hardcodes
status 'pending', so the agent owed the event is never told. Direction
correct, audience operator-only. Only a question that asks who hears it
surfaces that residue.

Stacked on #1209 (rule 18); merge that first.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 docs/development/review-checklist.md | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/docs/development/review-checklist.md b/docs/development/review-checklist.md
index 4a3022f25..c7d5582b8 100644
--- a/docs/development/review-checklist.md
+++ b/docs/development/review-checklist.md
@@ -56,3 +56,7 @@
 17. **A mutation test proves a term matters to the suite. It cannot tell you the suite's shape is real.** Deleting a term and watching a test go red is the standard way to show the term is load-bearing, and it is sound *within* the harness — which is exactly the boundary that gets skipped, because the red test feels like production talking back. It isn't: the harness chose the input shape, so a mutation only ever reports which terms that chosen shape reaches. If the shape is one production never produces, every result is valid and every conclusion is about a world that does not exist. **The check that closes it is one line and orthogonal to the mutation: grep what the real middleware assigns, and confirm the harness produces that same shape.** Two tells that it does not, both visible without running anything: the fixture *constructs* the auth object inline rather than calling the middleware, and — the loud one — the suite `jest.mock`s the real auth middleware into a pass-through, so the shape under discussion is not merely unexercised but deliberately excluded. Corollary for the fix: **a term found dead this way is often dead in the safe direction, so confirm what reviving it would do before calling it a bug.** And a rider on the closing check itself, because it has its own failure mode: **bind the projection to its own query, not to line proximity.** `grep '\.select(' ` near an assignment finds the wrong query whenever two run in the same block — a `.select('_id')` belonging to an interleaved `Pod.find` reads exactly like a projection on the `User.findOne` thirty-odd lines above it, and turns a live term into a confidently-reported dead one. Read the call the projection is chained to. *(Earned: 2026-08-22, the `tasksApi` identity terms. `resolveAgentInstanceId` reads `req.user.isBot`, while `agentRuntimeAuth` assigns only `req.agentUser` — so on every real agent call the term is undefined and `claimKey` falls through to the bot User's ObjectId. Dropping `req.user?.username` turned exactly one test red, and that was reported as the term being live; the harness sets `req.user = { id, _id, username, isBot }` from a test header and mocks `agentRuntimeAuth` to a bare `next()`, one line below. The reviewer had quoted that shim in their own review of the same file. *(Rider added 2026-08-23 by @sprint-review, from a near-miss running this very check against the fix that earned the rule: `agentRuntimeAuth.ts:98` is `.select('_id').lean()` on an interleaved DM-pod `Pod.find`, thirty-seven lines below the `User.findOne` — read as the User projection it would have condemned `req.agentUser?.username` as dead on arrival. Both `User.findOne` calls are in fact unprojected, so the term is live.)* Reviving the term would have made things worse, not better: `resolveHolder` only loads the holder's User row when `claimedBy` is a 24-hex ObjectId, so a readable instanceId key would null the holder, empty the `agentName` narrowing on the install lookup, and degrade the lease-rescue liveness check to buy a prettier column.)*
 
 18. **A source assertion is the right instrument when the claim is about absence — because absence of code cannot be demonstrated by execution.** The default suspicion of a test that greps the source instead of running it is correct and should stay: it pins text rather than behaviour, it goes red on a rename that changed nothing, and it is the lazy substitute for the behavioural test somebody did not want to write. But there is one class it is the *only* instrument for. A behavioural test can show that a branch produces the right answer; **it cannot show that a branch is gone.** Dead code is invisible to execution — it never runs, contributes nothing to any assertion, and sits there for the next reader to revive in good faith because it looks like a real path. So the question that separates the two cases is not "is this test grepping?" but **"is the property behavioural or structural?"** If the claim is *this input yields that output*, run it. If the claim is *this call site no longer exists*, *nothing outside this module reads that field*, or *no path constructs the deprecated shape*, then execution is silent on it by construction, and a source assertion is the instrument rather than the shortcut. Two riders. First, **say which it is in the test itself** — an unlabelled grep-test reads as the lazy kind to every future reviewer, and gets deleted by someone applying the default suspicion correctly. Second, **an absence assertion needs a positive control like any other negative**: a grep that matches nothing because the pattern is wrong is indistinguishable from one that matches nothing because the code is gone (see rule 12). *(Earned: 2026-08-23, #1149/#1162 — the `backfillPending` absence check was defended as "defensible as a barrier", which is a verdict about this one test rather than a rule anyone could reuse. The generalisation is what makes it decidable: it also explains why converting the ledger-read regex to an executing test was a strict gain in the same review — that property *was* behavioural, and the `catch` branch had no executing coverage at all.)*
+
+## Failure direction
+
+19. **When a guard cannot do its job, it must fail toward the noisy outcome — and "noisy" names an audience, not a log line.** Every guard has a degraded path: the projection came back empty, the lookup threw, the input is a value the guard has no theory of. That path picks a direction, and the direction is a design decision that gets made silently because each site argues it locally and each argument is good. The reviewer's question is one breath long and works on any guard on any surface: **which way does this fail, and who hears it?** Both halves are load-bearing. The first sorts the guard into over-delivering (a noisier pod, a redundant wake, an unnecessary expand) versus under-delivering (a message nobody sees, a thread silently collapsed, an event retired unread) — and under-delivering is the one that is invisible in production by construction, because the evidence of the failure is the thing that failed to appear. The second stops the rule being satisfiable by a `console.warn`: a failure that is loud to the operator and silent to the party that was owed the outcome is still a silent failure, wearing a receipt. This codebase already follows the rule in at least five independent places, none of which cite each other: a missing ledger row expands rather than collapses (#1115); `cutoffUnknown` resolves to `false` so an unresolved cutoff never collapses a thread (`ThreadUserState.ts` — the CASE-arm ORDER *is* the ruling); `resolveBotUserIds` returns an empty map on failure, so the narrowing degrades to unscoped delivery rather than dropping wakes (`agentMentionService.ts` — "losing the narrowing is a noisier pod; losing the wake is a message nobody sees"); `reRootOrphanedChains` is wrapped so repair can never fail the delete (`models/pg/Message.ts`); and `BARE_RUNTIME_ARTIFACTS` is an observed-values set rather than a shape heuristic, so an unsighted artifact posts (`agentMessageService.ts`). Five correct calls made independently is not a pattern the codebase is protected by — it is five coin flips that happened to land the same way, and nothing stops the sixth landing backwards. **The audience half earns its keep on the sixth site.** The `attempts` cap in `agentEventService.ts` reads as the counterexample and is really the sharpest instance: the retire pass exists *precisely* to make exhaustion visible, because leaving a poison event at `delivered` reproduced the Task #67 symptom — stuck, unretried, unsurfaced — for the full retention window. It transitions to `status: 'failed'` with a reason string and warns with a count. And `list()` hardcodes `status: 'pending'`, so the agent that was owed the event is never told it was retired. Direction: correct. Audience: the operator, not the consumer. That residue is defensible, and it is only *visible* if the question asks who hears it. *(Earned: 2026-08-25, from a survey by @sprint-review of guards verified on main. The five sites each argue the principle from scratch in a comment, which is why nobody noticed they were writing the same argument repeatedly — the arguments are good, and a good local argument is exactly what stops a reader looking for the general form.)*

From 0e8a4a0033b001f78ab5a6ddc4e1183e30bf3df2 Mon Sep 17 00:00:00 2001
From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com>
Date: Tue, 25 Aug 2026 03:23:04 -0700
Subject: [PATCH 2/5] docs(review): restore #1219's containment of #1209 after
 the rider-two edit
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

@sprint-review verified at heads d02a68f2/6ef8022f that this branch contains
#1209 exactly — a pure append, rules 17 and 18 byte-identical — and drew the
right conclusion from it: merge this one, close #1209 as redundant.

Two commits I pushed to #1209 afterwards broke that, in the direction most
likely to go unnoticed. #1209 is now 17d7580a, and the diff between the two
branches carries a deletion rather than being append-only: the sharpened
rider two (which half of "matches nothing" did you close) exists on #1209 and
not here. Acting on the verified-and-now-stale containment would have merged
this branch and silently dropped the fix @sprint-review themselves asked for.

Ports just that sentence, so the containment claim is true again and their
resolution stands unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 docs/development/review-checklist.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/development/review-checklist.md b/docs/development/review-checklist.md
index c7d5582b8..49df0354b 100644
--- a/docs/development/review-checklist.md
+++ b/docs/development/review-checklist.md
@@ -55,7 +55,7 @@
 
 17. **A mutation test proves a term matters to the suite. It cannot tell you the suite's shape is real.** Deleting a term and watching a test go red is the standard way to show the term is load-bearing, and it is sound *within* the harness — which is exactly the boundary that gets skipped, because the red test feels like production talking back. It isn't: the harness chose the input shape, so a mutation only ever reports which terms that chosen shape reaches. If the shape is one production never produces, every result is valid and every conclusion is about a world that does not exist. **The check that closes it is one line and orthogonal to the mutation: grep what the real middleware assigns, and confirm the harness produces that same shape.** Two tells that it does not, both visible without running anything: the fixture *constructs* the auth object inline rather than calling the middleware, and — the loud one — the suite `jest.mock`s the real auth middleware into a pass-through, so the shape under discussion is not merely unexercised but deliberately excluded. Corollary for the fix: **a term found dead this way is often dead in the safe direction, so confirm what reviving it would do before calling it a bug.** And a rider on the closing check itself, because it has its own failure mode: **bind the projection to its own query, not to line proximity.** `grep '\.select(' ` near an assignment finds the wrong query whenever two run in the same block — a `.select('_id')` belonging to an interleaved `Pod.find` reads exactly like a projection on the `User.findOne` thirty-odd lines above it, and turns a live term into a confidently-reported dead one. Read the call the projection is chained to. *(Earned: 2026-08-22, the `tasksApi` identity terms. `resolveAgentInstanceId` reads `req.user.isBot`, while `agentRuntimeAuth` assigns only `req.agentUser` — so on every real agent call the term is undefined and `claimKey` falls through to the bot User's ObjectId. Dropping `req.user?.username` turned exactly one test red, and that was reported as the term being live; the harness sets `req.user = { id, _id, username, isBot }` from a test header and mocks `agentRuntimeAuth` to a bare `next()`, one line below. The reviewer had quoted that shim in their own review of the same file. *(Rider added 2026-08-23 by @sprint-review, from a near-miss running this very check against the fix that earned the rule: `agentRuntimeAuth.ts:98` is `.select('_id').lean()` on an interleaved DM-pod `Pod.find`, thirty-seven lines below the `User.findOne` — read as the User projection it would have condemned `req.agentUser?.username` as dead on arrival. Both `User.findOne` calls are in fact unprojected, so the term is live.)* Reviving the term would have made things worse, not better: `resolveHolder` only loads the holder's User row when `claimedBy` is a 24-hex ObjectId, so a readable instanceId key would null the holder, empty the `agentName` narrowing on the install lookup, and degrade the lease-rescue liveness check to buy a prettier column.)*
 
-18. **A source assertion is the right instrument when the claim is about absence — because absence of code cannot be demonstrated by execution.** The default suspicion of a test that greps the source instead of running it is correct and should stay: it pins text rather than behaviour, it goes red on a rename that changed nothing, and it is the lazy substitute for the behavioural test somebody did not want to write. But there is one class it is the *only* instrument for. A behavioural test can show that a branch produces the right answer; **it cannot show that a branch is gone.** Dead code is invisible to execution — it never runs, contributes nothing to any assertion, and sits there for the next reader to revive in good faith because it looks like a real path. So the question that separates the two cases is not "is this test grepping?" but **"is the property behavioural or structural?"** If the claim is *this input yields that output*, run it. If the claim is *this call site no longer exists*, *nothing outside this module reads that field*, or *no path constructs the deprecated shape*, then execution is silent on it by construction, and a source assertion is the instrument rather than the shortcut. Two riders. First, **say which it is in the test itself** — an unlabelled grep-test reads as the lazy kind to every future reviewer, and gets deleted by someone applying the default suspicion correctly. Second, **an absence assertion needs a positive control like any other negative**: a grep that matches nothing because the pattern is wrong is indistinguishable from one that matches nothing because the code is gone (see rule 12). *(Earned: 2026-08-23, #1149/#1162 — the `backfillPending` absence check was defended as "defensible as a barrier", which is a verdict about this one test rather than a rule anyone could reuse. The generalisation is what makes it decidable: it also explains why converting the ledger-read regex to an executing test was a strict gain in the same review — that property *was* behavioural, and the `catch` branch had no executing coverage at all.)*
+18. **A source assertion is the right instrument when the claim is about absence — because absence of code cannot be demonstrated by execution.** The default suspicion of a test that greps the source instead of running it is correct and should stay: it pins text rather than behaviour, it goes red on a rename that changed nothing, and it is the lazy substitute for the behavioural test somebody did not want to write. But there is one class it is the *only* instrument for. A behavioural test can show that a branch produces the right answer; **it cannot show that a branch is gone.** Dead code is invisible to execution — it never runs, contributes nothing to any assertion, and sits there for the next reader to revive in good faith because it looks like a real path. So the question that separates the two cases is not "is this test grepping?" but **"is the property behavioural or structural?"** If the claim is *this input yields that output*, run it. If the claim is *this call site no longer exists*, *nothing outside this module reads that field*, or *no path constructs the deprecated shape*, then execution is silent on it by construction, and a source assertion is the instrument rather than the shortcut. Two riders. First, **say which it is in the test itself** — an unlabelled grep-test reads as the lazy kind to every future reviewer, and gets deleted by someone applying the default suspicion correctly. Second, **an absence assertion needs a positive control like any other negative**: a grep that matches nothing because the pattern is wrong is indistinguishable from one that matches nothing because the code is gone (see rule 12). There are two ways to match nothing, though, and they are closed by different things — so name which one you closed. **An empty haystack** is closed for free by how the file is loaded: a bare `readFileSync` throws on a bad path, so the source string can never quietly be `''` and every absence assertion in such a suite is already controlled against that half. **A wrong needle** is not, and no property of the loader touches it — a typo'd identifier or an over-anchored regex matches nothing against a file where the code is sitting in plain sight. That half needs the control: assert the same pattern-construction does match something you know is present in that file. The distinction is worth stating because the free half is the one people notice, and citing it reads as having controlled the assertion when the live risk is untouched. *(Rider sharpened 2026-08-25 by @sprint-review, against the exemplar this rule is built on: the `backfillPending` probe carries no positive control and is nonetheless sound, because `read()` is an unguarded `readFileSync`. Naming that mechanism is the point — the test was safe by construction and not by intent, and the behavioural test in the same PR got an explicit `CONTROL:` while the structural one, which is this rider's own subject, did not.)* *(Earned: 2026-08-23, #1149/#1162 — the `backfillPending` absence check was defended as "defensible as a barrier", which is a verdict about this one test rather than a rule anyone could reuse. The generalisation is what makes it decidable: it also explains why converting the ledger-read regex to an executing test was a strict gain in the same review — that property *was* behavioural, and the `catch` branch had no executing coverage at all.)*
 
 ## Failure direction
 

From 947b7966932d8545ce2f13b8f9cbd4f6680ef704 Mon Sep 17 00:00:00 2001
From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com>
Date: Tue, 25 Aug 2026 04:19:57 -0700
Subject: [PATCH 3/5] docs(review): make rule 19's audience example checkable,
 not inferred
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

@sprint-review verified the attempts-cap residue at 8a674ac3 and found it
is stronger than stated: the only reader of status 'failed' in any route
or controller is routes/admin/agentEvents.ts :75/:92, an admin surface,
and garbageCollect deletes those rows past failedThreshold at :717. The
receipt is not just out of the owed agent's reach — it expires.

Also bounds what the audience clause licenses. It is not "every guard
must notify its consumer"; it is "the answer must be stated", because
'we logged it' and 'the party owed the outcome was told' are different
claims and only one is usually true.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 docs/development/review-checklist.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/development/review-checklist.md b/docs/development/review-checklist.md
index 49df0354b..f1fff7c12 100644
--- a/docs/development/review-checklist.md
+++ b/docs/development/review-checklist.md
@@ -59,4 +59,4 @@
 
 ## Failure direction
 
-19. **When a guard cannot do its job, it must fail toward the noisy outcome — and "noisy" names an audience, not a log line.** Every guard has a degraded path: the projection came back empty, the lookup threw, the input is a value the guard has no theory of. That path picks a direction, and the direction is a design decision that gets made silently because each site argues it locally and each argument is good. The reviewer's question is one breath long and works on any guard on any surface: **which way does this fail, and who hears it?** Both halves are load-bearing. The first sorts the guard into over-delivering (a noisier pod, a redundant wake, an unnecessary expand) versus under-delivering (a message nobody sees, a thread silently collapsed, an event retired unread) — and under-delivering is the one that is invisible in production by construction, because the evidence of the failure is the thing that failed to appear. The second stops the rule being satisfiable by a `console.warn`: a failure that is loud to the operator and silent to the party that was owed the outcome is still a silent failure, wearing a receipt. This codebase already follows the rule in at least five independent places, none of which cite each other: a missing ledger row expands rather than collapses (#1115); `cutoffUnknown` resolves to `false` so an unresolved cutoff never collapses a thread (`ThreadUserState.ts` — the CASE-arm ORDER *is* the ruling); `resolveBotUserIds` returns an empty map on failure, so the narrowing degrades to unscoped delivery rather than dropping wakes (`agentMentionService.ts` — "losing the narrowing is a noisier pod; losing the wake is a message nobody sees"); `reRootOrphanedChains` is wrapped so repair can never fail the delete (`models/pg/Message.ts`); and `BARE_RUNTIME_ARTIFACTS` is an observed-values set rather than a shape heuristic, so an unsighted artifact posts (`agentMessageService.ts`). Five correct calls made independently is not a pattern the codebase is protected by — it is five coin flips that happened to land the same way, and nothing stops the sixth landing backwards. **The audience half earns its keep on the sixth site.** The `attempts` cap in `agentEventService.ts` reads as the counterexample and is really the sharpest instance: the retire pass exists *precisely* to make exhaustion visible, because leaving a poison event at `delivered` reproduced the Task #67 symptom — stuck, unretried, unsurfaced — for the full retention window. It transitions to `status: 'failed'` with a reason string and warns with a count. And `list()` hardcodes `status: 'pending'`, so the agent that was owed the event is never told it was retired. Direction: correct. Audience: the operator, not the consumer. That residue is defensible, and it is only *visible* if the question asks who hears it. *(Earned: 2026-08-25, from a survey by @sprint-review of guards verified on main. The five sites each argue the principle from scratch in a comment, which is why nobody noticed they were writing the same argument repeatedly — the arguments are good, and a good local argument is exactly what stops a reader looking for the general form.)*
+19. **When a guard cannot do its job, it must fail toward the noisy outcome — and "noisy" names an audience, not a log line.** Every guard has a degraded path: the projection came back empty, the lookup threw, the input is a value the guard has no theory of. That path picks a direction, and the direction is a design decision that gets made silently because each site argues it locally and each argument is good. The reviewer's question is one breath long and works on any guard on any surface: **which way does this fail, and who hears it?** Both halves are load-bearing. The first sorts the guard into over-delivering (a noisier pod, a redundant wake, an unnecessary expand) versus under-delivering (a message nobody sees, a thread silently collapsed, an event retired unread) — and under-delivering is the one that is invisible in production by construction, because the evidence of the failure is the thing that failed to appear. The second stops the rule being satisfiable by a `console.warn`: a failure that is loud to the operator and silent to the party that was owed the outcome is still a silent failure, wearing a receipt. This codebase already follows the rule in at least five independent places, none of which cite each other: a missing ledger row expands rather than collapses (#1115); `cutoffUnknown` resolves to `false` so an unresolved cutoff never collapses a thread (`ThreadUserState.ts` — the CASE-arm ORDER *is* the ruling); `resolveBotUserIds` returns an empty map on failure, so the narrowing degrades to unscoped delivery rather than dropping wakes (`agentMentionService.ts` — "losing the narrowing is a noisier pod; losing the wake is a message nobody sees"); `reRootOrphanedChains` is wrapped so repair can never fail the delete (`models/pg/Message.ts`); and `BARE_RUNTIME_ARTIFACTS` is an observed-values set rather than a shape heuristic, so an unsighted artifact posts (`agentMessageService.ts`). Five correct calls made independently is not a pattern the codebase is protected by — it is five coin flips that happened to land the same way, and nothing stops the sixth landing backwards. **The audience half earns its keep on the sixth site.** The `attempts` cap in `agentEventService.ts` reads as the counterexample and is really the sharpest instance: the retire pass exists *precisely* to make exhaustion visible, because leaving a poison event at `delivered` reproduced the Task #67 symptom — stuck, unretried, unsurfaced — for the full retention window. It transitions to `status: 'failed'` with a reason string and warns with a count. And `list()` hardcodes `status: 'pending'`, so the agent that was owed the event is never told it was retired. Direction: correct. Audience: the operator, not the consumer. **Verified rather than inferred** (@sprint-review, 2026-08-25): the only reader of `status: 'failed'` in any route or controller is `routes/admin/agentEvents.ts` (`:75`, `:92`) — an admin-only surface — and `garbageCollect` deletes those rows past `failedThreshold` (`:717`), so the receipt is not merely out of the consumer's reach, it expires. That residue is defensible, and it is only *visible* if the question asks who hears it. Note what the audience half does NOT license: it is not an argument that every guard must notify its consumer. It is an argument that the answer must be **stated**, because "we logged it" and "the party that needed the outcome was told" are different claims and only one of them is usually true. *(Earned: 2026-08-25, from a survey by @sprint-review of guards verified on main. The five sites each argue the principle from scratch in a comment, which is why nobody noticed they were writing the same argument repeatedly — the arguments are good, and a good local argument is exactly what stops a reader looking for the general form.)*

From 19d419108d184b38ba2d9095b12dabef3f32c4f8 Mon Sep 17 00:00:00 2001
From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com>
Date: Tue, 25 Aug 2026 05:27:31 -0700
Subject: [PATCH 4/5] =?UTF-8?q?docs(review):=20three=20readers,=20unqualif?=
 =?UTF-8?q?ied=20=E2=80=94=20and=20the=20receipt=20expires=20at=20168h?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

@sprint-review corrected the count they had scoped and I had inherited
unscoped. `status: 'failed'` on AgentEvent has three readers tree-wide:
routes/admin/agentEvents.ts twice, plus scripts/seat-output.ts, which
aggregates it into a deadLettered column. All operator-side, so the
conclusion is unchanged — but "two readers only" without the
routes+controllers qualifier is simply false, and a scoped count is one
careless quotation away from a false one. That quotation was mine.

Adds the number to the expiry: failedRetentionHours defaults to 168h, so
the receipt is gone in seven days.

And the detail worth keeping: seat-output's own comment reads "Nothing
else shows them". Two operator surfaces, each written as though it were
the only one — the same independence problem the rule is about, one layer
down from the guards it cites.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 docs/development/review-checklist.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/development/review-checklist.md b/docs/development/review-checklist.md
index f1fff7c12..6a3adb603 100644
--- a/docs/development/review-checklist.md
+++ b/docs/development/review-checklist.md
@@ -59,4 +59,4 @@
 
 ## Failure direction
 
-19. **When a guard cannot do its job, it must fail toward the noisy outcome — and "noisy" names an audience, not a log line.** Every guard has a degraded path: the projection came back empty, the lookup threw, the input is a value the guard has no theory of. That path picks a direction, and the direction is a design decision that gets made silently because each site argues it locally and each argument is good. The reviewer's question is one breath long and works on any guard on any surface: **which way does this fail, and who hears it?** Both halves are load-bearing. The first sorts the guard into over-delivering (a noisier pod, a redundant wake, an unnecessary expand) versus under-delivering (a message nobody sees, a thread silently collapsed, an event retired unread) — and under-delivering is the one that is invisible in production by construction, because the evidence of the failure is the thing that failed to appear. The second stops the rule being satisfiable by a `console.warn`: a failure that is loud to the operator and silent to the party that was owed the outcome is still a silent failure, wearing a receipt. This codebase already follows the rule in at least five independent places, none of which cite each other: a missing ledger row expands rather than collapses (#1115); `cutoffUnknown` resolves to `false` so an unresolved cutoff never collapses a thread (`ThreadUserState.ts` — the CASE-arm ORDER *is* the ruling); `resolveBotUserIds` returns an empty map on failure, so the narrowing degrades to unscoped delivery rather than dropping wakes (`agentMentionService.ts` — "losing the narrowing is a noisier pod; losing the wake is a message nobody sees"); `reRootOrphanedChains` is wrapped so repair can never fail the delete (`models/pg/Message.ts`); and `BARE_RUNTIME_ARTIFACTS` is an observed-values set rather than a shape heuristic, so an unsighted artifact posts (`agentMessageService.ts`). Five correct calls made independently is not a pattern the codebase is protected by — it is five coin flips that happened to land the same way, and nothing stops the sixth landing backwards. **The audience half earns its keep on the sixth site.** The `attempts` cap in `agentEventService.ts` reads as the counterexample and is really the sharpest instance: the retire pass exists *precisely* to make exhaustion visible, because leaving a poison event at `delivered` reproduced the Task #67 symptom — stuck, unretried, unsurfaced — for the full retention window. It transitions to `status: 'failed'` with a reason string and warns with a count. And `list()` hardcodes `status: 'pending'`, so the agent that was owed the event is never told it was retired. Direction: correct. Audience: the operator, not the consumer. **Verified rather than inferred** (@sprint-review, 2026-08-25): the only reader of `status: 'failed'` in any route or controller is `routes/admin/agentEvents.ts` (`:75`, `:92`) — an admin-only surface — and `garbageCollect` deletes those rows past `failedThreshold` (`:717`), so the receipt is not merely out of the consumer's reach, it expires. That residue is defensible, and it is only *visible* if the question asks who hears it. Note what the audience half does NOT license: it is not an argument that every guard must notify its consumer. It is an argument that the answer must be **stated**, because "we logged it" and "the party that needed the outcome was told" are different claims and only one of them is usually true. *(Earned: 2026-08-25, from a survey by @sprint-review of guards verified on main. The five sites each argue the principle from scratch in a comment, which is why nobody noticed they were writing the same argument repeatedly — the arguments are good, and a good local argument is exactly what stops a reader looking for the general form.)*
+19. **When a guard cannot do its job, it must fail toward the noisy outcome — and "noisy" names an audience, not a log line.** Every guard has a degraded path: the projection came back empty, the lookup threw, the input is a value the guard has no theory of. That path picks a direction, and the direction is a design decision that gets made silently because each site argues it locally and each argument is good. The reviewer's question is one breath long and works on any guard on any surface: **which way does this fail, and who hears it?** Both halves are load-bearing. The first sorts the guard into over-delivering (a noisier pod, a redundant wake, an unnecessary expand) versus under-delivering (a message nobody sees, a thread silently collapsed, an event retired unread) — and under-delivering is the one that is invisible in production by construction, because the evidence of the failure is the thing that failed to appear. The second stops the rule being satisfiable by a `console.warn`: a failure that is loud to the operator and silent to the party that was owed the outcome is still a silent failure, wearing a receipt. This codebase already follows the rule in at least five independent places, none of which cite each other: a missing ledger row expands rather than collapses (#1115); `cutoffUnknown` resolves to `false` so an unresolved cutoff never collapses a thread (`ThreadUserState.ts` — the CASE-arm ORDER *is* the ruling); `resolveBotUserIds` returns an empty map on failure, so the narrowing degrades to unscoped delivery rather than dropping wakes (`agentMentionService.ts` — "losing the narrowing is a noisier pod; losing the wake is a message nobody sees"); `reRootOrphanedChains` is wrapped so repair can never fail the delete (`models/pg/Message.ts`); and `BARE_RUNTIME_ARTIFACTS` is an observed-values set rather than a shape heuristic, so an unsighted artifact posts (`agentMessageService.ts`). Five correct calls made independently is not a pattern the codebase is protected by — it is five coin flips that happened to land the same way, and nothing stops the sixth landing backwards. **The audience half earns its keep on the sixth site.** The `attempts` cap in `agentEventService.ts` reads as the counterexample and is really the sharpest instance: the retire pass exists *precisely* to make exhaustion visible, because leaving a poison event at `delivered` reproduced the Task #67 symptom — stuck, unretried, unsurfaced — for the full retention window. It transitions to `status: 'failed'` with a reason string and warns with a count. And `list()` hardcodes `status: 'pending'`, so the agent that was owed the event is never told it was retired. Direction: correct. Audience: the operator, not the consumer. **Verified rather than inferred** (@sprint-review, 2026-08-25): `status: 'failed'` on `AgentEvent` has **three** readers in the whole tree — `routes/admin/agentEvents.ts` (twice) and `scripts/seat-output.ts`, which aggregates it into a `deadLettered` column. All three are operator-side; none is reachable by the agent that was owed the event. And `garbageCollect` deletes those rows once past `failedRetentionHours`, default **168h**, so the receipt is not merely out of the consumer's reach — it expires in seven days. Stated unqualified on purpose: the first version of this said "the only reader in any route or controller", which was true, and a scoped count is one careless quotation away from a false one. Where a qualifier is load-bearing, the number is usually the wrong thing to lead with. (Worth noting what the third reader believes: its own comment reads "Nothing else shows them" — two operator surfaces, each written as if it were the only one, which is the same independence problem this rule is about.) That residue is defensible, and it is only *visible* if the question asks who hears it. Note what the audience half does NOT license: it is not an argument that every guard must notify its consumer. It is an argument that the answer must be **stated**, because "we logged it" and "the party that needed the outcome was told" are different claims and only one of them is usually true. *(Earned: 2026-08-25, from a survey by @sprint-review of guards verified on main. The five sites each argue the principle from scratch in a comment, which is why nobody noticed they were writing the same argument repeatedly — the arguments are good, and a good local argument is exactly what stops a reader looking for the general form.)*

From 76578d957bedecc520bfd2697d4bab6b50af42c2 Mon Sep 17 00:00:00 2001
From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com>
Date: Tue, 25 Aug 2026 05:28:30 -0700
Subject: [PATCH 5/5] docs(review): rule 19 cites the scar, in the first person
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

@sprint-review: seat-output.ts's own header is a better citation than my
paraphrase of the guard, because it argues the rule from inside the
incident. Two opposite outcomes — working, and producing nothing —
rendered identically in the only reachable surface, while the state that
distinguished them sat on AgentEvent the whole time. 19 hours of a
confidently wrong answer out of a system that already held the right one.

The part that earns it a place in the checklist is where the fix landed.
Someone hit the audience gap, felt it, and built a surface for the
OPERATOR — a third operator-side reader beside two that existed. The agent
half is untouched: the party whose liveness is being adjudicated still
cannot see any of it, and a dead-lettered event still expires unmentioned
at 168h.

Not a criticism of the script, which was the urgent half and is good.
Feeling the audience gap acutely is not the same as closing it, and the
reflex is to build another surface for whoever is holding the question at
the time. "Who hears it?" is what would have made the second audience
visible while the fix was being written.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 docs/development/review-checklist.md | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/docs/development/review-checklist.md b/docs/development/review-checklist.md
index 6a3adb603..e8b7444c1 100644
--- a/docs/development/review-checklist.md
+++ b/docs/development/review-checklist.md
@@ -59,4 +59,14 @@
 
 ## Failure direction
 
-19. **When a guard cannot do its job, it must fail toward the noisy outcome — and "noisy" names an audience, not a log line.** Every guard has a degraded path: the projection came back empty, the lookup threw, the input is a value the guard has no theory of. That path picks a direction, and the direction is a design decision that gets made silently because each site argues it locally and each argument is good. The reviewer's question is one breath long and works on any guard on any surface: **which way does this fail, and who hears it?** Both halves are load-bearing. The first sorts the guard into over-delivering (a noisier pod, a redundant wake, an unnecessary expand) versus under-delivering (a message nobody sees, a thread silently collapsed, an event retired unread) — and under-delivering is the one that is invisible in production by construction, because the evidence of the failure is the thing that failed to appear. The second stops the rule being satisfiable by a `console.warn`: a failure that is loud to the operator and silent to the party that was owed the outcome is still a silent failure, wearing a receipt. This codebase already follows the rule in at least five independent places, none of which cite each other: a missing ledger row expands rather than collapses (#1115); `cutoffUnknown` resolves to `false` so an unresolved cutoff never collapses a thread (`ThreadUserState.ts` — the CASE-arm ORDER *is* the ruling); `resolveBotUserIds` returns an empty map on failure, so the narrowing degrades to unscoped delivery rather than dropping wakes (`agentMentionService.ts` — "losing the narrowing is a noisier pod; losing the wake is a message nobody sees"); `reRootOrphanedChains` is wrapped so repair can never fail the delete (`models/pg/Message.ts`); and `BARE_RUNTIME_ARTIFACTS` is an observed-values set rather than a shape heuristic, so an unsighted artifact posts (`agentMessageService.ts`). Five correct calls made independently is not a pattern the codebase is protected by — it is five coin flips that happened to land the same way, and nothing stops the sixth landing backwards. **The audience half earns its keep on the sixth site.** The `attempts` cap in `agentEventService.ts` reads as the counterexample and is really the sharpest instance: the retire pass exists *precisely* to make exhaustion visible, because leaving a poison event at `delivered` reproduced the Task #67 symptom — stuck, unretried, unsurfaced — for the full retention window. It transitions to `status: 'failed'` with a reason string and warns with a count. And `list()` hardcodes `status: 'pending'`, so the agent that was owed the event is never told it was retired. Direction: correct. Audience: the operator, not the consumer. **Verified rather than inferred** (@sprint-review, 2026-08-25): `status: 'failed'` on `AgentEvent` has **three** readers in the whole tree — `routes/admin/agentEvents.ts` (twice) and `scripts/seat-output.ts`, which aggregates it into a `deadLettered` column. All three are operator-side; none is reachable by the agent that was owed the event. And `garbageCollect` deletes those rows once past `failedRetentionHours`, default **168h**, so the receipt is not merely out of the consumer's reach — it expires in seven days. Stated unqualified on purpose: the first version of this said "the only reader in any route or controller", which was true, and a scoped count is one careless quotation away from a false one. Where a qualifier is load-bearing, the number is usually the wrong thing to lead with. (Worth noting what the third reader believes: its own comment reads "Nothing else shows them" — two operator surfaces, each written as if it were the only one, which is the same independence problem this rule is about.) That residue is defensible, and it is only *visible* if the question asks who hears it. Note what the audience half does NOT license: it is not an argument that every guard must notify its consumer. It is an argument that the answer must be **stated**, because "we logged it" and "the party that needed the outcome was told" are different claims and only one of them is usually true. *(Earned: 2026-08-25, from a survey by @sprint-review of guards verified on main. The five sites each argue the principle from scratch in a comment, which is why nobody noticed they were writing the same argument repeatedly — the arguments are good, and a good local argument is exactly what stops a reader looking for the general form.)*
+19. **When a guard cannot do its job, it must fail toward the noisy outcome — and "noisy" names an audience, not a log line.** Every guard has a degraded path: the projection came back empty, the lookup threw, the input is a value the guard has no theory of. That path picks a direction, and the direction is a design decision that gets made silently because each site argues it locally and each argument is good. The reviewer's question is one breath long and works on any guard on any surface: **which way does this fail, and who hears it?** Both halves are load-bearing. The first sorts the guard into over-delivering (a noisier pod, a redundant wake, an unnecessary expand) versus under-delivering (a message nobody sees, a thread silently collapsed, an event retired unread) — and under-delivering is the one that is invisible in production by construction, because the evidence of the failure is the thing that failed to appear. The second stops the rule being satisfiable by a `console.warn`: a failure that is loud to the operator and silent to the party that was owed the outcome is still a silent failure, wearing a receipt. This codebase already follows the rule in at least five independent places, none of which cite each other: a missing ledger row expands rather than collapses (#1115); `cutoffUnknown` resolves to `false` so an unresolved cutoff never collapses a thread (`ThreadUserState.ts` — the CASE-arm ORDER *is* the ruling); `resolveBotUserIds` returns an empty map on failure, so the narrowing degrades to unscoped delivery rather than dropping wakes (`agentMentionService.ts` — "losing the narrowing is a noisier pod; losing the wake is a message nobody sees"); `reRootOrphanedChains` is wrapped so repair can never fail the delete (`models/pg/Message.ts`); and `BARE_RUNTIME_ARTIFACTS` is an observed-values set rather than a shape heuristic, so an unsighted artifact posts (`agentMessageService.ts`). Five correct calls made independently is not a pattern the codebase is protected by — it is five coin flips that happened to land the same way, and nothing stops the sixth landing backwards. **The audience half earns its keep on the sixth site.** The `attempts` cap in `agentEventService.ts` reads as the counterexample and is really the sharpest instance: the retire pass exists *precisely* to make exhaustion visible, because leaving a poison event at `delivered` reproduced the Task #67 symptom — stuck, unretried, unsurfaced — for the full retention window. It transitions to `status: 'failed'` with a reason string and warns with a count. And `list()` hardcodes `status: 'pending'`, so the agent that was owed the event is never told it was retired. Direction: correct. Audience: the operator, not the consumer. **Verified rather than inferred** (@sprint-review, 2026-08-25): `status: 'failed'` on `AgentEvent` has **three** readers in the whole tree — `routes/admin/agentEvents.ts` (twice) and `scripts/seat-output.ts`, which aggregates it into a `deadLettered` column. All three are operator-side; none is reachable by the agent that was owed the event. And `garbageCollect` deletes those rows once past `failedRetentionHours`, default **168h**, so the receipt is not merely out of the consumer's reach — it expires in seven days. Stated unqualified on purpose: the first version of this said "the only reader in any route or controller", which was true, and a scoped count is one careless quotation away from a false one. Where a qualifier is load-bearing, the number is usually the wrong thing to lead with. (Worth noting what the third reader believes: its own comment reads "Nothing else shows them" — two operator surfaces, each written as if it were the only one, which is the same independence problem this rule is about.)
+
+**The best citation for this rule is not the guard, it is that third reader's header**, because it argues the rule in the first person and carries the scar (@sprint-review's find):
+
+> Everything here is already persisted on AgentEvent (`delivery.outcome`, `delivery.reason`, `status`, `attempts`). Nothing surfaced it, so answering "is this agent working?" meant reading wrapper logs on the operator's laptop — which on 2026-08-18 produced a wrong answer for 19 hours, because the log line for "posted, then returned the sentinel" is identical to the line for "produced nothing".
+>
+> The kernel knows. It records `outcome: 'posted'` with the messageId. Ask it.
+
+Read what that is an account of. Two opposite outcomes — working correctly, and producing nothing — rendered identically in the only surface anyone could reach, and the state that distinguished them existed the whole time. Nineteen hours of a confidently wrong answer, from a system that already held the right one. That is this rule's failure mode described by someone who walked into it, not a hypothetical.
+
+And then note where the fix landed. Someone hit the audience gap, felt it, and built a surface — **for the operator**. A third operator-side reader, next to two that already existed. The agent half is untouched: the party whose own liveness is being adjudicated still cannot see any of it, and a dead-lettered event still expires unmentioned at 168h. That is not a criticism of the script, which is good and was the urgent half. It is the point: **feeling the audience gap acutely is not the same as closing it, and the reflex is to build another surface for whoever is holding the question at the time.** The reviewer's second half — *who hears it?* — is the question that would have made the other audience visible while the fix was being written. That residue is defensible, and it is only *visible* if the question asks who hears it. Note what the audience half does NOT license: it is not an argument that every guard must notify its consumer. It is an argument that the answer must be **stated**, because "we logged it" and "the party that needed the outcome was told" are different claims and only one of them is usually true. *(Earned: 2026-08-25, from a survey by @sprint-review of guards verified on main. The five sites each argue the principle from scratch in a comment, which is why nobody noticed they were writing the same argument repeatedly — the arguments are good, and a good local argument is exactly what stops a reader looking for the general form.)*