Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .claude/agent-memory/archgate-developer/MEMORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ Skipping steps 2 or 3 is a workflow violation. The user should NEVER have to inv
- **SLSA reusable workflow MUST be tag-pinned, not SHA-pinned** — `slsa-framework/slsa-github-generator/.github/workflows/*` looks like it should follow CI-001 (SHA pin), but the SLSA generator's `generate-builder.sh` reads the workflow ref to download the prebuilt builder from a GitHub release and rejects non-tag refs (`Invalid ref: ... Expected ref of the form refs/tags/vX.Y.Z`, exit 2). Pinning by SHA broke the v0.31.0 release ([run 25107195589](https://github.com/archgate/cli/actions/runs/25107195589)). The CI-001 rule allowlists this path so it does NOT block a SHA repin — meaning a future agent could "fix" the tag pin and the rule would be silent until the next release fails. ALWAYS keep `@v2.x.y` for `release-binaries.yml:165` and read the inline comment + CI-001 "Carved-out exceptions" before changing. Upstream issue: [slsa-framework/slsa-github-generator#150](https://github.com/slsa-framework/slsa-github-generator/issues/150).
- **Windows binary upgrade: never use detached child processes for `.old` cleanup** — On Windows, `replaceBinary()` renames the running exe to `.old` because the OS file-locks it. Cleaning up the `.old` via a detached `cmd /c ping -n 2 ... & del` process is unreliable (process may not spawn, `del` may fail silently, timing races). Instead, `cleanupStaleBinary()` runs at the next CLI startup as a fire-and-forget `unlink()` — the file is guaranteed unlocked by then. The cleanup is platform-agnostic (uses `getArtifactInfo()` to resolve the binary name), so it works on any supported platform even though only Windows currently creates `.old` files. The sync `unlinkSync` in `replaceBinary()` is kept as defense-in-depth for leftover `.old` files from previous upgrades. Do NOT reintroduce detached cleanup processes.
- **`bun:sqlite` file handles persist after `db.close()` on Windows — wrap test cleanup in try/catch** — Tests that create temp SQLite databases via `new Database(path)` will fail with `EBUSY: resource busy or locked` when `rmSync` tries to remove the temp directory in `afterEach`, even after calling `db.close()`. Windows holds the file handle briefly. Fix: (1) set `PRAGMA journal_mode = DELETE` in test DBs to avoid creating WAL/SHM files, and (2) wrap `rmSync` in `afterEach` with `try { rmSync(...) } catch { /* SQLite handles may persist */ }`. Each test must use a unique temp dir name so leftover files don't collide.
- **Inquirer prompts leave cursor at wrong column on Windows — always reset with `cursorTo`** — After an `inquirer.prompt()` call finishes (especially checkbox with long wrapped answer lines), the cursor is left at the end of the rendered answer text. On Windows terminals, `\n` moves down but does NOT reset to column 0, so all subsequent output starts at the wrong horizontal offset. Fix: call `cursorTo(process.stdout, 0)` from `node:readline` after each prompt returns, guarded by `if (process.stdout.isTTY)`. Applied in `src/helpers/editor-detect.ts` for `promptEditorSelection()` and `promptSingleEditorSelection()`. The `upgrade.ts` command already uses the same `clearLine`/`cursorTo` pattern for download progress cleanup.
- **`GITHUB_TOKEN`-authored pushes do NOT trigger downstream workflows — release.yml MUST use the GH App token** — When an Actions workflow pushes commits or opens PRs using `${{ github.token }}` / `secrets.GITHUB_TOKEN`, GitHub intentionally suppresses the resulting `push` / `pull_request` events to prevent recursion. Symptom on release PRs: the head SHA has no `pull_request`-event check runs, so `Validate Code` / `Lint, Test & Check` / `DCO Sign-off Check` are missing from the PR rollup and branch protection treats the PR as missing required checks. PR [#131](https://github.com/archgate/cli/pull/131) papered over this by manually `gh workflow run` + posting commit statuses, but `workflow_dispatch` runs land on `head_branch: release` with `pull_requests: []` — they are not associated with the PR ref, so `Lint, Test & Check` stayed orphaned and the bug recurred on PR [#251](https://github.com/archgate/cli/pull/251). Root-cause fix: in `release.yml` the `pull-request` job MUST generate a GitHub App installation token via `actions/create-github-app-token` (using `secrets.GH_APP_APP_ID` / `secrets.GH_APP_PRIVATE_KEY`) and pass it to BOTH `actions/checkout` and `simple-release-action`. App-token-authored pushes DO trigger `pull_request` events naturally. Apply the same pattern to any future workflow that pushes to a branch whose downstream CI must run.

## Validation Pipeline
Expand Down
4 changes: 4 additions & 0 deletions src/commands/adr/create.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { cursorTo } from "node:readline";

import type { Command } from "@commander-js/extra-typings";
import inquirer from "inquirer";

Expand Down Expand Up @@ -75,6 +77,8 @@ export function registerAdrCreateCommand(adr: Command) {
message: "File patterns (comma-separated, optional):",
},
]);
// Windows cursor-reset — see editor-detect.ts for explanation.
if (process.stdout.isTTY) cursorTo(process.stdout, 0);

domain = answers.domain as AdrDomain;
title = answers.title;
Expand Down
3 changes: 3 additions & 0 deletions src/commands/init.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { existsSync } from "node:fs";
import { join } from "node:path";
import { cursorTo } from "node:readline";
import { styleText } from "node:util";

import type { Command } from "@commander-js/extra-typings";
Expand Down Expand Up @@ -91,6 +92,8 @@ export function registerInitCommand(program: Command) {
default: true,
},
]);
// Windows cursor-reset — see editor-detect.ts for explanation.
if (process.stdout.isTTY) cursorTo(process.stdout, 0);

if (wantPlugin) {
const result = await runLoginFlow({
Expand Down
8 changes: 8 additions & 0 deletions src/helpers/editor-detect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
* In non-TTY (agent) contexts, defaults to "claude" for backward compatibility.
*/

import { cursorTo } from "node:readline";

import inquirer from "inquirer";

import { EDITOR_LABELS } from "./init-project";
Expand Down Expand Up @@ -79,6 +81,10 @@ export async function promptEditorSelection(
input.length > 0 || "Select at least one editor.",
},
]);
// On Windows, inquirer leaves the cursor at the end of the wrapped answer
// line. Subsequent output calls inherit that column offset instead of
// starting at column 0. Explicitly reset the cursor to prevent garbled output.
if (process.stdout.isTTY) cursorTo(process.stdout, 0);
return selected;
}

Expand All @@ -104,5 +110,7 @@ export async function promptSingleEditorSelection(
default: defaultEditor,
},
]);
// Same Windows cursor-reset fix as promptEditorSelection above.
if (process.stdout.isTTY) cursorTo(process.stdout, 0);
return selected;
}
13 changes: 13 additions & 0 deletions src/helpers/login-flow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,19 @@
* used by both `login` and `init` commands.
*/

import { cursorTo } from "node:readline";
import { styleText } from "node:util";

import inquirer from "inquirer";

/**
* Reset cursor to column 0 after an inquirer prompt on Windows.
* See editor-detect.ts for the full explanation of the bug.
*/
function resetCursor(): void {
if (process.stdout.isTTY) cursorTo(process.stdout, 0);
}

import {
requestDeviceCode,
pollForAccessToken,
Expand Down Expand Up @@ -118,6 +127,7 @@ async function runSignupPrompt(
validate: (v: string) =>
/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(v) || "Enter a valid email address",
});
resetCursor();

let editor = preselectedEditor;
if (!editor) {
Expand All @@ -132,6 +142,7 @@ async function runSignupPrompt(
{ name: "Cursor", value: "cursor" },
],
});
resetCursor();
editor = ans.editor;
}

Expand All @@ -142,6 +153,7 @@ async function runSignupPrompt(
validate: (v: string) =>
v.trim().length > 0 || "Please describe your use case",
});
resetCursor();

const { confirmed } = await inquirer.prompt({
type: "confirm",
Expand All @@ -150,6 +162,7 @@ async function runSignupPrompt(
"I agree to be contacted by the Archgate team to provide feedback during the beta period.",
default: true,
});
resetCursor();

if (!confirmed) {
logInfo("Signup cancelled.");
Expand Down
133 changes: 131 additions & 2 deletions tests/helpers/editor-detect.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,44 @@
import { describe, expect, test } from "bun:test";
import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test";

import { detectEditors } from "../../src/helpers/editor-detect";
// ---------------------------------------------------------------------------
// Module mocks — must be declared before imports that use them.
// ---------------------------------------------------------------------------

/** Tracks calls to cursorTo from node:readline. */
const mockCursorTo = mock(() => true);
mock.module("node:readline", () => ({ cursorTo: mockCursorTo }));

/** Mock inquirer so prompts resolve immediately without user interaction. */
mock.module("inquirer", () => ({
default: { prompt: mock(() => Promise.resolve({ selected: ["claude"] })) },
}));

// ---------------------------------------------------------------------------
// Imports under test — loaded AFTER mocks are registered.
// ---------------------------------------------------------------------------

import type { DetectedEditor } from "../../src/helpers/editor-detect";
import {
detectEditors,
promptEditorSelection,
promptSingleEditorSelection,
} from "../../src/helpers/editor-detect";

// ---------------------------------------------------------------------------
// Shared test data
// ---------------------------------------------------------------------------

const MOCK_DETECTED: DetectedEditor[] = [
{ id: "claude", label: "Claude Code", available: true },
{ id: "cursor", label: "Cursor", available: false },
{ id: "vscode", label: "VS Code", available: true },
{ id: "copilot", label: "GitHub Copilot", available: false },
{ id: "opencode", label: "opencode", available: false },
];

// ---------------------------------------------------------------------------
// Tests
// ---------------------------------------------------------------------------

describe("editor-detect", () => {
describe("detectEditors", () => {
Expand All @@ -23,4 +61,95 @@ describe("editor-detect", () => {
}
});
});

// -------------------------------------------------------------------------
// Cursor reset after inquirer prompts (Windows spacing fix)
//
// On Windows terminals, inquirer leaves the cursor at the column where the
// wrapped answer text ended. Without an explicit cursorTo(stdout, 0),
// subsequent output lines start at the wrong horizontal offset.
// -------------------------------------------------------------------------

describe("promptEditorSelection — cursor reset", () => {
const originalIsTTY = process.stdout.isTTY;

beforeEach(() => {
mockCursorTo.mockClear();
});

afterEach(() => {
// Restore isTTY to whatever the test runner had
Object.defineProperty(process.stdout, "isTTY", {
value: originalIsTTY,
writable: true,
configurable: true,
});
});

test("resets cursor to column 0 after prompt when stdout is TTY", async () => {
Object.defineProperty(process.stdout, "isTTY", {
value: true,
writable: true,
configurable: true,
});

await promptEditorSelection(MOCK_DETECTED);

expect(mockCursorTo).toHaveBeenCalledTimes(1);
expect(mockCursorTo).toHaveBeenCalledWith(process.stdout, 0);
});

test("does not call cursorTo when stdout is not TTY", async () => {
Object.defineProperty(process.stdout, "isTTY", {
value: undefined,
writable: true,
configurable: true,
});

await promptEditorSelection(MOCK_DETECTED);

expect(mockCursorTo).not.toHaveBeenCalled();
});
});

describe("promptSingleEditorSelection — cursor reset", () => {
const originalIsTTY = process.stdout.isTTY;

beforeEach(() => {
mockCursorTo.mockClear();
});

afterEach(() => {
Object.defineProperty(process.stdout, "isTTY", {
value: originalIsTTY,
writable: true,
configurable: true,
});
});

test("resets cursor to column 0 after prompt when stdout is TTY", async () => {
Object.defineProperty(process.stdout, "isTTY", {
value: true,
writable: true,
configurable: true,
});

await promptSingleEditorSelection(MOCK_DETECTED);

expect(mockCursorTo).toHaveBeenCalledTimes(1);
expect(mockCursorTo).toHaveBeenCalledWith(process.stdout, 0);
});

test("does not call cursorTo when stdout is not TTY", async () => {
Object.defineProperty(process.stdout, "isTTY", {
value: undefined,
writable: true,
configurable: true,
});

await promptSingleEditorSelection(MOCK_DETECTED);

expect(mockCursorTo).not.toHaveBeenCalled();
});
});
});
Loading