diff --git a/README.md b/README.md index 0c13c95..006a667 100644 --- a/README.md +++ b/README.md @@ -324,7 +324,7 @@ Quote it — `--codex-config k='"true"'` — to keep a string a string. | --- | --- | | `--json` | Machine-readable JSON on stdout, no colour and no banner. | | `-q, --quiet` | Suppress the launch banner. Warnings still print. | -| `--debug` | Print stack traces on failure. | +| `--debug` | Print stack traces, and every git command that failed. | | `-h, --help` | Show this help. | | `-v, --version` | Print the version. | @@ -338,12 +338,22 @@ stops asking. Takes `--cwd` and the global flags. Needs a terminal. ### `airship doctor` -Checks, in order: `node`, `airship`, `config`, `git repo`, `overlay bundle`, `agent claude`, -`agent codex`, `agent opencode`, `dev server`. Each reports `ok`, `warn` or `fail` with a hint. -Only your preferred agent (`--agent`, default `claude`) can fail the run; the other two warn. +Checks, in order: `node`, `airship`, `config`, `git`, `git repo`, `overlay bundle`, +`agent claude`, `agent codex`, `agent opencode`, `dev server`. Each reports `ok`, `warn` or +`fail` with a hint. Only your preferred agent (`--agent`, default `claude`) can fail the run; +the other two warn. + +`git` and `git repo` are separate because they fail for different reasons and have different +fixes: whether git can run at all, and whether this directory is somewhere it can usefully run +(a work tree, with at least one commit, and a configured `user.name` / `user.email`). They fail +the run on `--agent codex` and `--agent opencode`, which reconstruct their diff baseline from +`HEAD`, and warn on `claude`, which snapshots its own before-state and needs no git to edit or +undo. Exits `1` if any check failed, so `airship doctor && airship` works. Takes `--cwd`, `--target`, -`--agent` and the global flags. +`--agent` and the global flags. `--json` prints the same checks as a machine-readable record, +which is the most useful thing to send someone when a run is failing on a machine you cannot +see. ### Exit codes @@ -415,6 +425,11 @@ editor "open in editor" prefers, otherwise probed in that order), `AIRSHIP_AGENT the Claude backend's raw stderr to the terminal — separate from `--debug`, which logs airship itself), and `NO_COLOR` / `FORCE_COLOR`. +`AIRSHIP_DEBUG=1` does what `--debug` does, which is worth knowing when the person who needs +the trace is not the person who typed the command. Both print every failed git invocation to +stderr with its argv, its exit status and the whole of its stderr — the detail behind the one +line a toast has room for. + ### `--cwd` `--cwd` is the folder your dev server treats as its root, which is not always your repository diff --git a/apps/cli/README.md b/apps/cli/README.md index 8d6b50d..33db9d2 100644 --- a/apps/cli/README.md +++ b/apps/cli/README.md @@ -326,7 +326,7 @@ Quote it — `--codex-config k='"true"'` — to keep a string a string. | --- | --- | | `--json` | Machine-readable JSON on stdout, no colour and no banner. | | `-q, --quiet` | Suppress the launch banner. Warnings still print. | -| `--debug` | Print stack traces on failure. | +| `--debug` | Print stack traces, and every git command that failed. | | `-h, --help` | Show this help. | | `-v, --version` | Print the version. | @@ -340,12 +340,22 @@ stops asking. Takes `--cwd` and the global flags. Needs a terminal. ### `airship doctor` -Checks, in order: `node`, `airship`, `config`, `git repo`, `overlay bundle`, `agent claude`, -`agent codex`, `agent opencode`, `dev server`. Each reports `ok`, `warn` or `fail` with a hint. -Only your preferred agent (`--agent`, default `claude`) can fail the run; the other two warn. +Checks, in order: `node`, `airship`, `config`, `git`, `git repo`, `overlay bundle`, +`agent claude`, `agent codex`, `agent opencode`, `dev server`. Each reports `ok`, `warn` or +`fail` with a hint. Only your preferred agent (`--agent`, default `claude`) can fail the run; +the other two warn. + +`git` and `git repo` are separate because they fail for different reasons and have different +fixes: whether git can run at all, and whether this directory is somewhere it can usefully run +(a work tree, with at least one commit, and a configured `user.name` / `user.email`). They fail +the run on `--agent codex` and `--agent opencode`, which reconstruct their diff baseline from +`HEAD`, and warn on `claude`, which snapshots its own before-state and needs no git to edit or +undo. Exits `1` if any check failed, so `airship doctor && airship` works. Takes `--cwd`, `--target`, -`--agent` and the global flags. +`--agent` and the global flags. `--json` prints the same checks as a machine-readable record, +which is the most useful thing to send someone when a run is failing on a machine you cannot +see. ### Exit codes @@ -417,6 +427,11 @@ editor "open in editor" prefers, otherwise probed in that order), `AIRSHIP_AGENT the Claude backend's raw stderr to the terminal — separate from `--debug`, which logs airship itself), and `NO_COLOR` / `FORCE_COLOR`. +`AIRSHIP_DEBUG=1` does what `--debug` does, which is worth knowing when the person who needs +the trace is not the person who typed the command. Both print every failed git invocation to +stderr with its argv, its exit status and the whole of its stderr — the detail behind the one +line a toast has room for. + ### `--cwd` `--cwd` is the folder your dev server treats as its root, which is not always your repository diff --git a/apps/cli/src/commands/doctor.ts b/apps/cli/src/commands/doctor.ts index 4b65a2a..bf0a74b 100644 --- a/apps/cli/src/commands/doctor.ts +++ b/apps/cli/src/commands/doctor.ts @@ -10,7 +10,7 @@ import { existsSync } from "node:fs"; import { createRequire } from "node:module"; import { fileURLToPath } from "node:url"; -import { type AgentKind, checkAuth, isGitRepo } from "@airship/server"; +import { type AgentKind, checkAuth, gitStatus } from "@airship/server"; import { defineCommand } from "citty"; import { AGENTS, @@ -150,16 +150,70 @@ function checkConfig(configSource: string | undefined): Check { }; } -function checkGit(cwd: string): Check { - const repo = isGitRepo(cwd); - return { - hint: repo - ? undefined - : "codex and opencode need git for their diff baseline, and undo needs it.", - label: "git repo", - level: repo ? "ok" : "warn", - value: cwd, +/** + * Two checks, because they fail for different reasons and have different fixes: + * whether git can run at all, and whether this directory is somewhere it can + * usefully run. They used to be one line that reported `isGitRepo` and nothing + * else, so a machine with no git on PATH was told it was not in a repository. + * + * The level follows the same rule `checkAgents` uses: only the backend actually + * being used can turn a missing dependency into a failure. Claude snapshots its + * own before-state through a pre-tool hook, so it edits and undoes without git + * at all; codex and opencode reconstruct their baseline from HEAD and cannot. + * `doctor` exits non-zero on any `fail` and is documented as scriptable, so a + * working Claude install must not be reported as broken. + */ +function checkGit(cwd: string, preferred: AgentKind | undefined): Check[] { + const status = gitStatus(cwd); + const needsGit = (preferred ?? "claude") !== "claude"; + const blocked: Level = needsGit ? "fail" : "warn"; + + if (!status.installed) { + return [ + { + hint: status.hint, + label: "git", + level: blocked, + value: "not installed", + }, + ]; + } + const version: Check = { + label: "git", + level: "ok", + value: status.version ?? "installed", }; + if (!status.workTree) { + return [ + version, + { + hint: status.hint, + label: "git repo", + level: blocked, + value: `${status.error ?? "unusable"} (${cwd})`, + }, + ]; + } + if (!status.hasCommits) { + return [ + version, + { + hint: status.hint, + label: "git repo", + level: blocked, + value: "no commits yet", + }, + ]; + } + if (!status.identity) { + return [ + version, + // Always a warning, whichever backend: it breaks committing, which is one + // opt-in button, and nothing else. + { hint: status.hint, label: "git repo", level: "warn", value: cwd }, + ]; + } + return [version, { label: "git repo", level: "ok", value: cwd }]; } /** @@ -222,7 +276,7 @@ export const doctor = defineCommand({ checkNode(), { label: "airship", level: "ok", value: VERSION }, checkConfig(configSource), - checkGit(cwd), + ...checkGit(cwd, agent), checkOverlay(), ...(await checkAgents(agent)), // The dev server last: it is the check most likely to be a transient diff --git a/apps/cli/src/index.ts b/apps/cli/src/index.ts index 049da6c..5a17208 100644 --- a/apps/cli/src/index.ts +++ b/apps/cli/src/index.ts @@ -10,6 +10,7 @@ * layer; the choke point is here. */ +import { onGitFailure } from "@airship/server"; import { runCommand } from "citty"; import { DOCTOR_FLAGS, doctor } from "./commands/doctor"; import { INIT_FLAGS, init } from "./commands/init"; @@ -20,6 +21,7 @@ import { out, setColorEnabled, shouldColor, + style, } from "./lib/terminal"; import { type CommandHelp, renderHelp } from "./lib/usage"; import { VERSION } from "./lib/version"; @@ -174,14 +176,39 @@ async function main(rawArgs: string[]): Promise { } const argv = process.argv.slice(2); +// Read off the raw argv rather than the parsed settings: this has to be known +// before parsing, which is itself something that can fail. +const debug = argv.includes("--debug") || Boolean(process.env.AIRSHIP_DEBUG); // Set before anything can fail, so an error raised during parsing is styled the // same as one raised after. Each command re-derives it once it knows --json. setColorEnabled(shouldColor({ json: argv.includes("--json") })); +/* + * Every git invocation that failed, in full. + * + * The result objects carry one line each, which is what a toast can show. This + * is the other half: the argv, the exit status and the whole of stderr, for the + * case where the one line is not enough. stderr rather than stdout, so `--json` + * stays parseable, and `AIRSHIP_DEBUG=1` works without changing how the daemon + * is launched — which matters when the person who needs the trace is not the + * person who knows the flags. + */ +if (debug) { + onGitFailure((failure) => { + const argvText = [failure.bin, ...failure.args].join(" "); + const status = failure.errno ?? `exit ${failure.code}`; + const detail = [failure.stderr, failure.stdout] + .map((part) => part.trimEnd()) + .filter(Boolean) + .join("\n"); + process.stderr.write( + style.dim( + ` ${argvText}\n ${status}${failure.cwd ? ` in ${failure.cwd}` : ""}\n${detail ? `${detail}\n` : ""}` + ) + ); + }); +} + main(argv).catch((err: unknown) => { - process.exit( - reportError(err, { - debug: argv.includes("--debug") || Boolean(process.env.AIRSHIP_DEBUG), - }) - ); + process.exit(reportError(err, { debug })); }); diff --git a/apps/cli/src/lib/args.ts b/apps/cli/src/lib/args.ts index 78c6b82..cfeb0c8 100644 --- a/apps/cli/src/lib/args.ts +++ b/apps/cli/src/lib/args.ts @@ -276,7 +276,7 @@ export const FLAGS: readonly FlagSpec[] = [ }, { group: "GLOBAL", - help: "Print stack traces on failure.", + help: "Print stack traces, and every git command that failed.", name: "debug", type: "boolean", }, diff --git a/apps/cli/src/lib/banner.test.ts b/apps/cli/src/lib/banner.test.ts index a84f17f..af28813 100644 --- a/apps/cli/src/lib/banner.test.ts +++ b/apps/cli/src/lib/banner.test.ts @@ -1,4 +1,15 @@ -import { afterEach, beforeAll, describe, expect, it, vi } from "vitest"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + afterAll, + afterEach, + beforeAll, + describe, + expect, + it, + vi, +} from "vitest"; import { exposureBanner, warnBackendLimits } from "./banner"; import { setColorEnabled } from "./terminal"; @@ -174,6 +185,50 @@ describe("warnBackendLimits", () => { expect(out).toBe(""); }); + + /* + * Said once at launch, rather than discovered at the first click. + * + * Unconditional, unlike the rest of this file's warnings: whatever is wrong + * with git breaks Commit and Create pull request on every backend. The + * backend-specific half is the diff baseline, which only codex and opencode + * reconstruct from HEAD. + */ + describe("git", () => { + const plain = mkdtempSync(join(tmpdir(), "airship-banner-nogit-")); + + afterAll(() => { + rmSync(plain, { force: true, maxRetries: 3, recursive: true }); + }); + + it("warns for a directory that is not a repository, naming it", () => { + const out = stderrFrom(() => + warnBackendLimits({ agent: "claude", cwd: plain }) + ); + + expect(out).toContain("not a git repository"); + expect(out).toContain(plain); + expect(out).toContain("git init"); + }); + + it("adds the baseline consequence only for a backend that has one", () => { + const claude = stderrFrom(() => + warnBackendLimits({ agent: "claude", cwd: plain }) + ); + const codex = stderrFrom(() => + warnBackendLimits({ agent: "codex", cwd: plain }) + ); + + expect(claude).not.toContain("diff baseline"); + expect(codex).toContain("diff baseline"); + }); + + it("stays silent in a healthy repository", () => { + expect( + stderrFrom(() => warnBackendLimits({ agent: "claude", cwd: REPO })) + ).toBe(""); + }); + }); }); describe("exposureBanner", () => { diff --git a/apps/cli/src/lib/banner.ts b/apps/cli/src/lib/banner.ts index 54a8f8f..6993b3d 100644 --- a/apps/cli/src/lib/banner.ts +++ b/apps/cli/src/lib/banner.ts @@ -7,7 +7,7 @@ */ import type { AgentKind, AirshipSurface } from "@airship/server"; -import { isGitRepo } from "@airship/server"; +import { gitStatus } from "@airship/server"; import { style } from "./terminal"; /** What `--safe` actually buys on this backend, stated where the user looks. */ @@ -57,12 +57,28 @@ export function warnBackendLimits(o: { process.stderr.write(`\n ${style.yellow(`⚠ ${message}`)}\n`); }; - // Codex refuses to run outside a git repo, and both non-Claude backends - // reconstruct their diff baseline from git — so this is worth saying before - // the first edit silently produces an empty diff. - if (o.agent !== "claude" && !isGitRepo(o.cwd)) { + /* + * Said once, before the first edit, rather than discovered at the first + * click. + * + * Unconditional now, where it used to fire only for the non-Claude backends. + * Whatever is wrong with git — not installed, not a repository, no commits + * yet, no commit identity — it breaks Commit and Create pull request on every + * backend. The extra sentence for codex and opencode is the part that really + * is backend-specific: they reconstruct their diff baseline from HEAD, so a + * broken git also costs them their undo. + */ + const git = gitStatus(o.cwd); + if (git.error) { + // `GitStatus.error` carries no path, because its other reader is a tooltip. + // Here there is room, and which directory is meant is the first thing a + // user asks. + const baseline = + o.agent === "claude" + ? "" + : ` ${o.agent} also needs git for its diff baseline, so undo will refuse rather than restore.`; warn( - `${o.cwd} is not a git repository. ${o.agent} needs git for its diff baseline, and undo needs git.` + `${git.error} (${o.cwd}).${baseline}${git.hint ? `\n ${git.hint}` : ""}` ); } if (o.agent !== "claude" && o.maxBudgetUsd !== undefined) { diff --git a/packages/server/src/index.ts b/packages/server/src/index.ts index 7ca50c3..7973c8a 100644 --- a/packages/server/src/index.ts +++ b/packages/server/src/index.ts @@ -66,7 +66,7 @@ export type { /** Re-exported so the CLI depends only on @airship/server. */ export { checkAuth, listModels } from "@airship/core"; export type { GitFailure, GitStatus } from "@airship/git"; -export { gitStatus, isGitRepo, onGitFailure } from "@airship/git"; +export { gitStatus, onGitFailure } from "@airship/git"; export type { AgentKind, AirshipSurface, Effort } from "@airship/protocol"; const WS_PATH = "/__airship/ws"; diff --git a/scripts/gen-models.mjs b/scripts/gen-models.mjs index 5ceb13c..88c3fa9 100644 --- a/scripts/gen-models.mjs +++ b/scripts/gen-models.mjs @@ -47,7 +47,18 @@ const SOURCE = "https://models.dev/models.json"; /** models.dev id prefix → the harness whose group the model belongs in. */ const HARNESS = { "anthropic/": "claude", "openai/": "codex" }; -const BIOME = new URL("../node_modules/.bin/biome", import.meta.url); +/** + * `.bin/biome` is the POSIX shell wrapper, which Windows cannot execute. pnpm + * writes a `biome.CMD` beside it for exactly this, and spawning that needs a + * shell — a batch file is not an executable image. The arguments here are + * fixed and the generated source travels on stdin rather than argv, so there is + * nothing for a shell to mis-split. + */ +const WIN32 = process.platform === "win32"; +const BIOME = new URL( + `../node_modules/.bin/biome${WIN32 ? ".CMD" : ""}`, + import.meta.url +); function die(message) { process.stderr.write(`gen-models: ${message}\n`); @@ -74,7 +85,7 @@ function format(source) { const out = spawnSync( fileURLToPath(BIOME), ["check", "--write", "--stdin-file-path=models.ts"], - { encoding: "utf8", input: source } + { encoding: "utf8", input: source, shell: WIN32 } ); if (out.error || out.status !== 0) { die(