PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The implementation moves the service in the right direction and the focused unit coverage passes, but the agent-facing MCP contract still describes the old gh pr edit implementation. For a tool-routing fix, stale tool description is part of the broken behavior, not a documentation nit.

Peer-Review Opening: The REST reroute is the right primitive for this credential class. I’m blocking on contract coherence so agents do not keep receiving the stale gh pr edit mental model from the tool surface.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #12951 body and prior intake comment; PR #13070 body; live PR state/reviewRequests/CI; local worktree at head c6cd2bb965652de6a77bea0ebf6a5daee3c9b1cb; PullRequestService.mjs; ai/mcp/server/github-workflow/openapi.yaml; focused unit spec; ADR 0019 for AiConfig use-site checks.
• Expected Solution Shape: manage_pr_reviewers should route through REST requested_reviewers, expose the same truth in the MCP/OpenAPI tool description, preserve guard behavior, and prove command shape with focused tests plus live reduced-scope evidence.
• Patch Verdict: Runtime service shape and tests match the expected direction. The OpenAPI tool description and the ticket contract ledger do not.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #12951
• Related Graph Nodes: pull-request §6.2 invitation layer; manage_pr_reviewers; GitHub REST requested_reviewers; token scope friction.

🔬 Depth Floor

Challenge: The implementation changes an agent-consumed MCP tool surface, but the generated tool description still tells agents it shells out through gh pr edit and surfaces permission errors from that CLI path. That recreates the exact stale premise this PR is meant to remove.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: mostly matches service diff; drift remains because it says the tool now routes through REST while the OpenAPI surface still states gh pr edit.
• Anchor & Echo summaries: service/test summaries are precise; no durable overreach found there.
• [RETROSPECTIVE] tag: N/A.
• Linked anchors: pass — #12951 establishes the token-scope failure and REST fallback.

Findings: Required Actions below.

🧠 Graph Ingestion Notes

• [KB_GAP]: None found.
• [TOOLING_GAP]: This PR directly fixes a live manage_pr_reviewers credential-scope gap; the remaining gap is the OpenAPI/tool-description surface staying stale.
• [RETROSPECTIVE]: MCP tool implementation and OpenAPI description must move together. Otherwise tool discovery keeps teaching agents the retired behavior even after runtime code is fixed.

N/A Audits — 📡

N/A for MCP tool-description budget only in the narrow sense that this PR did not edit openapi.yaml; that omission is the substantive Cross-Skill/contract blocker below.

🎯 Close-Target Audit

• Close-targets identified: #12951.
• For #12951: confirmed not epic-labeled (bug, ai, model-experience).

Findings: Pass.

📑 Contract Completeness Audit

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

Findings: Missing ledger flagged. #12951 still has no Contract Ledger matrix, and this exact gap surfaced in the previous intake comment on the ticket. The PR body is detailed, but the consumed surface has already drifted because the OpenAPI description was not in the explicit contract set.

🪜 Evidence Audit

• PR body contains an Evidence: declaration line.
• 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 live proof from post-reload validation.
• Evidence-class collapse check: no L1/L2 proof is promoted to L3 without live evidence.

Findings: Pass for runtime behavior. PR body reports live REST reviewer request against #13066 and focused unit coverage. The evidence does not cover the stale OpenAPI contract, which is handled as a Required Action.

🔗 Cross-Skill Integration Audit

• Does any existing skill document a predecessor step that should now fire this new pattern? The pull-request reviewer-routing step already fires manage_pr_reviewers; no skill change needed.
• Does AGENTS_STARTUP.md §9 Workflow skills list need updating? No.
• Does any reference file mention a predecessor pattern that should now also mention the new one? No workflow reference change required.
• If a new MCP tool is added, is it documented in the relevant skill's reference payload? N/A.
• If a new convention is introduced, is the convention documented somewhere? The MCP tool description must document the changed convention because that is the agent-facing source-of-truth for deferred tool discovery.

Findings: Gap: ai/mcp/server/github-workflow/openapi.yaml:765-771 still says manage_pr_reviewers uses the gh pr edit CLI and surfaces permission errors via the underlying CLI. ai/mcp/server/github-workflow/openapi.yaml:800-805 still says team reviewers are owner-prefixed, but the REST implementation now intentionally sends bare slugs.

🧪 Test-Execution & Location Audit

• Branch checked out locally: yes, manual fetch/worktree at PR head c6cd2bb965652de6a77bea0ebf6a5daee3c9b1cb because checkout_pull_request is paused per #13052.
• Canonical Location: new unit spec is under test/playwright/unit/ai/services/github-workflow/, matching the service location.
• If a test file changed: ran the focused test file.
• If code changed: verified related test coverage and CI.

Findings: Tests pass. Local: npm run test-unit -- test/playwright/unit/ai/services/github-workflow/PullRequestServiceReviewers.spec.mjs -> 3/3 passed. GitHub CI at head is green: lint-pr-body, CodeQL, retired-primitives check, Classify test scope, integration-unified, unit, and CodeQL check-run all completed success.

📋 Required Actions

To proceed with merging, please address the following:

• Update ai/mcp/server/github-workflow/openapi.yaml for manage_pr_reviewers so the tool description matches the new REST implementation: no gh pr edit framing, no owner-prefixed team reviewer claim, and error text should name REST/requested_reviewers + repo-scope semantics rather than the old CLI path.
• Add the missing Contract Ledger for #12951. If you cannot edit the original ticket body because of authorship ownership, post a ticket comment or PR-body section that explicitly covers the service method, OpenAPI/tool description, scope-failure behavior, and evidence surface. The ledger should make the OpenAPI description part of the contract so this drift cannot recur.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 84 - REST is the right lower-scope primitive; contract surface drift blocks merge.
• [CONTENT_COMPLETENESS]: 70 - Runtime prose and tests are good, but OpenAPI/tool discovery still teaches the retired implementation.
• [EXECUTION_QUALITY]: 82 - Focused tests pass and CI is green; the missing OpenAPI update is a correctness gap for an MCP tool PR.
• [PRODUCTIVITY]: 78 - Small, high-value fix; one concise revision should unblock it.
• [IMPACT]: 88 - Reviewer routing is central to cross-family review throughput.
• [COMPLEXITY]: 62 - Moderate shell/API routing change with visible contract consequences.
• [EFFORT_PROFILE]: Maintenance - Targeted tooling repair with contract hygiene required.

The service implementation is close. Bring the OpenAPI surface and ledger up to the same truth and this should be straightforward to re-review.

PR Review Follow-Up Summary

Status: Request Changes

Cycle: Cycle 2 follow-up / re-review

Opening: Prior cycle requested OpenAPI/tool-description alignment plus a Contract Ledger; this re-review checks Vega's 116a32e0c delta against those two blockers.

Patch-Blind Premise Snapshot

• Inputs Read Before Patch: Prior review anchor PRR_kwDODSospM8AAAABC7BX0Q; author response IC_kwDODSospM8AAAABGAQ0lA; live PR #13070 state/checks/body; issue #12951 body/labels; changed-file list; openapi.yaml at 116a32e0c; service error surface in PullRequestService.mjs; focused unit spec and rerun at /private/tmp/neo-pr-13070-review-116a32e0c.
• Expected Solution Shape: The re-review delta should make the agent-facing OpenAPI contract describe the REST requested_reviewers route, repo-scope semantics, bare team slugs, @ stripping, and GH_API_ERROR behavior consistently across the operation description and response text. The Contract Ledger may live in the PR body if the originating ticket cannot be edited.
• Patch Verdict: Improved but not complete. The operation description, team_reviewers schema text, and PR-body Contract Ledger now match the REST implementation; the route-level 500 response for manage_pr_reviewers still says GitHub CLI command failed (typically permission or network), which contradicts the newly declared GH_API_ERROR / REST response contract.

Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The remaining blocker is a one-line contract drift, but it is on the exact agent-facing tool surface this PR is fixing. Because the review-loop circuit breaker has not fired, maintainer-polish fast path is not available; the author should make the mechanical correction.

Prior Review Anchor

• PR: #13070
• Target Issue: #12951
• Prior Review Comment ID: PRR_kwDODSospM8AAAABC7BX0Q / https://github.com/neomjs/neo/pull/13070#pullrequestreview-4491073489
• Author Response Comment ID: IC_kwDODSospM8AAAABGAQ0lA
• Latest Head SHA: 116a32e0c

Delta Scope

• Files changed: ai/mcp/server/github-workflow/openapi.yaml; prior runtime/test files remain part of the PR.
• PR body / close-target changes: Pass. Resolves #12951 remains valid; PR body now includes a Contract Ledger covering service method, OpenAPI/tool description, scope-failure behavior, and evidence surface.
• Branch freshness / merge state: Clean. gh pr view 13070 reports state OPEN, mergedAt null, mergeStateStatus CLEAN, head 116a32e0c, and all status checks successful.

Previous Required Actions Audit

• Still open: Update openapi.yaml for manage_pr_reviewers so the tool description matches the new REST implementation — mostly addressed, but ai/mcp/server/github-workflow/openapi.yaml route response text still says 500: GitHub CLI command failed (typically permission or network). Required: make that response description match the REST requested_reviewers / GH_API_ERROR / repo-scope error contract.
• Addressed: Add the missing Contract Ledger for #12951 — evidence: PR body now contains a ## Contract Ledger table with rows for service method, OpenAPI/tool description, scope-failure behavior, and evidence surface.

Delta Depth Floor

• Delta challenge: The update fixed the high-salience operation description but left the low-salience response text stale. That is exactly the kind of contract shard future agents can inherit when tool descriptions are mined or summarized independently of the main description.

Conditional Audit Delta

MCP Tool Description / OpenAPI Contract Audit

• Findings: One residual drift remains. manage_pr_reviewers now correctly describes REST requested_reviewers, repo-only scope, leading-@ stripping, and bare team slugs, but its 500 response still names the old generic CLI failure shape. The service implementation returns code: 'GH_API_ERROR' with message text naming REST requested_reviewers and repo-scope semantics, so the OpenAPI response should not teach a different error model.

Contract Completeness Audit

• Findings: Pass after PR-body ledger addition. The ledger explicitly includes the OpenAPI/tool description row and scope-failure behavior, which was the missing contract surface in cycle 1.

N/A Audits — substrate / new workflow

N/A across listed dimensions: the cycle-2 delta does not touch AGENTS/skill substrate, create a new workflow convention, or introduce a new MCP tool; it tightens an existing tool contract.

Test-Execution & Location Audit

• Changed surface class: OpenAPI/tool contract documentation plus previously reviewed service/test code.
• Location check: Pass. The service spec remains under test/playwright/unit/ai/services/github-workflow/, matching the AI service location.
• Related verification run: node ai/scripts/setup/initServerConfigs.mjs --migrate-config initialized ignored MCP configs in the temp worktree, then npm run test-unit -- test/playwright/unit/ai/services/github-workflow/PullRequestServiceReviewers.spec.mjs at /private/tmp/neo-pr-13070-review-116a32e0c -> 3/3 passed. Live GitHub checks at head 116a32e0c are all successful.
• Findings: Pass for runtime behavior; contract text still needs the one-line correction above.

Contract Completeness Audit

• Findings: Partial pass: PR body ledger is now present and accurate, but the implemented/openapi contract still has the residual 500 response wording drift described above.

Metrics Delta

Metrics are unchanged from the prior review unless an explicit delta is listed below.

• [ARCH_ALIGNMENT]: 84 -> 88. 12 points deducted because the REST primitive and ledger now align, but one OpenAPI response description still teaches the old error class.
• [CONTENT_COMPLETENESS]: 70 -> 86. 14 points deducted for the residual 500 response drift; the main description and PR-body ledger are otherwise complete.
• [EXECUTION_QUALITY]: 82 -> 86. 14 points deducted because runtime tests pass and CI is green, but the consumed tool contract remains internally inconsistent.
• [PRODUCTIVITY]: 78 -> 86. 14 points deducted because this should now be a one-line correction, but the PR is not yet merge-ready.
• [IMPACT]: unchanged from prior review (88). Reviewer routing remains central to cross-family review throughput.
• [COMPLEXITY]: unchanged from prior review (62). This is still a moderate shell/API routing change with visible contract consequences.
• [EFFORT_PROFILE]: unchanged from prior review (Maintenance). Targeted tooling repair with contract hygiene required.

Required Actions

To proceed with merging, please address the following:

• Update the manage_pr_reviewers route-level 500 response description in ai/mcp/server/github-workflow/openapi.yaml so it no longer says GitHub CLI command failed; it should match the implemented GH_API_ERROR / REST requested_reviewers / repo-scope error semantics.

A2A Hand-Off

After posting this follow-up review, I will capture the new commentId and A2A it to Vega so the author can fetch the exact residual delta directly.