fix(opencode): reject unknown prompt variants instead of recording them - #51918
Open
monody0007 wants to merge 4 commits into
Open
monody0007 wants to merge 4 commits into
monody0007 wants to merge 4 commits into
Conversation
An explicit variant that the model does not declare was dropped when the
request was built, but still written to the user message and the session
model, so `opencode run --variant typo` exited 0 at the default effort
while the session claimed the typo. Validate explicit variants in
createUserMessage before anything is persisted ("default" stays a
passthrough sentinel).
Two internal callers relied on the silent drop and are fixed with it:
- commands that pin their own model no longer forward the caller's
variant when the pinned model does not offer it
- background task results follow the parent session's current model
selection instead of the spawn-time variant
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Follow-up to the unknown-variant rejection:
- membership uses the model's own declared keys, so "constructor",
"toString", "__proto__" and an explicit empty string are rejected
instead of passing a truthiness check ("default" is still the sentinel)
- a command that pins another model only drops a caller variant that the
caller's model actually declares; anything else is rejected
- the rejection is a typed SessionPrompt.VariantNotFoundError that the
prompt and command routes return as a 400 BadRequest with the message,
so `opencode run` shows the valid variants instead of a generic 500
- background task results carry the parent's current model together with
its variant, so an agent pinned to another model cannot reject them
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
|
The following comment was made by an LLM, it may be inaccurate: |
prompt() runs SessionRevert.cleanup before building the user message. A prompt rejected for an unknown variant therefore still deleted the undone messages and cleared the revert marker, and nothing could restore them. Resolve the agent, model and variant once, before cleanup, and create the user message from that same selection, so a rejected prompt leaves the session untouched and an accepted one is recorded with exactly what was checked. A session without its own model (a fork, for example) takes its model from the newest user message. Before cleanup that could be an undone message, so the check and the recorded message could see different models. currentModel now skips the messages a pending revert will drop, using the same cutoff as SessionRevert.cleanup, so every caller resolves the model the session is left on.
A command without its own model resolves the caller's model before the prompt drops undone messages. Check that it rejects a variant the model left after the undo does not declare, keeps the undone messages and the revert marker, and otherwise runs on that model with the variant.
This branch has not been deployed
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.
Issue for this PR
Closes #40182
Type of change
What does this PR do?
opencode run --variant <typo>currently succeeds without applying the variant, then records the unapplied value. This checks explicit variants before persistence and returns HTTP 400 with the model's valid values. Thedefaultsentinel remains supported; prototype names and an empty explicit value are rejected.The same check applies to every caller that passes a variant, so
session.commandand the GitHub action (VARIANTinput) now fail on an unknown variant instead of silently running without it.A prompt's agent, model and variant are now resolved once, before a pending undo is cleaned up, and the user message is created from that same selection. A rejected prompt therefore keeps the undone messages and the revert marker instead of deleting them. For a session without its own model (a fork, for example), the model fallback skips the messages the pending undo will drop, so the check sees the same model the message is recorded with.
Two compatibility paths are covered: pinned-model commands discard only a valid variant inherited from a different model, and background results forward the parent's current model together with its variant.
This follows the closed #40250 by @C0d3N1nja97342. Related #46106 changes the fallback for prompts without an explicit model; this patch supplies the model explicitly.
How did you verify your code works?
promptandcommand: with an explicit model, and in a fork without a session model where only the undone message is on a model that declares the variant. An unknown variant leaves the undone messages and the revert marker intact and restorable. Controls check that a valid prompt or command still replaces them and runs on the model left after the undo.bun test test/session(428 pass, 7 skip, 1 todo),bun test test/tool/task.test.ts test/session/revert-compact.test.ts(31 pass) andbun typecheckpass. On the previous revision, whose production code is unchanged here:test/tool(342 pass),test/acp(139 pass),test/clirun serially (372 pass, 5 skip) and the prompt/session server tests.run-processsubprocess cases intest/clitime out; all pass when run serially.Implementation and independent review used AI assistance; these checks ran locally.
Screenshots / recordings
N/A, no UI change.
Checklist