GeezerVison is showing github.com — Fix uv settings test failing on Windows nightly by mikolalysenko · Pull Request #1663 · SocketDev/socket-patch
Conversation
Problem
The nightly CI run on main is red. Run 39730006970, job test (windows-latest, 1):
---- patch::redirect::upstream::uv::tests::global_uv_settings_reads_the_user_then_first_system_file stdout ----
panicked at crates\socket-patch-core\src\patch\redirect\upstream\uv.rs:2389:9:
assertion `left == right` failed
left: ["C:\\...\\.tmp1nilbo\\xdg\\uv\\uv.toml"]
right: ["C:\\...\\.tmp1nilbo\\xdg/uv/uv.toml", "C:\\...\\.tmp1nilbo\\dirs2/uv/uv.toml"]
test result: FAILED. 6220 passed; 1 failed
The test landed yesterday in #1614. Windows unit tests run only on the nightly schedule, so PR and merge-queue CI never ran it. The failure is deterministic, not a flake, and it will fail every nightly until fixed.
Root cause
The test joins its XDG_CONFIG_DIRS entries with std::env::join_paths, which uses ; on Windows. global_uv_settings(.., windows = false) models uv's Unix lookup and splits on :. On Windows the joined value also contains drive letters (C:\...), so no entry resolves to the fixture directory and only the user file is found. The production code is correct: on Windows it calls global_uv_settings(.., cfg!(windows)), which reads %APPDATA% and %PROGRAMDATA% and never takes the XDG path.
Fix
The fix is in the test only:
- It builds the list with
:, the separator the code under test uses.
- It sets
XDG_CONFIG_DIRS, and expects the dirs2 system file, only on Unix. Unix is the only place that lookup is used, and a : list can't hold Windows paths.
- On Windows the test still checks the
XDG_CONFIG_HOME user file and the %APPDATA%/%PROGRAMDATA% branch.
No assertion is weakened on Unix, and the test keeps running everywhere it ran before (PR/merge-queue Linux, nightly Windows/macOS).
Proof
- On Linux,
cargo test -p socket-patch-core --lib global_uv_settings passes.
rustfmt was run on the touched file only.- Bugbot found that the expected paths were display strings of
root.join("xdg/uv/uv.toml"). On Windows those keep their /, while the function builds paths with native \ separators, so the assertion would still fail. It was right; the original failure output shows the same mismatch. 481872b builds every expected path one segment at a time, the same way the function does.
- On Windows,
XDG_CONFIG_DIRS is no longer set. The function falls back to the default /etc/xdg, which is root-relative on the runner's D: drive and doesn't exist, so the result is the user file alone. That matches expected. A local Windows run isn't possible here, so the next nightly, or a workflow_dispatch of CI on this branch, confirms it.
Generated by Claude Code
Note Low Risk
Test-only changes to env setup and path expectations; no production uv settings lookup logic is modified.
Overview
Fixes the nightly Windows failure in global_uv_settings_reads_the_user_then_first_system_file without changing production behavior.
The test no longer builds XDG_CONFIG_DIRS with join_paths (; on Windows) while asserting Unix-style global_uv_settings(.., false) lookup, which splits on :. Unix-only cases now use colon-separated dirs and still expect the first system uv.toml under dirs2. On Windows the test skips that XDG system branch and keeps checking user config plus %APPDATA% / %PROGRAMDATA%.
Expected paths are built via a new uv_toml helper that joins path segments like the implementation, so assertions use native separators instead of mixed / in root.join("…") display strings.
Reviewed by Cursor Bugbot for commit 481872b. Configure here.
Generated by Claude Code
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
global_uv_settings_reads_the_user_then_first_system_file joined its
XDG_CONFIG_DIRS entries with std::env::join_paths, which uses `;` on
Windows, while global_uv_settings(.., windows = false) models uv's
Unix lookup and splits on `:`. On Windows the joined list also holds
drive letters (`C:\...`), so no entry resolved and the test found
only the user file. Windows unit tests run only nightly, so the test
landed green in #1614 and has turned the nightly CI on main red.
Build the list with `:` and only add it, and expect the system file,
on Unix, where that lookup is used. Windows keeps checking the
XDG_CONFIG_HOME user file and the %APPDATA%/%PROGRAMDATA% branch.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_0192beEXGii8W4DwWxnr6Fbq
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
cursor
Bot
left a comment
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stale Bugbot comment from a previous run.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
The test compared display strings of root.join("xdg/uv/uv.toml")
against paths global_uv_settings builds one segment at a time, so on
Windows the expected side kept its forward slashes and every
assertion of a found path would still fail (Bugbot on #1663). Build
the expected paths segment by segment too.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_0192beEXGii8W4DwWxnr6Fbq
cursor[bot] reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
cursor[bot] reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 481872b. Configure here.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 481872b. Configure here.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
test-release (2) failed in e2e_redirect_cargo_shapes::cargo_hosted_refuses_a_lockless_project_with_other_dependencies (job). This PR changes only a unit test in uv.rs, so the failure is not from this PR.
The test's setup step cargo update -p [email protected] --precise 1.0.4 exited non-zero. The helper discards cargo's stderr, so the log doesn't say why. Its sibling cargo_hosted_refuses_a_crate_another_crate_depends_on ran the same pin step in the same run and passed, which points to a crates.io or index-fetch blip. #1654 raises CARGO_NET_RETRY for this exact helper (cargo_e2e_matrix::cargo_command) on CI. I'm not porting it here: the cause isn't confirmed, and porting it would touch #1654's file. If this recurs, the next step is to include cargo update's stderr in that assert. I'm re-running the failed job once.
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Ready for review at 481872b.
- CI: 53/53 checks green (34 success, 19 skipped). No main-wide failures.
- Bugbot: reviewed
481872b, no new findings. Its one earlier finding (expected paths kept / separators, so the Windows comparison would still fail) was fixed in 481872b and the thread is resolved.
- Mergeable, no conflicts; no
CHANGELOG.md change. Test-only diff in upstream/uv.rs; cargo test -p socket-patch-core --lib global_uv_settings passes on Linux.
- Reviewer note: PR CI doesn't run Windows unit tests, so the Windows leg this fixes is unproven until the next nightly (or a
workflow_dispatch of CI on this branch).
- Slack: not announced. This run had no Slack send tool, so the next run will retry.
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Final review brief
What it does: Fixes the nightly Windows failure in global_uv_settings_reads_the_user_then_first_system_file (added in #1614). The test built XDG_CONFIG_DIRS with join_paths (; on Windows), but the Unix lookup splits on :. It also compared against root.join("xdg/uv/uv.toml") strings, which keep / on Windows. The change is test-only. Production code is untouched.
Risk: low. One test in one file changed, and the Unix assertions are unchanged in strength. Windows unit tests run only nightly, so the Windows leg is unproven until the next nightly.
Look here:
- uv.rs:2383:``
uv_toml helper builds expected paths one segment at a time, as the function does at uv.rs:867
- uv.rs:2397-2407:````
XDG_CONFIG_DIRS and the `dirs2` expectation now apply on Unix only
- uv.rs:876-884:````` the
: split in the code under test. On Windows the default `/etc/xdg` is drive-relative and never exists, so expecting the user file alone is correct
Verified:
- Read the full diff. Only
uv.rs changes, and CHANGELOG.md is untouched.
- Walked through the rest of the test on Windows by hand. The
dirs4:dirs1::dirs2 case still passes there: splitting on the drive letter's : yields only paths that don't exist, and the empty entry stops the list.
cargo test -p socket-patch-core --lib global_uv_settings passes on Linux.- CI:
ci-ok and clippy are green on 481872b, and Cursor Bugbot passes. Its one thread (native separators) is fixed and resolved.
Changes I made: none.
Open questions: none. If you want the Windows leg proven before merging, run a workflow_dispatch of CI on this branch.
Auto-merge is armed: approving sends this PR straight to the merge queue.
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
The Windows leg is now confirmed. In the workflow_dispatch CI run on this branch (39745825228), test (windows-latest, 1) and test (windows-latest, 2) both passed. test-release (2) also passed on its one re-run, so its earlier failure was the crates.io blip.
That run's ci-ok is still red. The only cause is e2e-docker (sbt), which was cancelled at its 45-minute timeout on both attempts. This PR is not the cause:
- That job runs only on schedule and dispatch, never on PR or merge-queue CI.
- In both attempts it spent the whole budget building the sbt Docker image with no cache. The last log lines before the cancel are sbt 1.2.8 warming at
Updating ProjectRef(... warm-1-2-8-build), stalled for about 18 minutes.
- On main's nightly, with a warm cache, the same job takes about 6 minutes (2026-10-21: 05:52 to 05:59).
This PR touches nothing Docker or sbt related, and no fix for the cold-cache stall exists yet. I'm recording it as a CI reliability item. The pull_request run's ci-ok on this head is green (39739005695).
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Final reviewer: disarmed auto-merge on 481872b. ci-ok on this head is now red. The cause is the workflow_dispatch CI run 39745825228, attempt 2. Its e2e-docker (sbt) job hung in "Build sbt image" for 45 minutes (09:02 to 09:47 UTC) and was cancelled. This PR only changes a test in upstream/uv.rs, so that hang is infra, not this PR. The pull_request run on the same head is green (ci-ok and clippy).
That run was already re-run once (attempt 2), so I haven't re-run it again. CI janitor: the sbt image build hang needs a look. Once ci-ok is green on this head again, the next final-review pass re-arms auto-merge. The review brief above still applies, since the code is unchanged.
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Auto-merge was turned off, apparently because the newest ci-ok on this head was the red one from the manual workflow_dispatch run. That run's only failure is the nightly-only e2e-docker (sbt) timeout, explained above. I've re-run the ci-ok job of this PR's own pull_request run (39739005695). That job's result was already green, and the re-run makes the PR gate the newest ci-ok again. Once it's green, the final-review routine can re-arm auto-merge.
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Oct 22, 2026
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Oct 23, 2026
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Labels
ci-janitor
Opened by the CI janitor routine (flakes, redundant tests, CI perf)
Ready for review
Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
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.
|