Skip to content

Keep OpenCode worker model selection separate from the default - #24768

Merged
nwparker merged 205 commits into
mainfrom
nwparker/opencode-worker-model-v2-focused
Oct 5, 2026
Merged

nwparker merged 205 commits into
mainfrom
nwparker/opencode-worker-model-v2-focused

Conversation

@nwparker

@nwparker nwparker commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 5 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​505 $\color{#cf222e}{\Huge{\mathbf{−}}}$​48 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​457
Prod 3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​290 $\color{#cf222e}{\Huge{\mathbf{−}}}$​1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​289

ELI5

An OpenCode worker uses its requested model, while ordinary launches keep the configured default.

What Changed

Extend #24624's validation for verified OpenCode 2.0.16. Resolve the default opencode command from the launch environment, then keep the model override in that worker's private launch configuration. Preserve the original configuration for resume and reuse existing probe and process cleanup.

Why

A private launch configuration keeps a worker's choice from affecting shared defaults. Reusing the command resolver and process ownership keeps validation and cleanup with the execution host.

Linked Issue

Part of #19980; builds on merged #24624. Preserves #22134 and credits @user141514.

Visual Proof

Actual model B, later default-model A turns, and supervised completion. The real OpenCode shell tool sent worker_done and delivered one coordinator message. Fresh and nested startup completed their Unicode prompts. This evidence is pinned to source c8fdfdbe.

The shared catalog correction also passed four actual installed-CLI checks: an unfinished 262,144-byte catalog was accepted before repair and refused after, while the complete small catalog remained accepted.

Testing

  • Reproduced the installed bare-command refusal before repair.
  • Actual installed OpenCode 2.0.16 model isolation and supervised completion verified.
  • Automated coverage for defaults, availability, resume, response cleanup, and already-closed processes.

Earlier feature qualification passed 172 focused tests with one existing skip, all three typechecks, lint, format, and quality checks; its later integration passed 61 tests and all three typechecks. Review corrections passed 102 targeted tests and all three typechecks. The final shared catalog correction passed 10 tests, Node typechecks for both PRs, and affected lint, format, and quality checks.

After the gate merged, the remaining model-selection change passed 34 focused tests with one existing skip across five files, plus affected lint, format, and quality checks. The integration preserves the reviewed model code and current main.

Actual testing used a controlled local provider on macOS. Live Windows, WSL, and SSH model selection was not exercised.

Review

Independently reviewed. Eight actual unsupported requests left terminals, workers, tasks, settings, and configuration unchanged. Recorded processes and private ports closed. Cross-platform and remote boundaries have automated coverage.

Agent skill upstream boundary

  • Not applicable.

Notes

#22134 remains open until its complete replacement is merged. Linked model-isolation screenshots retain their original source identity. A final newline catches visibly incomplete catalog output; a cut exactly at a line boundary cannot be detected this way.

Checklist

  • Focused change; behavior, mechanism, and choice explained.
  • Visual proof linked.
  • Correctness, security, performance, and cross-platform/remote behavior reviewed.
  • Relevant tests, typechecks, lint, format, and quality checks pass.
  • Public CI and final head/base review complete.

brennanb2025 and others added 30 commits September 22, 2026 14:02
Allow supervised OpenCode workers to use per-launch model overrides through the existing launch-preference and receipt path.
Preserve existing OpenCode agent arguments while replacing only model flags, and reject unsupported effort values explicitly.
Keep Native Chat option exposure unchanged and update the worker CLI and orchestration guidance.
Reuse captured composer readiness on local and paired execution hosts and revoke launching-shell paste anchors.

Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com>
…apability

Co-authored-by: user141514 <user141514@users.noreply.github.com>
@nwparker nwparker changed the title Support verified OpenCode 2 worker model selection Keep OpenCode worker model selection separate from the default Oct 4, 2026

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

Reviewed the delta since the prior pullfrog review (7c765fdef3 → 255cc87769). The PR's authored layer over its declared base (8f950320e5) is byte-identical to the code approved at 7c765fdef3; this head is a merge that carries that layer onto a newer base/main and brings in the base branch's startup-prompt location binding.

  • Authored model-selection layer unchanged — opencode-launch-model-config.ts, opencode-launch-model-context.ts, and opencode-model-startup-plan.ts diff to empty between 7c765fdef3 and 255cc87769, so the per-launch OPENCODE_CONFIG_CONTENT override, the authenticated loopback probe, and the version-gated startup plan are exactly what was approved before.
  • Startup-prompt readiness now tracks the composer location — the base's opencode-startup-prompt-source.ts replaces the one-shot ctx.data.location.sync(ctx.location) at setup with a loop keyed on [directory, workspaceID]: it re-syncs when the home location changes and re-checks that identity before claiming and before submitting. A missing or rejecting sync still fails closed.
  • Base/main integration — the sync also pulls in upstream OpenCode overlay GC and codex/jcode work that was reviewed elsewhere.

87 focused tests pass at this head (1 opt-in skip).

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

Reviewed the delta since the prior pullfrog review (255cc87769 → 9c6e50ab27). The new commits harden the model-catalog probe against incomplete output, refuse new-worktree OpenCode model selection with an explicit message, and align the docs.

  • Refused incomplete catalogs — probeOpenCodeModelAvailability now also requires !result.outputTruncated and a trailing newline before matching the requested model; new tests cover the truncated head, the captured unterminated shape, and complete \n/\r\n catalogs.
  • Refused new-worktree model selection up front — resolveWorkerLaunchPreferences throws capability_unsupported ("requires an existing worktree") when createsWorktree and the agent is opencode, threaded through both local and federated start validation, with tests asserting the message and that no probe or effect runs.
  • Aligned the documentation — the CLI docs, bundled skill guide, and coordinator-loop reference now state OpenCode accepts a per-launch model only in an existing worktree.

ℹ️ The catalog-completeness guard is only reached on the legacy version path

The new outputTruncated / trailing-newline gate lives in probeOpenCodeModelAvailability, which the startup plan calls only inside isVerifiedOpenCodeLegacyModelVersion (1.18.30 / 1.18.32); 2.0.16 takes the probeOpenCodeLaunchModelContext /api/model path instead. The test that pins the unterminated shape labels it as captured from OpenCode 2.0.16, so the guard's actual target is the legacy versions — worth confirming their opencode models output also always ends with a newline, since the gate is fail-closed and would reject a complete catalog that lacks one.

Technical details
# Guard scope vs. evidence source

## Affected sites
- `src/main/opencode/opencode-model-startup-plan.ts:84` — `probeOpenCodeModelAvailability` is called only when `isVerifiedOpenCodeLegacyModelVersion` (1.18.30 / 1.18.32).
- `src/main/opencode/opencode-model-startup-plan.ts:90` — version 2.0.16 instead uses `probeOpenCodeLaunchModelContext`, so it never reaches the new trailing-newline check.
- `src/main/opencode/opencode-model-availability.test.ts:77` — the unterminated shape is labelled as captured from OpenCode 2.0.16.
- `src/main/opencode/opencode-model-availability.ts:52` — `!result.outputTruncated` covers stdout OR stderr overflow, so a >1 MiB stderr stream would also refuse a complete catalog.

## Required outcome
- Confirm the legacy `opencode models` output always ends with a newline, so the fail-closed `endsWith('\n')` gate cannot reject a complete legacy catalog.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@nwparker
nwparker changed the base branch from nwparker/opencode-worker-model to main October 4, 2026 23:45
@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 16784ce9-f122-43ad-9fbd-79a034609278
📥 Commits

Reviewing files that changed from the base of the PR and between 3f4af35 and f153666.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 2ed19e2c-7b7c-47b0-b3cf-ebd27c027ecb
📥 Commits

Reviewing files that changed from the base of the PR and between 9c6e50a and 3f4af35.

📒 Files selected for processing (2)
  • src/main/opencode/opencode-model-startup-plan.test.ts
  • src/main/opencode/opencode-model-startup-plan.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

The changes add OpenCode launch-context parsing and an authenticated loopback probe. They add a configuration resolver that preserves existing fields while setting the requested model. Startup planning uses these helpers to support model preferences for OpenCode 2.0.16, while retaining the existing legacy-version path. Tests cover response validation, probe cleanup, configuration handling, and startup acceptance and refusal cases.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 3f4af

The reviewed startup path preserves the requested model for launch and the default configuration for resume. No actionable merge-blocking issue remains.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 3f4af

Per-launch choices remain separate from shared defaults, and temporary validation uses authenticated local access. The remaining risk is failure cleanup: descendant processes may outlive a refused launch after their parent exits.

Retained concerns

  • Low · reliability · inferred: The new detached preflight lifecycle has no demonstrated cleanup owner after the root exits while descendants retain inherited pipes. It refuses startup and avoids signaling the former PID, but performs no further descendant termination. Such descendants could retain execution-host and launch-environment access after refusal. This is a conditional failure-containment concern, not an observed credential compromise.
Security review details

Security Blast Radius

  • inferred — The demonstrated exposure is the local execution host, requested working directory, and selected account environment. Probe processes run without an additional sandbox shown here. A surviving descendant would retain its inherited host permissions; no cross-tenant or remotely reachable exposure was established.

Security Findings and Attack Paths

  • inferred — The conditional containment path is a probe root exiting while descendants keep inherited pipes open. Startup is refused, but cleanup stops targeting the exited root and does not recover ownership of those descendants. The mocked regression test confirms this control flow, not exploitation or real-CLI descendant survival.

Trust Boundaries and Controls

  • observed — Command resolution accepts a single non-divergent command token. Probe requests remain on a parsed loopback address, use a fresh 32-byte random authentication secret, reject redirects, and limit each response to 1 MiB. Schema, directory, agent, and model checks constrain acceptance of server responses.

Resilience and Maintainability Implications

  • inferred — Per-invocation credentials, ephemeral ports, and copied configuration separate concurrent probe transport and configuration state. Timeout, abort, response cancellation, and refusal on unverified shutdown limit ordinary failure exposure. They do not prove isolation of external CLI filesystem effects or cleanup after every descendant failure.

Hardening Proposals

  • proposed — Give temporary probe descendants a stable cleanup owner that survives root exit, using an appropriate supervisor or operating-system containment mechanism. Preserve the protection against signaling a reused PID and verify recovery with a real descendant-holding-pipes scenario.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 9.38% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 32 functions across 39 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main change: keeping an OpenCode worker’s requested model separate from the configured default.
Description check ✅ Passed The description covers the required explanation, changes, rationale, linked issues, visual proof, testing, review, boundary, notes, and checklist. It also discloses that public CI and final head/base …
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

Reviewed the delta since the prior pullfrog review (9c6e50ab27 → 3f4af3542b, 1 commit). The new head is a clean merge of current main (8e5080c132, the merged OpenCode worker model gate #24624); the PR's authored OpenCode layer is byte-identical to the version approved at 9c6e50ab27.

  • Synced current main — merged main at 8e5080c132. The merge is conflict-free (git show <merge> --cc --name-only lists no combined-diff paths), so nothing was resolved by hand.
  • Authored layer preserved — opencode-model-startup-plan.ts, opencode-launch-model-config.ts, opencode-launch-model-context.ts, and the fetch audit are unchanged between 9c6e50ab27 and the merge; the only src/main/opencode difference is main's own hook-service.ts. git diff <main> HEAD still reduces to this PR's 8 authored files.
  • Focused tests pass — 33 tests across the four model files pass at this head (1 opt-in real-CLI skip).

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

Reviewed the delta since the prior pullfrog review (3f4af3542b → f153666141, 1 commit). The new head is a clean merge of current main (d9846e64fe); the PR's authored OpenCode layer is byte-identical to the version approved at 9c6e50ab27, and all prior review threads are resolved.

  • Synced current main — merged main at d9846e64fe. The merge is conflict-free (git show <merge> --cc --name-only lists no resolved paths), and git diff d9846e64fe..f153666141 reduces exactly to this PR's 8 authored files, so the merge result is main plus the authored layer.
  • Authored layer preserved — opencode-model-startup-plan.ts, opencode-launch-model-config.ts, opencode-launch-model-context.ts, and the fetch audit are unchanged between 3f4af3542b and the merge (git diff over the 8 authored files is empty).
  • Focused tests pass — 34 tests across the five authored test files pass at this head (1 opt-in real-CLI skip), confirming the main integration did not disturb the authored layer.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants