fix(chat): shape-check the caller-supplied model at the interactive request boundary (#2796)

[L4XB]

Description

First, a correction to the diagnosis in the issue. #2796 reports that Trinity
passes the caller's role into the model slot. It does not, and I could not make
it. Driving the real handlers with an admin caller:

interactive POST /task with an admin caller	row model_used	execute_task(model=)
A. body omits model — what the Chat tab sends by default	None	None
B. body carries model="claude-sonnet-5"	claude-sonnet-5	claude-sonnet-5
C. body carries model="admin"	admin	admin

Case A is the one that matters: with the request the UI actually builds, the model
slot is None. Nothing in the dispatch path reads a role.

The "admin" an operator sees on the row is almost certainly
source_user_email, not a leaked role. The admin account has no email, so
current_user.email or current_user.username falls back to the fixed username
admin — and it lands in its own column. On the same run, execute_task receives
source_user_email='admin' and model=None.

The same holds on the session path: run_resumable_turn forwards model=None
to execute_task on both the initial attempt and the cold retry.

But the symptom is real, by a different route — case C. model is an
unvalidated free string on all three interactive entry points, and it travels
verbatim:

ParallelTaskRequest.model
  -> create_task_execution(model_used=…)
  -> execute_task(model=…)
  -> agent payload {"model": …}
  -> --model <value>
  -> [claude-code:unrecognized_model] {"model":"admin"} — exit 1, no output

Nothing refuses it, and the failure names no field, which is why the issue reads
as "the agent is broken". That is exactly what the issue's own Suggested
handling asks to fix, and it is what this PR does.

This also explains the reported distribution without any field mix-up: scheduled
runs carry agent_schedules.model explicitly and are unaffected; interactive runs
are the only ones that can carry a caller-supplied model at all.

The change

validate_dispatch_model() in services/model_catalog.py — the module that is
already the single source of truth for model identity — normalises blank to
None (inherit) and refuses a value that cannot name a model, raising
InvalidModelError.

The families it accepts are mirrored from
model_context._FAMILY_PREFIX_WINDOWS, which is already this platform's answer
to "is this id one we recognise?" (a miss there logs unrecognized model id).
That keeps Claude, Gemini and Codex (gpt-*, codex) agents dispatching,
keeps the bare aliases working, and case-folds the same way. Mirrored rather than
imported because model_context is vendored byte-identically into the agent
image (Invariant #5) and model_catalog is a stdlib-only leaf whose docstring
keeps that registry deliberately out of scope — so
test_2796_gate_covers_every_family_model_context_knows fails the build if the
two lists drift, rather than letting a newly supported runtime become
undispatchable.

The three routers map the refusal to 422 naming the offending value, and
normalise in place so the execution row and the dispatch payload cannot disagree:

• POST /api/agents/{name}/task — before the #1068 timeout normalisation
• POST /api/agents/{name}/chat — before admit_chat_request, so a refused
  request does not burn an idempotency key
• POST /api/agents/{name}/sessions/{id}/message — before the user row is
  persisted, so a refused turn does not strand a message with no reply

run_resumable_turn splats one value into execute_task twice (initial attempt

• cold retry). Validating the router's single source covers both; a test asserts
  the second call is not missed.

Why not reuse client_portal.service.validate_requested_model

That function is the right shape and I copied its ordering (normalise blank
first, then refuse, with the rejected value bounded to 64 chars before it is
echoed). I did not call it, for three reasons:

1. Its allow-list is the wrong set for this surface. WORKSPACE_MODELS is 3
   of the 11 ids in model_catalog.py. Applied to the operator routes it would
   422 eight models this repo ships.
2. It would break documented behaviour. ModelSelector.vue and the generated
   catalog header both state that free-text passthrough is deliberate, including
   the [1m] extended-context suffix (claude-sonnet-4-6[1m]). A closed set
   closes that door; a newly shipped id would need a catalog bump before anyone
   could type it.
3. Its is_platform gate is dead here, and importing client_portal.service
   into routers/chat.py would invert the layering — the portal consumes the
   platform, not the reverse.

So the Workspace keeps its closed allow-list unchanged — it is a security control
against an untrusted external client, and the comment above WORKSPACE_MODELS
("never a regex and never a prefix check") still governs PortalChatRequest.

The operator gate is deliberately weaker and is not a new policy or a second
registry: it is model_context's existing family table, applied at the request
boundary the issue names. admin matches no family. A leading - matches none
either, so argv smuggling on the operator path closes in passing.

If maintainers would rather have a closed allow-list here and accept the
free-text regression, say so and I will swap it — it is a one-line change to the
predicate.

Related Issue

Fixes #2796

One caveat worth stating rather than burying: this closes the reported failure at
the boundary — model="admin" is refused with a 422 instead of reaching
--model. It does not explain how "admin" entered a request body in the first
place, and the measurement above shows the Chat tab's default request does not
put it there. If it reached the reporter from a client in this repo, there is a
second defect upstream of this one. Happy to track that separately if you would
rather this PR not carry it.

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update

Not marked breaking: every id in MODEL_CATALOG, every short alias, the [1m]
suffix and any future claude-*/gemini-* id are still accepted. The only newly
refused inputs are strings that could never have run.

Testing

tests/unit/test_2796_dispatch_model_validation.py — 33 tests, one discriminating
case per call-site plus a guard on the correction itself.

On the merge base (dev @ eb41896f), with the validator-unit tests removed
so the file collects and the endpoint assertions run byte-identically against
base code:

FAILED test_2796_task_refuses_a_non_model_before_it_reaches_the_agent
FAILED test_2796_task_treats_blank_as_inherit_not_as_invalid
FAILED test_2796_chat_refuses_a_non_model_before_the_admission_gate
FAILED test_2796_session_refuses_a_non_model_before_persisting_the_user_turn
4 failed, 6 passed, 18 warnings in 0.55s

The 6 that pass on base are the characterization guards and must pass on both —
test_2796_role_does_not_reach_the_model_slot (case A above),
..._still_dispatches_a_legitimate_model,
..._keeps_the_documented_free_text_passthrough, and
..._resumable_turn_carries_one_model_to_both_call_sites. If the first ever
starts failing, the reported role-into-model mix-up has become real.

The blank-normalisation failure is a second real bug the gate closes: on base,
model=" " is forwarded as three spaces instead of meaning "inherit".

test_2796_gate_covers_every_family_model_context_knows is there because the
first draft of this gate hard-coded claude-/gemini- and would have made every
Codex agent undispatchable (gpt-5.1-codex, gpt-5.6-sol, bare codex).
Deriving the families from model_context and asserting the two agree turns that
class of mistake into a build failure.

The two lint gates that run in backend-unit-test.yml also pass:

$ python tests/lint_sys_modules.py
OK: 140 violation(s) in 49 file(s); baseline allows 206 — no new violations.
$ python tests/lint_root_test_placement.py
OK: no self-contained tests in tests/ root (baseline allows 11); no unmarked async under tests/unit/.

On this branch:

$ python -m pytest tests/unit/test_2796_dispatch_model_validation.py -q
33 passed, 17 warnings in 1.03s

Full unit suite on this branch (covers every existing test naming a symbol
touched here — test_2086_model_catalog_parity,
test_1483_execute_parallel_task_characterization, test_894_public_channel_model,
test_ent403_workspace_model, test_router_model_validators):

$ python -m pytest tests/unit -q

15953 passed, 31 skipped, 0 failed, in 12m39s. The focused run:
on the merge base 4 failed, 6 passed, on this branch 33 passed, the six that
pass on both being the characterization guards.

Not run: the live-stack tiers (tests/integration, tests/journeys) — they need
a running backend and agent containers, which I do not have here.

• I have tested this locally
• New tests added (if applicable)
• All existing tests pass — full tests/unit sweep: 15953 passed, 31 skipped, 0 failed (12m39s)

Checklist

• My code follows the project's style guidelines
• I have updated the documentation (if applicable) — bug fix, so per
  CONTRIBUTING's documentation table this is "descriptive commit message
  only"; no architecture or feature-flow change
• I have not committed any sensitive data (API keys, credentials, etc.)
• I have added appropriate logging for new functionality — the refusal is a
  422 carrying the offending value, which is the observable the issue asks
  for; no new background path needs a log line

Screenshots (if applicable)

n/a — backend validation, no UI change.

[L4XB]

The full sweep I promised in the description has landed, so the checklist box is now ticked with a real number rather than an intention.

$ python -m pytest tests/unit -q
15953 passed, 31 skipped, 1329 warnings in 759.77s (0:12:39)

Nothing failed. Run at 8202f280 on this branch. The 1329 warnings are pre-existing datetime.utcnow() deprecations across the suite, none from files this PR touches.

The focused numbers in the description are unchanged: on the merge base 4 failed, 6 passed, on this branch 33 passed, with the six that pass on both being the characterization guards — including test_2796_role_does_not_reach_the_model_slot, which pins the case-A measurement so the diagnosis the issue proposed cannot quietly become true later.

Still not run, and still stated as such: the live-stack tiers (tests/integration, tests/journeys), which need a running backend and agent containers.

[L4XB]

Brought up to date with dev.

The red pytest (head, seed 12345) was a cancellation, not a failure: the other five shards on the same run, head seeds 67890 and 99999 and all three base seeds, completed green. The push above gives the seed a fresh run.

[vybe]

merge-train: validated on #2820 (batch of 6, full suite green). Lane B — /validate-pr + /review; coverage mutation-proven (neutralising all three gates fails exactly 4 tests, one per gate). Error path traced end to end: single exception type, all three 422 sites outside any enclosing try, and the string-typed detail verified live on all three consumer surfaces (ChatPanel, sessions store, MCP client). Follow-ups noted on the PR: fan_out.py and loops.py still take model unvalidated, and the gate is runtime-blind.