mirror of
https://github.com/mksglu/context-mode.git
synced 2026-10-02 04:14:38 +08:00
feat(doctor): proactive Tier C absolute-path + stale .mcp.json checks (PR #620 slice 4)
PR #620 fixed the WRITE-time root causes (#609 stop writing per-version cache .mcp.json; #613 emit CLI-dispatcher form for vscode/jetbrains-copilot hooks), but users running pre-v1.0.137 still carry poisoned state on disk: - Tier C workspace-committed files (.github/hooks/context-mode.json, .cursor/hooks.json, .jetbrains/copilot/hooks.json) with absolute Windows fnm shim paths baked by old /ctx-upgrade runs. - Leftover per-version .mcp.json files in ~/.claude/plugins/cache/context-mode/context-mode/<ver>/ that the architectural untrack now treats as drift. Per ISSUE-604-VERDICT §11 ("silent-green doctor while hooks are dead is itself a P0 trust bug"), doctor must SURFACE this state before the user hits the runtime failure. CHECK A (FAIL): scan each Tier C file under process.cwd(); recurse all string values; flag any absolute path (unix /, Windows [A-Z]:[/\\], double-backslash UNC), fnm_multishells shim, or process.execPath literal. Missing config -> SKIP (no false fail). Remediation points at /context-mode:ctx-upgrade. CHECK B (WARN): enumerate cache version dirs under homedir() (Mert standing Windows-safety rule -- never use literal '~/'); count stale .mcp.json. Recoverable, so WARN not FAIL. Remediation: next ctx_upgrade sweep removes them via sweepStaleMcpJson. TDD evidence: RED: 3 new tests in tests/core/cli.test.ts under 'PR #620 slice 4 -- doctor() surfaces persistence-tier bug class' -- all 3 fail on current main (anchors '#613' / '#609' / 'fnm_multishells' / homedir() absent from doctor()). GREEN: 3/3 pass; 160/160 cli.test.ts tests pass; tsc --noEmit clean. Tests slot into existing tests/core/cli.test.ts (CONTRIBUTING L275 -- no new test files). Static-source-analysis pattern matches the Issue #564 doctor test precedent (lines 2056-2101). No bundle files touched.
This commit is contained in:
Mert Koseoglu
+168
@@ -534,6 +534,174 @@ async function doctor(): Promise<number> {
|
||||
);
|
||||
}
|
||||
|
||||
// ── Issue #613 — proactive Tier C absolute-path detection ───────────
|
||||
// PR #620 fixed `buildHookCommand` for vscode-copilot + jetbrains-copilot
|
||||
// so future writes are CLI-dispatcher-shape. But users who ran
|
||||
// /ctx-upgrade on v1.0.136 or earlier are still carrying poisoned
|
||||
// committable files in their workspace:
|
||||
// - `.github/hooks/context-mode.json` (vscode-copilot, team-shared)
|
||||
// - `.jetbrains/copilot/hooks.json` (jetbrains-copilot, team-shared)
|
||||
// - `.cursor/hooks.json` (cursor, team-shared)
|
||||
// Per ISSUE-613-VERDICT §6.1 these are Tier C — workspace-committed
|
||||
// cross-machine config. Doctor scans them for absolute paths and
|
||||
// fnm_multishells shims; if found, FAIL with `ctx_upgrade` remediation.
|
||||
// Per ISSUE-604-VERDICT §11 ("silent-green doctor while hooks are dead
|
||||
// is itself a P0 trust bug") — surface poison BEFORE the user hits a
|
||||
// runtime failure.
|
||||
p.log.step("Checking workspace-committed hook configs (Tier C)...");
|
||||
{
|
||||
const projectDir = process.cwd();
|
||||
const tierCFiles = [
|
||||
".github/hooks/context-mode.json",
|
||||
".cursor/hooks.json",
|
||||
".jetbrains/copilot/hooks.json",
|
||||
];
|
||||
let tierCFails = 0;
|
||||
let tierCChecked = 0;
|
||||
|
||||
// Detect absolute-path patterns that should never appear in a
|
||||
// workspace-committed config. Per Mert's standing Windows-safety rule:
|
||||
// handle both `/` and `\\` separators.
|
||||
function isAbsoluteOrShimPath(s: string): boolean {
|
||||
// unix absolute
|
||||
if (s.startsWith("/")) return true;
|
||||
// Windows drive-letter absolute (e.g. C:/, C:\)
|
||||
if (/^[A-Za-z]:[/\\]/.test(s)) return true;
|
||||
// Windows UNC or escaped-backslash absolute fragments
|
||||
if (s.includes("\\\\")) return true;
|
||||
// fnm shim hint — issue #613 reporter's exact stderr shape
|
||||
if (s.includes("fnm_multishells")) return true;
|
||||
// process.execPath literal baked into JSON
|
||||
if (s.includes("process.execPath")) return true;
|
||||
return false;
|
||||
}
|
||||
|
||||
function recurseStrings(node: unknown, hit: (s: string) => void): void {
|
||||
if (typeof node === "string") {
|
||||
hit(node);
|
||||
} else if (Array.isArray(node)) {
|
||||
for (const item of node) recurseStrings(item, hit);
|
||||
} else if (node && typeof node === "object") {
|
||||
for (const v of Object.values(node)) recurseStrings(v, hit);
|
||||
}
|
||||
}
|
||||
|
||||
for (const rel of tierCFiles) {
|
||||
const abs = resolve(projectDir, rel);
|
||||
if (!existsSync(abs)) continue; // missing config → SKIP, no false fail
|
||||
tierCChecked++;
|
||||
try {
|
||||
const parsed = JSON.parse(readFileSync(abs, "utf-8"));
|
||||
const offenders: string[] = [];
|
||||
recurseStrings(parsed, (s) => {
|
||||
if (isAbsoluteOrShimPath(s)) offenders.push(s);
|
||||
});
|
||||
if (offenders.length > 0) {
|
||||
criticalFails++;
|
||||
tierCFails++;
|
||||
// Truncate to one example to keep output readable; show count.
|
||||
const example = offenders[0].length > 100
|
||||
? offenders[0].slice(0, 97) + "..."
|
||||
: offenders[0];
|
||||
p.log.error(
|
||||
color.red(`Tier C config: FAIL`) +
|
||||
` — ${rel} contains ${offenders.length} absolute path(s)` +
|
||||
color.dim(
|
||||
`\n Example: ${example}` +
|
||||
"\n Root cause: pre-v1.0.137 /ctx-upgrade baked machine-local paths into a workspace-committed file (#613)." +
|
||||
"\n Fix: run /context-mode:ctx-upgrade to rewrite to portable `context-mode hook <platform> <event>` form.",
|
||||
),
|
||||
);
|
||||
} else {
|
||||
p.log.success(
|
||||
color.green("Tier C config: PASS") +
|
||||
color.dim(` — ${rel} uses portable command shapes`),
|
||||
);
|
||||
}
|
||||
} catch (err: unknown) {
|
||||
// Malformed JSON should not crash doctor; warn and move on.
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
p.log.warn(
|
||||
color.yellow(`Tier C config: WARN`) +
|
||||
` — could not parse ${rel}` +
|
||||
color.dim(`\n ${msg.slice(0, 200)}`),
|
||||
);
|
||||
}
|
||||
}
|
||||
if (tierCChecked === 0) {
|
||||
p.log.info(
|
||||
color.dim("Tier C config: SKIP — no workspace-committed hook configs found"),
|
||||
);
|
||||
} else if (tierCFails === 0) {
|
||||
// already individual PASS messages above; no need for a summary
|
||||
}
|
||||
}
|
||||
|
||||
// ── Issue #609 — proactive stale `.mcp.json` detection ──────────────
|
||||
// PR #620 deleted the per-version cache `.mcp.json` write from cli.ts
|
||||
// and shipped `sweepStaleMcpJson` to clean up any pre-existing copies.
|
||||
// But users on the field may still have stale `.mcp.json` files left
|
||||
// by /ctx-upgrade flows that ran before PR #620 (or by Claude Code's
|
||||
// native auto-update copying a poisoned file forward). Surface those
|
||||
// as WARN (recoverable — next ctx_upgrade sweeps them) so the user
|
||||
// knows what to do instead of being told everything is green while
|
||||
// the file lingers on disk.
|
||||
// Per ISSUE-604-VERDICT §11 same trust contract as Tier C check above.
|
||||
p.log.step("Checking for stale per-version `.mcp.json` files...");
|
||||
{
|
||||
const cacheRoot = join(
|
||||
homedir(),
|
||||
".claude",
|
||||
"plugins",
|
||||
"cache",
|
||||
"context-mode",
|
||||
"context-mode",
|
||||
);
|
||||
if (!existsSync(cacheRoot)) {
|
||||
p.log.info(
|
||||
color.dim("Cache scan: SKIP — no plugin cache present at " + cacheRoot),
|
||||
);
|
||||
} else {
|
||||
let staleCount = 0;
|
||||
const staleVersions: string[] = [];
|
||||
try {
|
||||
const versionDirs = readdirSync(cacheRoot);
|
||||
for (const v of versionDirs) {
|
||||
const candidate = join(cacheRoot, v, ".mcp.json");
|
||||
if (existsSync(candidate)) {
|
||||
staleCount++;
|
||||
if (staleVersions.length < 5) staleVersions.push(v);
|
||||
}
|
||||
}
|
||||
} catch (err: unknown) {
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
p.log.warn(
|
||||
color.yellow("Cache scan: WARN") +
|
||||
` — could not enumerate ${cacheRoot}` +
|
||||
color.dim(`\n ${msg.slice(0, 200)}`),
|
||||
);
|
||||
staleCount = 0;
|
||||
}
|
||||
if (staleCount === 0) {
|
||||
p.log.success(
|
||||
color.green("Cache `.mcp.json` sweep: PASS") +
|
||||
color.dim(" — no stale files in per-version cache dirs"),
|
||||
);
|
||||
} else {
|
||||
// WARN, not FAIL — per architect spec this is recoverable.
|
||||
p.log.warn(
|
||||
color.yellow("Cache `.mcp.json` sweep: WARN") +
|
||||
` — ${staleCount} stale .mcp.json file(s) in ${cacheRoot}` +
|
||||
color.dim(
|
||||
`\n Versions: ${staleVersions.join(", ")}${staleCount > staleVersions.length ? ", ..." : ""}` +
|
||||
"\n Root cause: pre-v1.0.137 /ctx-upgrade wrote per-version `.mcp.json` into the plugin cache; PR #620 removed the write + added a sweep (#609)." +
|
||||
"\n Fix: run /context-mode:ctx-upgrade — `sweepStaleMcpJson` will remove these files on the next run.",
|
||||
),
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// FTS5 / SQLite
|
||||
p.log.step("Checking FTS5 / SQLite...");
|
||||
try {
|
||||
|
||||
@@ -2132,3 +2132,121 @@ describe("Upgrade native ABI bootstrap", () => {
|
||||
expect(region).toContain("ABI cache present");
|
||||
});
|
||||
});
|
||||
|
||||
// ── Issue #613/#609 — doctor() surfaces persistence-tier bug class ─────
|
||||
// PR #620 (Family A) shipped the architectural fixes:
|
||||
// - #609: stop writing per-version cache `.mcp.json` + post-bump sweep
|
||||
// - #613: vscode/jetbrains-copilot hook commands ship CLI-dispatcher form
|
||||
// (no absolute `process.execPath` + script path baked into
|
||||
// workspace-committed `.github/hooks/context-mode.json`)
|
||||
//
|
||||
// These two slices are the *prevention* surface — root-cause fixes that
|
||||
// stop the bug from being written. But users on the field can still be
|
||||
// holding pre-PR-620 poisoned state:
|
||||
// - already-committed `.github/hooks/context-mode.json` in their repo
|
||||
// with absolute Windows fnm shim paths from v1.0.136 or earlier
|
||||
// - leftover `.mcp.json` in `~/.claude/plugins/cache/.../<version>/`
|
||||
// from /ctx-upgrade flows that ran before PR #620
|
||||
//
|
||||
// Doctor's job per the verdict family ("silent-green doctor while hooks
|
||||
// are dead is itself a P0 trust bug" — ISSUE-604-VERDICT §11) is to
|
||||
// SURFACE that pre-PR state BEFORE the user hits a runtime failure.
|
||||
//
|
||||
// Architect contract for PR #620 + slice 4:
|
||||
// CHECK A: doctor scans workspace Tier C files (`.github/hooks/context-mode.json`,
|
||||
// `.cursor/hooks.json`, `.jetbrains/copilot/hooks.json` under
|
||||
// process.cwd()) — for each that exists, parse JSON, recurse
|
||||
// into all string values, FAIL if any matches absolute path
|
||||
// patterns (unix `/`, Windows `[A-Z]:[/\\]`, `\\`, fnm_multishells).
|
||||
// Remediation: "run `ctx_upgrade` to rewrite to portable form".
|
||||
// Missing config → SKIP (no false fail).
|
||||
//
|
||||
// CHECK B: doctor scans `~/.claude/plugins/cache/context-mode/context-mode/*/`
|
||||
// for `.mcp.json` files (post-PR-620 these should not exist).
|
||||
// Found → WARN (not fail) with remediation:
|
||||
// "ctx_upgrade will sweep on next run".
|
||||
//
|
||||
// Same static-analysis assertion pattern as Issue #564 doctor test (above)
|
||||
// and lines 962, 997, 1010 — runtime spawning would need fixture
|
||||
// workspaces on three OSes and is not portable; asserting the gate
|
||||
// exists in doctor() source catches the regression at PR time.
|
||||
describe("PR #620 slice 4 — doctor() surfaces persistence-tier bug class", () => {
|
||||
const CLI_SRC = readFileSync(resolve(ROOT, "src", "cli.ts"), "utf-8");
|
||||
|
||||
function doctorBody(): string {
|
||||
const start = CLI_SRC.indexOf("async function doctor(");
|
||||
expect(start).toBeGreaterThan(-1);
|
||||
const end = CLI_SRC.indexOf("async function insight", start);
|
||||
expect(end).toBeGreaterThan(start);
|
||||
return CLI_SRC.slice(start, end);
|
||||
}
|
||||
|
||||
it("doctor scans workspace Tier C config files for absolute paths (#613 proactive)", () => {
|
||||
const body = doctorBody();
|
||||
// Must reference all three Tier C path shapes that PR #620 covers
|
||||
// (vscode-copilot writes `.github/hooks/context-mode.json`,
|
||||
// cursor writes `.cursor/hooks.json`,
|
||||
// jetbrains-copilot writes `.jetbrains/copilot/hooks.json`
|
||||
// — workspace-committed per ISSUE-613-VERDICT §6.1 Tier C table).
|
||||
expect(body).toContain(".github/hooks/context-mode.json");
|
||||
expect(body).toContain(".cursor/hooks.json");
|
||||
expect(body).toContain(".jetbrains/copilot/hooks.json");
|
||||
// Must detect the fnm-shim pattern (reporter's stderr literally shows
|
||||
// `fnm_multishells/<pid>_<ts>/node.exe` per ISSUE-613-VERDICT §2 H2).
|
||||
expect(body).toMatch(/fnm_multishells/);
|
||||
// Must surface the failure with remediation pointing at ctx_upgrade.
|
||||
// The Tier C section is identifiable by the issue anchor `#613`.
|
||||
const anchorIdx = body.indexOf("#613");
|
||||
expect(anchorIdx).toBeGreaterThan(-1);
|
||||
const window_ = body.slice(
|
||||
Math.max(0, anchorIdx - 500),
|
||||
anchorIdx + 3000,
|
||||
);
|
||||
// The check must use p.log.error or p.log.warn (not info) AND
|
||||
// mention ctx_upgrade so the user knows the remediation.
|
||||
expect(window_).toMatch(/p\.log\.(error|warn)/);
|
||||
expect(window_).toMatch(/ctx[_-]?upgrade/i);
|
||||
});
|
||||
|
||||
it("doctor warns on stale `.mcp.json` files in cache version dirs (#609 proactive)", () => {
|
||||
const body = doctorBody();
|
||||
// Must reference the cache plugin path shape that PR #620 sweeps.
|
||||
// The path nests `context-mode/context-mode` (marketplace/plugin
|
||||
// nesting per ISSUE-609-VERDICT path examples). cli.ts uses
|
||||
// path.join() so the literal appears as adjacent string args:
|
||||
// join(homedir(), ".claude", "plugins", "cache",
|
||||
// "context-mode", "context-mode")
|
||||
// We assert on both the cache anchor segments AND the join args
|
||||
// (Mert standing rule — use platform-neutral path joins, not literal
|
||||
// separators that fail on Windows).
|
||||
expect(body).toMatch(/"plugins"\s*,\s*"cache"/);
|
||||
expect(body).toMatch(/"context-mode"\s*,\s*"context-mode"/);
|
||||
// Must check for `.mcp.json` (the file that should not exist after
|
||||
// PR #620's architectural untrack — ISSUE-609-VERDICT §H1 → PR #618 → #620).
|
||||
const anchorIdx = body.indexOf("#609");
|
||||
expect(anchorIdx).toBeGreaterThan(-1);
|
||||
const window_ = body.slice(
|
||||
Math.max(0, anchorIdx - 500),
|
||||
anchorIdx + 2500,
|
||||
);
|
||||
expect(window_).toContain(".mcp.json");
|
||||
// WARN (not FAIL) — the verdict spec is explicit that this is
|
||||
// recoverable: "ctx_upgrade will sweep on next run".
|
||||
expect(window_).toMatch(/p\.log\.warn/);
|
||||
expect(window_).toMatch(/ctx[_-]?upgrade/i);
|
||||
});
|
||||
|
||||
it("doctor uses homedir() (cross-platform) not literal '~' for cache scan", () => {
|
||||
const body = doctorBody();
|
||||
// Mert standing rule: Windows safety. Cache path must resolve via
|
||||
// os.homedir() — not a literal `~/` prefix which fails on Windows.
|
||||
const anchorIdx = body.indexOf("#609");
|
||||
expect(anchorIdx).toBeGreaterThan(-1);
|
||||
const window_ = body.slice(anchorIdx, anchorIdx + 2500);
|
||||
// The scan must use homedir() (already imported at the top of cli.ts).
|
||||
expect(window_).toMatch(/homedir\(\)|process\.env\.HOME/);
|
||||
// And must NOT use a literal `~/` path string (would be treated
|
||||
// literally on Windows).
|
||||
expect(window_).not.toMatch(/["']~\//);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user