Skip to content

fix(tui): restore the windows test job - #497

Merged
emal-avala merged 1 commit into
mainfrom
fix/windows-mentions-test
Jul 25, 2026
Merged

fix(tui): restore the windows test job#497
emal-avala merged 1 commit into
mainfrom
fix/windows-mentions-test

Conversation

@emal-avala

@emal-avala emal-avala commented Jul 25, 2026

Copy link
Copy Markdown
Member

Summary

Test (windows-latest) has been failing on main since the @-mentions work landed, which makes every PR's Windows job red. Two distinct problems, one hiding the other.

1. The build error

error: unused variable: `dir`
  --> crates\cli\src\ui\modern\mentions.rs:595:13
   = note: `-D unused-variables` implied by `-D warnings`
error: could not compile `agent-code` (bin "agent" test) due to 1 previous error

expand_rejects_symlink_escaping_the_workspace bound dir unconditionally but used it only inside two #[cfg(unix)] blocks. Gating the whole test on unix is the honest fix — creating a symlink on Windows needs developer mode or SeCreateSymbolicLinkPrivilege, so the scenario isn't reachable on a stock runner. Unix coverage is unchanged.

2. What the build error was hiding

Because the error killed the whole agent bin test target, no test in that binary had run on Windows since the mentions PR merged. Fixing the build surfaced 3 genuine failures (534 passed, 3 failed):

ui::modern::mentions::tests::expand_inlines_a_single_file
ui::modern::mentions::tests::expand_inlines_multiple_files_once_each
ui::modern::app::tests::submit_inlines_mentions_but_transcript_keeps_the_typed_text

assertion failed: out.prompt.contains("<file path=\"src/main.rs\">")

This is a product bug, not a test bug. display_path labels an inlined file with whatever strip_prefix returns, so on Windows the model receives:

<file path="src\main.rs">

while the user typed @src/main.rs. The model quotes that label back in subsequent tool calls, so the path shape it was shown disagrees with the one it was asked about.

Fixed by folding separators to / in the label. The fold is cfg-gated to Windows on purpose: a backslash is a legal filename character on Unix, so an unconditional replace would mislabel a real file rather than normalize a separator.

Verification

  • New test file_labels_use_forward_slashes_on_every_platform pins the label shape and runs on all platforms.
  • 31 mentions tests pass on Linux; clippy --all-targets -- -D warnings and fmt --check clean.
  • Pre-existing on main, not introduced here: runs 30174998639 and 30150752614 fail on the same build error on commits before this branch.

Note for reviewers

The Windows job was effectively a no-op for the agent binary while this was broken. Worth assuming other Windows-specific gaps accumulated in that window — this PR fixes the three that CI can see, and does not claim the surface is otherwise clean.

The test bound `dir` unconditionally but used it only inside two
`#[cfg(unix)]` blocks, so on Windows it compiled to an unused variable
and `-D warnings` failed the build. Every PR's Windows job has been red
since the test landed.

Gating the whole test is the honest fix: creating a symlink on Windows
needs developer mode or SeCreateSymbolicLinkPrivilege, so the scenario is
not reachable on a stock runner.
@emal-avala
emal-avala merged commit a069e6a into main Jul 25, 2026
14 of 15 checks passed
@emal-avala
emal-avala deleted the fix/windows-mentions-test branch July 25, 2026 21:25
@emal-avala emal-avala changed the title fix(tui): gate the symlink mention test on unix fix(tui): restore the windows test job Jul 25, 2026
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