feat(cli): name the git problem before it bites
`airship doctor` had one `git repo` line that reported `isGitRepo` and nothing else, so a machine with no git on PATH was told it was not in a repository. That is now 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. The level follows the rule `checkAgents` already set — only the backend being used can fail the run. Claude snapshots its own before-state through a pre-tool hook and edits and undoes with no git at all, so a working Claude install must not be reported as broken; codex and opencode reconstruct their baseline from HEAD and cannot. `doctor` exits non-zero on any fail and is documented as scriptable, which is what that distinction protects. The launch banner's git warning becomes unconditional. It used to fire only for the non-Claude backends, but whatever is wrong with git breaks Commit and Create pull request on every one of them; the diff-baseline sentence is the half that really is backend-specific. `--debug` had no consumer but `reportError`'s stack print. It now also prints every failed git invocation to stderr with its argv, exit status and full stderr — the detail behind the one line a toast has room for. stderr, 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. `isGitRepo` leaves @airship/server's exports with its last consumer. It stays in @airship/git as the cheap predicate `commitEdit` takes before paying for a `gitStatus`; it is no longer what anything user-facing asks, because it answers "false" to three different problems. Also: gen-models.mjs spawned `node_modules/.bin/biome`, the POSIX shell wrapper, with no shell and no `.CMD` fallback — the last un-forked spawn in the repo. Only reachable through `make models:refresh`, so it never failed in CI.
This commit is contained in:
Nayan
@@ -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
|
||||
|
||||
+20
-5
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
+32
-5
@@ -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<void> {
|
||||
}
|
||||
|
||||
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 }));
|
||||
});
|
||||
|
||||
@@ -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",
|
||||
},
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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";
|
||||
|
||||
+13
-2
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user