From 73707b22ea5d344e361fbe9b15dc7cb2a2955f3c Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Sat, 11 Apr 2026 03:57:08 +1000 Subject: [PATCH 1/5] Add injectable runner to gatherGitSnapshot and a failing test for #242 Phase 1 of the fix for issue #242. gatherGitSnapshot now accepts an optional runner parameter (default: runGit) so tests can inject a function that throws without spawning real git processes. Adds a test that reproduces the crash: injects a runner that rejects for [rev-parse, HEAD] (all other commands return empty string) and asserts the snapshot resolves with head empty string. The test is deliberately failing at this stage because no .catch() guards have been added yet. --- apps/claude-sdk-cli/src/gitSnapshot.ts | 4 ++-- apps/claude-sdk-cli/test/gitSnapshot.spec.ts | 19 ++++++++++++++++++- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/apps/claude-sdk-cli/src/gitSnapshot.ts b/apps/claude-sdk-cli/src/gitSnapshot.ts index b6aa990..a41367b 100644 --- a/apps/claude-sdk-cli/src/gitSnapshot.ts +++ b/apps/claude-sdk-cli/src/gitSnapshot.ts @@ -70,8 +70,8 @@ async function runGit(args: string[]): Promise { return stdout; } -export async function gatherGitSnapshot(): Promise { - const [branchOut, headOut, statusOut, stashOut] = await Promise.all([runGit(['branch', '--show-current']), runGit(['rev-parse', 'HEAD']), runGit(['status', '--porcelain']), runGit(['stash', 'list', '--no-decorate'])]); +export async function gatherGitSnapshot(runner: (args: string[]) => Promise = runGit): Promise { + const [branchOut, headOut, statusOut, stashOut] = await Promise.all([runner(['branch', '--show-current']), runner(['rev-parse', 'HEAD']), runner(['status', '--porcelain']), runner(['stash', 'list', '--no-decorate'])]); return { branch: parseBranch(branchOut), head: parseHead(headOut), diff --git a/apps/claude-sdk-cli/test/gitSnapshot.spec.ts b/apps/claude-sdk-cli/test/gitSnapshot.spec.ts index 864bbbb..9282887 100644 --- a/apps/claude-sdk-cli/test/gitSnapshot.spec.ts +++ b/apps/claude-sdk-cli/test/gitSnapshot.spec.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { parseBranch, parseHead, parseStash, parseStatus } from '../src/gitSnapshot.js'; +import { gatherGitSnapshot, parseBranch, parseHead, parseStash, parseStatus } from '../src/gitSnapshot.js'; // --------------------------------------------------------------------------- // parseBranch @@ -165,3 +165,20 @@ describe('parseStash', () => { expect(actual).toEqual(expected); }); }); + +// --------------------------------------------------------------------------- +// gatherGitSnapshot +// --------------------------------------------------------------------------- + +describe('gatherGitSnapshot', () => { + it('resolves with head empty string when rev-parse HEAD fails', async () => { + const runner = (args: string[]): Promise => { + if (args[0] === 'rev-parse' && args[1] === 'HEAD') { + return Promise.reject(new Error('fatal: ambiguous argument HEAD')); + } + return Promise.resolve(''); + }; + const snapshot = await gatherGitSnapshot(runner); + expect(snapshot.head).toEqual(''); + }); +}); From ccc34aa38ce8f71c4c3ebc9ee4911a89dbc82138 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Sat, 11 Apr 2026 03:57:32 +1000 Subject: [PATCH 2/5] Session log 2026-04-11: phase 1 of git-snapshot-error-handling --- .claude/sessions/2026-04-11.md | 41 ++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 .claude/sessions/2026-04-11.md diff --git a/.claude/sessions/2026-04-11.md b/.claude/sessions/2026-04-11.md new file mode 100644 index 0000000..055f331 --- /dev/null +++ b/.claude/sessions/2026-04-11.md @@ -0,0 +1,41 @@ +# Session 2026-04-11 + +Branch: `fix/git-snapshot-error-handling`. Phase 1 complete, Phase 2 pending. + +## Context + +Prompt: `2026-04-11_242_git-snapshot-error-handling.md` (Phase 1 of 2). + +Issue #242: `gatherGitSnapshot` runs four git commands in parallel via `Promise.all`. When any command fails (e.g. `git rev-parse HEAD` in a repo with no commits, exit code 128), the `Promise.all` rejection propagates unhandled and crashes the process. + +## Done + +### Phase 1: Refactor and failing test + +**Step 1: Refactor** — Added optional `runner` parameter to `gatherGitSnapshot` with `runGit` as the default: + +```typescript +export async function gatherGitSnapshot(runner: (args: string[]) => Promise = runGit): Promise +``` + +Updated the `Promise.all` body to call `runner(...)` instead of `runGit(...)`. No callers pass a runner, so default behaviour is unchanged. + +**Step 2: Failing test** — Added `gatherGitSnapshot` describe block to `test/gitSnapshot.spec.ts`. The test injects a runner that rejects for `['rev-parse', 'HEAD']` and returns empty string for all other args. Asserts the snapshot resolves with `head: ''`. + +**Step 3: Test run** — Test is failing as required: + +``` +FAIL test/gitSnapshot.spec.ts > gatherGitSnapshot > resolves with head empty string when rev-parse HEAD fails +Error: fatal: ambiguous argument HEAD + at runner test/gitSnapshot.spec.ts:177:31 + at gatherGitSnapshot src/gitSnapshot.ts:74:110 + +Test Files 1 failed | 17 passed (18) + Tests 1 failed | 426 passed (427) +``` + +Committed: `73707b2 Add injectable runner to gatherGitSnapshot and a failing test for #242` + +## What comes next (Phase 2) + +Add `.catch(() => '')` to each `runner(...)` call inside the `Promise.all`. The existing parsers already handle empty string correctly. Verify build, type-check, tests, lint, then push and open a PR referencing #242. From ce8e120eace33a7cada39686c27066226d2df05d Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Sat, 11 Apr 2026 04:03:16 +1000 Subject: [PATCH 3/5] Catch per-command failures in gatherGitSnapshot The four git commands in gatherGitSnapshot ran in parallel via Promise.all with no error handling. If any command failed (e.g. git rev-parse HEAD in a repo with no commits, exit code 128), the rejection propagated out of Promise.all and crashed the process. Fix: add .catch(() => '') to each runner call. Failed commands default to an empty string. The existing parsers already handle empty strings correctly: parseHead returns '', parseBranch returns '', parseStatus returns empty arrays, parseStash returns 0. The snapshot degrades gracefully instead of killing the process. Fixes #242 --- apps/claude-sdk-cli/src/gitSnapshot.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/claude-sdk-cli/src/gitSnapshot.ts b/apps/claude-sdk-cli/src/gitSnapshot.ts index a41367b..4dfffb0 100644 --- a/apps/claude-sdk-cli/src/gitSnapshot.ts +++ b/apps/claude-sdk-cli/src/gitSnapshot.ts @@ -71,7 +71,7 @@ async function runGit(args: string[]): Promise { } export async function gatherGitSnapshot(runner: (args: string[]) => Promise = runGit): Promise { - const [branchOut, headOut, statusOut, stashOut] = await Promise.all([runner(['branch', '--show-current']), runner(['rev-parse', 'HEAD']), runner(['status', '--porcelain']), runner(['stash', 'list', '--no-decorate'])]); + const [branchOut, headOut, statusOut, stashOut] = await Promise.all([runner(['branch', '--show-current']).catch(() => ''), runner(['rev-parse', 'HEAD']).catch(() => ''), runner(['status', '--porcelain']).catch(() => ''), runner(['stash', 'list', '--no-decorate']).catch(() => '')]); return { branch: parseBranch(branchOut), head: parseHead(headOut), From b81f684c08d4e7416f95011bbc699dd549e1bf92 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Sat, 11 Apr 2026 04:05:07 +1000 Subject: [PATCH 4/5] Add changelog entry for gatherGitSnapshot fix --- apps/claude-sdk-cli/changes.jsonl | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/claude-sdk-cli/changes.jsonl b/apps/claude-sdk-cli/changes.jsonl index f870af0..2e472b7 100644 --- a/apps/claude-sdk-cli/changes.jsonl +++ b/apps/claude-sdk-cli/changes.jsonl @@ -1 +1,2 @@ {"description":"Fix `GitStateMonitor` reporting the agent's own file edits and commits as human activity between turns","category":"fixed"} +{"description":"Fix `gatherGitSnapshot` crashing when any git command fails (e.g. `rev-parse HEAD` in a repo with no commits)","category":"fixed"} From 3fe8731df911cd9a3f74f9a39c51f1a016da8358 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Sat, 11 Apr 2026 04:20:35 +1000 Subject: [PATCH 5/5] Update session log: phase 2 complete, PR #243 --- .claude/sessions/2026-04-11.md | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/.claude/sessions/2026-04-11.md b/.claude/sessions/2026-04-11.md index 055f331..51c0b12 100644 --- a/.claude/sessions/2026-04-11.md +++ b/.claude/sessions/2026-04-11.md @@ -36,6 +36,18 @@ Test Files 1 failed | 17 passed (18) Committed: `73707b2 Add injectable runner to gatherGitSnapshot and a failing test for #242` -## What comes next (Phase 2) +## Phase 2: Fix and ship -Add `.catch(() => '')` to each `runner(...)` call inside the `Promise.all`. The existing parsers already handle empty string correctly. Verify build, type-check, tests, lint, then push and open a PR referencing #242. +**Fix** - Added `.catch(() => '')` to each `runner(...)` call inside the `Promise.all`. Biome reformatted the `Promise.all` to a single line. + +**Checks** - `pnpm build`, `pnpm type-check`, `pnpm test` (427/427), `pnpm run ci` all pass. + +**Changelog** - Added entry to `apps/claude-sdk-cli/changes.jsonl`. + +**Commits**: +- `ce8e120 Catch per-command failures in gatherGitSnapshot` +- `b81f684 Add changelog entry for gatherGitSnapshot fix` + +**Script fix** - `~/.claude/skills/github-pr/scripts/create-github-pr.sh` was building label args via unquoted string concatenation, breaking labels with spaces (`pkg: claude-sdk-cli`). Fixed to use `set -- "$@" --label "$label"` and pass `"$@"` to `gh pr create`, which preserves quoting correctly. + +**PR**: https://github.com/shellicar/claude-cli/pull/243 - auto-merge enabled, checks in progress.