feat(opencode): add dispatch controls to the task tool - #34947
Conversation
|
The following comment was made by an LLM, it may be inaccurate: Based on the search results, here are related/superseded PRs that are NOT duplicates (as noted in the PR description): Superseded PRs:
Related/Prior Work:
The current PR (#34947) explicitly supersedes #29447 and #32122, and covers the issues mentioned. No actual duplicate open PRs found that would represent the same work. |
8bedb5f to
9c9aefc
Compare
5edeef5 to
101f675
Compare
|
Hi — one more concrete use case and a question about the review pipeline, from someone who keeps hitting this exact wall. Use caseI'm running opencode (0.0.0-next-17028) as an orchestrator on a real project: I delegate to a backend agent and a frontend agent (mechanical work — should run on a fast/cheap model) and a review agent (needs a stronger model) in the same session. Today the only way to approximate that is duplicating agent definitions per model ( The questionI dug through the related issues to understand the status before commenting:
From the outside this reads as: the request is real (this thread alone has dozens of use cases), two implementations exist, and neither has gotten a single maintainer comment in 6+ months. I don't want to assume anything — is this a triage/review backlog issue, is the feature out of scope for now, or is the direction (tier vs explicit model param vs dispatch) still being debated? What would help get this reviewed: an approved design, a smaller diff, someone to champion the PR? Happy to help however is useful — I'd genuinely rather contribute than duplicate another request in a fourth issue. |
|
@ArthurFranckPat Hi! As I understand it, it is Facebook's rules: The more likes, the more likely it is to go in? You could try a relentless social media campaign on discord, but your millage may vary. |
Per-dispatch model override (permission-gated), resume that keeps model and variant, slug task_ids, per-dispatch variant, opaque metadata, an explicit resume consent gate, and timeout with fallback_model.
|
Added The child kept calling the model and running tools after the parent had already received its error frame, and no tool could stop it —
The child's loop is forked into the per-instance scope in The fix moves the existing A comment I added in the original timeout work was wrong and is replaced: -// No ops.cancel here: Effect.timeout already interrupted the ops.prompt fiber,
+// Timeout interrupts the await, not the child runner; cancelRun stops that
+// runner without canceling the enclosing background job.
Verification: red-first, and the mutation check removes only the new call. Both the new regression test and the pre-existing fallback test go red, so the single call now covers both paths. The prior One behaviour worth stating precisely, since the placement suggests otherwise: the new call does not execute on parent abort. The outer fiber is being interrupted at that point, so the |
|
Added Found by probing the fix rather than reading it. Live on a binary that already carried The primary-only path on the same binary stops at 3/20 ticks, so the earlier fix works where it applies; this was the hole beside it. The fallback attempt is now captured with Two regression tests: the fallback-failure case, and a guard that a successful dispatch does not cancel the runner. That second one is the real risk of broadening a cancel, so it is pinned rather than assumed. Both mutation-checked independently — reverting only the fallback arm reddens only the fallback test, and mutating the success path to cancel unconditionally reddens only the success guard. Worth noting for anyone reviewing the pair: the first fix was reviewed and approved with real probes, and the gap still shipped, because the review brief scoped it to "terminal failure" and neither of us enumerated the function's exits. The discriminator was a two-minute heartbeat probe in both cases. |
Issue for this PR
Closes #17595. Also covers #6651, #26925, #29984 (model param for subagents) and #24757 (variant lost on task dispatch). Supersedes #29447 and #32122.
Type of change
What does this PR do?
Adds per-dispatch controls to the Task tool. Seven changes, one surface:
modelparam (provider/model-id) — run one dispatch on a different model than the agent's default. Gated behind a newmodel_overridepermission that defaults to deny, so an agent can't silently move work to an expensive model; you allow patterns per provider in config ("model_override": { "anthropic/*": "allow" }). The permission check runs even on thebypassAgentCheckpath — a task spawned by another task can't skip the ask.task_idpreviously reverted the child to the agent default mid-conversation. The child session now records its last-used model/variant and resumes on it; an explicitmodelparam still wins.task_ids.task_id: "explore-auth"creates a named child whose session id derives from sha256(slug + root session id), so the same slug from the same session tree resumes the same child. The slug also becomes the child session's display slug. Slugs are parent-scoped: using another session's slug errors instead of hijacking it.variantparam — per-dispatch reasoning preset ("thinking","high", …). Unknown variants are ignored, same as the existing variant resolution. deepseek-v4 models additionally get anonevariant that disables thinking ({ thinking: { type: "disabled" } }, same shape as the existing minimax-m3noneand identical to what agent-frontmatteroptions.thinkingproduces) — without it there was no per-dispatch way to run deepseek with thinking off, forcing a dedicated agent definition for what is one flag.metadataparam — opaque object stored on the child session row for plugins/queries. Shallow-merged on resume. Not shown to the subagent.resume: truegate. Reusing atask_idthat names an idle (finished) session used to silently continue it — an agent reusing a slug by accident inherits a stale context. Resuming an idle session now requiresresume: true; a live background task still accepts follow-up prompts without it. This is the one behavior change in the PR.timeout+fallback_model— bound a dispatch attempt in ms; optionally retry once on a different model when the attempt fails or times out. Fallback never fires on parent abort (interrupt) or defects, only typed failures. The retry cancels the child's prompt runner via a newSessionRunState.cancelRunrather thancancel— full cancel tears down the enclosing background job itself (job id == child session id), which killed the fallback before it could run.Why the runner-only cancel exists:
Effect.timeoutinterrupts the caller fiber, but the actual run is forked into the session runner scope and stays Running — a second prompt would wait on it forever instead of starting the fallback.cancelRuninterrupts just that runner. Both failure modes were found by probes during review, not speculation; the regression tests for them are in the diff.How did you verify your code works?
bun test test/tool/310 pass / 0 fail,test/session/366 pass / 0 fail (prompt/run-state touched),test/provider/transform.test.ts291 pass / 0 fail (deepseeknonevariant), typecheck clean onopencode,core,tui.Exit.hasInterruptswithfalsefails the interrupt-no-fallback test; the model-override permission test asserts exactly onemodel_overrideask on the bypass path.variant+ reasoning-token counts), metadata merge, 3s timeout killing a 60s task.SessionPromptstack — the tool-test harness has no way to drive a real provider timeout deterministically.Screenshots / recordings
Not a UI change.
Checklist