diff --git a/packages/shared/gitbutler-core.test.ts b/packages/shared/gitbutler-core.test.ts index f1d2fb7df..9462c8582 100644 --- a/packages/shared/gitbutler-core.test.ts +++ b/packages/shared/gitbutler-core.test.ts @@ -217,6 +217,101 @@ describe("GitButler status contract", () => { }); }); +describe("GitButler status JSON flag fallback", () => { + // 0.21.x accepts only `--format json` (gitbutlerapp/gitbutler#14061); + // 0.22.0+ accepts only `--json` (gitbutlerapp/gitbutler#15026). Each + // rejects the other spelling with clap's unexpected-argument error. + const formatRejected = commandResult( + "", + "error: unexpected argument '--format' found\n\nUsage: but [OPTIONS] [COMMAND]\n\nFor more information, try '--help'.\n", + 2, + ); + const jsonRejected = commandResult( + "", + "error: unexpected argument '--json' found\n\nUsage: but [OPTIONS] [COMMAND]\n\nFor more information, try '--help'.\n", + 2, + ); + + test("falls back to `but --json status` once when the CLI rejects `--format`, then remembers it", async () => { + const fixture = createRuntime({ version: commandResult("but 0.22.0\n") }); + const originalRunBut = fixture.runtime.runBut.bind(fixture.runtime); + fixture.runtime.runBut = async (args, options) => { + if (args.join(" ") === "--format json status") { + fixture.butCalls.push(args); + return formatRejected; + } + if (args.join(" ") === "--json status") { + fixture.butCalls.push(args); + return commandResult(statusJson()); + } + return originalRunBut(args, options); + }; + + const context = await getGitButlerContext(fixture.runtime, ROOT); + expect(context).toMatchObject({ vcsType: "gitbutler", defaultBranch: MERGE_BASE }); + const result = await runGitButlerDiff(fixture.runtime, GITBUTLER_WORKSPACE_DIFF, ROOT); + expect(result).toMatchObject({ + patch: "diff --git a/file.txt b/file.txt\n-old\n+new\n", + label: "GitButler workspace (all applied changes)", + }); + expect(fixture.butCalls.filter((args) => args.includes("status"))).toEqual([ + ["--format", "json", "status"], + ["--json", "status"], + ]); + + // The accepted spelling is remembered, so the next status call after the + // cache TTL does not pay the failed `--format` probe again. + await Bun.sleep(1_050); + await getGitButlerContext(fixture.runtime, ROOT); + expect(fixture.butCalls.filter((args) => args.includes("status"))).toEqual([ + ["--format", "json", "status"], + ["--json", "status"], + ["--json", "status"], + ]); + }); + + test("a real status failure is never retried with the other syntax", async () => { + const fixture = createRuntime({ status: commandResult("", "database locked", 1) }); + await expect(getGitButlerContext(fixture.runtime, ROOT)).rejects.toThrow(GitButlerContractError); + const failed = createRuntime({ status: commandResult("", "database locked", 1) }); + await expect(getGitButlerContext(failed.runtime, ROOT)).rejects.toThrow( + "GitButler status (`but --format json status`) failed: database locked", + ); + // The fixture's runBut throws for any unexpected invocation, so reaching + // this assertion also proves `--json status` was never attempted. + expect(failed.butCalls.filter((args) => args.includes("status"))).toEqual([ + ["--format", "json", "status"], + ]); + }); + + test("rejecting both spellings yields a clear version-requirement contract error", async () => { + const fixture = createRuntime(); + const originalRunBut = fixture.runtime.runBut.bind(fixture.runtime); + fixture.runtime.runBut = async (args, options) => { + if (args.join(" ") === "--format json status") { + fixture.butCalls.push(args); + return formatRejected; + } + if (args.join(" ") === "--json status") { + fixture.butCalls.push(args); + return jsonRejected; + } + return originalRunBut(args, options); + }; + + const pending = getGitButlerContext(fixture.runtime, ROOT); + await expect(pending).rejects.toThrow(GitButlerContractError); + await expect(pending).rejects.toThrow( + "GitButler rejected both `but --format json status` and `but --json status`; " + + "Plannotator requires GitButler 0.21.0 or newer.", + ); + expect(fixture.butCalls.filter((args) => args.includes("status"))).toEqual([ + ["--format", "json", "status"], + ["--json", "status"], + ]); + }); +}); + describe("GitButler detection and context", () => { test("detects only an active GitButler workspace ref, including from a subdirectory", async () => { const fixture = createRuntime(); @@ -622,7 +717,7 @@ describe("GitButler diffs and expansion", () => { const failedStatus = createRuntime({ status: commandResult("", "database locked", 1) }); await expect(runGitButlerDiff(failedStatus.runtime, GITBUTLER_WORKSPACE_DIFF, ROOT)).resolves.toMatchObject({ patch: "", - error: "GitButler status failed: database locked", + error: "GitButler status (`but --format json status`) failed: database locked", }); const fixture = createRuntime(); diff --git a/packages/shared/gitbutler-core.ts b/packages/shared/gitbutler-core.ts index cc47e8bc9..fcb7483d0 100644 --- a/packages/shared/gitbutler-core.ts +++ b/packages/shared/gitbutler-core.ts @@ -42,6 +42,32 @@ const VERSION_TIMEOUT_MS = 5_000; const OBJECT_ID_RE = /^(?:[0-9a-f]{40}|[0-9a-f]{64})$/i; const STATUS_CACHE_MS = 1_000; +/** + * GitButler has shipped two spellings of the global JSON output flag: + * 0.21.x accepts only `--format json` (gitbutlerapp/gitbutler#14061) while + * 0.22.0+ accepts only `--json` (gitbutlerapp/gitbutler#15026), and each + * release rejects the other spelling as an unknown argument. The primary + * spelling matches GITBUTLER_MIN_VERSION; when a status call fails with + * clap's unexpected-argument rejection for the exact flag we passed, the + * other spelling is tried once. Any other failure propagates unchanged. + */ +interface GitButlerStatusSyntax { + args: readonly string[]; + /** The flag clap names in its unexpected-argument rejection. */ + flag: string; + label: string; +} + +const GITBUTLER_STATUS_SYNTAXES: readonly [GitButlerStatusSyntax, GitButlerStatusSyntax] = [ + { args: ["--format", "json", "status"], flag: "--format", label: "but --format json status" }, + { args: ["--json", "status"], flag: "--json", label: "but --json status" }, +]; + +/** Narrow sniff for clap rejecting exactly the JSON flag this syntax passed. */ +function isUnexpectedArgumentRejection(result: GitCommandResult, flag: string): boolean { + return result.exitCode !== 0 && result.stderr.includes(`unexpected argument '${flag}'`); +} + /** Runtime operations needed by the shared GitButler provider. */ export interface ReviewGitButlerRuntime extends ReviewGitRuntime { runBut(args: string[], options?: GitCommandOptions): Promise; @@ -67,7 +93,7 @@ export interface GitButlerStack { branches: GitButlerBranch[]; } -/** Validated subset of `but --format json status`. */ +/** Validated subset of `but --format json status` (0.21.x) / `but --json status` (0.22.0+). */ export interface GitButlerStatus { mergeBase: GitButlerCommit; stacks: GitButlerStack[]; @@ -153,12 +179,15 @@ function parseStack(value: unknown, path: string): GitButlerStack { } /** Parse and validate the status fields Plannotator relies on. Unknown fields are allowed. */ -export function parseGitButlerStatus(output: string): GitButlerStatus { +export function parseGitButlerStatus( + output: string, + commandLabel = GITBUTLER_STATUS_SYNTAXES[0].label, +): GitButlerStatus { let decoded: unknown; try { decoded = JSON.parse(output); } catch { - throw new GitButlerContractError("GitButler returned invalid JSON from `but --format json status`."); + throw new GitButlerContractError(`GitButler returned invalid JSON from \`${commandLabel}\`.`); } const root = requireRecord(decoded, "root"); @@ -202,6 +231,8 @@ function versionAtLeast(actual: ParsedVersion, minimum: ParsedVersion): boolean } const versionChecks = new WeakMap>(); +/** Index into GITBUTLER_STATUS_SYNTAXES of the spelling this runtime's CLI last accepted. */ +const statusSyntaxPreferences = new WeakMap(); const statusCaches = new WeakMap< ReviewGitButlerRuntime, Map }> @@ -255,17 +286,42 @@ async function loadStatus(runtime: ReviewGitButlerRuntime, cwd: string): Promise if (existing && (existing.inFlight || existing.expiresAt > now)) return existing.status; const status = (async () => { - const result = await runtime.runBut(["--format", "json", "status"], { + const preferredIndex = statusSyntaxPreferences.get(runtime) ?? 0; + let syntaxIndex = preferredIndex; + let syntax = GITBUTLER_STATUS_SYNTAXES[syntaxIndex]; + let result = await runtime.runBut([...syntax.args], { cwd, timeoutMs: STATUS_TIMEOUT_MS, }); + if (isUnexpectedArgumentRejection(result, syntax.flag)) { + // This CLI does not know this spelling of the JSON flag; try the other + // spelling exactly once. Only clap's unexpected-argument rejection for + // the flag we passed triggers the retry — a real status failure must + // keep failing loudly below. + syntaxIndex = preferredIndex === 0 ? 1 : 0; + const fallback = GITBUTLER_STATUS_SYNTAXES[syntaxIndex]; + const retried = await runtime.runBut([...fallback.args], { + cwd, + timeoutMs: STATUS_TIMEOUT_MS, + }); + if (isUnexpectedArgumentRejection(retried, fallback.flag)) { + throw new GitButlerContractError( + `GitButler rejected both \`${syntax.label}\` and \`${fallback.label}\`; ` + + `Plannotator requires GitButler ${GITBUTLER_MIN_VERSION} or newer.`, + ); + } + syntax = fallback; + result = retried; + } if (result.exitCode !== 0) { const detail = result.stderr.trim(); throw new GitButlerContractError( - `GitButler status failed${detail ? `: ${detail}` : ` with exit code ${result.exitCode}`}`, + `GitButler status (\`${syntax.label}\`) failed${detail ? `: ${detail}` : ` with exit code ${result.exitCode}`}`, ); } - return parseGitButlerStatus(result.stdout); + const parsed = parseGitButlerStatus(result.stdout, syntax.label); + statusSyntaxPreferences.set(runtime, syntaxIndex); + return parsed; })(); const entry = { expiresAt: 0, inFlight: true, status }; cache.set(cwd, entry);