Skip to content

fix(model): accept Discord 'name:provider/model' on every surface; refuse unknown provider - #980

Merged
Kyzcreig merged 1 commit into
mainfrom
fix/model-name-prefix-provider-split
Sep 25, 2026
Merged

Kyzcreig merged 1 commit into
mainfrom
fix/model-name-prefix-provider-split

Conversation

@Kyzcreig

@Kyzcreig Kyzcreig commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator

fix(model): accept Discord 'name:provider/model' on every surface; refuse unknown provider

Pasting Discord's native /model rendering (/model name:claude-bpx-5/claude-fable-5-1)
into Telegram kept the current provider and stored the whole string as the model
id (claude-apr/name:claude-bpx-5/claude-fable-5-1), breaking the session.

  • parse_model_flags_detailed (the one parser for CLI/gateway/TUI) strips a leading
    name:/name=/model:/model= option label, so all four forms converge:
    name:P/X, P/X, X --provider P, name:X --provider P.
  • switch_model refuses X/model when X is neither a resolvable provider nor a
    vendor namespace, the current provider is not an aggregator, and the id is not
    recognised by the endpoint or declared in config. No state change.
  • Gateway /model confirmation appends the resolved provider/model pair.

Verified: 21 parser/switch tests + 2 gateway tests (Telegram literal Discord form
switches provider AND model; unknown prefix refused, no override written).
RED on base: 12 fail without the impl. Neighbor /model + TUI suites: 989 passed,
2 failed. Both failures are in test_user_providers_model_switch.py and fail
identically on clean fork/main.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

…fuse unknown provider

Pasting Discord's native /model rendering (`/model name:claude-bpx-5/claude-fable-5-1`)
into Telegram kept the current provider and stored the whole string as the model
id (claude-apr/name:claude-bpx-5/claude-fable-5-1), breaking the session.

- parse_model_flags_detailed (the one parser for CLI/gateway/TUI) strips a leading
  name:/name=/model:/model= option label, so all four forms converge:
  `name:P/X`, `P/X`, `X --provider P`, `name:X --provider P`.
- switch_model refuses `X/model` when X is neither a resolvable provider nor a
  vendor namespace, the current provider is not an aggregator, and the id is not
  recognised by the endpoint or declared in config. No state change.
- Gateway /model confirmation appends the resolved `provider/model` pair.

Verified: 21 parser/switch tests + 2 gateway tests (Telegram literal Discord form
switches provider AND model; unknown prefix refused, no override written).
RED on base: 12 fail without the impl. Neighbor /model + TUI suites: 989 passed,
2 failed. Both failures are in test_user_providers_model_switch.py and fail
identically on clean fork/main.
@Kyzcreig

Copy link
Copy Markdown
Collaborator Author

Upstream port: NousResearch#121947. It carries only the name:/model: label strip in parse_model_flags_detailed, plus the parser tests (red/green on upstream main: 7 fail / 4 pass without the fix, 11 pass with it). The provider/model inline split and the unknown-prefix refusal are not ported. Upstream has no _parse_inline_provider_model, uses provider:model as its inline syntax, and treats a slash as an aggregator vendor slug, so the split would be a behaviour change there. The PR body says so.

@Kyzcreig

Copy link
Copy Markdown
Collaborator Author

🤖 merged-by: apollo · lane: kanban-merge-pass · gate: BYPASS: FleetReview paused by Ace 2026-09-22 (state/fleetreview-pause marker present) · why: t_6b449bc0: /model on Telegram (and every gateway) must parse the Discord-rendered 'name:pro; worker completed in place, CI green

@Kyzcreig
Kyzcreig added this pull request to the merge queue Sep 24, 2026
Merged via the queue into main with commit c01b930 Sep 25, 2026
55 checks passed
@Kyzcreig
Kyzcreig deleted the fix/model-name-prefix-provider-split branch September 25, 2026 01:23
@Kyzcreig Kyzcreig added the fleetreview:post-merge Ask FleetReview to review this MERGED pull (merge commit vs first parent) label Sep 25, 2026
@ang-prism

ang-prism Bot commented Sep 27, 2026

Copy link
Copy Markdown

FleetReview

Post-merge review (fleetreview:post-merge override): this reviewed the merge commit against its first parent — the bytes that already shipped. It is not a pre-merge gate pass.

Confidence: 3/5

Findings

  • P1 hermes_cli/model_switch.py:2494 — Wrong route
  • P1 hermes_cli/model_switch.py:782 — Stripping a leading name: in the shared parser breaks the relay's name:<subscription>/<model> routing syntax

FleetReview provenance · models: B=gpt-6-sol, C=claude-code-opus-5-5, D=grok-4.6, F=gpt-6-sol · cost: $1.94 · duration: 7m 26s · rounds: 1 · files examined: 4

Kyzcreig pushed a commit that referenced this pull request Sep 28, 2026
…_60634825)

C7 backfill (durable-state loss) slice A. Each fix has a regression test
that fails on 7a81d46 and passes here.

- k103 (#987) home-session guard: takeover/operator_override events now
  record the home read BEFORE the guarded mutation, so `update --session
  <new> --takeover` names the displaced home instead of <new>.
- k105 (#953) rate-limit circuit notify: the episode latch is written only
  after notify.py exits 0; a failed page is retried on the next tick.
  kanban_budget._run_notify now returns whether the send succeeded.
- k107 (#1081) request_changes: a coverage comment the gate refuses on an
  already-held review run is rolled back with the refusal.
- k109 (#1050) worker identity: `spawned` records the Linux boot_id next to
  the start token; a token stamped under another boot is ignored and the
  causal window decides.
- k110 (#994) end_orphaned_terminal_runs merges its payload into existing
  run metadata (keeps `pool`) instead of overwriting it.
- k112 (#980) model switch: the unknown-provider refusal is exempted only by
  a declaration on the CURRENT provider, same match as the override block.
- k114 (#1254) strip_overlay restores PATH components the overlay removed.
- k115 (#1035) desktop cron ticker passes a serve admission gate as
  can_dispatch, so a checkout hold refuses scheduled dispatch.
- k121 (#1014) dashboard create homes the card to the viewing ?session=,
  so it stays visible under the default "this" facet.
- k138 (#1024) kanban_attach refuses a supplied but invalid expected_sha256
  instead of storing the file unverified.

Dropped (no code change): k108, k111, k113. Reasons are in the PR body.

test_desktop_cron_ticker_profiles: two exact-kwargs asserts now ignore the
new can_dispatch key.
Kyzcreig pushed a commit that referenced this pull request Sep 28, 2026
…_60634825) (#1373)

C7 backfill (durable-state loss) slice A. Each fix has a regression test
that fails on 7a81d46 and passes here.

- k103 (#987) home-session guard: takeover/operator_override events now
  record the home read BEFORE the guarded mutation, so `update --session
  <new> --takeover` names the displaced home instead of <new>.
- k105 (#953) rate-limit circuit notify: the episode latch is written only
  after notify.py exits 0; a failed page is retried on the next tick.
  kanban_budget._run_notify now returns whether the send succeeded.
- k107 (#1081) request_changes: a coverage comment the gate refuses on an
  already-held review run is rolled back with the refusal.
- k109 (#1050) worker identity: `spawned` records the Linux boot_id next to
  the start token; a token stamped under another boot is ignored and the
  causal window decides.
- k110 (#994) end_orphaned_terminal_runs merges its payload into existing
  run metadata (keeps `pool`) instead of overwriting it.
- k112 (#980) model switch: the unknown-provider refusal is exempted only by
  a declaration on the CURRENT provider, same match as the override block.
- k114 (#1254) strip_overlay restores PATH components the overlay removed.
- k115 (#1035) desktop cron ticker passes a serve admission gate as
  can_dispatch, so a checkout hold refuses scheduled dispatch.
- k121 (#1014) dashboard create homes the card to the viewing ?session=,
  so it stays visible under the default "this" facet.
- k138 (#1024) kanban_attach refuses a supplied but invalid expected_sha256
  instead of storing the file unverified.

Dropped (no code change): k108, k111, k113. Reasons are in the PR body.

test_desktop_cron_ticker_profiles: two exact-kwargs asserts now ignore the
new can_dispatch key.

Co-authored-by: ang-fleet-workers[bot] <333956806+ang-fleet-workers[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fleetreview:post-merge Ask FleetReview to review this MERGED pull (merge commit vs first parent)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant