fix: clear local repo dir before re-extracting on validation loop iteration 2+ - #1038
Merged
Merged
Conversation
…ration 2+ openshell sandbox download uses tar extraction that fails with "File exists (os error 17)" when the destination directory already contains files from a previous iteration. Clear the local repo directory before each SafeDownload call on iteration 2+, mirroring the sandbox-side cleanup already done for output and transcripts. Fixes scribe workflow failure after upgrading to fullsend 0.8.1. Signed-off-by: Wayne Sun <gsun@redhat.com>
Site previewPreview: https://3ee2336b-site.fullsend-ai.workers.dev Commit: |
Address review findings: - Make SafeDownload failure after RemoveAll a hard error on iteration 2+, since the local repo directory is gone and continuing to validation with a missing TARGET_REPO_DIR is pointless. - Rename err to clearErr to match the naming convention at line 483. - Include repoSrc path in the warning message for CI log debugging. Signed-off-by: Wayne Sun <gsun@redhat.com>
ReviewReason: stale-head The review agent reviewed commit |
ralphbean
approved these changes
May 15, 2026
This was referenced May 15, 2026
3 tasks
2 tasks
This was referenced Jul 12, 2026
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
"File exists (os error 17)"tar extraction failure when the validation loop runs iteration 2+openshell sandbox downloaduses tar extraction that cannot overwrite existing directories — on iteration 2+ the localrepoSrcstill has files from the previous extractionos.RemoveAll(repoSrc)beforeSafeDownloadon iteration 2+, mirroring the sandbox-side cleanup already done for output and transcripts (lines 480-486)Root cause
The between-iteration cleanup in
runAgent()clears sandbox-side output and transcripts but not the local extraction target. WhenSafeDownloadre-extracts the target repo into a non-empty directory, the underlying tar extractor fails on pre-existing directories.Surfaced as a scribe workflow failure after upgrading from fullsend 0.7.0 to 0.8.1 (fullsend-ai/.fullsend#73).
Test plan
make go-vet— passesmake go-test—internal/clipasses (41.4% coverage)make lint— passesmax_iterations > 1)