diff --git a/.gitignore b/.gitignore
index a7b050ca..73ec2e4b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -21,3 +21,5 @@ PROGRESS.md
.coverage
tools/skill_audit_analysis/data/
tools/skill_audit_analysis/output/
+design_ideas/
+.claude/
diff --git a/tests/test_coordinator_endpoints.py b/tests/test_coordinator_endpoints.py
index 1d104b24..2b802c7a 100644
--- a/tests/test_coordinator_endpoints.py
+++ b/tests/test_coordinator_endpoints.py
@@ -38,6 +38,7 @@ from turnstone.console.server import (
coordinator_history,
coordinator_list,
coordinator_open,
+ coordinator_saved,
coordinator_send,
coordinator_tasks,
)
@@ -70,6 +71,13 @@ def _make_client(
methods=["POST"],
),
Route("/v1/api/coordinator", coordinator_list, methods=["GET"]),
+ # Literal path before the /{ws_id} routes below so Starlette
+ # matches "saved" as the literal, not as a ws_id.
+ Route(
+ "/v1/api/coordinator/saved",
+ coordinator_saved,
+ methods=["GET"],
+ ),
Route(
"/v1/api/coordinator/{ws_id}/send",
coordinator_send,
@@ -279,6 +287,144 @@ def test_list_admin_sees_all(storage):
assert len(resp.json()["coordinators"]) == 2
+def _seed_closed_coord_with_history(
+ mgr,
+ storage,
+ *,
+ user_id: str,
+ name: str,
+) -> str:
+ """Create + close a coordinator and seed one conversation row.
+
+ list_workstreams_with_history's WHERE EXISTS guard skips coords with no
+ messages, so the saved-list endpoint won't surface a freshly-closed
+ coordinator unless we've stamped at least one conversation row.
+ """
+ ws = mgr.create(user_id=user_id, name=name)
+ storage.save_message(ws.id, role="user", content="seed")
+ closed = mgr.close(ws.id)
+ assert closed
+ return ws.id
+
+
+@pytest.fixture
+def saved_storage(tmp_path):
+ """Storage fixture for saved-coordinator tests.
+
+ coordinator_saved goes through ``list_workstreams_with_history``
+ which calls ``get_storage()`` (the singleton registry), not whatever
+ backend the manager holds. This fixture initialises the registry to
+ a fresh SQLite db and yields the same backend so the test can also
+ seed conversation rows directly.
+ """
+ from turnstone.core.storage import init_storage, reset_storage
+
+ db_path = str(tmp_path / "saved.db")
+ reset_storage()
+ backend = init_storage("sqlite", path=db_path, run_migrations=False)
+ try:
+ yield backend
+ finally:
+ reset_storage()
+
+
+def test_saved_filters_by_caller(saved_storage):
+ storage = saved_storage
+ mgr = _build_mgr(storage)
+ mine_id = _seed_closed_coord_with_history(mgr, storage, user_id="user-1", name="mine")
+ _seed_closed_coord_with_history(mgr, storage, user_id="user-2", name="theirs")
+ client = _make_client(storage, coord_mgr=mgr, registry=_fake_registry())
+ resp = client.get("/v1/api/coordinator/saved", headers=_COORD_HEADERS)
+ assert resp.status_code == 200
+ body = resp.json()
+ assert {c["ws_id"] for c in body["coordinators"]} == {mine_id}
+
+
+def test_saved_admin_sees_all(saved_storage):
+ storage = saved_storage
+ mgr = _build_mgr(storage)
+ a = _seed_closed_coord_with_history(mgr, storage, user_id="user-1", name="a")
+ b = _seed_closed_coord_with_history(mgr, storage, user_id="user-2", name="b")
+ client = _make_client(storage, coord_mgr=mgr, registry=_fake_registry())
+ resp = client.get(
+ "/v1/api/coordinator/saved",
+ headers={
+ "X-Test-User": "admin-1",
+ "X-Test-Perms": "admin.coordinator,admin.users",
+ },
+ )
+ assert resp.status_code == 200
+ assert {c["ws_id"] for c in resp.json()["coordinators"]} == {a, b}
+
+
+def test_saved_blank_uid_returns_empty(saved_storage):
+ """Non-admin caller with no sub gets fail-closed empty list.
+
+ Mirrors list_saved_workstreams — empty user_id must not fall through
+ to a cluster-wide query (would leak orphan / migration rows).
+ """
+ storage = saved_storage
+ mgr = _build_mgr(storage)
+ _seed_closed_coord_with_history(mgr, storage, user_id="someone", name="x")
+ client = _make_client(storage, coord_mgr=mgr, registry=_fake_registry())
+ resp = client.get(
+ "/v1/api/coordinator/saved",
+ headers={"X-Test-User": "", "X-Test-Perms": "admin.coordinator"},
+ )
+ assert resp.status_code == 200
+ assert resp.json() == {"coordinators": []}
+
+
+def test_saved_excludes_currently_loaded(saved_storage):
+ """A coordinator currently in coord_mgr must NOT appear in saved cards.
+
+ Even if its DB row says state='closed' (e.g. mid-restart race), the
+ in-memory presence wins so the same ws_id can't be in both the
+ active list and the saved-cards grid simultaneously.
+ """
+ storage = saved_storage
+ mgr = _build_mgr(storage)
+ closed_id = _seed_closed_coord_with_history(mgr, storage, user_id="user-1", name="closed")
+ # Create another coord, leave it loaded — should never appear in saved.
+ loaded_ws = mgr.create(user_id="user-1", name="loaded")
+ storage.save_message(loaded_ws.id, role="user", content="seed")
+ # Force it to state='closed' on disk without removing from memory, to
+ # exercise the defence-in-depth ``loaded`` filter.
+ storage.update_workstream_state(loaded_ws.id, "closed")
+ client = _make_client(storage, coord_mgr=mgr, registry=_fake_registry())
+ resp = client.get("/v1/api/coordinator/saved", headers=_COORD_HEADERS)
+ assert resp.status_code == 200
+ saved_ids = {c["ws_id"] for c in resp.json()["coordinators"]}
+ assert closed_id in saved_ids
+ assert loaded_ws.id not in saved_ids
+
+
+def test_saved_excludes_active_state_rows(saved_storage):
+ """Only state='closed' rows surface in the saved list.
+
+ A coordinator that's idle on disk but not currently loaded into
+ coord_mgr (e.g. orphaned across a console restart that hasn't
+ rehydrated yet) is NOT 'saved' — it's just not loaded yet, and the
+ saved grid is for explicit user-closed sessions.
+ """
+ storage = saved_storage
+ mgr = _build_mgr(storage)
+ closed_id = _seed_closed_coord_with_history(mgr, storage, user_id="user-1", name="closed")
+ # An idle row in storage with no in-memory presence — must not appear.
+ orphan = mgr.create(user_id="user-1", name="orphan")
+ storage.save_message(orphan.id, role="user", content="seed")
+ # Drop from memory without changing state (simulates manager restart).
+ mgr._workstreams.pop(orphan.id, None)
+ if orphan.id in mgr._order:
+ mgr._order.remove(orphan.id)
+ assert storage.get_workstream(orphan.id)["state"] == "idle"
+ client = _make_client(storage, coord_mgr=mgr, registry=_fake_registry())
+ resp = client.get("/v1/api/coordinator/saved", headers=_COORD_HEADERS)
+ assert resp.status_code == 200
+ saved_ids = {c["ws_id"] for c in resp.json()["coordinators"]}
+ assert saved_ids == {closed_id}
+
+
def test_send_to_someone_elses_coord_returns_404(storage):
mgr = _build_mgr(storage)
ws = mgr.create(user_id="owner", name="theirs")
diff --git a/tests/test_coordinator_manager.py b/tests/test_coordinator_manager.py
index 6755a1a0..549aeff3 100644
--- a/tests/test_coordinator_manager.py
+++ b/tests/test_coordinator_manager.py
@@ -388,18 +388,40 @@ def test_open_admin_ignores_ownership(built_mgr):
assert ws is not None
-def test_open_refuses_closed_coordinator(built_mgr):
- """A coordinator that was closed (state=closed in storage) must not
- silently resurrect on the next GET. Otherwise the Close button is
- reversible on URL revisit and burns max_active capacity."""
+def test_open_resurrects_closed_coordinator(built_mgr):
+ """A coordinator that was closed (state='closed' in storage) IS now
+ resurrectable via open(). Restore is an explicit user action via
+ the Saved Coordinators landing UI; ``_reserve_and_install_locked``
+ still enforces ``max_active`` (evicts an idle peer or 429s). The
+ old "URL revisit silently undoes Close" safety lives in the slot
+ accounting now, not in a flat refusal at the open path."""
mgr, _calls, storage = built_mgr
ws = mgr.create(user_id="u1")
mgr.close(ws.id)
- # Direct GET via open() must NOT rehydrate the closed row.
+ assert storage.get_workstream(ws.id)["state"] == "closed"
+
reopened = mgr.open(ws.id, "u1")
- assert reopened is None
- # Admin path must also refuse to resurrect — closed means closed.
- assert mgr.open_admin(ws.id) is None
+ assert reopened is not None
+ assert reopened.id == ws.id
+ # Re-loaded into memory.
+ assert mgr.get(ws.id) is reopened
+
+ # Admin path also resurrects.
+ mgr.close(ws.id)
+ assert mgr.open_admin(ws.id) is not None
+
+
+def test_open_refuses_deleted_coordinator(built_mgr):
+ """A coordinator marked state='deleted' is a tombstone — open() must
+ refuse to resurrect even though closed-state is now resurrectable."""
+ mgr, _calls, storage = built_mgr
+ ws = mgr.create(user_id="u1")
+ mgr.close(ws.id)
+ storage.update_workstream_state(ws.id, "deleted")
+ user_open = mgr.open(ws.id, "u1")
+ assert user_open is None
+ admin_open = mgr.open_admin(ws.id)
+ assert admin_open is None
def test_open_refuses_empty_owner_for_non_admin(built_mgr):
diff --git a/turnstone/console/coordinator.py b/turnstone/console/coordinator.py
index 245643d5..5aff8f2b 100644
--- a/turnstone/console/coordinator.py
+++ b/turnstone/console/coordinator.py
@@ -360,11 +360,17 @@ class CoordinatorManager:
row = self._storage.get_workstream(ws_id)
if row is None or row.get("kind") != WorkstreamKind.COORDINATOR:
return None
- # close()/delete() only soft-mark the row — refuse to
- # resurrect those sessions on any subsequent GET, or the
- # Close button becomes silently reversible on URL revisit
- # and burns max_active capacity.
- if row.get("state") in {"closed", "deleted"}:
+ # ``deleted`` is a tombstone — the row is on its way out
+ # and must never resurrect. ``closed`` used to be in the
+ # same bucket (so a stray URL revisit couldn't silently
+ # reverse a Close), but the Saved Coordinators landing UI
+ # makes restore an explicit user action: clicking a saved
+ # card calls POST /open and the user is consenting to
+ # reload. ``_reserve_and_install_locked`` still enforces
+ # ``max_active`` (evicting an idle peer or 429-ing) so the
+ # safety the old guard provided now lives in the slot
+ # accounting, not in a flat-refusal.
+ if row.get("state") == "deleted":
return None
row_owner = row.get("user_id") or ""
# Strict equality (not short-circuit on empty row_owner)
@@ -437,6 +443,23 @@ class CoordinatorManager:
ws_id[:8],
exc_info=True,
)
+
+ # No DB state-flip on resurrect. The in-memory session
+ # is now IDLE; the DB row may still say 'closed' from the
+ # last close(). We deliberately don't write 'idle' here
+ # because:
+ # (a) it would race a concurrent close() (which writes
+ # 'closed' under self._lock without acquiring this
+ # per-ws open_lock) and could overwrite the
+ # authoritative close;
+ # (b) the next set_state() call (any state transition
+ # — running, attention, idle-after-running) syncs
+ # the DB naturally;
+ # (c) the saved-coordinators list filters by state +
+ # excludes coordinators currently loaded into
+ # coord_mgr, so the stale 'closed' state on disk
+ # doesn't make a still-loaded coordinator appear
+ # as a saved card.
# Rehydration: fan out a console-pseudo-node
# ``ws_created`` so a tab that opens a persisted-but-
# unloaded coordinator sees it live on every other
diff --git a/turnstone/console/server.py b/turnstone/console/server.py
index 6b99fe4f..022fda55 100644
--- a/turnstone/console/server.py
+++ b/turnstone/console/server.py
@@ -2834,6 +2834,93 @@ async def coordinator_list(request: Request) -> JSONResponse:
)
+async def coordinator_saved(request: Request) -> JSONResponse:
+ """GET /v1/api/coordinator/saved — list saved (closed-but-persisted) coordinator
+ sessions for the caller.
+
+ Mirrors :func:`turnstone.server.list_saved_workstreams` for the
+ coordinator surface so the frontend can render a "saved coordinators"
+ card grid identically to the interactive "saved workstreams" sidebar.
+ Same response item shape, same tenant/admin scoping.
+
+ Returns ONLY rows with ``state='closed'`` — explicitly closed
+ coordinators that the user can re-open via the saved card. Active /
+ in-flight rows live in the active list above; deleted rows are
+ tombstones that ``_open_impl`` refuses to resurrect.
+
+ Also filters out coordinators currently loaded into ``coord_mgr``
+ (defence-in-depth: a coordinator could be 'closed' on disk but
+ still in the warm pool right after a restart of the close-emit
+ sequence races the in-memory pop).
+ """
+ from turnstone.core.memory import list_workstreams_with_history
+ from turnstone.core.workstream import WorkstreamKind
+
+ err = _require_admin_coordinator(request)
+ if err is not None:
+ return err
+
+ if _is_admin(request):
+ user_filter: str | None = None
+ else:
+ caller_uid = _auth_user_id(request)
+ if not caller_uid:
+ # Blank sub on a non-service / non-admin token — fail closed
+ # rather than match every orphan / migration row with empty
+ # user_id. Mirrors list_saved_workstreams.
+ return JSONResponse({"coordinators": []})
+ user_filter = caller_uid
+
+ # Offload the blocking storage call + the lock-acquiring list_all
+ # off the event loop, matching coordinator_create's pattern (#perf-2
+ # from the saved-coordinators review). list_workstreams_with_history
+ # runs a correlated COUNT subquery; coord_mgr.list_all() takes the
+ # manager lock. Either can stall every other async handler if run
+ # inline.
+ rows = await asyncio.to_thread(
+ list_workstreams_with_history,
+ limit=50,
+ kind=WorkstreamKind.COORDINATOR,
+ user_id=user_filter,
+ state="closed",
+ )
+
+ coord_mgr = getattr(request.app.state, "coord_mgr", None)
+ loaded: set[str] = set()
+ if coord_mgr is not None:
+ try:
+ loaded = await asyncio.to_thread(
+ lambda: {ws.id for ws in coord_mgr.list_all()},
+ )
+ except Exception:
+ log.debug("coordinator_saved.list_all_failed", exc_info=True)
+
+ # Column order from list_workstreams_with_history is
+ # (ws_id, alias, title, name, created, updated, count, node_id) — see
+ # the SELECT in turnstone/core/storage/_sqlite.py:list_workstreams_with_history.
+ # ``*_extra`` swallows the trailing node_id (and any future columns
+ # appended at the tail); keep this comment in sync if the SELECT
+ # changes the prefix order.
+ result = [
+ {
+ "ws_id": wid,
+ "alias": alias,
+ "title": title,
+ "name": name,
+ "created": created,
+ "updated": updated,
+ "message_count": count,
+ }
+ for wid, alias, title, name, created, updated, count, *_extra in rows
+ if wid not in loaded
+ ]
+ # Key is ``coordinators`` (not ``workstreams``) for consistency with
+ # the sibling coordinator_list endpoint. Item shape matches the
+ # interactive saved-workstreams response so the frontend card renderer
+ # stays a 1:1 mirror.
+ return JSONResponse({"coordinators": result})
+
+
async def coordinator_page(request: Request) -> Response:
"""GET /coordinator/{ws_id} — serve the one-pane coordinator HTML.
@@ -10393,6 +10480,13 @@ def create_app(
methods=["POST"],
),
Route("/api/coordinator", coordinator_list, methods=["GET"]),
+ # Literal path BEFORE the /{ws_id} routes below so
+ # Starlette doesn't match "saved" as a ws_id.
+ Route(
+ "/api/coordinator/saved",
+ coordinator_saved,
+ methods=["GET"],
+ ),
Route(
"/api/coordinator/{ws_id}/send",
coordinator_send,
diff --git a/turnstone/console/static/app.js b/turnstone/console/static/app.js
index 82408e72..e8083ad1 100644
--- a/turnstone/console/static/app.js
+++ b/turnstone/console/static/app.js
@@ -20,6 +20,9 @@ window.onLoginSuccess = function () {
// (#9) — no poller to restart after login. The home-view renderer
// reads from clusterState.nodes["console"].workstreams on every SSE
// patch, so authenticating just unblocks the normal event stream.
+ if (typeof loadSavedCoordinators === "function") {
+ loadSavedCoordinators();
+ }
};
window.onLogout = function () {
if (evtSource) {
@@ -141,12 +144,30 @@ function patchClusterState(data) {
});
}
} else if (t === "ws_closed") {
+ // Peek BEFORE the filter so we can tell whether the closed ws was a
+ // coordinator (lives on the console pseudo-node, kind="coordinator")
+ // and only then refetch the saved list. ws_closed payloads from
+ // real-node interactive closes don't carry kind on the wire, but
+ // they're already typed in clusterState from the matching ws_created
+ // event. Skipping interactive closes avoids per-close fan-out into
+ // /v1/api/coordinator/saved on busy clusters.
+ var wasCoordinator = false;
+ Object.keys(clusterState.nodes).forEach(function (nid) {
+ (clusterState.nodes[nid].workstreams || []).forEach(function (ws) {
+ if (ws.id === data.ws_id && ws.kind === "coordinator") {
+ wasCoordinator = true;
+ }
+ });
+ });
Object.keys(clusterState.nodes).forEach(function (nid) {
var n = clusterState.nodes[nid];
n.workstreams = (n.workstreams || []).filter(function (ws) {
return ws.id !== data.ws_id;
});
});
+ if (wasCoordinator && typeof loadSavedCoordinators === "function") {
+ loadSavedCoordinators();
+ }
} else if (t === "ws_rename") {
Object.keys(clusterState.nodes).forEach(function (nid) {
(clusterState.nodes[nid].workstreams || []).forEach(function (ws) {
@@ -2199,6 +2220,114 @@ function _renderHomeView() {
if (line) line.textContent = summaryText;
}
+// ---------------------------------------------------------------------------
+// Saved coordinators — closed sessions persisted on disk. Mirrors the
+// interactive UI's "Saved Workstreams" card grid (same /shared/cards.css
+// primitives, same /shared/cards.js renderSessionCard helper, same
+// response item shape from /v1/api/coordinator/saved). Click a card →
+// POST /open then /coordinator/{ws_id}; coordinator_detail lazily
+// rehydrates from storage on the GET miss.
+// ---------------------------------------------------------------------------
+
+// In-flight de-dup for loadSavedCoordinators. ws_closed events can
+// arrive in bursts on a busy cluster; without this guard each one
+// triggers a parallel fetch. Single boolean is enough because the
+// renderer reads from the latest response — a coalesced re-fetch right
+// after the in-flight one resolves catches any state change.
+var _savedCoordsInFlight = false;
+var _savedCoordsRetry = false;
+
+function loadSavedCoordinators() {
+ if (!_hasCoordPermission()) return;
+ if (_savedCoordsInFlight) {
+ _savedCoordsRetry = true;
+ return;
+ }
+ _savedCoordsInFlight = true;
+ authFetch("/v1/api/coordinator/saved")
+ .then(function (r) {
+ return r.ok ? r.json() : { coordinators: [] };
+ })
+ .then(function (data) {
+ renderSavedCoordinators(data.coordinators || []);
+ })
+ .catch(function () {
+ /* silent — saved list is informational, not load-bearing */
+ })
+ .finally(function () {
+ _savedCoordsInFlight = false;
+ // If at least one call arrived while we were in flight, fire one
+ // catch-up fetch (not N) so the UI reflects the latest state
+ // without a per-event fan-out.
+ if (_savedCoordsRetry) {
+ _savedCoordsRetry = false;
+ loadSavedCoordinators();
+ }
+ });
+}
+
+function renderSavedCoordinators(items) {
+ var section = document.getElementById("saved-coordinators");
+ var cards = document.getElementById("saved-coord-cards");
+ var countEl = document.getElementById("saved-coord-count");
+ if (!section || !cards) return;
+ if (!items.length) {
+ section.style.display = "none";
+ cards.replaceChildren();
+ if (countEl) countEl.textContent = "";
+ return;
+ }
+ section.style.display = "";
+ if (countEl) countEl.textContent = "(" + items.length + ")";
+ cards.replaceChildren();
+ items.forEach(function (sess) {
+ var card = renderSessionCard(sess, {
+ ariaLabel: function (s) {
+ return (
+ "Resume coordinator: " + (s.alias || s.title || s.name || s.ws_id)
+ );
+ },
+ onActivate: function (s, cardEl) {
+ // POST /open BEFORE navigating so capacity issues surface as a
+ // toast instead of a broken-looking detail page. The /open
+ // endpoint calls the same lazy-rehydrate path the GET would,
+ // but we get the status code synchronously so the user learns
+ // "all slots in use" instead of staring at a 404.
+ cardEl.classList.add("is-busy");
+ authFetch(
+ "/v1/api/coordinator/" + encodeURIComponent(s.ws_id) + "/open",
+ { method: "POST" },
+ )
+ .then(function (r) {
+ if (r.ok) {
+ window.location.href =
+ "/coordinator/" + encodeURIComponent(s.ws_id);
+ return;
+ }
+ cardEl.classList.remove("is-busy");
+ if (r.status === 429) {
+ showToast(
+ "All coordinator slots are active — close one first to restore this session",
+ );
+ } else if (r.status === 404) {
+ showToast("Coordinator no longer available");
+ loadSavedCoordinators();
+ } else if (r.status === 503) {
+ showToast("Coordinator subsystem not configured");
+ } else {
+ showToast("Failed to restore coordinator (" + r.status + ")");
+ }
+ })
+ .catch(function () {
+ cardEl.classList.remove("is-busy");
+ showToast("Failed to restore coordinator");
+ });
+ },
+ });
+ cards.appendChild(card);
+ });
+}
+
// --- Init ---
// SSE connects after auth is confirmed — either via onLoginSuccess after
// login, or after the first successful data load (page refresh with valid cookie).
@@ -2217,10 +2346,28 @@ initLogin();
// coordinator ws_created / ws_closed / cluster_state events.
loadOverview();
_ensureHomeComposerInit();
-// Refresh the coord button visibility after initial whoami lands in
-// sessionStorage (auth.js populates it asynchronously). A short delay
-// is good enough — the shared pattern for permission-gated UI.
-setTimeout(_refreshHomeComposerVisibility, 500);
+// Refresh the coord button visibility once auth.js has populated
+// sessionStorage from the initial whoami. window.permissionsReady
+// resolves after that completes (success or failure); fall back to a
+// short timeout if the promise isn't available (older auth.js).
+//
+// NOTE: permissionsReady is one-shot — it fires exactly once per page
+// load (see auth.js). Subsequent re-logins are caught by the
+// onLoginSuccess hook above which calls loadSavedCoordinators() again.
+if (
+ window.permissionsReady &&
+ typeof window.permissionsReady.then === "function"
+) {
+ window.permissionsReady.then(function () {
+ _refreshHomeComposerVisibility();
+ loadSavedCoordinators();
+ });
+} else {
+ setTimeout(function () {
+ _refreshHomeComposerVisibility();
+ loadSavedCoordinators();
+ }, 500);
+}
// --- Node Metadata Panel (read-only in node detail view) ---
function _loadNodeMetadataPanel(nodeId) {
diff --git a/turnstone/console/static/index.html b/turnstone/console/static/index.html
index 8b8b1a1d..72ca8b2a 100644
--- a/turnstone/console/static/index.html
+++ b/turnstone/console/static/index.html
@@ -1,1692 +1,4042 @@
-
+
-
-
-
-turnstone console
-
-
-
-
-
-
-
-
-
-