PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: Delivers #12776 — extracts the duplicated runtimeFreshness logic across the three MCP servers I reviewed (#12766 / #12773 / #12775) into one shared RuntimeFreshnessService, settles the gitHead-cry-wolf note I raised on all three (gitHead is now contextual, not status-driving), and — critically — preserves the #12775 fix (KB still digests the shared ai/config.mjs). No blockers; CI 8/8 green; 78 specs pass.

Opening: This is the consolidation done right. One RuntimeFreshnessService with per-consumer trackers, each keeping its correct config target (the #12775 shared-ai/config.mjs fix intact for KB + MC; openApiDigest-only for github-workflow), and the gitHead-breadth fix I flagged across all three folded in by construction (excluded from statusFields) rather than patched per-surface.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #12776 (the consolidation ticket — my cross-cutting input); my #12766/#12773/#12775 reviews (the 3 surfaces, the gitHead note, the #12775 shared-config fix); the new RuntimeFreshnessService.mjs, the three consumers' createTracker wiring, and the openapi changes. Fetched PR head 26ce78bbf; ran the helper + 3 consumer specs.
• Expected Solution Shape: a shared helper that (a) unifies classify/boot-capture/file-digest, (b) lets EACH consumer pass its OWN config target (KB/MC keep the shared ai/config.mjs — the #12775 fix — github-workflow stays openApiDigest-only), (c) demotes gitHead to contextual (not status-driving), (d) stays B4-safe (file-digest, no AiConfig re-import/mutation). It must NOT regress the #12775 false-negative.
• Patch Verdict: Matches (improves). createTracker({files, statusFields, rootDir}) is per-consumer: KB passes configPath = path.resolve(serviceDir, '../../config.mjs') (the shared ai/config.mjs) ✓, MC the same ✓, github-workflow openApiDigest-only ✓; statusFields excludes gitHead on all three → gitHead drift becomes a contextualStaleFields note, not status:'stale' (the cry-wolf fix); the per-tracker cache + shared classifyRuntimeFreshness are clean and B4-safe.

🕸️ Context & Graph Linking

• Target Issue ID: Resolves #12776
• Related Graph Nodes: #12740 (cost-safety epic), #12766 / #12773 / #12775 (the three runtimeFreshness surfaces this consolidates), #12742 (the config-drift symptom the configDigest catches).

🔬 Depth Floor

Challenge (non-blocking): the per-tracker cache (cacheDuration = 30 * 1000) means a config/openapi change mid-window won't surface until the cache expires (≤30s). That's the right cost trade-off for a per-healthcheck diagnostic (and #12773's end-to-end test already exercised the cached-vs-fresh path), but a one-line note that runtimeFreshness has a ≤30s detection latency by design would pre-empt a "why didn't it flip immediately?" question. Non-blocking.

Rhetorical-Drift Audit: Pass. The body's "gitHead drift is intentionally contextual rather than status-driving" and the JSDoc ("gitHead is kept as contextual diagnostic metadata only; a repo-wide commit can be unrelated") accurately describe the statusFieldSet-excludes-gitHead mechanism. "Compact payload" matches the openapi field tightening. No overshoot.

🎯 Close-Target Audit (§5.2)

• Resolves #12776 (newline-isolated); labels enhancement/ai/refactoring/architecture/performance/model-experience — not epic. ✓ Related: #12740 (the epic) correctly non-closing.

📡 MCP-Tool-Description Budget Audit (§5.3)

• The three openapi.yaml changes are healthcheck response-schema field-description updates (the runtimeFreshness object: gitHead "informational, doesn't drive status" + the compact shape) — not tool descriptions; concise, no narrative/cross-refs. Pass.

🔌 Wire-Format Compatibility Audit

• The three healthcheck runtimeFreshness payloads became compact, but the per-field stale flags (stale.gitHead / configDigest / openApiDigest) remain. The semantic change is precise and intentional: a gitHead-only drift now reports status:'current' (with a contextual note) instead of status:'stale'. Field-level backward-compatible (no consumer reads a now-removed field); the three consumer specs assert the new shape.

Findings: Pass — additive/refinement, covered by the consumer specs.

🧪 Test-Execution & Location Audit (§7.5)

• Checked out PR head 26ce78bbf; ran RuntimeFreshnessService.spec.mjs + the three consumer specs (KB providerReady, MC, github-workflow) → 78 passed.
• Canonical locations: new helper spec at test/playwright/unit/ai/mcp/server/shared/services/ ✓; consumer specs in their respective dirs ✓.

Findings: Pass — the helper and all three migrated consumers are green; the KB spec (the #12775 shared-config proof) still passes under the shared helper.

🛂 CI / Security Checks Audit (§7.6)

• CI 8/8 green on 26ce78bbf (0 pending, 0 failed) — verified immediately before posting.

N/A Audits — 🔗

N/A: runtimeFreshness auto-fires on each healthcheck; no new cross-skill convention to invoke. (A one-line self-repair pointer at the unified RuntimeFreshnessService would be a nice non-blocking follow-up — same note I made on #12773, now doubly worth it since all three servers share one helper.)

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 95 — clean DRY consolidation (one RuntimeFreshnessService, per-consumer trackers, B4-safe file-digest preserved, gitHead policy by construction); 5 deducted for the 30s cache-latency being implicit.
• [CONTENT_COMPLETENESS]: 95 — thorough JSDoc (the gitHead-contextual rationale, per-tracker semantics) + the openapi clarifications; 5 deducted for the cache-latency note.
• [EXECUTION_QUALITY]: 95 — preserves the #12775 fix, settles the gitHead note by construction, 78 specs green, CI 8/8; 5 deducted: the exact 30s cache boundary isn't a named test case (the #12773 cached-path test covers the mechanism).
• [PRODUCTIVITY]: 100 — delivers #12776 (the consolidation) + the gitHead-breadth fix across all three + preserves #12775; the friction-to-gold chain (#12764→#12769→#12773→#12775→#12776) closes.
• [IMPACT]: 80 — unifies the runtimeFreshness substrate across all three MCP servers (the swarm's diagnostic surface) and removes the cry-wolf false-positive; meaningful for cost-safety/diagnostics reliability.
• [COMPLEXITY]: 60 — Moderate-high: extracting a shared service + migrating three consumers + the per-tracker cache + the statusFields policy + boot/current capture; the real reasoning is the per-consumer parameterization.
• [EFFORT_PROFILE]: Heavy Lift — a 900/674 consolidation across 11 files (helper + 3 services + 3 openapis + 4 specs).

[KB_GAP] / [TOOLING_GAP]: none.

📋 Required Actions

No required actions — eligible for human merge. (The ≤30s cache-latency note and the self-repair cross-reference are non-blocking.)

Closing: The consolidation done right — one shared RuntimeFreshnessService that preserves each consumer's correct config target (the #12775 shared-ai/config.mjs fix intact), settles the gitHead-cry-wolf note across all three by construction (gitHead → contextual; only digests drive stale), and stays B4-safe. 78 specs green, CI 8/8. The friction-to-gold chain that started with my #12764 config-mismatch finding now closes in a clean shared helper. Eligible for human merge (@tobiu's gate).