diff --git a/.claude/agent-memory/archgate-developer/MEMORY.md b/.claude/agent-memory/archgate-developer/MEMORY.md index 8e4dc6d3..da9f25d1 100644 --- a/.claude/agent-memory/archgate-developer/MEMORY.md +++ b/.claude/agent-memory/archgate-developer/MEMORY.md @@ -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 diff --git a/src/commands/adr/create.ts b/src/commands/adr/create.ts index 2b4ee9dd..0b840976 100644 --- a/src/commands/adr/create.ts +++ b/src/commands/adr/create.ts @@ -1,3 +1,5 @@ +import { cursorTo } from "node:readline"; + import type { Command } from "@commander-js/extra-typings"; import inquirer from "inquirer"; @@ -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; diff --git a/src/commands/init.ts b/src/commands/init.ts index f5f94425..61c8d764 100644 --- a/src/commands/init.ts +++ b/src/commands/init.ts @@ -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"; @@ -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({ diff --git a/src/helpers/editor-detect.ts b/src/helpers/editor-detect.ts index 7b58a695..74f93a19 100644 --- a/src/helpers/editor-detect.ts +++ b/src/helpers/editor-detect.ts @@ -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"; @@ -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; } @@ -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; } diff --git a/src/helpers/login-flow.ts b/src/helpers/login-flow.ts index a3bbbd44..1cc63e54 100644 --- a/src/helpers/login-flow.ts +++ b/src/helpers/login-flow.ts @@ -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, @@ -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) { @@ -132,6 +142,7 @@ async function runSignupPrompt( { name: "Cursor", value: "cursor" }, ], }); + resetCursor(); editor = ans.editor; } @@ -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", @@ -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."); diff --git a/tests/helpers/editor-detect.test.ts b/tests/helpers/editor-detect.test.ts index 17fef71b..41c6cc94 100644 --- a/tests/helpers/editor-detect.test.ts +++ b/tests/helpers/editor-detect.test.ts @@ -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", () => { @@ -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(); + }); + }); });