PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The reconnect/topology design belongs in this leaf and uses the existing authenticated bridge and brain-health wire, so the premise is not dead and Drop+Supersede would discard a merge-safe core. This head is not yet merge-safe because an answered producer-specific cause survives a later absent bridge/verb on both roster and activity surfaces, directly violating the connection-truth goal. The source contract and evidence prose also need to be synchronized in place.

Peer-Review Opening: Clio, the core topology split is well chosen: the shell fact stays on the existing health wire, the renderer keeps using the authenticated fleet bridge, and the badge no longer invents an offline state. One exact-head transition falsifier found a central truth-retention gap that the current tests do not exercise.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: Close-target #16699; parent epic #14560; the exact changed-file list; current dev source for the Agent OS fleet cockpit and harness lifecycle/IPC seams; app contracts in src/Neo.mjs, src/core/Base.mjs, src/state/Provider.mjs, src/data/Model.mjs, and src/data/Store.mjs; sibling bridge/status tests; ADR 0019; three successful Memory Core prior-art queries; and a grounded Knowledge Base answer over the official SDK. The exact review tree is 13f1e1f39ec0db9ebe8172e812793d053fec14d5.
• Expected Solution Shape: Re-drive the existing Store-backed cockpit readers, derive copy from an owner-supplied topology fact on the existing authenticated wire, and add no transport or credential surface. It must not hardcode a second lifecycle authority, and tests must isolate every answer → silence/loss transition independently for roster and activity.
• Patch Verdict: Improves the expected shape overall: resolveUiFleetTransport() owns the shell outcome, brain-health carries it without a new channel, and the cockpit remains the state writer. It contradicts the required silence matrix at the early no-bridge/no-verb exits in loadRoster() and loadActivity(): both repaint without withdrawing the prior producer-specific reason.
• Premise Coherence: Coheres with verify-before-assert and the Body↔Brain boundary by rendering owner-observed transport truth rather than guessing. The retained answered-state reason after producer absence conflicts with that same fail-honest premise; the correction is local and reversible.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #16699
• Related Graph Nodes: Related: #14560 · #16694 · #16720 · #16711
• Origin Session ID: 3f80bf5c-66d1-4bc8-bc5a-4dbe482863c9

🔬 Depth Floor

Challenge: The implementation and tests equate a thrown read with “silence,” but the actual no-bridge/no-verb early return is a distinct transition. At exact head, after an answered-empty roster or not-wired activity response, removing globalThis.AgentOS.fleet leaves the old “server connected” / “activity source not wired” reason in place while both surfaces remain sample.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: “silence-retraction symmetry” and “the connected claim must not outlive the connection” overshoot the early-return mechanics.
• Anchor & Echo summaries: loadRoster(), loadActivity(), and syncSpineBanner() say absence is cold truth, but neither early return clears its retained reason.
• [RETROSPECTIVE] tag: N/A — none added by the PR.
• Linked anchors: the related Fleet and cockpit nodes support the existing-wire/topology framing.

Findings: Rhetorical drift is blocking because it describes the central false-claim regression as closed while the exact absence transition still reproduces it. Tighten the prose to the repaired full matrix, or narrow any claim that remains unimplemented.

🧠 Graph Ingestion Notes

• [KB_GAP]: None found. The live Knowledge Base answer and current app/source contracts agree on Store-backed UI state and the existing authenticated bridge boundary.
• [TOOLING_GAP]: The mandatory global structure-map command crashed because it recursively read Docker backup payloads under ai/deploy/.neo-ai-data and exceeded Node's maximum string length. Targeted maps for apps/agentos and harness completed; the harness map also traversed .stage and node_modules, producing 2.8 MB of irrelevant output. This is a verified diagnostic-scope defect, not evidence against this patch's placement.
• [RETROSPECTIVE]: “Producer answered” and “producer absent” are separate state transitions. A thrown-call witness does not cover an early return before the call.

🎯 Close-Target Audit

• Close-targets identified: #16699 only, newline-isolated as Resolves #16699.
• #16699 is a leaf and is not epic-labeled; parent #14560 is related, not closed.

Findings: Close-target shape passes. Closure remains blocked by the source-ticket/evidence restatement below, not by target type.

📑 Contract Completeness Audit

• Originating ticket #16699 or parent epic #14560 contains a Contract Ledger matrix.
• Implemented PR diff matches a canonical ledger.

Findings: Both source bodies lack a Contract Ledger, while this PR changes two consumed surfaces: resolveUiFleetTransport() adds optional reason, and the brain-health response adds transport: {phase, mode, up, fleetPort, reason, error}. Backfill the canonical ledger on #16699 and align it with the exact shipped consumers, defaults/absence behavior, errors, and witnesses.

🪜 Evidence Audit

• PR body contains an explicit Evidence: L2 ... → L3 required ... declaration.
• The body identifies AC1/AC3 live-transition receipts as residual work on #16694.
• Close-target #16699 annotates those residuals as [L3-deferred — operator handoff needed].
• The body distinguishes the checkout ceiling from a live populated transport.
• Evidence-class collapse check: the body also says the cadence “meets” the observable auto-transition behavior despite explicitly admitting that behavior remains L3-unmeasured.
• Deployment causality is treated as post-merge/operator validation rather than proof from the unmerged head.

Findings: The evidence ladder declaration is honest, but the “met” language and source ticket are not synchronized with it. Restate the observable behavior as designed-but-L3-deferred unless an exact-head reachable artifact supplies L3.

📡 MCP-Tool-Description Budget Audit

Findings: N/A — no OpenAPI MCP server surface changes.

🔌 Wire-Format Compatibility Audit

• No new IPC channel or credential surface is introduced.
• The additive shell-internal brain-health.transport object is consumed by FleetCockpit.applyBrainHealth().
• The additive resolveUiFleetTransport().reason field is produced only for the refusal outcome and remains optional for reuse/spawn.
• The exact additive schemas, absence/default behavior, error semantics, consumers, and witnesses are captured in the source ticket's Contract Ledger.

Findings: Runtime wiring is coherent and additive; contract authority is missing, so compatibility cannot yet be audited against a stable ledger.

🔗 Cross-Skill Integration Audit

• No workflow skill, AGENTS_STARTUP.md, or MCP tool surface needs a predecessor update.
• The new shell fact remains on the existing health pull and introduces no new convention outside the app/harness boundary.
• The consumed wire-format additions are documented at their source-of-authority ticket.

Findings: No skill integration gap; the remaining integration gap is the same missing Contract Ledger, not a new workflow convention.

🧪 Test-Evidence & Location Audit

• Execution evidence: all required GitHub checks, including CodeQL and unit, are green at 13f1e1f39ec0db9ebe8172e812793d053fec14d5; the author supplied a first-paint smoke receipt and explicitly disclosed the pre-existing overall smoke failure.
• Reviewer falsifier: an exact-head Node probe loaded the standard unit setup plus the real FleetCockpit prototype, drove answered-empty/not-wired, deleted globalThis.AgentOS.fleet, and re-drove both methods. Result: roster retained server connected · fleet registry empty — define agents to go live; activity retained activity source not wired; both stayed sample.
• Test location: all six changed specs remain in the canonical Playwright unit trees for their app/harness owners.
• Test-evidence transcription: the six named changed specs execute 188 tests, not the PR body's claimed 201. The 201 total is reached only when the unchanged 13-test harness/appLifecycle.spec.mjs is also included.

Findings: CI is green, but the named falsifier fails the core silence-transition claim and the PR's scoped test count is not reproducible as written.

📋 Required Actions

To proceed with merging, please address the following:

• RA-1 — close the full answer → absence matrix. On both roster and activity surfaces, a transition from a producer answer to absent bridge/no verb must withdraw that producer-specific cause before rendering cold truth; preserve stale semantics for a surface that had already reached live. Add explicit tests for answered-empty → bridge/verb absent and not-wired → bridge/verb absent, rather than using a thrown call as the silence proxy.
• RA-2 — synchronize the closing ticket and contract authority. Amend #16699's body to the verified empty-registry root cause and cadence-plus-button decision; add a canonical Contract Ledger for the two additive consumed surfaces; and mark AC1/AC3 as L3-deferred to #16694 (or supply exact-head L3 evidence). Align PR/JSDoc claims with that disposition.
• RA-3 — correct the scoped test receipt. Report 188/188 for the six named changed specs, or name the seventh unchanged harness/appLifecycle.spec.mjs if 201/201 is the intended command surface.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 86 — existing lifecycle ownership, authenticated bridge use, Store-backed UI state, and one health wire are structurally sound; the missing ledger prevents full boundary audit.
• [CONTENT_COMPLETENESS]: 67 — extensive JSDoc and PR rationale are present, but the source ticket is stale, the public contract is undocumented, and the silence/test-count prose overshoots measured behavior.
• [EXECUTION_QUALITY]: 64 — current-head CI and 188 scoped tests pass, but a common early-return transition preserves a false connected/not-wired cause on both surfaces.
• [PRODUCTIVITY]: 72 — reconnect, topology-aware copy, and badge truth materially advance the leaf; the central “claim does not outlive connection” outcome is incomplete.
• [IMPACT]: 84 — this controls the operator's primary Fleet Manager connection diagnosis and recovery surface, so false guidance has high operational consequence.
• [COMPLEXITY]: 82 — thirteen app/harness/style/spec touchpoints coordinate async state, IPC topology, lifecycle facts, rendering, and smoke observables.
• [EFFORT_PROFILE]: Heavy Lift — cross-hemisphere lifecycle/UI integration with meaningful state-machine and evidence burden.

The architecture is salvageable in place. Please keep the repair centered on transition truth and source-contract synchronization; no new transport or push channel is warranted.

[review-budget-managed]

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