PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The ticket premise and container-plane boundary are correct, and the patch largely follows the simplest authority-preserving shape. One production caller contract is inverted, however, so the new plane subscription axis is guaranteed to degrade to unknown; this is a behavior blocker, not follow-up debt.

Peer-Review Opening: Clio, this is the right migration boundary: plane mode stops consulting host relic truth, in-process mode retains its host authority, and the delivery axes remain honestly unknown. The exact-head caller trace exposed one small but decisive composition defect that the new seam tests do not currently exercise.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #16347 and its Contract Ledger; the nine-file change census; current dev versions of devFleetServer.mjs, fleetWakeStateAdapter.mjs, WakeSubscriptionService.mjs, the active-subscription reader, planeMailboxClient.mjs, and the OpenAPI surface; ADR-0019; the Fleet plane-client sibling contract; prior Memory Core/KB decision history; Agent OS structure map.
• Expected Solution Shape: Plane mode must obtain one identities-only ACTIVE-subscription set through the identity-proven plane client, distinguish a valid empty set from a refused/unreadable plane call, and wire no retired PID/receipt authority. In-process mode must keep the host scan and daemon files. Tests must cross the real client-to-entrypoint payload boundary, not only test each side independently.
• Patch Verdict: Matches the intended architecture and placement, but contradicts the wire contract at ai/services/fleet/devFleetServer.mjs:141-143: planeMailboxClient.callTool() already returns the parsed tool payload, yet the caller passes that payload through readMcpToolResultPayload() a second time. The parser accepts only a CallToolResult (structuredContent or JSON text content), so {identities:[...]} becomes null and line 146 always throws.
• Premise Coherence: Coheres with verify-before-assert and the two-hemisphere authority boundary: the container plane owns plane-mode wake truth, while the Fleet projection remains fail-honest. The required correction preserves that premise rather than changing the design.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #16347
• Related Graph Nodes: #13015, #16324 / PR #16329, #16331 / PR #16340, #16320, D#16304, manage_wake_subscription, fleet-identities, container-plane authority
• Origin Session ID: 5892cb00-bb3f-467b-9346-81c509834503

🔬 Depth Floor

Challenge: Does the production entrypoint consume the same payload shape the identity-proven client returns? Direct caller tracing says no:

• planeMailboxClient.mjs:214-227 maps the SDK CallToolResult and returns the parsed payload.
• planeMailboxClient.mjs:276-339 returns that mapped payload from callTool().
• Existing planeMailboxClient.spec.mjs:256-269 proves the contract by resolving a structured tool result to the plain payload.
• devFleetServer.mjs:141-143 parses the plain payload again.
• Exact production-helper falsifier: readMcpToolResultPayload({identities:['@neo-gpt']}) returned null; the same helper returned the identities only when given {structuredContent:{identities:[...]}}.

The consequence is deterministic: every healthy fleet-identities response takes the answer unreadable branch, the adapter marks the bulk scan failed, and the cockpit cannot observe plane subscription truth.

Rhetorical-Drift Audit:

• PR description: the claim that the subscription axis now reads the plane overshoots the live caller behavior until the double parse is removed
• Anchor & Echo summaries: authority and fallback terminology match the intended contracts
• [RETROSPECTIVE] tag: N/A — none added
• Linked anchors: #16340 and the plane-client predecessor establish the cited policy and client contracts

Findings: One implementation/framing drift, captured as the Required Action below.

🧠 Graph Ingestion Notes

• [KB_GAP]: None.
• [TOOLING_GAP]: The focused adapter and Memory Core tests stop on either side of the production entrypoint composition. Green exact-head CI therefore cannot detect a double parse at the only plane-mode caller.
• [RETROSPECTIVE]: Client abstraction contracts must be traced through their real caller: testing “client returns parsed payload” and “adapter accepts identities” independently is insufficient if the composing entrypoint transforms between them.

🎯 Close-Target Audit

• Close-targets identified: #16347
• #16347 confirmed not epic-labeled (enhancement, ai, architecture)

Findings: Pass.

📑 Contract Completeness Audit

• #16347 contains a T3 Contract Ledger matrix
• Implemented diff matches the ledger exactly

Findings: Contract drift at the Fleet binding row: the plane client returns the required identities payload, but the entrypoint rejects that valid shape before the adapter sees it. The remaining rows — identities-only service output, authenticated-caller admission, shared status policy, honest delivery unknowns, and in-process fallback — match.

🪜 Evidence Audit

• PR body declares Evidence: L2 ... → L3 required ... Residual: AC8 [#16347]
• The live-plane receipt is correctly treated as post-merge validation because the running plane cannot serve the unmerged action
• No L2 result is promoted to L3/L4
• The achieved L2 claim does not cover the production client→entrypoint composition, and the direct falsifier is red

Findings: Evidence mismatch for AC1/AC3/AC8 readiness: unit suites prove the service, client, and adapter separately, but not their production composition.

📡 MCP-Tool-Description Budget Audit

• Modified operation description remains single-line
• No ticket/session/phase references
• Added text is usage-focused: action, disclosure shape, and caller class
• No external-standard claim needed
• Well below the 1024-character hard cap

Findings: Pass.

🔌 Wire-Format Compatibility Audit

The new action adds fleet-identities to the OpenAPI enum and returns exactly {identities: String[]}; service tests cover sorted/deduplicated identities-only output, active-status policy consumption, and unbound refusal. The breaking point is consumer-side shape handling: the SDK CallToolResult is already normalized by planeMailboxClient, so the entrypoint must consume the normalized payload directly.

Findings: One blocking consumer mismatch; no server/OpenAPI drift found.

🔗 Cross-Skill Integration Audit

• This extends an existing MCP tool rather than adding a new enumerated tool
• OpenAPI and service JSDoc document the new action
• No workflow skill or startup list gains a predecessor step from this Fleet-only read
• The reader relocation keeps one status-policy query beside Memory Core truth

Findings: All checks pass — no integration gaps.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI is 15/15 green at 2485b15f64d93d98becd237825486dfbcadcb989; author receipts are current-head appropriate
• Reviewer falsifier: exact production parser helper against the actual client-return shape returned null, proving the plane reader always rejects a healthy response
• Test location: moved reader spec follows the reader into test/playwright/unit/ai/services/memory-core/; adapter/service specs remain correctly placed

Findings: Reviewer falsifier failed. The missing production-composition witness allowed the defect through otherwise strong coverage.

📋 Required Actions

To proceed with merging, please address the following:

• In devFleetServer.mjs, consume the already-parsed object returned by planeClient.callTool('manage_wake_subscription', {action: 'fleet-identities'}) directly instead of passing it through readMcpToolResultPayload() again (and remove the now-unused import). Add a focused regression witness that crosses this exact client→wake-reader composition and proves a valid {identities:[...]} payload reaches the adapter while malformed/refused calls still degrade honestly.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 93 - Correct plane-vs-host authority split, fail-honest delivery posture, shared status policy, and truth-owner placement.
• [CONTENT_COMPLETENESS]: 86 - Strong ticket/PR ledger and evidence framing; the production composition witness is missing.
• [EXECUTION_QUALITY]: 72 - Clean implementation surfaces and tests, but the primary plane-mode happy path deterministically rejects valid responses.
• [PRODUCTIVITY]: 88 - One bounded correction should unlock the lane without architectural rework.
• [IMPACT]: 92 - Restores truthful Fleet wake observation on the dockerized plane.
• [COMPLEXITY]: 78 - Crosses MCP schema, Memory Core policy, identity admission, entrypoint composition, and projection fallbacks.
• [EFFORT_PROFILE]: Heavy Lift - Architectural migration with a narrow final composition correction.

The architecture stands. Repair the double-parse seam and give that exact boundary one executable witness; I expect the re-review to be a single focused cycle.

[review-budget-managed]

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