Repository navigation
feat(orchestration): support pi and devin worker launch models - #27012
Gabrielgvl wants to merge 3 commits into
Conversation
worker-start --model/--effort rejected pi and devin because neither had a session-option catalog. Add both, following the OMP pattern: - pi maps effort to --thinking (off|minimal|low|medium|high|xhigh|max) and discovers models through `pi --list-models`. - devin is model-only -- effort is part of the model id and it has no effort flag, so --effort is refused. Discovery reads `devin models list --format json`. - Both strip a configured --model/--thinking before injecting the launch flags, so the flag is never sent twice. The pi table parser moves into pi-model-list-probe.ts, shared by commit-message discovery, the RPC catalog probe, and the new catalog; it also gains the minimal and max levels pi's flag domain lists.
…spec - reuse iterateProcessOutputLines/getProcessOutputFields for the pi table and commit-message parsers instead of a third private copy - extract the noisy-stdout JSON object extraction both omp and devin model listings need into model-list-json-output.ts - pin the devin MODEL_DISCOVERY_ONLY_SPECS entry with an owner-boundary test like grok's, and align the pi thinking default with the parser's 'low'
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (23)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe change adds dynamic model discovery and launch-time model selection for Pi and Devin. Pi effort preferences map to its Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to This change adds model selection for pi and devin workers, plus thinking-level selection for pi. Nothing in the supplied material points to a merge-blocking problem. The author-reported failures in host-integration tests are said to be in unrelated paths. Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 47.06% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 17 functions across 20 files. (2 skipped:… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly and concisely describes the main change: adding worker launch model support for Pi and Devin. |
| Description check | |
The description includes all required sections, links issue #26971, explains the change and rationale, documents testing and known full-suite failures, and completes the checklist. |
| Linked Issues check | |
Direct issue #26971 has coding requirements for launch-scoped model selection and receipt values for pi and devin. PI_SESSION_OPTION_CATALOG enables worker preferences, maps model to --model… |
| Out of Scope Changes check | |
The changed files stay connected to #26971. Catalog code, probe parsers, argument-plumbing updates, launch and probe tests, receipt tests, help text, orchestration documentation, and bundled skill-gui… |
Full details: Docstring Coverage
Explanation
Docstring coverage is 47.06% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 17 functions across 20 files. (2 skipped: 2 unsupported.)
- Fix all pre-merge checks with AI
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
This branch has not been deployed
1 participant
ELI5
Orca coordinators can already pick a model (and effort) when they launch some worker agents. This PR makes that work for the
pianddevinCLIs too:worker-start --agent pi --model <provider/id> --effort <level>startspi --model <id> --thinking <level>, andworker-start --agent devin --model <id>startsdevin --model <id>— per worker launch, without touching machine-wide config files that concurrent coordinators could race on.What Changed
--model <provider/id>; Orca--effortmaps to pi's--thinkingflag using pi's own level domain (off|minimal|low|medium|high|xhigh|max). Model discovery comes frompi --list-models(text table parsed intoprovider/modelids, reasoning-capable rows marked). Configuredpi --model/--thinkingargs are replaced rather than duplicated.devin --model <id>. Devin encodes effort in the model id (e.g.swe-2-medium) and has no effort flag, so--effortis refused up front, as it is for OMP. Model discovery comes fromdevin models list --format json(families → variants with labels, descriptions, context windows). Configureddevin --modelargs are replaced; permission/trust args are preserved.launch.effectivereports the requested model/effort for both agents — the same admission-record semantics the seven previously opted agents already have.agentArgTerminatorIndex/VALUE_OPTIONSso overrides insert before--and value tokens are never mistaken for flags.parseJsonObjectintomodel-list-json-output.ts(shared by OMP and devin probes) and reusedprocess-output-field-scannerline iteration in the pi and commit-message parsers instead of a third copy.--model/--efforthelp, orchestration docs, and bundled skill guides list the newly supported agents.Why
Issue #26971 asks for launch-scoped model selection for pi and devin workers. The session-option catalog +
launchPreferencespath is the established mechanism (OMP #24796, OpenCode #24624/#24768), so this extends it rather than adding per-agent flag handling or config mutation. This supersedes the earlier pi attempt #17704 by @kukapu (Albert Alvarez Estelles) — same pi-side mechanism, rebased onto the current catalog architecture and paired with devin support.Linked Issue
Fixes #26971
Visual Proof
N/A— CLI/RPC-only change; no UI surface is altered.Testing
Platform tested: Linux (this host). Discovery commands verified against the real installed CLIs:
pi 1.1.0(--list-models,--thinkingdomain) anddevin 3000.11.3(models list --format jsonshape). New focused tests cover: both catalogs, launch args for pi model+thinking and devin model, devin--effortrefusal, configured-flag replacement/dedup, terminator insertion, receipt values, and probe-spec ownership. Commands run:pnpm test(full suite, repo-pinned Bun 1.4.2): 117,135 pass / 56 fail across 20 files. Every failure is a host-integration test needing resources this machine lacks or forbids touching — real fish/zsh binaries, PTY process groups, cross-version relay fixtures, missing agent CLIs, live Electron windows. None touch the changed code paths; all catalog/launch/receipt/discovery tests pass.AI Disclosure
Author: GitHub @Gabrielgvl · X @kvlao. Implemented by Devin (SWE-2 Max) as a dispatched Orca worker; independently reviewed by pi-review (cross-model, 3 rounds).
Review
AI review summary (pi-review, adversarial finders + refuters):
runProcessplumbing;--terminator handling is covered by tests.--extra-agent-argsallowlist are untouched.One known semantic note kept open by reviewers: for structured (native-chat) pi workers, launch preferences are seeded into the session exactly like a saved chat selection — if pi's session layer rejects a value, the worker runs with the configured model while the receipt still records the request. That is the existing contract shared by every opted structured agent; making
launch.effectiveprovider-verified would be a generic settlement change spanning all of them and is deliberately out of scope here.Agent skill upstream boundary
Notes
No issues expected in: security (argv-only flag composition), cross-platform (Linux verified; pure argv/JSON logic elsewhere), Remote SSH (existing probe plumbing), Mobile (untouched), backwards compatibility (additive opt-in;
--effort/--modelflags previously rejected for these agents), performance (on-demand probes only).Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)