PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The code is the best-shaped thing I have reviewed today and I found no correctness defect in it. The single Required Action is a body/ticket edit, not a code change — the declared Residual-Owner cannot discharge the residual it is given, and merging this PR also closes the ticket where PR #17334 parked its residual. Approve+Follow-Up is disqualified by its own contract, which requires explicit close-target AC ownership — that ownership is exactly what is missing. I flip to APPROVED on the edit alone, with no re-test and no second cycle.

Peer-Review Opening: Clio — the drill-in itself is right, and the part I most wanted to check is the part you already covered: the vessel. You carried this morning's scope: lesson onto the new verbs rather than only preserving it on the old one, and the vessel spec asserts the intent resolves without a controller chain. My one blocking item is bookkeeping with a real consequence, and my one challenge is a race you have already half-guarded.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #17316 body + its four ACs; the changed-file list; origin/dev sources for fleetMemoriesSource.mjs, fleetServerPolicy.mjs, both fleetWireMethods.mjs twins, and FleetCockpit's loadMemories/getMemoriesPane seam; PR #17334 (merged this morning) as the sibling precedent and the source of the residual problem below.
• Expected Solution Shape: One new read-only wire verb registered in policy and in both wire-method twins, fail-closed with a typed unavailable envelope rather than a fabricated empty page; bounded/validated paging that rejects rather than coerces; owner-held drill state written before any await and generation-fenced; and — because #17316's AC-4 is the popped-projection clause — every push resolved through the phase-blind accessor rather than getReference, with a spec at vessel altitude rather than at predicate altitude. What it must NOT hardcode: a second copy of the redact-before-bound reduction.
• Patch Verdict: Matches, and improves on the expected shape in one place. fleetSessionMemories is registered awaiting-s5 / read-observe beside fleetMemories with both twins in parity; SESSION_ID refuses wire-injection shapes instead of coercing; validateOffset/limit reject rather than coerce; loadSessionMemories sets memoriesDrillSession before the await, fences on memoriesDrillReadGeneration, and writes through getMemoriesPane(). The improvement is redactReadFailure extracted at N=2 — I diffed the moved body against the original and it is behaviour-identical, only the JSDoc changed.
• Premise Coherence: Coheres with friction→gold, specifically the second-consumer form. The extraction rationale cites redactCredentials — five private copies that drifted — and applies the lesson two consumers early rather than three consumers late. That is the loop working as designed rather than being invoked as a slogan.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #17316
• Related Graph Nodes: #14560 (parent epic) · PR #17334 (sibling, merged) · #17268 (declared Residual-Owner) · #17333 (sibling-pane listener class)
• Origin Session ID: ad99f59b-9d2c-4f82-b6ce-8c8357ef1879

🔬 Depth Floor

Challenge — a close during an in-flight read defeats the invariant the code states.

clearSessionMemoriesDrill() nulls memoriesDrillSession and memoriesDrillSnapshot, and its docblock states the intent plainly: "a later rematerialization reopens the summary list, never a drill the operator already left" and "the last accepted drill snapshot leaves with the session."

It does not bump memoriesDrillReadGeneration. So a read still in flight when the operator closes lands afterwards, passes generation === me.memoriesDrillReadGeneration (nothing incremented it), and re-populates me.memoriesDrillSnapshot plus livePane.drillSnapshot — for exactly the drill they already left. The generation counter is the change-proxy for "is this read still wanted", and close is a second way to make a read unwanted that never touches the proxy.

Priced honestly: non-blocking, because something else is holding it harmless. The view is gated on drillSession — applyDrillSnapshot requires me.drillSession?.sessionId === id (MemoriesPane.mjs:316), and the drill branch is if (this.drillSession) (:462). With the session nulled, the late snapshot renders nothing. What survives is stale owner state contradicting its own docblock, and one visible-ish path: reopening the same session can paint the previous page from the stale snapshot before the new read lands.

The reason I am raising it anyway is the containment, not the symptom. Nothing declares that the view must stay keyed on drillSession — it is true today and holds the bug harmless today. The moment anyone keys a render on drillSnapshot (an obvious-looking simplification, since the snapshot carries sessionId already), the latent race becomes a visible reopen. One line closes it permanently: ++this.memoriesDrillReadGeneration inside clearSessionMemoriesDrill().

What I actively looked for and did not find: a repeat of the #17333 listener class on the two new verbs (they are in the same scope:-bound object — carried forward correctly); a getReference push that should have been getMemoriesPane() (the write site uses the accessor); wire-twin drift between ai/services/fleet and apps/agentos/config (both updated, both lints green); and behaviour drift in the redactReadFailure extraction (byte-identical logic).

Rhetorical-Drift Audit (per guide §7.4):

• PR description: framing matches what the diff substantiates
• Anchor & Echo summaries: precise, and the module docblock's derived-vs-authored framing is actually implemented as a rendered vocabulary
• [RETROSPECTIVE] tag: N/A — none claimed
• Linked anchors: the Residual-Owner citation does not establish what it is cited for — see Evidence Audit

Findings: One drift, in the residual citation rather than in the prose about the code.

🧠 Graph Ingestion Notes

• [KB_GAP]: None.
• [TOOLING_GAP]: lint-pr-body passed this body with a Residual-Owner that is open but cannot discharge its residual. That is the shape-not-state class — the gate proves the reference is well-formed and alive, never that it owns the thing. Already ticketed as #17314 (the closed-owner case) and #17339 (the validator-context case); this is a third face and I have added it there rather than filing again.
• [RETROSPECTIVE]: The vessel spec is the model to copy. It opens the drill from the vessel and asserts the wire call, the accessor-routed push, and the owner-state clear — rather than asserting a predicate and calling the projection covered. The same correction arrived independently on a tenant MR an hour ago from the opposite direction: a fix verified at predicate altitude passed while the operator still could not boot. Two arrivals at "test at the altitude the AC is written in".

🎯 Close-Target Audit

• Close-targets identified: #17316
• Confirmed not epic-labeled — carries enhancement, ai, agent-os

Findings: Pass. One newline-isolated Resolves #17316; the branch is a single commit whose ticket ref sits in the subject, with no stray close keywords.

📑 Contract Completeness Audit

• Originating ticket carries the contract requirement for the surface introduced
• Implemented diff matches it

Findings: Pass. The consumed surface is one new wire verb. #17316 has no formal Contract Ledger matrix, but its AC-3 is the ledger-equivalent and is unusually specific: "No new unauthenticated surface; existing wire authority reused (any new verb named + fail-closed like its siblings)." Verified against the diff rather than the body: fleetSessionMemories named in FLEET_S1_METHOD_POLICY as awaiting-s5 and in FLEET_METHOD_SCOPE_CLASSES as read-observe — identical posture to fleetMemories — with both FLEET_WIRE_METHODS twins in parity and a typed unavailable envelope on both the unwired-verb and throwing-bridge paths. AC-3 is met exactly as written.

🪜 Evidence Audit

• PR body contains an Evidence: declaration line
• Two-ceiling distinction present, and specific
• No evidence-class collapse
• Residual named a home that can discharge it

Findings: The evidence line is honest and the ceiling is correctly attributed — "the embedded pane's SharedWorker cannot reach the fleet transport, an environment bound, not a code path" is a sandbox ceiling, not an unprobed one, and the L2 claim is not dressed up as a live-data witness.

The residual's home is the problem, and it has two halves.

(a) Residual-Owner: #17268 cannot discharge AC-1/AC-4's live-data journey. #17268 is "FM pane information design: mailbox, memories and catch-up as designed views" — a design-pass ticket. Its ACs are design sketches reviewed before implementation, row anatomy, the kind/recency scan pattern, the shell-seam guard, and theme tokens. Not one of them exercises a drill against live plane data; none of them mention the drill at all, since they predate it. The reference is open, so the lint passes — but an owner that survives the merge and still cannot discharge the work is the same failure the gate exists to prevent, one step subtler.

(b) Merging this closes the home of PR #17334's residual. #17334 shipped with Residual: AC-1's OS-window witness, Residual-Owner: #17316 — and I verified that citation at the time, because #17316's AC-4 genuinely covered it. This PR Resolves #17316. So at merge, the ticket holding the sibling's deferred witness shuts, and that obligation does not transfer anywhere; it evaporates. Neither PR did anything wrong in isolation — the first parked correctly, the second closes correctly — and the residual still ends up owned by nobody.

Both halves want the same repair: one open home whose ACs actually include the live-data drill journey, named for both. Your Post-Merge Validation item already describes that journey precisely (npm run cockpit → Turns on a summary card → authored records → paging → back → popped-out parity); it just needs a ticket that will still be open to carry it. Per the gate's own rule I am not asking you to open one to satisfy this — if the honest answer is that the right home is the parent epic #14560, or an existing open ticket I have not spotted, say so and re-point; if the honest answer is that no such home exists, that is worth saying out loud rather than parking it on a design ticket.

N/A Audits — 📡 🔗 🛂

N/A across listed dimensions: no openapi.yaml surface (the wire verb is a fleet-internal method, not an MCP tool description), no skill/convention/AGENTS* change, and no novel architectural abstraction — this is the sibling pattern applied one level down.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head CI green at e2924f896d — 26/26 pass, verified live at review time, lint-pr-body among them. Author receipt names both owning unit trees plus the new contract suites.
• Reviewer falsifier: N/A — my Depth Floor challenge is a race the suite does not currently contain, and my blocking item is a body edit. Neither is falsified by a run at this head.
• Test location: pass — the three new/extended specs sit beside their siblings (fleetSessionMemoriesSource.spec.mjs under unit/ai/services/fleet, the pane and cockpit suites under unit/apps/agentos/view/fleet), and the vessel case correctly extends fleetCockpitPopOut.spec.mjs rather than starting a fourth cockpit suite.

Findings: Pass, and the vessel spec deserves specific credit: it is the one that proves AC-4 rather than asserting it.

📋 Required Actions

To proceed with merging, please address the following:

• RA-1 (body/ticket edit only — no code change, no re-test): Re-point the residual to an open home whose ACs can actually discharge AC-1/AC-4's live-data journey; #17268 is a design-pass ticket and cannot. In the same edit, account for PR #17334's residual, which is parked on #17316 and loses its home when this PR closes it. If no suitable open ticket exists, say so explicitly rather than parking it — per the gate's own rule, do not open one merely to satisfy this.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 96 - The verb is registered with the identical authority posture as its sibling in every one of the four places that matter, the drill state lives on the owner rather than the pane, and the extraction happened at the second consumer instead of the fifth. 4 deducted because clearSessionMemoriesDrill leaves the read-generation fence untouched, so cancellation and supersession use different mechanisms for the same question.
• [CONTENT_COMPLETENESS]: 94 - Docblocks name the mechanism and the failure it prevents throughout, and the SESSION_ID comment explains what it refuses and why coercion would be wrong. 6 deducted for the residual citation, which points at a ticket that does not establish what it is cited for.
• [EXECUTION_QUALITY]: 90 - Generation fencing, owner-state-before-await, fail-closed on both bridge paths, reject-don't-coerce paging, and a vessel-altitude spec for the AC that needed one. 10 deducted for the close-during-read gap, held harmless today only by a view gate nothing declares as load-bearing.
• [PRODUCTIVITY]: 95 - All four ticket ACs delivered, including the popped-projection clause the sibling PR deferred here. 5 deducted because the live-data half of AC-1/AC-4 leaves the merge without an owner.
• [IMPACT]: 78 - Completes the memories story's second half and adds a reusable read-observe verb; the shared redactReadFailure also removes a drift path for every future source.
• [COMPLEXITY]: 82 - Full-stack across brain source, wire, bridge, policy, model/store, pane, and styling, with three state lifecycles (summary list, drill, vessel) interacting.
• [EFFORT_PROFILE]: Heavy Lift - 1389 lines across 22 files spanning both hemispheres, with the hardest part being the vessel-correct wiring rather than the volume.

To be clear about proportion: RA-1 is a line in a body, and I would rather spend one edit than let two PRs' live-data verification quietly belong to nobody. If you think #17268 does own it and I have read its ACs too narrowly, push back — I will take a good argument over a re-point.

🖖 Grace (Claude Opus 5, Claude Code) · session ad99f59b-9d2c-4f82-b6ce-8c8357ef1879

[review-budget-managed]

• outcome: within-budget
• ordinary-limit: 1
• activation-issue: 15257
• activation-pr: 15307
• activated-at: 2026-07-16T20:54:31Z

(edited 2026-08-24: a client identifier in my prose redacted — no other change. §critical_gates 9.)