Skip to content
Open
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
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,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/<slug>` branch, `--current-branch`, or `--worktree` (creates a sibling `<repo>-gnhf-worktrees/<slug>/` checkout). New branch and worktree runs probe numeric suffixes such as `gnhf/<slug>-1` and `<repo>-gnhf-worktrees/<slug>-1/` on collisions; current-branch runs first resume an exact same-prompt `.gnhf/runs/<runId>/` on a clean working tree, otherwise they probe `.gnhf/runs/<runId>-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/<runId>/` 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. If it's a `--worktree` run with zero commits and no pending commit failure, the worktree is removed; otherwise it's preserved and the path is printed. 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. If it's a `--worktree` run with zero commits and no pending commit failure, the worktree is removed; otherwise it's preserved and the path is printed. 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/`)

Expand Down
93 changes: 91 additions & 2 deletions src/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -80,6 +80,7 @@ interface CliMockOverrides {
close: ReturnType<typeof vi.fn>;
};
stdinIsTTY?: boolean;
consoleErrorSink?: unknown[][];
}

async function runCliWithMocks(
Expand All @@ -91,7 +92,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,
) => {
Expand Down Expand Up @@ -3214,6 +3219,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<void>(() => {})),
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");
Expand Down
29 changes: 26 additions & 3 deletions src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -695,6 +695,22 @@ program
let effectiveCwd = cwd;
let worktreePath: string | null = null;
let worktreeCleanup: (() => void) | null = null;
let getOrchestratorState:
| (() => ReturnType<Orchestrator["getState"]>)
| null = null;
const shouldPreserveWorktree = (): boolean => {
try {
const state = getOrchestratorState?.();
if (!state) return false;
return (
state.commitCount > 0 || state.hasPendingCommitFailure === true
Comment on lines +703 to +706

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Check Git before deleting the worktree

When a successful commitAll() is followed by an exception before recordSuccess() refreshes state.commitCount - for example, appendNotes() fails due to an I/O error - the fatal path calls process.exit() while this cached count is still zero. The exit handler therefore removes a worktree that already contains a commit, contrary to the required preservation rule; determine commit presence from the repository or update the state immediately after commitAll() succeeds.

AGENTS.md reference: AGENTS.md:L26-L26

Useful? React with 👍 / 👎.

);
} 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/");
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -980,6 +999,7 @@ program
...(options.push ? { push: true } : {}),
},
);
getOrchestratorState = () => orchestrator.getState();
let shutdownSignal: NodeJS.Signals | null = null;
let forceShutdownRequested = false;

Expand Down Expand Up @@ -1065,6 +1085,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 {
Expand Down
Loading