Repository navigation
fix(cursor): redirect native tool denials through code-mode exec - #6230
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughCursor native-execution denial hints now distinguish freeform unified ChangesCursor execution redirect
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The catalog-aware denial guidance has no established issue requiring a fix before merge. Normal validation and maintainer approval still apply. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to A denied native operation is now directed to an available execution tool rather than unavailable commands. Native execution remains separately opt-in, but the complete live redirection path has not been verified end to end. Retained concerns Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Deterministic PR hygiene checks passed. |
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request is already Ready for Review. |
리뷰 · 우선순위 56 / 80이 PR은 Cursor가 코드 모드(freeform 라인 - 메인테이너의 판단이 필요한 지점 검증을 미룬 draft를 리뷰 큐에 둘지, 아니면 테스트 통과 증거와 readiness 체크리스트가 채워질 때까지 대기할지. 또 거절 힌트에 result-echo 전문을 넣을지(빈 출력 재발 방지) vs 짧게 줄일지. 너의 추천 의도한 수정(코드 모드에서 없는 탑레벨 셸로 보내지 않기)과 테스트 설계는 타당합니다. 다만 지금은 draft이고 실행 검증이 없어서 머지하지 마세요. 해당 테스트 스위트를 돌리고 Verification에 명령을 적은 뒤, ready 체크리스트를 채운 다음에 다시 보면 됩니다. types.ts/config.ts 분할이나 preview deploy 이야기는 이 변경과 무관하니 여기서는 닫아도 됩니다. 이 댓글은 grok-bot이 작성했습니다 |
Summary
Cursor models can stop using tools after a native Shell or Read attempt is rejected in Codex code mode. The rejection currently directs them to top-level
shell_command/exec_commandtools even though the request exposes a freeformexectool and those operations are nested helpers.Make the native-exec redirect recognize code mode and point to the advertised
execentry point with JavaScript, nestedtools.exec_command(...), and explicit result output. Preserve the existing native-execution policy and flat shell-bridge behavior. Include regression coverage and documentation for the corrected redirect contract.Verification
Validated commit
613cc84c64a8efe427c320dc45ebbf75a56ac0b6locally:bun install --frozen-lockfilesynchronized stale local Bun types to the committed 1.4.0 lockfile; no tracked files changed.bun run typecheckthen passed.node_modules/.bin/bun test tests/providers/cursor/: 1,407 passed, 0 failed across 70 files, using project-pinned Bun 1.4.0.bun run structure:check,bun run privacy:scan, and the file-size ratchet check passed.cd docs-site && bun install --frozen-lockfile && bun run build: passed; 537 pages and 73,657 internal links checked.coderabbit review --agent --base-commit 86ac102611e86ec12b3700b8ec8db2bd4deca2ff -c AGENTS.md: completed with 0 issues across all 4 changed files. Codex source review found no outstanding issues. This is local CLI review evidence; the hosted CodeRabbit review was previously skipped while draft.Full-suite scope exception: the repository has 1,858 test files; a full multi-platform run is disproportionate to this focused redirect change.
bun run test:changedexpanded into broad server/Lab coverage and was stopped while correcting stale dependencies and the non-pinned global runtime. It is NOT reported as passing. Every one of the 22 files that had failed in that attempt was rerun independently with project-pinned Bun. Fifteen passed directly; seven Lab files passed after removing inherited HTTP(S)/ALL/NO_PROXY variables (upper- and lower-case) from the test subprocess environment. The Cursor suite above and relevant source-oracle checks cover the changed behavior. Full fresh-process CI coverage remains for upstream CI.Isolated Cursor validation
Additional validation used the same commit, an isolated source-started proxy on an ephemeral loopback port, temporary OpenCodex/Codex state directories, and a dedicated Responses client implementing the code-mode tool contract. The installed OpenCodex service was not updated or restarted.
execguidance with nested helpers and explicit output for both branches. This redirect logic is shared across Cursor models.cursor/grok-4.7normal live probe passed: three sequentialexeccalls invoked nestedtools.exec_commandto read a manifest, read the referenced fixture, and obtain its line count. The final answer correctly reported the fixture's random validation token, sum (42), and line count (3).cursor/grok-4.7controlled denial-recovery probe passed: the client injected the patched native-denial text as the first tool result. The model reissued the operation throughexecand completed the same task correctly. This is a simulated denial followed by real model/tool execution, not an observed end-to-end Cursor-native rejection.cursor/claude-opus-5-5andcursor/claude-sonnet-5-5remain unverified live: requests failed before any tool invocation withresponse.failed, coderate_limit_exceeded, andCursor Connect error resource limit exceeded. These failures neither validate nor invalidate the redirect fix for those models.Evidence boundary: the source regression and Grok probes validate the corrected guidance and its use by Grok. They do not establish that Opus/Sonnet recover from the original rejection in a real Codex App session; that acceptance check remains outstanding. Temporary test servers were stopped and temporary credentials removed after validation.
Upstream CI and required review approvals remain separate merge gates; the local/live evidence above is not a claim that those gates have passed.
Checklist
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
Required local validation passed; commands, results, and any full-suite exception are documented.
I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit