Skip to content

fix(cli): require --yes flag for non-TTY atomic-agent update #213

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yablokolabs wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(cli): require --yes flag for non-TTY atomic-agent update #213

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 48 additions & 6 deletions src/cli/update-command.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ describe("atomic-agent update", () => {
expect(await updateCommand(["--help"], deps)).toBe(0);
expect(stdout()).toMatch(/atomic-agent update/);
expect(stdout()).toMatch(/--check/);
expect(stdout()).toMatch(/--yes/);
expect(check).not.toHaveBeenCalled();
});

Expand Down Expand Up @@ -134,8 +135,20 @@ describe("atomic-agent update", () => {
expect(runInstaller).not.toHaveBeenCalled();
});

it("updates in place when a newer version exists (non-interactive)", async () => {
expect(await updateCommand([], deps)).toBe(0);
it("refuses non-TTY update without --yes, exiting 1", async () => {
expect(await updateCommand([], deps)).toBe(1);
expect(stderr()).toMatch(/non-interactive.*--yes/);
expect(runInstaller).not.toHaveBeenCalled();
});

it("refuses non-TTY --version without --yes, exiting 1", async () => {
expect(await updateCommand(["--version", "v0.3.2"], deps)).toBe(1);
expect(stderr()).toMatch(/non-interactive.*--yes/);
expect(runInstaller).not.toHaveBeenCalled();
});

it("updates in place with --yes in non-TTY", async () => {
expect(await updateCommand(["--yes"], deps)).toBe(0);
expect(stdout()).toMatch(/current: 0\.3\.1 → latest: 0\.3\.2/);
expect(runInstaller).toHaveBeenCalledTimes(1);
expect(runInstaller).toHaveBeenCalledWith(
Expand All @@ -147,6 +160,20 @@ describe("atomic-agent update", () => {
expect(stdout()).toMatch(/updated to 0\.3\.2/);
});

it("short flag -y works the same as --yes", async () => {
expect(await updateCommand(["-y"], deps)).toBe(0);
expect(runInstaller).toHaveBeenCalledTimes(1);
});

it("--yes skips the interactive prompt in TTY", async () => {
const confirm = vi.fn().mockResolvedValue(true);
expect(await updateCommand(["--yes"], { ...deps, isTTY: () => true, confirm })).toBe(
0,
);
expect(confirm).not.toHaveBeenCalled();
expect(runInstaller).toHaveBeenCalledTimes(1);
});

it("streams installer lines prefixed with [update]", async () => {
runInstaller.mockImplementation(
async (opts?: { onLine?: (line: string) => void }) => {
Expand All @@ -155,7 +182,7 @@ describe("atomic-agent update", () => {
return { ok: true, installDir: "/tmp/install" };
},
);
expect(await updateCommand([], deps)).toBe(0);
expect(await updateCommand(["--yes"], deps)).toBe(0);
expect(stdout()).toMatch(/\[update\] downloading atomic-agent/);
expect(stdout()).toMatch(/\[update\] installed atomic-agent/);
});
Expand Down Expand Up @@ -184,19 +211,34 @@ describe("atomic-agent update", () => {
runInstaller.mockRejectedValue(
new AppUpdateError("install script exited with code 7"),
);
expect(await updateCommand([], deps)).toBe(1);
expect(await updateCommand(["--yes"], deps)).toBe(1);
expect(stderr()).toMatch(/install script exited with code 7/);
expect(stdout()).not.toMatch(/updated to/);
});

it("--version pins a specific tag even when the running version is newer", async () => {
it("--version pins a specific tag with --yes", async () => {
check.mockResolvedValue(
makeResult({ updateAvailable: false, latestTag: "v0.3.1", latestVersion: "0.3.1" }),
);
expect(await updateCommand(["--version", "v0.3.2"], deps)).toBe(0);
expect(await updateCommand(["--version", "v0.3.2", "--yes"], deps)).toBe(0);
expect(stdout()).toMatch(/installing v0\.3\.2/);
expect(runInstaller).toHaveBeenCalledWith(
expect.objectContaining({ version: "v0.3.2" }),
);
});

it("--version refuses non-TTY without --yes", async () => {
expect(await updateCommand(["--version", "v0.3.2"], deps)).toBe(1);
expect(stderr()).toMatch(/non-interactive.*--yes/);
expect(runInstaller).not.toHaveBeenCalled();
});

it("--version proceeds in TTY without --yes", async () => {
const confirm = vi.fn().mockResolvedValue(true);
expect(
await updateCommand(["--version", "v0.3.2"], { ...deps, isTTY: () => true, confirm }),
).toBe(0);
expect(confirm).toHaveBeenCalledTimes(1);
expect(runInstaller).toHaveBeenCalledTimes(1);
});
});
28 changes: 23 additions & 5 deletions src/cli/update-command.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ const HELP = [
"",
"Flags:",
" --check Check only: report current vs latest, install nothing",
" --yes, -y Skip confirmation prompt (required in non-TTY environments)",
" --version <tag> Install a specific release tag (e.g. v0.3.2) instead of latest",
" -h, --help Show this help",
"",
Expand All @@ -58,18 +59,23 @@ const HELP = [
/** Parse flags into a discriminated plan; returns a usage error string on bad input. */
function parseArgs(
args: string[],
): { ok: true; checkOnly: boolean; version?: string } | { ok: false; error: string } {
): { ok: true; checkOnly: boolean; version?: string; yes: boolean } | { ok: false; error: string } {
let checkOnly = false;
let version: string | undefined;
let yes = false;
for (let i = 0; i < args.length; i += 1) {
const arg = args[i];
if (arg === "-h" || arg === "--help") {
return { ok: true, checkOnly: false };
return { ok: true, checkOnly: false, yes: false };
}
if (arg === "--check") {
checkOnly = true;
continue;
}
if (arg === "--yes" || arg === "-y") {
yes = true;
continue;
}
if (arg === "--version") {
const value = args[i + 1];
if (!value || value.startsWith("-")) {
Expand All @@ -87,7 +93,7 @@ function parseArgs(
error: "--check and --version are mutually exclusive",
};
}
return { ok: true, checkOnly, version };
return { ok: true, checkOnly, version, yes };
}

async function defaultConfirm(prompt: string): Promise<boolean> {
Expand Down Expand Up @@ -145,7 +151,13 @@ export async function updateCommand(
return 1;
}
process.stdout.write(`installing ${parsed.version}…\n`);
if (isTTY()) {
if (!parsed.yes) {
if (!isTTY()) {
process.stderr.write(
"non-interactive update requires --yes (or -y) to confirm\n",
);
return 1;
}
const ok = await confirm(`update to ${parsed.version}? [y/N] `);
if (!ok) {
process.stdout.write("update cancelled\n");
Expand Down Expand Up @@ -184,7 +196,13 @@ export async function updateCommand(
process.stdout.write(
`current: ${result.currentVersion} → latest: ${result.latestVersion}\n`,
);
if (isTTY()) {
if (!parsed.yes) {
if (!isTTY()) {
process.stderr.write(
"non-interactive update requires --yes (or -y) to confirm\n",
);
return 1;
}
const ok = await confirm(`update to ${result.latestVersion}? [y/N] `);
if (!ok) {
process.stdout.write("update cancelled\n");
Expand Down