feat(acp): let the owner switch an agent's model mid-session with !model - #3380
feat(acp): let the owner switch an agent's model mid-session with !model#3380troyhoffman-oss wants to merge 4 commits into
Conversation
… strings
`is_owner_control_command` required exact content equality, so a command
could not carry an argument and each of the three commands needed its own
inline block re-doing the same owner check. Replace it with
`owner_control_command`, which splits a kind:9 agent mention into
`(command, args)`; the three blocks collapse into one owner check plus a
match.
Arity is enforced by the arms (`("!shutdown", "")`), so "!shutdown please"
is not a shutdown, and an unrecognized command or a non-owner author still
falls through to normal prompt handling — a stranger typing "!cancel" is
still sending the agent a message. That fall-through is load-bearing now
that the parser accepts any `!token`, so it is pinned by a test rather than
left to be rediscovered.
The parser also accepts a leading mention. Mentioning is how a client
addresses an agent, so `@Name !rotate` is the natural way to type a
command, and by the time the parser runs the `p` tag has already proved the
mention is ours — the name after `@` carries no further information.
Scanning to the first whitespace-delimited `!`-token rather than skipping
one token is what makes this work on real deployments: display names hold
spaces (`@Codex (Sol)`, `@Will Pfleger`), so a single-token skip would drop
the command for most of them. Matching the name itself was the alternative
and was rejected — the harness knows its ACP adapter name, not its Nostr
profile name, so it would need a kind:0 fetch to learn a string the `p` tag
already made redundant.
The leading `@` is what arms the scan, so a bang mid-sentence still never
fires: `hello !rotate` and `hello @claude !rotate` both stay prompts. The
accepted cost is the mirror case — a message that opens with a mention and
happens to contain a bang-token later in prose now parses as a command.
That is owner-gated and rare, and it fails loudly with an unmatched arm
rather than silently swallowing the message.
Behaviour of `!shutdown`, `!cancel`, and `!rotate` is otherwise unchanged.
Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
`!model [id]` is a second front door onto the switch path kind:24200 `switch_model` already drives, not a new mechanism. It lands as a fourth arm in the control-command match. Extract `switch_model_for_channel` from `handle_switch_model_control` so both front doors share one implementation, with `SwitchOutcome` naming the outcomes the observer path renders as `control_result` statuses. Matching is on exact IDs. Adapters ship near-identical pairs (`opus[1m]` vs `claude-opus-5`, `gpt-5.3-codex` vs `gpt-5.3-codex/low`) where a prefix rule would silently pick a different context lane and price point. A miss — or no argument — renders the catalog, so the listing is also the error-recovery story. The pick is pre-validated so a typo never cancels a live turn, which requires a catalog that is readable mid-turn. It used to live on `OwnedAgent`, and with the default `--agents 1` the single agent is moved out of its slot for the duration of a turn, so a slot lookup finds nothing exactly on the busy path the guard is for. Hoist it to a process-wide `ModelCatalog` on `PromptContext`, captured task-side by `apply_session_config`: it is a property of the agent command, not of a session or a slot, so nothing is lost by sharing it. Validation then sits in one place at the top of `switch_model_for_channel`, covering both front doors equally, and `IdleSwitchResult` sheds its now-dead `UnsupportedModel` arm. The "← current" marker resolves the channel's override first, then the adapter's own reported selection via `AgentModelCapabilities::reported_current` (`currentValue`, then `currentModelId`, mirroring the precedence `models()` uses). Override alone would mark nothing on a plain `!model` — the most common call — since `desired_model` is `None` until `--model` or a switch sets one. The override half is read from the agent that actually serves the channel, so with `--agents N > 1` another slot's pick is never marked as this channel's. Two `configOptions` spellings are factored into `config_option_id` / `config_option_label`. Responses carry `id` and `name` (`SessionConfigOption`), while `configId` is request-only (`SetSessionConfigOptionRequest`) and `displayName` is pre-standardization; both spellings are accepted in one place rather than at each read site, so the catalog listing and the `models` CLI stop resolving labels to `None` on a conformant adapter. `post_failure_notice` becomes `post_notice` — it now carries owner command replies as well as dead-letter warnings. Known limits, documented rather than half-fixed: `desired_model` is per-agent-process, so with `--agents N > 1` a later turn on another slot uses the old model (pre-existing; the desktop picker has the same hole). Closes block#2543 Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f9a08b987
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| content | ||
| .char_indices() | ||
| .find(|&(i, c)| c == '!' && content[..i].ends_with(char::is_whitespace)) | ||
| .map_or("", |(i, _)| &content[i..]) |
There was a problem hiding this comment.
When an owner-authored message begins with @, this scans all remaining prose for the first whitespace-prefixed bang token. Consequently, an ordinary prompt such as @Agent explain what happens when I use !shutdown is parsed as the exact !shutdown command and terminates the harness; the same pattern can unexpectedly cancel or rotate an active turn. Limit command detection to the command position immediately following the addressed mention rather than searching the entire message.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 29e5af0. Command detection now requires the command at command position — the token immediately following the leading mention. Prose that merely names a command no longer triggers it.
| channel_id: Uuid, | ||
| model_id: &str, | ||
| ) -> SwitchOutcome { | ||
| if catalog.is_some_and(|caps| !caps.contains(model_id)) { |
There was a problem hiding this comment.
During the first turn's session/new, the task is already registered as in flight but ModelCatalog::capture may not have run yet. In that window catalog is None, so even a typo passes this guard, receives a SwitchModel signal, cancels and requeues the active request, and gets the misleading chat reply that the switch is proceeding; the subsequent fresh session merely falls back to the unchanged model. Treat an unavailable catalog as not yet switchable so the documented unknown-ID behavior does not disrupt the first turn.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8a18553. A missing catalog is no longer treated as "nothing to check" — the pick is deferred instead of passed through, so the first turn is never cancelled on an unvalidated ID. !model replies with the same "I don't know my model list yet" the no-argument branch gives, which is the honest answer at that point. On the observer path it reports as unsupported_model: same meaning for the picker (rejected, session untouched), and the desktop cannot reach the case anyway since its list comes from this same catalog.
| let current = pool | ||
| .channel_model_override(channel_id) | ||
| .or_else(|| caps.reported_current()); |
There was a problem hiding this comment.
After a model override, querying !model while that channel has an active turn cannot find the override because the owning OwnedAgent has been removed from the pool. This falls back to the catalog's first-session reported_current() value and marks the old/default model as current for the entire turn. Keep the per-channel override in state reachable from the main loop or task metadata so busy-channel listings report the model actually in force.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Acknowledged, and deferring this one. The inaccuracy is reporting-only and scoped to an in-flight turn: while the agent is checked out, the listing falls back to the adapter-reported current, so the current marker can point at the pre-override model until the turn ends. No switch is lost, the override still applies on the next turn.
Fixing it properly means threading the desired model through task metadata so the listing can read it off the in-flight task, which touches ~19 TaskMeta construction sites. That is more churn than this PR should carry. Tracked as #3535.
An owner-authored message beginning with `@` armed a scan across all remaining prose for the first whitespace-prefixed bang token. That made asking an agent *about* a command indistinguishable from issuing it: `@Agent explain what happens when I use !shutdown` parsed as `!shutdown` and terminated the harness, and the same shape could cancel or rotate an active turn mid-sentence. Commands are now recognized only at command position — first in the message, or immediately after the run of leading `@mention` tokens. This deliberately reverses an earlier trade-off in this branch, which accepted a bang anywhere after a leading mention so that `@Agent please run !model haiku` would fire. That was too wide: the same permissiveness is exactly what lets a question execute. `@Agent !model haiku` still works; the prose form no longer does. It also drops mentions holding spaces (`@Will Pfleger !rotate`), which the previous scan tolerated by accident. The parser has no display-name list, so where such a mention ends is genuinely ambiguous — and any rule loose enough to end it at `Pfleger` also ends it at the last word of a sentence. Not firing is the safe side of that ambiguity. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
The pre-cancel guard treated a missing catalog as "nothing to check", so any pick passed through unvalidated. The only window where the catalog is missing is the process's first `session/new` — which is already registered in flight — so in exactly that window a typo cancelled and requeued a live turn, replied that the switch was proceeding, and then had the fresh session fall back to the unchanged model. No catalog now means the pick is deferred rather than passed through. On the `!model` front door that reads back as the same "I don't know my model list yet" the no-argument branch already gives, which is the honest answer: the harness cannot say whether the ID is real. `CatalogUnavailable` reports as `unsupported_model` on the observer path rather than adding a status. Both mean "rejected, session untouched" — all the desktop acts on — and the desktop cannot reach the variant anyway, since its model list is populated from this same catalog. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
1 participant
!modellets the owner see and change the model backing an agent from thechannel itself, rather than the desktop picker.
!model— lists the agent's models as a numbered catalog, with← currentmarking the one in force for that channel. Resolves the channel's own
override first, then the adapter's reported selection, so a stock agent
nobody has overridden still marks something.
!model <id>— validates the pick against the catalog, then switches. Aturn in flight is cancelled and re-run on the new model; an idle agent takes
it on its next turn. Either way the reply says which happened.
left untouched. Matching is on exact ids: adapters ship near-identical pairs
(
opus[1m]vsclaude-opus-5,gpt-5.3-codexvsgpt-5.3-codex/low) wherea prefix rule would silently pick a different context lane and price point.
@Agent !model), since mentioning is how aclient addresses an agent.
!modelis a second front door onto the kind:24200switch_modelpath, not anew mechanism — both share one
switch_model_for_channelimplementation.The first commit is a prerequisite refactor:
is_owner_control_commandrequired exact content equality, so no command could carry an argument.
Replacing it with a parser that splits
(command, args)collapses the threeexisting inline owner checks into one match and makes
!model <id>expressibleat all. Behaviour of
!shutdown,!cancel, and!rotateis unchanged.Closes #2543