feat(console): per-call model + judge_model on coord composer (#440)

* feat(console): per-call model + judge_model on coord composer

Brings the landing-page coordinator composer toward parity with the
interactive new-ws modal — operators can now pick a model and judge
model per session without round-tripping through the Models admin tab.

- Add Model + Judge Model selects to the home composer's options
  panel, populated from /v1/api/models. Empty / non-string fields
  collapse to None so the factory falls back to ConfigStore defaults
  (coordinator.model_alias, judge.model).
- _coord_create_build_kwargs threads the body fields onto mgr.create.
- Console session factory accepts judge_model and overrides the
  JudgeConfig via dataclasses.replace, mirroring the server-side
  interactive factory's pattern (alias preserved for IntentJudge's
  provider/client resolution).
- Sanitise the 503 factory-misconfig response across make_open_handler,
  make_create_handler, and make_detail_handler: a new
  _safe_factory_misconfig_message helper strips control characters
  and caps at 200 chars before echoing exc text. Operators still get
  the full alias in the warning log; clients see a bounded printable
  string. Defends the user-controlled body["model"] reflection
  surface on the create path.
- _build_mgr_with_factory test helper extracted from _build_mgr so
  tests that need to capture factory kwargs don't reconstruct the
  CoordinatorAdapter + SessionManager scaffolding inline.
- Tests cover: passthrough of model + judge_model, empty / whitespace
  / non-string body fields collapsing to None, and the 503 sanitiser
  truncating + scrubbing a hostile alias payload.

* fixup: address PR #440 Copilot review

- _safe_factory_misconfig_message: hard-cap return at
  _FACTORY_MISCONFIG_MAX_LEN total (was MAX_LEN+1 because the slice
  was MAX_LEN long with the ellipsis appended on top).  Reserve one
  codepoint for the ellipsis so the cap is honoured.  Update the
  regression test to assert the tighter bound.
- Composer judge_model placeholder: "Default (agent model)" was
  misleading when ConfigStore judge.model is set — the actual fallback
  is judge.model when set, IntentJudge's agent-model fallback when
  not.  Use "Default judge model" instead so the label matches both
  configs.

tests/_coord_test_helpers.py

@@ -79,18 +79,18 @@ def _fake_registry() -> MagicMock:
     return reg
 
 
-def _build_mgr(storage: Any) -> SessionManager:
-    """Build a SessionManager(CoordinatorAdapter) with stub factories (test default)."""
-
-    def _sf(ui, model_alias=None, ws_id=None, **kw):  # type: ignore[no-untyped-def]
-        s = MagicMock()
-        s.send.return_value = None
-        return s
+def _build_mgr_with_factory(storage: Any, session_factory: Any) -> SessionManager:
+    """Build a SessionManager(CoordinatorAdapter) with a caller-supplied factory.
 
+    Used by tests that need to capture or assert factory kwargs (e.g.
+    per-call ``model`` / ``judge_model`` overrides). Plain :func:`_build_mgr`
+    is the right entry point when the test doesn't care about the
+    factory.
+    """
     adapter = CoordinatorAdapter(
         collector=MagicMock(),
         ui_factory=lambda ws: ConsoleCoordinatorUI(ws_id=ws.id, user_id=ws.user_id or ""),
-        session_factory=_sf,
+        session_factory=session_factory,
     )
     mgr = SessionManager(
         adapter,
@@ -103,6 +103,17 @@ def _build_mgr(storage: Any) -> SessionManager:
     return mgr
 
 
+def _build_mgr(storage: Any) -> SessionManager:
+    """Build a SessionManager(CoordinatorAdapter) with stub factories (test default)."""
+
+    def _sf(ui, model_alias=None, ws_id=None, **kw):  # type: ignore[no-untyped-def]
+        s = MagicMock()
+        s.send.return_value = None
+        return s
+
+    return _build_mgr_with_factory(storage, _sf)
+
+
 class MockStorage:
     """Minimal storage mock that implements ``list_services``.
 

tests/test_coordinator_endpoints.py

@@ -27,6 +27,7 @@ from starlette.testclient import TestClient
 from tests._coord_test_helpers import (
     _AuthMiddleware,
     _build_mgr,
+    _build_mgr_with_factory,
     _fake_registry,
     _FakeConfigStore,
 )
@@ -414,6 +415,107 @@ def test_create_returns_ws_id_and_records_audit(storage):
     assert "coordinator.create" in actions
 
 
+def _capture_factory_pair():
+    """Return ``(factory, captured)`` — factory records model_alias +
+    judge_model into the captured dict on every call so tests can assert
+    the per-call override threading."""
+    captured: dict = {}
+
+    def _factory(ui, model_alias=None, ws_id=None, **kw):  # type: ignore[no-untyped-def]
+        captured["model_alias"] = model_alias
+        captured["judge_model"] = kw.get("judge_model")
+        return MagicMock()
+
+    return _factory, captured
+
+
+def test_create_forwards_model_and_judge_model_overrides(storage):
+    """Per-call ``model`` + ``judge_model`` body fields land on the
+    coord session factory (mirrors interactive's create surface)."""
+    factory, captured = _capture_factory_pair()
+    mgr = _build_mgr_with_factory(storage, factory)
+    client = _make_client(storage, coord_mgr=mgr, registry=_fake_registry())
+    resp = client.post(
+        "/v1/api/workstreams/new",
+        json={
+            "name": "tuned-coord",
+            "model": "gpt-5",
+            "judge_model": "gpt-5-mini",
+        },
+        headers=_COORD_HEADERS,
+    )
+    assert resp.status_code == 200, resp.text
+    assert captured == {"model_alias": "gpt-5", "judge_model": "gpt-5-mini"}
+
+
+def test_create_empty_model_fields_collapse_to_none(storage):
+    """Empty-string ``model`` / ``judge_model`` body fields don't override
+    the ConfigStore default — they collapse to ``None`` so the factory
+    falls back to ``coordinator.model_alias`` / ``judge.model``."""
+    factory, captured = _capture_factory_pair()
+    mgr = _build_mgr_with_factory(storage, factory)
+    client = _make_client(storage, coord_mgr=mgr, registry=_fake_registry())
+    resp = client.post(
+        "/v1/api/workstreams/new",
+        json={"name": "default-coord", "model": "  ", "judge_model": ""},
+        headers=_COORD_HEADERS,
+    )
+    assert resp.status_code == 200, resp.text
+    assert captured == {"model_alias": None, "judge_model": None}
+
+
+def test_create_503_factory_misconfig_message_is_sanitised(storage):
+    """503 response from a factory ``ValueError`` strips ASCII control
+    chars and caps the echoed alias text — defence-in-depth for the
+    user-controlled ``body["model"]`` reflection surface. Operators
+    keep the actionable message in the log; clients see a clean
+    bounded string."""
+
+    def _factory_raises(ui, model_alias=None, ws_id=None, **kw):  # type: ignore[no-untyped-def]
+        # Simulate the registry's actual exception shape, plus a
+        # control char + a long-tail attacker payload.
+        raise ValueError("Unknown model alias: \x00\x07attack\x1b[31m" + ("A" * 1000))
+
+    mgr = _build_mgr_with_factory(storage, _factory_raises)
+    client = _make_client(storage, coord_mgr=mgr, registry=_fake_registry())
+    resp = client.post(
+        "/v1/api/workstreams/new",
+        json={"name": "c"},
+        headers=_COORD_HEADERS,
+    )
+    assert resp.status_code == 503
+    err = resp.json()["error"]
+    # Cap enforced (hard-capped at _FACTORY_MISCONFIG_MAX_LEN total —
+    # the truncation reserves one codepoint for the ellipsis).
+    assert len(err) <= 200
+    assert "\x00" not in err
+    assert "\x1b" not in err
+    assert "Unknown model alias" in err
+    assert err.endswith("…")
+
+
+def test_create_non_string_model_fields_collapse_to_none(storage):
+    """Non-string ``model`` / ``judge_model`` body fields (e.g. a hostile
+    dict / list / int) collapse to ``None`` rather than reaching
+    ``.strip()`` and crashing into the lifted handler's generic 500
+    path. Defense-in-depth — the auth gate already requires
+    ``admin.coordinator``."""
+    factory, captured = _capture_factory_pair()
+    mgr = _build_mgr_with_factory(storage, factory)
+    client = _make_client(storage, coord_mgr=mgr, registry=_fake_registry())
+    resp = client.post(
+        "/v1/api/workstreams/new",
+        json={
+            "name": "hostile-body",
+            "model": {"url": "http://evil"},
+            "judge_model": [1, 2, 3],
+        },
+        headers=_COORD_HEADERS,
+    )
+    assert resp.status_code == 200, resp.text
+    assert captured == {"model_alias": None, "judge_model": None}
+
+
 _PNG_1X1 = (
     b"\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x00\x01\x00\x00\x00\x01"
     b"\x08\x06\x00\x00\x00\x1f\x15\xc4\x89\x00\x00\x00\rIDATx\x9cc\xfc\xcf"

turnstone/console/server.py

@@ -2636,12 +2636,14 @@ def _coord_create_build_kwargs(
 ) -> dict[str, Any]:
     """Build kwargs for ``coord_mgr.create`` from a parsed coord create body.
 
-    Coord's create takes a smaller set than interactive's (no
-    ``model`` / ``judge_model`` / ``client_type`` / ``parent_ws_id`` /
-    ``ws_id``) — those concepts either don't apply to coordinators
-    (no parent on coord; coord ws_id is always server-generated)
-    or live on a separate ConfigStore knob (the dashboard-managed
-    plan/task model + reasoning_effort settings).
+    Coord's create still takes a smaller set than interactive's
+    (no ``client_type`` / ``parent_ws_id`` / ``ws_id`` — coord ws_id
+    is always server-generated and coord has no parent), but
+    per-call ``model`` and ``judge_model`` overrides flow through
+    here onto the coord session factory the same way they flow
+    through interactive's: ConfigStore (``coordinator.model_alias``
+    / ``judge.model``) sets the default; this body field overrides
+    for one session.
     """
     # Use the canonical skill name from the resolved row when one was
     # found; falls back to the stripped body value (which is what the
@@ -2653,12 +2655,25 @@ def _coord_create_build_kwargs(
     else:
         canonical_skill = (body.get("skill") or "").strip() or None
     name = (body.get("name") or "").strip()
+    # Empty / non-string / whitespace-only body fields collapse to None
+    # so the factory falls back to ConfigStore defaults rather than
+    # treating "" (or a hostile dict / list) as a request to override
+    # with the empty alias.  The isinstance guard also keeps a
+    # truthy-non-string body (e.g. ``{"model": {"url": "x"}}``) from
+    # reaching ``.strip()`` and crashing into the lifted handler's
+    # generic 500 path.
+    model_raw = body.get("model")
+    judge_raw = body.get("judge_model")
+    model = (model_raw.strip() if isinstance(model_raw, str) else "") or None
+    judge_model = (judge_raw.strip() if isinstance(judge_raw, str) else "") or None
     return {
         "user_id": uid,
         "name": name,
         "skill": canonical_skill,
         "skill_id": skill_id,
         "skill_version": applied_skill_version,
+        "model": model,
+        "judge_model": judge_model,
     }
 
 

turnstone/console/session_factory.py

@@ -88,6 +88,7 @@ def build_console_session_factory(
         client_type: str = "web",
         kind: WorkstreamKind = WorkstreamKind.COORDINATOR,
         parent_ws_id: str | None = None,
+        judge_model: str | None = None,
     ) -> ChatSession:
         assert ui is not None, "console session_factory requires a non-None UI"
         if kind != WorkstreamKind.COORDINATOR:
@@ -134,6 +135,28 @@ def build_console_session_factory(
         # name that provider may not even know about (e.g. coordinator on
         # Anthropic, judge alias pointing at OpenAI gpt-5-mini → silent
         # ``llm_fallback`` verdicts on every tool call).
+        if live_judge_config and judge_model:
+            import dataclasses
+
+            # Per-call judge_model override mirrors the server-side
+            # interactive factory: pin the alias on the JudgeConfig but
+            # leave alias→client/provider resolution to IntentJudge for
+            # the same reason as above.  ``registry.resolve`` is only
+            # called as a typo / unknown-alias guard so a misconfigured
+            # body field surfaces in the log instead of silently falling
+            # back to the session's provider.
+            try:
+                registry.resolve(judge_model)
+                live_judge_config = dataclasses.replace(
+                    live_judge_config,
+                    model=judge_model,
+                )
+            except Exception as e:
+                log.warning(
+                    "coord_factory.judge_model_resolve_failed alias=%r err=%s",
+                    judge_model,
+                    e,
+                )
 
         eff_temperature = (
             r_cfg.temperature

turnstone/console/static/app.js

@@ -1450,6 +1450,8 @@ function _hasCoordPermission() {
 function _createCoordinator(opts) {
   var name = (opts.name || "").trim();
   var skill = opts.skill || "";
+  var model = (opts.model || "").trim();
+  var judgeModel = (opts.judge_model || "").trim();
   var task = (opts.task || "").trim();
   var errEl = opts.errEl;
   var setBusy = opts.setBusy || function () {};
@@ -1463,6 +1465,8 @@ function _createCoordinator(opts) {
   var body = {};
   if (name) body.name = name;
   if (skill) body.skill = skill;
+  if (model) body.model = model;
+  if (judgeModel) body.judge_model = judgeModel;
   if (task) body.initial_message = task;
 
   authFetch("/v1/api/workstreams/new", {
@@ -1527,6 +1531,7 @@ function _ensureHomeComposerInit() {
   _homeComposerInit = true;
   _mountHomeCoordComposer();
   _populateHomeSkillDropdown();
+  _populateHomeModelDropdowns();
   _probeCoordSubsystem();
   _refreshHomeComposerVisibility();
 }
@@ -1555,6 +1560,8 @@ function _mountHomeCoordComposer() {
         var bits = [];
         if (v.name) bits.push(v.name);
         if (v.skill) bits.push(v.skill);
+        if (v.model) bits.push(v.model);
+        if (v.judge_model) bits.push("judge: " + v.judge_model);
         return bits.join(" \u00b7 ");
       },
       fields: [
@@ -1571,6 +1578,22 @@ function _mountHomeCoordComposer() {
           type: "select",
           choices: [{ value: "", text: "Use defaults" }],
         },
+        {
+          id: "model",
+          label: "Model",
+          type: "select",
+          choices: [{ value: "", text: "Default model" }],
+        },
+        {
+          id: "judge_model",
+          label: "Judge Model",
+          type: "select",
+          // Neutral label — the actual default is ConfigStore
+          // ``judge.model`` when set, IntentJudge's agent-model
+          // fallback when not.  "Default judge model" doesn't
+          // mislead either way.
+          choices: [{ value: "", text: "Default judge model" }],
+        },
       ],
     },
     onSend: function (text) {
@@ -1599,6 +1622,30 @@ function _populateHomeSkillDropdown() {
     });
 }
 
+// Populate Model + Judge Model dropdowns from /v1/api/models — same
+// list the interactive new-ws modal uses.  Empty/default option stays
+// at the top so submitting without a choice falls back to the
+// ConfigStore-configured coordinator.model_alias / judge.model.
+function _populateHomeModelDropdowns() {
+  if (!_homeCoordComposer) return;
+  authFetch("/v1/api/models")
+    .then(function (r) {
+      return r.ok ? r.json() : { models: [] };
+    })
+    .then(function (data) {
+      var choices = (data.models || []).map(function (m) {
+        var label =
+          m.alias === m.model ? m.alias : m.alias + " (" + m.model + ")";
+        return { value: m.alias, text: label };
+      });
+      _homeCoordComposer.setOptionChoices("model", choices);
+      _homeCoordComposer.setOptionChoices("judge_model", choices);
+    })
+    .catch(function () {
+      /* defaults still work even without the dropdown populated */
+    });
+}
+
 // Probe GET /v1/api/workstreams — 200 = subsystem ready; 503 = no model
 // alias resolvable, show remediation banner.  4xx (auth / permission) is
 // treated as "unknown, don't flip the banner" because the probe cannot
@@ -1654,6 +1701,8 @@ function submitHomeCoord(textFromComposer) {
   _createCoordinator({
     name: opts.name || "",
     skill: opts.skill || "",
+    model: opts.model || "",
+    judge_model: opts.judge_model || "",
     task: task,
     errEl: document.getElementById("home-coord-error"),
     setBusy: function (b) {

turnstone/core/session_routes.py

@@ -52,6 +52,37 @@ if TYPE_CHECKING:
 log = get_logger(__name__)
 
 
+# Cap echoed factory-misconfig messages.  ``ValueError`` from the
+# session factory carries an operator-actionable remediation hint
+# (``"Unknown model alias: <alias>"`` etc.) that the lifted handlers
+# surface as a 503 — but the alias portion is user-controlled on the
+# create path (body ``model`` / ``judge_model`` fields) so a raw echo
+# reflects arbitrary input back into anything that renders the JSON
+# error verbatim.  Length cap + control-char strip keep the message
+# actionable for legit alias typos while neutralising hostile payloads.
+_FACTORY_MISCONFIG_MAX_LEN = 200
+
+
+def _safe_factory_misconfig_message(exc: BaseException) -> str:
+    """Sanitise a factory-misconfig ``ValueError`` for echo in a 503 body.
+
+    Strips ASCII control characters (``\\x00``-``\\x1f`` + ``\\x7f``)
+    and truncates to :data:`_FACTORY_MISCONFIG_MAX_LEN`.  Empty after
+    sanitisation falls back to a fixed generic message so a control-
+    char-only payload doesn't surface as ``"error": ""``.
+    """
+    text = str(exc)
+    cleaned = "".join(ch for ch in text if ch.isprintable())
+    if not cleaned:
+        return "session factory misconfigured"
+    if len(cleaned) > _FACTORY_MISCONFIG_MAX_LEN:
+        # Reserve one codepoint for the ellipsis so the returned string
+        # is hard-capped at _FACTORY_MISCONFIG_MAX_LEN total, not
+        # MAX_LEN+1.
+        cleaned = cleaned[: _FACTORY_MISCONFIG_MAX_LEN - 1] + "…"
+    return cleaned
+
+
 Handler = Callable[["Request"], Awaitable["Response"]]
 PermissionGate = Callable[["Request"], "JSONResponse | None"]
 ManagerLookup = Callable[["Request"], tuple["SessionManager | None", "JSONResponse | None"]]
@@ -1197,7 +1228,8 @@ def make_open_handler(
             # text as a 503 so the operator can fix it without
             # digging through stack traces. Same shape coord used
             # pre-lift; standardised across both kinds here.
-            return JSONResponse({"error": str(exc)}, status_code=503)
+            log.warning("ws.open.factory_misconfig ws_id=%s exc=%r", ws_id[:8], exc)
+            return JSONResponse({"error": _safe_factory_misconfig_message(exc)}, status_code=503)
         except Exception:
             # Bare ``Exception`` is intentional: ``mgr.open`` can
             # raise from ``adapter.build_session`` (no documented
@@ -1768,8 +1800,11 @@ def make_create_handler(
             # (model alias points at a model that no longer exists,
             # etc.). Surface the factory's remediation text as 503 so
             # operators get the actionable message instead of a
-            # stack-traced 500.
-            return JSONResponse({"error": str(exc)}, status_code=503)
+            # stack-traced 500. Sanitiser caps + scrubs the echoed
+            # text since the alias is user-controlled on the create
+            # path (body ``model`` / ``judge_model``).
+            log.warning("ws.create.factory_misconfig exc=%r", exc)
+            return JSONResponse({"error": _safe_factory_misconfig_message(exc)}, status_code=503)
         except Exception:
             # Don't echo the exception text — it can leak internal
             # paths / frame names. Log with a correlation id and
@@ -2235,7 +2270,10 @@ def make_detail_handler(cfg: SessionEndpointConfig) -> Handler:
                 # Session factory misconfig (e.g. a model alias that no
                 # longer resolves). Surface remediation text as 503
                 # mirroring :func:`make_open_handler`.
-                return JSONResponse({"error": str(exc)}, status_code=503)
+                log.warning("ws.detail.factory_misconfig ws_id=%s exc=%r", ws_id[:8], exc)
+                return JSONResponse(
+                    {"error": _safe_factory_misconfig_message(exc)}, status_code=503
+                )
             except Exception:
                 # Bare ``Exception`` is intentional — see
                 # :func:`make_open_handler` for the rationale