fix(recipes): let Codex model args override defaults#8773
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughCodex argument planning now recognizes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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 |
Contributor
Author
|
Final verification is green:
|
andrewyatesai
pushed a commit
to andrewyatesai/orc
that referenced
this pull request
Jul 15, 2026
…erm fork Bidirectional alignment with upstream public Orca, reconciling every conflict against the fork's Rust-superseded surface (aterm engine, orca-* crates, the Rust-backed daemon + orchestration store) so upstream's genuinely-new behavior lands without reintroducing xterm or the Node daemon on the Unix path. Rust-superseded reconciliations (kept fork structure, grafted upstream behavior): - stablyai#8773 Codex --model/-m recipe override ported to Rust orca-agents (commit_message_plan): apply_recipe_option_override for codex, +2 tests. - stablyai#8452/stablyai#8514 getStaleDispatches: TS delegates to the Rust orchestration store; the stale-dispatch tests drive the store's set_dispatch_timestamps seam instead of a better-sqlite3 handle the fork no longer exposes. - stablyai#7986 git-credential-guard HOST predicate corrected in daemon-pty-adapter. Terminal exit pipeline (fork reattach-snapshot feature × upstream stablyai#8597/stablyai#8689): - Adopt upstream's hadPrimary 2-arg sidecar contract (terminal-parked-tab- watchers branches on it) + restore deliverPtyExitToHandlers (pty-exit- delivery.ts) so a throwing primary still runs every sidecar and rethrows; fold in the fork's recent-exit record for late-subscribe replay. - connect(): gate the pre-spawn exitedBeforeAttach short-circuit on buffered DATA (hasPreHandlerPtyData) so a fully-captured dead parked session skips the respawn, while a bare exit racing an in-flight reattach still spawns and the daemon snapshot is painted before the deferred exit drains. - registerPtyExitHandler returns hadBufferedExit + handles throwing pre-attach cleanup, unifying the fork's deferred-drain with upstream's exitedBeforeAttach. - stablyai#8689 daemon grace-retry loop reconciled with the fork's reconcile path. aterm facade + probes: - Replace the xterm-only isXtermInstanceDisposed probe with a facade-aware terminal-instance-disposed reading a new AtermTerminalFacade.isDisposed accessor (the facade fires write callbacks post-dispose, so the flag — not callback loss — is how restore detects a dead target). Housekeeping: - Split terminal-scroll-intent DOM event-tracking into its own module and collapse the daemon protocol-version array to stay under max-lines (no disables added). - Fix pre-existing curly lint debt in git-wasm/aterm-worker-search tests. Gates: typecheck, oxlint, full vitest (30285 passed), cargo --workspace all green; napi addon rebuilt for the stablyai#8773 Rust change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
andrewyatesai
pushed a commit
to andrewyatesai/orc
that referenced
this pull request
Jul 15, 2026
…o the Rust/aterm fork Ports the +22 upstream commits since the v1.4.142 merge base: ssh Kerberos/GSSAPI (stablyai#7507) + csh/tcsh relay wrapper (stablyai#8709) + lsof selector fix (stablyai#8849), terminal descendant-process kill on teardown (stablyai#8706) + stale Windows Codex focus-report filtering (stablyai#8842) + keep-legacy-daemon-PTYs-mounted (stablyai#8817), manual project order across paired hosts, gitea /pulls scan cache (stablyai#8858), rich-editor markdown-style preservation (stablyai#8862) + heading toggles (stablyai#8822), Windows firewall pairing repair (stablyai#8846) + mobile host-card dedupe, agent-status roster reaping (stablyai#8825), and i18n. Conflict resolutions (7): - SUPERSEDED — ssh-config-parser.ts / ssh-g-config-resolution.ts kept as Rust shims; the GSSAPI config keys are RE-DERIVED in Rust (orca-ssh config_parser + resolved_config, both orca-dispatch serializers, and the napi ssh_config_host_to_json) with first-value-wins `??=` semantics + tests. The ssh-connection transport half (GSSAPI system-ssh fallback) is a plain TS add. - pty-connection.ts — merged BOTH behaviors into one all-platform agent-done subscription: the fork's kitty-keyboard recovery (stablyai#3941, aterm panes) AND upstream's Windows Codex focus-report suppression (stablyai#8842). Kept the fork's `unsubscribeAgentDoneTerminalModeReset` (the disposal path references it); unioned the fork's 2 kitty tests with upstream's 4 focus tests. - repos.ts — kept the fork's git-wasm imports (repo icon/badge/project-groups), added upstream's new manual-repo-order helpers. - zh.json — took upstream's corrected translation, kept the fork's minimax key. - package.json — 1.4.142-fork.2 (dropped the upstream rc.7 bump). descendant-process kill (stablyai#8706) stays in TS: it is a Node-main capability (parses `ps`, `process.kill` on arbitrary PIDs to reach detached-pgid grandchildren) and is fully wired into the fork's TS teardown (session/local-pty-provider) — not an orca-pty concern. Stale-artifact sync (pre-existing, exposed by rebuilding): the committed git-wasm blob lagged the Rust source, masking the codex stablyai#8773 `--model` override — rebuilt napi + git-wasm and corrected the one commit-message-plan test that still asserted the pre-stablyai#8773 append. Gates: typecheck 0 errors; cargo --workspace 999/0; full vitest 30466/0; parity golden 1397/1397 + differential 1484/1484; parity:daemon PASS; lint clean; worker-ON curated e2e 12/12. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #8722 by making a Codex action recipe's model argument override Orca's generated model argument instead of appending a conflicting second singleton option.
Supported recipe forms:
--model gpt-5.6-luna-m gpt-5.6-luna--model=gpt-5.6-luna-mgpt-5.6-lunaThe recipe's exact model tokens replace Orca's generated model pair in place, preserving the established position of reasoning config and the relative order of all other recipe arguments.
Evidence
Before, a recipe with
--model gpt-5.6-lunaproduced the equivalent of:Codex rejects that command because
--modelis a singleton.After this change, the planner emits:
Regression coverage proves the long, short, equals, attached-short, sibling-argument, no-override, and
--terminator cases. The focused suite passes 19/19 tests; the full suite passes 29,764 tests with 43 skipped.ELI5
Orca used to put two destination labels on the same package: its default model and the recipe's model. Codex refused to guess which label was right. Orca now swaps its own label for the more-specific recipe label, leaving exactly one model while keeping the rest of the package instructions in the same order.
Trade-offs and user regressions
Expected user regressions: zero.
-mVALUE.--are positional text and do not override the generated model.Community PR decision
Compared community PR #8740 after this independent PR was opened. #8740's strongest idea was replacing the generated model in place so trailing reasoning configuration stays ordered; that behavior is incorporated here with credit to @xianjianlf2.
This PR is the selected integration path because it is based on current
main, scopes the-malias to Codex instead of assuming unrelated CLIs share that spelling, and adds coverage for equals syntax, attached short values,--termination, sibling arguments, and unchanged no-override behavior.Screenshots
No visual change.
Testing
pnpm lintpnpm typecheckpnpm test(29,764 passed, 43 skipped)pnpm exec vitest run --config config/vitest.config.ts src/shared/commit-message-plan.test.ts(19 passed)pnpm build— not run; this changes pure argv planning with no UI, bundling, native module, or dependency surfaceAI Review Report
An independent adversarial sub-agent reviewed the diff without inspecting community implementations. It checked argv parsing, singleton precedence, malformed values, exact/equals/separated/attached syntax,
--termination, argument ordering, non-Codex isolation, command injection, and macOS/Linux/Windows/WSL/SSH behavior.Findings and resolution:
-mVALUEsyntax was not initially recognized and still caused duplicate models. Fixed by recognizing attached values only for short aliases and adding a regression case verified against Codex CLI parsing.--sandbox, and for recipes with no model override.--handling, non-Codex isolation, and unchanged security/platform risk.Cross-platform conclusion: the change operates on already-tokenized argv arrays and adds no shell quoting, paths, shortcuts, labels, or Electron platform branches. It follows the existing macOS, Linux, Windows, WSL, SSH, local, and remote spawn paths.
Security Audit
No new command-execution surface is added. Recipe CLI arguments were already user-controlled, tokenized without a shell, and passed as argv. The change only replaces Orca's generated model pair with an explicitly supplied recipe model token sequence. No shell evaluation, path handling, auth, secrets, dependency, IPC, network, or permission behavior changes.
Notes
The override is intentionally scoped to Codex's generated model option to fix the reported failure without changing precedence semantics for unrelated CLIs or singleton flags.