Skip to content

app-server: make thread/shellCommand tests shell-aware#16635

Merged
bolinfest merged 1 commit intomainfrom
pr16635
Apr 3, 2026
Merged

app-server: make thread/shellCommand tests shell-aware#16635
bolinfest merged 1 commit intomainfrom
pr16635

Conversation

@bolinfest
Copy link
Copy Markdown
Collaborator

@bolinfest bolinfest commented Apr 3, 2026

Why

thread/shellCommand executes the raw command string through the current user shell, which is PowerShell on Windows. The two v2 app-server tests in app-server/tests/suite/v2/thread_shell_command.rs used POSIX printf, so Bazel CI on Windows failed with printf not being recognized as a PowerShell command.

For reference, the user-shell task wraps commands with the active shell before execution: core/src/tasks/user_shell.rs.

What Changed

Added a test-local helper that builds a shell-appropriate output command and expected newline sequence from default_user_shell():

  • PowerShell: Write-Output '...' with \r\n
  • Cmd: echo ... with \r\n
  • POSIX shells: printf '%s\n' ... with \n

Both thread_shell_command_runs_as_standalone_turn_and_persists_history and thread_shell_command_uses_existing_active_turn now use that helper.

Verification

  • cargo test -p codex-app-server thread_shell_command

@bolinfest bolinfest changed the title app-server: make thread shell command tests shell-aware app-server: make thread/shellCommand tests shell-aware Apr 3, 2026
@bolinfest bolinfest marked this pull request as ready for review April 3, 2026 00:18
@bolinfest bolinfest merged commit 862158b into main Apr 3, 2026
41 of 52 checks passed
@bolinfest bolinfest deleted the pr16635 branch April 3, 2026 00:28
@github-actions github-actions bot locked and limited conversation to collaborators Apr 3, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant