fix: clear local repo unconditionally before SafeDownload - #1045
Merged
Conversation
The iteration > 1 guard was wrong — the target-repo directory already exists on iteration 1 because the GHA workflow checks out the repo before fullsend runs. openshell's tar extraction fails with "File exists (os error 17)" on any pre-existing directory, not just on iteration 2+. Remove the iteration guard so RemoveAll runs before every SafeDownload call. Also make SafeDownload failure a hard error on all iterations since the local repo was just cleared and continuing with a missing directory is unsafe. Signed-off-by: Wayne Sun <gsun@redhat.com>
Site previewPreview: https://30eacee3-site.fullsend-ai.workers.dev Commit: |
ReviewNo findings. |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
fullsend-ai-review
Bot
added
the
ready-for-merge
- Promote RemoveAll failure from warning to hard error — if local cleanup fails, SafeDownload will hit the same EEXIST error, so fail fast with a clear message. - Surface transcript errors before aborting on SafeDownload failure — transcripts are already extracted at step 9c but their errors were skipped by the early return. - Remove unnecessary else block after SafeDownload error return (Go idiom: no else after early return). Signed-off-by: Wayne Sun <gsun@redhat.com>
5 tasks
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
iteration > 1guard on theos.RemoveAll(repoSrc)cleanup — it must run on every iteration, not just 2+SafeDownloadfailure a hard error on all iterations (not just 2+)Root cause
PR #1038 added
RemoveAllbeforeSafeDownloadbut guarded it withiteration > 1, assuming iteration 1 starts with a clean directory. In practice, the GHA workflow's "Checkout target repository" step populatestarget-repo/before fullsend runs. When the agent finishes iteration 1 andSafeDownloadextracts back into the pre-existing checkout, openshell's tar extractor fails with"File exists (os error 17)".Evidence: https://github.com/fullsend-ai/.fullsend/actions/runs/25944154001/job/76268502952 — downloaded
fullsend v0.8.2(which includes #1038) but still failed on iteration 1.Test plan
go test ./internal/cli/— passesgo vet ./...— passes