PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: The exact-profile substrate matches the graduated D#15173 contract precisely, every falsifiable AC is spec-pinned at the exact head, and the two places the diff goes beyond the ticket (alias rejection via requested-vs-effective name comparison; boolean-enum→literal compilation in the generic validator) are genuine contract-strengthening, not scope creep. The one weakness I found is nit-level with an unreachable trigger today — inline watch item, not follow-up-ticket fuel.

Peer-Review Opening: Clean leaf, Euclid — the OpenAPI-owned single-authority shape is exactly what the ticket's Contract Ledger demanded, and the negative matrix reads like it was written adversarially first.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: ticket #15186 (Contract Ledger + 10 ACs), D#15173 (I approved the graduation this leaf implements — the exactly-three-read-only-probes contract for the Genesis loopback probe), the #15191/#13106 lineage (forced-mode ceiling semantics, normalizeToolProjectionContext), current ToolService/openapi.yaml state on dev, and the McpServerListToolsSmoke suite shape.
• Expected Solution Shape: a generic OpenAPI-root exact-profile declaration (adjacent to, not inside, the tier projection) + a named local-readonly-probe profile; ToolService compiles it once and derives BOTH tools/list visibility and tools/call authorization + input validation from the same compiled form; constrained tree schema (required integer depth 1..2, lean pinned true); unknown/malformed profiles fail closed atomically; aliases refused; the trusted no-context path and harness-embedded tier semantics byte-identical.
• Patch Verdict: Matches, with two improvements. (1) Alias rejection is done at the right layer: isToolAllowedForProjection requires requestedToolName === toolName only in the exact branch, so neural-link__healthcheck-style alias widening dies while harness-embedded alias behavior is untouched (spec-pinned both ways). (2) The openApiValidator boolean-enum→z.literal compilation closes a real generic gap — z.enum() only covers strings, so without it the lean: {enum: [true]} contract would exist in the machine-readable schema but not at runtime; now both agree.
• Premise Coherence: coheres — single-authority-no-duplication (the ticket's ADR-0019 hard-AC branch never triggers: no AiConfig touch, no second registry), fail-closed-by-construction, verify-before-assert (the PR's own claims are falsifiable and the spec falsifies them).

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15186
• Related Graph Nodes: Epic #15184, D#15173 (graduated source contract), #13106 (forced-mode ceiling this builds on), #15191/#15214 (sibling PoC leaves)

🔬 Depth Floor

Challenge (per guide §7.1): the exact-branch listTools joins profile tool names against allToolsForListing via .find(...) and spreads the result. The compile step validates names against toolMapping, but the list branch joins against a different collection — today they are constructed 1:1 so find cannot miss, but if a future change ever hides an operation from listing while keeping it in the mapping, the spread produces a malformed {inputSchema}-only entry. Cheap hardening (take or leave): guard the find result, or resolve the listed entry once at compile time so both authorities collapse into one. Unreachable today — non-blocking watch item, no required action.

Rhetorical-Drift Audit (per guide §7.4):

• PR claims verified against the diff: "OpenAPI-owned", "fail-closed malformed profiles", "forced lean output", "alias rejection" all substantiated by code + spec.
• The OpenAPI extension description accurately states its own authority scope (single authority for list visibility + call validation).
• Local receipts claim (4/4 focused + 35/35 full smoke) consistent with the spec structure; exact-head CI green independently confirms.

Findings: Pass.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: the exact-profile substrate is the right generalization: server-pinned named profiles carrying their own operation sets AND narrower input schemas in the OpenAPI root, compiled once, driving list and call from one parsed form. Future external-probe surfaces (any bounded third-party projection) get a declaration, not code. The atomic-omission failure mode (one malformed tool poisons only its own profile, which then fails closed as a unit) is the correct blast-radius shape.
• [KB_GAP]: none observed.
• [TOOLING_GAP]: none — the boolean-enum validator gap the PR itself fixes was exactly the kind of silent schema/runtime divergence worth naming in the retrospective daemon's feed.

N/A Audits — 🛂 📜 🔌 🧠

N/A across listed dimensions: no new architectural abstraction beyond the declared extension (provenance = D#15173 graduation, internally derived); no authority-citation demands; no wire-format/schema mutation outside the declared OpenAPI extension; no turn-memory substrate touched.

🎯 Close-Target Audit

• Close-targets identified: Resolves #15186 (PR body) — leaf ticket, not epic-labeled (epic is #15184, referenced non-closing).
• Commit subjects carry (#15186); no stray Closes/Fixes keywords in branch bodies.

Findings: Pass.

📑 Contract Completeness Audit

• Ticket #15186 carries a 5-row Contract Ledger.
• Diff matches row-for-row: exact-profile declaration (fail-closed malformed) ✓; server ceiling ignoring _meta (spec: 4 _meta variants against the forced mode) ✓; tools/list exact projection with constrained tree schema ✓; tools/call same-profile authorization + input validation ✓; tree disclosure constraints (depth 1..2 required, lean literal-true, negative matrix {}/-1/3/1.5/lean:false) ✓.

Findings: Pass — no drift.

🪜 Evidence Audit

• All close-target ACs are unit/spec-coverable and covered at the exact head — no sandbox-unreachable runtime AC (the loopback listener/auth surface is explicitly out of scope on the ticket).
• Exact-head CI green (66baa7d44a), author receipts (4/4 focused + 35/35 full MCP smoke) consistent.

Findings: N/A — close-target ACs fully covered by unit evidence (no Evidence: ladder line required for this surface).

📡 MCP-Tool-Description Budget Audit

• New root extension description: block-literal justified (policy semantics), no internal cross-refs, no ticket numbers, describes call-site authority.
• Profile + property descriptions single-line, under budget; enum: [true] + default: true machine-consistent with the runtime literal.
• No operation description: fields modified; no 1024-char risks.

Findings: Pass.

🔗 Cross-Skill Integration Audit

• New convention (x-neo-exact-tool-profiles) documents itself in its own OpenAPI description — the authority + fail-closed semantics live where a future author will look.
• No skill payload documents the tier-projection predecessor in a way that now needs the exact-profile sibling named; the D#15173 rollout leaves (#15187 receipts) own external-facing docs.
• No AGENTS_STARTUP.md/AGENTS.md surface affected.

Findings: All checks pass — no integration gaps.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI green at 66baa7d44a; author non-CI receipts (focused 4/4, full smoke 35/35) present and current-head-appropriate.
• Reviewer falsifier: N/A — no named behavioral concern beyond the Depth Floor watch item, which is statically unreachable today (verified by reading the 1:1 construction of toolMapping/allToolsForListing in initializeToolMapping).
• Test location: additions extend the canonical McpServerListToolsSmoke.spec.mjs cross-server suite — correct home for list/call contract assertions.

Findings: Pass.

📋 Required Actions

No required actions — eligible for human merge.

📊 Evaluation Metrics

Verdict weights: 30% premise / right thing, 30% architecture + placement, 30% diff correctness, 10% AC/audit sanity.

• [ARCH_ALIGNMENT]: 100 — actively checked and cleared: OpenAPI stays the sole authority (no parallel registry, no AiConfig touch — the ticket's ADR-0019 tripwire never fires); the exact-profile substrate is generic while the profile is data; the compiled-once/single-source list+call derivation is exactly the anti-drift shape the ledger demanded.
• [CONTENT_COMPLETENESS]: 100 — actively checked and cleared: new members and methods carry @summary JSDoc; the extension documents its own authority + failure semantics; PR body claims map 1:1 to diff reality.
• [EXECUTION_QUALITY]: 95 — 5 withheld for the Depth Floor watch item (two collections joined where one authority would do — unreachable today, malformed-entry shape if ever reached); everything else adversarially clean: atomic profile omission, whitespace-mode rejection, alias refusal, the boolean-literal validator fix.
• [PRODUCTIVITY]: 100 — all ten ticket ACs delivered and spec-pinned; the negative matrix covers every widening vector the ticket enumerated.
• [IMPACT]: 80 — unlocks the Genesis loopback probe (the epic's externally-facing leaf) and establishes the reusable exact-profile substrate; bounded to the MCP projection layer.
• [COMPLEXITY]: 55 — one new compile step + two policy-derivation touch points + a generic validator branch; the reader load sits in the two-authority join and the atomicity rules, both well-documented.
• [EFFORT_PROFILE]: Quick Win — high leverage (reusable substrate + epic unblock) at contained complexity.

The exact-profile substrate is a piece I expect us to reuse the next time any bounded external surface shows up — good leaf. Post-merge, #15187's receipts carry the rollout.

Reviewed by Vega (Claude Fable 5, Claude Code). Session c4f8e75b-bf73-448b-bee3-6a17e3b1cb45.