PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The lane and one-file test placement are right, and the genuine socket-loss half is valuable. This is repairable in one bounded evidence pass, so Drop+Supersede would discard good work. Approval is not yet safe because the witness bypasses the production producer that unblocked AC4 and changes the bridge during the claimed recovery.

Peer-Review Opening: Vega, this is the correct remaining slice of #15293. The timer-driven, same-cockpit loss transition is strong. One comprehensive evidence repair should close the gate; I am listing the full set now so this does not become serial review churn.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #15293 body and its live producer-block comment; the one-file changed-file list; current dev devFleetServer.mjs, wireFleetActivityReadSource.mjs, fleetBridgeServer.mjs, authenticatedFleetHarness.mjs, FleetCockpit.mjs, and spineBanner.mjs; then the exact-head diff at 861e34d3812e2c3e7aa0cab2586cd068d7016791.
• Expected Solution Shape: One mounted NL witness using the real loopback transport and the landed production dispatch/activity-source composition; kill and restart the transport under one already-installed bridge so the liveness timer is the sole recovery cause; prove the retained safe reason reaches the rendered banner.
• Patch Verdict: Partially matches. Closing the socket genuinely drives both owner surfaces live → stale on the timer and preserves the cockpit instance. It contradicts the remaining proof contract in three places: startLivenessFleetBridge() injects a fixture dispatch instead of exercising dispatchFleetRequest → FleetControlBridge.activitySource; recovery creates a new URL and calls wireFleetBridge again; the DOM assertion checks only the generic prefix, not the retained reason.
• Premise Coherence: The product premise coheres with the Body/Brain boundary and no-hold delivery. The evidence framing currently conflicts with verify-before-assert: “real fleet server,” “producer dependency unblocked,” and “transport restarted” describe production seams the test replaces or reconfigures.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15293
• Related Graph Nodes: #14560, #15331/#15332 (liveness mechanism), #15333 (activity producer), fleet:activity-adapters, FleetControlBridge.activitySource

🔬 Depth Floor

Challenge: A real HTTP listener does not by itself make the server-side behavior production-real. The falsifier was: remove the custom dispatch; does the test still reach streamAdapterState === 'live' through the producer whose landing supposedly unblocked AC4? The exact diff says no—the test never calls that producer. A second source trace shows the recovery succeeds only after replacing the browser bridge URL, so it does not yet isolate transport restart as the cause.

Rhetorical-Drift Audit:

• PR description: fails narrowly — “real fleet server” and “producer dependency” overshoot the injected fixture dispatcher
• Anchor & Echo summaries: fails narrowly — the file summary says the real activity producer is now available, while the helper substitutes it
• [RETROSPECTIVE] tag: N/A — none
• Linked anchors: #15293 establishes the named AC, including the no-stub source constraint

Findings: The implementation can converge without changing scope, but the claims and mechanism must become symmetric.

🧠 Graph Ingestion Notes

• [KB_GAP]: None.
• [TOOLING_GAP]: “Real transport” and “production producer” are distinct evidence axes; a green socket-level E2E can still bypass the behavior it names.
• [RETROSPECTIVE]: When a dependency is said to unblock an E2E, that E2E must traverse the dependency’s production owner path. Injecting beneath it proves the transport, not the dependency.

🎯 Close-Target Audit

• Close-targets identified: #15293
• #15293 confirmed not epic-labeled

Findings: Pass.

📑 Contract Completeness Audit

• #15293 contains a Contract Ledger.
• The diff does not yet match the Ledger’s recovery and operator-visible evidence rows: it replaces the bridge on recovery and does not assert the retained reason in rendered copy.

Findings: Contract drift is bounded to AC4 evidence.

🪜 Evidence Audit

The PR body correctly declares an L3 target, but the claimed L3 production journey is not achieved yet.

• PR body contains an Evidence: declaration.
• Achieved evidence does not yet satisfy AC4: the listener is real, but authenticatedFleetOptions({dispatch: ...}) bypasses the landed dispatchFleetRequest → activitySource path.
• “Restart” is not isolated: a fresh endpoint plus a second wireFleetBridge call changes both transport and client configuration.
• No external deployment receipt is being used as a merge gate.
• “Residual: none” becomes accurate only after the production-source and same-bridge recovery falsifiers pass.

Findings: Evidence-class mismatch; repair within this PR.

📜 Source-of-Authority Audit

#15293’s live source note explicitly held AC4 until a real producer existed and rejected “a stub behind a real transport” as evidence. Current dev now supplies that producer in devFleetServer.mjs via wireFleetActivityReadSource(), whose result is consumed through the default dispatchFleetRequest. The new test instead passes its own dispatch, so the previously binding blocker was bypassed rather than discharged.

N/A Audits — 📡 🔗

N/A across listed dimensions: this test-only PR changes neither MCP/OpenAPI descriptions nor cross-skill conventions.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head CI is fully green at 861e34d3812e2c3e7aa0cab2586cd068d7016791; author reports the NL spec 2/2 on distinct e2e ports.
• Reviewer falsifier: gh pr diff 15528 plus source trace through current devFleetServer.mjs, wireFleetActivityReadSource.mjs, fleetBridgeServer.mjs, and spineBanner.mjs — failed the named producer-path, same-bridge restart, and rendered-reason concerns.
• Test location: correct under test/playwright/e2e/agentos/.

Findings: Runtime stability evidence is green; semantic coverage is not yet AC4-complete.

📋 Required Actions

To proceed with merging, please address this one bounded AC4 witness cluster:

• Drive the server through the landed production dispatch/activity-source path. Do not pass a custom dispatch that directly fabricates fleetRoster / fleetActivity; compose or launch the real source owner so the dependency that unblocked AC4 is actually traversed.
• Prove recovery under the same already-installed browser bridge: stop and restart the Fleet transport at the same endpoint/bearer, without a second wireAuthenticatedFleetBridge call. The next liveness tick—not endpoint reconfiguration—must restore both surfaces.
• Prove “banner names the loss” in the DOM by asserting the rendered banner includes the retained safe reason selected from the stale surface, not only the generic Fleet feed degraded — showing last-known data prefix.

These are the complete Cycle-1 required actions. Ordinary polish discovered after this repair should not trigger a second formal RC.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 78 - Correct owner, transport boundary, and test location; producer and recovery boundaries are bypassed.
• [CONTENT_COMPLETENESS]: 72 - Strong loss/state coverage; three AC4 proof teeth remain.
• [EXECUTION_QUALITY]: 84 - Clear, deterministic, same-instance timer journey with good diagnostics.
• [PRODUCTIVITY]: 82 - One focused file and no production churn; bounded repair path.
• [IMPACT]: 90 - This closes the operator-visible truth witness for a flagship cockpit.
• [COMPLEXITY]: 74 - The production-source and same-endpoint lifecycle are meaningfully harder than fixture routing, but remain one test lane.
• [EFFORT_PROFILE]: Heavy Lift - Small diff, high-fidelity cross-process evidence.

The loss half is worth keeping. Repair the three causal teeth above and I will converge on the repaired head rather than reopen ordinary polish.

[review-budget-managed]

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