fix(cursor): steer Windows bridge shell away from PS 5.1 syntax loops - #627
Conversation
Native-shell rejection told the model to replay the same bash/CMD command through the Codex bridge, which on Windows PowerShell 5.1 fails and loops (lidge-jun#604). Adapt for the host shell and inject PS-safe guidance instead.
|
@codex review |
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR replaces static native-shell denial text with shared host-shell-aware guidance and updates Cursor tool notes with Windows PowerShell 5.1 and POSIX command examples. Tests verify denial wording and generated platform-specific guidance. ChangesCursor shell guidance
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2611629ed2
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/adapters/cursor/native-exec-shell.ts`:
- Around line 30-32: The shell guidance in native-exec-shell.ts must explicitly
allow at most one corrected bridge attempt, then require reporting the failure
and stopping; update both Windows and non-Windows branches to prevent repeating
equivalent commands. Apply the same retry-stop contract to both fallback notes
in tool-definitions.ts, covering each platform-specific instruction without
changing unrelated guidance.
In `@tests/cursor-native-exec-policy.test.ts`:
- Around line 145-146: Update nativeShellDisabledMessage() and the rejection
test to make platform selection injectable, defaulting to process.platform for
existing callers. Add deterministic coverage that exercises both the POSIX and
win32 message branches through the same rejection path, asserting each
platform-specific wording rather than only generic text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 764aa1bc-b8a7-4663-8fbb-82a663a37913
📒 Files selected for processing (4)
src/adapters/cursor/native-exec-shell.tssrc/adapters/cursor/tool-definitions.tstests/cursor-native-exec-policy.test.tstests/cursor-tool-definitions.test.ts
…stop guidance Drop proxy-OS platform branching and `;`-as-`&&` advice; use if ($?) and one corrected bridge attempt for Codex/CodeRabbit feedback on lidge-jun#627.
|
@codex review |
|
✅ Action performedReview finished.
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/adapters/cursor/native-exec-shell.ts`:
- Around line 35-37: Update the shell guidance in
src/adapters/cursor/native-exec-shell.ts:35-37 and
src/adapters/cursor/tool-definitions.ts:420-423 to explicitly state that
PowerShell 5.1 does not support && or ||, and require if ($?) { ... } for
conditional follow-up steps; add matching assertions in
tests/cursor-native-exec-policy.test.ts:146-149 and
tests/cursor-tool-definitions.test.ts:335-337.
In `@tests/cursor-native-exec-policy.test.ts`:
- Around line 146-149: Update the deniedShellText assertions in the
cursor-native execution policy regression test to explicitly require guidance
that PowerShell 5.1 does not support &&. Preserve the existing assertions for if
($?), semicolon handling, and the PowerShell version while adding coverage for
the rejected && syntax.
In `@tests/cursor-tool-definitions.test.ts`:
- Around line 335-337: Update the assertions for the PowerShell note in the
relevant cursor tool definition test to explicitly verify that `&&` is
unsupported and that `if ($?)` is the replacement. Replace or supplement the
existing semicolon assertion while preserving the checks for the corrected
bridge-attempt limit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b583905e-d035-477e-99d5-859095a12a32
📒 Files selected for processing (4)
src/adapters/cursor/native-exec-shell.tssrc/adapters/cursor/tool-definitions.tstests/cursor-native-exec-policy.test.tstests/cursor-tool-definitions.test.ts
CodeRabbit follow-up: make unsupported-parser-error wording explicit and assert it in rejection/guidance tests.
|
@codex review Note: prior windows-latest failure was unrelated suite flake (storage cleanup / policy-preview timeouts), not #604 guidance tests. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== PR metadata and changed files ==='
gh pr view 627 --repo lidge-jun/opencodex --json number,title,headRefName,baseRefName,mergeable,statusCheckRollup
echo '=== PR diff ==='
gh pr diff 627 --repo lidge-jun/opencodex -- \
src/adapters/cursor/native-exec-shell.ts \
src/adapters/cursor/tool-definitions.ts \
tests/cursor-native-exec-policy.test.ts \
tests/cursor-tool-definitions.test.ts
echo '=== Source outlines ==='
ast-grep outline src/adapters/cursor/native-exec-shell.ts --items all
ast-grep outline src/adapters/cursor/tool-definitions.ts --items all
echo '=== Relevant source occurrences with context ==='
rg -n -C 10 \
'nativeShellDisabledMessage|NATIVE_SHELL_DISABLED|rejectShellExecForPolicy|rejectShellStreamExecForPolicy|rejectBackgroundShellSpawnExecForPolicy|rejectWriteShellStdinExecForPolicy|buildCursorToolGuidanceSystemNote|hostShellNote' \
src/adapters/cursor/native-exec-shell.ts \
src/adapters/cursor/tool-definitions.ts
echo '=== Relevant test assertions with context ==='
rg -n -C 10 \
'native shell|nativeShellDisabledMessage|PowerShell|cd /d|<<EOF|if \(\$\?\)|bridge attempt|host-shell-safe|buildCursorToolGuidanceSystemNote' \
tests/cursor-native-exec-policy.test.ts \
tests/cursor-tool-definitions.test.tsLength of output: 4369 🏁 Script executed: #!/bin/bash
set -euo pipefail
base_ref="origin/dev"
if ! git rev-parse --verify "$base_ref" >/dev/null 2>&1; then
base_ref="dev"
fi
merge_base="$(git merge-base "$base_ref" HEAD)"
echo '=== Changed files relative to PR base ==='
git diff --name-status "$merge_base" HEAD
echo '=== Complete PR diff ==='
git diff --no-ext-diff --unified=80 "$merge_base" HEAD -- \
src/adapters/cursor/native-exec-shell.ts \
src/adapters/cursor/tool-definitions.ts \
tests/cursor-native-exec-policy.test.ts \
tests/cursor-tool-definitions.test.ts
echo '=== Source structure ==='
ast-grep outline src/adapters/cursor/native-exec-shell.ts --items all
ast-grep outline src/adapters/cursor/tool-definitions.ts --items all
echo '=== Native shell policy implementation ==='
nl -ba src/adapters/cursor/native-exec-shell.ts | sed -n '1,420p'
echo '=== Cursor tool guidance implementation ==='
nl -ba src/adapters/cursor/tool-definitions.ts | sed -n '1,420p'
echo '=== Native shell policy tests ==='
nl -ba tests/cursor-native-exec-policy.test.ts | sed -n '1,460p'
echo '=== Cursor tool guidance tests ==='
nl -ba tests/cursor-tool-definitions.test.ts | sed -n '1,520p'Length of output: 1504 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Authoritative PR file patches from GitHub ==='
gh api --paginate \
'repos/lidge-jun/opencodex/pulls/627/files?per_page=100' \
--jq '.[] | "FILE: \(.filename)\nSTATUS: \(.status)\nPATCH:\n\(.patch // "[no textual patch returned]")\n---"'
echo '=== Locate reviewed files in checkout ==='
for name in \
native-exec-shell.ts \
tool-definitions.ts \
cursor-native-exec-policy.test.ts \
cursor-tool-definitions.test.ts
do
fd -a -t f "^${name}$" . || true
done
echo '=== Native-shell implementation and guidance source ==='
for path in \
src/adapters/cursor/native-exec-shell.ts \
src/adapters/cursor/tool-definitions.ts
do
if [ -f "$path" ]; then
echo "--- $path ---"
nl -ba "$path" | sed -n '1,460p'
fi
done
echo '=== Relevant tests ==='
for path in \
tests/cursor-native-exec-policy.test.ts \
tests/cursor-tool-definitions.test.ts
do
if [ -f "$path" ]; then
echo "--- $path ---"
nl -ba "$path" | sed -n '1,560p'
fi
doneLength of output: 11519 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Native execution policy implementation (line-numbered via sed) ==='
sed -n '=' src/adapters/cursor/native-exec-shell.ts | sed -n '1,280p' >/dev/null
sed -n '1,280p' src/adapters/cursor/native-exec-shell.ts | cat -n
echo '=== Guidance construction ==='
sed -n '360,490p' src/adapters/cursor/tool-definitions.ts | cat -n
echo '=== Bridge tool schemas and working-directory handling ==='
rg -n -C 8 --type ts \
'(shell_command|exec_command|workingDirectory|working_directory|cwd)' \
src/adapters/cursor \
tests/cursor-tool-definitions.test.ts
echo '=== All policy-denial function usage and focused test coverage ==='
rg -n -C 8 --type ts \
'(rejectShellExecForPolicy|rejectShellStreamExecForPolicy|rejectBackgroundShellSpawnExecForPolicy|rejectWriteShellStdinExecForPolicy|nativeShellDisabledMessage|shellStream|backgroundShellSpawn|writeShellStdin)' \
src/adapters/cursor \
tests/cursor-native-exec-policy.test.ts
echo '=== Full changed test sections ==='
sed -n '1,280p' tests/cursor-native-exec-policy.test.ts | cat -n
sed -n '250,380p' tests/cursor-tool-definitions.test.ts | cat -nLength of output: 180070
[P1] Keep filesystem policy-denial recovery guidance host-shell-safe — The PR updates shell-denial guidance and the system note to prevent replaying POSIX/CMD syntax through a PowerShell 5.1 bridge. However, native filesystem policy denials still tell the model to continue with:
This string is emitted by Please update this denial text to use the same host-shell-aware guidance as the new shell message—for example, POSIX The core shell change itself is otherwise consistent: I did not run tests because the review sandbox prohibits executing repository files. The current CI status retrieved during review showed ✅ Action performedReview finished.
|
|
Ready to merge. |
|
Transition complete for the Carry onto Go decision: deferred — Cursor tool-guidance / shell-bridge notes under |
…#627) * fix(cursor): steer Windows bridge shell away from PS 5.1 syntax loops Native-shell rejection told the model to replay the same bash/CMD command through the Codex bridge, which on Windows PowerShell 5.1 fails and loops (#604). Adapt for the host shell and inject PS-safe guidance instead. * fix(cursor): address #604 review — host-shell-neutral retry-stop guidance Drop proxy-OS platform branching and `;`-as-`&&` advice; use if ($?) and one corrected bridge attempt for Codex/CodeRabbit feedback on #627. * fix(cursor): state PowerShell 5.1 has no &&/|| in #604 guidance CodeRabbit follow-up: make unsupported-parser-error wording explicit and assert it in rejection/guidance tests.
fix(cursor): carry Windows bridge shell guidance onto dev2-go (#627)
1 participant
Summary
cursor/autoon Windows PowerShell 5.1 could burn 20–45 tool calls after shell failures by repeatedly replaying bash/CMD syntax through the Codex shell bridge.win32, inject PowerShell 5.1 guidance into Cursor tool system notes (nocd /d, no bash heredocs, no&&; prefer PowerShell cmdlets /;).Why
Native Cursor shell is deny-by-default; rejection stderr previously said “silently call that bridge tool with the same command.” Models then replay
cd /d … && …and bash heredocs into PS 5.1, fail, and continue — a known Cursor/Codex Windows pattern (forum reports + Webwright #53-style loops). Guidance previously only listed POSIXcat/ls/rgexamples.Test plan
bun test tests/cursor-tool-definitions.test.ts tests/cursor-native-exec-policy.test.tsbun run typecheck/privacy:scanCloses #604
Summary by CodeRabbit