teams.json cannot pin a per-agent model; the documented claude:opus syntax is dead - #1845
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| await relay.spawn({ | ||
| name: agent.name, | ||
| cli: agent.cli, | ||
| ...(agent.model ? { model: agent.model } : {}), |
There was a problem hiding this comment.
🟡 Inline overrides retain the wrong model
When cli overrides agent.model, spec.model retains the pin while the harness runs the inline model. Worker metadata and model-based prompt selection then use the wrong model.
Learn more
The auto-spawn request now carries both the configured pin and inline CLI arguments. The broker detects an inline --model or -m in resolve_model_flag_for_cli and returns no injected model flag. However, the original pin remains in spec.model. That field is later exposed by worker listings and spawn events, used for telemetry attribution, and passed to relay_skill_prefix. The actual inline model and recorded model therefore diverge.
Example: With { "cli": "claude --model haiku", "model": "opus" }, Claude runs Haiku. Relay reports Opus and omits the small-model relay prefix because it evaluates the stale Opus pin.
Recommended fix: Parse the effective inline model in the broker and replace spec.model with it when an override is present. Keep suppressing the separately injected flag so the CLI receives only one model selection.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Relayflow: the adversarial review did not pass. This branch is not approved: the flow stopped here and did not mark it ready to merge. Review: PR #1845Reviewed FindingP2 — Resolve inline overrides before selecting prompts and recording the modelLocation: The new forwarding path sends both the model pin and the inline command to the broker. For an agent configured as
Resolve the effective model from inline CLI arguments and explicit spawn arguments before prompt selection, and preserve that effective value in the worker specification. Continue suppressing duplicate model-flag injection. Updating metadata only inside worker startup is too late to repair prompt selection. Add regression coverage for conflicting pins and inline This independently confirms Devin's inline review comment. No fix is included in this review pass; Validation and comments
|
`resolve_model_flag_for_cli` returns None when argv already carries a model
override, so no second flag is injected. Its callers only wrote `spec.model`
when a flag came back, so an agent configured as
`{"cli": "claude --model haiku", "model": "opus"}` ran Haiku while the spec,
worker listings, spawn events and telemetry all said Opus.
The pin also drives `relay_skill_prefix`, chosen in the HTTP spawn path
before the worker starts, so the wrong model there omits the small-model
relay guidance (or adds it to a large model). Repairing the metadata inside
worker startup would be after that decision, so the effective model is now
resolved before it.
`model_override_from_args` reads `--model value`, `--model=value`,
`-m value` and `-m=value`, last occurrence winning as argv does. A bare
`--model` names nothing and yields None while still suppressing injection,
so the harness is never handed two model flags.
Addresses the review finding on PR #1845.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…are-garden-3027e1e7 # Conflicts: # CHANGELOG.md
|
Fixed in 78e219c, and rebased onto main. The finding was right, including the part about ordering. And repairing it inside worker startup would indeed have been too late:
On the note that the existing Rust test passes despite the mismatch — correct, it only asserted suppression. The new tests assert the effective value, and each one is mutation-proved. Making the extractor blind to the space-separated form fails exactly one test, and the failure names the form: Validation, with cargo available this time:
The merge from main conflicted only in |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7c74ae4. Configure here.
| ); | ||
| spec.model = Some(inline.to_string()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Relaycast skips inline model reconciliation
Medium Severity
Inline --model/-m is reconciled onto spec.model only on the HTTP spawn path before relay_skill_prefix. The Relaycast path still prefixes from the pin, so a superseded pin can inject or omit small-model guidance the running harness never sees.
Reviewed by Cursor Bugbot for commit 7c74ae4. Configure here.
![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: 7c74ae4021
ℹ️ 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".
| if let Some(inline) = crate::worker::model_override_from_args(&{ | ||
| let command = spec.cli.as_deref().unwrap_or(&cli); | ||
| let mut tokens = shlex::split(command).unwrap_or_default(); | ||
| tokens.extend(spec.args.iter().cloned()); |
There was a problem hiding this comment.
Apply this effective-model resolution to the fleet/Relaycast action.invoke path as well. spawn_worker_from_request in runtime/relaycast_events.rs calls relay_skill_prefix using the pinned spec.model before WorkerRegistry::spawn; the correction at worker.rs:1110 therefore happens too late. For example, a Relaycast spawn with cli: "claude --model haiku" and model: "opus" runs Haiku but omits the small-model guidance because the prefix was selected as Opus, undermining the reliability guarantee documented for that guidance.
Useful? React with 👍 / 👎.
2 participants
Per-agent model pins in teams.json
Agents configured with
{"name":"Worker","cli":"claude","model":"opus"}now retain their model pin through config loading andupauto-spawn (includingup --spawn). The config schema and core spawn API expose the optional field. Model names are trimmed; invalid or empty values warn and are omitted without dropping the agent. Normalization preserves role/task and excludes unknown agent keys.Harness-specific handling stays in the existing broker path, which already carries model on the wire and emits model arguments for Claude, Codex, and OpenCode. No second TypeScript translation layer or catalog validation is introduced. Explicit inline
--model/-mcontinues to win. The CLI doc-comment now recommends a separate model field and explains the inline-argument escape hatch and its node-capacity limitation.The obsolete public
mapModelToCliandgetBaseClihelpers are deprecated for removal next major. Their root and subpath exports remain intact: the reviewed plan reserved breaking removal for a release-level decision, so this change takes the compatibility-preserving option and raises the pending changelog to Minor.model-commands.ts, broker colon-syntax branches, and node-capacity normalization remain follow-ups. No workflow files or production Rust code changed.Validation:
npm test -w @agent-relay/config: 103 passed, including loader normalization and schema round-trip/description regressions.npm run build -w @agent-relay/config: passed.npm test -w @agent-relay/utils: 235 passed.core.test.ts,fleet-sidecar.test.ts,client-factory.test.ts): 178 passed, including model forwarding for Claude, Codex, OpenCode, and inline arguments. The existing exact no-model spawn assertion remains green.npx vitest run packages/cli/src): 1,885 passed, 17 skipped, 2 failed. Both failures reproduce on unmodifiedb274b7ain an isolated worktree:sdk-client.test.tsexpects no gateway URL but receiveshttps://cast.agentrelay.com;fleet-lifecycle-integration.test.tsexits from local-agent routing.cargo test -p agent-relay-broker modelcould not run because Cargo/Rust is not installed; broker model and ordered-PTY-argument tests remain unverified here.git diff --check: passed.Checks
Relayflow ran this repository's checks (.relayflow/check.sh) and they passed.
What ran (.relayflow/check.sh)
Fixes #1381
Note
Medium Risk
Changes broker spawn model resolution and teams auto-spawn wiring; incorrect precedence could misreport models or inject duplicate flags, though tests cover inline override behavior.
Overview
Adds an optional
modelfield onteams.jsonagents soup --spawnforwards a model pin to the broker (schema, loader trim/validation, CLI spawn types). Inline **--model/-minclistill wins; configs can keep both for broker precedence.The broker now records the argv-effective model when spawn args already carry
--model/-m:model_override_from_argsruns on the HTTP spawn path before relay-skill selection, and worker startup updatesspec.modelwhen injection is suppressed so listings, events, and telemetry match what the harness runs.mapModelToCliandgetBaseCliin@agent-relay/utilsare deprecated (next major); use separatecliandmodelinstead of colon-suffixed CLI strings.Reviewed by Cursor Bugbot for commit 7c74ae4. Bugbot is set up for automated code reviews on this repo. Configure here.