Skip to content

Catch per-command failures in gatherGitSnapshot#243

Merged
shellicar merged 5 commits intomainfrom
fix/git-snapshot-error-handling
Apr 10, 2026
Merged

Catch per-command failures in gatherGitSnapshot#243
shellicar merged 5 commits intomainfrom
fix/git-snapshot-error-handling

Conversation

@shellicar
Copy link
Copy Markdown
Owner

Summary

  • Four git commands in gatherGitSnapshot ran 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 crashed the process
  • Added .catch(() => '') to each runner call so failures degrade gracefully to empty string
  • Existing parsers already handle empty string: parseHead returns '', parseBranch returns '', parseStatus returns empty arrays, parseStash returns 0

Related Issues

Fixes #242

Co-Authored-By: Claude noreply@anthropic.com

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.
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
@shellicar shellicar added this to the 1.0 milestone Apr 10, 2026
@shellicar shellicar added bug Something isn't working pkg: claude-sdk-cli The new SDK-based CLI labels Apr 10, 2026
@shellicar shellicar self-assigned this Apr 10, 2026
@shellicar shellicar enabled auto-merge (squash) April 10, 2026 18:20
@shellicar shellicar requested a review from bananabot9000 April 10, 2026 18:20
Copy link
Copy Markdown
Collaborator

@bananabot9000 bananabot9000 left a comment

Choose a reason for hiding this comment

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

Clean fix for the empty-repo crash. .catch(() => '') on each Promise.all member is the right approach — graceful degradation per command, not a blanket catch on the whole Promise.all.

The injectable runner parameter for testability is a nice touch — no mocking execFile, no filesystem setup, just inject a function. Test is focused: rejects for rev-parse HEAD, empty string for everything else, asserts head: ''.

Session log is a good example of the new delivery notes pattern in action.

Approve.

@shellicar shellicar merged commit b2b64b9 into main Apr 10, 2026
4 checks passed
@shellicar shellicar deleted the fix/git-snapshot-error-handling branch April 10, 2026 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working pkg: claude-sdk-cli The new SDK-based CLI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GitStateMonitor crashes when repo has no HEAD

2 participants