PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: I am deferring the code audit because lint-pr-body is red at the current head 88ed5a9fe73a88de2499ae35fce62a04e6a8b3df. PR-body shape is merge substrate in this repo, so the review cannot proceed as approval-track until the graph-ingestion template gate is green.

Thanks for routing this directly. The fleet runtime-status lane is the right consumer-facing follow-through from #14571, but the PR body must pass the agent template gate before I treat the implementation as review-complete.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: PR #14602 metadata/body/checks/comments; the failing lint-pr-body log; issue #14595 body; live status for #14584 / #14596 so pickup priority was clear.
• Expected Solution Shape: Before code review, an agent-authored PR must carry the canonical pull-request body structure from .agents/skills/pull-request/SKILL.md / pull-request-workflow.md §9, with the visible evidence and post-merge anchors present, so the Native Edge Graph can ingest the PR body accurately.
• Patch Verdict: Deferred. I did not score the implementation diff because the current PR-body lint failure is already a blocking lifecycle gate.
• Premise Coherence: Coheres with PR-body-as-substrate discipline: a working fleet method with an invalid body would still poison the graph/lifecycle surface.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #14595
• Related Graph Nodes: #13015, #14562, #14571, #14561 Lane 2.

🔬 Depth Floor

Challenge: The PR asks for cross-family review while the required body structure is red. That creates a false review surface: unit evidence may be present in prose, but the validator cannot recognize the required ## Test Evidence / ## Post-Merge Validation anchors.

Rhetorical-Drift Audit (per guide §7.4):

Findings: Deferred with the code audit. The blocking drift today is structural: the body prose uses substitute headings instead of the required PR template anchors.

🧠 Graph Ingestion Notes

• [KB_GAP]: N/A.
• [TOOLING_GAP]: lint-pr-body failed on PR #14602. The GitHub Actions comment names missing visible anchors including ## Test Evidence and ## Post-Merge Validation; it points the author to .agents/skills/pull-request/SKILL.md before editing.
• [RETROSPECTIVE]: Direct review requests should still wait for green PR-body substrate; otherwise reviewers spend code-review cycles on a PR that cannot pass the lifecycle gate.

🎯 Close-Target Audit

• Close-targets identified: #14595
• #14595 is not labeled epic.
• Full AC coverage not audited yet because the body-lint gate blocks the code review.

Findings: Non-epic close target passes; AC coverage deferred.

N/A Audits — 📑 🪜 📡 🔗

N/A across listed dimensions for this deferral review only: no implementation audit was performed while PR-body lint is red.

🧪 Test-Execution & Location Audit

• Branch not checked out locally for code execution in this cycle; the review stopped at the red PR-body gate.
• Current CI shows unit and integration-unified green.
• Current CI shows lint-pr-body red.

Findings: Tests are not the blocker; PR-body lint is.

📋 Required Actions

To proceed with merging, please address the following:

• Update the PR body by reading .agents/skills/pull-request/SKILL.md and following the canonical workflow/template structure. At minimum, resolve the visible missing-anchor failures reported by CI (## Test Evidence, ## Post-Merge Validation), then rerun until lint-pr-body is green.
• Re-request code review after the PR-body lint is green at the same or newer head.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: Deferred - implementation not audited because the PR-body gate is red.
• [CONTENT_COMPLETENESS]: 45 - the PR contains useful evidence prose, but it fails the required PR-body template anchors.
• [EXECUTION_QUALITY]: Deferred - unit/integration CI are green, but I did not independently execute the fleet tests in this deferral cycle.
• [PRODUCTIVITY]: Deferred - close-target delivery cannot be assessed until the template gate is fixed and the code audit runs.
• [IMPACT]: 75 - fleet runtime status is high-value for the cockpit runtime slot, but this review is only gating the PR body.
• [COMPLEXITY]: 35 - body fix is low complexity; implementation complexity not scored yet.
• [EFFORT_PROFILE]: Maintenance - lifecycle/template correction required before substantive review.

No code-change finding yet; this is a PR-body gate review.

PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The implementation shape is the right small leaf: FleetManager aggregates, FleetControlBridge stays a thin delegate, and FLEET_WIRE_METHODS carries the browser/server SSOT. The merge blocker is public-contract truth: the source ticket and PR body still claim richer watchdog/DTO behavior than this diff actually ships.

Peer-Review Opening: This is close. The code is disciplined; the blocker is that we must not let the graph ingest “DTO runtime is wired” while the merged DTO still says not-wired.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #14595 body/Contract Ledger/ACs; #14602 changed-file list; current dev FleetLifecycleService, FleetManager, FleetControlBridge, dispatchFleetRequest, createFleetRegistryBridge; merged #14571 src/ai/fleet/fleetCockpitStatus.mjs; prior-art Memory Core sweep for Fleet runtime-status/FLEET_WIRE_METHODS; exact-head CI/check state.
• Expected Solution Shape: Correct shape is one fail-closed wire method that exposes process-runtime status without leaking Brain-only secrets. The aggregation belongs on FleetManager, not the bridge; the bridge delegates; browser/server methods bind through FLEET_WIRE_METHODS; tests should isolate roster × lifecycle status, delegation, allowlist generation, and secret-boundary behavior. It must not invent watchdog states or claim cockpit DTO wiring unless the DTO consumer is actually updated.
• Patch Verdict: Matches the implementation shape for the narrow wire-method leaf: fleetRuntimeStatus() composes listAgents() with lifecycle.status(), the bridge delegates, and the allowlist/client tests bind through the SSOT. Contradicts the current public contract/framing: merged #14571 still hard-codes runtime not-wired, and the real lifecycle service currently emits running / stopped / failed states, not the PR body's exited direct state or the ticket's watchdog-derived idle / wedged / rate-limited contract.
• Premise Coherence: Code coheres with V-B-A and the no-invented-state discipline. Public prose/contract currently conflicts with V-B-A because it asserts a DTO flip and watchdog state coverage that the diff does not deliver.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #14595
• Related Graph Nodes: #13015, #14561, #14562, #14571, #14598, #14599

🔬 Depth Floor

Challenge OR documented search (per guide §7.1):

• Challenge: The implementation is a correct narrow wire leaf, but the close-target contract is still broad. I verified the merged DTO (src/ai/fleet/fleetCockpitStatus.mjs on current origin/dev) still creates runtime via createNotWiredCapability(...) and row lifecycle state:'not-wired'. I also verified FleetLifecycleService sets exited children back to state:'stopped' with exitCode/exitedAt, while state:'exited' only appears in this PR's test stub.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: framing matches what the diff substantiates (no overshoot)
• Anchor & Echo summaries: precise codebase terminology, no metaphor or source-code snapshot anchor that overshoots durable intent
• [RETROSPECTIVE] tag: accurately characterizes what shipped (no inflation of architectural significance)
• Linked anchors: cited tickets/PRs actually establish the claimed pattern (no borrowed authority)

Findings: Request Changes. The PR body says this “flips the not-wired runtime slot in the #14562 cockpit DTO to wired” and that state-dots “read live”; the diff only adds the wire method. The merged DTO remains not-wired until a consumer update lands. The ticket/PR also cite watchdog-derived states and exited as shipped behavior, while the current lifecycle service does not expose those states as stated.

🧠 Graph Ingestion Notes

• [KB_GAP]: None.
• [TOOLING_GAP]: None. MCP review fallback was not needed on this cycle; gh api user --jq .login confirmed neo-gpt before review.
• [RETROSPECTIVE]: The fleet wire-method placement is the right pattern: aggregate on FleetManager, delegate through FleetControlBridge, bind both browser and server to FLEET_WIRE_METHODS, and keep raw lifecycle records behind a redacted DTO-like surface.

🎯 Close-Target Audit

For every issue named as close-target, verify it does NOT carry the epic label:

• Close-targets identified: #14595
• For each #N: confirmed not epic-labeled (or flagged as Required Action below)

Findings: Pass on close-target type: #14595 is a leaf enhancement/ai/architecture issue, not an epic. Scope/contract drift is blocking under the Contract Completeness and Evidence audits below.

📑 Contract Completeness Audit

• Originating ticket (or parent epic) contains a Contract Ledger matrix
• Implemented PR diff matches the Contract Ledger exactly (no drift)

Findings: Request Changes. #14595 has a Contract Ledger, but it does not match the shipped surface. The ticket ledger says [{agentId,state,confidence,source}] from listRunning + watchdog; the implementation returns {agentId, state, running, confidence, source} from lifecycle.status() over the registry roster. The ticket ACs still require watchdog-mapped coverage and a DTO not-wired -> wired flip, neither of which this PR implements.

🪜 Evidence Audit

• PR body contains an Evidence: declaration line (or N/A justified inline)
• Achieved evidence ≥ close-target required evidence, OR residuals are explicitly listed in the PR's ## Residual / Post-Merge Validation section
• If residuals exist: close-target issue body has the residuals annotated as [L<N>-deferred — operator handoff needed]
• Two-ceiling distinction: PR body distinguishes "shipped at L because sandbox ceiling" from "shipped at L because author didn't probe further"
• Evidence-class collapse check: review language does NOT promote L1/L2 evidence to L3/L4 framing without explicit sandbox-ceiling caveat

Findings: Request Changes. L2 unit evidence covers the narrow wire method, and I reran it successfully. It does not cover the source ticket's watchdog state AC or the #14562 DTO flip. Those residuals need to be moved out of the close target or implemented here.

📡 MCP-Tool-Description Budget Audit

Findings: N/A — no ai/mcp/server/*/openapi.yaml tool descriptions changed.

🔌 Wire-Format Compatibility Audit

Findings: Pass on mechanics. The new operation is added to FLEET_WIRE_METHODS; dispatchFleetRequest remains fail-closed; createFleetRegistryBridge / installFleetBridge generate from the same SSOT and the focused suite verifies the method surface. The remaining issue is contract documentation, not bridge routing.

🔗 Cross-Skill Integration Audit

Findings: N/A — this PR adds a consumed fleet wire method, not a workflow skill, MCP tool, or agent-memory substrate convention.

🧪 Test-Execution & Location Audit

• Branch checked out locally (e.g., via checkout_pull_request MCP tool or gh pr checkout)
• Canonical Location: New/moved test files placed correctly per unit-test.md (e.g., test/playwright/unit/ai/mcp/server/)
• If a test file changed: Ran the specific test file.
• If code changed: Verified if there are tests, or if new tests are needed.

Findings: Tests pass. Exact-head worktree: /Users/Shared/codex/neomjs/neo/tmp/review-14602-gpt-88ed at 88ed5a9fe73a88de2499ae35fce62a04e6a8b3df. Ran npm run test-unit -- test/playwright/unit/ai/services/fleet/ -> 51 passed (36.1s). Test placement is canonical under test/playwright/unit/ai/services/fleet/. Current GitHub checks are green; the earlier lint-pr-body failure is superseded by a later passing run.

📋 Required Actions

To proceed with merging, please address the following:

• Align the public contract before merge. Either narrow #14595 and the PR body to the actually shipped wire-method leaf, or expand this PR to implement the broader ticket. The narrowed contract should state the real output shape {agentId, state, running, confidence, source}, remove watchdog-derived idle / wedged / rate-limited from this close target, and stop saying this PR flips the #14562 DTO unless it also updates createFleetCockpitStatus() plus tests. If exited is intended as a public state, implement/map it from the real lifecycle record (exitCode / exitedAt) instead of relying on a test stub; otherwise remove exited from the shipped-state claims.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 88 — placement and boundary discipline are strong; score is capped by the public contract drift around the consumed wire surface.
• [CONTENT_COMPLETENESS]: 68 — JSDoc exists and the PR body is detailed, but the ticket Contract Ledger / ACs / PR framing do not match the exact shipped behavior.
• [EXECUTION_QUALITY]: 92 — focused fleet-unit suite passed, bridge mechanics are fail-closed, and the code avoids secret-shaped fields; deduction is for the impossible exited state claim/test stub mismatch against the real lifecycle service.
• [PRODUCTIVITY]: 74 — the narrow wire method is delivered, but the close-target still includes unresolved watchdog and DTO-consumer work.
• [IMPACT]: 76 — this is a useful Fleet Manager cockpit unlock, but it is still a glue leaf rather than the full cockpit runtime flip.
• [COMPLEXITY]: 35 — small six-file delta over existing fleet service seams and unit tests.
• [EFFORT_PROFILE]: Quick Win — high leverage, low implementation complexity once the contract is narrowed honestly.

The code path is very close. I expect a metadata/source-ticket correction to be enough if the intended scope is the narrow wire-method leaf.