From ef2edd3aead194a7fda0d37584d63064a4d3f392 Mon Sep 17 00:00:00 2001 From: aron <263346377+aron-cf@users.noreply.github.com> Date: Fri, 11 Sep 2026 17:12:13 +0100 Subject: [PATCH 1/2] computer/git: clone full history, and fill three CLI gaps clone defaulted to depth 1. That made it fast, but it quietly cost the caller their history: pushing such a clone somewhere else sent only the single commit it had fetched, reported success, and left a remote whose tip hash matched while every earlier commit was missing. A content diff against that remote passes, so nothing surfaces the loss until someone looks for a parent that is not there. Default to full history and leave the shallow case to an explicit --depth, where the caller is choosing speed knowingly. Three smaller gaps go with it. cat-file grew -t and -s, which readObject already had the type and size for. log grew --format and its --pretty alias over the commonly scripted placeholders, leaving an unrecognized one as written so it shows up in the output rather than vanishing. help grew a per-command form; it previously ignored its argument and reprinted the list, which left no way to discover a command's flags except guessing and reading the exit code. --- .changeset/git-full-history-clone.md | 11 ++ docs/13_git_interface.md | 28 ++++- packages/computer/src/git/cli.test.ts | 111 ++++++++++++++++- packages/computer/src/git/cli.ts | 153 ++++++++++++++++++++++-- packages/computer/src/git/clone.test.ts | 36 +++++- packages/computer/src/git/clone.ts | 21 +++- packages/computer/src/git/plumbing.ts | 19 ++- 7 files changed, 351 insertions(+), 28 deletions(-) create mode 100644 .changeset/git-full-history-clone.md diff --git a/.changeset/git-full-history-clone.md b/.changeset/git-full-history-clone.md new file mode 100644 index 00000000..58a28c55 --- /dev/null +++ b/.changeset/git-full-history-clone.md @@ -0,0 +1,11 @@ +--- +"@cloudflare/computer": minor +--- + +`git clone` now fetches the full history by default instead of a single commit. The shallow default was faster, but a caller who cloned a repository and then pushed it somewhere else sent only the one commit it had fetched: the push reported success and the remote's tip matched, while every earlier commit was missing. Pass `--depth` to ask for a shallow clone when the history genuinely is not needed. + +`git cat-file` gained `-t` and `-s` to report an object's type and size, alongside the existing `-p`. Exactly one of the three is required, as in real git. + +`git log` gained `--format` and its alias `--pretty`, expanding the placeholders `%H`, `%h`, `%s`, `%b`, `%an`, `%ae`, `%ad`, `%cn`, `%ce`, `%cd`, and `%%`, plus the named format `oneline`. A placeholder outside that set is left as written so it is visible in the output rather than silently dropped. + +`git help ` now prints the usage line for one command instead of ignoring its argument and reprinting the full list. Only the flags this wrapper accepts are listed, so the output says what works here rather than what real git would take. diff --git a/docs/13_git_interface.md b/docs/13_git_interface.md index 9996168b..0f7458ea 100644 --- a/docs/13_git_interface.md +++ b/docs/13_git_interface.md @@ -44,6 +44,10 @@ clean fetch reset rev-parse switch update-ref ``` +`git help` lists these, and `git help ` prints the usage line for +one of them, covering only the flags this wrapper accepts. Asking for a +command that is not supported reports that rather than reprinting the list. + Global options accepted before the subcommand: - **`-C `** — run the subcommand as though invoked from @@ -412,6 +416,7 @@ ws.git.log({ | `-n ` | `depth` | | `-` (e.g. `-1`, `-5`) | `depth` | | `--oneline` | (CLI formatter) | +| `--format=` / `--pretty=` | (CLI formatter) | | `` (positional) | `ref` | The positional `` accepts revision suffixes (`HEAD~2`, @@ -423,6 +428,12 @@ blocks; `--oneline` collapses each entry to ` message, tree, parent, author, committer) so callers can format their own way. +`--format=` (and its alias `--pretty=`) expands the +placeholders `%H`, `%h`, `%s`, `%b`, `%an`, `%ae`, `%ad`, `%cn`, `%ce`, +`%cd`, and `%%`, plus the named format `oneline`. A placeholder outside +that set is left as written, so an unsupported one is visible in the +output rather than silently dropped. + *Not mapped:* `--graph`, `--all`, `--since`, `--until`, `-p`, `--stat`, `--follow`, `--reverse`. @@ -545,7 +556,7 @@ ws.git.clone({ dir?: string, ref?: string, paths?: string[], // partial checkout only - depth?: number, // default 1 + depth?: number, // default: full history singleBranch?: boolean, noTags?: boolean, headers?: Record, @@ -564,6 +575,11 @@ ws.git.clone({ | `` (positional) | `url` | | `` (positional) | `dir` | +Clone fetches the full history unless `--depth` asks otherwise. A shallow +clone is faster, but it can only push the commits it actually fetched: the +push succeeds and the remote's tip matches, while every earlier commit is +missing. Ask for a shallow clone when the history genuinely is not needed. + When `` is omitted, the CLI derives it from the last path segment of the URL, stripping a trailing `.git` — `git clone https://github.com/owner/repo.git` lands in `./repo`, matching @@ -869,7 +885,7 @@ readback that the typed API serves better directly. ### `cat-file` ``` -git cat-file -p [:] +git cat-file (-p|-t|-s) [:] ``` ```ts @@ -877,11 +893,13 @@ ws.git.catFile({ dir?: string, oid: string, filepath?: string, -}): Promise<{ oid: string; bytes: Uint8Array }> +}): Promise<{ oid: string; bytes: Uint8Array; type?: string }> ``` -Supports the `:` shorthand for tree subreads. -*Not mapped:* `-t` type, `-s` size, `--batch`. +Supports the `:` shorthand for tree subreads. `-p` prints the +object's bytes, `-t` its type, and `-s` its size in bytes; exactly one of +the three is required, as in real git. +*Not mapped:* `--batch`. ### `update-ref` diff --git a/packages/computer/src/git/cli.test.ts b/packages/computer/src/git/cli.test.ts index c3d903c0..16316621 100644 --- a/packages/computer/src/git/cli.test.ts +++ b/packages/computer/src/git/cli.test.ts @@ -1754,11 +1754,11 @@ describe("runGitCli — cat-file", () => { }); }); - it("without -p is an error", async () => { + it("without a mode flag is an error", async () => { const { client } = fakeClient(); const res = await runGitCli(client, { argv: ["cat-file", "a".repeat(40)] }); expect(res.exitCode).toBe(129); - expect(res.stderr).toContain("only -p is supported"); + expect(res.stderr).toContain("one of -p, -t, or -s is required"); }); }); @@ -2340,3 +2340,110 @@ describe("runGitCli — end-to-end against an in-process Workspace", () => { expect(res.stdout).toBe(""); }); }); + +describe("runGitCli — cat-file type and size", () => { + it("-t reports the object type", async () => { + const { client } = fakeClient( + {}, + { + catFile: () => ({ + oid: "a".repeat(40), + bytes: new TextEncoder().encode("hello\n"), + type: "commit" as const, + }), + }, + ); + const res = await runGitCli(client, { argv: ["cat-file", "-t", "a".repeat(40)] }); + expect(res.exitCode).toBe(0); + expect(res.stdout).toBe("commit\n"); + }); + + it("-s reports the object size in bytes", async () => { + const { client } = fakeClient( + {}, + { + catFile: () => ({ + oid: "a".repeat(40), + bytes: new TextEncoder().encode("hello\n"), + }), + }, + ); + const res = await runGitCli(client, { argv: ["cat-file", "-s", "a".repeat(40)] }); + expect(res.exitCode).toBe(0); + expect(res.stdout).toBe("6\n"); + }); + + it("rejects combining -p and -t", async () => { + const { client } = fakeClient(); + const res = await runGitCli(client, { argv: ["cat-file", "-p", "-t", "a".repeat(40)] }); + expect(res.exitCode).toBe(129); + expect(res.stderr).toMatch(/mutually exclusive/); + }); +}); + +describe("runGitCli — help for one command", () => { + it("prints usage for a named command", async () => { + const { client } = fakeClient(); + const res = await runGitCli(client, { argv: ["help", "log"] }); + expect(res.exitCode).toBe(0); + expect(res.stdout).toMatch(/^usage: git log /); + expect(res.stdout).toMatch(/--oneline/); + }); + + it("still prints the command list when given no argument", async () => { + const { client } = fakeClient(); + const res = await runGitCli(client, { argv: ["help"] }); + expect(res.exitCode).toBe(0); + expect(res.stdout).toMatch(/Supported workspace git commands/); + }); + + it("reports an unknown command rather than reprinting the list", async () => { + const { client } = fakeClient(); + const res = await runGitCli(client, { argv: ["help", "rebase"] }); + expect(res.exitCode).toBe(1); + expect(res.stderr).toMatch(/no help available for 'rebase'/); + }); +}); + +describe("runGitCli — log --format", () => { + const sample = (oid: string, msg: string): CommitView => ({ + oid, + message: msg, + tree: "", + parent: [], + author: { name: "A", email: "a@x", timestamp: 1_700_000_000, timezoneOffset: 0 }, + committer: { name: "C", email: "c@x", timestamp: 1_700_000_000, timezoneOffset: 0 }, + }); + + it("expands the common placeholders", async () => { + const { client } = fakeClient({}, { log: () => [sample("a".repeat(40), "second")] }); + const res = await runGitCli(client, { argv: ["log", "--format=%h %s (%an)"] }); + expect(res.exitCode).toBe(0); + expect(res.stdout).toBe("aaaaaaa second (A)\n"); + }); + + it("expands the full hash and the committer separately from the author", async () => { + const { client } = fakeClient({}, { log: () => [sample("a".repeat(40), "x")] }); + const res = await runGitCli(client, { argv: ["log", "--format=%H|%an|%cn"] }); + expect(res.stdout).toBe(`${"a".repeat(40)}|A|C\n`); + }); + + it("treats --pretty as an alias and understands the oneline format", async () => { + const { client } = fakeClient({}, { log: () => [sample("a".repeat(40), "second")] }); + const res = await runGitCli(client, { argv: ["log", "--pretty=oneline"] }); + expect(res.exitCode).toBe(0); + expect(res.stdout).toBe("aaaaaaa second\n"); + }); + + it("leaves an unknown placeholder as written", async () => { + const { client } = fakeClient({}, { log: () => [sample("a".repeat(40), "x")] }); + const res = await runGitCli(client, { argv: ["log", "--format=%h %zz"] }); + expect(res.stdout).toBe("aaaaaaa %zz\n"); + }); + + it("rejects --format combined with --oneline", async () => { + const { client } = fakeClient(); + const res = await runGitCli(client, { argv: ["log", "--oneline", "--format=%h"] }); + expect(res.exitCode).toBe(129); + }); +}); diff --git a/packages/computer/src/git/cli.ts b/packages/computer/src/git/cli.ts index 702c03df..8209f876 100644 --- a/packages/computer/src/git/cli.ts +++ b/packages/computer/src/git/cli.ts @@ -80,7 +80,7 @@ export async function runGitCli( case "help": case "--help": case "-h": - return printHelp(); + return printHelp(rest[0]); case "version": case "--version": return printVersion(); @@ -155,14 +155,66 @@ export async function runGitCli( // help / version // --------------------------------------------------------------- -function printHelp(): GitCliResult { +// Usage lines for `git help `. Only the flags this wrapper +// actually accepts are listed: the whole point is to tell a caller what +// works here, rather than what real git would take. +const COMMAND_USAGE: Record = { + add: "git add [-A] ...", + branch: "git branch [-d|-D ] [--show-current] []", + "cat-file": "git cat-file (-p|-t|-s) [:]", + checkout: "git checkout [-b ] [-f] [--] [...]", + clean: "git clean [-f] [-d] [-n] [...]", + clone: + "git clone [--depth ] [--branch ] [--single-branch|--no-single-branch] []", + commit: "git commit -m [-a]", + config: "git config [--get] []", + diff: "git diff [--stat] [--name-only] [] [--] [...]", + fetch: "git fetch [] []", + "hash-object": "git hash-object [-w] [-t ] ", + init: "git init []", + log: "git log [-n ] [-] [--oneline] []", + "ls-files": "git ls-files []", + "ls-tree": "git ls-tree []", + merge: "git merge [--ff-only] [--no-ff] ", + pull: "git pull [] []", + push: "git push [-f|--force] [--delete] [] []", + remote: "git remote [-v] [add ] [remove ]", + reset: "git reset [--hard|--soft|--mixed] [] [--] [...]", + "rev-parse": "git rev-parse ", + rm: "git rm [--cached] [-r] ...", + show: "git show []", + stash: "git stash [push|pop|apply|list|drop]", + status: "git status [-s|--short|--porcelain]", + switch: "git switch [-c ] ", + "symbolic-ref": "git symbolic-ref []", + tag: "git tag [-d ] [ []]", + "update-ref": "git update-ref ", + help: "git help []", + version: "git version", +}; + +function printHelp(topic?: string): GitCliResult { + // `git help ` used to ignore its argument and reprint the + // full list, which left no way to discover a command's flags short of + // guessing and reading the exit code. + if (topic !== undefined) { + const usage = COMMAND_USAGE[topic]; + if (usage === undefined) { + return { + stdout: "", + stderr: `git help: no help available for '${topic}'\n`, + exitCode: 1, + }; + } + return { stdout: `usage: ${usage}\n`, stderr: "", exitCode: 0 }; + } const lines = [ "usage: git []", "", "Supported workspace git commands:", " add Stage paths into the index.", " branch Create, delete, or list branches.", - " cat-file Read raw bytes for an object by oid.", + " cat-file Read an object's bytes (-p), type (-t), or size (-s).", " checkout Move HEAD to a ref, or restore paths.", " clean Remove untracked files from the working tree.", " clone Clone a remote repository into the workspace.", @@ -189,7 +241,7 @@ function printHelp(): GitCliResult { " symbolic-ref Print the current branch name.", " tag Create, delete, or list tags.", " update-ref Write a ref directly.", - " help Show this help.", + " help Show this help, or usage for one command.", " version Print the workspace git wrapper version.", "", ]; @@ -767,10 +819,21 @@ async function runLog( const parsed = parseFlags(rewritten, { n: { kind: "value" }, oneline: { kind: "bool" }, + format: { kind: "value" }, + pretty: { kind: "value" }, }); if ("error" in parsed) { return { stdout: "", stderr: `git log: ${parsed.error}\n`, exitCode: 129 }; } + // --pretty and --format are the same option in real git. + const formatSpec = (parsed.flags.format ?? parsed.flags.pretty) as string | undefined; + if (formatSpec !== undefined && parsed.flags.oneline === true) { + return { + stdout: "", + stderr: "git log: --oneline cannot be combined with --format\n", + exitCode: 129, + }; + } if (shorthandDepth !== undefined && parsed.flags.n === undefined) { parsed.flags.n = shorthandDepth; } @@ -797,13 +860,64 @@ async function runLog( try { const ref = await resolveRevisionRef(client, dir, parsed.positional[0]); const commits = await client.log({ dir, ref, depth }); - const stdout = parsed.flags.oneline ? formatLogOneline(commits) : formatLogFull(commits); + let stdout: string; + if (formatSpec !== undefined) { + // `oneline` is the one named format worth carrying, since it is + // the documented alias for the --oneline flag. + stdout = + formatSpec === "oneline" ? formatLogOneline(commits) : formatLogCustom(commits, formatSpec); + } else { + stdout = parsed.flags.oneline ? formatLogOneline(commits) : formatLogFull(commits); + } return { stdout, stderr: "", exitCode: 0 }; } catch (cause) { return mapGitError("log", cause); } } +// Expand the `%`-placeholders `git log --format` understands. This is +// the commonly scripted subset, not the full set: a placeholder that is +// not handled is left as written rather than silently dropped, so a +// caller can see what was not understood. +function formatLogCustom(commits: CommitView[], spec: string): string { + if (commits.length === 0) return ""; + const out = commits.map((c) => + // Two-letter codes are listed first: a character class holding `a` + // would otherwise match `%a` and leave the `n` of `%an` behind. + spec.replace(/%(a[dne]|c[dne]|[Hhsb%])/g, (match, code: string) => { + switch (code) { + case "H": + return c.oid; + case "h": + return c.oid.slice(0, 7); + case "s": + return firstLine(c.message); + case "b": { + const rest = c.message.split("\n").slice(1).join("\n"); + return rest.replace(/^\n+/, "").trimEnd(); + } + case "an": + return c.author.name; + case "ae": + return c.author.email; + case "ad": + return formatGitTimestamp(c.author.timestamp, c.author.timezoneOffset); + case "cn": + return c.committer.name; + case "ce": + return c.committer.email; + case "cd": + return formatGitTimestamp(c.committer.timestamp, c.committer.timezoneOffset); + case "%": + return "%"; + default: + return match; + } + }), + ); + return `${out.join("\n")}\n`; +} + function formatLogOneline(commits: CommitView[]): string { if (commits.length === 0) return ""; return `${commits.map((c) => `${c.oid.slice(0, 7)} ${firstLine(c.message)}`).join("\n")}\n`; @@ -1778,25 +1892,38 @@ async function runCatFile( args: string[], input: GitCliInput, ): Promise { - // `git cat-file -p ` -- pretty-print the object's raw - // bytes to stdout. Other forms (`-t`, `-s`) are out of scope. + // `git cat-file (-p|-t|-s) ` -- pretty-print the object's raw + // bytes, its type, or its size in bytes. Exactly one of the three + // has to be given, matching real git, which treats them as + // alternatives rather than combinable flags. const parsed = parseFlags(args, { p: { kind: "bool" }, + t: { kind: "bool" }, + s: { kind: "bool" }, }); if ("error" in parsed) { return { stdout: "", stderr: `git cat-file: ${parsed.error}\n`, exitCode: 129 }; } - if (parsed.flags.p !== true) { + const modes = (["p", "t", "s"] as const).filter((f) => parsed.flags[f] === true); + if (modes.length === 0) { + return { + stdout: "", + stderr: "git cat-file: one of -p, -t, or -s is required\n", + exitCode: 129, + }; + } + if (modes.length > 1) { return { stdout: "", - stderr: "git cat-file: only -p is supported\n", + stderr: "git cat-file: -p, -t, and -s are mutually exclusive\n", exitCode: 129, }; } + const mode = modes[0]; if (parsed.positional.length !== 1) { return { stdout: "", - stderr: "git cat-file: usage: git cat-file -p [:]\n", + stderr: `git cat-file: usage: git cat-file -${mode} [:]\n`, exitCode: 129, }; } @@ -1808,6 +1935,12 @@ async function runCatFile( const dir = resolveDir(undefined, input.cwd); try { const result = await client.catFile({ dir, oid, filepath }); + if (mode === "t") { + return { stdout: `${result.type ?? "blob"}\n`, stderr: "", exitCode: 0 }; + } + if (mode === "s") { + return { stdout: `${result.bytes.byteLength}\n`, stderr: "", exitCode: 0 }; + } const text = new TextDecoder("utf-8", { fatal: false }).decode(result.bytes); return { stdout: text, stderr: "", exitCode: 0 }; } catch (cause) { diff --git a/packages/computer/src/git/clone.test.ts b/packages/computer/src/git/clone.test.ts index 7b429c0c..5488a5cb 100644 --- a/packages/computer/src/git/clone.test.ts +++ b/packages/computer/src/git/clone.test.ts @@ -53,7 +53,7 @@ describe("cloneWith option translation", () => { http: fakeHttp, url: "https://example.test/repo.git", dir: "/", - depth: 1, + depth: undefined, singleBranch: true, noTags: true, noCheckout: true, @@ -264,3 +264,37 @@ describe("cloneWith subset checkout (real isomorphic-git + memfs)", () => { expect(await memfs.promises.readFile(`${DIR}/README.md`, "utf8")).toBe("readme\n"); }); }); + +describe("cloneWith history retention", () => { + // A shallow clone used to be the default. It made clones fast, but a + // caller who then pushed the branch somewhere else sent only the single + // commit it had: the push reported success, the tip hash matched, and + // every earlier commit was gone. Full history is the safe default, and a + // caller who wants the old behavior asks for it with --depth. + it("requests full history when no depth is given", async () => { + const { git, cloneCalls } = fakeGit(); + + await cloneWith({ + git, + http: fakeHttp, + fs: fakeFs, + url: "https://example.test/repo.git", + }); + + expect(cloneCalls[0].depth).toBeUndefined(); + }); + + it("still honors an explicit shallow depth", async () => { + const { git, cloneCalls } = fakeGit(); + + await cloneWith({ + git, + http: fakeHttp, + fs: fakeFs, + url: "https://example.test/repo.git", + depth: 1, + }); + + expect(cloneCalls[0].depth).toBe(1); + }); +}); diff --git a/packages/computer/src/git/clone.ts b/packages/computer/src/git/clone.ts index 51a1aefb..56019102 100644 --- a/packages/computer/src/git/clone.ts +++ b/packages/computer/src/git/clone.ts @@ -63,9 +63,12 @@ export interface GitCloneOptions { */ paths?: string[]; /** - * Shallow-clone depth. Defaults to 1. Pass `0` or `Infinity` for - * full history. Even with depth=1, every blob reachable from the - * tip tree is fetched — see the package README for why. + * Shallow-clone depth. Defaults to full history. Pass a positive + * number for a shallow clone, which is faster but cannot be pushed + * elsewhere without losing the commits it did not fetch. `0` and + * `Infinity` both mean full history. Even at depth 1, every blob + * reachable from the tip tree is fetched — see the package README + * for why. */ depth?: number; /** Fetch only the requested ref's branch. Default: true. */ @@ -105,10 +108,18 @@ export interface CloneWithDeps extends GitCloneOptions { export async function cloneWith(opts: CloneWithDeps): Promise { const dir = opts.dir ?? "/"; const ref = opts.ref; - const depthRaw = opts.depth ?? 1; + // Full history by default. A shallow clone used to be the default + // because it is faster, but it silently cost the caller their + // history: pushing such a clone elsewhere sends only the commits it + // fetched, reports success, and leaves a remote whose tip matches + // while every earlier commit is missing. Speed is the caller's call + // to make with --depth, correctness is not. + // // depth=0 and depth=Infinity both mean "no shallow limit" on this // surface. isomorphic-git interprets `undefined` that way. - const depth = depthRaw > 0 && Number.isFinite(depthRaw) ? depthRaw : undefined; + const depthRaw = opts.depth; + const depth = + depthRaw !== undefined && depthRaw > 0 && Number.isFinite(depthRaw) ? depthRaw : undefined; await opts.git.clone({ fs: opts.fs, diff --git a/packages/computer/src/git/plumbing.ts b/packages/computer/src/git/plumbing.ts index 9a14aabf..d5ece53a 100644 --- a/packages/computer/src/git/plumbing.ts +++ b/packages/computer/src/git/plumbing.ts @@ -9,6 +9,9 @@ import { GitError, isNotARepositoryCause, NotARepositoryError } from "./errors.js"; +/** The four object types git stores, as `cat-file -t` names them. */ +export type GitObjectType = "blob" | "tree" | "commit" | "tag"; + export interface IsomorphicGitPlumbingClient { hashBlob(args: { object: Uint8Array | string }): Promise<{ oid: string; type: string }>; writeBlob(args: { fs: object; dir: string; blob: Uint8Array }): Promise; @@ -26,8 +29,8 @@ export interface IsomorphicGitPlumbingClient { format?: "content" | "parsed" | "deflated" | "wrapped"; cache?: object; }): Promise< - | { oid: string; type: string; format: string; object: Uint8Array } - | { oid: string; type: string; format: string; object: unknown } + | { oid: string; type: GitObjectType; format: string; object: Uint8Array } + | { oid: string; type: GitObjectType; format: string; object: unknown } >; writeRef(args: { fs: object; @@ -113,6 +116,12 @@ export interface CatFileResult { oid: string; /** Raw object bytes. */ bytes: Uint8Array; + /** + * Object type, as `git cat-file -t` reports it. Present when the + * object was read through `readObject`; a blob read by the fast path + * is always `"blob"`. + */ + type?: GitObjectType; } export async function catFileWith(opts: CatFileWithDeps): Promise { @@ -130,7 +139,7 @@ export async function catFileWith(opts: CatFileWithDeps): Promise filepath: opts.filepath, cache: opts.cache, }); - return { oid, bytes: blob }; + return { oid, bytes: blob, type: "blob" }; } try { const { oid, blob } = await opts.git.readBlob({ @@ -139,7 +148,7 @@ export async function catFileWith(opts: CatFileWithDeps): Promise oid: opts.oid, cache: opts.cache, }); - return { oid, bytes: blob }; + return { oid, bytes: blob, type: "blob" }; } catch { // Not a blob; fall through to readObject's content form. } @@ -152,7 +161,7 @@ export async function catFileWith(opts: CatFileWithDeps): Promise }); const bytes = obj.object instanceof Uint8Array ? obj.object : new TextEncoder().encode(String(obj.object)); - return { oid: obj.oid, bytes }; + return { oid: obj.oid, bytes, type: obj.type }; } catch (cause) { if (isNotARepositoryCause(cause)) throw new NotARepositoryError(dir, { cause }); throw new GitError("ECATFILEFAIL", `git cat-file failed: ${errorMessage(cause)}`, { From ba8591b178356bdcafec1cb56588998782bfc8bc Mon Sep 17 00:00:00 2001 From: aron <263346377+aron-cf@users.noreply.github.com> Date: Fri, 11 Sep 2026 17:12:13 +0100 Subject: [PATCH 2/2] computer/git: make per-command help match the parsers The usage lines went in by hand and several described a command that does not exist. hash-object advertised a positional path when it only reads --stdin, reset offered --soft and --mixed when both are explicitly refused, and log omitted the --format it had just gained. A caller following that help got exit 129 for doing what it said. Correct every line against the flags its parser actually declares, including the short spellings that were missing, and add a test that walks each advertised long flag back through the command it belongs to. Drift now fails a test rather than reaching a caller. Normalize --pretty to --format while parsing, rather than reading whichever key happened to be set. They are one option in real git, so the last one written should win; before this, --format always did regardless of order. --- packages/computer/src/git/cli.test.ts | 74 +++++++++++++++++++++++++++ packages/computer/src/git/cli.ts | 63 +++++++++++++---------- 2 files changed, 109 insertions(+), 28 deletions(-) diff --git a/packages/computer/src/git/cli.test.ts b/packages/computer/src/git/cli.test.ts index 16316621..d8f82370 100644 --- a/packages/computer/src/git/cli.test.ts +++ b/packages/computer/src/git/cli.test.ts @@ -2441,9 +2441,83 @@ describe("runGitCli — log --format", () => { expect(res.stdout).toBe("aaaaaaa %zz\n"); }); + it("lets the last of --format and --pretty win, in either order", async () => { + const { client } = fakeClient({}, { log: () => [sample("a".repeat(40), "release")] }); + const formatThenPretty = await runGitCli(client, { + argv: ["log", "--format=%s", "--pretty=%h"], + }); + expect(formatThenPretty.stdout).toBe("aaaaaaa\n"); + const prettyThenFormat = await runGitCli(client, { + argv: ["log", "--pretty=%h", "--format=%s"], + }); + expect(prettyThenFormat.stdout).toBe("release\n"); + }); + it("rejects --format combined with --oneline", async () => { const { client } = fakeClient(); const res = await runGitCli(client, { argv: ["log", "--oneline", "--format=%h"] }); expect(res.exitCode).toBe(129); }); }); + +describe("runGitCli — help stays honest about the flags each command takes", () => { + // The usage lines are maintained by hand, so they can drift from the + // parsers they describe. Rather than re-check every line by eye, take + // each long flag the help advertises and confirm the command does not + // reject it as unknown or unsupported. A flag that no longer exists, + // or was never accepted, fails here instead of misleading a caller. + const commands = [ + "add", + "branch", + "cat-file", + "checkout", + "clean", + "clone", + "commit", + "config", + "diff", + "fetch", + "hash-object", + "init", + "log", + "ls-files", + "ls-tree", + "merge", + "pull", + "push", + "remote", + "reset", + "rev-parse", + "rm", + "show", + "stash", + "status", + "switch", + "symbolic-ref", + "tag", + "update-ref", + ]; + + it("advertises only flags the command's parser accepts", async () => { + const offenders: string[] = []; + for (const command of commands) { + const { client } = fakeClient(); + const help = await runGitCli(client, { argv: ["help", command] }); + expect(help.exitCode, `git help ${command}`).toBe(0); + + for (const flag of help.stdout.match(/--[a-z][a-z-]*/g) ?? []) { + const { client: probe } = fakeClient(); + const res = await runGitCli(probe, { argv: [command, flag] }); + // The parser rejects an unknown flag at 129 with a message naming + // it. Any other failure means the flag was understood and the + // command merely wanted different arguments, which is fine here. + const unknown = + res.stderr.includes(`unknown option '${flag}'`) || + res.stderr.includes(`${flag} is not supported`) || + res.stderr.includes(`only --stdin is supported`); + if (unknown) offenders.push(`${command} ${flag}: ${res.stderr.trim()}`); + } + } + expect(offenders).toEqual([]); + }); +}); diff --git a/packages/computer/src/git/cli.ts b/packages/computer/src/git/cli.ts index 8209f876..9cdb799c 100644 --- a/packages/computer/src/git/cli.ts +++ b/packages/computer/src/git/cli.ts @@ -159,36 +159,37 @@ export async function runGitCli( // actually accepts are listed: the whole point is to tell a caller what // works here, rather than what real git would take. const COMMAND_USAGE: Record = { - add: "git add [-A] ...", - branch: "git branch [-d|-D ] [--show-current] []", + add: "git add [-A|--all] [-f|--force] ...", + branch: "git branch [-d|-D|--delete] [-f|--force] [--show-current] []", "cat-file": "git cat-file (-p|-t|-s) [:]", - checkout: "git checkout [-b ] [-f] [--] [...]", - clean: "git clean [-f] [-d] [-n] [...]", + checkout: "git checkout [-b] [-f|--force] [--] [...]", + clean: "git clean [-f|--force] [-d] [-n|--dry-run] [...]", clone: - "git clone [--depth ] [--branch ] [--single-branch|--no-single-branch] []", - commit: "git commit -m [-a]", - config: "git config [--get] []", - diff: "git diff [--stat] [--name-only] [] [--] [...]", - fetch: "git fetch [] []", - "hash-object": "git hash-object [-w] [-t ] ", - init: "git init []", - log: "git log [-n ] [-] [--oneline] []", - "ls-files": "git ls-files []", + "git clone [--depth ] [-b|--branch ] [--single-branch|--no-single-branch] [--tags|--no-tags] []", + commit: 'git commit -m|--message [-a|--all] [--amend] [--author "Name "]', + config: "git config [--get|--get-all|--add|--unset] []", + diff: "git diff [--stat] [--name-only] [--name-status] [] [--] [...]", + fetch: + "git fetch [--depth ] [--single-branch|--no-single-branch] [--tags|--no-tags] [--prune] [] []", + "hash-object": "git hash-object --stdin [-w]", + init: "git init [-b|--initial-branch ] [--bare] []", + log: "git log [-n ] [-] [--oneline] [--format|--pretty=] []", + "ls-files": "git ls-files [--ref ]", "ls-tree": "git ls-tree []", - merge: "git merge [--ff-only] [--no-ff] ", - pull: "git pull [] []", - push: "git push [-f|--force] [--delete] [] []", - remote: "git remote [-v] [add ] [remove ]", - reset: "git reset [--hard|--soft|--mixed] [] [--] [...]", - "rev-parse": "git rev-parse ", - rm: "git rm [--cached] [-r] ...", + merge: "git merge [--ff-only] [--no-ff] [-m|--message ] ", + pull: "git pull [--ff-only] [--no-ff] [] []", + push: "git push [-f|--force] [-d|--delete] [] []", + remote: "git remote [add ] [remove ]", + reset: "git reset [--hard] [] [--] [...]", + "rev-parse": "git rev-parse [--abbrev-ref] [--show-toplevel] ", + rm: "git rm [--cached] ...", show: "git show []", stash: "git stash [push|pop|apply|list|drop]", - status: "git status [-s|--short|--porcelain]", - switch: "git switch [-c ] ", - "symbolic-ref": "git symbolic-ref []", - tag: "git tag [-d ] [ []]", - "update-ref": "git update-ref ", + status: "git status [-s|--short] [--porcelain[=]]", + switch: "git switch [-c] ", + "symbolic-ref": "git symbolic-ref [--short] [-q|--quiet] []", + tag: "git tag [-d|--delete] [-f|--force] [ []]", + "update-ref": "git update-ref [--force] ", help: "git help []", version: "git version", }; @@ -814,19 +815,25 @@ async function runLog( shorthandDepth = m[1]; continue; } + // --pretty is the same option as --format in real git. Normalize it + // to the one key so the parser's last-write behavior decides which + // wins: a caller appending an override to a command it did not build + // gets the last one written, whichever spelling either of them used. + if (arg === "--pretty" || arg.startsWith("--pretty=")) { + rewritten.push(`--format${arg.slice("--pretty".length)}`); + continue; + } rewritten.push(arg); } const parsed = parseFlags(rewritten, { n: { kind: "value" }, oneline: { kind: "bool" }, format: { kind: "value" }, - pretty: { kind: "value" }, }); if ("error" in parsed) { return { stdout: "", stderr: `git log: ${parsed.error}\n`, exitCode: 129 }; } - // --pretty and --format are the same option in real git. - const formatSpec = (parsed.flags.format ?? parsed.flags.pretty) as string | undefined; + const formatSpec = parsed.flags.format as string | undefined; if (formatSpec !== undefined && parsed.flags.oneline === true) { return { stdout: "",