fix(#832): retry on stale tree SHA in commitFiles and DeleteFiles - #865
fix(#832): retry on stale tree SHA in commitFiles and DeleteFiles#865fullsend-ai-coder[bot] wants to merge 2 commits into
Conversation
When the GitHub API returns a 422 "Tree SHA does not exist" during tree or commit creation, the base_tree SHA has gone stale due to concurrent branch operations (e.g. parallel e2e cleanup). This is the same class of race as a non-fast-forward ref update. Add isStaleTreeSHAError helper to detect these 422 responses and wrap them with forge.ErrNonFastForward so the existing retry loops re-fetch and retry from scratch. Changes: - Add isStaleTreeSHAError helper (case-insensitive, checks both top-level message and error details) - commitFilesTo: wrap stale-tree-SHA 422 at create-tree and create-commit steps so commitFilesWithRetry retries - DeleteFiles: extract deleteFilesOnBranch and add deleteFilesWithRetry wrapper (same pattern as commitFilesWithRetry) with stale-SHA handling at both steps Tests: - TestIsStaleTreeSHAError: helper detection coverage - TestCommitFiles_StaleTreeSHARetry: end-to-end retry for CommitFiles - TestDeleteFiles_StaleTreeSHARetry: end-to-end retry for DeleteFiles Note: pre-commit could not run (network access unavailable in sandbox). The post-script runs an authoritative pre-commit on the runner. Closes #832
|
🤖 Finished Review · ✅ Success · Started 9:44 AM UTC · Completed 9:59 AM UTC |
|
Looks good to me Previous runReviewFindingsMedium
Low
Labels: PR fixes a bug in the GitHub forge client's error handling during concurrent operations |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
requires-manual-review
|
/fs-fix |
|
🤖 Finished Fix · ✅ Success · Started 10:32 AM UTC · Completed 10:39 AM UTC |
…Branch Add explicit 422 non-fast-forward detection at the ref-update step in deleteFilesOnBranch, matching commitFilesTo's pattern. Without this, a concurrent push that moves the ref between our read and the PATCH would not be wrapped with forge.ErrNonFastForward, so deleteFilesWithRetry would not retry the operation. Add TestDeleteFiles_NonFastForwardRefUpdateRetry integration test. Addresses review feedback on #865
🔧 Fix agent — iteration 1 (human-triggered)Fixed the medium-severity error-handling-gap: added 422 non-fast-forward detection at the ref-update step in deleteFilesOnBranch, matching commitFilesTo's existing pattern, with integration test. Disagreed with 2 low-severity findings that the reviewer explicitly marked as follow-up or visibility-only. Fixed (1):
Disagreed (2):
Tests: passedNext steps:
|
|
🤖 Finished Review · ✅ Success · Started 10:41 AM UTC · Completed 10:54 AM UTC |
fullsend-ai-review
Bot
added
ready-for-merge
|
This pull request has been automatically marked as stale because it has not had any activity in the last month. It will be closed in 2 weeks if no further activity occurs. Remove the |
1 participant
Summary
Retry
commitFilesToandDeleteFileswhen the GitHub API returns a 422 "Tree SHA does not exist" error, fixing intermittent behaviour test failures during concurrent after-scenario cleanup.Related Issue
Fixes #832
Changes
isStaleTreeSHAErrorhelper to detect 422 "Tree SHA does not exist" API errors (case-insensitive, checks both top-level message and error details)forge.ErrNonFastForwardat both the create-tree and create-commit stages incommitFilesTo, socommitFilesWithRetryretries the full operation from scratchDeleteFilesto extractdeleteFilesOnBranchand adddeleteFilesWithRetrywrapper (same retry pattern ascommitFilesWithRetry) with stale-SHA handling at both tree creation and commit creation stepsisStaleTreeSHAErrorcovering top-level messages, error details, case insensitivity, and negative casesCommitFilesandDeleteFilesstale-tree-SHA retry pathsTesting
TestIsStaleTreeSHAError— verifies the new helper detects stale tree SHA errors correctlyTestCommitFiles_StaleTreeSHARetry— end-to-end retry for CommitFiles when tree creation returns stale SHATestDeleteFiles_StaleTreeSHARetry— end-to-end retry for DeleteFiles when tree creation returns stale SHATestIsNonFastForwardError— existing tests still pass (no regression)TestDeleteFiles_Atomic— existing tests still pass (no regression)TestCommitFiles_NonFastForwardRetry— existing tests still pass (no regression)internal/forge/githubpackage tests pass with-racego vet ./internal/forge/github/...passesgo build ./...passesChecklist
Closes #832
Post-script verification
agent/832-delete-files-stale-sha)c887fc47c59d8b87b5282ca44044d2933d30df41..HEAD)