Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: deba695a71
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| setlocal EnableDelayedExpansion | ||
| echo(!first_line! | ||
| endlocal | ||
| more +1 tokens.txt > tokens.next |
There was a problem hiding this comment.
Skip first token line when rotating Windows auth script
In ProviderAuthScript::new, more +1 tokens.txt > tokens.next does not remove the consumed token; per Windows more semantics, +<n> starts output at line n, so +1 starts at the first line. This leaves tokens.txt unchanged on each invocation, so Windows runs won’t ever surface the second token (e.g., refresh-path tests in auth_tests.rs/manager_tests.rs will keep reading the initial token and can fail or stop validating refresh behavior).
Useful? React with 👍 / 👎.
|
@codex review the latest version of this PR |
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Why
The Windows
ProviderAuthScripttest helpers do not need PowerShell. Running them throughcmd.exeis enough to emit the next fixture token and rotatetokens.txt, and it avoids a PowerShell-specific dependency in these tests.What changed
print-token.ps1fixtures withprint-token.cmdincodex-rs/core/src/models_manager/manager_tests.rsandcodex-rs/login/src/auth/auth_tests.rs.codex-rs/login/src/auth/auth_tests.rsfrompowershell.exe -Command 'exit 1'tocmd.exe /d /s /c 'exit /b 1'.Verification
cargo test -p codex-logincargo test -p codex-core(fails in unrelatedcore/src/config/config_tests.rsassertions in this checkout)