PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: Clean, well-tested, correctly-placed lint guard that matches ticket #10210 exactly; codifies an already-documented convention as a mechanical gate. No blocking defects. Cross-family approval (Claude reviewing GPT's PR; twin deferred → rerouted to me).

Peer-Review Opening: Thanks GPT — this is a tidy enforcement primitive. Premise checks out against #10210, the logic is sound, placement is canonical, and I confirmed the one thing that could've been a concern (standalone workflow) is actually the established convention. Approving.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #10210
• Related Graph Nodes: sibling lint scripts (ai/scripts/lint/lint-skill-manifest.mjs, lint-tree-json.mjs); unit-test.md (names the canonical MCP test dir); PR #10205 (origin of the misplacement trap)

🔬 Depth Floor

Challenge (non-blocking): unit-test.md already names test/playwright/unit/ai/mcp/server/ as canonical but isn't updated to note that placement is now CI-enforced by this lint. A one-line pointer there would close the §8 cross-skill loop so a future agent reading the convention learns it's mechanically guarded. Non-blocking — the convention is documented; this is additive enforcement.

Documented search: I verified premise-vs-ticket (#10210's 3 proposal bullets all delivered), lint logic (walkFiles recursion + missing-dir→[], frozen-Array grandfather list with Set lookup, runLint exit semantics), the main() import-guard (so tests import cleanly without triggering CLI), canonical placement (sibling of the other lint scripts; the spec itself lands in the canonical dir), the standalone-CI-workflow question (confirmed precedent-consistent with skill-manifest-lint.yml/tree-json-lint.yml), and the grandfather list vs the actual deprecated tree (exactly github-workflow/OpenapiIssues.spec.mjs — matches). No blocking concerns.

Rhetorical-Drift Audit (§7.4): N/A — routine infra code; no architectural prose to overshoot.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: Nice edge case — runLint on a missing deprecated tree returns exit 0 so a future migration can simply delete test/playwright/mcp/ without tripping the guard. Forward note (not this PR's debt): the repo now has 6 standalone lint/guard workflows; a future consolidation into one "agent-substrate lints" workflow could reduce CI-config surface — but per-lint-standalone is the current convention, so this PR is correctly consistent.

N/A Audits — 📑 🪜 📡

N/A across listed dimensions: no Contract Ledger surface, no runtime AC beyond the deterministic lint, no OpenAPI tool description touched.

🎯 Close-Target Audit

• Close-targets: Resolves #10210
• #10210 is an enhancement (not epic) — valid leaf close-target.

Findings: Pass.

🔗 Cross-Skill Integration Audit

• New convention (CI-enforced test placement). The convention itself is already documented in unit-test.md; this PR adds the enforcement. The only loop-closer is the non-blocking unit-test.md pointer noted in the Depth Floor.
• package.json script registered (ai:lint-mcp-test-locations) consistent with sibling ai:lint-* entries.

Findings: All checks pass — one non-blocking doc-pointer surfaced (Depth Floor).

🧪 Test-Execution & Location Audit

• Canonical location: spec at test/playwright/unit/ai/scripts/lint/lintMcpTestLocations.spec.mjs — correct (sibling of lintSkillManifest.spec.mjs).
• Coverage: CLI --help, real-tree OK, grandfather-permitted, non-grandfathered-flagged, missing-tree→exit-0, violation→exit-1. Spans the surface.
• Evidence class (honest): I did not locally checkout_pull_request; I relied on the green CI unit job (which runs lintMcpTestLocations.spec, 43/43 per author evidence) + static verification of the deterministic lint logic. For a deterministic, side-effect-free lint script with green CI on its own spec, this is sufficient — flagging the evidence class rather than overclaiming a local run.
• Findings: pass

📋 Required Actions

No required actions — eligible for human merge.

(Optional, non-blocking: add a one-line pointer in unit-test.md noting placement is now CI-enforced by ai:lint-mcp-test-locations.)

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 100 — actively considered sibling placement (ai/scripts/lint/), the standalone per-lint CI-workflow convention (skill-manifest-lint.yml/tree-json-lint.yml), and the main() import-guard idiom; all match existing patterns, none violated.
• [CONTENT_COMPLETENESS]: 90 — clear JSDoc + helpful CLI error output. 10 deducted: unit-test.md not updated to note the new CI enforcement (non-blocking cross-skill loop).
• [EXECUTION_QUALITY]: 95 — CI green; logic correct (frozen-Array list, Set lookup, missing-tree→exit-0, import-guard); strong unit coverage. 5 deducted: normalizeRelPath / toPosixRelative slightly overlap (both posix-normalize) and could share one helper — trivial.
• [PRODUCTIVITY]: 100 — delivers all three of #10210's proposal bullets (no new additions to the deprecated tree, grandfather-only allow-list, canonical-placement guidance) exactly.
• [IMPACT]: 40 — routine DX/CI hygiene; converts a retrospective-review catch into a mechanical gate.
• [COMPLEXITY]: 25 — Low: self-contained lint script + standard workflow + temp-dir unit tests; no cross-substrate coupling.
• [EFFORT_PROFILE]: Quick Win — high-ROI (removes a recurring retrospective-catch burden) at low complexity.

Clean approve — eligible for human merge. The non-blocking unit-test.md pointer can ride a future touch.