Skip to content

fix: A hung Git hook wedges commit operations and can survive server restart - #1108

Open
sam-saffron-jarvis wants to merge 1 commit into
SamSaffron:mainfrom
sam-saffron-jarvis:feat/codereview-65b91733
Open

fix: A hung Git hook wedges commit operations and can survive server restart#1108
sam-saffron-jarvis wants to merge 1 commit into
SamSaffron:mainfrom
sam-saffron-jarvis:feat/codereview-65b91733

Conversation

@sam-saffron-jarvis

Copy link
Copy Markdown
Contributor

What changed

  • Bound the host-owned git commit subprocess to a generous five-minute deadline without making an admitted commit depend on client cancellation.
  • Run Git in its own process group so deadline cancellation kills hooks and helper descendants, and add a bounded WaitDelay so inherited output pipes cannot keep Wait blocked.
  • Preserve the existing post-process HEAD verification, so a timeout racing with a completed commit is still reported as success when the resulting commit can be verified.
  • Add an integration test with a blocking pre-commit hook. It verifies the deadline-backed failure, hook termination, checkout lease release, and a successful follow-up commit.

Why this is high-value

Both web and TUI commit workflows use this host-owned transaction path, and it holds the checkout mutex plus the outer mutation lease while Git runs. Previously, a malformed hook or stuck signing/helper process could wait forever, permanently blocking later commits and coordinated root mutations for that checkout and preventing clean server shutdown. The bounded process-group execution turns that indefinite checkout outage into a classified, recoverable failure while retaining commit-outcome verification.

Validation

  • gofmt -w internal/gitcommit/process.go internal/gitcommit/gitcommit_test.go
  • go build ./... (passed after generating the ignored frontend assets required by go:embed)
  • go test ./internal/gitcommit -run 'TestCommitTimesOutHungHookAndReleasesCheckout|TestUnbornHookFailureIsRecoverableNotUncertain' -count=1
  • go test ./internal/gitcommit -count=3
  • go test -race ./internal/gitcommit -count=1
  • go test ./... in a clean user environment: every package passed except the unrelated existing internal/serveui.TestProductionBundleSizeBudgets; current generated app.js/app.css gzip sizes exceed main's budgets by 130/291 bytes. This change does not touch frontend sources or bundle budgets.
  • git diff --check

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant