PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: Delivers #12765 — a read-only boot-vs-current source-identity drift detector in the github-workflow MCP healthcheck, directly targeting the ghost-debugging loop (an agent asserting stale-source facts when its long-lived MCP process is behind the checkout). Honest "stale = diagnostic warning, not outage" framing. No blockers; one substantive non-blocking challenge on signal scope.

Opening: Clean, well-scoped diagnostic — exactly the right shape for the stale-runtime problem the ticket describes. boot-captured gitHead + openApiDigest vs a healthcheck-time re-read, with a graceful unknown and a restart hint, and a test seam that keeps the classification unit-testable. Notes below; no blockers.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: ticket #12765 (enhancement/developer-experience/architecture — the ghost-debugging-loop context), the github-workflow HealthService.mjs on dev, the openapi.yaml healthcheck schema, the #12740/#12706 stale-metadata context.
• Expected Solution Shape: a read-only diagnostic exposing source/schema drift between the running MCP process and the current checkout, framed as a warning (not a hard outage), with a unit-testable classification. Boundary NOT to break: it must not mutate state or gate the healthcheck hard on drift.
• Patch Verdict: Matches. Captures gitHead (git rev-parse HEAD) + openApiDigest (SHA-256 of openapi.yaml) at module-load (boot), re-reads at healthcheck, classifies current/stale/unknown, and surfaces a restart hint on stale. Read-only; boot read sync (one-time), current read async; graceful unknown on read failure; injectable test seam.

🕸️ Context & Graph Linking

• Target Issue ID: Resolves #12765
• Related Graph Nodes: #12740 (cost-safety / dev-experience), #12706 (the originating stale-metadata friction)

🔬 Depth Floor

Challenge (non-blocking): The gitHead comparison flags stale on any repo commit since boot — not just changes to this server's files. So in an active dev repo, runtimeFreshness will report stale after the first unrelated commit, even when the github-workflow server's own code and openApiDigest are unchanged. The openApiDigest is the server-scoped precise signal; the gitHead is the repo-wide broad one, and classifyRuntimeFreshness escalates status to stale if either differs. Two readings: (a) intentional safe-by-default — warn whenever the checkout moved, so the agent restarts before asserting any source fact; (b) cry-wolf risk — constant stale in an active repo trains agents to ignore it, defeating the ghost-debugging-loop purpose. Worth a deliberate call: e.g., drive status primarily off the openApiDigest (+ the server's own file set) and demote a bare gitHead drift to a softer details note ("repo advanced since boot") rather than escalating the whole status. Non-blocking — the warning is honest and the precise signal is present.

Secondary (minor): readBootGitHead runs execFileSync at module-load (a sync child-process spawn on import) — acceptable as a one-time boot cost and gracefully degraded to unknown in a no-git environment (e.g., a container), where the immutable openApiDigest carries the signal. Worth a one-line comment that this is intentionally sync-at-boot.

Rhetorical-Drift Audit: Pass. "Boot-vs-current runtime source identity for detecting stale MCP client/server attachments" accurately describes the mechanic (compare boot-loaded vs current gitHead/openApiDigest). The stale JSDoc explicitly scopes it as "a diagnostic warning, not a hard outage" — matches the code (it never fails the healthcheck). No overshoot.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: A boot-vs-current source-identity drift detector (git SHA + contract digest captured at module-load, re-read live) is a reusable diagnostic primitive for catching the stale-long-lived-runtime failure mode — the swarm's V-B-A discipline now has a machine-checkable "is my process behind the source?" signal.

🎯 Close-Target Audit

• Close-target: #12765 (newline-isolated Resolves); confirmed enhancement/ai/developer-experience/architecture — not epic. ✓

Findings: Pass. Related: #12740 is non-closing.

📡 MCP-Tool-Description Budget Audit

(§5.3 — the PR touches ai/mcp/server/github-workflow/openapi.yaml.)

• The +50 lines are healthcheck response-schema field descriptions (the runtimeFreshness object), not a tool description.
• Single-line, concise, call-site-usage-focused; no internal cross-refs / ticket numbers / narrative; well under any budget concern.

Findings: Pass — additive response-schema only; no description bloat.

🔌 Wire-Format Compatibility Audit

• The healthcheck response gains a runtimeFreshness object (additive, backward-compatible — existing consumers that ignore it are unaffected), and the openapi.yaml documents it.

Findings: Pass (additive, non-breaking).

🧪 Test-Execution & Location Audit

• Branch checked out (gh pr checkout 12766, head 3c79b3911).
• Canonical Location: test/playwright/unit/ai/services/github-workflow/HealthService.spec.mjs — correct service-test dir; describe.serial + afterEach reset (singleton-safe).
• Ran the spec → 3 passed (current / stale-with-restart-hint / unknown-without-throwing).

Findings: Pass.

N/A Audits — 📑 🪜 🛂

N/A: no Contract Ledger drift — additive diagnostic field, the existing healthcheck contract is preserved (📑); ACs are classification + schema-contract, fully L2 unit-covered, matching the PR's Evidence: L2 with no residuals (🪜); standard git-SHA + SHA-256 digest comparison, not an external-framework port (🛂).

🔗 Cross-Skill Integration Audit

• The new runtimeFreshness field is exposed on every healthcheck, so it fires without other skills invoking it. But its discoverability is the gap: agents hitting the ghost-debugging loop need to know to look at it. Non-blocking follow-up worth considering — a one-line reference from self-repair (or a stale-metadata debugging note) pointing at healthcheck.runtimeFreshness, so the diagnostic actually gets used when the symptom recurs.

Findings: No blocking integration gap (the field is live + documented in the openapi schema); the discoverability cross-reference is a non-blocking follow-up.

📋 Required Actions

No required actions — eligible for human merge. (The signal-scope challenge, the boot-sync note, and the self-repair cross-reference are all non-blocking.)

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 90 — 10 deducted: the gitHead repo-wide escalation to status: stale (vs the server-scoped openApiDigest) risks cry-wolf in an active repo; the read-only boot-capture pattern + the warning-not-outage framing + the test seam are otherwise idiomatic.
• [CONTENT_COMPLETENESS]: 95 — 5 deducted: JSDoc on classifyRuntimeFreshness/resolveRuntimeFreshness/#readCurrentRuntimeIdentity + the schema are thorough; a one-line note on the sync-at-boot read and the gitHead-breadth would close it.
• [EXECUTION_QUALITY]: 90 — 10 deducted: correct boot-vs-current comparison, read-only, graceful unknown, 3-case test; the gitHead-breadth cry-wolf and the module-load sync-exec are the deductions.
• [PRODUCTIVITY]: 100 — confirmed the #12765 ACs (runtime healthcheck classification + the schema contract) are met + tested; nothing left unaddressed.
• [IMPACT]: 75 — directly mitigates a recurring swarm failure mode (asserting stale-source facts / ghost-debugging) with a machine-checkable signal; scoped to the github-workflow server's healthcheck.
• [COMPLEXITY]: 55 — Moderate: boot-time capture + live re-read + classification + error-handling + a test seam; the reasoning load is the boot-vs-current lifecycle, not control flow.
• [EFFORT_PROFILE]: Heavy Lift — substantial, careful diagnostic (235-line service change + schema + tests) with boot-capture + graceful degradation.

[KB_GAP] / [TOOLING_GAP]: none.

Closing: A genuinely useful diagnostic — it turns the "is my MCP process behind the source?" question into a machine-checkable signal, which is exactly what the ghost-debugging loop needed. The one thing worth a deliberate decision is the gitHead breadth (repo-wide stale vs server-scoped), so the signal doesn't become background noise in an active repo. Eligible for human merge.