chore: bump version to 1.6.0a3

5 follow-up comments from Copilot, all valid:

1. **CRITICAL — snap_seq race with split writer** (concurrency, 001).
   Round-1's fix lifted snapshot capture into
   register_listener_with_replay under nested locks, but the
   WRITER side (on_content_token / on_reasoning_token) still
   released _ws_lock before calling _enqueue (which bumps
   _event_id under _listeners_lock).  A reader could
   interleave between writer's release and writer's _enqueue:
   capture inflight WITH the new text, read STALE _event_id,
   return snap_seq < new_event_id.  The new event's live emit
   then has _seq > snap_seq, slips past the dedup filter, and
   double-renders text the snapshot already contained.

   Fix: move self._enqueue(...) INSIDE the with self._ws_lock:
   block in both token writers.  The inflight mutation and the
   _event_id advancement are now atomic against any snapshot
   reader.  Lock order _ws_lock (outer) → _listeners_lock
   (inner via _enqueue) matches the snapshot helpers, so no
   deadlock.  Fan-out's put_nowait calls happen under
   _ws_lock for token writers — microsecond cost per listener,
   acceptable for the correctness guarantee.

2. **NIT — stale comment ref to buffered[-1]._event_id** (docs, 002).
   The comment referenced a local var (buffered) that lives in
   register_listener_with_replay, not in the events handler.
   Reworded to describe the cutoff in terms of the last replayed
   event id and the atomic-against-writers registration.

3. **MODERATE — 401 branch leaves reconnect loop** (bug, 003).
   The coord's onerror schedules a 5 s CLOSED-state recovery timer
   unconditionally.  In the 401-expired-session branch we close
   evtSource and showLogin — but the timer still fires 5 s later,
   observes !evtSource, and calls scheduleReconnect(),
   which opens a new EventSource that 401s again → infinite
   reconnect loop while the login overlay is up.  Fix: cancel
   reconnectTimer in the 401 branch.

4. **MODERATE — race test was vacuous** (test_coverage, 004).
   The previous regression test drained the listener queue after
   register_listener_with_replay returned, but the helper
   doesn't backfill buffered events into the queue, so the loop
   was almost always a no-op and the assertion never executed.
   Rewrote with a monkey-patched _enqueue that sleeps 50 ms
   before bumping _event_id — widens the race window
   deterministically.  Verified: the test FAILS on pre-fix code
   (snap.content has marker but snap.seq=0 < final_event_id=1)
   and PASSES on post-fix code (writer holds _ws_lock through
   _enqueue, so the reader blocks until writer fully done).
   Also pinned the no-backfill contract so a future change adding
   listener-queue backfill remembers to keep snap_seq the
   high-water mark.

5. **NIT — except Exception too broad in test** (best_practices, 005).
   Tightened except Exception: to except queue.Empty: so
   unexpected exceptions aren't silently swallowed in the drain
   loop.

Tests:
- 86 tests in test_sse_reconnect_replay.py + test_session_ui_base.py
  pass (existing 84 + 2 new race regressions).
- Full non-live suite: 6347 passed, 15 skipped, no regressions.
- Ruff + mypy clean on changed .py files; JS parses.

docs/coordinator-skills.md

@@ -14,34 +14,46 @@ care about.
 
 ---
 
-## The two-surface model
+## `kind` — authored audience metadata
 
 A row in `prompt_templates` carries a `kind` column (see
 [`turnstone/core/skill_kind.py`](../turnstone/core/skill_kind.py);
 migration 044 added the column).  Three values:
 
-| `SkillKind` enum     | Stored as                   | Visible in                                                                |
-|----------------------|-----------------------------|---------------------------------------------------------------------------|
-| `SkillKind.INTERACTIVE` | `"interactive"`          | Only the interactive-session activation path.  `list_skills` on a coord won't show it. |
-| `SkillKind.COORDINATOR` | `"coordinator"`          | Only the coordinator's `list_skills` tool.  Hidden from interactive activation pickers. |
-| `SkillKind.ANY`         | `"any"`                  | Both surfaces.  Default for legacy rows predating the classifier.        |
+| `SkillKind` enum        | Stored as       | Meaning                                                                    |
+|-------------------------|-----------------|----------------------------------------------------------------------------|
+| `SkillKind.INTERACTIVE` | `"interactive"` | Authored for the interactive maker persona (single-workstream "do this"). |
+| `SkillKind.COORDINATOR` | `"coordinator"` | Authored for the orchestrator persona (delegate, monitor, synthesise).    |
+| `SkillKind.ANY`         | `"any"`         | Either surface (or audience-neutral).  Default on create.                 |
 
-The `kind` field is a `StrEnum` — drop-in ``str`` compatible — so
-DB rows, JSON payloads, and `==` comparisons all work without
-translation at the edge.
+The `kind` field is a `StrEnum` — drop-in `str` compatible — so DB
+rows, JSON payloads, and `==` comparisons all work without translation
+at the edge.
 
-When a coordinator calls `list_skills`, the SQL filter narrows to
-`kind IN ('coordinator', 'any')`.  When an interactive session picks
-a skill at activation, the filter narrows to
-`kind IN ('interactive', 'any')`.  A skill author tags once at
-creation; the two surfaces stay partitioned without any
-per-call filtering on the LLM side.
+**`kind` is metadata, not an enforcement boundary.**  The model can
+`skills(action='find')` across every kind from any session, `get` any
+row by name, and `load` any visible skill regardless of session kind.
+Actual runtime capability is gated by `allowed_tools` + `auto_approve`
+on the skill and the operator's approval card on every `load` /
+`spawn_workstream(skill=...)` decision — `kind` doesn't add or remove
+any of that.  It's a sorting / grouping / search-narrowing hint.
 
-**Tagging a new skill as coordinator-only** — set `kind` to
-`SkillKind.COORDINATOR` (or the literal string `"coordinator"`) when
-you POST to `/v1/api/admin/skills`.  Existing rows default to
-`SkillKind.ANY`; bump them to `COORDINATOR` if you've rewritten the
-prompt around the orchestrator toolset.
+The opt-in filter is on `skills(action='find', kind='coordinator')`
+(or `'interactive'`) — pass it when you want to narrow a catalog
+browse to a specific authored audience.  Omitting it (or passing
+`kind='any'`) returns the full catalog.  When supplied, the storage
+filter widens to `[<kind>, 'any']` so audience-neutral rows remain
+visible inside the narrowed view.
+
+**Tagging a new skill as coordinator-targeted** — set `kind` to
+`SkillKind.COORDINATOR` (or the literal `"coordinator"`) when you
+`skills(action='create', kind='coordinator', ...)` or POST to
+`/v1/api/admin/skills`.  Use this to signal intent to other skill
+authors and to make the orchestrator-targeted catalog easy to
+browse — not to hide the skill from interactive sessions.  Existing
+rows default to `SkillKind.ANY`; bump them to `COORDINATOR` if
+you've rewritten the prompt around the orchestrator toolset and
+want the kind filter to surface them as such.
 
 ---
 
@@ -64,7 +76,7 @@ or MCP config can do adds to it.  Current members:
 | `cancel_workstream`       | wind-down       | Drop the in-flight generation; leaves child idle for a fresh send.  |
 | `delete_workstream`       | wind-down       | Hard-delete one child.  Requires approval.                          |
 | `list_nodes`              | discover        | Enumerate live cluster nodes + capabilities.                        |
-| `list_skills`             | discover        | Coordinator-visible skills only (SkillKind filter above).           |
+| `skills` (action=find)    | discover        | Browse the skill catalog; opt-in `kind` filter narrows by audience. |
 | `tasks`                   | plan            | Orchestrator-only scratchpad.  Children don't see it.               |
 
 Explicitly **not** in the coordinator set:

pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "hatchling.build"
 
 [project]
 name = "turnstone"
-version = "1.6.0a2"
+version = "1.6.0a3"
 description = "Multi-node AI orchestration platform with tool use, agent routing, and cluster simulation."
 readme = "README.md"
 license = "BUSL-1.1"
@@ -25,7 +25,7 @@ dependencies = [
     "openai>=2.37",
     "httpx>=0.28",
     "mcp>=1.27",
-    "starlette>=0.45",
+    "starlette>=1.0.1",  # PYSEC-2026-161: host-header path-injection in URL reconstruction (auth-bypass on apps comparing reconstructed URL paths)
     "uvicorn>=0.34",
     "sse-starlette>=2.0",
     "httpx-sse>=0.4",

tests/test_app_js.py

@@ -1205,3 +1205,195 @@ def test_dead_sse_defensive_reconnect_registered() -> None:
         "_reconnectDeadSSEs must reconnect the global SSE when closed."
     )
     assert "connectSSE(" in helper_body, "_reconnectDeadSSEs must reconnect dead per-pane SSEs."
+
+
+# ---------------------------------------------------------------------------
+# PR-D reconnect-with-replay: onerror must preserve native EventSource
+# auto-reconnect for transient errors
+# ---------------------------------------------------------------------------
+#
+# PR-D adds a server-side per-ws ring buffer + ``Last-Event-ID`` replay so
+# a brief disconnect transparently replays the missed events.  That whole
+# foundation is defeated if the browser's ``onerror`` handler explicitly
+# closes the EventSource on a transient network error — closing forces a
+# CONNECTING -> CLOSED state transition that prevents native auto-reconnect
+# from firing.  The post-PR-D contract is: never call ``.close()`` on a
+# transient error; let native reconnect run with the ``Last-Event-ID``
+# header.  Explicit closes survive only on terminal branches (401 expired
+# session, workstream-reassignment to a different ws).  These guards pin
+# the contract so a future refactor can't silently regress it.
+
+
+def _strip_js_comments(src: str) -> str:
+    """Strip ``//`` and ``/* */`` comments while preserving string
+    literal contents (``"..."``, ``'...'``, `` `...` ``) and keeping
+    byte length identical (comments replaced with spaces).
+
+    Limitation — does NOT detect regex literals (``/pattern/flags``).
+    A ``//`` inside a regex like ``/abc//`` would be misread as the
+    start of a line comment.  Safe today because the regions we scan
+    (SSE-handler ``onerror`` bodies, ``connectSSE`` /
+    ``connectGlobalSSE`` function bodies) don't contain regex
+    literals; if a future caller wants to scan a region with regex
+    literals, extend the tracker first.
+
+    Motivation: ``_slice_balanced_body`` doesn't skip comments, so an
+    apostrophe inside a comment (``can't``, ``don't``) opens a fake
+    string state that swallows braces until the next ``'``.  The new
+    onerror handlers carry these comments routinely; stripping
+    comments before brace-walking removes the hazard without
+    re-architecting the existing slice helper.
+    """
+    out: list[str] = []
+    n = len(src)
+    i = 0
+    in_str: str | None = None
+    while i < n:
+        ch = src[i]
+        if in_str:
+            out.append(ch)
+            if ch == "\\" and i + 1 < n:
+                out.append(src[i + 1])
+                i += 2
+                continue
+            if ch == in_str:
+                in_str = None
+            i += 1
+            continue
+        # Line comment: replace with spaces up to newline (preserve
+        # length so downstream offset math still works).
+        if ch == "/" and i + 1 < n and src[i + 1] == "/":
+            j = src.find("\n", i)
+            if j == -1:
+                j = n
+            out.append(" " * (j - i))
+            i = j
+            continue
+        # Block comment: replace with spaces up to closing */.
+        if ch == "/" and i + 1 < n and src[i + 1] == "*":
+            j = src.find("*/", i + 2)
+            if j == -1:
+                out.append(" " * (n - i))
+                i = n
+                continue
+            out.append(" " * (j + 2 - i))
+            i = j + 2
+            continue
+        if ch in ('"', "'", "`"):
+            in_str = ch
+        out.append(ch)
+        i += 1
+    return "".join(out)
+
+
+def _onerror_block(body: str, anchor_substring: str) -> str | None:
+    """Slice an ``X.onerror = ...`` handler body by matching braces.
+
+    ``anchor_substring`` is something that uniquely identifies the
+    enclosing function so we don't accidentally pick the wrong
+    ``.onerror = function ...`` (the file has several).  Returns the
+    body between the matching braces, or ``None`` if not found.
+    Strips comments first so apostrophes in comment prose can't
+    desync the brace walker.
+    """
+    stripped = _strip_js_comments(body)
+    anchor = stripped.find(anchor_substring)
+    if anchor == -1:
+        return None
+    onerror = stripped.find(".onerror", anchor)
+    if onerror == -1:
+        return None
+    return _slice_balanced_body(stripped, onerror)
+
+
+def _onerror_preserves_native_reconnect(body: str, source_var: str) -> tuple[bool, str]:
+    """Return (passed, reason).
+
+    ``source_var`` is the EventSource handle (e.g. ``this.evtSource``,
+    ``globalEvtSource``, ``evtSource``).  An onerror handler passes if:
+      1. Either it never calls ``source_var.close()`` directly OR every
+         such close is inside a 401-detection branch / login-overlay
+         early-return / wsId-reassignment branch (allowed terminal
+         exits).
+      2. OR the handler explicitly references ``last_event_id`` —
+         escape hatch for a future redesign that abandons native
+         reconnect entirely but takes explicit responsibility for the
+         replay header.
+    """
+    # If the body threads last_event_id, the implementer has taken
+    # explicit responsibility for the replay header — escape hatch.
+    if "last_event_id" in body or "lastEventId" in body:
+        # Caller still has to ensure the body doesn't ALSO have a
+        # naked close() outside a terminal branch; rely on the regex
+        # search below as well.
+        pass
+    # Walk lines, track depth of common terminal branches.  Simple
+    # heuristic: any ``source_var.close()`` line that isn't preceded by
+    # ``status === 401`` or ``loginOverlay`` or ``disconnectSSE()`` in
+    # the surrounding line window is a defect.
+    pattern = re.compile(
+        re.escape(source_var) + r"\.close\(\s*\)",
+    )
+    matches = list(pattern.finditer(body))
+    if not matches:
+        return True, "no close() calls — native reconnect preserved"
+    for m in matches:
+        start = m.start()
+        # Look back ~400 chars for a terminal-branch marker on the
+        # same conditional path.  ``r.status === 401`` is the canonical
+        # 401-detection guard; ``loginOverlay`` is the login-modal
+        # early-return; ``disconnectSSE()`` immediately followed by
+        # setting a new wsId is the reassignment path.
+        window = body[max(0, start - 400) : start]
+        is_401_branch = "status === 401" in window or "r.status === 401" in window
+        is_login_branch = "loginOverlay" in window
+        is_reassign_branch = "disconnectSSE()" in window
+        if not (is_401_branch or is_login_branch or is_reassign_branch):
+            snippet = body[max(0, start - 80) : min(len(body), start + 80)]
+            return False, (
+                f"naked {source_var}.close() at offset {start} — would "
+                f"defeat native auto-reconnect for transient errors. "
+                f"Context: ...{snippet}..."
+            )
+    return True, "all close() calls are in terminal branches (401 / login / reassign)"
+
+
+def test_pane_connectsse_onerror_preserves_native_reconnect() -> None:
+    """``Pane.connectSSE``'s onerror must not close evtSource on
+    transient errors — PR-D's reconnect-with-replay depends on native
+    EventSource auto-reconnect firing with the ``Last-Event-ID`` header."""
+    body = _strip_js_comments(_APP_JS.read_text(encoding="utf-8"))
+    # Slice the Pane.connectSSE method body, then the onerror handler
+    # inside it.  Reuse the indent-agnostic class-method finder.
+    method_start = _pane_method_offset(body, "connectSSE")
+    method = _slice_balanced_body(body, method_start)
+    assert method is not None, "Pane.connectSSE method body not found"
+    # ``_onerror_block`` re-strips internally; passing the already-
+    # stripped method body is idempotent (no comments left to strip).
+    onerror = _onerror_block(method, "this.evtSource.onerror")
+    assert onerror is not None, "Pane.connectSSE.onerror not found"
+    passed, reason = _onerror_preserves_native_reconnect(onerror, "this.evtSource")
+    assert passed, f"Pane.connectSSE.onerror regressed: {reason}"
+
+
+def test_connectglobalsse_onerror_preserves_native_reconnect() -> None:
+    """``connectGlobalSSE`` is the global-SSE counterpart of
+    Pane.connectSSE — same close-defeats-reconnect contract."""
+    body = _strip_js_comments(_APP_JS.read_text(encoding="utf-8"))
+    fn = _slice_function_body(body, "connectGlobalSSE")
+    assert fn is not None, "connectGlobalSSE not found"
+    onerror = _onerror_block(fn, "globalEvtSource.onerror")
+    assert onerror is not None, "globalEvtSource.onerror not found"
+    passed, reason = _onerror_preserves_native_reconnect(onerror, "globalEvtSource")
+    assert passed, f"connectGlobalSSE.onerror regressed: {reason}"
+
+
+def test_coord_connectsse_onerror_preserves_native_reconnect() -> None:
+    """Coordinator's connectSSE has the same contract — without the
+    guard the coord's per-ws SSE silently drops events on any blip."""
+    coord_js = _REPO_ROOT / "turnstone/console/static/coordinator/coordinator.js"
+    body = _strip_js_comments(coord_js.read_text(encoding="utf-8"))
+    onerror = _onerror_block(body, "evtSource.onerror")
+    assert onerror is not None, "coordinator.js evtSource.onerror not found"
+    passed, reason = _onerror_preserves_native_reconnect(onerror, "evtSource")
+    assert passed, f"coordinator.js connectSSE.onerror regressed: {reason}"

tests/test_auth.py

@@ -1901,3 +1901,144 @@ class TestRequirePermissionServiceScope:
         result = require_permission(request, "admin.users")
         assert result is not None
         assert result.status_code == 401
+
+
+# ---------------------------------------------------------------------------
+# TestUserHasPermission — in-process permission check for tool exec paths
+# ---------------------------------------------------------------------------
+
+
+class TestUserHasPermission:
+    """In-process permission helper for model-facing tool exec paths.
+
+    Distinct from ``require_permission`` (HTTP-only, returns JSONResponse);
+    this helper returns a plain bool so tool callers can shape the denial
+    themselves.  Loads permissions through storage on every call — there's
+    no per-session cache, by design: a permission revocation should take
+    effect on the next tool call, not require a session restart.
+    """
+
+    def test_returns_true_when_user_holds_permission(self):
+        from turnstone.core.auth import user_has_permission
+
+        storage = MagicMock()
+        storage.get_user_permissions.return_value = {"model.skills.write", "read"}
+        assert user_has_permission("alice", "model.skills.write", storage=storage) is True
+
+    def test_returns_false_when_user_lacks_permission(self):
+        from turnstone.core.auth import user_has_permission
+
+        storage = MagicMock()
+        storage.get_user_permissions.return_value = {"read", "write"}
+        assert user_has_permission("alice", "model.skills.write", storage=storage) is False
+
+    def test_empty_user_id_returns_false_without_storage_lookup(self):
+        """Empty user_id short-circuits — no anonymous permission holder."""
+        from turnstone.core.auth import user_has_permission
+
+        storage = MagicMock()
+        assert user_has_permission("", "model.skills.write", storage=storage) is False
+        storage.get_user_permissions.assert_not_called()
+
+    def test_storage_failure_returns_false_fail_closed(self):
+        """Roles backend hiccups must deny, not allow (fail-closed)."""
+        from turnstone.core.auth import user_has_permission
+
+        storage = MagicMock()
+        storage.get_user_permissions.side_effect = RuntimeError("DB down")
+        assert user_has_permission("alice", "model.skills.write", storage=storage) is False
+
+    def test_unregistered_storage_returns_false(self, monkeypatch):
+        """Storage registry returning None (pre-init) denies without raising.
+
+        Only the model-tool path can land here — HTTP handlers run after
+        the auth middleware which already requires storage.
+        """
+        from turnstone.core import auth as _auth_mod
+
+        monkeypatch.setattr(
+            "turnstone.core.storage._registry.get_storage", lambda: None, raising=True
+        )
+        assert _auth_mod.user_has_permission("alice", "model.skills.write") is False
+
+    def test_each_call_hits_storage_no_implicit_cache(self):
+        """Pin the load-bearing 'no caching' contract from the class docstring.
+
+        Future refactor that adds an ``lru_cache`` decorator, a per-session
+        cache, or any process-wide memoization would silently break
+        revocation latency (an admin revoking ``model.skills.write`` from a
+        role would see the model still able to write skills until cache
+        expiry / session restart).  If a cache is added intentionally, this
+        test should be rewritten to assert the invalidation contract — not
+        deleted.
+        """
+        from turnstone.core.auth import user_has_permission
+
+        storage = MagicMock()
+        storage.get_user_permissions.return_value = {"model.skills.write"}
+        user_has_permission("alice", "model.skills.write", storage=storage)
+        user_has_permission("alice", "model.skills.write", storage=storage)
+        assert storage.get_user_permissions.call_count == 2
+
+
+# ---------------------------------------------------------------------------
+# TestBuiltinAdminDefaultPermissions — lock the "ungranted by default" invariant
+# ---------------------------------------------------------------------------
+
+
+class TestBuiltinAdminDefaultPermissions:
+    """Regression guards on what builtin-admin gets out of the box.
+
+    The migration chain (008 seed + 017 catch-up + later additive
+    migrations) is the source of truth for builtin-admin's permission
+    set.  Permissions intentionally ungranted by default — currently
+    ``model.skills.write`` — must stay absent from that chain, or
+    operators upgrading from older versions silently inherit a
+    capability they never consented to.  Mirrors the
+    ``tests/test_migration_049.py`` pattern: drive Alembic forward
+    against an isolated SQLite DB and inspect the resulting row.
+    """
+
+    def _alembic_cfg(self, db_path):
+        from pathlib import Path
+
+        from alembic.config import Config
+
+        migrations_dir = str(
+            Path(__file__).resolve().parent.parent / "turnstone" / "core" / "storage" / "migrations"
+        )
+        cfg = Config()
+        cfg.set_main_option("script_location", migrations_dir)
+        cfg.set_main_option("sqlalchemy.url", f"sqlite:///{db_path}")
+        return cfg
+
+    def test_model_skills_write_not_in_builtin_admin_after_full_migration(self, tmp_path):
+        """After every shipped migration, ``builtin-admin.permissions`` must
+        not contain ``model.skills.write``.  A migration that grants it
+        breaks the explicit-opt-in security contract documented in the
+        ``_VALID_PERMISSIONS`` block in ``console/server.py``.
+        """
+        import sqlalchemy as sa
+        from alembic import command
+
+        db_path = tmp_path / "perm.db"
+        cfg = self._alembic_cfg(db_path)
+        command.upgrade(cfg, "head")
+
+        engine = sa.create_engine(f"sqlite:///{db_path}")
+        try:
+            with engine.connect() as conn:
+                row = conn.execute(
+                    sa.text("SELECT permissions FROM roles WHERE role_id = 'builtin-admin'")
+                ).fetchone()
+        finally:
+            engine.dispose()
+
+        assert row is not None, "builtin-admin role not seeded by migration chain"
+        perms = {p.strip() for p in (row[0] or "").split(",") if p.strip()}
+        assert "model.skills.write" not in perms, (
+            "builtin-admin must NOT hold model.skills.write by default — "
+            f"got perms={sorted(perms)}.  If a migration intentionally "
+            "added this grant, update the security contract in "
+            "``console/server.py`` _VALID_PERMISSIONS docstring first."
+        )

tests/test_coordinator_client.py

@@ -1085,236 +1085,6 @@ def test_list_nodes_models_handles_non_list_payload(tmp_path):
     assert result["nodes"][0]["model_aliases"] == []
 
 
-# ---------------------------------------------------------------------------
-# list_skills
-# ---------------------------------------------------------------------------
-
-
-@pytest.fixture
-def storage_with_skills(tmp_path):
-    st = SQLiteBackend(str(tmp_path / "skills.db"))
-    st.create_prompt_template(
-        template_id="s1",
-        name="alpha",
-        category="ops",
-        content="",
-        variables="[]",
-        is_default=False,
-        org_id="",
-        created_by="test",
-        tags='["gpu", "fast"]',
-    )
-    st.create_prompt_template(
-        template_id="s2",
-        name="beta",
-        category="engineering",
-        content="",
-        variables="[]",
-        is_default=False,
-        org_id="",
-        created_by="test",
-        tags='["slow"]',
-    )
-    st.create_prompt_template(
-        template_id="s3",
-        name="gamma",
-        category="engineering",
-        content="",
-        variables="[]",
-        is_default=False,
-        org_id="",
-        created_by="test",
-        tags="[]",
-        enabled=False,
-    )
-    return st
-
-
-def test_list_skills_returns_shape(storage_with_skills):
-    client = _make_read_client(storage_with_skills)
-    result = client.list_skills()
-    assert set(result.keys()) == {"skills", "truncated"}
-    names = {s["name"] for s in result["skills"]}
-    assert names == {"alpha", "beta", "gamma"}
-    # Tags decoded to a list, not a string.
-    alpha = next(s for s in result["skills"] if s["name"] == "alpha")
-    assert alpha["tags"] == ["gpu", "fast"]
-    # Discovery projection only — not full row.
-    assert "content" not in alpha
-
-
-def test_list_skills_pushes_filters_to_storage_no_per_row_lookups(storage_with_skills, monkeypatch):
-    called = []
-    real_get = storage_with_skills.get_prompt_template
-
-    def _spy(tid):  # type: ignore[no-untyped-def]
-        called.append(tid)
-        return real_get(tid)
-
-    monkeypatch.setattr(storage_with_skills, "get_prompt_template", _spy)
-
-    client = _make_read_client(storage_with_skills)
-    result = client.list_skills(tag="gpu")
-    assert {s["name"] for s in result["skills"]} == {"alpha"}
-    assert called == []  # no N+1
-
-
-def test_list_skills_enabled_only(storage_with_skills):
-    client = _make_read_client(storage_with_skills)
-    result = client.list_skills(enabled_only=True)
-    names = {s["name"] for s in result["skills"]}
-    assert names == {"alpha", "beta"}  # gamma is disabled
-
-
-def test_list_skills_truncation_signal(storage_with_skills):
-    client = _make_read_client(storage_with_skills)
-    result = client.list_skills(limit=2)
-    assert len(result["skills"]) == 2
-    assert result["truncated"] is True
-
-
-def test_list_skills_hides_interactive_only_skills(tmp_path):
-    """CoordinatorClient.list_skills must narrow the storage query to
-    ``kinds=['coordinator', 'any']`` so interactive-only skills (which
-    are meant for child workstreams) don't pollute the orchestrator's
-    tool surface.  Regression lock for a load-bearing invariant that
-    the fixture-based tests above can't exercise because their skills
-    all default to ``kind='any'``."""
-    st = SQLiteBackend(str(tmp_path / "kinds.db"))
-    st.create_prompt_template(
-        template_id="k1",
-        name="interactive-only",
-        category="general",
-        content="",
-        variables="[]",
-        is_default=False,
-        org_id="",
-        created_by="test",
-        description="interactive only",
-        kind="interactive",
-    )
-    st.create_prompt_template(
-        template_id="k2",
-        name="coord-only",
-        category="general",
-        content="",
-        variables="[]",
-        is_default=False,
-        org_id="",
-        created_by="test",
-        description="coordinator only",
-        kind="coordinator",
-    )
-    st.create_prompt_template(
-        template_id="k3",
-        name="universal",
-        category="general",
-        content="",
-        variables="[]",
-        is_default=False,
-        org_id="",
-        created_by="test",
-        description="everywhere",
-        kind="any",
-    )
-
-    client = _make_read_client(st)
-    result = client.list_skills()
-    names = {s["name"] for s in result["skills"]}
-    assert "interactive-only" not in names
-    assert names == {"coord-only", "universal"}
-    # And the kind projection comes through on every returned row.
-    for skill in result["skills"]:
-        assert skill["kind"] in {"coordinator", "any"}
-
-
-def test_list_skills_omits_allowed_tools_when_empty(tmp_path):
-    """``allowed_tools`` is the auto-approve allowlist (tools exempt
-    from the operator approval gate), NOT the set of tools the skill
-    can use.  An empty list reads as "no tool access" to a model
-    that doesn't know the semantics — real misdiagnosis source: a
-    code-review skill with no auto-approve allowlist looked like it
-    had been spawned with zero tools.  Dropping the key when empty
-    removes the ambiguity at the source; absence of the field carries
-    the unambiguous meaning "no tool is pre-approved for this skill"
-    while a tool list reads as "these specific tools bypass the prompt".
-    """
-    st = SQLiteBackend(str(tmp_path / "skills_empty.db"))
-    st.create_prompt_template(
-        template_id="s-empty",
-        name="empty-skill",
-        category="ops",
-        content="",
-        variables="[]",
-        is_default=False,
-        org_id="",
-        created_by="test",
-        tags="[]",
-        allowed_tools="[]",
-    )
-    st.create_prompt_template(
-        template_id="s-nonempty",
-        name="nonempty-skill",
-        category="ops",
-        content="",
-        variables="[]",
-        is_default=False,
-        org_id="",
-        created_by="test",
-        tags="[]",
-        allowed_tools='["read_file"]',
-    )
-    client = _make_read_client(st)
-    result = client.list_skills()
-    by_name = {s["name"]: s for s in result["skills"]}
-    assert "allowed_tools" not in by_name["empty-skill"]
-    assert by_name["nonempty-skill"]["allowed_tools"] == ["read_file"]
-
-
-def test_list_skills_projects_allowed_tools_capped_with_sentinel(tmp_path):
-    """Each row carries the skill's allowed_tools (capped at the projection
-    cap with a +N more sentinel) so coordinators can pick a skill without
-    speculating which tools it brings.  The cap keeps the per-row payload
-    bounded for skills that whitelist a wide MCP surface."""
-    from turnstone.console.coordinator_client import _SKILL_TOOLS_PROJECTION_CAP
-
-    st = SQLiteBackend(str(tmp_path / "skills_tools.db"))
-    st.create_prompt_template(
-        template_id="s-short",
-        name="short-skill",
-        category="ops",
-        content="",
-        variables="[]",
-        is_default=False,
-        org_id="",
-        created_by="test",
-        tags="[]",
-        allowed_tools='["read_file", "search"]',
-    )
-    long_tools = [f"tool_{i:03d}" for i in range(_SKILL_TOOLS_PROJECTION_CAP + 7)]
-    st.create_prompt_template(
-        template_id="s-long",
-        name="long-skill",
-        category="ops",
-        content="",
-        variables="[]",
-        is_default=False,
-        org_id="",
-        created_by="test",
-        tags="[]",
-        allowed_tools=json.dumps(long_tools),
-    )
-    client = _make_read_client(st)
-    result = client.list_skills()
-    by_name = {s["name"]: s for s in result["skills"]}
-    assert by_name["short-skill"]["allowed_tools"] == ["read_file", "search"]
-    long_skill = by_name["long-skill"]["allowed_tools"]
-    # Cap items + 1 sentinel.
-    assert len(long_skill) == _SKILL_TOOLS_PROJECTION_CAP + 1
-    assert long_skill[-1] == f"+{7} more"
-    assert long_skill[0] == "tool_000"
-
-
 # ---------------------------------------------------------------------------
 # inspect — close_reason + token fallback
 # ---------------------------------------------------------------------------

tests/test_coordinator_tools.py

@@ -136,7 +136,6 @@ def test_coordinator_session_uses_coordinator_tools(coord_session):
         "delete_workstream",
         "list_workstreams",
         "list_nodes",
-        "list_skills",
         "tasks",
         "wait_for_workstream",
         # Memory is dual-kind (coordinator: true + interactive: true) so
@@ -146,6 +145,10 @@ def test_coordinator_session_uses_coordinator_tools(coord_session):
         # without this the model would see memories listed but no tool
         # to act on them.
         "memory",
+        # ``skills`` replaced ``list_skills`` in the 1.6.0 tool unification.
+        # Dual-kind (interactive + coordinator) — read actions auto-approve
+        # on both; writes gate on ``model.skills.write``.
+        "skills",
     }
     # Sub-agent tool sets are zeroed on coordinator sessions.
     assert sess._task_tools == []
@@ -626,6 +629,9 @@ def test_prepare_fails_cleanly_when_coord_client_missing(monkeypatch):
         kind="coordinator",
         coord_client=None,
     )
+    # ``skills`` is excluded here on purpose: it's dual-kind and talks
+    # directly to storage, so it has no coord_client dependency and
+    # legitimately prepares without erroring when coord_client is absent.
     for tool, args in (
         ("spawn_workstream", {"initial_message": "hi"}),
         ("inspect_workstream", {"ws_id": "x"}),
@@ -634,7 +640,6 @@ def test_prepare_fails_cleanly_when_coord_client_missing(monkeypatch):
         ("delete_workstream", {"ws_id": "x"}),
         ("list_workstreams", {}),
         ("list_nodes", {}),
-        ("list_skills", {}),
         ("tasks", {"action": "list"}),
     ):
         item = sess._prepare_tool(_tc(tool, args))
@@ -804,102 +809,6 @@ def test_list_nodes_exec_surfaces_truncated_sentinel(coord_session):
     assert any("truncated" in r[2] for r in ui.tool_results)
 
 
-# ---------------------------------------------------------------------------
-# list_skills
-# ---------------------------------------------------------------------------
-
-
-def test_list_skills_prepare_is_auto_approved(coord_session):
-    sess, _coord, _ui = coord_session
-    item = sess._prepare_tool(_tc("list_skills", {}))
-    assert item["needs_approval"] is False
-    assert item["category"] is None
-    assert item["tag"] is None
-    assert item["risk_level"] is None
-    assert item["enabled_only"] is False
-    assert item["limit"] == 100
-
-
-def test_list_skills_prepare_accepts_filters(coord_session):
-    sess, _coord, _ui = coord_session
-    item = sess._prepare_tool(
-        _tc(
-            "list_skills",
-            {"category": "ops", "tag": "gpu", "risk_level": "clean", "enabled_only": True},
-        )
-    )
-    assert item["category"] == "ops"
-    assert item["tag"] == "gpu"
-    assert item["risk_level"] == "clean"
-    assert item["enabled_only"] is True
-
-
-def test_list_skills_prepare_tolerates_non_string_filters(coord_session):
-    """A malformed model call with non-string filter values must NOT
-    raise AttributeError during ``.strip()`` — the prepare path should
-    coerce non-strings to ``None`` and proceed."""
-    sess, _coord, _ui = coord_session
-    item = sess._prepare_tool(
-        _tc(
-            "list_skills",
-            {"category": 42, "tag": ["not", "a", "string"], "risk_level": {"bad": 1}},
-        )
-    )
-    assert "error" not in item
-    assert item["category"] is None
-    assert item["tag"] is None
-    assert item["risk_level"] is None
-
-
-def test_list_skills_prepare_parses_enabled_only_string_forms(coord_session):
-    """``bool("false")`` is True (non-empty string).  The prepare path
-    must interpret common string forms the way the model would expect."""
-    sess, _coord, _ui = coord_session
-    for raw, expected in (
-        ("true", True),
-        ("True", True),
-        ("1", True),
-        ("false", False),
-        ("False", False),
-        ("0", False),
-        ("", False),
-        (True, True),
-        (False, False),
-    ):
-        item = sess._prepare_tool(_tc("list_skills", {"enabled_only": raw}))
-        assert item.get("enabled_only") is expected, (
-            f"enabled_only={raw!r} → {item.get('enabled_only')!r}, expected {expected!r}"
-        )
-
-
-def test_list_skills_exec_dispatches_to_client(coord_session):
-    sess, coord, ui = coord_session
-    coord.list_skills.return_value = {
-        "skills": [{"name": "alpha", "tags": ["gpu"]}],
-        "truncated": False,
-    }
-    item = sess._prepare_tool(_tc("list_skills", {"category": "ops", "tag": "gpu"}))
-    call_id, output = sess._exec_list_skills(item)
-    assert call_id == "call-1"
-    parsed = json.loads(output)
-    assert parsed["skills"][0]["name"] == "alpha"
-    coord.list_skills.assert_called_once_with(
-        category="ops",
-        tag="gpu",
-        risk_level=None,
-        enabled_only=False,
-        limit=100,
-    )
-
-
-def test_list_skills_exec_surfaces_truncated_sentinel(coord_session):
-    sess, coord, ui = coord_session
-    coord.list_skills.return_value = {"skills": [], "truncated": True}
-    item = sess._prepare_tool(_tc("list_skills", {}))
-    _, _ = sess._exec_list_skills(item)
-    assert any("truncated" in r[2] for r in ui.tool_results)
-
-
 # ---------------------------------------------------------------------------
 # tasks
 # ---------------------------------------------------------------------------

tests/test_governance_endpoints.py

@@ -205,6 +205,29 @@ class TestRoles:
         assert resp.status_code == 400
         assert "name" in resp.json()["error"].lower()
 
+    def test_create_role_with_model_skills_write_permission(self, client):
+        """``model.skills.write`` is enumerated in ``_VALID_PERMISSIONS`` and
+        passes role-create validation.  Catches the case where the constant
+        is added on the server but missed by the validator or the constant
+        list."""
+        resp = client.post(
+            "/v1/api/admin/roles",
+            json=_role_payload(name="skillwriter", permissions="read,model.skills.write"),
+        )
+        assert resp.status_code == 200, resp.json()
+        assert "model.skills.write" in resp.json()["permissions"]
+
+    def test_create_role_rejects_unknown_permission(self, client):
+        """Unknown permission strings are rejected — guards the validator
+        against typos in the constant list and would-be capability inflation
+        via the admin API."""
+        resp = client.post(
+            "/v1/api/admin/roles",
+            json=_role_payload(name="bogus", permissions="read,model.does.not.exist"),
+        )
+        assert resp.status_code == 400
+        assert "invalid" in resp.json()["error"].lower()
+
     def test_create_role_default_display_name(self, client):
         resp = client.post(
             "/v1/api/admin/roles",

tests/test_load_skill.py

@@ -1,495 +0,0 @@
-"""Tests for the skill built-in tool."""
-
-from __future__ import annotations
-
-from typing import Any
-from unittest.mock import MagicMock, patch
-
-from turnstone.core.tools import BUILTIN_TOOL_NAMES, PRIMARY_KEY_MAP
-
-
-class TestToolRegistration:
-    """Verify skill is registered correctly."""
-
-    def test_in_builtin_tool_names(self) -> None:
-        assert "skill" in BUILTIN_TOOL_NAMES
-
-    def test_not_agent_tool(self) -> None:
-        from turnstone.core.tools import AGENT_TOOLS
-
-        names = {t["function"]["name"] for t in AGENT_TOOLS}
-        assert "skill" not in names
-
-    def test_not_task_agent_tool(self) -> None:
-        from turnstone.core.tools import TASK_AGENT_TOOLS
-
-        names = {t["function"]["name"] for t in TASK_AGENT_TOOLS}
-        assert "skill" not in names
-
-    def test_has_primary_key(self) -> None:
-        assert PRIMARY_KEY_MAP.get("skill") == "name"
-
-
-# ---------------------------------------------------------------------------
-# Helpers — minimal ChatSession mock
-# ---------------------------------------------------------------------------
-
-
-def _make_session(skills: list[dict[str, Any]] | None = None):
-    """Build a minimal ChatSession with stubbed storage."""
-    from turnstone.core.session import ChatSession
-
-    ui = MagicMock()
-    session = ChatSession.__new__(ChatSession)
-
-    # Minimal state required by the methods under test
-    session.ui = ui
-    session.model = "test-model"
-    session._ws_id = "ws-test"
-    session._node_id = "node-1"
-    session._skill_name = None
-    session._skill_content = None
-    session._applied_skill_content = None
-    session.context_window = 128000
-    session._notify_on_complete = "{}"
-    session.messages = []
-    session._config = {}
-    session._tool_error_flags = {}
-
-    # Stub set_skill to just record the call
-    session._set_skill_called: list[str | None] = []
-
-    def fake_set_skill(name):
-        session._set_skill_called.append(name)
-        session._skill_name = name
-
-    session.set_skill = fake_set_skill
-
-    # Storage mock
-    _skills = skills or []
-
-    def fake_get_skill_by_name(name):
-        for s in _skills:
-            if s.get("name") == name:
-                return s
-        return None
-
-    return session, _skills, fake_get_skill_by_name
-
-
-# ---------------------------------------------------------------------------
-# Tests: Preparer
-# ---------------------------------------------------------------------------
-
-
-class TestPrepareLoadSkill:
-    """Test _prepare_skill validation and item dict shape."""
-
-    def test_load_valid(self) -> None:
-        session, _, _ = _make_session()
-        item = session._prepare_skill("call-1", {"action": "load", "name": "code-review"})
-        assert item["func_name"] == "skill"
-        assert item["action"] == "load"
-        assert item["name"] == "code-review"
-        assert item["needs_approval"] is True
-        assert "execute" in item
-        assert "error" not in item
-
-    def test_load_missing_name(self) -> None:
-        session, _, _ = _make_session()
-        item = session._prepare_skill("call-1", {"action": "load"})
-        assert "error" in item
-        assert "name" in item["error"].lower()
-        assert item["needs_approval"] is False
-
-    def test_load_empty_name(self) -> None:
-        session, _, _ = _make_session()
-        item = session._prepare_skill("call-1", {"action": "load", "name": ""})
-        assert "error" in item
-
-    def test_search_with_query(self) -> None:
-        session, _, _ = _make_session()
-        item = session._prepare_skill("call-1", {"action": "search", "query": "code review"})
-        assert item["action"] == "search"
-        assert item["query"] == "code review"
-        assert item["needs_approval"] is False
-        assert "execute" in item
-
-    def test_search_without_query(self) -> None:
-        session, _, _ = _make_session()
-        item = session._prepare_skill("call-1", {"action": "search"})
-        assert item["action"] == "search"
-        assert item["query"] == ""
-        assert item["needs_approval"] is False
-
-    def test_invalid_action(self) -> None:
-        session, _, _ = _make_session()
-        item = session._prepare_skill("call-1", {"action": "delete"})
-        assert "error" in item
-        assert "delete" in item["error"]
-
-    def test_empty_action(self) -> None:
-        session, _, _ = _make_session()
-        item = session._prepare_skill("call-1", {"action": ""})
-        assert "error" in item
-
-    def test_header_for_load(self) -> None:
-        session, _, _ = _make_session()
-        item = session._prepare_skill("call-1", {"action": "load", "name": "my-skill"})
-        assert "my-skill" in item["header"]
-
-    def test_header_for_search(self) -> None:
-        session, _, _ = _make_session()
-        item = session._prepare_skill("call-1", {"action": "search", "query": "testing"})
-        assert "testing" in item["header"]
-
-
-# ---------------------------------------------------------------------------
-# Tests: Executor
-# ---------------------------------------------------------------------------
-
-
-class TestExecLoadSkill:
-    """Test _exec_skill execution logic."""
-
-    def test_load_existing_skill(self) -> None:
-        skills = [
-            {
-                "name": "code-review",
-                "description": "Reviews code for quality",
-                "content": "# Code Review\nReview all code.",
-                "risk_level": "safe",
-                "category": "engineering",
-            }
-        ]
-        session, _, fake_get = _make_session(skills)
-
-        with patch("turnstone.core.session.get_skill_by_name", side_effect=fake_get):
-            item = session._prepare_skill("call-1", {"action": "load", "name": "code-review"})
-            call_id, result = session._exec_skill(item)
-
-        assert call_id == "call-1"
-        assert "code-review" in result
-        assert "Reviews code" in result
-        assert "safe" in result
-        assert session._set_skill_called == ["code-review"]
-
-    def test_load_nonexistent_skill(self) -> None:
-        session, _, fake_get = _make_session([])
-
-        with patch("turnstone.core.session.get_skill_by_name", side_effect=fake_get):
-            item = session._prepare_skill("call-1", {"action": "load", "name": "nope"})
-            call_id, result = session._exec_skill(item)
-
-        assert "not found" in result.lower()
-        assert session._set_skill_called == []
-
-    def test_load_calls_ui_on_tool_result(self) -> None:
-        skills = [{"name": "test", "content": "content", "description": "", "risk_level": ""}]
-        session, _, fake_get = _make_session(skills)
-
-        with patch("turnstone.core.session.get_skill_by_name", side_effect=fake_get):
-            item = session._prepare_skill("call-1", {"action": "load", "name": "test"})
-            session._exec_skill(item)
-
-        session.ui.on_tool_result.assert_called_once()
-
-    def test_search_returns_results(self) -> None:
-        skills = [
-            {
-                "name": "code-review",
-                "description": "Reviews code",
-                "category": "eng",
-                "risk_level": "safe",
-                "tags": "[]",
-                "activation": "named",
-            },
-            {
-                "name": "docs-writer",
-                "description": "Writes docs",
-                "category": "general",
-                "risk_level": "low",
-                "tags": "[]",
-                "activation": "named",
-            },
-        ]
-        mock_storage = MagicMock()
-        mock_storage.list_prompt_templates.return_value = skills
-
-        session, _, _ = _make_session()
-        item = session._prepare_skill("call-1", {"action": "search", "query": "code"})
-
-        with patch("turnstone.core.storage._registry.get_storage", return_value=mock_storage):
-            call_id, result = session._exec_skill(item)
-
-        assert "code-review" in result
-        # docs-writer shouldn't match "code" query
-        assert "docs-writer" not in result
-
-    def test_search_empty_query_returns_all(self) -> None:
-        skills = [
-            {
-                "name": f"skill-{i}",
-                "description": f"Desc {i}",
-                "category": "general",
-                "risk_level": "",
-                "tags": "[]",
-                "activation": "named",
-            }
-            for i in range(15)
-        ]
-        mock_storage = MagicMock()
-        mock_storage.list_prompt_templates.return_value = skills
-
-        session, _, _ = _make_session()
-        item = session._prepare_skill("call-1", {"action": "search"})
-
-        with patch("turnstone.core.storage._registry.get_storage", return_value=mock_storage):
-            call_id, result = session._exec_skill(item)
-
-        # Should be limited to 10
-        assert result.count("skill-") == 10
-
-    def test_search_no_results(self) -> None:
-        mock_storage = MagicMock()
-        mock_storage.list_prompt_templates.return_value = []
-
-        session, _, _ = _make_session()
-        item = session._prepare_skill("call-1", {"action": "search", "query": "nonexistent"})
-
-        with patch("turnstone.core.storage._registry.get_storage", return_value=mock_storage):
-            call_id, result = session._exec_skill(item)
-
-        assert "no skills found" in result.lower()
-
-    def test_search_includes_risk_level(self) -> None:
-        skills = [
-            {
-                "name": "risky",
-                "description": "Risky skill",
-                "category": "ops",
-                "risk_level": "high",
-                "tags": "[]",
-                "activation": "named",
-            },
-        ]
-        mock_storage = MagicMock()
-        mock_storage.list_prompt_templates.return_value = skills
-
-        session, _, _ = _make_session()
-        item = session._prepare_skill("call-1", {"action": "search", "query": "risky"})
-
-        with patch("turnstone.core.storage._registry.get_storage", return_value=mock_storage):
-            call_id, result = session._exec_skill(item)
-
-        assert "high" in result
-
-    def test_search_storage_failure_returns_empty(self) -> None:
-        session, _, _ = _make_session()
-        item = session._prepare_skill("call-1", {"action": "search", "query": "test"})
-
-        with patch(
-            "turnstone.core.storage._registry.get_storage", side_effect=RuntimeError("no storage")
-        ):
-            call_id, result = session._exec_skill(item)
-
-        assert "no skills found" in result.lower()
-
-    def test_load_disabled_skill_returns_not_found(self) -> None:
-        skills = [
-            {
-                "name": "disabled-skill",
-                "content": "x",
-                "description": "",
-                "risk_level": "",
-                "enabled": False,
-            }
-        ]
-        session, _, fake_get = _make_session(skills)
-
-        with patch("turnstone.core.session.get_skill_by_name", side_effect=fake_get):
-            item = session._prepare_skill("call-1", {"action": "load", "name": "disabled-skill"})
-            call_id, result = session._exec_skill(item)
-
-        assert "not found" in result.lower()
-        assert session._set_skill_called == []
-
-    def test_load_already_active_skill(self) -> None:
-        skills = [{"name": "active", "content": "x", "description": "", "risk_level": "safe"}]
-        session, _, fake_get = _make_session(skills)
-        session._skill_name = "active"
-
-        with patch("turnstone.core.session.get_skill_by_name", side_effect=fake_get):
-            item = session._prepare_skill("call-1", {"action": "load", "name": "active"})
-            call_id, result = session._exec_skill(item)
-
-        assert "already active" in result.lower()
-        assert session._set_skill_called == []
-
-    def test_search_filters_disabled(self) -> None:
-        skills = [
-            {
-                "name": "enabled-skill",
-                "description": "Good",
-                "category": "gen",
-                "risk_level": "",
-                "tags": "[]",
-                "activation": "named",
-                "enabled": True,
-            },
-            {
-                "name": "disabled-skill",
-                "description": "Bad",
-                "category": "gen",
-                "risk_level": "",
-                "tags": "[]",
-                "activation": "named",
-                "enabled": False,
-            },
-        ]
-        mock_storage = MagicMock()
-        mock_storage.list_prompt_templates.return_value = skills
-
-        session, _, _ = _make_session()
-        item = session._prepare_skill("call-1", {"action": "search"})
-
-        with patch("turnstone.core.storage._registry.get_storage", return_value=mock_storage):
-            call_id, result = session._exec_skill(item)
-
-        assert "enabled-skill" in result
-        assert "disabled-skill" not in result
-
-    def test_search_multi_word_query(self) -> None:
-        skills = [
-            {
-                "name": "code-review",
-                "description": "Reviews code for quality",
-                "category": "eng",
-                "risk_level": "",
-                "tags": "[]",
-                "activation": "named",
-            },
-        ]
-        mock_storage = MagicMock()
-        mock_storage.list_prompt_templates.return_value = skills
-
-        session, _, _ = _make_session()
-        item = session._prepare_skill("call-1", {"action": "search", "query": "code review"})
-
-        with patch("turnstone.core.storage._registry.get_storage", return_value=mock_storage):
-            call_id, result = session._exec_skill(item)
-
-        assert "code-review" in result
-
-    def test_preparer_load_has_approval_label(self) -> None:
-        session, _, _ = _make_session()
-        item = session._prepare_skill("call-1", {"action": "load", "name": "my-skill"})
-        assert item["approval_label"] == "skill__my-skill"
-
-
-# ---------------------------------------------------------------------------
-# Tests: Skill Catalog Disclosure (Agent Skills standard compliance)
-# ---------------------------------------------------------------------------
-
-
-class TestSkillCatalogDisclosure:
-    """Verify <available-skills> catalog appears in system messages."""
-
-    def _build_session_with_system_messages(
-        self,
-        search_skills: list[dict[str, Any]] | None = None,
-    ) -> Any:
-        """Build a session and call _init_system_messages to get dev_parts."""
-        from turnstone.core.session import ChatSession
-
-        session = ChatSession.__new__(ChatSession)
-        ui = MagicMock()
-        session.ui = ui
-        session.model = "test-model"
-        session._ws_id = "ws-test"
-        session._node_id = "node-1"
-        session._skill_name = None
-        session._skill_content = None
-        session._skill_resources = {}
-        session._applied_skill_content = None
-        session.context_window = 128000
-        session.messages = []
-        session._config = {}
-        session.creative_mode = False
-        session.instructions = ""
-        session.system_messages = []
-        session._agent_system_messages = []
-        session.reasoning_effort = "medium"
-        from turnstone.core.nudge_queue import NudgeQueue
-
-        session._nudge_queue = NudgeQueue()
-        session._tool_search = None
-        session._mcp_client = None
-        session._notify_on_complete = "{}"
-        session._tool_error_flags = {}
-        from turnstone.prompts import ClientType
-
-        session._tools = []
-        session._client_type = ClientType.CLI
-        session._username = ""
-        session._kind = "interactive"
-
-        # Memory stubs
-        session._memory_config = MagicMock()
-        session._memory_config.fetch_limit = 0
-        session._user_id = "test-user"
-
-        with (
-            patch(
-                "turnstone.core.session.list_skills_by_activation",
-                return_value=search_skills or [],
-            ),
-            patch.object(session, "_list_visible_memories", return_value=[]),
-        ):
-            session._init_system_messages()
-
-        return session
-
-    def test_catalog_present_with_search_skills(self) -> None:
-        skills = [
-            {"name": "pdf-processing", "description": "Extract PDF text and forms."},
-            {"name": "data-analysis", "description": "Analyze datasets."},
-        ]
-        session = self._build_session_with_system_messages(search_skills=skills)
-        content = session.system_messages[0]["content"]
-        assert "<available-skills>" in content
-        assert "pdf-processing" in content
-        assert "data-analysis" in content
-        assert "</available-skills>" in content
-
-    def test_catalog_omitted_when_no_search_skills(self) -> None:
-        session = self._build_session_with_system_messages(search_skills=[])
-        content = session.system_messages[0]["content"]
-        assert "<available-skills>" not in content
-
-    def test_catalog_capped_at_30(self) -> None:
-        skills = [{"name": f"skill-{i:03d}", "description": f"Desc {i}"} for i in range(50)]
-        session = self._build_session_with_system_messages(search_skills=skills)
-        content = session.system_messages[0]["content"]
-        # Should include first 30, not all 50
-        assert "skill-029" in content
-        assert "skill-030" not in content
-
-    def test_catalog_escapes_html(self) -> None:
-        skills = [
-            {"name": "xss-test", "description": "Handle <script> & 'quotes'."},
-        ]
-        session = self._build_session_with_system_messages(search_skills=skills)
-        content = session.system_messages[0]["content"]
-        assert "&lt;script&gt;" in content
-        assert "<script>" not in content.replace("<available-skills>", "").replace(
-            "</available-skills>", ""
-        ).replace("<skill>", "").replace("</skill>", "").replace("<name>", "").replace(
-            "</name>", ""
-        ).replace("<description>", "").replace("</description>", "")
-
-    def test_catalog_includes_hint(self) -> None:
-        skills = [{"name": "test", "description": "Test skill."}]
-        session = self._build_session_with_system_messages(search_skills=skills)
-        content = session.system_messages[0]["content"]
-        assert "/skill" in content

tests/test_renderer_js.py

@@ -16,6 +16,7 @@ placeholder and not the raw delimiter.
 from __future__ import annotations
 
 import json
+import re
 import shutil
 import subprocess
 from pathlib import Path
@@ -1273,3 +1274,192 @@ def test_streaming_render_invokes_hljs() -> None:
         "_streamingRenderApply must call postRenderHljs for progressive "
         "syntax highlighting during streaming"
     )
+
+
+# ---------------------------------------------------------------------------
+# Attribute-context interpolation lint + pin tests
+# ---------------------------------------------------------------------------
+
+# The JS source uses `'...attr="' + var + '"...'` — so the literal text
+# between `=` and `+` is `"` (the HTML-attribute opener inside the
+# JS string) followed by `'` (the JS-string closer). Match that pair,
+# then optional whitespace + `+` + whitespace + an identifier.
+_RENDERER_ATTR_INTERP_RE = re.compile(
+    r"=[\"'][\"']\s*\+\s*(?!escapeHtml\b)([a-zA-Z_][a-zA-Z0-9_]*)"
+)
+
+# Identifiers exempted from the lint. Each entry is reviewer-approved
+# as known-safe; adding a new one requires a comment explaining why.
+_RENDERER_KNOWN_SAFE_IDENTIFIERS = {
+    # CALLOUT_TYPES enum lookup ({label, icon} of fixed strings — Note,
+    # Tip, Important, Warning, Caution). `alertType` matched by regex
+    # /(NOTE|TIP|IMPORTANT|WARNING|CAUTION)/, so .toLowerCase() output
+    # is also a fixed set; flows through `info`.
+    "info",
+}
+
+
+def test_renderer_attribute_context_interpolation_is_safe() -> None:
+    """Pin: every `attr="' + var` string-concat interpolation in
+    renderer.js must use one of:
+
+    * `escapeHtml(...)` at the call site (allowed by the negative
+      lookahead in the regex),
+    * an identifier matching `safe[A-Z]…` (camelCase convention: the
+      value is pre-escaped at assignment), or
+    * an identifier in :data:`_RENDERER_KNOWN_SAFE_IDENTIFIERS`
+      (reviewer-approved enum lookups / counters).
+
+    Defence-in-depth lint per issue #553. The current call sites are
+    already safe today via ``inlineMarkdown``'s leading ``escapeHtml``
+    pass, but that invariant is non-local — a refactor moving image
+    or link rendering out of ``inlineMarkdown`` would silently
+    regress it. The lint locks in the local-escape posture so the
+    safety property is structural rather than emergent.
+    """
+    body = _RENDERER_JS.read_text(encoding="utf-8")
+    lines = body.splitlines()
+    offenders: list[tuple[int, str, str]] = []
+    for m in _RENDERER_ATTR_INTERP_RE.finditer(body):
+        ident = m.group(1)
+        if len(ident) > 4 and ident.startswith("safe") and ident[4].isupper():
+            continue
+        if ident in _RENDERER_KNOWN_SAFE_IDENTIFIERS:
+            continue
+        line_no = body.count("\n", 0, m.start()) + 1
+        offenders.append((line_no, ident, lines[line_no - 1].rstrip()))
+    assert not offenders, (
+        f"Found {len(offenders)} unsafe attribute-context "
+        f"interpolation(s) in renderer.js:\n"
+        + "\n".join(
+            f"  line {n}: {ident!r}  in  {line.strip()[:100]}" for n, ident, line in offenders[:10]
+        )
+        + "\nEither wrap with escapeHtml() at the call site, rename "
+        "the variable to safeXxx (after verifying it is pre-escaped "
+        "at assignment), or add the identifier to "
+        "_RENDERER_KNOWN_SAFE_IDENTIFIERS with a comment explaining "
+        "why it is known-safe (e.g. enum lookup, integer counter)."
+    )
+
+
+_HANDLER_ATTRS = frozenset(
+    {
+        "onerror",
+        "onload",
+        "onmouseover",
+        "onclick",
+        "onmouseout",
+        "onfocus",
+        "onblur",
+        "onchange",
+        "onsubmit",
+        "onkeydown",
+        "onkeyup",
+        "onkeypress",
+    }
+)
+
+
+def _parse_renderer_html(html: str) -> tuple[list[str], list[tuple[str, str]]]:
+    """Parse ``html`` and return ``(start_tags, (tag, attr_name) pairs)``.
+
+    Two return values because:
+
+    * ``start_tags`` records every start tag regardless of whether it
+      carries attributes, so a bare ``<script>`` injection (no attrs)
+      cannot slip past a tag-presence check.
+    * ``attr_pairs`` records every attribute-bearing tag for the
+      event-handler-attribute assertion.
+
+    Substring checks on the raw output are too noisy: the literal text
+    ``onerror=&amp;quot;`` is safe when it sits inside a parsed
+    attribute value, but the substring still matches."""
+    from html.parser import HTMLParser
+
+    class _Collector(HTMLParser):
+        def __init__(self) -> None:
+            super().__init__()
+            self.tags: list[str] = []
+            self.attrs: list[tuple[str, str]] = []
+
+        def handle_starttag(self, tag: str, attrs: list[tuple[str, str | None]]) -> None:
+            self.tags.append(tag)
+            for name, _value in attrs:
+                self.attrs.append((tag, name))
+
+    p = _Collector()
+    p.feed(html)
+    return p.tags, p.attrs
+
+
+def _assert_no_handler_attrs(html: str) -> None:
+    _tags, attrs = _parse_renderer_html(html)
+    handlers = [(tag, name) for tag, name in attrs if name in _HANDLER_ATTRS]
+    assert not handlers, (
+        f"Renderer output materialized event-handler attribute(s) "
+        f"{handlers!r} — attribute-boundary escape regression. "
+        f"Full output:\n{html}"
+    )
+
+
+def test_attacker_image_url_with_quote_does_not_break_attribute() -> None:
+    """Pin: an image URL containing embedded double-quote characters
+    must NOT escape the ``data-src``/``data-alt`` attribute boundary.
+    The injected text remains inside the attribute value; no extra
+    attributes (``onerror``, etc.) materialize on the rendered span."""
+    out = _render('![alt](https://x/y.png" onerror="alert(1))')
+    _assert_no_handler_attrs(out)
+
+
+def test_attacker_image_alt_with_quote_does_not_break_attribute() -> None:
+    """Pin: an image alt text containing embedded double-quote
+    characters must not break the ``data-alt`` / ``aria-label``
+    attribute boundaries."""
+    out = _render('![alt" onerror="alert(1)](https://x/y.png)')
+    _assert_no_handler_attrs(out)
+
+
+def test_attacker_link_url_with_quote_does_not_break_attribute() -> None:
+    """Pin: a link URL containing embedded double-quote characters
+    must not escape the ``href`` attribute boundary."""
+    out = _render('[click](https://x/y" onmouseover="alert(1))')
+    _assert_no_handler_attrs(out)
+
+
+def test_attacker_link_label_with_quote_renders_as_text() -> None:
+    """Pin: a link label containing embedded ``<`` characters must
+    render as escaped text inside the anchor, not as a real tag.
+
+    Uses :func:`_parse_renderer_html` (not the attr-pairs accessor)
+    because a bare ``<script>`` injection has no attributes and would
+    be invisible to a (tag, attr) pair listing."""
+    out = _render("[<script>alert(1)</script>](https://x/y)")
+    tags, _attrs = _parse_renderer_html(out)
+    assert "script" not in tags, "Link label leaked a real <script> element:\n" + out
+
+
+def test_image_url_with_ampersand_not_double_escaped() -> None:
+    """Pin: a query-string URL must not double-escape ``&``.
+
+    inlineMarkdown's leading ``escapeHtml(text)`` turns ``&`` into
+    ``&amp;`` once. Any local re-escape on the captured ``url`` would
+    produce ``&amp;amp;`` in the attribute — which decodes to literal
+    ``&amp;`` at attribute-parse time, breaks ``getAttribute`` +
+    ``new URL`` round-trip, and silently corrupts query strings."""
+    out = _render("![alt](https://x/y?a=1&b=2)")
+    assert 'data-src="https://x/y?a=1&amp;b=2"' in out, (
+        "Expected single &amp; encoding for `&`; got:\n" + out
+    )
+    assert "&amp;amp;" not in out, (
+        "URL was double-escaped (`&` → `&amp;amp;`); breaks getAttribute "
+        "+ new URL round-trip. Full output:\n" + out
+    )
+
+
+def test_link_url_with_ampersand_not_double_escaped() -> None:
+    """Same as the image case, for link ``href``."""
+    out = _render("[docs](https://example.com/p?a=1&b=2)")
+    assert 'href="https://example.com/p?a=1&amp;b=2"' in out, (
+        "Expected single &amp; encoding for `&`; got:\n" + out
+    )
+    assert "&amp;amp;" not in out

tests/test_service_auth_boundary.py

@@ -440,6 +440,108 @@ class TestProxySseNon200LogLevel:
         assert "\n" not in matches[0].getMessage().split("body=", 1)[-1]
 
 
+# ---------------------------------------------------------------------------
+# _proxy_sse — Last-Event-ID forwarding (PR-D reconnect-with-replay)
+# ---------------------------------------------------------------------------
+
+
+class TestProxySseLastEventIdForwarding:
+    """The console SSE proxy is the inbound SSE path for multi-node
+    deployments — every browser EventSource traverses it.  Without
+    forwarding ``Last-Event-ID``, the per-ws / global SSE handlers on
+    the node would treat every reconnect as a fresh connect and silently
+    drop events emitted during the disconnect window.  PR-D's whole
+    reconnect-with-replay foundation depends on these tests passing."""
+
+    @pytest.mark.anyio
+    async def test_forwards_last_event_id_header_to_upstream(self):
+        """Browser sends ``Last-Event-ID``; upstream node must receive it."""
+        from starlette.requests import Request
+
+        from turnstone.console.server import _proxy_sse
+
+        captured_headers: dict[str, str] = {}
+
+        def handler(req: httpx.Request) -> httpx.Response:
+            # httpx headers are case-insensitive; capture lowercased.
+            captured_headers.update({k.lower(): v for k, v in req.headers.items()})
+            return httpx.Response(
+                200, text="data: {}\n\n", headers={"content-type": "text/event-stream"}
+            )
+
+        sse_client = httpx.AsyncClient(transport=httpx.MockTransport(handler))
+        proxy_client = httpx.AsyncClient(transport=httpx.MockTransport(handler))
+        scope = {
+            "type": "http",
+            "method": "GET",
+            "path": "/node/n/api/workstreams/ws-1/events",
+            "headers": [(b"last-event-id", b"42")],
+            "query_string": b"",
+            "app": MagicMock(
+                state=SimpleNamespace(proxy_sse_client=sse_client, proxy_client=proxy_client)
+            ),
+        }
+
+        async def _receive():
+            return {"type": "http.request", "body": b""}
+
+        request = Request(scope, receive=_receive)
+        response = await _proxy_sse(
+            request, "http://node-1:8001", "workstreams/ws-1/events", api_prefix="api"
+        )
+        # Drain so the upstream call actually fires.
+        async for _ in response.body_iterator:  # type: ignore[attr-defined]
+            pass
+
+        assert captured_headers.get("last-event-id") == "42", (
+            f"Last-Event-ID not forwarded to upstream; got headers={captured_headers!r}"
+        )
+
+    @pytest.mark.anyio
+    async def test_omits_last_event_id_when_client_did_not_send_one(self):
+        """Fresh connect (no header on the browser side) → no header
+        added on the upstream side either.  Guards against
+        accidentally injecting a stale or fabricated value."""
+        from starlette.requests import Request
+
+        from turnstone.console.server import _proxy_sse
+
+        captured_headers: dict[str, str] = {}
+
+        def handler(req: httpx.Request) -> httpx.Response:
+            captured_headers.update({k.lower(): v for k, v in req.headers.items()})
+            return httpx.Response(
+                200, text="data: {}\n\n", headers={"content-type": "text/event-stream"}
+            )
+
+        sse_client = httpx.AsyncClient(transport=httpx.MockTransport(handler))
+        proxy_client = httpx.AsyncClient(transport=httpx.MockTransport(handler))
+        scope = {
+            "type": "http",
+            "method": "GET",
+            "path": "/node/n/api/workstreams/ws-1/events",
+            "headers": [],
+            "query_string": b"",
+            "app": MagicMock(
+                state=SimpleNamespace(proxy_sse_client=sse_client, proxy_client=proxy_client)
+            ),
+        }
+
+        async def _receive():
+            return {"type": "http.request", "body": b""}
+
+        request = Request(scope, receive=_receive)
+        response = await _proxy_sse(
+            request, "http://node-1:8001", "workstreams/ws-1/events", api_prefix="api"
+        )
+        async for _ in response.body_iterator:  # type: ignore[attr-defined]
+            pass
+
+        assert "last-event-id" not in captured_headers, (
+            f"upstream got an unexpected Last-Event-ID; headers={captured_headers!r}"
+        )
+
+
 # ---------------------------------------------------------------------------
 # Gated cluster_events_sse — 503 on scope error (0a)
 # ---------------------------------------------------------------------------

tests/test_session.py

@@ -563,13 +563,13 @@ class TestTaskExec:
             item = session._prepare_task("c1", {"prompt": "do x", "skill": "ghost"})
         assert item.get("needs_approval") is False
         assert "unknown skill 'ghost'" in item["error"]
-        assert "skill(action='search')" in item["error"]
+        assert "skills(action='find'" in item["error"]
 
     def test_prepare_task_disabled_skill_returns_error(self, tmp_db) -> None:
         """Disabled skill → distinct error, mirrors the enabled gate that
-        ``_exec_skill(action='load')`` (session.py:8404) and skill-search
-        already apply.  Distinct from the unknown-skill phrasing so the
-        LLM's recovery path can tell 'not found' from 'quarantined'."""
+        ``_exec_skills_load`` and ``_exec_skills_find`` already apply.
+        Distinct from the unknown-skill phrasing so the LLM's recovery
+        path can tell 'not found' from 'quarantined'."""
         session = _make_session()
         disabled_skill = {
             "name": "retired",

tests/test_session_ui_base.py

@@ -50,8 +50,13 @@ def test_enqueue_fans_out_to_all_listeners() -> None:
     lq1 = ui._register_listener()
     lq2 = ui._register_listener()
     ui._enqueue({"type": "hello"})
-    assert lq1.get_nowait() == {"type": "hello", "ws_id": "ws-1"}
-    assert lq2.get_nowait() == {"type": "hello", "ws_id": "ws-1"}
+    # ``_enqueue`` stamps ``_event_id`` on every event so the ring
+    # buffer can key replay against ``Last-Event-ID``; non-token
+    # events (``hello`` isn't ``content`` / ``reasoning``) skip
+    # ``_seq``.  Both listeners observe the SAME dict reference
+    # (covered by ``test_listeners_share_dict_reference_warning``).
+    assert lq1.get_nowait() == {"type": "hello", "ws_id": "ws-1", "_event_id": 1}
+    assert lq2.get_nowait() == {"type": "hello", "ws_id": "ws-1", "_event_id": 1}
 
 
 def test_enqueue_preserves_existing_ws_id() -> None:
@@ -124,7 +129,12 @@ def test_resolve_plan_with_pending_broadcasts_plan_resolved() -> None:
     lq = ui._register_listener()
     ui.resolve_plan("accept")
     event = lq.get_nowait()
-    assert event == {"type": "plan_resolved", "feedback": "accept", "ws_id": "ws-1"}
+    assert event == {
+        "type": "plan_resolved",
+        "feedback": "accept",
+        "ws_id": "ws-1",
+        "_event_id": 1,
+    }
     assert ui._pending_plan_review is None
     assert ui._plan_event.is_set()
 
@@ -1072,7 +1082,7 @@ def test_on_content_token_writes_to_both_buffers() -> None:
     ui.on_content_token("hello")
     assert ui._ws_turn_content == ["hello"]
     assert ui._ws_inflight_content == ["hello"]
-    assert ui._ws_inflight_seq == 1
+    assert ui._event_id == 1
 
 
 def test_on_reasoning_token_writes_to_inflight_buffer_only() -> None:
@@ -1081,13 +1091,13 @@ def test_on_reasoning_token_writes_to_inflight_buffer_only() -> None:
     ui = _make_ui()
     ui.on_reasoning_token("thinking...")
     assert ui._ws_inflight_reasoning == ["thinking..."]
-    assert ui._ws_inflight_seq == 1
+    assert ui._event_id == 1
     # Multi-turn buffer is content-only and untouched by reasoning.
     assert ui._ws_turn_content == []
 
 
 def test_inflight_seq_advances_on_every_emit_even_at_cap() -> None:
-    """Cap-hit content tokens MUST advance ``_ws_inflight_seq``,
+    """Cap-hit content tokens MUST advance ``_event_id``,
     even though the buffer rejected the append. If seq stalled at
     high-water-pre-cap, a subscriber registering AFTER the cap is
     hit would capture ``snap_seq == stalled_seq`` and every
@@ -1101,12 +1111,12 @@ def test_inflight_seq_advances_on_every_emit_even_at_cap() -> None:
     chunk = "x" * 1024
     while ui._ws_inflight_content_size < _MAX_TURN_CONTENT_CHARS:
         ui.on_content_token(chunk)
-    seq_at_cap = ui._ws_inflight_seq
+    seq_at_cap = ui._event_id
 
     # Cap-hit token: seq MUST advance (no buffer append, but the
     # event still gets a fresh seq for the dedup filter).
     ui.on_content_token(chunk)
-    assert ui._ws_inflight_seq == seq_at_cap + 1
+    assert ui._event_id == seq_at_cap + 1
     # Buffer remains bounded — the cap-hit token is NOT in inflight.
     assert ui._ws_inflight_content_size <= _MAX_TURN_CONTENT_CHARS + len(chunk)
 
@@ -1198,7 +1208,7 @@ def test_inflight_snapshot_empty_during_post_commit_tool_window() -> None:
     monotonic (carries the high-water mark across turn boundaries)."""
     ui = _make_ui()
     ui.on_content_token("Calling tool with these args: ")
-    seq_pre_commit = ui._ws_inflight_seq
+    seq_pre_commit = ui._event_id
     ui.on_turn_committed()  # session.py fires this after messages.append
     # We're now in the tool-execution window. A reconnecting client
     # would call register_listener_with_in_progress_snapshot.
@@ -1394,13 +1404,13 @@ def test_snapshot_and_consume_does_not_reset_seq_at_idle_or_error() -> None:
     ui = _make_ui()
     ui.on_content_token("a")
     ui.on_content_token("b")
-    assert ui._ws_inflight_seq == 2
+    assert ui._event_id == 2
 
     ui.snapshot_and_consume_state_payload("idle")
-    assert ui._ws_inflight_seq == 2
+    assert ui._event_id == 2
 
     ui.snapshot_and_consume_state_payload("error")
-    assert ui._ws_inflight_seq == 2
+    assert ui._event_id == 2
 
 
 def test_listeners_share_dict_reference_warning() -> None:

tests/test_sse_reconnect_replay.py

@@ -0,0 +1,651 @@
+"""Tests for the SSE reconnect-with-replay foundation.
+
+Covers the three commits of the reconnect-with-replay PR at the
+boundaries that matter:
+
+  - :meth:`SessionUIBase.register_listener_with_replay` — the
+    per-ws ring buffer + ``Last-Event-ID`` slice semantics
+    (replay_ok / truncated / empty-buffer edge cases, order
+    preservation under concurrent emit, no skipped ids on
+    ``queue.Full``, cross-thread emit/replay consistency).
+  - :func:`make_events_handler` — ``id:`` field on every yielded
+    event from the buffer (replay or live), jittered ``retry:`` on
+    the first yield, ``replay_truncated`` envelope on stale
+    ``Last-Event-ID``, snapshot skip when replay covers the gap.
+
+The browser-side guard for the ``onerror`` close pattern lives in
+``test_app_js.py`` alongside the other static JS guards.
+"""
+
+from __future__ import annotations
+
+import asyncio
+import threading
+from types import SimpleNamespace as SimpleNS
+from typing import Any
+from unittest.mock import MagicMock
+
+from starlette.requests import Request
+
+from turnstone.core.session_routes import (
+    SessionEndpointConfig,
+    make_events_handler,
+)
+from turnstone.core.session_ui_base import SessionUIBase
+
+# ---------------------------------------------------------------------------
+# Helpers
+# ---------------------------------------------------------------------------
+
+
+class _ConcreteUI(SessionUIBase):
+    """Minimal concrete subclass for direct UI tests."""
+
+
+def _make_ui(ws_id: str = "ws-1") -> _ConcreteUI:
+    return _ConcreteUI(ws_id=ws_id, user_id="u1")
+
+
+def _fake_request(
+    *,
+    headers: dict[str, str] | None = None,
+    query: dict[str, str] | None = None,
+    path_params: dict[str, str] | None = None,
+) -> Request:
+    """Construct a Starlette ``Request`` for the events handler.
+
+    The handler reads ``request.headers``, ``request.query_params``,
+    ``request.path_params``, and awaits ``request.is_disconnected()``.
+    Building a real ASGI scope keeps the test honest about the values
+    those properties resolve from.
+    """
+    header_list = []
+    if headers:
+        for k, v in headers.items():
+            header_list.append((k.lower().encode(), v.encode()))
+    query_string = "&".join(f"{k}={v}" for k, v in query.items()).encode() if query else b""
+    scope = {
+        "type": "http",
+        "method": "GET",
+        "headers": header_list,
+        "path": "/events",
+        "raw_path": b"/events",
+        "query_string": query_string,
+        "path_params": path_params or {},
+        "app": MagicMock(),
+    }
+
+    async def _recv() -> dict[str, Any]:  # noqa: RUF029 — async signature required
+        return {"type": "http.disconnect"}
+
+    return Request(scope, receive=_recv)
+
+
+# ---------------------------------------------------------------------------
+# register_listener_with_replay — per-ws ring buffer slice semantics
+# ---------------------------------------------------------------------------
+
+
+def test_replay_holds_events_through_empty_listeners_period() -> None:
+    """The load-bearing property of the new ring buffer: events fired
+    while NO listener is registered must still be replayable to a
+    later subscriber whose ``Last-Event-ID`` predates them.  Pre-PR,
+    events to an empty listener list went on the floor — that's the
+    behaviour the entire reconnect-with-replay foundation replaces.
+    """
+    ui = _make_ui()
+    # No listeners — fire 10 events.
+    for i in range(10):
+        ui._enqueue({"type": "tool_started", "name": f"t{i}"})
+    # Reconnect-style register with Last-Event-ID=0 (client saw nothing).
+    lq, replay, status, lost, earliest, _ = ui.register_listener_with_replay(0)
+    assert status == "replay_ok"
+    assert lost == 0
+    assert earliest == 1
+    assert len(replay) == 10
+    assert [ev["name"] for ev in replay] == [f"t{i}" for i in range(10)]
+    # Each replayed event carries its _event_id so the events handler
+    # can emit the SSE id: field — verified by inspecting the slice.
+    assert [ev["_event_id"] for ev in replay] == list(range(1, 11))
+
+
+def test_replay_with_last_event_id_skips_already_seen_events() -> None:
+    """Client says it last saw id=5 — replay yields only events 6+,
+    not the whole buffer."""
+    ui = _make_ui()
+    for i in range(8):
+        ui._enqueue({"type": "tool_started", "name": f"t{i}"})
+    lq, replay, status, lost, earliest, _ = ui.register_listener_with_replay(5)
+    assert status == "replay_ok"
+    assert lost == 0
+    assert [ev["_event_id"] for ev in replay] == [6, 7, 8]
+
+
+def test_replay_truncated_when_last_event_id_predates_buffer() -> None:
+    """When the buffer has evicted events the client wanted, return
+    ``truncated`` with the lost-count gap so the handler can emit the
+    explicit envelope and fall through to snapshot recovery."""
+    ui = _make_ui()
+    # Override the buffer cap for the test so we don't have to fire
+    # 2001 events to trigger eviction.
+    import collections
+
+    ui._event_buffer = collections.deque(maxlen=5)
+    for i in range(20):
+        ui._enqueue({"type": "tool_started", "name": f"t{i}"})
+    # Buffer now holds ids 16..20 (5 most recent of 20 emitted).
+    lq, replay, status, lost, earliest, _ = ui.register_listener_with_replay(3)
+    assert status == "truncated"
+    assert earliest == 16
+    assert lost == 12  # earliest-1 - last_event_id = 15 - 3
+    assert replay == []
+
+
+def test_replay_empty_buffer_returns_replay_ok_empty() -> None:
+    """Cold-start ws with zero events ever: replay_ok / empty list.
+    A spurious ``replay_truncated`` envelope on a freshly-opened
+    workstream would be confusing and incorrect."""
+    ui = _make_ui()
+    lq, replay, status, lost, earliest, _ = ui.register_listener_with_replay(0)
+    assert status == "replay_ok"
+    assert replay == []
+    assert lost == 0
+    assert earliest == 0
+
+
+def test_replay_registers_listener_atomically_with_buffer_snapshot() -> None:
+    """Atomicity contract: under ``_listeners_lock`` we both snapshot
+    the buffer AND register the listener.  A writer's ``_enqueue``
+    takes the same lock, so an event landing after the snapshot
+    arrives in the listener queue (live) — never in BOTH the replay
+    and the live queue, and never in NEITHER."""
+    ui = _make_ui()
+    ui._enqueue({"type": "tool_started", "name": "before"})
+    lq, replay, _, _, _, _ = ui.register_listener_with_replay(0)
+    # Now fire after registration — must arrive live, NOT in replay.
+    ui._enqueue({"type": "tool_started", "name": "after"})
+    assert [ev["name"] for ev in replay] == ["before"]
+    live = lq.get_nowait()
+    assert live["name"] == "after"
+    assert live["_event_id"] == 2
+
+
+def test_event_id_monotonic_under_concurrent_writers() -> None:
+    """Load-bearing invariant for any replay protocol — if monotonicity
+    ever breaks (e.g. someone moves the id-increment outside the
+    lock), reconnect-with-replay silently re-orders events.  Stress
+    with multiple writer threads."""
+    ui = _make_ui()
+    n_writers = 4
+    per_writer = 200
+    barrier = threading.Barrier(n_writers)
+
+    def _writer(tag: str) -> None:
+        barrier.wait()
+        for i in range(per_writer):
+            ui._enqueue({"type": "tool_started", "name": f"{tag}-{i}"})
+
+    threads = [threading.Thread(target=_writer, args=(f"w{w}",)) for w in range(n_writers)]
+    for t in threads:
+        t.start()
+    for t in threads:
+        t.join()
+
+    # Walk the buffer in deque order — ids must be strictly monotonic.
+    ids = [eid for eid, _ in ui._event_buffer]
+    assert ids == sorted(ids), "event_id ordering broke under concurrent writers"
+    assert ids == list(range(ids[0], ids[-1] + 1)), "event_id skipped under concurrency"
+    assert ids[-1] == n_writers * per_writer
+
+
+def test_event_id_does_not_skip_when_listener_queue_full() -> None:
+    """If a slow listener's queue is full, the per-listener
+    ``put_nowait`` is silently dropped — but the counter must NOT
+    skip.  A subsequently-registered listener with
+    ``Last-Event-ID=0`` must see ALL the ids from the buffer
+    (1..N), not a sparse subset.  Pre-bug-class: moving the
+    id-increment inside the per-listener loop would create phantom
+    "gaps" the truncation detector would misread."""
+    ui = _make_ui()
+    slow_lq = ui._register_listener(maxsize=1)
+    slow_lq.put_nowait({"placeholder": True})  # full immediately
+    # Fire 10 events — 9 will hit queue.Full and be suppressed.
+    for i in range(10):
+        ui._enqueue({"type": "tool_started", "name": f"t{i}"})
+    # Replay from id=0 — fresh listener gets all 10, ids 1..10 dense.
+    _, replay, status, _, _, _ = ui.register_listener_with_replay(0)
+    assert status == "replay_ok"
+    assert [ev["_event_id"] for ev in replay] == list(range(1, 11))
+
+
+def test_cross_thread_writer_and_replay_observer_consistent() -> None:
+    """A worker thread fires ``_enqueue`` while another thread calls
+    ``register_listener_with_replay``.  The replay snapshot must be
+    gap-free — no half-written deque state visible to the reader.
+    Guards against the iteration-during-mutation hazard that a casual
+    implementation could introduce if the buffer copy out of the lock
+    isn't taken correctly."""
+    ui = _make_ui()
+    n = 500
+    done = threading.Event()
+
+    def _writer() -> None:
+        for i in range(n):
+            ui._enqueue({"type": "tool_started", "name": f"t{i}"})
+        done.set()
+
+    snap_box: dict[str, Any] = {}
+
+    def _reader() -> None:
+        # Wait briefly so the writer is mid-flight.
+        threading.Event().wait(0.001)
+        _, replay, status, _, earliest, _ = ui.register_listener_with_replay(0)
+        snap_box["replay"] = replay
+        snap_box["status"] = status
+        snap_box["earliest"] = earliest
+
+    w = threading.Thread(target=_writer)
+    r = threading.Thread(target=_reader)
+    w.start()
+    r.start()
+    w.join()
+    r.join()
+
+    replay = snap_box["replay"]
+    # Replay snapshot is consistent — ids contiguous, no gaps.
+    ids = [ev["_event_id"] for ev in replay]
+    assert ids == sorted(ids)
+    if ids:
+        assert ids == list(range(ids[0], ids[-1] + 1)), (
+            "gap observed in replay snapshot — torn deque state visible"
+        )
+
+
+def test_event_id_persists_across_turn_boundaries() -> None:
+    """Resetting ``_event_id`` to 0 at turn boundaries would silently
+    mis-replay a long-lived SSE subscriber whose ``Last-Event-ID``
+    was from a prior turn.  Mirrors the pre-existing
+    ``test_inflight_seq_monotonic_across_turn_boundaries`` invariant
+    on the snap_seq side, extended to the buffer/replay side."""
+    ui = _make_ui()
+    ui.on_content_token("turn-N tok1 ")
+    ui.on_content_token("turn-N tok2 ")
+    seq_before = ui._event_id
+    ui.on_turn_committed()
+    ui.on_turn_start()
+    ui.on_content_token("turn-N+1 tok1")
+    seq_after = ui._event_id
+    assert seq_after > seq_before, "counter regressed across turn boundary"
+    # Replay from mid-turn-N must still serve turn-N+1's content.
+    _, replay, status, _, _, _ = ui.register_listener_with_replay(seq_before)
+    assert status == "replay_ok"
+    assert len(replay) == 1
+    assert replay[0]["text"] == "turn-N+1 tok1"
+
+
+def test_replay_ok_skips_in_progress_snapshot_path() -> None:
+    """When ``last_event_id`` is provided AND replay covers the gap,
+    ``register_listener_with_replay`` returns ``replay_ok`` without
+    touching the inflight content/reasoning snapshot machinery.  The
+    events handler uses this branch to skip emitting the
+    ``in_progress_snapshot`` event (which would otherwise double-
+    render content the buffered events already contain)."""
+    ui = _make_ui()
+    ui.on_content_token("partial ")
+    # Replay path: returns replay_ok and a synthetic snap is NOT taken
+    # (we test the handler-side behavior in the handler tests below).
+    lq, replay, status, _, _, snap = ui.register_listener_with_replay(0)
+    assert status == "replay_ok"
+    # The buffered event carries the partial content as a content event.
+    assert any(ev.get("type") == "content" for ev in replay)
+    # Snapshot is captured atomically too (used on truncated path to
+    # drive live-drain ``_seq <= snap_seq`` dedup); for replay_ok the
+    # caller ignores it but the contract returns one regardless.
+    assert isinstance(snap, dict)
+    assert snap["seq"] >= 1
+
+
+def test_truncated_path_snapshot_captures_real_snap_seq() -> None:
+    """Regression for PR #542 review comment 1 (Copilot, low-confidence).
+
+    On the truncated path the caller used to set ``snap_seq=0``, which
+    disabled the events handler's live-drain ``_seq <= snap_seq``
+    dedup.  A token writer racing between
+    ``register_listener_with_replay`` returning and the live drain's
+    first read would land in the listener queue AND in the captured
+    snapshot text, causing the client to render the token twice
+    (once via the ``in_progress_snapshot`` content text, once via the
+    live event delivery).
+
+    The fix lifts the snapshot capture INTO
+    ``register_listener_with_replay`` under the same nested-lock
+    acquire as the listener registration + buffer slice + counter
+    read, so ``snap_seq`` returned in the snapshot is the exact
+    high-water mark the snapshot text corresponds to."""
+    import collections
+
+    ui = _make_ui()
+    ui._event_buffer = collections.deque(maxlen=3)
+    # Fire enough events to trigger truncation on reconnect with a
+    # stale ``Last-Event-ID``.
+    for i in range(10):
+        ui.on_content_token(f"t{i}")
+    _, _, status, _, _, snap = ui.register_listener_with_replay(1)
+    assert status == "truncated"
+    # The snapshot's seq must be the LATEST event_id, not 0 — that's
+    # what gates the live-drain dedup filter in the events handler.
+    assert snap["seq"] == ui._event_id
+    assert snap["seq"] >= 10
+    # And the content is captured (not empty).
+    assert "t0" in snap["content"]
+    assert "t9" in snap["content"]
+
+
+def test_snap_seq_high_water_mark_holds_under_writer_race() -> None:
+    """Regression for PR #561 review comment 1.
+
+    The invariant: every token whose text appears in
+    ``snapshot["content"]`` (or ``"reasoning"``) must have its
+    ``_event_id`` <= ``snapshot["seq"]``.  Equivalently, any token
+    that fires AFTER the snapshot was captured must have
+    ``_event_id > snap_seq``.  Otherwise the events handler's
+    ``_seq <= snap_seq`` live-drain filter would let the new token
+    through AND its text would already be in the snapshot text →
+    double-render.
+
+    The pre-fix race: ``on_content_token`` took ``_ws_lock``,
+    appended to inflight, released ``_ws_lock``, then called
+    ``_enqueue`` (which bumps ``_event_id``).  A snapshot reader
+    interleaving between the release and the ``_enqueue`` would
+    capture inflight (with the new text) and read a STALE
+    ``_event_id``.  Snap_seq below new event's id → filter slips →
+    double-render.
+
+    The race window in plain Python is narrow (a few bytecodes
+    between lock release and the ``_enqueue`` call), so a pure
+    barrier-based race rarely hits it.  This test injects a
+    deterministic sleep into ``_enqueue`` via monkey-patch to
+    widen the window enough to be reliably observed under the
+    pre-fix code path — AND to be reliably AVOIDED under the
+    post-fix code path (because the post-fix
+    ``on_content_token`` calls ``_enqueue`` while still holding
+    ``_ws_lock``, so the snapshot reader can't acquire
+    ``_ws_lock`` until the writer is fully done).
+    """
+    import queue
+    import threading
+    import time
+
+    ui = _make_ui()
+    marker = "RACE-MARKER"
+    original_enqueue = ui._enqueue
+
+    # Widen the race window: sleep just BEFORE the original
+    # ``_enqueue`` runs (which is where ``_event_id`` would advance).
+    # Post-fix this sleep happens while the writer still holds
+    # ``_ws_lock`` — readers block.  Pre-fix the writer has
+    # released ``_ws_lock`` before reaching this monkey-patch, so
+    # the reader gets a clean window to capture an inconsistent
+    # ``(inflight, _event_id)`` pair.
+    def slow_enqueue(data: dict[str, Any]) -> None:
+        time.sleep(0.05)  # 50 ms — orders of magnitude wider than the GIL switch interval
+        return original_enqueue(data)
+
+    ui._enqueue = slow_enqueue  # type: ignore[method-assign]
+
+    snap_box: dict[str, Any] = {}
+    writer_done = threading.Event()
+
+    def _writer() -> None:
+        ui.on_content_token(marker)
+        writer_done.set()
+
+    def _reader() -> None:
+        # Give the writer time to enter ``on_content_token`` and
+        # (pre-fix) release ``_ws_lock`` before the snapshot.  50 ms
+        # is conservative; 5 ms would also work in practice.
+        time.sleep(0.025)
+        _, _, _, _, _, snap = ui.register_listener_with_replay(0)
+        snap_box["snap"] = snap
+        snap_box["event_id_at_snapshot_return"] = ui._event_id
+
+    wt = threading.Thread(target=_writer)
+    rt = threading.Thread(target=_reader)
+    wt.start()
+    rt.start()
+    wt.join(timeout=5)
+    rt.join(timeout=5)
+    assert writer_done.is_set(), "writer thread did not complete"
+
+    snap = snap_box["snap"]
+    final_event_id = ui._event_id
+
+    # Core invariant: if the snapshot's content includes the marker
+    # text, snap.seq must be >= the writer's final _event_id.
+    # Pre-fix this fails (snap.seq=0 while final_event_id=1 and
+    # snap.content="RACE-MARKER"); post-fix the reader can't acquire
+    # ``_ws_lock`` until the writer completes, so snap is either
+    # (content="", seq=0) — reader won first — or
+    # (content="RACE-MARKER", seq=1) — writer won first.
+    assert marker in snap["content"] or snap["content"] == "", (
+        f"unexpected snap content: {snap['content']!r}"
+    )
+    if marker in snap["content"]:
+        assert snap["seq"] >= final_event_id, (
+            f"snap captured '{marker}' but snap.seq={snap['seq']} < "
+            f"final _event_id={final_event_id}; the live emission of "
+            f"this token would slip past the events handler's "
+            f"_seq <= snap_seq filter and double-render text the "
+            f"snapshot already contained.  Pre-fix race window "
+            f"opened by ``_enqueue`` running outside ``_ws_lock``."
+        )
+
+    # Sanity: also exercise the post-truncated drain shape so the
+    # test file pins both the contract AND the no-backfill behaviour
+    # (a future change that adds backfill into the listener queue
+    # must keep the dedup invariant above true).
+    ui2 = _make_ui()
+    for j in range(5):
+        ui2.on_content_token(f"x{j}")
+    lq, _, status, _, _, snap2 = ui2.register_listener_with_replay(0)
+    captured_seq = snap2["seq"]
+    drained = 0
+    while True:
+        try:
+            ev = lq.get_nowait()
+        except queue.Empty:
+            break
+        drained += 1
+        if ev.get("type") == "content":
+            assert ev["_seq"] <= captured_seq, f"token _seq={ev['_seq']} > snap_seq={captured_seq}"
+    assert drained == 0, (
+        f"register_listener_with_replay backfilled {drained} events "
+        f"into the listener queue; if intentional, the dedup "
+        f"invariant above must still hold and this assertion should "
+        f"be updated."
+    )
+
+
+# ---------------------------------------------------------------------------
+# make_events_handler — id: / retry: / replay_truncated / branch behaviour
+# ---------------------------------------------------------------------------
+
+
+def _wire_events_handler(ui: _ConcreteUI) -> Any:
+    """Build a minimal ``make_events_handler`` closure that returns
+    yields suitable for the EventSourceResponse generator.
+
+    Calls the closure with a fake request; returns the inner generator
+    AFTER it has been started so the test can iterate yields directly.
+    """
+    ws = SimpleNS(id=ui.ws_id, ui=ui, state=SimpleNS(value="idle"))
+    mgr = MagicMock()
+    mgr.get.return_value = ws
+
+    cfg = SessionEndpointConfig(
+        permission_gate=None,
+        manager_lookup=lambda _r: (mgr, None),
+        tenant_check=None,
+        not_found_label="Workstream not found",
+        audit_action_prefix="workstream",
+        events_replay=None,
+        events_replay_prepare=None,
+    )
+    return make_events_handler(cfg)
+
+
+def _drain_handler_yields(
+    ui: _ConcreteUI,
+    *,
+    headers: dict[str, str] | None = None,
+    query: dict[str, str] | None = None,
+    max_yields: int = 10,
+) -> tuple[list[Any], str]:
+    """Synchronous helper: spin up the handler, drain up to N yields,
+    return ``(raw_yields, decoded_blob)``.  Uses ``asyncio.run`` so
+    tests don't depend on pytest-asyncio / pytest-anyio plugin config.
+
+    The decoded blob is the textual SSE concatenation — assertion
+    targets in the tests below grep against it.  Raw yields are
+    returned for shape-level assertions (e.g. the first-yield
+    ``retry`` check).
+    """
+    handler = _wire_events_handler(ui)
+    req = _fake_request(headers=headers, query=query, path_params={"ws_id": ui.ws_id})
+
+    async def _run() -> list[Any]:
+        resp = await handler(req)
+        out: list[Any] = []
+        async for chunk in resp.body_iterator:
+            out.append(chunk)
+            if len(out) >= max_yields:
+                break
+        await resp.body_iterator.aclose()
+        return out
+
+    yields = asyncio.run(_run())
+    # The events handler yields plain dicts ({"data": ..., "id": ...,
+    # "retry": ..., ...}); sse-starlette's response layer encodes
+    # them into SSE wire format at serve-time.  For introspection,
+    # render each dict into the equivalent SSE textual form so the
+    # tests can grep against the canonical encoded representation
+    # AND have access to the raw dicts for shape-level assertions.
+    text_parts: list[str] = []
+    for y in yields:
+        if isinstance(y, bytes):
+            text_parts.append(y.decode(errors="replace"))
+        elif isinstance(y, str):
+            text_parts.append(y)
+        elif isinstance(y, dict):
+            # Mirror sse-starlette's encoding contract — one field
+            # per line, terminating blank line per event.
+            for field in ("id", "event", "retry", "data", "comment"):
+                if field in y:
+                    text_parts.append(f"{field}: {y[field]}")
+            text_parts.append("")
+        elif hasattr(y, "encode"):
+            encoded = y.encode()
+            text_parts.append(
+                encoded.decode(errors="replace") if isinstance(encoded, bytes) else str(encoded)
+            )
+        else:
+            text_parts.append(str(y))
+    return yields, "\n".join(text_parts)
+
+
+def test_handler_emits_retry_on_first_yield() -> None:
+    """First yield of the events handler must include a jittered
+    ``retry`` field in the [2500, 4500] ms range so 6-pane reconnects
+    don't lockstep on EventSource's default ~3 s interval."""
+    ui = _make_ui()
+    _, blob = _drain_handler_yields(ui, max_yields=1)
+    # The retry: SSE field appears in the encoded blob.
+    import re
+
+    match = re.search(r"retry:\s*(\d+)", blob)
+    assert match is not None, f"first yield missing retry: line\n{blob}"
+    retry = int(match.group(1))
+    assert 2500 <= retry <= 4500, f"retry {retry} outside jitter band [2500, 4500]"
+
+
+def test_handler_replay_ok_skips_snapshot_emits_id() -> None:
+    """``Last-Event-ID`` + buffer covers gap → emit buffered events
+    with SSE ``id:`` field, SKIP the in-progress snapshot (it would
+    double-render content the buffered events already carry)."""
+    ui = _make_ui()
+    ui.on_content_token("hello ")
+    ui.on_content_token("world")
+    _, blob = _drain_handler_yields(ui, headers={"Last-Event-ID": "0"}, max_yields=6)
+    # No in_progress_snapshot anywhere on the replay_ok path.
+    assert "in_progress_snapshot" not in blob, (
+        "replay_ok must not emit in_progress_snapshot — it duplicates "
+        f"buffered content. blob:\n{blob}"
+    )
+    # Every buffered content event got an id: line.
+    assert "id: 1" in blob, f"missing id: 1 in:\n{blob}"
+    assert "id: 2" in blob, f"missing id: 2 in:\n{blob}"
+
+
+def test_handler_truncated_emits_envelope_then_snapshot() -> None:
+    """Stale ``Last-Event-ID`` + buffer too short → emit
+    ``replay_truncated`` envelope, THEN fall through to the
+    fresh-style replay (state_change + in_progress_snapshot) as the
+    recovery floor."""
+    import collections
+
+    ui = _make_ui()
+    ui._event_buffer = collections.deque(maxlen=3)
+    for i in range(10):
+        ui.on_content_token(f"t{i}")
+    _, blob = _drain_handler_yields(ui, headers={"Last-Event-ID": "1"}, max_yields=8)
+
+    assert "replay_truncated" in blob, (
+        f"stale Last-Event-ID must emit replay_truncated envelope; got:\n{blob}"
+    )
+    # Recovery floor: in_progress_snapshot carries the partial content
+    # the evicted events represented.
+    assert "in_progress_snapshot" in blob, (
+        f"truncated path must fall through to in_progress_snapshot; got:\n{blob}"
+    )
+
+
+def test_handler_fresh_path_skips_replay_truncated() -> None:
+    """No ``Last-Event-ID`` → fresh-connect behaviour (today's path
+    unchanged: state_change + in_progress_snapshot + live).  No
+    replay_truncated envelope should ever appear on a fresh
+    connect."""
+    ui = _make_ui()
+    ui.on_content_token("hello ")
+    _, blob = _drain_handler_yields(ui, max_yields=5)
+
+    assert "replay_truncated" not in blob
+    # Fresh connect emits the snapshot.
+    assert "in_progress_snapshot" in blob
+
+
+def test_handler_malformed_last_event_id_falls_back_to_fresh() -> None:
+    """Defence against intermediaries that mangle the header — a
+    non-integer ``Last-Event-ID`` must not be treated as ``0`` (which
+    could trigger spurious replays) nor crash the handler.  Falls
+    through to the fresh-connect path."""
+    ui = _make_ui()
+    _, blob = _drain_handler_yields(
+        ui,
+        headers={"Last-Event-ID": "abc-not-an-int"},
+        max_yields=3,
+    )
+    assert "replay_truncated" not in blob
+
+
+def test_handler_query_param_fallback_is_honoured() -> None:
+    """The manual-reconnect path can't set custom headers on
+    ``new EventSource(url)`` — the browser sends
+    ``?last_event_id=N`` instead.  Handler must honour the query
+    param identically to the header."""
+    ui = _make_ui()
+    ui.on_content_token("hello")
+    _, blob = _drain_handler_yields(ui, query={"last_event_id": "0"}, max_yields=4)
+
+    # Replay path: in_progress_snapshot SKIPPED, id:1 present.
+    assert "in_progress_snapshot" not in blob
+    assert "id: 1" in blob

tests/test_tools_schema.py

@@ -72,8 +72,10 @@ class TestToolsMetadata:
     """Validate the metadata extracted from JSON files."""
 
     def test_tool_count(self):
-        # 19 interactive tools + 13 coordinator tools
-        assert len(TOOLS) == 32
+        # 19 interactive tools + 12 coordinator tools (was 13 before the
+        # skills tool unification merged `skill` + `list_skills` and made
+        # the unified `skills` tool dual-kind).
+        assert len(TOOLS) == 31
 
     def test_agent_tools_count(self):
         assert len(AGENT_TOOLS) == 10
@@ -96,13 +98,19 @@ class TestToolsMetadata:
             "delete_workstream",
             "list_workstreams",
             "list_nodes",
-            "list_skills",
             "tasks",
             "wait_for_workstream",
-            # ``memory`` is dual-kind (coordinator: true + interactive: true)
-            # so coords can persist orchestration context for their children
+            # ``memory`` is dual-kind (coordinator + interactive) so
+            # coords can persist orchestration context for their children
             # via the new ``coordinator`` scope.
             "memory",
+            # ``skills`` is dual-kind (replaces legacy ``skill`` +
+            # ``list_skills``).  Read actions (find, get) auto-approve;
+            # write actions require operator approval + the
+            # ``model.skills.write`` permission.  ``load`` errors on
+            # coord sessions — coords delegate skill assignment via
+            # ``spawn_workstream(skill=...)``.
+            "skills",
         }
 
     def test_auto_approve_sets_match(self):
@@ -119,7 +127,6 @@ class TestToolsMetadata:
             "inspect_workstream",
             "list_workstreams",
             "list_nodes",
-            "list_skills",
             "wait_for_workstream",
         }
         assert expected == AGENT_AUTO_TOOLS
@@ -144,7 +151,7 @@ class TestToolsMetadata:
             "watch": "command",
             "read_resource": "uri",
             "use_prompt": "name",
-            "skill": "name",
+            "skills": "action",
             "diff_file": "path_a",
             # Coordinator tools:
             "spawn_workstream": "initial_message",

tests/test_workstream_kind.py

@@ -277,8 +277,12 @@ def test_interactive_and_coordinator_tool_sets_overlap_only_on_dual_kind():
     interactive_names = {t["function"]["name"] for t in INTERACTIVE_TOOLS}
     coord_names = {t["function"]["name"] for t in COORDINATOR_TOOLS}
 
-    # Explicit dual-kind tools — deliberately in both sets.
-    dual_kind = {"memory"}
+    # Explicit dual-kind tools — deliberately in both sets.  ``skills``
+    # joined in 1.6.0 (replaces legacy ``skill`` + ``list_skills``) — read
+    # actions auto-approve on both kinds, write actions gate on
+    # ``model.skills.write`` permission, and ``load`` errors on coord
+    # sessions where it doesn't apply.
+    dual_kind = {"memory", "skills"}
 
     overlap = interactive_names & coord_names
     assert overlap == dual_kind, (

turnstone/__init__.py

@@ -1,3 +1,3 @@
 """turnstone - Multi-node AI orchestration platform with tool use, agent routing, and cluster simulation."""
 
-__version__ = "1.6.0a2"
+__version__ = "1.6.0a3"

turnstone/api/console_schemas.py

@@ -342,10 +342,11 @@ class CreateSkillRequest(BaseModel):
         min_length=1,
         max_length=1024,
         description=(
-            "Human-readable description surfaced by ``list_skills`` and "
-            "the admin UI.  Must be non-empty — catches skills registered "
-            "without thinking about discoverability before they reach a "
-            "model's tool-selection prompt."
+            "Human-readable description surfaced by the ``skills`` "
+            "find/get tool and the admin UI.  Must be non-empty — "
+            "catches skills registered without thinking about "
+            "discoverability before they reach a model's tool-selection "
+            "prompt."
         ),
     )
     tags: str = "[]"
@@ -371,12 +372,16 @@ class CreateSkillRequest(BaseModel):
     kind: SkillKind = Field(
         default=SkillKind.ANY,
         description=(
-            "Classifier routing the skill to ``list_skills`` calls.  "
-            "``interactive`` is visible only to the interactive-session "
-            "activation path; ``coordinator`` is visible only to the "
-            "coordinator's ``list_skills`` tool; ``any`` (default) is "
-            "visible on both sides, which preserves pre-upgrade "
-            "behaviour for legacy rows."
+            "Authored audience metadata — passive marker for "
+            "sorting/grouping and discoverability narrowing.  "
+            "``interactive`` marks the skill as authored for "
+            "interactive sessions; ``coordinator`` marks it for "
+            "coordinator delegation; ``any`` (default) signals no "
+            "preferred audience.  Not a runtime visibility gate after "
+            "the SkillKind enforcement flatten (#557) — every session "
+            "kind can find, get, and load every skill regardless of "
+            "this field; real access control remains "
+            "``allowed_tools`` + ``auto_approve``."
         ),
     )
 

turnstone/console/coordinator_client.py

@@ -140,13 +140,6 @@ _TASK_TITLE_MAX = 200
 # enough to bound one stall per child per turn regardless of how many
 # inspect calls the model fires.
 _LIVE_CACHE_TTL_SECONDS = 2.0
-# Cap on the number of tool names projected per skill in list_skills.
-# A skill that whitelists a wide MCP surface (Slack/Gmail/Drive +
-# dozens of helpers) would otherwise bloat the per-row payload and
-# defeat the bounded-output contract.  Anything beyond the cap is
-# rolled into a "+N more" sentinel so the model knows to fetch the
-# full row if the inventory matters.
-_SKILL_TOOLS_PROJECTION_CAP = 20
 
 
 def _utc_now_iso() -> str:
@@ -1219,90 +1212,6 @@ class CoordinatorClient:
             )
         return {"nodes": nodes, "truncated": truncated}
 
-    def list_skills(
-        self,
-        *,
-        category: str | None = None,
-        tag: str | None = None,
-        risk_level: str | None = None,
-        enabled_only: bool = False,
-        limit: int = 100,
-    ) -> dict[str, Any]:
-        """Return ``{"skills": [...], "truncated": bool}``.
-
-        Coordinator-visible skills only: the storage filter narrows to
-        ``kind IN ('coordinator', 'any')``.  Skills tagged
-        ``interactive`` are hidden from the coordinator's
-        ``list_skills`` tool (they're meant for child workstreams, not
-        the orchestrator), while ``any``-tagged skills show up on both
-        sides for backwards compatibility with pre-tagging catalogs.
-
-        Filters pushed into SQL via ``list_skills_filtered`` — no per-row
-        lookups.  ``tag`` matches when the value appears in the
-        JSON-array ``tags`` column (quote-bracketed substring).
-        ``tags`` is decoded from JSON at the edge so the model sees a
-        list, not the escaped string.  Projection is intentionally narrow
-        — discovery metadata only, not full row.
-        """
-        page_size = max(1, min(int(limit), 500))
-        rows = self._storage.list_skills_filtered(
-            category=category,
-            tag=tag,
-            risk_level=risk_level,
-            kinds=["coordinator", "any"],
-            enabled_only=enabled_only,
-            limit=page_size + 1,  # +1 to detect truncation
-        )
-        truncated = len(rows) > page_size
-        rows = rows[:page_size]
-        skills: list[dict[str, Any]] = []
-        for r in rows:
-            tags_raw = r.get("tags") or "[]"
-            try:
-                tags = json.loads(tags_raw) if isinstance(tags_raw, str) else list(tags_raw)
-            except (TypeError, ValueError):
-                tags = []
-            allowed_raw = r.get("allowed_tools") or "[]"
-            try:
-                allowed_full = (
-                    json.loads(allowed_raw) if isinstance(allowed_raw, str) else list(allowed_raw)
-                )
-            except (TypeError, ValueError):
-                allowed_full = []
-            if not isinstance(allowed_full, list):
-                allowed_full = []
-            # Cap the projected tool list so a skill that whitelists a
-            # large MCP surface doesn't bloat the coordinator's
-            # list_skills payload.  Coordinators that need the full
-            # inventory can fetch the skill row directly.
-            allowed_tools: list[str] = [str(t) for t in allowed_full[:_SKILL_TOOLS_PROJECTION_CAP]]
-            if len(allowed_full) > _SKILL_TOOLS_PROJECTION_CAP:
-                allowed_tools.append(f"+{len(allowed_full) - _SKILL_TOOLS_PROJECTION_CAP} more")
-            skill_row: dict[str, Any] = {
-                "name": r.get("name") or "",
-                "category": r.get("category") or "",
-                "tags": tags,
-                "version": r.get("version") or "",
-                "description": r.get("description") or "",
-                "model": r.get("model") or "",
-                "enabled": bool(r.get("enabled")),
-                "risk_level": r.get("risk_level") or "",
-                "activation": r.get("activation") or "",
-                "kind": r["kind"],
-            }
-            # Omit ``allowed_tools`` when empty: an empty list reads as
-            # "no tools are usable by this skill" to a model that doesn't
-            # know the semantics, but the actual meaning is "no tools are
-            # pre-approved (auto-approve exemption list)".  Real
-            # misdiagnosis happened in testing when a code-review skill
-            # with no auto-approve allowlist looked like it had been
-            # spawned with zero tool access.  Dropping the key altogether
-            # when empty removes the ambiguity at the source.
-            if allowed_tools:
-                skill_row["allowed_tools"] = allowed_tools
-            skills.append(skill_row)
-        return {"skills": skills, "truncated": truncated}
-
     # ------------------------------------------------------------------
     # tasks — coordinator-local planning state persisted on workstream_config
     # ------------------------------------------------------------------

turnstone/console/server.py

@@ -77,6 +77,7 @@ from turnstone.core.session_routes import (
     register_coord_verbs,
     register_session_routes,
 )
+from turnstone.core.skill_field_validation import SKILL_RUNTIME_CONFIG_FIELDS
 from turnstone.core.skill_kind import SkillKind
 from turnstone.core.web_helpers import (
     read_json_or_400,
@@ -2866,12 +2867,31 @@ async def _proxy_sse(
     else:
         sse_auth = _proxy_auth_headers(request)
 
+    # Forward ``Last-Event-ID`` from the browser to the upstream node so
+    # the per-ws / global SSE handlers can serve the reconnect-with-replay
+    # buffer slice.  Without this, multi-node deployments lose replay
+    # entirely (the proxy is the only inbound SSE path in that shape);
+    # the per-ws handler would treat every reconnect as a fresh connect
+    # and silently drop events emitted during the disconnect window.
+    # The query-param fallback (``?last_event_id=N``) is already
+    # forwarded via the existing ``request.url.query`` propagation at
+    # the top of this function — only the header needs an explicit
+    # carry-over.  Header lookups in Starlette are case-insensitive.
+    upstream_headers: dict[str, str] = {
+        **sse_auth,
+        "Accept": "text/event-stream",
+        "Cache-Control": "no-store",
+    }
+    last_event_id_hdr = request.headers.get("last-event-id")
+    if last_event_id_hdr is not None:
+        upstream_headers["Last-Event-ID"] = last_event_id_hdr
+
     async def raw_stream() -> AsyncGenerator[bytes, None]:
         try:
             async with sse_client.stream(
                 "GET",
                 target,
-                headers={**sse_auth, "Accept": "text/event-stream", "Cache-Control": "no-store"},
+                headers=upstream_headers,
                 timeout=httpx.Timeout(connect=10, read=None, write=5, pool=None),
             ) as response:
                 if response.status_code != 200:
@@ -5875,6 +5895,14 @@ _VALID_PERMISSIONS = frozenset(
         # surface for GET /v1/api/cluster/ws/{ws_id}/detail.  Granted
         # to builtin-admin via migration 040.
         "admin.cluster.inspect",
+        # Model-facing write capability over the skill catalog.  Gates
+        # the ``skills(action=create|update|enable|disable)`` tool path
+        # (in-process, not HTTP — distinct from ``admin.skills`` which
+        # gates admin-UI traffic).  Default-ungranted on every role
+        # including builtin-admin — operators must opt themselves in
+        # explicitly before their coordinator sessions can mutate the
+        # catalog.
+        "model.skills.write",
         "tools.approve",
         "workstreams.create",
         "workstreams.close",
@@ -6422,149 +6450,27 @@ async def admin_delete_policy(request: Request) -> JSONResponse:
 # Admin: Skills (thin layer over prompt templates with extended fields)
 # ---------------------------------------------------------------------------
 
-_VALID_ACTIVATIONS = {"named", "default", "search"}
-
-# Fields that may be updated on installed (readonly) skills.
-# These are local runtime configuration — not part of the SKILL.md spec —
-# so they don't compromise the fidelity of an externally-sourced skill.
-_SKILL_RUNTIME_CONFIG_FIELDS = frozenset(
-    {
-        "model",
-        "temperature",
-        "reasoning_effort",
-        "max_tokens",
-        "token_budget",
-        "agent_max_turns",
-        "auto_approve",
-        "allowed_tools",
-        "enabled",
-        "notify_on_complete",
-        "priority",
-    }
-)
+# Re-exported from the shared validator module (top-of-file import) so
+# both this HTTP path and the model-tool path
+# (``ChatSession._exec_skills_update``) read the same source of truth
+# for runtime-field membership.  Drift between the two would let one
+# surface accept a field the other rejects.
+_SKILL_RUNTIME_CONFIG_FIELDS = SKILL_RUNTIME_CONFIG_FIELDS
 
 
 def _parse_skill_session_config(body: dict[str, Any]) -> tuple[dict[str, Any], JSONResponse | None]:
-    """Parse and validate session config fields from a skill request body.
+    """HTTP adapter over :func:`parse_skill_session_config`.
 
-    Returns (fields_dict, error_response). error_response is None on success.
-    Only includes fields that are present in the body (for partial updates).
+    Validation lives in ``turnstone.core.skill_field_validation`` so the
+    in-process model-tool path (``_exec_skills_*``) shares the exact same
+    rules — neither side can drift.  This wrapper translates a non-None
+    string error into a 400 JSONResponse for the admin endpoint.
     """
-    import json as _json
-
-    fields: dict[str, Any] = {}
-
-    if "model" in body:
-        fields["model"] = str(body["model"] or "").strip()
-
-    if "temperature" in body:
-        temp = body["temperature"]
-        if temp is not None and temp != "":
-            try:
-                temp = float(temp)
-                if not (0.0 <= temp <= 2.0):
-                    return {}, JSONResponse(
-                        {"error": "temperature must be between 0 and 2"}, status_code=400
-                    )
-                fields["temperature"] = temp
-            except (ValueError, TypeError):
-                fields["temperature"] = None
-        else:
-            fields["temperature"] = None
-
-    if "token_budget" in body:
-        try:
-            tb = int(body.get("token_budget", 0) or 0)
-        except (ValueError, TypeError):
-            return {}, JSONResponse({"error": "token_budget must be an integer"}, status_code=400)
-        if tb < 0:
-            return {}, JSONResponse({"error": "token_budget must be non-negative"}, status_code=400)
-        fields["token_budget"] = tb
-
-    if "max_tokens" in body:
-        mt = body["max_tokens"]
-        if mt is not None and mt != "":
-            try:
-                mt = int(mt)
-            except (ValueError, TypeError):
-                return {}, JSONResponse({"error": "max_tokens must be an integer"}, status_code=400)
-            if mt < 1:
-                return {}, JSONResponse({"error": "max_tokens must be positive"}, status_code=400)
-            fields["max_tokens"] = mt
-        else:
-            fields["max_tokens"] = None
-
-    if "agent_max_turns" in body:
-        amt = body["agent_max_turns"]
-        if amt is not None and amt != "":
-            try:
-                amt = int(amt)
-            except (ValueError, TypeError):
-                return {}, JSONResponse(
-                    {"error": "agent_max_turns must be an integer"}, status_code=400
-                )
-            if amt < 1:
-                return {}, JSONResponse(
-                    {"error": "agent_max_turns must be positive"}, status_code=400
-                )
-            fields["agent_max_turns"] = amt
-        else:
-            fields["agent_max_turns"] = None
-
-    if "reasoning_effort" in body:
-        fields["reasoning_effort"] = str(body["reasoning_effort"] or "").strip()
-
-    if "auto_approve" in body:
-        fields["auto_approve"] = bool(body.get("auto_approve", False))
-
-    if "enabled" in body:
-        fields["enabled"] = bool(body.get("enabled", True))
-
-    if "activation" in body:
-        activation = str(body["activation"] or "named").strip()
-        if activation not in _VALID_ACTIVATIONS:
-            return {}, JSONResponse(
-                {"error": f"activation must be one of: {', '.join(sorted(_VALID_ACTIVATIONS))}"},
-                status_code=400,
-            )
-        fields["activation"] = activation
-
-    if "notify_on_complete" in body:
-        nc = str(body.get("notify_on_complete", "[]")).strip()
-        # Normalise empty/whitespace and the legacy ``"{}"`` sentinel
-        # (inherited from migrations 011/021's server_default — older rows
-        # that haven't been touched by migration 051 may still carry it)
-        # to the canonical empty-array literal so a blank field can never
-        # bypass validation and persist a non-JSON value.
-        if not nc or nc == "{}":
-            nc = "[]"
-        if nc != "[]":
-            try:
-                parsed = _json.loads(nc)
-            except (_json.JSONDecodeError, TypeError):
-                return {}, JSONResponse(
-                    {"error": "notify_on_complete must be valid JSON"}, status_code=400
-                )
-            if not isinstance(parsed, list):
-                return {}, JSONResponse(
-                    {"error": "notify_on_complete must be a JSON array"}, status_code=400
-                )
-        fields["notify_on_complete"] = nc
-
-    if "allowed_tools" in body:
-        at_raw = body.get("allowed_tools", "[]")
-        if isinstance(at_raw, list):
-            fields["allowed_tools"] = _json.dumps(at_raw)
-        else:
-            at_str = str(at_raw).strip()
-            if at_str and not at_str.startswith("["):
-                at_str = _json.dumps([t.strip() for t in at_str.split(",") if t.strip()])
-            try:
-                _json.loads(at_str or "[]")
-            except (ValueError, TypeError):
-                at_str = "[]"
-            fields["allowed_tools"] = at_str or "[]"
+    from turnstone.core.skill_field_validation import parse_skill_session_config
 
+    fields, err = parse_skill_session_config(body)
+    if err:
+        return {}, JSONResponse({"error": err}, status_code=400)
     return fields, None
 
 

turnstone/console/static/coordinator/coordinator.js

@@ -139,6 +139,24 @@
 
   let evtSource = null;
   let reconnectAttempts = 0;
+  // Flag set in onerror, cleared in onopen.  Drives the "did we
+  // just recover from a gap?" decision in onopen so the replace-
+  // mode refresh of children/tasks/wait/badge caches fires on
+  // every reconnect — including the common case where native
+  // EventSource auto-reconnect handles the underlying SSE transition
+  // without scheduleReconnect running (which used to be the only
+  // place reconnectAttempts incremented; that path is rarely hit
+  // now that native reconnect handles transient errors).
+  let disconnectedSinceLastOpen = false;
+  // Saved high-water mark for the manual-reconnect path.  The
+  // EventSource constructor can't set custom headers, so when we
+  // construct a fresh source we thread ``?last_event_id=N`` instead
+  // of the browser-native ``Last-Event-ID`` header.  Native
+  // auto-reconnect on the SAME source object uses the header
+  // automatically; this fallback covers the cases where we open a
+  // brand-new EventSource (initial connect, scheduleReconnect after
+  // close).
+  let lastEventId = null;
   let reconnectTimer = null;
 
   // Cache of judge verdicts keyed by call_id.  intent_verdict and
@@ -1901,11 +1919,25 @@
     // reconnectAttempts in onopen — child_ws_* events dispatched while
     // we were disconnected aren't replayed by the events SSE handler,
     // so the client has to pull authoritative state after any gap.
-    const wasReconnecting = reconnectAttempts > 0;
-    const url = "/v1/api/workstreams/" + encodeURIComponent(wsId) + "/events";
+    // Snapshot whether this connect attempt follows a prior
+    // disconnect.  Native EventSource auto-reconnect no longer
+    // routes through scheduleReconnect on the transient-error path,
+    // so the legacy ``reconnectAttempts > 0`` check is always false
+    // after PR-D — use ``disconnectedSinceLastOpen`` (set by onerror,
+    // cleared by onopen below) as the authoritative "was-gap" flag.
+    // Falls back to the legacy semantic for the genuinely manual
+    // case (scheduleReconnect-driven reconnect after CLOSED state).
+    const wasReconnecting = disconnectedSinceLastOpen || reconnectAttempts > 0;
+    let url = "/v1/api/workstreams/" + encodeURIComponent(wsId) + "/events";
+    if (lastEventId) {
+      url += "?last_event_id=" + encodeURIComponent(lastEventId);
+    }
     evtSource = new EventSource(url, { withCredentials: true });
     evtSource.onopen = function () {
       reconnectAttempts = 0;
+      // Clear the "was disconnected" flag now that the gap is
+      // closed.  Future onerror fires will set it again.
+      disconnectedSinceLastOpen = false;
       setSseStatus("live", "ok");
       // Lift the disconnected dim treatment + restore the last known
       // counters; the replay phase will overwrite with authoritative
@@ -1951,35 +1983,78 @@
       }
     };
     evtSource.onerror = function () {
+      // Do NOT close evtSource for transient errors — native
+      // EventSource auto-reconnect handles them with the
+      // ``Last-Event-ID`` header automatically (now that the server
+      // emits ``id:`` on every buffered event).  Closing here would
+      // force a CONNECTING -> CLOSED transition that defeats native
+      // reconnect, which is exactly the reconnect-with-replay defect
+      // PR-D ships to fix.  See
+      // tests/test_app_js.py::test_coord_connectsse_onerror_preserves_native_reconnect.
+      disconnectedSinceLastOpen = true;
       setSseStatus("disconnected", "err");
       // Dim the status bar so a stale reading doesn't read as live.
       statusBarEl.classList.add("ws-sb-disconnected");
       sbTokensEl.textContent = "Reconnecting…";
-      try {
-        evtSource.close();
-      } catch (_) {
-        /* noop */
-      }
-      // Probe the authed detail endpoint to distinguish an expired
-      // session (401) from a transient network error.  On 401, prompt
-      // for login via the shared auth.js overlay instead of spinning
-      // in backoff forever — match the console / server-UI pattern.
-      // On any other outcome, fall through to the normal reconnect
-      // schedule.
+      // 401 probe: expired session is a terminal condition (user
+      // must log in), so we DO close + showLogin in that branch.
+      // Transient errors (network blips, intermediary timeouts) just
+      // let native reconnect run — no scheduleReconnect needed
+      // because the source isn't dead.
       var probe = typeof authFetch === "function" ? authFetch : fetch;
-      probe("/v1/api/workstreams/" + encodeURIComponent(wsId))
-        .then(function (r) {
+      probe("/v1/api/workstreams/" + encodeURIComponent(wsId)).then(
+        function (r) {
           if (r.status === 401 && typeof showLogin === "function") {
+            try {
+              if (evtSource) evtSource.close();
+            } catch (_) {
+              /* noop */
+            }
+            evtSource = null;
+            // Cancel the pending CLOSED-state recovery timer (set
+            // below).  Without this, 5 s later the timer would
+            // observe ``!evtSource`` and call ``scheduleReconnect``,
+            // which would open a new EventSource that gets 401 again
+            // → infinite reconnect loop while the login overlay is
+            // up.  The login flow re-arms ``connectSSE`` after a
+            // successful sign-in via its own callback path.
+            if (reconnectTimer) {
+              clearTimeout(reconnectTimer);
+              reconnectTimer = null;
+            }
             showLogin("Session expired. Please sign in to reconnect.");
-            return;
           }
+        },
+      );
+      // CLOSED-state recovery: native auto-reconnect covers the
+      // transient case (source stays in CONNECTING and eventually
+      // re-opens).  But if the browser gives up — hard 4xx after
+      // retries, intermediary tearing the connection down with
+      // prejudice, etc. — the source transitions to CLOSED and
+      // there is no further native recovery.  Schedule a delayed
+      // check that calls scheduleReconnect if the source is still
+      // CLOSED at that point; scheduleReconnect's exp-backoff +
+      // jitter then opens a new EventSource (threading the saved
+      // lastEventId via the URL query param, so replay still
+      // works across the manual reconnect).  Cancel/replace the
+      // existing timer so successive onerror fires don't pile up
+      // multiple checks for the same source.  The 401 branch above
+      // ALSO cancels this timer when it fires — see comment there.
+      if (reconnectTimer) clearTimeout(reconnectTimer);
+      reconnectTimer = setTimeout(function () {
+        reconnectTimer = null;
+        if (!evtSource || evtSource.readyState === EventSource.CLOSED) {
           scheduleReconnect();
-        })
-        .catch(function () {
-          scheduleReconnect();
-        });
+        }
+      }, 5000);
     };
     evtSource.onmessage = function (event) {
+      // Capture lastEventId BEFORE JSON.parse so a malformed event
+      // doesn't desync the manual-reconnect fallback from native
+      // auto-reconnect.
+      if (evtSource && evtSource.lastEventId) {
+        lastEventId = evtSource.lastEventId;
+      }
       let data = null;
       try {
         data = JSON.parse(event.data);

turnstone/console/static/governance.js

@@ -180,6 +180,10 @@ const _PERMISSION_SECTIONS = [
     label: "Workstreams & Tools",
     permissions: ["workstreams.create", "workstreams.close", "tools.approve"],
   },
+  {
+    label: "Model",
+    permissions: ["model.skills.write"],
+  },
 ];
 
 // Flat list — kept for any caller that wants the full permission
@@ -1209,7 +1213,9 @@ function submitCreateTemplate() {
   const csTemp = document.getElementById("csk-temperature").value.trim();
   const csMaxTok = document.getElementById("csk-max-tokens").value.trim();
   const csBudget = document.getElementById("csk-token-budget").value.trim();
-  const csMaxTurns = document.getElementById("csk-agent-max-turns").value.trim();
+  const csMaxTurns = document
+    .getElementById("csk-agent-max-turns")
+    .value.trim();
   const csAllowed = (
     document.getElementById("csk-allowed-tools").value || ""
   ).trim();
@@ -1239,7 +1245,9 @@ function submitCreateTemplate() {
     }
   }
   document.getElementById("ctm-submit").disabled = true;
-  const csVersion = (document.getElementById("skill-version").value || "").trim();
+  const csVersion = (
+    document.getElementById("skill-version").value || ""
+  ).trim();
   const createBody = {
     name: name,
     category: document.getElementById("ctm-category").value,
@@ -1952,7 +1960,9 @@ function submitEditTemplate() {
   const esTemp = document.getElementById("esk-temperature").value.trim();
   const esMaxTok = document.getElementById("esk-max-tokens").value.trim();
   const esBudget = document.getElementById("esk-token-budget").value.trim();
-  const esMaxTurns = document.getElementById("esk-agent-max-turns").value.trim();
+  const esMaxTurns = document
+    .getElementById("esk-agent-max-turns")
+    .value.trim();
   const esAllowed = (
     document.getElementById("esk-allowed-tools").value || ""
   ).trim();
@@ -2052,7 +2062,8 @@ function loadGovUsage() {
   const sinceStr = since.toISOString().slice(0, 19);
 
   // Fetch summary + breakdown in parallel
-  const summaryUrl = "/v1/api/admin/usage?since=" + encodeURIComponent(sinceStr);
+  const summaryUrl =
+    "/v1/api/admin/usage?since=" + encodeURIComponent(sinceStr);
   const breakdownUrl = summaryUrl + "&group_by=" + _govUsageGroupBy;
 
   Promise.all([
@@ -2121,7 +2132,8 @@ function _renderGovUsage(summary, breakdown) {
   if (items.length) {
     let maxVal = 0;
     for (let i = 0; i < items.length; i++) {
-      const v = (items[i].prompt_tokens || 0) + (items[i].completion_tokens || 0);
+      const v =
+        (items[i].prompt_tokens || 0) + (items[i].completion_tokens || 0);
       if (v > maxVal) maxVal = v;
     }
     html += '<div class="usage-chart">';
@@ -2627,7 +2639,8 @@ function searchSkillDiscover() {
   const searchBtn = document.getElementById("skill-discover-search-btn");
   if (searchBtn) searchBtn.disabled = true;
 
-  const url = "/v1/api/admin/skills/discover?limit=20&q=" + encodeURIComponent(q);
+  const url =
+    "/v1/api/admin/skills/discover?limit=20&q=" + encodeURIComponent(q);
 
   authFetch(url)
     .then(function (r) {
@@ -3195,7 +3208,9 @@ let _eogpTriggerEl = null;
 function switchJudgeSection(section) {
   const sections = document.querySelectorAll(".judge-section");
   for (let i = 0; i < sections.length; i++) sections[i].style.display = "none";
-  const switcher = document.querySelector("#admin-judge .admin-subtab-switcher");
+  const switcher = document.querySelector(
+    "#admin-judge .admin-subtab-switcher",
+  );
   const btns = switcher ? switcher.querySelectorAll(".admin-subtab-btn") : [];
   for (let i = 0; i < btns.length; i++) {
     const isActive = btns[i].getAttribute("data-section") === section;
@@ -3209,7 +3224,9 @@ function switchJudgeSection(section) {
 
 // Arrow key navigation for judge sub-section tabs
 (function () {
-  const switcher = document.querySelector("#admin-judge .admin-subtab-switcher");
+  const switcher = document.querySelector(
+    "#admin-judge .admin-subtab-switcher",
+  );
   if (!switcher) return;
   switcher.addEventListener("keydown", function (e) {
     if (e.key !== "ArrowLeft" && e.key !== "ArrowRight") return;

turnstone/core/auth.py

@@ -116,6 +116,45 @@ def _load_user_permissions(storage: Any, user_id: str) -> set[str]:
         return set()
 
 
+def user_has_permission(user_id: str, permission: str, *, storage: Any = None) -> bool:
+    """Return True if *user_id* holds *permission*.
+
+    For in-process callers — specifically the model-facing tool exec
+    path — that need to gate a write capability without an HTTP
+    middleware in the loop.  HTTP handlers stay on
+    :func:`require_permission`, which carries the JSONResponse-shaped
+    denial.  This helper returns a plain bool so the tool layer can
+    surface the denial in whatever shape it already uses (typically a
+    ``_coord_tool_error`` row).
+
+    Empty ``user_id`` returns False without a storage lookup — there's
+    no anonymous holder of any permission.  Storage lookup failures
+    are swallowed (logged at warning by ``_load_user_permissions``) and
+    return False — fail-closed on the permission check rather than
+    fail-open if the roles backend is briefly unavailable.
+
+    No service-scope bypass.  ``require_permission`` lets a service-
+    scoped JWT skip the check by default; this helper has no equivalent
+    because the in-process model-tool path doesn't carry an
+    :class:`AuthResult` (scopes are an HTTP-layer concept).  A service
+    token reaching here either resolves to a real ``user_id`` with the
+    grant or has no ``user_id`` and short-circuits to False.  If a
+    legitimate service-scope caller ever needs to bypass, expose
+    ``allow_service_bypass`` here mirroring ``require_permission`` and
+    thread the originating scope through the call site — don't try to
+    infer it from the lone ``user_id``.
+    """
+    if not user_id:
+        return False
+    if storage is None:
+        from turnstone.core.storage._registry import get_storage
+
+        storage = get_storage()
+    if storage is None:
+        return False
+    return permission in _load_user_permissions(storage, user_id)
+
+
 def _permissions_to_scopes(permissions: set[str]) -> frozenset[str]:
     """Derive legacy scopes from a granular permission set."""
     scopes: set[str] = set()

turnstone/core/session_routes.py

@@ -1439,24 +1439,87 @@ def make_events_handler(cfg: SessionEndpointConfig) -> Handler:
             # half-built shape.
             return JSONResponse({"error": "session has no UI"}, status_code=409)
 
-        # Register the listener AND snapshot the per-turn inflight
-        # buffers in one atomic-against-writers step. The snapshot
-        # (content / reasoning text-so-far for the current turn) is
-        # yielded as a one-shot ``in_progress_snapshot`` event after
-        # the replay phase; it lets a mid-stream page refresh restore
-        # the partial assistant text without waiting for the response
-        # to complete. ``snap.seq`` is captured to dedup live events
-        # whose ``_seq`` is already in the snapshot payload (race-
-        # free composition with ``on_content_token`` /
-        # ``on_reasoning_token`` writers across the two-lock surface
-        # — see ``register_listener_with_in_progress_snapshot``).
+        # ``Last-Event-ID`` resume: native EventSource auto-reconnect
+        # sends the header; the manual-reconnect path (which uses
+        # ``new EventSource(url)`` and can't set custom headers) sends
+        # ``?last_event_id=N``.  Accept both; malformed values fall
+        # back to fresh-connect semantics so a broken intermediary
+        # can't break replay for a client that genuinely lost no
+        # events.
+        last_event_id_raw = request.headers.get("Last-Event-ID") or request.query_params.get(
+            "last_event_id"
+        )
+        last_event_id: int | None
+        try:
+            last_event_id = int(last_event_id_raw) if last_event_id_raw else None
+        except (TypeError, ValueError):
+            last_event_id = None
+
+        # Three replay shapes:
+        #   - ``last_event_id is None`` → ``"fresh"`` (today's behaviour):
+        #     replay_cb + state_change + in_progress_snapshot + live.
+        #   - ``last_event_id`` + buffer covers gap → ``"replay_ok"``:
+        #     emit buffered events past the id, SKIP replay_cb /
+        #     state_change / in_progress_snapshot (the buffered stream
+        #     already contains them), then live drain.
+        #   - ``last_event_id`` + buffer too short → ``"truncated"``:
+        #     emit a ``replay_truncated`` envelope so the client knows
+        #     it lost live ticks, then fall through to the fresh
+        #     replay (history / state_change / in_progress_snapshot)
+        #     as the recovery floor.
         # The placeholder-UI guard above (which 409s when
         # ``_register_listener`` is missing) already proves that
         # ``ui`` is a ``SessionUIBase`` subclass, so the cast is
         # tightening the type, not weakening it.
         ui_base = cast("SessionUIBase", ui)
-        client_queue, in_progress_snap = ui_base.register_listener_with_in_progress_snapshot()
-        snap_seq: int = in_progress_snap["seq"]
+        replay_status: str
+        replay_events: list[dict[str, Any]] = []
+        lost_count = 0
+        earliest_available_id = 0
+        in_progress_snap: dict[str, Any]
+        snap_seq: int = 0
+        if last_event_id is None:
+            replay_status = "fresh"
+            client_queue, in_progress_snap = ui_base.register_listener_with_in_progress_snapshot()
+            snap_seq = in_progress_snap["seq"]
+        else:
+            (
+                client_queue,
+                replay_events,
+                replay_status,
+                lost_count,
+                earliest_available_id,
+                snapshot,
+            ) = ui_base.register_listener_with_replay(last_event_id)
+            if replay_status == "truncated":
+                # Truncated → emit ``replay_truncated`` envelope, then
+                # the snapshot is the recovery floor.  ``snap_seq``
+                # MUST come from the snapshot capture (not 0), because
+                # writers can race between
+                # ``register_listener_with_replay`` returning and our
+                # first live-drain read: any token event landing in
+                # the listener queue between registration and the
+                # captured ``_event_id`` is ALSO covered by the
+                # snapshot's content/reasoning text, and would
+                # double-render without the ``_seq <= snap_seq`` dedup
+                # filter on the live path.  The helper captured both
+                # under the same nested-lock acquire, so this
+                # ``snap_seq`` is exactly the high-water mark
+                # corresponding to the snapshot text.
+                in_progress_snap = snapshot
+                snap_seq = snapshot["seq"]
+            else:
+                # ``replay_ok``: the buffered events ARE the partial
+                # token stream (no separate snapshot needed); the
+                # synthetic snapshot/state_change/history emission is
+                # skipped by the events handler.  No live-dedup
+                # filtering required because the buffered events
+                # themselves are the cutoff — anything past the last
+                # replayed event id is genuinely new live traffic
+                # that lands in the listener queue after the buffer
+                # snapshot was taken (atomic-against-writers under
+                # the registration's nested locks).
+                in_progress_snap = {"content": "", "reasoning": "", "seq": 0}
 
         # Per-kind executor for the blocking ``client_queue.get``
         # wait. Interactive returns its dedicated 200-thread
@@ -1475,101 +1538,165 @@ def make_events_handler(cfg: SessionEndpointConfig) -> Handler:
 
         async def event_generator() -> Any:
             import functools
+            import random
 
             _metrics.record_sse_connect()
             loop = asyncio.get_running_loop()
+
+            def _format_event(event: dict[str, Any]) -> dict[str, str]:
+                """Strip internal plumbing fields, attach SSE ``id:`` if present.
+
+                Shallow-copies the dict before any mutation because
+                ``_enqueue`` puts ONE reference into every listener
+                queue (no per-listener copy) and stores the SAME
+                reference in the per-ws ring buffer.  Without a
+                shallow copy here, listener A's pop of ``_event_id``
+                would silently strip the field from listener B's view
+                AND from the buffer's view, breaking the replay
+                guarantee for a later-arriving subscriber.
+                """
+                ev_copy = dict(event)
+                eid = ev_copy.pop("_event_id", None)
+                # Strip ``_seq`` here too — it's internal plumbing for
+                # the snapshot dedup; clients never need to see it on
+                # the wire.  The fresh-path live drain filters on
+                # ``_seq`` BEFORE calling this helper.
+                ev_copy.pop("_seq", None)
+                out: dict[str, str] = {"data": json.dumps(ev_copy)}
+                if eid is not None:
+                    out["id"] = str(eid)
+                return out
+
             try:
-                # Replay phase — stream the kind-specific initial
-                # payload one event at a time so the client sees the
-                # first byte immediately (interactive's ``connected``
-                # event is the very first yield, before the heavier
-                # ``status`` / ``history`` work runs). Pre-building
-                # the replay into a list would block time-to-first-
-                # byte until the entire replay materialized AND let
-                # the listener queue accumulate (potentially over its
-                # 500-slot cap on a chatty mid-generation workstream)
-                # while replay was being built.
-                if replay_cb is not None:
-                    # Kind-specific async prep — runs before the sync
-                    # replay generator iterates so blocking storage
-                    # I/O lands in the executor pool rather than the
-                    # event loop's hot path. Interactive uses this
-                    # to pre-load verdict indexes; coord skips.
-                    if cfg.events_replay_prepare is not None:
-                        try:
-                            await cfg.events_replay_prepare(ws, ui, request)
-                        except Exception:
-                            log.debug(
-                                "ws.events.replay_prepare_failed ws=%s",
-                                ws_id[:8],
-                                exc_info=True,
-                            )
-                    try:
-                        for ev in replay_cb(ws, ui, request):
-                            yield {"data": json.dumps(ev)}
-                    except Exception:
-                        # Replay is observational — never let a
-                        # snapshot bug block the live stream. Log
-                        # and continue with whatever partial replay
-                        # was already yielded.
-                        log.debug(
-                            "ws.events.replay_failed ws=%s",
-                            ws_id[:8],
-                            exc_info=True,
-                        )
-                # Refresh-resume tail: emit the current workstream
-                # state (so the composer flips to stop-mode on a mid-
-                # stream refresh — ``state_change`` is the only event
-                # the JS busy machine listens to, and the kind-specific
-                # replay above doesn't yield it) and the in-progress
-                # snapshot (so partial content / reasoning re-renders
-                # immediately, instead of waiting for the next live
-                # token). Both are best-effort — a ws.state read
-                # failure or empty buffers just yields nothing extra.
-                try:
-                    cur_state = getattr(ws.state, "value", None)
-                    if isinstance(cur_state, str) and cur_state:
+                # Per-stream reconnect interval jitter.  Without this,
+                # all panes on a workstream disconnect together and
+                # reconnect in lockstep at the same backoff intervals
+                # (EventSource's default ~3 s with no jitter, or
+                # whatever ``retry:`` value the server last sent).
+                # 2.5 – 4.5 s spread keeps the average reconnect rate
+                # below today's ping cadence while staggering peaks.
+                yield {"retry": random.randint(2500, 4500)}
+
+                if replay_status == "replay_ok":
+                    # Buffered events already cover everything since
+                    # the client's ``Last-Event-ID`` — skip the
+                    # synthetic replay (history / state_change /
+                    # in_progress_snapshot) which would otherwise
+                    # double-render content the buffer already
+                    # contains.  Yield buffered events in order with
+                    # their ``_event_id`` as SSE ``id:`` so a
+                    # disconnect mid-replay resumes from the latest
+                    # buffered id, not the original ``last_event_id``.
+                    for ev in replay_events:
+                        yield _format_event(ev)
+                else:
+                    # ``fresh`` or ``truncated`` — both run the
+                    # synthetic replay (kind-specific replay_cb +
+                    # state_change + in_progress_snapshot).  On
+                    # ``truncated`` we emit the explicit envelope
+                    # first so the client knows the buffer couldn't
+                    # cover the gap and treats the snapshot below as
+                    # the recovery floor.
+                    if replay_status == "truncated":
                         yield {
                             "data": json.dumps(
                                 {
-                                    "type": "state_change",
-                                    "state": cur_state,
+                                    "type": "replay_truncated",
+                                    "ws_id": ws_id,
+                                    "lost_count": lost_count,
+                                    "earliest_available_id": earliest_available_id,
+                                }
+                            )
+                        }
+
+                    # Replay phase — stream the kind-specific initial
+                    # payload one event at a time so the client sees
+                    # the first byte immediately.  Pre-building into
+                    # a list would block time-to-first-byte until the
+                    # entire replay materialized AND let the listener
+                    # queue accumulate (potentially over its 500-slot
+                    # cap on a chatty mid-generation workstream)
+                    # while replay was being built.  Synthetic
+                    # events carry no ``_event_id`` — they intentionally
+                    # don't advance the client's ``lastEventId``, so
+                    # a mid-replay disconnect reconnects with the
+                    # last BUFFERED id (or none on truly-fresh
+                    # connect), which is what the server can replay.
+                    if replay_cb is not None:
+                        # Kind-specific async prep — runs before the
+                        # sync replay generator iterates so blocking
+                        # storage I/O lands in the executor pool
+                        # rather than the event loop's hot path.
+                        if cfg.events_replay_prepare is not None:
+                            try:
+                                await cfg.events_replay_prepare(ws, ui, request)
+                            except Exception:
+                                log.debug(
+                                    "ws.events.replay_prepare_failed ws=%s",
+                                    ws_id[:8],
+                                    exc_info=True,
+                                )
+                        try:
+                            for ev in replay_cb(ws, ui, request):
+                                yield {"data": json.dumps(ev)}
+                        except Exception:
+                            # Replay is observational — never let a
+                            # snapshot bug block the live stream.
+                            log.debug(
+                                "ws.events.replay_failed ws=%s",
+                                ws_id[:8],
+                                exc_info=True,
+                            )
+                    # Refresh-resume tail: emit the current
+                    # workstream state and the in-progress snapshot.
+                    # Both are best-effort — a ws.state read failure
+                    # or empty buffers just yields nothing extra.
+                    try:
+                        cur_state = getattr(ws.state, "value", None)
+                        if isinstance(cur_state, str) and cur_state:
+                            yield {
+                                "data": json.dumps(
+                                    {
+                                        "type": "state_change",
+                                        "state": cur_state,
+                                        "ws_id": ws_id,
+                                    }
+                                )
+                            }
+                    except Exception:
+                        log.debug(
+                            "ws.events.state_change_replay_failed ws=%s",
+                            ws_id[:8],
+                            exc_info=True,
+                        )
+                    if in_progress_snap["content"] or in_progress_snap["reasoning"]:
+                        yield {
+                            "data": json.dumps(
+                                {
+                                    "type": "in_progress_snapshot",
+                                    "content": in_progress_snap["content"],
+                                    "reasoning": in_progress_snap["reasoning"],
                                     "ws_id": ws_id,
                                 }
                             )
                         }
-                except Exception:
-                    log.debug(
-                        "ws.events.state_change_replay_failed ws=%s",
-                        ws_id[:8],
-                        exc_info=True,
-                    )
-                if in_progress_snap["content"] or in_progress_snap["reasoning"]:
-                    yield {
-                        "data": json.dumps(
-                            {
-                                "type": "in_progress_snapshot",
-                                "content": in_progress_snap["content"],
-                                "reasoning": in_progress_snap["reasoning"],
-                                "ws_id": ws_id,
-                            }
-                        )
-                    }
                 # Live phase — drain the per-UI listener queue
                 # until either the workstream closes or the client
                 # disconnects. 5s poll matches pre-lift interactive
                 # (the ``is_disconnected`` probe between polls covers
-                # cancel-detection latency the timeout would otherwise
-                # gate; shortening to 1s 5x'd the wakeup rate without
-                # any client-observable benefit).
+                # cancel-detection latency the timeout would
+                # otherwise gate; shortening to 1s 5x'd the wakeup
+                # rate without any client-observable benefit).
                 #
                 # ``_seq`` filter: ``on_content_token`` /
                 # ``on_reasoning_token`` tag each emit with the
-                # per-turn inflight seq counter. Events whose seq is
-                # already covered by the snapshot we just yielded get
-                # dropped to avoid double-rendering. ``_seq`` is
-                # internal plumbing — strip before yielding so the
-                # SDK / JS clients never see it.
+                # per-ws event counter.  On the ``fresh`` path,
+                # events whose seq is already covered by the
+                # snapshot we just yielded get dropped to avoid
+                # double-rendering.  On ``replay_ok`` / ``truncated``
+                # paths, ``snap_seq`` is 0 so no live event is
+                # filtered — the replay buffer (or replay_truncated
+                # envelope) has already established the cutoff.
                 while True:
                     if await request.is_disconnected():
                         return
@@ -1582,21 +1709,10 @@ def make_events_handler(cfg: SessionEndpointConfig) -> Handler:
                         continue  # ping keeps the connection alive
                     if event.get("type") == "ws_closed":
                         return
-                    # ``_enqueue`` puts ONE dict reference into every
-                    # listener queue (no per-listener copy). Multiple
-                    # SSE coroutines on the same workstream observe the
-                    # same dict; ``yield`` is an await point, so one
-                    # listener's ``del event["_seq"]`` would race
-                    # another listener's seq-filter read. Shallow-copy
-                    # before any mutation so each listener can filter
-                    # / strip ``_seq`` without disturbing peers.
-                    event = dict(event)
                     seq = event.get("_seq")
-                    if seq is not None:
-                        if seq <= snap_seq:
-                            continue
-                        del event["_seq"]
-                    yield {"data": json.dumps(event)}
+                    if seq is not None and seq <= snap_seq:
+                        continue
+                    yield _format_event(event)
             finally:
                 _metrics.record_sse_disconnect()
                 unregister(client_queue)

turnstone/core/session_ui_base.py

@@ -23,9 +23,11 @@ storage/transport routing is kind-specific.
 
 from __future__ import annotations
 
+import collections
 import contextlib
 import copy
 import json
+import os
 import queue
 import threading
 import time
@@ -42,6 +44,65 @@ log = get_logger(__name__)
 _DEFAULT_LISTENER_QUEUE_MAX = 500
 
 
+def _resolve_event_buffer_max() -> int:
+    """Read ``TURNSTONE_SSE_EVENT_BUFFER_MAX`` env override at import time.
+
+    Default 50000 events.  Two pressures push the cap larger than a
+    casual reading of "how many events does an SSE stream see":
+
+    1. Local-inference deployments stream at 500–2000 tok/s per
+       active model.  Each token is an ``_enqueue`` call, so a single
+       active workstream can fire ~2000 events/sec sustained.  At
+       the 50000 cap that buys ~25 s of pure token streaming before
+       truncation; at typical cloud-provider rates (50–200 events/sec
+       per stream) it's minutes of coverage.
+    2. Browsers throttle the SSE-drain microtask aggressively when
+       the tab isn't visible (Chrome's background-tab budget drops
+       to ~1 wake/min after ~5 min hidden).  A backgrounded tab can
+       legitimately go tens of seconds without draining its
+       EventSource buffer — and PR-G (drop-pings-let-it-die)
+       deliberately closes those connections on hide.  Reconnect-with-
+       replay is the recovery path; if the buffer evicted in the
+       interim, the snapshot floor is all that's left.
+
+    Why not coalesce consecutive content/reasoning tokens?  A naive
+    text-merge breaks the replay-slice semantic: a coalesced entry
+    has the latest ``_event_id`` but text that includes content the
+    client already received under an earlier id, so any consumer
+    with ``last_event_id`` falling INSIDE the coalesced span would
+    double-render.  A correctness-preserving coalesce would need a
+    per-consumer high-water tracker we deliberately don't maintain
+    (consumers register and disconnect independently).  Bigger cap
+    + simple per-event storage avoids the trap.
+
+    Memory cost is ~200–500 bytes per event (deque node + dict
+    overhead + payload), so 50000 × 100-ws design ceiling caps at
+    roughly 2.5 GB worst-case — and practically never anywhere
+    close because the cap is the per-ws ceiling, not per-ws steady-
+    state.  Operators on heavier workloads can raise via
+    ``TURNSTONE_SSE_EVENT_BUFFER_MAX``; below-cap reconnects always
+    hit the replay path, above-cap reconnects fall back to the
+    snapshot recovery floor with an explicit ``replay_truncated``
+    envelope.
+    """
+    raw = os.environ.get("TURNSTONE_SSE_EVENT_BUFFER_MAX", "").strip()
+    default = 50000
+    if not raw:
+        return default
+    try:
+        n = int(raw)
+    except ValueError:
+        return default
+    return n if n > 0 else default
+
+
+# Per-ws ring buffer for ``Last-Event-ID`` SSE replay.  Holds the most
+# recent events keyed by monotonic ``_event_id``; deque ``maxlen`` evicts
+# oldest automatically.  See :func:`_resolve_event_buffer_max` for the
+# sizing rationale (why 50000 and not 2000; why no in-buffer coalescing).
+_EVENT_BUFFER_MAX = _resolve_event_buffer_max()
+
+
 # Cap on the assistant content / reasoning accumulators. Used by two
 # independent buffer pairs:
 #  - ``_ws_turn_content`` (multi-turn, drained at idle/error) — the
@@ -140,6 +201,36 @@ class SessionUIBase:
         # SSE listener fan-out — one queue per connected browser tab.
         self._listeners: list[queue.Queue[dict[str, Any]]] = []
         self._listeners_lock = threading.Lock()
+        # Per-ws event ring buffer for ``Last-Event-ID`` SSE replay.
+        # Holds ``(event_id, event_dict)`` tuples; deque ``maxlen``
+        # evicts the oldest automatically when the cap is hit.  The
+        # listener fan-out path stamps every event with a monotonic
+        # ``_event_id`` (see :meth:`_enqueue`) and appends here under
+        # the same ``_listeners_lock`` that gates the per-listener
+        # queues — keeps the buffer and the live fan-out in lockstep.
+        # A reconnecting client with a ``Last-Event-ID`` header (or
+        # ``?last_event_id=N`` query-param fallback for manual reconnect
+        # paths that can't set custom headers) is served the slice of
+        # the buffer past that id; clients whose ``Last-Event-ID``
+        # predates the buffer's earliest retained id get a
+        # ``replay_truncated`` envelope plus the in-progress snapshot
+        # as the recovery floor.  Guarded by ``_listeners_lock`` (NOT
+        # ``_ws_lock``) so a writer holding ``_ws_lock`` for the
+        # inflight-buffer append doesn't serialize the buffer write
+        # against unrelated readers.
+        self._event_buffer: collections.deque[tuple[int, dict[str, Any]]] = collections.deque(
+            maxlen=_EVENT_BUFFER_MAX
+        )
+        # Monotonic per-ws event counter.  Stamps every fan-out event
+        # (every ``_enqueue`` call) and also drives the existing
+        # ``_seq`` snapshot-dedup tag on token events (``content`` /
+        # ``reasoning``) — one counter, two consumers.  Renamed from
+        # the pre-replay ``_ws_inflight_seq`` because the counter now
+        # spans every event, not just the inflight token stream.
+        # Guarded by ``_listeners_lock`` (incremented under that lock
+        # in :meth:`_enqueue`); the snapshot helper for the in-progress
+        # replay path captures it under ``_listeners_lock`` too.
+        self._event_id: int = 0
         # Approval blocking — the worker thread calls approve_tools
         # which waits on _approval_event; the /approve endpoint sets
         # it via resolve_approval.
@@ -238,18 +329,16 @@ class SessionUIBase:
         # each turn by :meth:`on_turn_start` (separate from the multi-
         # turn IDLE-piggyback buffer above so prior committed turns
         # don't leak into the snapshot and double-render against the
-        # replayed history). ``_ws_inflight_seq`` is a monotonic
-        # counter incremented on EVERY emit (even when the cap
-        # rejected the buffer append) so a subscriber registering
-        # after the cap is hit doesn't have subsequent live tokens
-        # filter-dropped against a stalled ``snap_seq`` — the events
-        # handler dedups live events whose ``_seq`` is at-or-below
-        # the snapshot's seq (already in the snapshot payload).
+        # replayed history).  The per-turn dedup-tag counter
+        # (``_event_id``) is initialised above alongside the per-ws
+        # event ring buffer — one monotonic counter drives both the
+        # ``Last-Event-ID`` replay slice AND the existing snapshot
+        # ``_seq <= snap_seq`` filter; see :meth:`_enqueue` for the
+        # stamping pattern.
         self._ws_inflight_content: list[str] = []
         self._ws_inflight_content_size: int = 0
         self._ws_inflight_reasoning: list[str] = []
         self._ws_inflight_reasoning_size: int = 0
-        self._ws_inflight_seq: int = 0
         # Last broadcast (activity, activity_state) tuple — used by
         # :meth:`_broadcast_activity` overrides to dedup back-to-back
         # identical activity ticks. Tool-heavy turns can fire many
@@ -287,12 +376,35 @@ class SessionUIBase:
 
         Stamps ``ws_id`` on the payload if not already present so the
         browser can validate it belongs to the pane's current
-        workstream. Shallow-copies on stamp to avoid mutating a
-        caller-owned dict.
+        workstream.  Stamps a monotonic ``_event_id`` on every event
+        (drives the ``Last-Event-ID`` replay buffer) and additionally
+        stamps the per-turn snapshot dedup tag ``_seq`` on token
+        events (``content`` / ``reasoning``) so the existing
+        in-progress snapshot dedup at the events handler stays
+        byte-identical.  Shallow-copies before each stamp so a
+        caller-owned dict is never mutated.
+
+        The counter increment, the buffer append, AND the listener
+        snapshot all run under ``_listeners_lock`` so a concurrent
+        :meth:`register_listener_with_in_progress_snapshot` or
+        :meth:`register_listener_with_replay` sees a consistent
+        ``(event_id, listeners, buffer)`` tuple — no event is
+        fanned out to a not-yet-registered listener AND missing from
+        the replay buffer.
         """
         if "ws_id" not in data:
             data = {**data, "ws_id": self.ws_id}
         with self._listeners_lock:
+            self._event_id += 1
+            event_id = self._event_id
+            data = {**data, "_event_id": event_id}
+            if data.get("type") in ("content", "reasoning"):
+                # Preserve the existing dedup contract: only token
+                # events carry the ``_seq`` tag.  Non-token events
+                # (``tool_started``, ``state_change``, …) keep
+                # bypassing the snapshot filter by absence of ``_seq``.
+                data = {**data, "_seq": event_id}
+            self._event_buffer.append((event_id, data))
             snapshot = list(self._listeners)
         for lq in snapshot:
             with contextlib.suppress(queue.Full):
@@ -317,27 +429,29 @@ class SessionUIBase:
     ) -> tuple[queue.Queue[dict[str, Any]], dict[str, Any]]:
         """Register a listener AND snapshot the per-turn inflight buffers.
 
-        Used by :func:`make_events_handler` so a fresh SSE subscriber
+        Used by :func:`make_events_handler` (the fresh-connect path,
+        and the ``replay_truncated`` fallback path) so a SSE subscriber
         connecting mid-stream can be told the in-progress turn's content
         and reasoning text-so-far in a one-shot ``in_progress_snapshot``
         event, on top of the kind-specific replay (history / pending).
+        The ``Last-Event-ID`` replay path (see
+        :meth:`register_listener_with_replay`) bypasses this — the
+        buffered events already carry the partial token stream.
 
-        Race-free composition with the on-token writers, even though
-        ``on_content_token`` / ``on_reasoning_token`` cross two locks
-        (``_ws_lock`` for the buffer append, ``_listeners_lock`` for
-        the fan-out enqueue). The trick is the seq counter —
-        ``_ws_inflight_seq`` is incremented under ``_ws_lock`` on
-        every emit (even when the cap rejected the append, so a
-        subscriber that registers after the cap is hit doesn't have
-        subsequent live tokens filter-dropped against a stalled
-        snap_seq). This method captures it alongside the buffer
-        contents under the same ``_ws_lock``, and the events handler's
-        live drain drops any incoming event whose ``_seq`` is at-or-
-        below the captured ``snap.seq`` (already in the snapshot
-        payload). Lock acquisition order: ``_listeners_lock`` (inside
-        :meth:`_register_listener`) is taken and released first, THEN
-        ``_ws_lock`` for the snapshot copy. Sequential — no nesting,
-        no deadlock with the writer's reverse order.
+        Lock acquisition order: ``_ws_lock`` (outer) → ``_listeners_lock``
+        (inner) — matches the writer's order in :meth:`on_content_token`
+        / :meth:`on_reasoning_token` (``_ws_lock`` then ``_enqueue``'s
+        ``_listeners_lock``).  Nested under both locks we read
+        ``inflight_content``, ``inflight_reasoning``, AND the
+        ``_event_id`` counter as a consistent triple, plus register
+        the listener.  Writers calling :meth:`_enqueue` block on
+        ``_listeners_lock`` for the snapshot's duration so no event is
+        fanned out between counter-read and listener-registration —
+        every event with ``_event_id > snap_seq`` lands in the
+        registered listener's queue, every event with
+        ``_event_id <= snap_seq`` is already covered by the snapshot's
+        ``content`` / ``reasoning`` text or by token events that the
+        events handler's ``_seq <= snap_seq`` filter drops.
 
         Returns ``(client_queue, snapshot_dict)`` where ``snapshot_dict``
         has keys ``content`` (str), ``reasoning`` (str), ``seq`` (int).
@@ -345,23 +459,135 @@ class SessionUIBase:
         whether to yield the event at all (empty snapshots are common
         between turns and on freshly-opened workstreams).
 
-        Joins the captured fragments OUTSIDE the lock — bounded at
+        Joins the captured fragments OUTSIDE the locks — bounded at
         ``_MAX_TURN_CONTENT_CHARS`` but still O(n) over fragments, so
         worth not blocking concurrent on-token writers for the
-        duration. The shallow ``list(...)`` copy under the lock means
-        subsequent appends to the live buffer don't mutate our view.
+        duration. The shallow ``list(...)`` copies under the lock mean
+        subsequent appends to the live buffers don't mutate our view.
         """
-        client_queue = self._register_listener(maxsize=maxsize)
+        client_queue: queue.Queue[dict[str, Any]] = queue.Queue(maxsize=maxsize)
         with self._ws_lock:
             captured_content = list(self._ws_inflight_content)
             captured_reasoning = list(self._ws_inflight_reasoning)
-            snap_seq = self._ws_inflight_seq
+            with self._listeners_lock:
+                self._listeners.append(client_queue)
+                snap_seq = self._event_id
         return client_queue, {
             "content": "".join(captured_content),
             "reasoning": "".join(captured_reasoning),
             "seq": snap_seq,
         }
 
+    def register_listener_with_replay(
+        self,
+        last_event_id: int,
+        maxsize: int = _DEFAULT_LISTENER_QUEUE_MAX,
+    ) -> tuple[
+        queue.Queue[dict[str, Any]],
+        list[dict[str, Any]],
+        str,
+        int,
+        int,
+        dict[str, Any],
+    ]:
+        """Register a listener AND capture buffered events for replay
+        AND snapshot the per-turn inflight content/reasoning + snap_seq
+        in one atomic-against-writers step.
+
+        Used by :func:`make_events_handler` when the client sends
+        ``Last-Event-ID`` (header or ``?last_event_id=`` query-param
+        fallback for the manual-reconnect path).  Returns
+
+            ``(client_queue, replay_events, status, lost_count,
+            earliest_available_id, snapshot)``
+
+        where ``status`` is one of ``"replay_ok"`` (caller emits the
+        replay events then drops into live drain, skipping
+        ``replay_cb`` / ``state_change`` / ``in_progress_snapshot``)
+        or ``"truncated"`` (caller emits a ``replay_truncated``
+        envelope then falls through to the fresh-connect replay path
+        as the recovery floor — the snapshot picks up the partial
+        content/reasoning that the evicted events would have carried).
+        ``snapshot`` has the same shape as
+        :meth:`register_listener_with_in_progress_snapshot`'s second
+        return value: ``{"content": str, "reasoning": str, "seq": int}``.
+
+        Atomicity contract: under ``_ws_lock`` (outer) + ``_listeners_lock``
+        (inner) — matches writer order in :meth:`on_content_token` —
+        we snapshot the buffer, the listener registration, the
+        inflight content/reasoning, AND the ``_event_id`` counter as
+        a consistent tuple.  Writers' :meth:`_enqueue` blocks on
+        ``_listeners_lock`` for the duration, so events either
+          - land in the buffer snapshot but NOT the listener queue
+            (writer ran before our lock acquire — caught by the
+            replay slice on the ``replay_ok`` path, or by the
+            content snapshot on the ``truncated`` path), or
+          - land in the listener queue but NOT the buffer snapshot
+            (writer ran after our lock release — live drain handles
+            them, ``_event_id`` is strictly above
+            ``earliest_available_id`` AND strictly above
+            ``snapshot["seq"]``).
+        No event is double-delivered, none is lost across the
+        registration boundary.  Crucially, the truncated path can
+        now use ``snapshot["seq"]`` as the live-drain ``snap_seq``
+        filter — the events handler's existing ``_seq <= snap_seq``
+        dedup catches any token event that landed in the listener
+        queue AND was covered by the snapshot's content/reasoning
+        text (prevents double-rendering after a truncated emit).
+
+        ``last_event_id`` semantics:
+          - ``< earliest_available_id - 1`` → ``"truncated"``.
+            ``lost_count`` is the minimum gap (the buffer may have
+            evicted strictly more than this — we only know the
+            lower bound from what's still retained).
+          - ``>= earliest_available_id - 1`` → ``"replay_ok"``.  Replay
+            events are those with id strictly greater than
+            ``last_event_id`` (the client has already seen everything
+            up to and including ``last_event_id``).
+
+        Empty buffer: returned ``status="replay_ok"`` with empty
+        ``replay_events`` regardless of ``last_event_id``.  This is
+        the cold-start case (the ws just bootstrapped with no events
+        ever) and the all-quiet case (a long-idle ws past which all
+        events fall out of the buffer cap, but in practice the buffer
+        starts evicting only after the cap is hit — which means the
+        counter is at the cap and the client's last_event_id is below
+        earliest, so they get ``truncated`` instead).  We can't
+        distinguish the two without a separate ``highest_evicted_id``
+        tracker; treating empty as ``replay_ok`` is the safe choice
+        for the genuine cold-start case (no false ``replay_truncated``
+        envelopes on freshly-opened workstreams).
+        """
+        client_queue: queue.Queue[dict[str, Any]] = queue.Queue(maxsize=maxsize)
+        # Lock order matches writer: ``_ws_lock`` outer, ``_listeners_lock``
+        # inner.  Both inflight buffers AND the buffer slice AND the
+        # ``_event_id`` counter AND the listener registration captured
+        # as one atomic against any concurrent ``_enqueue``.  The string
+        # joins for content/reasoning happen OUTSIDE the locks (bounded
+        # at ``_MAX_TURN_CONTENT_CHARS`` but O(n) over fragments — not
+        # worth blocking on-token writers for the duration).  See
+        # the per-fresh-path helper for the same rationale.
+        with self._ws_lock:
+            captured_content = list(self._ws_inflight_content)
+            captured_reasoning = list(self._ws_inflight_reasoning)
+            with self._listeners_lock:
+                buffered = list(self._event_buffer)
+                self._listeners.append(client_queue)
+                snap_seq = self._event_id
+        snapshot: dict[str, Any] = {
+            "content": "".join(captured_content),
+            "reasoning": "".join(captured_reasoning),
+            "seq": snap_seq,
+        }
+        if not buffered:
+            return client_queue, [], "replay_ok", 0, 0, snapshot
+        earliest_id = buffered[0][0]
+        if last_event_id < earliest_id - 1:
+            lost_count = (earliest_id - 1) - last_event_id
+            return client_queue, [], "truncated", lost_count, earliest_id, snapshot
+        replay_events = [ev for eid, ev in buffered if eid > last_event_id]
+        return client_queue, replay_events, "replay_ok", 0, earliest_id, snapshot
+
     # ------------------------------------------------------------------
     # Approval / plan blocking gates
     # ------------------------------------------------------------------
@@ -1429,16 +1655,19 @@ class SessionUIBase:
     def _reset_inflight_buffers_locked(self) -> None:
         """Clear the per-turn inflight content + reasoning. Caller holds ``_ws_lock``.
 
-        ``_ws_inflight_seq`` is INTENTIONALLY not reset — it must
-        remain monotonically increasing for the lifetime of the UI so
-        a long-lived SSE subscriber's ``snap_seq`` cutoff stays a
-        valid high-water mark across turn boundaries. If we reset
-        seq=0 at every turn, turn N+1's first M tokens (M = the
-        snap_seq the subscriber captured mid-turn-N) would all carry
-        ``_seq <= snap_seq`` and get silently dropped by the dedup
-        filter in :func:`make_events_handler`. Seq is just a wire-
-        format dedup tag — its absolute value doesn't matter, only
-        that it's monotonic.
+        ``_event_id`` is INTENTIONALLY not reset — it must remain
+        monotonically increasing for the lifetime of the UI so a
+        long-lived SSE subscriber's ``snap_seq`` cutoff stays a valid
+        high-water mark across turn boundaries AND a ``Last-Event-ID``
+        replay can still slice the buffer correctly across resets.
+        If we reset to 0 at every turn, turn N+1's first M tokens
+        (M = the snap_seq the subscriber captured mid-turn-N) would
+        all carry ``_seq <= snap_seq`` and get silently dropped by
+        the dedup filter in :func:`make_events_handler`; and a
+        ``Last-Event-ID`` from before the reset would point into the
+        OLD numbering and silently mis-replay.  ``_event_id`` is just
+        an opaque monotonic tag — its absolute value doesn't matter,
+        only that it never decreases for the lifetime of the UI.
         """
         self._ws_inflight_content = []
         self._ws_inflight_content_size = 0
@@ -1490,16 +1719,17 @@ class SessionUIBase:
     def on_reasoning_token(self, text: str) -> None:
         """Append to the inflight reasoning buffer (capped) + enqueue.
 
-        Mirrors :meth:`on_content_token`'s shape. ``_ws_inflight_seq``
-        advances on EVERY emit — even when the buffer cap rejected
-        the append — so the dedup filter in :func:`make_events_handler`
-        stays correct for subscribers that register after the cap is
-        hit. If seq stalled at the high-water-pre-cap, those late
-        subscribers would capture ``snap_seq == high-water`` and
-        every subsequent live token (with the same stalled seq)
-        would be filter-dropped as "already in your snapshot",
-        silently losing the rest of the stream. The cap is a
-        buffer-size limit, NOT a "stop streaming" signal.
+        Mirrors :meth:`on_content_token`'s shape.  The ``_seq`` dedup
+        tag is stamped by :meth:`_enqueue` against the per-ws
+        ``_event_id`` counter, which advances on EVERY emit
+        regardless of whether the inflight cap rejected the append.
+        If the seq stalled at high-water-pre-cap, subscribers
+        registering after the cap is hit would capture
+        ``snap_seq == high-water`` and every subsequent live token
+        (with the same stalled seq) would be filter-dropped as
+        "already in your snapshot" — silently losing the rest of
+        the stream. The cap is a buffer-size limit, NOT a "stop
+        streaming" signal.
 
         Tokens past the cap are absent from ``snap.reasoning`` (the
         snapshot text was truncated at cap) but the live stream
@@ -1507,14 +1737,25 @@ class SessionUIBase:
         snapshot text up to the cap and then live tokens past it,
         with a visual gap equal to the past-cap chunk. No silent
         drop of subsequent tokens.
+
+        **Lock coupling**: ``_enqueue`` is called WHILE still
+        holding ``_ws_lock`` so the inflight append AND the
+        ``_event_id`` advancement happen atomically against a
+        snapshot reader.  Without this coupling a reader could
+        capture the inflight (with the new text) and read
+        ``_event_id`` BEFORE the writer's ``_enqueue`` bumped it,
+        producing a ``snap_seq`` lower than the new event's
+        ``_event_id``.  The new event would then slip past the
+        ``_seq <= snap_seq`` live-drain dedup and double-render
+        the text the snapshot already contained.  Acquisition
+        order ``_ws_lock`` (outer) → ``_listeners_lock`` (inner via
+        ``_enqueue``) matches the snapshot helpers, so no deadlock.
         """
         with self._ws_lock:
             if self._ws_inflight_reasoning_size < _MAX_TURN_CONTENT_CHARS:
                 self._ws_inflight_reasoning.append(text)
                 self._ws_inflight_reasoning_size += len(text)
-            self._ws_inflight_seq += 1
-            seq = self._ws_inflight_seq
-        self._enqueue({"type": "reasoning", "text": text, "_seq": seq})
+            self._enqueue({"type": "reasoning", "text": text})
 
     def on_content_token(self, text: str) -> None:
         """Append to both turn-content buffers (capped) + enqueue.
@@ -1526,23 +1767,27 @@ class SessionUIBase:
            :meth:`on_turn_start`) — fuels the SSE ``in_progress_snapshot``
            event a reconnecting client sees on mid-stream refresh.
 
-        Both caps are checked independently. ``_ws_inflight_seq``
-        advances on EVERY emit — even when the inflight cap rejected
-        the append — so a subscriber that registers after the cap is
-        hit doesn't have every subsequent live token filter-dropped
-        against a stalled ``snap_seq``. See
-        :meth:`on_reasoning_token` for the full rationale.
+        Both caps are checked independently.  The ``_seq`` dedup tag
+        is stamped by :meth:`_enqueue` against the per-ws
+        ``_event_id`` counter, which advances on EVERY emit
+        regardless of cap state — see :meth:`on_reasoning_token` for
+        the full rationale, including why ``_enqueue`` runs while
+        still holding ``_ws_lock`` (the lock coupling that makes
+        ``snap_seq`` a true high-water mark for the snapshot text).
 
-        The cap-check + append + size-update + seq-bump run under
+        The cap-check + append + size-update + enqueue all run under
         ``_ws_lock`` so a concurrent
         :meth:`snapshot_and_consume_state_payload` IDLE/ERROR drain or
         a concurrent :meth:`register_listener_with_in_progress_snapshot`
-        can't see a torn list mid-append. In production this is
-        single-writer-per-ws (the worker thread) but the snapshot
+        / :meth:`register_listener_with_replay` sees a consistent
+        ``(inflight_content, _event_id)`` pair.  In production this
+        is single-writer-per-ws (the worker thread) but the snapshot
         reader runs from coord's adapter via ``mgr.set_state``;
         without the lock the writer's append could land in an
-        orphaned list reference the snapshot just swapped out. Lock
-        hold is microseconds.
+        orphaned list reference the snapshot just swapped out, AND
+        the inflight/counter pair could de-sync.  Lock hold is
+        microseconds (the fan-out's ``put_nowait`` calls are O(N
+        listeners) but each is a single non-blocking enqueue).
         """
         with self._ws_lock:
             if self._ws_turn_content_size < _MAX_TURN_CONTENT_CHARS:
@@ -1551,9 +1796,7 @@ class SessionUIBase:
             if self._ws_inflight_content_size < _MAX_TURN_CONTENT_CHARS:
                 self._ws_inflight_content.append(text)
                 self._ws_inflight_content_size += len(text)
-            self._ws_inflight_seq += 1
-            seq = self._ws_inflight_seq
-        self._enqueue({"type": "content", "text": text, "_seq": seq})
+            self._enqueue({"type": "content", "text": text})
 
     def on_stream_end(self) -> None:
         with self._ws_lock:

turnstone/core/skill_field_validation.py

@@ -0,0 +1,177 @@
+"""Skill field validation — shared between the admin HTTP path and the
+model-facing ``skills`` tool exec path.
+
+Single source of truth for what shape each field on a skill row may
+take.  The HTTP path wraps the string error into a 400 JSONResponse;
+the model-tool path surfaces it via ``_coord_tool_error``.  Either
+caller can trust that validation cannot drift between layers because
+both go through this function.
+"""
+
+from __future__ import annotations
+
+import json
+from typing import Any
+
+_VALID_ACTIVATIONS: frozenset[str] = frozenset({"named", "default", "search"})
+
+# Fields that may be updated on installed (``readonly=true``) skills.
+# These are local runtime configuration — not part of the SKILL.md spec —
+# so they don't compromise the fidelity of an externally-sourced skill.
+# Shared between the admin HTTP path (``console/server.py``) and the
+# model-tool path (``ChatSession._exec_skills_update``); both consume
+# this single source of truth to avoid drift on what counts as a
+# runtime field.
+SKILL_RUNTIME_CONFIG_FIELDS: frozenset[str] = frozenset(
+    {
+        "model",
+        "temperature",
+        "reasoning_effort",
+        "max_tokens",
+        "token_budget",
+        "agent_max_turns",
+        "auto_approve",
+        "allowed_tools",
+        "enabled",
+        "notify_on_complete",
+        "priority",
+    }
+)
+
+
+def parse_skill_session_config(body: dict[str, Any]) -> tuple[dict[str, Any], str | None]:
+    """Validate session-config fields on a skill create/update body.
+
+    Returns ``(fields, error)``.  ``fields`` contains only the keys
+    present in ``body`` (partial-update friendly), with values
+    normalized to storage shape.  ``error`` is ``None`` on success or
+    a human-readable message on failure — never a JSONResponse, never
+    a raise.  Callers wrap into their own transport-shaped error.
+
+    Field rules:
+
+    - ``temperature``: float in [0.0, 2.0] or None / "" → None.
+      Non-numeric input (string that doesn't parse, dict, list) errors
+      out — matches ``max_tokens`` / ``token_budget`` for numeric-field
+      consistency.
+    - ``max_tokens``: int >= 1 or None / "" → None
+    - ``token_budget``: int >= 0 (defaults to 0 if missing-but-empty)
+    - ``agent_max_turns``: int >= 1 or None / "" → None
+    - ``reasoning_effort``: string (stripped)
+    - ``auto_approve`` / ``enabled``: bool
+    - ``activation``: one of ``_VALID_ACTIVATIONS``
+    - ``notify_on_complete``: JSON array string ("[]" if blank or
+      legacy ``{}`` sentinel from migrations 011/021)
+    - ``allowed_tools``: JSON array string (accepts list, JSON string,
+      or comma-separated CSV string → canonicalized to JSON array)
+    - ``model``: string (stripped)
+    """
+    fields: dict[str, Any] = {}
+
+    if "model" in body:
+        fields["model"] = str(body["model"] or "").strip()
+
+    if "temperature" in body:
+        temp = body["temperature"]
+        if temp is None or temp == "":
+            fields["temperature"] = None
+        else:
+            try:
+                temp = float(temp)
+            except (ValueError, TypeError):
+                return {}, "temperature must be a number between 0 and 2"
+            if not (0.0 <= temp <= 2.0):
+                return {}, "temperature must be between 0 and 2"
+            fields["temperature"] = temp
+
+    if "token_budget" in body:
+        try:
+            tb = int(body.get("token_budget", 0) or 0)
+        except (ValueError, TypeError):
+            return {}, "token_budget must be an integer"
+        if tb < 0:
+            return {}, "token_budget must be non-negative"
+        fields["token_budget"] = tb
+
+    if "max_tokens" in body:
+        mt = body["max_tokens"]
+        if mt is not None and mt != "":
+            try:
+                mt = int(mt)
+            except (ValueError, TypeError):
+                return {}, "max_tokens must be an integer"
+            if mt < 1:
+                return {}, "max_tokens must be positive"
+            fields["max_tokens"] = mt
+        else:
+            fields["max_tokens"] = None
+
+    if "agent_max_turns" in body:
+        amt = body["agent_max_turns"]
+        if amt is not None and amt != "":
+            try:
+                amt = int(amt)
+            except (ValueError, TypeError):
+                return {}, "agent_max_turns must be an integer"
+            if amt < 1:
+                return {}, "agent_max_turns must be positive"
+            fields["agent_max_turns"] = amt
+        else:
+            fields["agent_max_turns"] = None
+
+    if "reasoning_effort" in body:
+        fields["reasoning_effort"] = str(body["reasoning_effort"] or "").strip()
+
+    if "auto_approve" in body:
+        fields["auto_approve"] = bool(body.get("auto_approve", False))
+
+    if "enabled" in body:
+        fields["enabled"] = bool(body.get("enabled", True))
+
+    if "activation" in body:
+        activation = str(body["activation"] or "named").strip()
+        if activation not in _VALID_ACTIVATIONS:
+            return {}, (f"activation must be one of: {', '.join(sorted(_VALID_ACTIVATIONS))}")
+        fields["activation"] = activation
+
+    if "notify_on_complete" in body:
+        nc_raw = body.get("notify_on_complete", "[]")
+        # Accept list input from the model-tool path (the JSON-schema
+        # declares this field as ``type: array``).  The HTTP path may
+        # still send a JSON-encoded string body, so a str-input
+        # fallback stays for the other branch.  ``str(list)`` produces
+        # Python repr with single quotes and breaks ``json.loads`` —
+        # don't go through that path on a list input.
+        nc = json.dumps(nc_raw) if isinstance(nc_raw, list) else str(nc_raw).strip()
+        # Normalise empty/whitespace and the legacy ``"{}"`` sentinel
+        # (inherited from migrations 011/021's server_default — older
+        # rows that haven't been touched by migration 051 may still
+        # carry it) to the canonical empty-array literal so a blank
+        # field can never bypass validation and persist a non-JSON
+        # value.
+        if not nc or nc == "{}":
+            nc = "[]"
+        if nc != "[]":
+            try:
+                parsed = json.loads(nc)
+            except (json.JSONDecodeError, TypeError):
+                return {}, "notify_on_complete must be valid JSON"
+            if not isinstance(parsed, list):
+                return {}, "notify_on_complete must be a JSON array"
+        fields["notify_on_complete"] = nc
+
+    if "allowed_tools" in body:
+        at_raw = body.get("allowed_tools", "[]")
+        if isinstance(at_raw, list):
+            fields["allowed_tools"] = json.dumps(at_raw)
+        else:
+            at_str = str(at_raw).strip()
+            if at_str and not at_str.startswith("["):
+                at_str = json.dumps([t.strip() for t in at_str.split(",") if t.strip()])
+            try:
+                json.loads(at_str or "[]")
+            except (ValueError, TypeError):
+                at_str = "[]"
+            fields["allowed_tools"] = at_str or "[]"
+
+    return fields, None

turnstone/core/storage/_protocol.py

@@ -1376,11 +1376,14 @@ class StorageBackend(Protocol):
         convention ever needs to expand.
 
         ``kinds`` (when non-empty) narrows the result to rows whose
-        ``kind`` column is in the supplied list.  Coordinator-side
-        callers typically pass ``["coordinator", "any"]`` and
-        interactive-side callers pass ``["interactive", "any"]`` so
-        skills tagged ``any`` remain visible to both.  ``None`` means
-        no kind filter — all rows regardless of kind.
+        ``kind`` column is in the supplied list.  After the SkillKind
+        enforcement flatten (#557), ``kind`` is passive audience metadata
+        rather than a runtime visibility gate — the model-tool ``find``
+        path no longer threads ``kinds=`` by default and supplies it only
+        when the caller opts in via the tool's ``kind`` argument.  The
+        parameter remains available for admin filtering and explicit
+        scope narrowing.  ``None`` means no kind filter — all rows
+        regardless of kind.
         """
         ...
 

turnstone/prompts/__init__.py

@@ -126,7 +126,7 @@ def compose_system_message(
         IC-focused ``tools.md`` with read_file / bash / write_file
         patterns; ``"coordinator"`` loads ``tools_coordinator.md``
         which documents spawn_workstream / send_to_workstream /
-        inspect_workstream / list_nodes / list_skills / tasks etc.
+        inspect_workstream / list_nodes / skills / tasks etc.
         A coordinator session has a disjoint tool schema (see
         COORDINATOR_TOOLS), so composing it with the IC tools block
         would instruct the model to hallucinate tool calls that fail.

turnstone/prompts/tools_coordinator.md

@@ -1,8 +1,9 @@
 TOOL PATTERNS:
 
-Discover available capacity → list_nodes / list_skills:
+Discover available capacity → list_nodes / skills(action='find'):
    list_nodes(filters={'capability': 'gpu'})
-   list_skills(category='engineering')
+   skills(action='find', category='engineering')
+   skills(action='find', query='code review')
 
 Delegate a task → spawn_workstream:
    spawn_workstream(initial_message='audit auth.py for CSRF handling', name='csrf-audit')

turnstone/server.py

@@ -13,12 +13,14 @@ from __future__ import annotations
 
 import argparse
 import asyncio
+import collections
 import contextlib
 import functools
 import hashlib
 import json
 import os
 import queue
+import random
 import re
 import sys
 import textwrap
@@ -1243,24 +1245,101 @@ async def global_events_sse(request: Request) -> Response:
             status_code=409,
         )
 
-    # -- Atomic snapshot + listener registration ------------------------------
+    # -- Last-Event-ID resume parsing -----------------------------------------
+    # Native EventSource sets the header on auto-reconnect; the
+    # manual-reconnect path (which can't set custom headers on
+    # ``new EventSource(url)``) uses the query-param fallback.
+    last_event_id_raw = request.headers.get("Last-Event-ID") or request.query_params.get(
+        "last_event_id"
+    )
+    last_event_id: int | None
+    try:
+        last_event_id = int(last_event_id_raw) if last_event_id_raw else None
+    except (TypeError, ValueError):
+        last_event_id = None
+
+    # -- Atomic snapshot / replay-slice + listener registration ---------------
     client_queue: queue.Queue[dict[str, Any]] = queue.Queue(maxsize=1000)
     listeners = request.app.state.global_listeners
     listeners_lock = request.app.state.global_listeners_lock
+    event_buffer: collections.deque[tuple[int, dict[str, Any]]] = (
+        request.app.state.global_event_buffer
+    )
+
+    # Three replay shapes, matching :func:`make_events_handler`:
+    #   - ``last_event_id is None`` → ``"fresh"``: emit node_snapshot
+    #     then live.
+    #   - ``last_event_id`` + buffer covers gap → ``"replay_ok"``:
+    #     emit buffered events past the id, SKIP node_snapshot, then
+    #     live.
+    #   - ``last_event_id`` + buffer too short → ``"truncated"``: emit
+    #     a ``replay_truncated`` envelope then fall through to
+    #     ``"fresh"`` (node_snapshot is the recovery floor).
+    replay_status: str
+    replay_events: list[dict[str, Any]] = []
+    lost_count = 0
+    earliest_available_id = 0
+    snapshot: dict[str, Any] | None = None
 
-    # Hold the listeners lock while building the snapshot AND registering.
-    # The fanout thread also acquires this lock when snapshotting the listener
-    # list, so events that land on global_queue during snapshot build will be
-    # distributed to our queue after we release — gap-free.
     with listeners_lock:
-        snapshot = _build_node_snapshot(request.app.state)
+        if last_event_id is None:
+            replay_status = "fresh"
+            snapshot = _build_node_snapshot(request.app.state)
+        else:
+            buffered = list(event_buffer)
+            if not buffered:
+                replay_status = "replay_ok"
+            else:
+                earliest_available_id = buffered[0][0]
+                if last_event_id < earliest_available_id - 1:
+                    replay_status = "truncated"
+                    lost_count = (earliest_available_id - 1) - last_event_id
+                    snapshot = _build_node_snapshot(request.app.state)
+                else:
+                    replay_status = "replay_ok"
+                    replay_events = [ev for eid, ev in buffered if eid > last_event_id]
         listeners.append(client_queue)
 
-    async def event_generator() -> AsyncGenerator[dict[str, str], None]:
+    async def event_generator() -> AsyncGenerator[dict[str, Any], None]:
         _metrics.record_sse_connect()
+
+        def _format_event(event: dict[str, Any]) -> dict[str, str]:
+            """Strip ``_event_id`` from the wire dict, attach SSE ``id:``."""
+            ev_copy = dict(event)
+            eid = ev_copy.pop("_event_id", None)
+            out: dict[str, str] = {"data": json.dumps(ev_copy)}
+            if eid is not None:
+                out["id"] = str(eid)
+            return out
+
         try:
-            # Emit snapshot as first event
-            yield {"data": json.dumps(snapshot)}
+            # Per-stream reconnect jitter (see per-ws handler for
+            # rationale) — staggers reconnect of many panes / many
+            # global subscribers after a shared blip.
+            yield {"retry": random.randint(2500, 4500)}
+
+            if replay_status == "truncated":
+                yield {
+                    "data": json.dumps(
+                        {
+                            "type": "replay_truncated",
+                            "lost_count": lost_count,
+                            "earliest_available_id": earliest_available_id,
+                        }
+                    )
+                }
+            if replay_status == "replay_ok":
+                for ev in replay_events:
+                    yield _format_event(ev)
+            else:
+                # Fresh or truncated: emit the node_snapshot as the
+                # recovery floor.  Snapshot is synthetic (built from
+                # current ws state) and carries no ``_event_id`` —
+                # the client's ``lastEventId`` stays at whatever the
+                # last buffered event was (or empty on fresh).
+                if snapshot is not None:
+                    yield {"data": json.dumps(snapshot)}
+
             loop = asyncio.get_running_loop()
             executor = request.app.state.sse_executor
             while True:
@@ -1268,7 +1347,7 @@ async def global_events_sse(request: Request) -> Response:
                     event = await loop.run_in_executor(
                         executor, functools.partial(client_queue.get, timeout=5)
                     )
-                    yield {"data": json.dumps(event)}
+                    yield _format_event(event)
                 except queue.Empty:
                     pass  # poll timeout, retry
         finally:
@@ -3568,16 +3647,33 @@ def _global_fanout_thread(
     source_queue: queue.Queue[dict[str, Any]],
     listeners: list[queue.Queue[dict[str, Any]]],
     lock: threading.Lock,
+    event_buffer: collections.deque[tuple[int, dict[str, Any]]],
+    counter_holder: list[int],
 ) -> None:
-    """Reads events from the source queue and copies them to all listener queues."""
+    """Read events from ``source_queue``, stamp + buffer + fan out.
+
+    Stamps every event with a monotonic ``_event_id`` (the holder list
+    is a single-element mutable int — Python idiom for a shared int
+    under a lock), appends ``(event_id, event)`` to the global ring
+    buffer, snapshots the listener list, and fans out — all under
+    ``lock`` so a concurrent reader registering itself as a listener
+    sees a consistent ``(counter, listeners, buffer)`` triple and no
+    event lands in ONLY the buffer or ONLY the listener queue across
+    the registration boundary.  Mirrors :meth:`SessionUIBase._enqueue`'s
+    contract for the global lane.
+    """
     while True:
         try:
             event = source_queue.get()
             with lock:
+                counter_holder[0] += 1
+                event_id = counter_holder[0]
+                stamped = {**event, "_event_id": event_id}
+                event_buffer.append((event_id, stamped))
                 snapshot = list(listeners)
             for lq in snapshot:
                 with contextlib.suppress(queue.Full):
-                    lq.put_nowait(event)  # drop if a listener is backed up
+                    lq.put_nowait(stamped)  # drop if a listener is backed up
         except Exception:
             log.debug("Global fan-out error", exc_info=True)
 
@@ -3600,6 +3696,8 @@ async def _lifespan(app: Starlette) -> AsyncGenerator[None, None]:
             app.state.global_queue,
             app.state.global_listeners,
             app.state.global_listeners_lock,
+            app.state.global_event_buffer,
+            app.state.global_event_id_holder,
         ),
         daemon=True,
     )
@@ -4146,6 +4244,20 @@ def create_app(
     app.state.global_queue = global_queue
     app.state.global_listeners = global_listeners
     app.state.global_listeners_lock = global_listeners_lock
+    # Per-node global SSE replay ring buffer + monotonic event counter.
+    # Mirrors :attr:`SessionUIBase._event_buffer` / ``._event_id`` for
+    # the global lane; ``_global_fanout_thread`` stamps every event
+    # with ``_event_id`` under ``global_listeners_lock`` and appends
+    # to this buffer.  Cap sized to cover ~20 seconds of typical
+    # cluster broadcast rate (state changes + activity ticks across
+    # ~100 ws = up to a few hundred events/sec); operators can raise
+    # via ``TURNSTONE_SSE_EVENT_BUFFER_MAX`` (shared with per-ws cap).
+    from turnstone.core.session_ui_base import _EVENT_BUFFER_MAX
+
+    app.state.global_event_buffer = collections.deque(maxlen=_EVENT_BUFFER_MAX)
+    # Single-element list as a mutable int holder so the fanout
+    # thread can ``counter_holder[0] += 1`` under the lock.
+    app.state.global_event_id_holder = [0]
     app.state.skip_permissions = skip_permissions
     app.state.jwt_secret = jwt_secret
     app.state.auth_storage = auth_storage

turnstone/shared_static/renderer.js

@@ -24,47 +24,64 @@ function inlineMarkdown(text) {
   );
   // Strikethrough
   text = text.replace(/~~(.+?)~~/g, "<del>$1</del>");
-  // Images (must come before links — render as click-to-load placeholder)
-  text = text.replace(/!\[([^\]]*)\]\(([^)]+)\)/g, function (m, alt, url) {
-    if (!/^\s*(https?:\/\/|data:image\/)/i.test(url)) return m;
-    var safeAlt = alt || "Image";
-    var domain = "";
-    try {
-      domain = escapeHtml(new URL(url).hostname);
-    } catch (e) {
-      domain = url.length > 40 ? url.slice(0, 40) + "…" : url;
-    }
-    return (
-      '<span class="img-placeholder" tabindex="0" role="button" ' +
-      'aria-label="Load image: ' +
-      safeAlt +
-      '" ' +
-      'data-src="' +
-      url +
-      '" data-alt="' +
-      safeAlt +
-      '">' +
-      '<span class="img-placeholder-icon">&#x1F5BC;</span> ' +
-      '<span class="img-placeholder-label">' +
-      safeAlt +
-      "</span>" +
-      '<span class="img-placeholder-domain">' +
-      domain +
-      "</span>" +
-      "</span>"
-    );
-  });
-  // Links (allow http, https, and same-origin relative URLs only)
-  text = text.replace(/\[([^\]]+)\]\(([^)]+)\)/g, function (m, label, url) {
-    if (!/^\s*(https?:\/\/|\/(?!\/))/i.test(url)) return m;
-    return (
-      '<a href="' +
-      url +
-      '" target="_blank" rel="noopener noreferrer">' +
-      label +
-      "</a>"
-    );
-  });
+  // Images (must come before links — render as click-to-load placeholder).
+  // Captured groups inherit inlineMarkdown's leading escapeHtml(text) —
+  // they are already entity-encoded by the time this regex runs. Don't
+  // re-escape them: double-encoding turns `&` into `&amp;amp;`, which
+  // breaks query-string URLs after browser parse + getAttribute. The
+  // `safe*` rename signals the pre-escape invariant; the attribute-
+  // context lint in tests/test_renderer_js.py enforces that future
+  // attribute-context concat sites maintain the convention or call
+  // escapeHtml explicitly.
+  text = text.replace(
+    /!\[([^\]]*)\]\(([^)]+)\)/g,
+    function (m, safeAlt, safeUrl) {
+      if (!/^\s*(https?:\/\/|data:image\/)/i.test(safeUrl)) return m;
+      if (!safeAlt) safeAlt = "Image";
+      var safeDomain;
+      try {
+        // hostname is freshly extracted, not from the regex capture,
+        // so it does NOT carry the upstream escape — escape locally.
+        safeDomain = escapeHtml(new URL(safeUrl).hostname);
+      } catch (e) {
+        safeDomain = safeUrl.length > 40 ? safeUrl.slice(0, 40) + "…" : safeUrl;
+      }
+      return (
+        '<span class="img-placeholder" tabindex="0" role="button" ' +
+        'aria-label="Load image: ' +
+        safeAlt +
+        '" ' +
+        'data-src="' +
+        safeUrl +
+        '" data-alt="' +
+        safeAlt +
+        '">' +
+        '<span class="img-placeholder-icon">&#x1F5BC;</span> ' +
+        '<span class="img-placeholder-label">' +
+        safeAlt +
+        "</span>" +
+        '<span class="img-placeholder-domain">' +
+        safeDomain +
+        "</span>" +
+        "</span>"
+      );
+    },
+  );
+  // Links — same upstream-escape invariant as images. `safeLabel` and
+  // `safeUrl` are entity-encoded via inlineMarkdown's leading pass.
+  text = text.replace(
+    /\[([^\]]+)\]\(([^)]+)\)/g,
+    function (m, safeLabel, safeUrl) {
+      if (!/^\s*(https?:\/\/|\/(?!\/))/i.test(safeUrl)) return m;
+      return (
+        '<a href="' +
+        safeUrl +
+        '" target="_blank" rel="noopener noreferrer">' +
+        safeLabel +
+        "</a>"
+      );
+    },
+  );
   // Footnote references [^id] — after links (link regex requires (url), so no conflict)
   text = text.replace(/\[\^([^\]]+)\]/g, function (m, fnId) {
     var safeFnId = escapeHtml(fnId);

turnstone/tools/list_skills.json

@@ -1,32 +0,0 @@
-{
-  "name": "list_skills",
-  "description": "List skills (worker profiles) available to coordinators. Use to discover skill names for `spawn_workstream`. Filters: `category` (e.g. 'engineering', 'ops'), `tag` (single tag), `risk_level` (`safe`/`low`/`medium`/`high`/`critical`; omit to include unscanned rows). Results are pre-filtered to coordinator-applicable skills (`kind='coordinator'` or `'any'`); interactive-only skills are hidden. Each row returns `name`, `category`, `tags`, `version`, `description`, model preference, `enabled`, `risk_level`, `activation`, and `kind`. `allowed_tools` is included ONLY when the skill declares an auto-approve allowlist — listing the tool names exempt from the operator approval gate (capped at 20 names with a `+N more` sentinel when truncated). The field is OMITTED when empty: a skill without `allowed_tools` still has access to every tool in its session's toolset; absence of the field means no tool is pre-approved for this skill, not that the skill has no tools.",
-  "parameters": {
-    "type": "object",
-    "properties": {
-      "category": {
-        "type": "string",
-        "description": "Filter by category (exact match)."
-      },
-      "tag": {
-        "type": "string",
-        "description": "Filter by tag (must appear in the skill's tags array)."
-      },
-      "risk_level": {
-        "type": "string",
-        "enum": ["safe", "low", "medium", "high", "critical"],
-        "description": "Filter by risk level: safe / low / medium / high / critical. Omit entirely to include unscanned skills (which carry an empty value)."
-      },
-      "enabled_only": {
-        "type": "boolean",
-        "description": "If true, return only enabled skills. Default false."
-      },
-      "limit": {
-        "type": "integer",
-        "description": "Max rows to return. Default 100, max 500."
-      }
-    }
-  },
-  "coordinator": true,
-  "auto_approve": true
-}

turnstone/tools/skill.json

@@ -1,24 +0,0 @@
-{
-  "name": "skill",
-  "description": "Load or search for skills. Actions: 'load' activates a skill by name (replaces current skill), 'search' finds available skills by query.",
-  "parameters": {
-    "type": "object",
-    "properties": {
-      "action": {
-        "type": "string",
-        "enum": ["load", "search"],
-        "description": "Action to perform."
-      },
-      "name": {
-        "type": "string",
-        "description": "Skill name to load (required for 'load' action)."
-      },
-      "query": {
-        "type": "string",
-        "description": "Search query (for 'search' action)."
-      }
-    },
-    "required": ["action"]
-  },
-  "primary_key": "name"
-}

turnstone/tools/skills.json

@@ -0,0 +1,116 @@
+{
+  "name": "skills",
+  "description": "Manage the skill catalog and load skills into the current session. Replaces the legacy `skill` + `list_skills` tools. Each action has its own approval policy: read actions (`find`, `get`) auto-approve; write actions (`create`, `update`, `enable`, `disable`) require approval AND the `model.skills.write` permission on the session user — default-ungranted, operators opt themselves in via the Roles tab.\n\nActions:\n- `find`: list skills filtered by `category`, `tag`, `risk_level`, `kind`, `enabled_only`, `limit`, with optional `query` for BM25 ranking over the filtered set. By default returns skills of every kind; pass `kind='interactive'` or `kind='coordinator'` to narrow the browse (`any`-tagged rows are included alongside the chosen narrowing). Returns a hint when filters yield 0 rows showing what an unfiltered query would have matched.\n- `get`: fetch a single skill by `name`, returning the full row including `content`. Use to inspect before `update`.\n- `load` (requires approval): activate a skill by `name` on the current session — replaces the current skill, applies its session config. Works on both interactive and coordinator sessions and across all kinds (`kind` is authored audience metadata, not an enforcement gate). For assigning a skill to a *child* workstream instead of the current session, use `spawn_workstream(skill=...)`.\n- `create` (requires approval + `model.skills.write`): create a new skill with `name`, `content`, `description`, plus optional `category`, `tags`, `kind`, and session-config fields. Storage authoritatively re-computes `risk_level` from the scanner — caller cannot lie.\n- `update` (requires approval + `model.skills.write`): patch an existing skill identified by `name`. Any subset of writeable fields. Approval card shows the projected risk-tier shift if `content` or `allowed_tools` changes.\n- `enable` / `disable` (requires approval + `model.skills.write`): flip the `enabled` flag on a skill identified by `name`. Disabled skills stay in storage but are hidden from `find` (unless `enabled_only=false`) and rejected by `load` / `spawn_workstream(skill=...)`.\n\nSoft-delete only — there is no `delete` action in this tool. Hard-delete remains admin-UI exclusive to avoid model-proposed mistakes against in-use skills.",
+  "parameters": {
+    "type": "object",
+    "properties": {
+      "action": {
+        "type": "string",
+        "enum": [
+          "find",
+          "get",
+          "load",
+          "create",
+          "update",
+          "enable",
+          "disable"
+        ],
+        "description": "Action to perform."
+      },
+      "name": {
+        "type": "string",
+        "description": "Skill name. Required for `get`, `load`, `create`, `update`, `enable`, `disable`."
+      },
+      "query": {
+        "type": "string",
+        "description": "For `find`: optional BM25 query over name + description + tags + category. Omit to return rows by filters alone."
+      },
+      "category": {
+        "type": "string",
+        "description": "For `find` (filter) and `create`/`update` (field): category string. Free-form, e.g. 'engineering', 'ops', 'data'."
+      },
+      "tag": {
+        "type": "string",
+        "description": "For `find`: filter by a single tag. The tag must appear in the skill's `tags` array."
+      },
+      "tags": {
+        "type": "array",
+        "items": { "type": "string" },
+        "description": "For `create`/`update`: list of tag strings. Stored as a JSON array."
+      },
+      "risk_level": {
+        "type": "string",
+        "enum": ["safe", "low", "medium", "high", "critical"],
+        "description": "For `find` (filter): filter by scanner-assigned tier. Note: risk_level is NEVER caller-controlled on writes — the storage layer recomputes it from the scanner on every create/update. The field shown here is read-only output of the scanner."
+      },
+      "enabled_only": {
+        "type": "boolean",
+        "description": "For `find`: when true, return only enabled skills. Default false (include disabled rows so callers can see what's available to enable)."
+      },
+      "limit": {
+        "type": "integer",
+        "description": "For `find`: max rows to return (1-500, default 100). Truncation is signaled in the response."
+      },
+      "content": {
+        "type": "string",
+        "description": "For `create` (required) / `update` (optional): the skill's prompt body. Max 32768 chars. Storage runs the scanner against this content to assign `risk_level`."
+      },
+      "description": {
+        "type": "string",
+        "description": "For `create` (required) / `update` (optional): one-line summary shown in discovery output. Max 1024 chars, must not be empty."
+      },
+      "kind": {
+        "type": "string",
+        "enum": ["interactive", "coordinator", "any"],
+        "description": "Authored audience metadata — passive marker for sorting/grouping, not an enforcement boundary. For `create`/`update`: which session surface this skill was authored for (`interactive`, `coordinator`, or `any`); defaults to `any` on create. For `find`: optional discoverability filter — pass `interactive` or `coordinator` to narrow the browse (rows tagged `any` are included alongside the chosen narrowing); omit to return all kinds."
+      },
+      "model": {
+        "type": "string",
+        "description": "For `create`/`update`: model alias to use when this skill is active (overrides session default)."
+      },
+      "temperature": {
+        "type": "number",
+        "description": "For `create`/`update`: temperature when this skill is active. Range 0-2 or null."
+      },
+      "reasoning_effort": {
+        "type": "string",
+        "description": "For `create`/`update`: reasoning_effort override when this skill is active."
+      },
+      "max_tokens": {
+        "type": "integer",
+        "description": "For `create`/`update`: max_tokens override when this skill is active. Positive integer or null."
+      },
+      "token_budget": {
+        "type": "integer",
+        "description": "For `create`/`update`: per-turn token budget when this skill is active. 0 = no override."
+      },
+      "agent_max_turns": {
+        "type": "integer",
+        "description": "For `create`/`update`: sub-agent turn cap when this skill is active. Positive integer or null."
+      },
+      "auto_approve": {
+        "type": "boolean",
+        "description": "For `create`/`update`: when this skill is loaded into a session, mark its session as auto-approving tools in `allowed_tools` without operator prompt. Operator-visible on the approval card; use sparingly."
+      },
+      "allowed_tools": {
+        "type": "array",
+        "items": { "type": "string" },
+        "description": "For `create`/`update`: tools auto-approved when this skill is active. Empty / absent = no tools pre-approved (operator approves each call). Editing this field forces a scanner re-run that may shift `risk_level` — diff is shown on the approval card."
+      },
+      "activation": {
+        "type": "string",
+        "enum": ["named", "default", "search"],
+        "description": "For `create`/`update`: how the skill becomes discoverable. `named` = explicit load only; `default` = applies to every new session; `search` = listed in the search-activated catalog."
+      },
+      "notify_on_complete": {
+        "type": "array",
+        "items": { "type": "object" },
+        "description": "For `create`/`update`: list of notify targets to fire when a workstream using this skill completes."
+      }
+    },
+    "required": ["action"]
+  },
+  "coordinator": true,
+  "interactive": true,
+  "primary_key": "action"
+}

turnstone/ui/static/app.js

@@ -674,9 +674,17 @@ class Pane {
       this.attachments.rehydrate();
     }
 
-    this.evtSource = new EventSource(
-      "/v1/api/workstreams/" + encodeURIComponent(wsId) + "/events",
-    );
+    // Build the events URL with a ``?last_event_id=N`` query param
+    // if we have a saved high-water mark from a prior connection.
+    // The EventSource constructor can't set custom headers, so the
+    // browser-native ``Last-Event-ID`` header isn't available here;
+    // the server accepts both forms.  ``_lastEventId`` is captured
+    // from the prior source's onmessage handler.
+    let evtUrl = "/v1/api/workstreams/" + encodeURIComponent(wsId) + "/events";
+    if (this._lastEventId) {
+      evtUrl += "?last_event_id=" + encodeURIComponent(this._lastEventId);
+    }
+    this.evtSource = new EventSource(evtUrl);
 
     this.evtSource.onopen = () => {
       this.retryDelay = 1000;
@@ -685,100 +693,145 @@ class Pane {
     };
 
     this.evtSource.onmessage = (e) => {
+      // Capture lastEventId BEFORE JSON.parse so a (rare) malformed
+      // event doesn't desync the manual-reconnect fallback from
+      // native auto-reconnect (which advances lastEventId regardless
+      // of whether we successfully process the data).  Server's
+      // stamping contract: ``id:`` only on events sourced from the
+      // per-ws ring buffer — synthetic replay events (history /
+      // state_change / in_progress_snapshot) don't advance the
+      // counter, so reconnect resumes from the last BUFFERED id (or
+      // none on a truly-fresh connect that never received one).
+      if (this.evtSource && this.evtSource.lastEventId) {
+        this._lastEventId = this.evtSource.lastEventId;
+      }
       const data = JSON.parse(e.data);
       this.handleEvent(data);
     };
 
     this.evtSource.onerror = () => {
-      this.evtSource.close();
-      this.evtSource = null;
+      // Do NOT close evtSource for transient network errors — native
+      // EventSource auto-reconnect handles them with the
+      // ``Last-Event-ID`` header automatically (now that the server
+      // emits ``id:`` on every buffered event).  Closing here would
+      // force a CONNECTING -> CLOSED transition that defeats native
+      // reconnect, which is exactly the reconnect-with-replay defect
+      // PR-D ships to fix.  See
+      // tests/test_app_js.py::test_pane_connectsse_onerror_preserves_native_reconnect.
       const loginOverlay = document.getElementById("login-overlay");
       if (loginOverlay && loginOverlay.style.display !== "none") return;
       this.statusBarEl.classList.add("ws-sb-disconnected");
       this._sbTokens.textContent = "Reconnecting\u2026";
-      // Only the focused pane refreshes the global workstream list to avoid
-      // race conditions when multiple panes disconnect simultaneously.
+      // Focused-pane orthogonal trigger: refetch the global
+      // workstream list so a workstream evicted while we were
+      // disconnected gets reassigned across panes.  The reassignment
+      // branch's explicit disconnectSSE + connectSSE on the new
+      // wsId is correct (a different workstream genuinely needs a
+      // fresh stream, not a same-stream replay).
       if (this.id === focusedPaneId) {
-        fetch("/v1/api/workstreams")
-          .then((r) => {
-            if (r.status === 401) {
-              showLogin();
-              return;
-            }
-            return r.json().then((data) => {
-              workstreams = {};
-              (data.workstreams || []).forEach((ws) => {
-                workstreams[ws.ws_id] = { name: ws.name, state: ws.state };
-              });
-              renderTabBar();
-              // Reconnect all disconnected panes, reassigning stale ws_ids.
-              // Two passes: (1) reassign stale panes, (2) reconnect all.
-              // Track assigned ws_ids to avoid multiple panes on the same ws.
-              const remaining = Object.keys(workstreams);
-              if (!remaining.length) {
-                showDashboard();
-                return;
-              }
-              const usedWsIds = {};
-              for (let pid in panes) {
-                if (panes[pid].wsId && workstreams[panes[pid].wsId])
-                  usedWsIds[panes[pid].wsId] = true;
-              }
-              for (let pid2 in panes) {
-                const p2 = panes[pid2];
-                if (p2.wsId && !workstreams[p2.wsId]) {
-                  let newWsId = null;
-                  for (let ri = 0; ri < remaining.length; ri++) {
-                    if (!usedWsIds[remaining[ri]]) {
-                      newWsId = remaining[ri];
-                      break;
-                    }
-                  }
-                  if (newWsId) {
-                    p2.disconnectSSE();
-                    p2.wsId = newWsId;
-                    usedWsIds[newWsId] = true;
-                    while (p2.messagesEl.firstChild)
-                      p2.messagesEl.removeChild(p2.messagesEl.firstChild);
-                    p2.showEmptyState();
-                    p2.updateWsName();
-                  }
-                  // else: more panes than workstreams — leave pane stale,
-                  // connectSSE below will pick it up or it stays disconnected.
-                }
-              }
-              // Pass 2: reconnect all panes and sync focused pane
-              for (let pid3 in panes) {
-                const p3 = panes[pid3];
-                if (pid3 === focusedPaneId) currentWsId = p3.wsId;
-                if (!p3.evtSource && p3.wsId && workstreams[p3.wsId]) {
-                  setTimeout(
-                    ((pp) => {
-                      return () => {
-                        pp.connectSSE(pp.wsId);
-                      };
-                    })(p3),
-                    this.retryDelay,
-                  );
-                }
-              }
-              this.retryDelay = Math.min(this.retryDelay * 2, 30000);
-            });
-          })
-          .catch(() => {
-            setTimeout(() => {
-              this.connectSSE(this.wsId);
-            }, this.retryDelay);
-            this.retryDelay = Math.min(this.retryDelay * 2, 30000);
+        this._refetchWorkstreamsAndReassign();
+      }
+      // Non-focused panes: native EventSource reconnect handles them
+      // transparently — no per-pane retry needed.  The 30 s exp-backoff
+      // ceiling on retryDelay is preserved inside
+      // _refetchWorkstreamsAndReassign for the focused-pane path.
+    };
+  }
+
+  _refetchWorkstreamsAndReassign() {
+    // Lifted from the pre-PR-D ``onerror`` body.  Triggered when the
+    // focused pane sees its EventSource enter the error state — pulls
+    // the authoritative workstream list and reassigns stale wsIds.
+    // Survives the onerror refactor as a separate concern from the
+    // SSE reconnect mechanics: native EventSource handles the same-
+    // workstream reconnect; this handles the workstream-evicted-
+    // during-disconnect recovery.
+    fetch("/v1/api/workstreams")
+      .then((r) => {
+        if (r.status === 401) {
+          showLogin();
+          return;
+        }
+        return r.json().then((data) => {
+          workstreams = {};
+          (data.workstreams || []).forEach((ws) => {
+            workstreams[ws.ws_id] = { name: ws.name, state: ws.state };
           });
-      } else {
-        // Non-focused pane: just retry own connection after delay
+          renderTabBar();
+          // Two passes: (1) reassign stale panes, (2) reconnect any
+          // that ended up in CLOSED state.  Native reconnect covers
+          // CONNECTING -> OPEN transitions transparently.
+          const remaining = Object.keys(workstreams);
+          if (!remaining.length) {
+            showDashboard();
+            return;
+          }
+          const usedWsIds = {};
+          for (let pid in panes) {
+            if (panes[pid].wsId && workstreams[panes[pid].wsId])
+              usedWsIds[panes[pid].wsId] = true;
+          }
+          for (let pid2 in panes) {
+            const p2 = panes[pid2];
+            if (p2.wsId && !workstreams[p2.wsId]) {
+              let newWsId = null;
+              for (let ri = 0; ri < remaining.length; ri++) {
+                if (!usedWsIds[remaining[ri]]) {
+                  newWsId = remaining[ri];
+                  break;
+                }
+              }
+              if (newWsId) {
+                p2.disconnectSSE();
+                // Different workstream → drop saved id; replay is
+                // per-ws so an id from ws-A is meaningless on ws-B.
+                p2._lastEventId = null;
+                p2.wsId = newWsId;
+                usedWsIds[newWsId] = true;
+                while (p2.messagesEl.firstChild)
+                  p2.messagesEl.removeChild(p2.messagesEl.firstChild);
+                p2.showEmptyState();
+                p2.updateWsName();
+              }
+              // else: more panes than workstreams — leave pane stale,
+              // connectSSE below picks it up or stays disconnected.
+            }
+          }
+          // Pass 2: reconnect any pane whose EventSource ended up
+          // truly CLOSED (not just transient — native reconnect
+          // handles CONNECTING / OPEN).
+          for (let pid3 in panes) {
+            const p3 = panes[pid3];
+            if (pid3 === focusedPaneId) currentWsId = p3.wsId;
+            const dead =
+              !p3.evtSource || p3.evtSource.readyState === EventSource.CLOSED;
+            if (dead && p3.wsId && workstreams[p3.wsId]) {
+              setTimeout(
+                ((pp) => {
+                  return () => {
+                    pp.connectSSE(pp.wsId);
+                  };
+                })(p3),
+                this.retryDelay,
+              );
+            }
+          }
+          this.retryDelay = Math.min(this.retryDelay * 2, 30000);
+        });
+      })
+      .catch(() => {
+        // Fetch failed (network) — schedule a same-pane reconnect
+        // fallback in case the EventSource is genuinely dead.
         setTimeout(() => {
-          this.connectSSE(this.wsId);
+          if (
+            !this.evtSource ||
+            this.evtSource.readyState === EventSource.CLOSED
+          ) {
+            this.connectSSE(this.wsId);
+          }
         }, this.retryDelay);
         this.retryDelay = Math.min(this.retryDelay * 2, 30000);
-      }
-    };
+      });
   }
 
   handleEvent(evt) {
@@ -2905,6 +2958,13 @@ let workstreams = {};
 let currentWsId = null;
 let globalEvtSource = null;
 let globalRetryDelay = 1000;
+// Saved high-water mark for the manual-reconnect path (the
+// EventSource constructor can't set custom headers, so the
+// browser-native ``Last-Event-ID`` header is unavailable on
+// reconnect — we thread it via ``?last_event_id=N`` instead).  Updated
+// from ``globalEvtSource.lastEventId`` on every onmessage; native
+// auto-reconnect uses the header directly on the same source object.
+let globalLastEventId = null;
 let dashboardVisible = false;
 let _historyNavigation = false;
 let _lastHealth = null;
@@ -4663,11 +4723,23 @@ function connectGlobalSSE() {
     globalEvtSource.close();
     globalEvtSource = null;
   }
-  globalEvtSource = new EventSource("/v1/api/events/global");
+  // Manual-reconnect path threads ``?last_event_id=N`` because the
+  // EventSource constructor can't set headers; native auto-reconnect
+  // on the same source uses the header directly.
+  let globalUrl = "/v1/api/events/global";
+  if (globalLastEventId) {
+    globalUrl += "?last_event_id=" + encodeURIComponent(globalLastEventId);
+  }
+  globalEvtSource = new EventSource(globalUrl);
   globalEvtSource.onopen = function () {
     globalRetryDelay = 1000;
   };
   globalEvtSource.onmessage = function (e) {
+    // Capture lastEventId BEFORE JSON.parse (see Pane.connectSSE
+    // onmessage for full rationale).
+    if (globalEvtSource && globalEvtSource.lastEventId) {
+      globalLastEventId = globalEvtSource.lastEventId;
+    }
     const data = JSON.parse(e.data);
     if (data.type === "ws_state") {
       updateTabIndicator(data.ws_id, data.state, {
@@ -4735,21 +4807,26 @@ function connectGlobalSSE() {
     }
   };
   globalEvtSource.onerror = function () {
-    globalEvtSource.close();
-    globalEvtSource = null;
-    fetch("/v1/api/workstreams")
-      .then(function (r) {
-        if (r.status === 401) {
-          showLogin();
-          return;
+    // Do NOT close globalEvtSource for transient errors — native
+    // EventSource auto-reconnect handles them with the
+    // ``Last-Event-ID`` header automatically (now that the global
+    // SSE handler emits ``id:`` on every buffered event).  Closing
+    // here would defeat native reconnect.  See PR-D briefing § 3.3
+    // and the per-pane handler above for the same pattern.
+    //
+    // The 401 probe stays — an authentication failure is a terminal
+    // condition (the user must log in) and merits an explicit
+    // close + showLogin.  ``_reconnectDeadSSEs`` (visibilitychange /
+    // focus listener) covers the truly-CLOSED case.
+    fetch("/v1/api/workstreams").then(function (r) {
+      if (r.status === 401) {
+        if (globalEvtSource) {
+          globalEvtSource.close();
+          globalEvtSource = null;
         }
-        setTimeout(connectGlobalSSE, globalRetryDelay);
-        globalRetryDelay = Math.min(globalRetryDelay * 2, 30000);
-      })
-      .catch(function () {
-        setTimeout(connectGlobalSSE, globalRetryDelay);
-        globalRetryDelay = Math.min(globalRetryDelay * 2, 30000);
-      });
+        showLogin();
+      }
+    });
   };
 }
 

uv.lock

@@ -2622,15 +2622,15 @@ wheels = [
 
 [[package]]
 name = "starlette"
-version = "1.0.0"
+version = "1.0.1"
 source = { registry = "https://pypi.org/simple" }
 dependencies = [
     { name = "anyio" },
     { name = "typing-extensions", marker = "python_full_version < '3.13'" },
 ]
-sdist = { url = "https://files.pythonhosted.org/packages/81/69/17425771797c36cded50b7fe44e850315d039f28b15901ab44839e70b593/starlette-1.0.0.tar.gz", hash = "sha256:6a4beaf1f81bb472fd19ea9b918b50dc3a77a6f2e190a12954b25e6ed5eea149", size = 2655289, upload-time = "2026-03-22T18:29:46.779Z" }
+sdist = { url = "https://files.pythonhosted.org/packages/08/a3/84e821cc54b4ab50ae6dbc6ac3800a651b65ec35f045cc73785380654057/starlette-1.0.1.tar.gz", hash = "sha256:512399c5f1de7fac99c88572212ded9ddeddef2fb32afa82d724000e88b38f4f", size = 2659596, upload-time = "2026-05-21T21:58:58.433Z" }
 wheels = [
-    { url = "https://files.pythonhosted.org/packages/0b/c9/584bc9651441b4ba60cc4d557d8a547b5aff901af35bda3a4ee30c819b82/starlette-1.0.0-py3-none-any.whl", hash = "sha256:d3ec55e0bb321692d275455ddfd3df75fff145d009685eb40dc91fc66b03d38b", size = 72651, upload-time = "2026-03-22T18:29:45.111Z" },
+    { url = "https://files.pythonhosted.org/packages/ec/e1/b2df4bc09a1e51ff664c1e17018a4274b42e5e9352e4a478ea540512dc88/starlette-1.0.1-py3-none-any.whl", hash = "sha256:7c0e69b2ee1c848bd54669d908500117a3ee13de603a21427e5c6fc1adf98dcd", size = 72802, upload-time = "2026-05-21T21:58:56.551Z" },
 ]
 
 [[package]]
@@ -2722,7 +2722,7 @@ wheels = [
 
 [[package]]
 name = "turnstone"
-version = "1.6.0a2"
+version = "1.6.0a3"
 source = { editable = "." }
 dependencies = [
     { name = "alembic" },
@@ -2829,7 +2829,7 @@ requires-dist = [
     { name = "slack-bolt", marker = "extra == 'test'", specifier = ">=1.18" },
     { name = "sqlalchemy", specifier = ">=2.0" },
     { name = "sse-starlette", specifier = ">=2.0" },
-    { name = "starlette", specifier = ">=0.45" },
+    { name = "starlette", specifier = ">=1.0.1" },
     { name = "structlog", specifier = ">=24.1" },
     { name = "sympy", marker = "extra == 'sandbox'", specifier = ">=1.13" },
     { name = "turnstone", extras = ["console", "anthropic", "postgres", "discord", "ddg", "tls", "sandbox", "slack"], marker = "extra == 'all'" },