Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 6 minutes and 1 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughBoth fixes: (1) explicitly close spawned processes' stdin in command execution functions to avoid hangs; (2) update Codex default model and available model list to include gpt-5.4 variants and a new spark entry. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/utils/command.ts (1)
62-62: Please add a regression test for the stdin-EOF hang path.This bug is easy to reintroduce; a focused test that verifies
executeCommand*completes when no stdin is provided would lock in the fix.Also applies to: 167-167
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/command.ts` at line 62, Add a regression test that verifies the stdin-EOF hang is fixed by exercising the executeCommand* entry points (e.g., executeCommand and any related exported variants) with no stdin supplied and asserting the promise completes (resolves) rather than hanging; simulate a child process that would otherwise wait for stdin, call the function without writing to stdin, and use a short test timeout or explicit completion assertion to fail if it blocks — this ensures child.stdin.end() behavior is exercised and prevents future regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/utils/command.ts`:
- Line 62: Add a regression test that verifies the stdin-EOF hang is fixed by
exercising the executeCommand* entry points (e.g., executeCommand and any
related exported variants) with no stdin supplied and asserting the promise
completes (resolves) rather than hanging; simulate a child process that would
otherwise wait for stdin, call the function without writing to stdin, and use a
short test timeout or explicit completion assertion to fail if it blocks — this
ensures child.stdin.end() behavior is exercised and prevents future regressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b081c752-4792-4b18-b914-a979660c7776
📒 Files selected for processing (1)
src/utils/command.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/types.ts`:
- Line 16: Tests failing because they hardcode the old model string; import and
use the exported DEFAULT_CODEX_MODEL constant in the test instead of expecting
the literal 'gpt-5.3-codex' so future changes won’t break; in
default-model.test.ts add an import for DEFAULT_CODEX_MODEL from src/types and
replace the assertions that compare to the hardcoded model (the two assertion
blocks around the previously failing snippets) to assert against
DEFAULT_CODEX_MODEL.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
|
|
||
| // Codex model constants | ||
| export const DEFAULT_CODEX_MODEL = 'gpt-5.3-codex' as const; | ||
| export const DEFAULT_CODEX_MODEL = 'gpt-5.4' as const; |
There was a problem hiding this comment.
Update default-model tests for the Line 16 constant change.
Changing DEFAULT_CODEX_MODEL to gpt-5.4 will break current hardcoded expectations in src/__tests__/default-model.test.ts (snippets at lines 43-63 and 101-124 still expect gpt-5.3-codex). Please update those assertions, ideally by referencing DEFAULT_CODEX_MODEL in tests to prevent future drift.
Suggested test-side adjustment
- 'gpt-5.3-codex'
+ DEFAULT_CODEX_MODEL🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/types.ts` at line 16, Tests failing because they hardcode the old model
string; import and use the exported DEFAULT_CODEX_MODEL constant in the test
instead of expecting the literal 'gpt-5.3-codex' so future changes won’t break;
in default-model.test.ts add an import for DEFAULT_CODEX_MODEL from src/types
and replace the assertions that compare to the hardcoded model (the two
assertion blocks around the previously failing snippets) to assert against
DEFAULT_CODEX_MODEL.
Signed-off-by: mirusu400 <mirusu400@naver.com>
Signed-off-by: mirusu400 <mirusu400@naver.com>
1 participant
Summary
When codex-mcp-server is used as an MCP server inside Claude Code (or any MCP client), the codex tool hangs indefinitely. The codex exec subprocess never completes because it waits for stdin EOF that never arrives. So I fix it
Changes
src/utils/command.tsTesting
Related Issues
Fixes #153
Summary by CodeRabbit
Bug Fixes
New Features