diff --git a/AGENTS.md b/AGENTS.md index 32134b8b..ca88fc5b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -24,7 +24,7 @@ Entry point is `src/cli.ts`. It parses flags with commander, resolves config, ha 1. `cli.ts` decides one of four modes: new branch, resume an existing `gnhf/` branch, `--current-branch`, or `--worktree` (creates a sibling `-gnhf-worktrees//` checkout). New branch and worktree runs probe numeric suffixes such as `gnhf/-1` and `-gnhf-worktrees/-1/` on collisions; current-branch runs first resume an exact same-prompt `.gnhf/runs//` on a clean working tree, otherwise they probe `.gnhf/runs/-1/` metadata collisions without creating a branch; worktree mode also resumes preserved suffixed worktrees before creating a new one. When resuming with a different prompt, it asks whether to update `prompt.md` and continue the existing run history, start a new branch, or quit; if stdin is piped, that confirmation comes from the controlling terminal before any sleep-prevention re-exec. `setupRun`/`resumeRun` in `src/core/run.ts` create `.gnhf/runs//` with `prompt.md`, `notes.md`, `output-schema.json`, `base-commit`, optional `stop-when`, `commit-message`, and `gnhf.log`, and add `.gnhf/runs/` to `.git/info/exclude` so run metadata stays local. 2. `Orchestrator` (`src/core/orchestrator.ts`) is an `EventEmitter` loop. Each iteration: build prompt via `src/templates/iteration-prompt.ts` (injects current `notes.md`), add commit-repair instructions when a prior `git commit` failed, call `agent.run(...)`, then on success `commitAll` + append to `notes.md` and optionally `pushCurrentBranch`; on failure `resetHard` unless a pending commit failure is preserving uncommitted work for repair. The user-visible failure/rollback contract lives in the README's "How It Works"; in code, `commitAll` throws `CommitFailedError`, logs `git:commit:failed`, and records the commit output in `notes.md` so the next iteration can repair the workspace, retryable thrown agent errors increment the backoff streak, and `PermanentAgentError` aborts after rollback with `lastAgentError` set for the renderer. The `RunLimits` object enforces `--max-iterations` (between iterations), `--max-tokens` (mid-iteration via AbortController), `--stop-when` (post-iteration via the agent's `should_fully_stop` output, deferred while a commit failure awaits repair), and `--push` post-success publishing. 3. `Renderer` (`src/renderer.ts` + `src/renderer-diff.ts`) is a cell-based TUI using the alt screen buffer. `cli.ts` enters/exits alt screen around it. The renderer subscribes to orchestrator events, diffs frames to minimize writes, and updates the terminal title live. `MockOrchestrator` (`src/mock-orchestrator.ts`) drives the renderer offline via `--mock` for demos/testing. -4. Shutdown path: `SIGINT` routes through `orchestrator.handleInterrupt()`. The first press requests a graceful stop, letting the current iteration finish or ending backoff early; the second press force-stops via `orchestrator.stop()`. `SIGTERM` force-stops immediately. `cli.ts` only keeps the done screen open for aborted runs; graceful stops exit once shutdown cleanup finishes. The README's [How It Works](./README.md#how-it-works) section owns worktree preservation and cleanup behavior. After cleanup, `cli.ts` collects final branch/diff stats via `src/core/git.ts` and writes the permanent stdout summary rendered by `src/core/exit-summary.ts`, including an uncommitted-work warning when a commit failure is still pending. +4. Shutdown path: `SIGINT` routes through `orchestrator.handleInterrupt()`. The first press requests a graceful stop, letting the current iteration finish or ending backoff early; the second press force-stops via `orchestrator.stop()`. `SIGTERM` force-stops immediately. `cli.ts` only keeps the done screen open for aborted runs; graceful stops exit once shutdown cleanup finishes. The README's [How It Works](./README.md#how-it-works) section owns worktree preservation and cleanup behavior. That rule must hold on every exit path, not just the normal cleanup block: the `process.on("exit")` cleanup fallback re-checks orchestrator state before removing a worktree, and the force-exit timeout prints the preserved path before calling `process.exit()`. After cleanup, `cli.ts` collects final branch/diff stats via `src/core/git.ts` and writes the permanent stdout summary rendered by `src/core/exit-summary.ts`, including an uncommitted-work warning when a commit failure is still pending. ### Agents (`src/core/agents/`) diff --git a/src/cli.test.ts b/src/cli.test.ts index eeed2642..8af0c281 100644 --- a/src/cli.test.ts +++ b/src/cli.test.ts @@ -9,7 +9,7 @@ import { writeFileSync, } from "node:fs"; import { tmpdir } from "node:os"; -import { dirname, join } from "node:path"; +import { dirname, isAbsolute, join } from "node:path"; import { describe, expect, it, vi } from "vitest"; import { CONVENTIONAL_COMMIT_MESSAGE } from "./core/commit-message.js"; import type { Config } from "./core/config.js"; @@ -81,6 +81,7 @@ interface CliMockOverrides { close: ReturnType; }; stdinIsTTY?: boolean; + consoleErrorSink?: unknown[][]; } async function runCliWithMocks( @@ -92,7 +93,11 @@ async function runCliWithMocks( const stdoutWrite = vi .spyOn(process.stdout, "write") .mockImplementation(() => true); - const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); + const consoleError = vi + .spyOn(console, "error") + .mockImplementation((...args: unknown[]) => { + overrides.consoleErrorSink?.push(args); + }); const exitSpy = vi.spyOn(process, "exit").mockImplementation((( code?: string | number | null, ) => { @@ -3215,6 +3220,90 @@ describe("cli", () => { expect(removeWorktree).not.toHaveBeenCalled(); }); + it("preserves and reports a worktree with commits when exit handler fires before preservation block", async () => { + vi.useFakeTimers(); + const removeWorktree = vi.fn(); + const createWorktree = vi.fn(); + const consoleErrorSink: unknown[][] = []; + const exitHandlers: (() => void)[] = []; + const processOnSpy = vi.spyOn(process, "on"); + processOnSpy.mockImplementation(((event: string, handler: () => void) => { + if (event === "exit") { + exitHandlers.push(handler); + } + return process; + }) as typeof process.on); + + try { + const cliPromise = runCliWithMocks( + ["ship it", "--worktree"], + { + agent: "claude", + agentPathOverride: {}, + agentArgsOverride: {}, + acpRegistryOverrides: {}, + maxConsecutiveFailures: 3, + preventSleep: false, + }, + { + removeWorktree, + createWorktree, + consoleErrorSink, + orchestratorStart: vi.fn(() => new Promise(() => {})), + orchestratorGetState: vi.fn(() => ({ + status: "completed" as const, + gracefulStopRequested: false, + currentIteration: 2, + totalInputTokens: 0, + totalOutputTokens: 0, + commitCount: 3, + iterations: [], + successCount: 2, + failCount: 0, + consecutiveFailures: 0, + startTime: new Date("2026-01-01T00:00:00Z"), + waitingUntil: null, + lastMessage: null, + })), + }, + ); + const exitPromise = expect(cliPromise).rejects.toThrow( + "process.exit unexpectedly called with 1", + ); + + await vi.waitFor(() => { + expect(exitHandlers).toHaveLength(1); + }); + await vi.advanceTimersByTimeAsync(5_000); + await exitPromise; + + // process.exit() synchronously runs exit handlers before control can + // reach the normal preservation block that nulls worktreeCleanup. + for (const handler of exitHandlers) { + handler(); + } + + expect(removeWorktree).not.toHaveBeenCalled(); + + const createdWorktreePath = createWorktree.mock.calls[0]?.[1] as string; + expect(isAbsolute(createdWorktreePath)).toBe(true); + const timeoutOutput = consoleErrorSink.map((call) => call.join(" ")); + expect( + timeoutOutput.some((line) => line.includes("shutdown timed out")), + ).toBe(true); + expect( + timeoutOutput.some( + (line) => + line.includes("worktree preserved at") && + line.includes(createdWorktreePath), + ), + ).toBe(true); + } finally { + processOnSpy.mockRestore(); + vi.useRealTimers(); + } + }); + it("resumes a preserved suffixed worktree instead of creating another one", async () => { const tempDir = mkdtempSync(join(tmpdir(), "gnhf-cli-worktree-resume-")); const repoRoot = join(tempDir, "repo"); diff --git a/src/cli.ts b/src/cli.ts index 6c6005e3..62213426 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -695,6 +695,22 @@ program let effectiveCwd = cwd; let worktreePath: string | null = null; let worktreeCleanup: (() => void) | null = null; + let getOrchestratorState: + | (() => ReturnType) + | null = null; + const shouldPreserveWorktree = (): boolean => { + try { + const state = getOrchestratorState?.(); + if (!state) return false; + return ( + state.commitCount > 0 || state.hasPendingCommitFailure === true + ); + } catch { + // Orchestrator not yet created or already torn down - safe to + // clean up since no iteration could have committed anything. + return false; + } + }; const currentBranch = getCurrentBranch(cwd); const onGnhfBranch = currentBranch.startsWith("gnhf/"); @@ -765,11 +781,14 @@ program // Ensure worktree cleanup runs even if die() or process.exit() is // called before reaching the normal cleanup block (e.g. orchestrator // crash to .catch to die to process.exit(1)). + // However, preserve worktrees that already have commits - the + // normal preservation block (worktreeCleanup = null) may not have + // run yet when force-shutdown or timeout triggers process.exit(). const exitCleanup = worktreeCleanup; process.on("exit", () => { - if (worktreeCleanup === exitCleanup) { - exitCleanup(); - } + if (worktreeCleanup !== exitCleanup) return; + if (shouldPreserveWorktree()) return; + exitCleanup(); }); } } else if (options.currentBranch) { @@ -983,6 +1002,7 @@ program ...(options.push ? { push: true } : {}), }, ); + getOrchestratorState = () => orchestrator.getState(); let shutdownSignal: NodeJS.Signals | null = null; let forceShutdownRequested = false; @@ -1068,6 +1088,9 @@ program console.error( `\n gnhf: shutdown timed out after ${FORCE_EXIT_TIMEOUT_MS / 1000}s, forcing exit\n`, ); + if (worktreePath && shouldPreserveWorktree()) { + console.error(` gnhf: worktree preserved at ${worktreePath}\n`); + } process.exit(getSignalExitCode(shutdownSignal ?? "SIGINT")); } } finally {