fix(snapshot): stop the CI Test flake — never snapshot from a failed git add, verify revert deletions against a full tree listing#13
Closed
Aayam Bansal (aayambansal) wants to merge 2 commits into
Conversation
The 'unicode filenames modification and restore' test failed CI on push 43b5e18 with ENOENT after revert: every file in the sandbox was deleted. That happens when the 'git add' inside track() fails transiently and .nothrow() swallows it — write-tree then snapshots an empty index, and revert() against that tree treats every file as 'not in snapshot' and unlinks it. - stage via a shared stageAll() that retries once on failure and reports it; track() returns no snapshot instead of a wrong one when staging or write-tree fails - revert() no longer deletes a file when the ls-tree verification itself errors; deletion now requires a successful ls-tree that confirms the file was absent from the snapshot
The per-file 'ls-tree <hash> -- <pathspec>' check behaved differently across platforms for unusual filenames, so revert either kept files it should delete or vice versa depending on OS. List the snapshot tree once per patch with core.quotepath=false (the same mechanism patch() uses) and check membership in code: deterministic on every platform, one git call per snapshot instead of per file, and still fails safe — if the listing errors, files are kept rather than deleted on an unverified miss.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Member
Author
|
Superseded — the same snapshot hardening landed on main via #9's squash merge. A follow-up PR replaces the checkout-based restore entirely. |
Aayam Bansal (aayambansal)
added a commit
that referenced
this pull request
Jul 7, 2026
…omic temp, env prefixes (#123) Four defensive fixes in the managed-account module: - atlasFetch now COMBINES a caller-supplied signal with the timeout via AbortSignal.any instead of replacing it, so passing a signal never drops the 60s hang guard. (#46) - refreshIfStale coerces a numeric-string sync version ({"v":"5"}) instead of collapsing it to null — otherwise cached_v never updates and the CLI keeps deferring the background sync forever. (#47) - atomicWrite's temp path is unique per call (pid + uuid), so two concurrent syncs in the same process can't write the same temp file, interleave, and publish a torn synced-env.json. (#13) - filterEnvForSubprocess matches SAFE_ENV_PREFIXES exactly for plain names and only prefixes entries ending in '_' (LC_, XDG_), so HOME no longer matches HOMEBREW_GITHUB_API_TOKEN etc. (#49)
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
1 participant
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.
The required Test check has been failing on most Linux runs with
unicode filenames modification and restore— ENOENT after revert, every sandbox file deleted. Two defects compound:track()swallowedgit addfailures via.nothrow().write-treethen snapshots a stale/empty index, andrevert()against that tree deletes every file as 'not in snapshot'. Now staged viastageAll(), which retries once and reports failure —track()returns no snapshot instead of a wrong one. This was also a destructive-in-production bug: a transient git error could delete user files on revert.revert()verified deletions with a per-filels-tree <hash> -- <pathspec>whose behavior differs across platforms for unusual filenames. Replaced with onels-tree -r --name-onlylisting per snapshot (quotepath=false, the same mechanismpatch()uses), checked in code — deterministic on every platform, fewer git calls, and fail-safe: if the listing errors, files are kept rather than deleted.Same change already passed the full Test suite on the #9 branch. Landing it on main separately so every open PR inherits a green base.