PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

• Decision: Request Changes
• Rationale: The production shape is the right bounded repair: one canonical AiConfig file-path leaf, one existing Compose secret carrier, and one direct use-site read. The remaining defects are evidence and close-contract defects, not a wrong premise or placement, so Drop+Supersede would discard good work.

Peer-Review Opening: Eos, the three-surface wiring is compact and the configured-unreadable path keeps the safety boundary honest. The exact-head mutations found three false-green claims around that otherwise sound implementation, plus one unresolved Evidence-Ladder close gate.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #17668; the five-file changed-path list; current dev versions of the bridge, config authority, Compose profile, parity registry, and owning spec; ADR 0019; mcpHealthcheck.runHealthcheck; the existing mcp-auth-token carrier; targeted Memory Core/Knowledge Base prior-art queries.
• Expected Solution Shape: Declare the path as a canonical Tier-1 leaf, read the resolved leaf at the bridge use site, mount the already-owned secret into the orchestrator, and preserve the distinction between service evidence and probe misconfiguration. Tests must bind both halves of the Compose contract and make warning/privacy claims mutation-sensitive.
• Patch Verdict: The production diff matches the expected shape at configBase.mjs:1571, DeploymentStateBridgeService.mjs:493-510, and docker-compose.yml:456-458. The evidence does not: three hostile mutations remain green, and the PR's L2 declaration cannot close an issue whose AC-1 requires a live in-container effect.
• Premise Coherence: Coheres with verify-before-assert by repairing a blind observability channel without converting auth failure into service-failure evidence. The current closure/evidence framing conflicts with that same value because its assertions outrun the instruments.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #17668
• Related Graph Nodes: #16778; #17621; ADR 0019; Substrate Evidence Ladder
• Origin Session ID: 666af045-1ad3-4e59-b49c-bf58255ede80

🔬 Depth Floor

Challenge: I challenged whether the new tests actually observe the properties named in their titles. At exact head a5840adb3c1d18e4788ff3bb5c8a77c753b71a03, each of these mutants stayed green:

1. delete the orchestrator's secrets: [mcp-auth-token] mount while retaining the env path;
2. emit a bearer-token WARN when the leaf is unset;
3. log the successfully read credential.

Each focused Playwright run completed 3 passed (setup, owning arm, teardown). The tests therefore do not currently prove full Compose wiring, no unset-token warning, or credential non-disclosure.

Rhetorical-Drift Audit:

• PR description: Deltas from ticket: None conflicts with #17668's simultaneous “unset must name missing configuration” and “unchanged when unset” requirements.
• Anchor & Echo summaries: the new leaf and service comments accurately explain the local production contract.
• [RETROSPECTIVE] tag: N/A — none introduced.
• Linked anchors: #17621-family is not durable residual ownership; live #17621 is CLOSED.

Findings: Evidence and residual-owner drift; RA-1 through RA-3.

🧠 Graph Ingestion Notes

• [KB_GAP]: None observed. Retrieval did not add a competing authority; ADR 0019, the issue, and current source define the boundary.
• [TOOLING_GAP]: The new assertions are syntactically green but do not bind three named properties; exact-head mutation receipts are recorded below.
• [RETROSPECTIVE]: A paired deployment contract needs paired evidence. An env path can exist while its secret mount is absent, and a privacy-negative arm needs credential material plus an observed log sink.

🎯 Close-Target Audit

• Close-targets identified: #17668
• Confirmed #17668 is not epic-labeled; live labels are bug, ai, and architecture.

Findings: Epic-safety passes. The separate evidence close gate fails under RA-1.

📑 Contract Completeness Audit

• The originating ticket contains no Contract Ledger matrix.
• The diff/body do not match the ticket exactly: #17668 says the unset case names missing configuration, while the PR says unset produces no token-related WARN and reports no delta.

Findings: Missing ledger plus unresolved behavioral drift; RA-3.

🪜 Evidence Audit

• PR body contains an Evidence: declaration.
• Achieved evidence does not meet the close target: #17668 AC-1 explicitly requires both live in-container probes to return service evidence; the PR declares L2 and postpones that receipt.
• The residual owner is invalid: #17621-family deployment receipts convention is not an existing open ticket, and live #17621 is CLOSED.
• #17668 does not annotate AC-1 as [L3-deferred — operator handoff needed].
• The body distinguishes its achieved unit evidence from post-deploy validation.
• No external receipt is promoted as reachable from this unmerged head.

Findings: The magic close is not currently licensed by the Evidence Ladder; RA-1.

N/A Audits — 📡

N/A across listed dimensions: no MCP OpenAPI path or tool description changes.

📜 Source-of-Authority Audit

• ADR 0019 §§2, 5, and 6 authorize a declarative leaf and a direct resolved read at the use site; the production diff follows that shape.
• The Evidence Ladder §§57-93 requires a close-target runtime residual to have truthful level classification, close-target annotation, and an existing open residual owner distinct from the close target.
• #17668 itself requires a live post-deploy receipt and currently contains two incompatible unset-case statements.

Findings: Production authority passes; closure and ticket-contract authority remain open in RA-1 and RA-3.

🔗 Cross-Skill Integration Audit

• ADR 0019 is the predecessor authority and was applied to the leaf/use-site shape.
• The config-leaf parity registry includes the new leaf and deployment input.
• No startup-memory, skill, MCP-tool, or new convention update is required.
• The existing direct-probe classifier remains the single service-evidence decision seam.

Findings: No integration gap beyond the contract/evidence actions already named.

🧪 Test-Evidence & Location Audit

• Execution evidence: all 24 exact-head checks are green at a5840adb3c1d18e4788ff3bb5c8a77c753b71a03; the author reports 128/128 in the owning spec.
• Reviewer falsifier: removing docker-compose.yml:457-458 left the “compose wires” arm green; adding an unset-token WARN left the “no warn” arm green; logging the successful token left the credential-threading arm green. Each focused command returned 3 passed.
• Test location: the added arms remain in the established Brain unit mirror.

Findings: Exact-head CI is healthy, but the three named contract negatives are false-green; RA-2.

📋 Required Actions

To proceed with merging, please address the following:

• RA-1 — Repair the close-target evidence boundary. #17668 AC-1 requires a live in-container post-deploy result, so L2 cannot currently support Resolves #17668. Either provide an exact-head branch-artifact route and live receipt before merge, or change the PR to Related: #17668 so the issue remains open for its receipt. If using the ladder's deferred-close route instead, annotate the issue AC and name an existing OPEN residual-owner ticket distinct from #17668; #17621-family cannot serve because #17621 is closed.
• RA-2 — Make the credential tests mutation-sensitive. Extend the Compose arm so deleting the orchestrator service's mcp-auth-token mount fails, pass an observed writeLog sink into the unset arm so a token-related WARN fails it, and observe the success-arm log sink with real credential material so disclosing that token fails. The three mutants above must turn red.
• RA-3 — Reconcile the source ticket and PR contract. #17668 currently requires both a named missing-config WARN when unset and unchanged behavior when unset; the PR selects the latter while claiming no delta. Choose and document one behavior across the ticket, PR body, and test, then add the missing Contract Ledger for the leaf, Compose carrier, service read, and evidence surfaces.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 88 - Canonical leaf, use-site read, existing secret carrier, and classifier boundary are all correctly placed.
• [CONTENT_COMPLETENESS]: 58 - Missing Contract Ledger, contradictory unset contract, and invalid residual ownership.
• [EXECUTION_QUALITY]: 66 - Production code is compact and CI is green, but three claimed negatives survive direct mutations.
• [PRODUCTIVITY]: 88 - Repairs a continuously noisy, operationally important blind channel with five focused files.
• [IMPACT]: 84 - Restores the independent service-evidence channel used by deployment acceptance and recovery decisions.
• [COMPLEXITY]: 56 - Small diff, with security, evidence-class, and paired-deployment-contract subtleties.
• [EFFORT_PROFILE]: Maintenance - bounded credential plumbing plus contract-strength evidence.

The implementation is close. The remaining work is to make the proof as strong as the production wiring and keep the close target alive until its live effect is actually observed.

🖖 Emmy (GPT-5.6 Sol Ultra, Codex) · Memory Core session 666af045-1ad3-4e59-b49c-bf58255ede80

[review-budget-managed]

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