Skip to content

fix(download): preserve existing files when transfers fail - #3988

Open
LeonSGP43 wants to merge 1 commit into
Tencent:mainfrom
LeonSGP43:codex/weknora-download-atomic-20261007
Open

LeonSGP43 wants to merge 1 commit into
Tencent:mainfrom
LeonSGP43:codex/weknora-download-atomic-20261007

Conversation

@LeonSGP43

Copy link
Copy Markdown

Description

An interrupted download currently truncates an existing destination and then deletes it. This affects doc download --clobber and both SDK download methods. Copy the response to a temporary file in the destination directory, close it successfully, and replace the destination only afterward. Failed transfers clean up the temporary file while preserving the previous download.

Existing permissions and destination symlinks are preserved, including relative symlinks whose target does not exist yet. New files have mode 0600 instead of the previous 0666 subject to umask. Rename atomicity is platform dependent; Windows runtime testing remains pending CI.

Type of Change

  • 🐛 Bug fix

Related Issue

No linked issue. Reproduced using the real SDK and local HTTP test servers.

Testing

Go 1.26.8 on macOS arm64. Baseline CLI regression fails once; SDK single and batch regressions fail twice (plus their parent test). Final CLI go test -race -count=1 -coverprofile=../download-cli-coverage.out -json ./...: 1191 tests/subtests pass, 21.224s. Client equivalent: 43 pass, 3.871s. Both modules pass go vet ./... and golangci-lint run --new-from-rev=origin/main ./... using official v2.12.2. CLI go build ./..., skill-wire vocabulary and secret-token checks pass. All final commands exit 0.

Tests cover interruption, successful single/batch replacement, permission preservation, symlink destinations, dangling symlinks, rename failures, and temporary cleanup. Symlink tests explicitly skip where symlink creation is unavailable. Windows/Linux runtime and unrelated backend/frontend checks were not run. An initial coverage run included an unrelated pending regression test from another branch; it was isolated before the final full rerun.

Checklist

  • git diff --check origin/main...HEAD passes
  • Changed source files are formatted
  • Targeted tests for the changed packages/components pass
  • Diff-scoped lint passes where applicable
  • Full-repository checks were run, or any unrelated/environment-dependent failures are documented above
  • Self-reviewed the code
  • Added/updated tests covering the change
  • Updated related documentation
  • Breaking changes are clearly called out in the description above

Implemented with assistance from gpt-6.1-sol; the final diff was reviewed locally.

This branch has not been deployed

No deployments
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