fix: make spawn model and effort handling honest - #351
Conversation
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_3162dfae-e736-49fe-9e5f-aac41295f4dc) |
📝 WalkthroughWalkthroughSpawn requests now support optional Codex effort values. Model and effort policies validate requests before spawning. Validated effort values reach Codex launch commands, while unsupported models and efforts are rejected. ChangesSpawn policy and launch flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant spawn_agent
participant model-policy
participant agent-engine
participant CLI
Client->>spawn_agent: Request model and optional effort
spawn_agent->>model-policy: Validate model and effort
model-policy-->>spawn_agent: Return validated policy
spawn_agent->>agent-engine: Start agent with effort
agent-engine->>CLI: Launch Codex with effort flag
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![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: a1646e83a4
ℹ️ 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".
| throw new Error( | ||
| `Unsupported model "${requestedModel}" for cli "${cli}": without a valid alias, the launcher would actually run "${defaultModel}". Accepted models: ${acceptedModels.join(", ")}. No agent was spawned.`, | ||
| ); |
There was a problem hiding this comment.
When spawn_in_workspace contains an unsupported model, this new exception is reached only from engine.spawnAgent() after src/server.ts has created the workspace, and if the invalid entry follows a valid one, earlier agents have already been spawned. The catch path reports the failure but does not roll those mutations back, so the error's claim that no agent was spawned is false and the nominally atomic tool leaves a partial team; new_worktree_split similarly prepares a worktree before reaching this validation. Validate every requested model at each tool boundary before creating workspaces/worktrees or starting the spawn loop.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5c38c8f. new_worktree_split now validates the requested model before worktree preparation, and spawn_in_workspace validates every normalized agent model before workspace/surface mutation. Added regressions proving no worktree command, workspace creation, or split occurs on invalid input. Exact-head verification: 2,381/2,381 full tests, 63/63 pre-PR checks, build and typecheck green.
![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/model-policy.ts`:
- Around line 231-251: The rejected-model path in the model policy flow must not
include override-only values in its accepted-model list. Update
acceptedModelNames or the logic around resolveLaunchModelFlag in the validation
block so the error lists only aliases passable without MODEL_OVERRIDE_ENV,
excluding values such as gpt-5.5-xhigh and sonnet while preserving valid
launcher-supported aliases.
In `@tests/server-agent-tools.test.ts`:
- Around line 670-709: Update the “spawn_agent rejects an unsupported effort”
test to validate the registered input schema via spawn.inputSchema.parse, then
assert the generic invalid-enum error for “max” without invoking the handler;
alternatively, use a reachable non-Codex handler case such as Claude or Gemini
with medium effort. Keep the assertion that no “new-split” command is created
only if the handler is actually invoked.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ef37364a-9df3-4b46-8fdb-a595395c2279
📒 Files selected for processing (5)
src/agent-engine.tssrc/model-policy.tssrc/server.tstests/agent-engine.test.tstests/server-agent-tools.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: test
- GitHub Check: Macroscope - Correctness Check
🧰 Additional context used
📓 Path-based instructions (5)
src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.ts: Build the project with TypeScript (tsc) and keep source code compatible with Node 20+ and Zod-based typing.
Use theok(data)anderr(error)helpers for consistent MCP tool responses.
All MCP tool handlers must return{ content: TextContent[], structuredContent?, isError? }.
Files:
src/model-policy.tssrc/server.tssrc/agent-engine.ts
tests/**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.test.ts: Mirror source layout in tests (src/foo.ts->tests/foo.test.ts).
Do not add integration tests that require a running cmux instance; tests should be fully mocked.
Files:
tests/agent-engine.test.tstests/server-agent-tools.test.ts
tests/**/*agent-engine*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Agent engine tests should use 1-second timeouts for state-change detection.
Files:
tests/agent-engine.test.ts
src/server.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Register all MCP tools in
server.ts, including the 33 tool handlers, and conditionally skip agent-lifecycle tools whenskipAgentLifecycle: true.
Files:
src/server.ts
src/agent-engine.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Implement agent lifecycle behavior in
agent-engine.ts, including spawning, monitoring, and quality tracking.
Files:
src/agent-engine.ts
🧠 Learnings (1)
📚 Learning: 2026-03-15T10:42:35.917Z
Learnt from: EtanHey
Repo: EtanHey/cmuxlayer PR: 1
File: tests/quality-tracking.test.ts:171-200
Timestamp: 2026-03-15T10:42:35.917Z
Learning: In tests/quality-tracking.test.ts for the cmuxlayer project, ensure that at or above 80% context quality degradation, behavior depends on depth: depth-0 agents receive a /compact command; depth > 0 agents are killed and logged (kill + log). Respawn of non-root agents is out of scope for v1. Treat the design doc quality tracking section as the authoritative source for this behavior, and align test expectations accordingly.
Applied to files:
tests/agent-engine.test.tstests/server-agent-tools.test.ts
🔇 Additional comments (7)
src/model-policy.ts (2)
4-10: LGTM!
139-164: LGTM!src/agent-engine.ts (2)
98-100: LGTM!Also applies to: 151-151, 738-743, 757-765, 4768-4768
4551-4558: 🗄️ Data Integrity & IntegrationNo change needed. The original launch command is captured in
originalLaunchCommandsBySurfacebefore boot-prompt delivery and passed into the post-update relaunch fallback, so the resolved Codex effort cannot be lost there.AgentRecordandSpawnAgentResultdo not reserve aneffortfield.src/server.ts (1)
26-30: LGTM!Also applies to: 8589-8594, 8700-8701, 8803-8803
tests/agent-engine.test.ts (1)
4238-4280: LGTM!Also applies to: 8536-8537, 10301-10308
tests/server-agent-tools.test.ts (1)
549-549: LGTM!Also applies to: 563-563, 596-596, 711-734
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_f691847f-e15e-4ed0-aa9b-42c3486ec2f8) |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_ba9c2b6a-e85c-4e65-8dcf-07ec29db9ddd) |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_16789ec5-fff9-4abc-828c-92c068493b84) |
8839906 to
6a24336
Compare
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_795357ea-7c5a-4dd5-b782-b71e8e257b54) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a243369c0
ℹ️ 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".
| launcherModel === null | ||
| ) { | ||
| const acceptedModels = acceptedModelNames(cli, overrideAllowed); | ||
| throw new Error( | ||
| `Unsupported model "${requestedModel}" for cli "${cli}": without a valid alias, the launcher would actually run "${defaultModel}". Accepted models: ${acceptedModels.join(", ")}. No agent was spawned.`, |
There was a problem hiding this comment.
opus spawn alias
When REPOGOLEM_ALLOW_MODEL is unset, callers that explicitly request Claude model: "opus" now reach this branch and fail, even though opus was accepted before this commit and the deployed Pages demo still presents successful spawn_agent(..., model="opus") calls in landing/index.html:1074 and landing/index.html:1538. This is especially disruptive for the legacy aggregate tools whose nested model remains required; normalize opus to the default Claude launch with no model flag rather than rejecting an established tool input.
AGENTS.md reference: AGENTS.md:L44-L47
Useful? React with 👍 / 👎.
1 participant
Summary
spawn_agent.effortand passmedium|high|xhigh|ultrato repoGolem as-Emaxfails loudly because the current launcher rejects itclaude-opus-5[1m]) and its effective-model responseWhy
spawn_agenthad no reasoning-effort path even though repoGolem supports one. Its model policy also reported arbitrary model strings as effective while the command builder silently dropped unknown aliases.Verification
CMUXLAYER_FORCE_INPROCESS=1): schema exposed the four-value effort enum;maxandfable-5returned errors without creating panes; a boundedeffort:mediumspawn returnedok:trueand its probe surface was force-stopped and verified absentRuntime note
The Codex footer rendered its effort label as
default, so that footer is not claimed as evidence ofmedium. Pass-through is verified at the emitted launcher-command boundary; repoGolem's parser maps-E mediumtomodel_reasoning_effort="medium".Release
No release or reconnect sweep was run. Installed remains
0.4.19; deployment is lead-owned per the brief.Note
Medium Risk
Stricter spawn validation changes behavior for callers that relied on silent model coercion or unlisted aliases; launch-command shape for Claude sonnet also changed.
Overview
Spawn paths now fail fast on bad
model/effortbefore workspaces, worktrees, or panes are created, and the emitted launcher command matches what policy claims.Model policy rejects explicit unknown aliases (non-Cursor CLIs) with an error that names the requested model, the default the launcher would run, and the accepted set—instead of reporting a bogus effective model or silently coercing. Claude
sonnetis passed to repoGolem as-Srather than-m sonnet; gated Claude aliases no longer get a launcher flag without override.Codex effort: optional
effortonspawn_agent(medium|high|xhigh|ultra) is validated byresolveSpawnEffortand forwarded as-E <effort>on the Codex launch command. Effort on non-Codex CLIs is rejected.Server tools (
spawn_agent,new_worktree_split,spawn_in_workspace) call model/effort resolution up front so invalid spawns never mutate cmux state.Reviewed by Cursor Bugbot for commit 6a24336. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Note
Fix spawn model validation and add effort flag support for Codex CLI
resolveSpawnModelPolicyin model-policy.ts now throws with a list of accepted models when an unsupported model alias is explicitly requested, instead of silently coercing to a default.-Sinstead of-m sonnetwhen the model alias issonnet; other Claude aliases are suppressed when model override is not permitted.SpawnAgentParamsand the MCPspawn_agentschema in server.ts accept an optionaleffortfield (validated viaresolveSpawnEffort); valid values are appended as-E <effort>to the Codex launch command.haiku,opusfor Claude without override) now throw errors.Macroscope summarized 6a24336.