From 4d9b8acadb45a894ea68e47de1f6135cbc09b87d Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 5 Apr 2026 00:52:11 -0400 Subject: [PATCH] fix(doctor): strip --fix flag before positional parse (#1919) (#1926) The flag-stripping regex in handleDoctor() removed --json, --dry-run, --build, and --test but not --fix. When a user ran `/gsd doctor --fix`, "--fix" leaked into requestedScope, mode stayed "doctor", and fix was never enabled -- silently suppressing all issues and fixes. Extract parseDoctorArgs() as a pure, exported function. Add --fix to the stripping regex and propagate fixFlag into the fix option passed to runGSDDoctor. Fixes #1919 Co-authored-by: Claude Opus 4.6 (1M context) --- .../extensions/gsd/commands-handlers.ts | 14 ++- .../gsd/tests/doctor-fix-flag.test.ts | 92 +++++++++++++++++++ 2 files changed, 102 insertions(+), 4 deletions(-) create mode 100644 src/resources/extensions/gsd/tests/doctor-fix-flag.test.ts diff --git a/src/resources/extensions/gsd/commands-handlers.ts b/src/resources/extensions/gsd/commands-handlers.ts index b40b7bc25..16af7230b 100644 --- a/src/resources/extensions/gsd/commands-handlers.ts +++ b/src/resources/extensions/gsd/commands-handlers.ts @@ -43,21 +43,27 @@ export function dispatchDoctorHeal(pi: ExtensionAPI, scope: string | undefined, ); } -export async function handleDoctor(args: string, ctx: ExtensionCommandContext, pi: ExtensionAPI): Promise { +/** Parse doctor command args into structured flags and positionals (pure, no I/O). */ +export function parseDoctorArgs(args: string) { const trimmed = args.trim(); - // Extract flags before positional parsing const jsonMode = trimmed.includes("--json"); const dryRun = trimmed.includes("--dry-run"); + const fixFlag = trimmed.includes("--fix"); const includeBuild = trimmed.includes("--build"); const includeTests = trimmed.includes("--test"); - const stripped = trimmed.replace(/--json|--dry-run|--build|--test/g, "").trim(); + const stripped = trimmed.replace(/--json|--dry-run|--build|--test|--fix/g, "").trim(); const parts = stripped ? stripped.split(/\s+/) : []; const mode = parts[0] === "fix" || parts[0] === "heal" || parts[0] === "audit" ? parts[0] : "doctor"; const requestedScope = mode === "doctor" ? parts[0] : parts[1]; + return { jsonMode, dryRun, fixFlag, includeBuild, includeTests, mode, requestedScope }; +} + +export async function handleDoctor(args: string, ctx: ExtensionCommandContext, pi: ExtensionAPI): Promise { + const { jsonMode, dryRun, fixFlag, includeBuild, includeTests, mode, requestedScope } = parseDoctorArgs(args); const scope = await selectDoctorScope(projectRoot(), requestedScope); const effectiveScope = mode === "audit" ? requestedScope : scope; const report = await runGSDDoctor(projectRoot(), { - fix: mode === "fix" || mode === "heal" || dryRun, + fix: mode === "fix" || mode === "heal" || dryRun || fixFlag, dryRun, scope: effectiveScope, includeBuild, diff --git a/src/resources/extensions/gsd/tests/doctor-fix-flag.test.ts b/src/resources/extensions/gsd/tests/doctor-fix-flag.test.ts new file mode 100644 index 000000000..f2919ca4e --- /dev/null +++ b/src/resources/extensions/gsd/tests/doctor-fix-flag.test.ts @@ -0,0 +1,92 @@ +/** + * Regression test for #1919: --fix flag not stripped before positional parse. + * + * parseDoctorArgs("--fix") must: + * 1. Set fixFlag = true + * 2. Not leak "--fix" into requestedScope + * 3. Keep mode as "doctor" (the flag is not a positional subcommand) + */ + +import { parseDoctorArgs } from "../commands-handlers.js"; +import { createTestContext } from "./test-helpers.ts"; + +const { assertEq, assertTrue, report } = createTestContext(); + +async function main(): Promise { + // ── 1. Bare --fix flag ────────────────────────────────────────────────────── + console.log("\n=== bare --fix flag (#1919) ==="); + { + const r = parseDoctorArgs("--fix"); + assertTrue(r.fixFlag, "--fix sets fixFlag to true"); + assertEq(r.mode, "doctor", "--fix does not change mode from doctor"); + assertEq(r.requestedScope, undefined, "--fix is stripped and does not become requestedScope"); + } + + // ── 2. --fix with a scope ────────────────────────────────────────────────── + console.log("\n=== --fix with scope ==="); + { + const r = parseDoctorArgs("--fix M001/S01"); + assertTrue(r.fixFlag, "--fix M001/S01 sets fixFlag to true"); + assertEq(r.mode, "doctor", "--fix M001/S01 keeps mode as doctor"); + assertEq(r.requestedScope, "M001/S01", "scope is M001/S01 after stripping --fix"); + } + + // ── 3. Positional fix still works ────────────────────────────────────────── + console.log("\n=== positional fix subcommand ==="); + { + const r = parseDoctorArgs("fix"); + assertEq(r.fixFlag, false, "positional fix does not set fixFlag"); + assertEq(r.mode, "fix", "positional fix sets mode to fix"); + assertEq(r.requestedScope, undefined, "no scope with bare positional fix"); + } + + // ── 4. Positional fix with scope ─────────────────────────────────────────── + console.log("\n=== positional fix with scope ==="); + { + const r = parseDoctorArgs("fix M001"); + assertEq(r.mode, "fix", "fix M001 sets mode to fix"); + assertEq(r.requestedScope, "M001", "fix M001 parses scope as M001"); + } + + // ── 5. --fix combined with other flags ───────────────────────────────────── + console.log("\n=== --fix combined with --dry-run ==="); + { + const r = parseDoctorArgs("--fix --dry-run"); + assertTrue(r.fixFlag, "--fix --dry-run sets fixFlag"); + assertTrue(r.dryRun, "--fix --dry-run sets dryRun"); + assertEq(r.requestedScope, undefined, "no scope leaked from combined flags"); + } + + // ── 6. --fix combined with --json ────────────────────────────────────────── + console.log("\n=== --fix with --json ==="); + { + const r = parseDoctorArgs("--fix --json"); + assertTrue(r.fixFlag, "--fix --json sets fixFlag"); + assertTrue(r.jsonMode, "--fix --json sets jsonMode"); + assertEq(r.requestedScope, undefined, "no scope leaked from --fix --json"); + } + + // ── 7. Empty args (baseline) ─────────────────────────────────────────────── + console.log("\n=== empty args baseline ==="); + { + const r = parseDoctorArgs(""); + assertEq(r.fixFlag, false, "empty args: fixFlag false"); + assertEq(r.mode, "doctor", "empty args: mode is doctor"); + assertEq(r.requestedScope, undefined, "empty args: no scope"); + } + + // ── 8. heal and audit modes unaffected ───────────────────────────────────── + console.log("\n=== heal and audit modes ==="); + { + const rh = parseDoctorArgs("heal M001/S01"); + assertEq(rh.mode, "heal", "heal mode parsed correctly"); + assertEq(rh.requestedScope, "M001/S01", "heal scope parsed correctly"); + + const ra = parseDoctorArgs("audit"); + assertEq(ra.mode, "audit", "audit mode parsed correctly"); + } + + report(); +} + +main();