fix(ui): apply max-effort review + fix-sanity findings (composer caches)

A max-effort review of the composer-cache branch found 3 correctness + 2 cleanup issues; fix-sanity refined the plan before implementing. - Modal select stickiness [0]: the reused new-ws <dialog> kept the last open's model/judge/skill pick and silently applied it to the next chat (sharp for a fork — model/judge were sent unguarded). The composer selects now render fresh each open (a fresh open has no `previous` selection to preserve) across ALL five selects, while a within-open async repaint still preserves a mid-window pick. The modal now shows the resolved default ("Default — gpt-5"). A fork INHERITS its source's model + judge (hidden + submit-gated on !_forkFromWsId, matching skill/persona/project). - models default-alias reset [1]: the shared core's extra-reset-on-failure is now opt-in (resetExtraOnError). projects keeps it (require_project gates the picker, must fail open); models opts out, so a transient failure keeps the last-known resolved-default annotation instead of blanking it. - models_changed coalescing race [2]: an opt-in trailing refresh in the core — a force caller (models_changed) awaits a refetch chained after the in-flight one and converges to the latest state instead of a response predating the change; startup/open callers stay coalesced. - cleanups: the 4x paint-then-refresh block collapses into _paintModelSelects / _paintSkillSelect [6]; the "alias (model)" label centralizes into models.js modelLabel (registered on the window bridge) [7], deleting both local copies. Tests: rewrote the 5 guards that pinned pre-fix literals + added fresh-matrix, fork-inherit, bridge-registration, both-error-branch reset, and trailing-refresh guards. 106 pass; ruff + mypy green.
2026-08-12 23:12:23 -06:00 · 2026-07-18 21:02:09 -07:00
parent 71b1365e7b
commit 8d57697b2a
6 changed files with 362 additions and 192 deletions
@@ -2481,7 +2481,9 @@ def test_paint_project_picker_syncs_before_refresh() -> None:
    the two can't drift and silently re-introduce the FOUC) seeds the required/
    optional hint + paints the picker via _populateProjectSelect SYNCHRONOUSLY,
    then refreshes-and-repaints.  It skips a fork, and both paints route through
-    the same _populateProjectSelect (preserving the #867 strict-picker invariant)."""
+    the same _populateProjectSelect (preserving the #867 strict-picker invariant).
+    The sync paint passes freshOnOpen; the async repaint MUST pass fresh=false, or
+    it clobbers a project the user picked mid-refresh (require_project -> 400)."""
    body = _APP_JS.read_text(encoding="utf-8")
    fn = _slice_top_level_fn(body, "function _paintProjectPicker(")
    assert "_populateProjectSelect(" in fn, (
@@ -2489,12 +2491,15 @@ def test_paint_project_picker_syncs_before_refresh() -> None:
    )
    assert "hint.textContent" in fn, "_paintProjectPicker must seed the required/optional hint"
    assert "opts.fork" in fn, "_paintProjectPicker must skip for a fork"
-    sync_call = fn.find("paint();")
+    sync_call = fn.find("paint(freshOnOpen)")
    async_refresh = fn.find("refreshProjects().then")
    assert async_refresh >= 0, "_paintProjectPicker must keep refreshProjects().then"
    assert 0 <= sync_call < async_refresh, (
-        "_paintProjectPicker must paint synchronously (paint()) BEFORE the async "
-        "refreshProjects().then repaint"
+        "_paintProjectPicker must paint synchronously (paint(freshOnOpen)) BEFORE the "
+        "async refreshProjects().then repaint"
+    )
+    assert "paint(false)" in fn[async_refresh:], (
+        "_paintProjectPicker's async repaint must pass fresh=false (preserve a mid-window pick)"
    )


@@ -2516,57 +2521,103 @@ _PROJECTS_JS = Path(__file__).resolve().parent.parent / "turnstone/shared_static
 _PERSONAS_JS = Path(__file__).resolve().parent.parent / "turnstone/shared_static/personas.js"


-def test_new_ws_modal_paints_model_and_skill_from_cache_synchronously() -> None:
-    """The modal's model + skill pickers paint from the warm cache SYNCHRONOUSLY
-    on open, BEFORE the async refresh-and-repaint (no empty-dropdown flash)."""
+def test_paint_model_and_skill_wrappers_sync_before_refresh() -> None:
+    """The shared _paintModelSelects / _paintSkillSelect wrappers (used by BOTH the
+    modal and dashboard — collapsing the 4x paint-then-refresh dup) paint from the
+    warm cache SYNCHRONOUSLY, then refresh-and-repaint.  CRITICAL: the sync paint
+    passes fresh=freshOnOpen but the ASYNC repaint MUST pass fresh:false — a wrapper
+    leaking freshOnOpen into the async paint would clobber a mid-window pick (for the
+    project picker that reconciles to "" -> a require_project 400)."""
+    body = _APP_JS.read_text(encoding="utf-8")
+    for wrapper, populate, refresh in (
+        ("function _paintModelSelects(", "_populateModelSelect(", "refreshModels().then"),
+        ("function _paintSkillSelect(", "_populateSkillSelect(", "refreshSkills().then"),
+    ):
+        fn = _slice_top_level_fn(body, wrapper)
+        sync = fn.find(populate)
+        async_ = fn.find(refresh)
+        assert 0 <= sync < async_, f"{wrapper} must paint synchronously before the async refresh"
+        assert "{ fresh: freshOnOpen }" in fn, f"{wrapper} sync paint must pass fresh=freshOnOpen"
+        assert "{ fresh: false }" in fn, (
+            f"{wrapper} async repaint must pass fresh:false (never leak freshOnOpen, "
+            "or it clobbers a mid-window pick)"
+        )
+
+
+def test_new_ws_modal_renders_all_selects_fresh_on_open() -> None:
+    """Finding [0] fix — every composer select in the reused new-ws <dialog> renders
+    its ACTUAL value fresh on open (no stale carryover from the last open): model +
+    skill via the wrappers with freshOnOpen:true, persona via {fresh:true}, project
+    via _paintProjectPicker freshOnOpen:true.  The model paint is fork-gated."""
    body = _APP_JS.read_text(encoding="utf-8")
    fn = _slice_top_level_fn(body, "function showNewWsModal(")
-    sync_model = fn.find("_populateModelSelect(modelSelect")
-    async_model = fn.find("refreshModels().then")
-    assert sync_model >= 0, "the modal must paint the model picker from cache synchronously"
-    assert async_model >= 0, "the modal must keep refreshModels().then"
-    assert sync_model < async_model, (
-        "the synchronous model paint must precede the async refreshModels().then repaint"
+    assert "_paintModelSelects(modelSelect, judgeSelect, { freshOnOpen: true })" in fn, (
+        "the modal must paint model+judge fresh-on-open via the shared wrapper"
    )
-    sync_skill = fn.find("_populateSkillSelect(tplSelect)")
-    async_skill = fn.find("refreshSkills().then")
-    assert sync_skill >= 0, "the modal must paint the skill picker from cache synchronously"
-    assert async_skill >= 0, "the modal must keep refreshSkills().then"
-    assert sync_skill < async_skill, (
-        "the synchronous skill paint must precede the async refreshSkills().then repaint"
+    assert "_paintSkillSelect(tplSelect, { freshOnOpen: true })" in fn, (
+        "the modal must paint skill fresh-on-open"
    )
+    assert "_populatePersonaSelect(personaSelect, { fresh: true })" in fn, (
+        "the modal must paint persona fresh-on-open (kind default, no stale carryover)"
+    )
+    proj = fn[fn.find("_paintProjectPicker(projSelect") :]
+    assert "freshOnOpen: true" in proj[:120], (
+        "the modal must paint the project picker fresh-on-open"
+    )
+    # the model wrapper call is fork-gated (a fork inherits + hides model/judge)
+    guard = fn.find("if (!_forkFromWsId) {")
+    model_paint = fn.find("_paintModelSelects(modelSelect")
+    assert 0 <= guard < model_paint, "the modal model paint must be skipped for a fork"


-def test_dashboard_paints_model_and_skill_from_cache_synchronously() -> None:
-    """Dashboard twin — model + skill paint synchronously before the refresh, and
-    the old fetch-once ``options.length <= 1`` guard is GONE (upgraded to
-    refresh-on-open via the coalesced cache, matching project/persona)."""
+def test_dashboard_paints_model_and_skill_via_wrappers_preserving() -> None:
+    """Dashboard twin — model + skill paint via the SAME wrappers but freshOnOpen:false
+    (a persistent panel preserves a pick across a repaint), and the old fetch-once
+    ``options.length <= 1`` guard is gone."""
    body = _APP_JS.read_text(encoding="utf-8")
    fn = _slice_top_level_fn(body, "function _loadDashboardOptionsLists(")
-    sync_model = fn.find("_populateModelSelect(modelSel")
-    async_model = fn.find("refreshModels().then")
-    assert sync_model >= 0, "the dashboard must paint the model picker from cache synchronously"
-    assert 0 <= sync_model < async_model, (
-        "the synchronous dashboard model paint must precede the async refresh repaint"
+    assert "_paintModelSelects(modelSel, judgeSel, { freshOnOpen: false })" in fn, (
+        "the dashboard must paint model+judge via the wrapper, preserving (freshOnOpen:false)"
    )
-    sync_skill = fn.find("_populateSkillSelect(skillSel)")
-    async_skill = fn.find("refreshSkills().then")
-    assert sync_skill >= 0, "the dashboard must paint the skill picker from cache synchronously"
-    assert 0 <= sync_skill < async_skill, (
-        "the synchronous dashboard skill paint must precede the async refresh repaint"
+    assert "_paintSkillSelect(skillSel, { freshOnOpen: false })" in fn, (
+        "the dashboard must paint skill via the wrapper, preserving"
    )
    assert "options.length <= 1" not in fn, (
        "the dashboard model/skill fetch-once guard must be removed (refresh-on-open now)"
    )


+def test_new_ws_modal_fork_inherits_model_and_judge() -> None:
+    """Q2 — a fork INHERITS its source's model + judge: the modal hides both selects
+    for a fork (like skill/persona/project), and submitNewWs gates body.model AND
+    body.judge_model on !_forkFromWsId.  Asserts the model line SPECIFICALLY — its
+    guard `model && !_forkFromWsId` is a substring of the judge line, so a
+    model-unguarded regression would otherwise false-pass."""
+    body = _APP_JS.read_text(encoding="utf-8")
+    modal = _slice_top_level_fn(body, "function showNewWsModal(")
+    assert "modelSelect.hidden = !!_forkFromWsId" in modal, (
+        "modal must hide the model select for a fork"
+    )
+    assert "judgeSelect.hidden = !!_forkFromWsId" in modal, (
+        "modal must hide the judge select for a fork"
+    )
+    submit = _slice_top_level_fn(body, "function submitNewWs(")
+    assert "if (model && !_forkFromWsId) body.model = model;" in submit, (
+        "submitNewWs must fork-gate body.model (distinct from the judge line)"
+    )
+    assert "if (judge_model && !_forkFromWsId) body.judge_model = judge_model;" in submit, (
+        "submitNewWs must fork-gate body.judge_model"
+    )
+
+
 def test_console_launcher_paints_model_and_skill_from_cache_synchronously() -> None:
    """Console launcher — the model + skill refresh wrappers paint synchronously
-    from the warm cache before the async refresh-and-repaint."""
+    from the warm cache before the async refresh-and-repaint.  The model wrapper
+    threads callOpts so models_changed can force a trailing refresh."""
    body = _CONSOLE_APP_JS.read_text(encoding="utf-8")
    model_fn = _slice_top_level_fn(body, "function _refreshAndPopulateModels(")
    sync_model = model_fn.find("_populateHomeModelDropdowns()")
-    async_model = model_fn.find("refreshModels().then")
+    async_model = model_fn.find("refreshModels(callOpts).then")
    assert sync_model >= 0, "the launcher must paint the model pickers from cache synchronously"
    assert 0 <= sync_model < async_model, (
        "the synchronous launcher model paint must precede the async refresh repaint"
@@ -2574,7 +2625,6 @@ def test_console_launcher_paints_model_and_skill_from_cache_synchronously() -> N
    skill_fn = _slice_top_level_fn(body, "function _refreshAndPopulateSkills(")
    sync_skill = skill_fn.find("_populateHomeSkillDropdown()")
    async_skill = skill_fn.find("refreshSkills().then")
-    assert sync_skill >= 0, "the launcher must paint the skill picker from cache synchronously"
    assert 0 <= sync_skill < async_skill, (
        "the synchronous launcher skill paint must precede the async refresh repaint"
    )
@@ -2591,18 +2641,38 @@ def test_console_relogin_rewarms_model_and_skill() -> None:
    assert "_refreshAndPopulateModels()" in login, "onLoginSuccess must re-warm models"


-def test_models_changed_single_repaint_path() -> None:
-    """The console ``models_changed`` SSE handler repaints the launcher via the
-    refresh wrapper (which reads the warm cache), and the launcher does NOT ALSO
-    subscribe onModelsChange — one repaint path, no double rebuild (graph R7)."""
+def test_models_changed_forces_trailing_single_repaint_path() -> None:
+    """The console ``models_changed`` handler repaints via the refresh wrapper with
+    {force:true} — so a burst of model-config changes converges to the latest server
+    state (trailing refresh) — and the launcher does NOT ALSO subscribe onModelsChange
+    (one repaint path, no double rebuild)."""
    body = _CONSOLE_APP_JS.read_text(encoding="utf-8")
    mc = body.index('data.type === "models_changed"')
-    handler = body[mc : mc + 600]
-    assert "_refreshAndPopulateModels()" in handler, (
-        "models_changed must repaint the launcher model dropdowns via the refresh wrapper"
+    handler = body[mc : mc + 700]
+    assert "_refreshAndPopulateModels({ force: true })" in handler, (
+        "models_changed must force a trailing refresh so it converges to the latest"
    )
    assert "onModelsChange" not in body, (
-        "the console must not ALSO subscribe onModelsChange (single repaint path — R7)"
+        "the console must not ALSO subscribe onModelsChange (single repaint path)"
+    )
+
+
+def test_models_label_centralized_on_bridge() -> None:
+    """Finding [7] — the "alias (model)" label lives ONCE in models.js: modelLabel is
+    exported AND registered on the window.TurnstoneModels bridge (the classic app.js
+    bundles reach it only via the bridge — an ES-only export throws at runtime), and
+    neither app keeps a local _resolveModelLabel copy."""
+    models_src = _MODELS_JS.read_text(encoding="utf-8")
+    assert "export function modelLabel(" in models_src, "models.js must export modelLabel"
+    assert "modelLabel: modelLabel" in models_src, (
+        "models.js must register modelLabel on the window.TurnstoneModels bridge "
+        "(classic bundles call it via the bridge)"
+    )
+    assert "_resolveModelLabel" not in _APP_JS.read_text(encoding="utf-8"), (
+        "the ui app must not keep a local _resolveModelLabel (use TurnstoneModels.modelLabel)"
+    )
+    assert "_resolveModelLabel" not in _CONSOLE_APP_JS.read_text(encoding="utf-8"), (
+        "the console app must not keep a local _resolveModelLabel"
    )


@@ -2616,24 +2686,47 @@ def test_models_skills_module_tagged_in_both_apps() -> None:
        assert "/shared/skills.js" in html, f"skills.js must be module-tagged in {idx.name}"


-def test_list_cache_core_is_failopen_and_coalesced() -> None:
-    """The extracted list_cache.js core coalesces concurrent refreshes, fails open
-    (keeps the prior cache + records the error on a non-OK/exception, never
-    rejects — only the SUCCESS branch calls _setCache), and fires subscribers only
-    on first load or a changed fingerprint."""
+def test_list_cache_core_is_failopen_coalesced_and_gated() -> None:
+    """The extracted list_cache.js core: non-force callers coalesce onto the in-flight
+    refresh; it fails open (keeps the prior cache + records the error, never rejects —
+    only the SUCCESS branch calls _setCache); the extra-reset is GATED on
+    resetExtraOnError across BOTH error branches (finding [1]); and a force caller
+    schedules a trailing refetch that converges to the latest (finding [2])."""
    src = _LIST_CACHE_JS.read_text(encoding="utf-8")
-    assert "if (_inflight) return _inflight" in src, "refresh must coalesce concurrent callers"
+    assert "return _inflight;" in src, "non-force callers must coalesce onto the in-flight refresh"
    assert "_lastError = r.status" in src and "_lastError = 0" in src, (
        "a non-OK status and a network/parse error must both be recorded (fail-open)"
    )
    assert src.count("_setCache(data[dataKey]") == 1, (
        "only the success branch may repopulate the cache (non-OK/catch keep the prior cache)"
    )
+    # finding [1]: the extra-reset must be gated on resetExtraOnError on BOTH the
+    # non-OK branch AND the network/parse .catch branch (a cosmetic extra must
+    # survive a network drop too, not just a non-OK status).
+    assert src.count("resetExtraOnError && extraDefaults") == 2, (
+        "resetExtraOnError must gate the extra-reset on BOTH error branches"
+    )
+    # finding [2]: a force caller gets a trailing refetch (converges to latest).
+    assert "callOpts.force" in src and "_pending" in src, (
+        "a force caller must schedule a trailing refetch so it converges to the latest state"
+    )
    assert "firstLoad || fp !== _fingerprint" in src, (
        "subscribers must fire on first load or a changed fingerprint only"
    )


+def test_reset_extra_policy_per_cache() -> None:
+    """require_project (an advisory that GATES the picker) must fail open on error;
+    the models default-aliases (a COSMETIC annotation) must keep their last-known
+    value.  So projects opts INTO the reset, models opts OUT (finding [1])."""
+    assert "resetExtraOnError: true" in _PROJECTS_JS.read_text(encoding="utf-8"), (
+        "projects.js must reset the require_project advisory on error (fail-open)"
+    )
+    assert "resetExtraOnError: false" in _MODELS_JS.read_text(encoding="utf-8"), (
+        "models.js must keep last-known default aliases on error (cosmetic, not a gate)"
+    )
+
+
 def test_models_cache_exposes_both_server_schemas() -> None:
    """models.js must carry ALL default-alias fields — the node server sends
    default_alias, the console sends coordinator_default_alias, both send
@@ -429,9 +429,11 @@ function handleClusterEvent(data) {
    // Server emits this when a model definition or a role-assignment
    // setting (model.default_alias, judge.model, coordinator.model_alias,
    // coordinator.reasoning_effort) changes.  Refresh anything that
-    // renders model aliases so labels stay accurate without a reload.
+    // renders model aliases so labels stay accurate without a reload.  Pass
+    // {force:true} so a burst of changes converges to the latest (the coalesced
+    // open/startup path stays plain — see _refreshAndPopulateModels).
    if (typeof _refreshAndPopulateModels === "function") {
-      _refreshAndPopulateModels();
+      _refreshAndPopulateModels({ force: true });
    }
    if (typeof _sklcInvalidateModelsCache === "function") {
      _sklcInvalidateModelsCache();
@@ -1764,27 +1766,18 @@ function _populateHomeSkillDropdown() {
 // dropdown rows ("alias (model)", or just "alias" when they coincide).
 // Returns "" when alias is empty or unknown so callers can fall back
 // to a neutral placeholder.
-function _resolveModelLabel(alias, models) {
-  if (!alias) return "";
-  for (let i = 0; i < (models || []).length; i++) {
-    const m = models[i];
-    if (m.alias === alias) {
-      return m.alias === m.model ? m.alias : m.alias + " (" + m.model + ")";
-    }
-  }
-  return "";
-}
-
 // Refresh the shared models cache then repaint the launcher's Model + Judge
 // Model pickers.  Sync paint from the warm cache first (no empty flash for the
 // round-trip), then refresh-and-repaint.  This is ALSO the single repaint path
-// for the `models_changed` SSE event: it refreshes the cache and repaints, so
-// there is no second (subscription) path that would double-repaint.
-function _refreshAndPopulateModels() {
+// for the `models_changed` SSE event, which passes {force:true} so the refresh
+// converges to the latest server state (and its .then repaints from THAT) even
+// when a burst of model-config changes lands mid-fetch — no second
+// (subscription) path that would double-repaint.
+function _refreshAndPopulateModels(callOpts) {
  const TM = window.TurnstoneModels;
  if (!TM) return;
  _populateHomeModelDropdowns();
-  TM.refreshModels().then(_populateHomeModelDropdowns);
+  TM.refreshModels(callOpts).then(_populateHomeModelDropdowns);
 }

 // Populate the launcher's Model + Judge Model pickers from the shared cache —
@@ -1799,8 +1792,11 @@ function _populateHomeModelDropdowns() {
  const TM = window.TurnstoneModels;
  if (!TM) return;
  const choices = TM.modelChoices();
-  const models = TM.getModels();
  const defaults = TM.modelDefaults();
+  // The launcher composer is a PERSISTENT panel (it never reopens like the new-ws
+  // modal), so preserving the prior pick across a repaint is INTENDED — not the
+  // modal's fresh-on-open reset.  A background models_changed must not clobber a
+  // mid-window model/judge choice.
  const prevModel = _homeCoordComposer.getOptionValue("model");
  const prevJudge = _homeCoordComposer.getOptionValue("judge_model");
  _homeCoordComposer.setOptionChoices("model", choices);
@@ -1811,14 +1807,8 @@ function _populateHomeModelDropdowns() {
  // "Default model" line above it.  Em-dash separator (rather than nested
  // parens) keeps the alias's "(model)" suffix legible and matches the
  // ``(default — alias (model))`` pattern used in the admin Roles tab.
-  const coordDefault = _resolveModelLabel(
-    defaults.coordinator_default_alias || "",
-    models,
-  );
-  const judgeDefault = _resolveModelLabel(
-    defaults.judge_default_alias || "",
-    models,
-  );
+  const coordDefault = TM.modelLabel(defaults.coordinator_default_alias || "");
+  const judgeDefault = TM.modelLabel(defaults.judge_default_alias || "");
  _homeCoordComposer.setOptionPlaceholder(
    "model",
    coordDefault ? "Default — " + coordDefault : "Default model",
@@ -41,9 +41,20 @@ import { authFetch } from "./auth.js";
 *                                      successful refresh (returns the new
 *                                      `extra` object, replacing the prior one)
 * @param {object} [opts.extraDefaults] the fail-open `extra` value — installed
- *                                      at init AND restored on every refresh
- *                                      failure, so a stale-true advisory can't
- *                                      survive a transient error
+ *                                      at init AND (when resetExtraOnError)
+ *                                      restored on a refresh failure, so a
+ *                                      stale-true advisory can't survive a
+ *                                      transient error
+ * @param {boolean} [opts.resetExtraOnError=true]  whether a failed refresh resets
+ *                                      `extra` to extraDefaults.  True for an
+ *                                      ADVISORY that gates UI (must fail open);
+ *                                      false for a merely COSMETIC extra that
+ *                                      should keep its last-known value through a
+ *                                      transient failure rather than blank.
+ * `refresh(opts)` accepts `{force:true}` — an INVALIDATION (e.g. a *_changed SSE
+ * event) that must converge to the latest server state: if a refresh is already
+ * in flight it chains ONE trailing refetch after it and awaits that, instead of
+ * coalescing onto the in-flight response that may predate the change.
 * @returns {{refresh:Function, get:Function, getByKey:Function,
 *            loaded:Function, error:Function, extra:Function, onChange:Function}}
 */
@@ -56,12 +67,17 @@ export function makeListCache(opts) {
  const fpExtra = opts.fpExtra || null;
  const captureExtra = opts.captureExtra || null;
  const extraDefaults = opts.extraDefaults || null;
+  // Default true: an advisory that gates UI (projects' require_project) must fail
+  // open, so a stale-true value can't survive an error.  A cosmetic extra
+  // (models' resolved default aliases) passes false to keep the last-known value.
+  const resetExtraOnError = opts.resetExtraOnError !== false;

  let _cache = []; // last-fetched rows (caller-visible)
  let _byKey = {}; // keyField value -> row, for O(1) lookup (when keyField set)
  let _loaded = false; // has the first refresh attempt completed (ok or failed)?
  let _lastError = null; // last failure: HTTP status, 0 for network/parse, null when ok
  let _inflight = null; // shared pending refresh so concurrent callers coalesce
+  let _pending = null; // trailing refresh chained after _inflight for a force() call
  let _fingerprint = null; // last fired signature, for change-detection
  let _extra = extraDefaults ? Object.assign({}, extraDefaults) : {};
  const _subs = []; // () => void, fired after each CHANGED refresh
@@ -102,20 +118,43 @@ export function makeListCache(opts) {
    }
  }

-  function refresh() {
+  function refresh(callOpts) {
    // Coalesce concurrent callers (startup warm + a picker open can both fire
    // this) onto one in-flight request — they share the promise and _setCache
    // runs once.
-    if (_inflight) return _inflight;
+    if (_inflight) {
+      if (callOpts && callOpts.force) {
+        // A force caller is an INVALIDATION ("the data definitely changed" — a
+        // *_changed SSE event): it must converge to the latest server state, so
+        // chain ONE trailing refetch after the in-flight one and await THAT,
+        // rather than coalescing onto a response that may predate the change.
+        // Reused if several force calls land during the same fetch (a burst
+        // collapses to one trailing).  INVARIANT: refresh() never rejects (the
+        // chain below always resolves via .catch), so this .then always runs; if
+        // a reject path is ever added, clear _pending in a .catch or force
+        // callers wedge on a stuck promise.
+        _pending =
+          _pending ||
+          _inflight.then(function () {
+            _pending = null;
+            return refresh();
+          });
+        return _pending;
+      }
+      return _inflight;
+    }
    _inflight = authFetch(url)
      .then(function (r) {
        if (r.ok) return r.json();
        // A non-OK status (403 when a grant is missing, a 5xx, ...) is NOT "you
-        // have zero rows": keep the prior cache rather than blanking it, record
-        // the status so the failure is visible, and reset the advisory extra to
-        // its fail-open default (a stale-true value must not survive an error).
+        // have zero rows": keep the prior cache rather than blanking it and
+        // record the status.  Reset the extra to its fail-open default ONLY when
+        // resetExtraOnError (an advisory that gates UI must not survive stale); a
+        // cosmetic extra keeps its last-known value.
        _lastError = r.status;
-        if (extraDefaults) _extra = Object.assign({}, extraDefaults);
+        if (resetExtraOnError && extraDefaults) {
+          _extra = Object.assign({}, extraDefaults);
+        }
        console.warn(name + ": GET " + url + " -> " + r.status);
        return null;
      })
@@ -130,9 +169,11 @@ export function makeListCache(opts) {
      .catch(function (e) {
        // Network drop or a non-JSON body — same policy as a non-OK status:
        // preserve the last-known cache, never reject (callers chain a bare
-        // .then), surface the failure, fail-open the extra.
+        // .then), surface the failure, fail-open the extra (when configured).
        _lastError = 0;
-        if (extraDefaults) _extra = Object.assign({}, extraDefaults);
+        if (resetExtraOnError && extraDefaults) {
+          _extra = Object.assign({}, extraDefaults);
+        }
        console.warn(name + ": refresh failed", e);
        return _cache;
      })
@@ -57,16 +57,23 @@ const _core = makeListCache({
    judge_default_alias: "",
    coordinator_default_alias: "",
  },
+  // The default aliases are a COSMETIC placeholder annotation ("Default — gpt-5"),
+  // not a UI-gating advisory, so keep the last-known value through a transient
+  // refresh failure instead of blanking it back to an un-annotated "Default
+  // model".  A first-load failure still shows all-"" (the seed above).
+  resetExtraOnError: false,
 });

 /**
 * Fetch /v1/api/models into the cache.  Resolves to the row list and NEVER
 * rejects — a failed/forbidden fetch keeps the prior cache (a picker never
- * blanks) and resets the default aliases to "".  Recorded (see
- * {@link modelsError}) and warned rather than masqueraded as "no models".
+ * blanks) AND keeps the last-known default aliases; a first-load failure still
+ * shows all-"".  Recorded (see {@link modelsError}) rather than masqueraded as
+ * "no models".  Pass `{force:true}` to force a fresh fetch that converges to the
+ * latest server state even mid-flight (the `models_changed` SSE path uses this).
 */
-export function refreshModels() {
-  return _core.refresh();
+export function refreshModels(callOpts) {
+  return _core.refresh(callOpts);
 }

 /** Cached model rows (empty until the first refresh resolves).  Raw rows so
@@ -87,24 +94,42 @@ export function modelsError() {
  return _core.error();
 }

-/** `{value, text}` choices for a model <select> — the label ("alias (model)",
- *  or just "alias" when they coincide) is identical across all three creation
- *  surfaces, so it is centralized here.  Callers seed their own static
- *  "Default …" placeholder as option 0. */
+// The one place the "alias (model)" label (or just "alias" when they coincide)
+// is formatted — modelChoices, modelLabel, and every composer placeholder
+// annotation resolve through here so the option labels and the "Default — …"
+// placeholder can never disagree.
+function _fmtLabel(m) {
+  return m.alias === m.model ? m.alias : m.alias + " (" + m.model + ")";
+}
+
+/** `{value, text}` choices for a model <select> — the label is identical across
+ *  all three creation surfaces, so it is centralized here.  Callers seed their
+ *  own static "Default …" placeholder as option 0. */
 export function modelChoices() {
  return _core.get().map(function (m) {
-    return {
-      value: m.alias,
-      text: m.alias === m.model ? m.alias : m.alias + " (" + m.model + ")",
-    };
+    return { value: m.alias, text: _fmtLabel(m) };
  });
 }

+/** The "alias (model)" label for a model alias, or "" when the alias is empty or
+ *  unknown — lets a composer annotate its "Default — <resolved>" placeholder
+ *  without re-implementing the format.  The models cache has no key index, so
+ *  this scans the (small) row list. */
+export function modelLabel(alias) {
+  if (!alias) return "";
+  const rows = _core.get();
+  for (let i = 0; i < rows.length; i++) {
+    if (rows[i].alias === alias) return _fmtLabel(rows[i]);
+  }
+  return "";
+}
+
 /** The resolved default aliases the local server reported:
 *  `{default_alias, judge_default_alias, coordinator_default_alias}` (each ""
 *  when not sent).  The ui dashboard reads default_alias, the console launcher
- *  reads coordinator_default_alias; both read judge_default_alias.  Fails open
- *  to all-"" on a refresh error. */
+ *  reads coordinator_default_alias; both read judge_default_alias.  Keeps the
+ *  last-known values on a refresh error (a cosmetic annotation, not a UI gate);
+ *  all-"" only before the first successful load. */
 export function modelDefaults() {
  return _core.extra();
 }
@@ -123,6 +148,7 @@ window.TurnstoneModels = {
  modelsLoaded: modelsLoaded,
  modelsError: modelsError,
  modelChoices: modelChoices,
+  modelLabel: modelLabel,
  modelDefaults: modelDefaults,
  onModelsChange: onModelsChange,
 };
@@ -39,6 +39,11 @@ const _core = makeListCache({
    return { requireProject: !!data.require_project };
  },
  extraDefaults: { requireProject: false },
+  // require_project GATES the picker (strict mode hides the projectless option),
+  // so it must fail OPEN: a failed refresh resets it to false rather than letting
+  // a stale-true value hide options.  (Default, but explicit for the contrast
+  // with models.js, whose cosmetic extra opts out of the reset.)
+  resetExtraOnError: true,
 });

 /**
@@ -336,27 +336,26 @@ function showNewWsModal(forkFromWsId) {
  if (skillLabel) skillLabel.hidden = !!_forkFromWsId;
  if (skillSelect) skillSelect.hidden = !!_forkFromWsId;

-  // Model + judge pickers — paint from the warm models cache first, then
-  // refresh-and-repaint.  The modal keeps the plain static "Default …"
-  // placeholders (annotate:false); the dashboard annotates with the resolved
-  // default.  On a cold cache the sync paint is a no-op the refresh fills.
+  // Model + judge pickers — HIDDEN for a fork: a fork inherits its source's
+  // model + judge (like skill/persona/project), and submitNewWs gates both on
+  // !_forkFromWsId.  For a fresh create, paint from the warm cache with the
+  // resolved-default annotation ("Default — gpt-5"), FRESH on open (the reused
+  // dialog has no prior pick to carry over), then refresh-and-repaint.
+  const modelLabel = document.querySelector('label[for="new-ws-model"]');
+  const judgeLabel = document.querySelector('label[for="new-ws-judge-model"]');
  const modelSelect = document.getElementById("new-ws-model");
  const judgeSelect = document.getElementById("new-ws-judge-model");
-  _populateModelSelect(modelSelect, judgeSelect, { annotate: false });
-  if (window.TurnstoneModels) {
-    window.TurnstoneModels.refreshModels().then(function () {
-      _populateModelSelect(modelSelect, judgeSelect, { annotate: false });
-    });
+  if (modelLabel) modelLabel.hidden = !!_forkFromWsId;
+  if (judgeLabel) judgeLabel.hidden = !!_forkFromWsId;
+  if (modelSelect) modelSelect.hidden = !!_forkFromWsId;
+  if (judgeSelect) judgeSelect.hidden = !!_forkFromWsId;
+  if (!_forkFromWsId) {
+    _paintModelSelects(modelSelect, judgeSelect, { freshOnOpen: true });
  }

-  // Skill picker — same paint-from-cache-then-refresh.
+  // Skill picker — paint fresh-on-open from the warm cache, then refresh.
  const tplSelect = document.getElementById("new-ws-skill");
-  _populateSkillSelect(tplSelect);
-  if (window.TurnstoneSkills) {
-    window.TurnstoneSkills.refreshSkills().then(function () {
-      _populateSkillSelect(tplSelect);
-    });
-  }
+  _paintSkillSelect(tplSelect, { freshOnOpen: true });

  // Project picker — populated from the shared projects cache, refreshed on
  // open. Fresh creates SHOW it; forks HIDE it — a fork's project is its
@@ -378,7 +377,10 @@ function showNewWsModal(forkFromWsId) {
  // refresh-and-repaint — shared with the dashboard via _paintProjectPicker so
  // the two can't drift; skips for a fork (its picker is hidden above,
  // inheritance is server-enforced).
-  _paintProjectPicker(projSelect, projHint, { fork: !!_forkFromWsId });
+  _paintProjectPicker(projSelect, projHint, {
+    fork: !!_forkFromWsId,
+    freshOnOpen: true,
+  });

  // Persona picker — hidden when forking (a fork resumes the source's
  // stamped persona; the create handler skips resolution on resume_ws).
@@ -387,13 +389,13 @@ function showNewWsModal(forkFromWsId) {
  if (personaLabel) personaLabel.hidden = !!_forkFromWsId;
  if (personaSelect) personaSelect.hidden = !!_forkFromWsId;
  if (personaSelect && !_forkFromWsId && window.TurnstonePersonas) {
-    // Sync paint from the warm cache first, then refresh-and-repaint to catch a
-    // persona created elsewhere. _populatePersonaSelect preserves a mid-window
-    // pick and only applies the kind default when nothing valid is selected, so
-    // the second (async) paint can't clobber the user's choice.
-    _populatePersonaSelect(personaSelect);
+    // Fresh-on-open sync paint (renders the kind default; the reused dialog has
+    // no prior pick to carry over), then refresh-and-repaint. The async paint
+    // passes fresh:false so it preserves a mid-window pick and only re-applies
+    // the kind default when nothing valid is selected.
+    _populatePersonaSelect(personaSelect, { fresh: true });
    window.TurnstonePersonas.refreshPersonas().then(function () {
-      _populatePersonaSelect(personaSelect);
+      _populatePersonaSelect(personaSelect, { fresh: false });
    });
  }

@@ -471,9 +473,12 @@ function _ensureStandaloneProjectCreator(sel) {
 // — this dialog only creates interactive workstreams), preselecting the kind
 // default so a zero-touch create behaves exactly like today.  No-op when the
 // personas bridge is absent (module still loading / pre-seed database).
-function _populatePersonaSelect(sel) {
+function _populatePersonaSelect(sel, opts) {
  if (!sel || !window.TurnstonePersonas) return;
-  const previous = sel.value;
+  // A fresh modal open has no prior selection to preserve — render the kind
+  // default; a repaint (fresh=false) keeps a mid-window pick.
+  const fresh = !!(opts && opts.fresh);
+  const previous = fresh ? "" : sel.value;
  const placeholder = sel.options.length ? sel.options[0] : null;
  sel.replaceChildren();
  if (placeholder) sel.appendChild(placeholder);
@@ -518,37 +523,66 @@ function _optionExists(sel, val) {
 // #867 strict-picker invariant (never auto-select a real project) holds on each.
 function _paintProjectPicker(sel, hint, opts) {
  if ((opts && opts.fork) || !sel || !window.TurnstoneProjects) return;
-  const paint = function () {
+  // paint(fresh): a fresh open renders the actual default (no prior selection);
+  // the async repaint MUST pass fresh=false, or it clobbers a project the user
+  // picked during the refresh round-trip — under require_project that reconciles
+  // the select back to "" and submit then sends no project -> a 400.
+  const paint = function (fresh) {
    const strict = !!window.TurnstoneProjects.requireProject();
    if (hint) hint.textContent = strict ? "required" : "optional";
-    _populateProjectSelect(sel, { requireProject: strict });
+    _populateProjectSelect(sel, { requireProject: strict, fresh: fresh });
  };
-  paint(); // sync from the warm cache (no-op when cold; the async fills it)
-  window.TurnstoneProjects.refreshProjects().then(paint);
+  const freshOnOpen = !!(opts && opts.freshOnOpen);
+  paint(freshOnOpen); // sync from the warm cache (no-op when cold; the async fills it)
+  window.TurnstoneProjects.refreshProjects().then(function () {
+    paint(false);
+  });
 }

-// Fill the model + judge-model <select>s from the shared models cache.  One
-// list feeds BOTH selects with the same "alias (model)" labels; each keeps its
-// own static "Default …" placeholder (option 0) and its own preserved pick.
-// When opts.annotate is set (the dashboard) the placeholders show the
-// server-resolved default alias; the modal passes annotate:false and keeps the
-// plain static text.  No-op when the models bridge is absent (still loading) —
-// the async refresh then fills it, exactly as before this cache existed.
+// Paint the model + judge pickers from the warm cache then refresh-and-repaint,
+// collapsing the identical sync-then-refresh dance the modal and dashboard both
+// need (mirrors _paintProjectPicker).  freshOnOpen renders the actual defaults on
+// a reused-dialog open; the async repaint ALWAYS preserves (fresh:false) so a
+// mid-window pick survives.  The populate helper no-ops when the bridge/select is
+// absent (cold cache -> the async refresh fills it, never worse than before).
+function _paintModelSelects(modelSel, judgeSel, opts) {
+  const freshOnOpen = !!(opts && opts.freshOnOpen);
+  _populateModelSelect(modelSel, judgeSel, { fresh: freshOnOpen });
+  if (window.TurnstoneModels) {
+    window.TurnstoneModels.refreshModels().then(function () {
+      _populateModelSelect(modelSel, judgeSel, { fresh: false });
+    });
+  }
+}
+
+// Skill twin of _paintModelSelects.
+function _paintSkillSelect(sel, opts) {
+  const freshOnOpen = !!(opts && opts.freshOnOpen);
+  _populateSkillSelect(sel, { fresh: freshOnOpen });
+  if (window.TurnstoneSkills) {
+    window.TurnstoneSkills.refreshSkills().then(function () {
+      _populateSkillSelect(sel, { fresh: false });
+    });
+  }
+}
+
+// Fill the model + judge-model <select>s from the shared models cache.  One list
+// feeds BOTH selects with the same "alias (model)" labels; each placeholder is
+// annotated with the server-resolved default alias ("Default — gpt-5"), resolved
+// once via the cache's modelLabel.  A fresh open renders the default (no prior
+// selection); a repaint preserves each select's mid-window pick independently.
+// No-op when the models bridge is absent (still loading) — the async refresh then
+// fills it, exactly as before this cache existed.
 function _populateModelSelect(modelSel, judgeSel, opts) {
  if (!modelSel || !window.TurnstoneModels) return;
-  const annotate = !!(opts && opts.annotate);
+  const fresh = !!(opts && opts.fresh);
  const M = window.TurnstoneModels;
  const choices = M.modelChoices();
  const defaults = M.modelDefaults();
-  const models = M.getModels();
-  const prevModel = modelSel.value;
-  const prevJudge = judgeSel ? judgeSel.value : "";
-  const modelDefault = annotate
-    ? _resolveModelLabel(defaults.default_alias || "", models)
-    : "";
-  const judgeDefault = annotate
-    ? _resolveModelLabel(defaults.judge_default_alias || "", models)
-    : "";
+  const prevModel = fresh ? "" : modelSel.value;
+  const prevJudge = fresh || !judgeSel ? "" : judgeSel.value;
+  const modelDefault = M.modelLabel(defaults.default_alias || "");
+  const judgeDefault = M.modelLabel(defaults.judge_default_alias || "");
  modelSel.textContent = "";
  _appendOption(
    modelSel,
@@ -569,10 +603,11 @@ function _populateModelSelect(modelSel, judgeSel, opts) {
    _appendOption(modelSel, c.value, c.text, false);
    if (judgeSel) _appendOption(judgeSel, c.value, c.text, false);
  });
-  // Preserve a mid-window pick on EACH select independently so the async
-  // repaint can't clobber a fast user's choice.
-  if (prevModel && _optionExists(modelSel, prevModel))
+  // Preserve a mid-window pick on EACH select independently so the async repaint
+  // can't clobber a fast user's choice (a fresh open zeroed both above).
+  if (prevModel && _optionExists(modelSel, prevModel)) {
    modelSel.value = prevModel;
+  }
  if (judgeSel && prevJudge && _optionExists(judgeSel, prevJudge)) {
    judgeSel.value = prevJudge;
  }
@@ -583,9 +618,10 @@ function _populateModelSelect(modelSel, judgeSel, opts) {
 // ui label appends " [MCP]" for MCP-origin skills (the console launcher does
 // not — which is why the cache returns raw rows).  No-op when the bridge is
 // absent.
-function _populateSkillSelect(sel) {
+function _populateSkillSelect(sel, opts) {
  if (!sel || !window.TurnstoneSkills) return;
-  const previous = sel.value;
+  const fresh = !!(opts && opts.fresh);
+  const previous = fresh ? "" : sel.value;
  sel.textContent = "";
  _appendOption(sel, "", "Use defaults", false);
  window.TurnstoneSkills.getSkills().forEach(function (t) {
@@ -615,10 +651,15 @@ function _populateSkillSelect(sel) {
 function _populateProjectSelect(sel, opts) {
  if (!sel || !window.TurnstoneProjects) return;
  const mode = opts || sel._projMode || {};
-  sel._projMode = mode;
+  // Stamp ONLY the picker mode onto the element (the inline "+ New project…"
+  // creator's no-opts repaint reuses it); `fresh` is read from the LIVE opts
+  // per-call and deliberately NOT stamped, so a fresh:true modal open can't
+  // persist into a later preserve-repaint.
+  sel._projMode = { requireProject: !!mode.requireProject };
  const strict = !!mode.requireProject;
+  const fresh = !!(opts && opts.fresh);
  _ensureStandaloneProjectCreator(sel);
-  const previous = sel.value;
+  const previous = fresh ? "" : sel.value;
  const choices = window.TurnstoneProjects.projectChoices();
  sel.replaceChildren();
  if (!strict) {
@@ -687,8 +728,10 @@ function submitNewWs() {
  const initEl = document.getElementById("new-ws-initial-message");
  const initial_message = initEl ? initEl.value.trim() : "";
  if (name) body.name = name;
-  if (model) body.model = model;
-  if (judge_model) body.judge_model = judge_model;
+  // Forks inherit their source's model + judge (the selects are hidden for a
+  // fork), so never override them — matches the skill/persona/project fork guards.
+  if (model && !_forkFromWsId) body.model = model;
+  if (judge_model && !_forkFromWsId) body.judge_model = judge_model;
  if (skill && !_forkFromWsId) body.skill = skill;
  // Only a FRESH create sends a project_id; a fork's project is its source's,
  // enforced server-side (the picker is hidden for forks, and any explicit pid is
@@ -1450,46 +1493,18 @@ function _refreshDashboardSubmitLabel() {
  btn.textContent = hasText || hasFiles ? "Send" : "Create";
 }

-// Format a resolved alias with its model suffix the same way as the
-// dropdown rows ("alias (model)", or just "alias" when they coincide).
-// Returns "" when alias is empty or unknown so callers fall back to a
-// neutral placeholder.
-function _resolveModelLabel(alias, models) {
-  if (!alias) return "";
-  for (let i = 0; i < (models || []).length; i++) {
-    const m = models[i];
-    if (m.alias === alias) {
-      return m.alias === m.model ? m.alias : m.alias + " (" + m.model + ")";
-    }
-  }
-  return "";
-}
-
 function _loadDashboardOptionsLists() {
-  // Models — paint from the warm cache (placeholders annotated with the
-  // server-resolved default alias) then refresh-and-repaint.  Previously
-  // fetch-once (guarded on options.length); now refresh-on-open via the
-  // coalesced models cache, matching the project/persona pickers.
+  // Models + skills — paint from the warm cache (model placeholders annotated
+  // with the server-resolved default) then refresh-and-repaint, via the shared
+  // wrappers (same paths the modal uses).  The dashboard composer is persistent
+  // (not a reused dialog), so freshOnOpen:false — it preserves a pick across a
+  // repaint.  Previously fetch-once (guarded on options.length); now
+  // refresh-on-open via the coalesced caches, matching the project/persona pickers.
  const modelSel = document.getElementById("dashboard-model");
  const judgeSel = document.getElementById("dashboard-judge-model");
-  if (modelSel) {
-    _populateModelSelect(modelSel, judgeSel, { annotate: true });
-    if (window.TurnstoneModels) {
-      window.TurnstoneModels.refreshModels().then(function () {
-        _populateModelSelect(modelSel, judgeSel, { annotate: true });
-      });
-    }
-  }
-  // Skills — same paint-from-cache-then-refresh.
+  _paintModelSelects(modelSel, judgeSel, { freshOnOpen: false });
  const skillSel = document.getElementById("dashboard-skill");
-  if (skillSel) {
-    _populateSkillSelect(skillSel);
-    if (window.TurnstoneSkills) {
-      window.TurnstoneSkills.refreshSkills().then(function () {
-        _populateSkillSelect(skillSel);
-      });
-    }
-  }
+  _paintSkillSelect(skillSel, { freshOnOpen: false });

  // Project picker — paint from the warm cache then refresh-and-repaint (also
  // feeds the rail's group-by-project). Dashboard quick-create is ALWAYS a fresh