mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-13 15:32:24 -06:00
Compare commits
29 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| ac1fd67137 | |||
| 1b40ae79f9 | |||
| b078ddccf0 | |||
| 4b6c93a0e9 | |||
| 9d283e951f | |||
| 4e407e7d4f | |||
| 7ab24e500b | |||
| 5bcbcb73b9 | |||
| af6749421a | |||
| 4d6cb77075 | |||
| 5c225ef39b | |||
| f5a843f44a | |||
| cf44841624 | |||
| 7ffab6a272 | |||
| ba3bc9d989 | |||
| dbe023b4dd | |||
| 3d3a8b7367 | |||
| 99eff73a97 | |||
| 99fcd30299 | |||
| 423c2e80b7 | |||
| a0eb77360d | |||
| 3dd0e196fe | |||
| 19c3db5329 | |||
| 0bea72019e | |||
| b9ff52d582 | |||
| 961f999c93 | |||
| 8bdb916064 | |||
| 25fe4e728a | |||
| 0d1a32ff65 |
+1
-1
@@ -4,7 +4,7 @@ build-backend = "hatchling.build"
|
||||
|
||||
[project]
|
||||
name = "turnstone"
|
||||
version = "1.5.0"
|
||||
version = "1.5.3"
|
||||
description = "Multi-node AI orchestration platform with tool use, agent routing, and cluster simulation."
|
||||
readme = "README.md"
|
||||
license = "BUSL-1.1"
|
||||
|
||||
@@ -0,0 +1,397 @@
|
||||
"""Console-side coord_registry auto-refresh on model-definition CRUD + reload.
|
||||
|
||||
The console builds ``app.state.coord_registry`` once at lifespan startup
|
||||
and the coordinator session factory closes over that exact instance.
|
||||
Without these refresh hooks, an admin who edits a model definition
|
||||
through the UI sees the DB change immediately but coordinator sessions
|
||||
keep calling the prior model name — the on-disk truth diverges from the
|
||||
in-process registry until the console is restarted.
|
||||
|
||||
These tests cover both the helper (``_refresh_coord_registry``)
|
||||
and the four wired endpoints (create / update / delete / explicit reload)
|
||||
to lock in:
|
||||
|
||||
- in-place mutation: ``coord_registry`` object identity is preserved
|
||||
across refreshes (factory closure must not be invalidated);
|
||||
- failure isolation: a load or reload failure leaves the existing
|
||||
registry intact rather than tearing down a working coordinator;
|
||||
- no-op safety: the helper short-circuits when ``coord_registry`` is
|
||||
``None`` so a coord-less console (no model rows at boot) doesn't
|
||||
500 on routine model-definition CRUD.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Any
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
from starlette.applications import Starlette
|
||||
from starlette.middleware import Middleware
|
||||
from starlette.routing import Route
|
||||
from starlette.testclient import TestClient
|
||||
|
||||
from tests._coord_test_helpers import _AuthMiddleware
|
||||
from turnstone.console.server import (
|
||||
_refresh_coord_registry,
|
||||
admin_create_model_definition,
|
||||
admin_delete_model_definition,
|
||||
admin_model_reload,
|
||||
admin_update_model_definition,
|
||||
)
|
||||
from turnstone.core.model_registry import ModelConfig, ModelRegistry
|
||||
from turnstone.core.storage._sqlite import SQLiteBackend
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Fixtures
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def storage(tmp_path: Any) -> SQLiteBackend:
|
||||
return SQLiteBackend(str(tmp_path / "models.db"))
|
||||
|
||||
|
||||
def _seed_model_def(
|
||||
storage: SQLiteBackend,
|
||||
*,
|
||||
definition_id: str,
|
||||
alias: str,
|
||||
model: str,
|
||||
base_url: str = "http://localhost:8000/v1",
|
||||
enabled: bool = True,
|
||||
) -> None:
|
||||
"""Insert a model definition row directly via the storage API."""
|
||||
storage.create_model_definition(
|
||||
definition_id=definition_id,
|
||||
alias=alias,
|
||||
model=model,
|
||||
provider="openai-compatible",
|
||||
base_url=base_url,
|
||||
api_key="sk-test",
|
||||
context_window=8192,
|
||||
capabilities="{}",
|
||||
enabled=enabled,
|
||||
created_by="admin",
|
||||
)
|
||||
|
||||
|
||||
def _make_config(alias: str, model: str) -> ModelConfig:
|
||||
return ModelConfig(
|
||||
alias=alias,
|
||||
base_url="http://localhost:8000/v1",
|
||||
api_key="sk-test",
|
||||
model=model,
|
||||
context_window=8192,
|
||||
provider="openai-compatible",
|
||||
source="db",
|
||||
)
|
||||
|
||||
|
||||
def _make_registry(
|
||||
*,
|
||||
alias: str = "local",
|
||||
model: str = "old-model",
|
||||
extras: dict[str, str] | None = None,
|
||||
) -> ModelRegistry:
|
||||
"""Build a real ModelRegistry seeded with ``alias`` (the default) plus
|
||||
any ``extras`` (alias → model). ``ModelRegistry.__init__`` rejects an
|
||||
empty model dict so tests that exercise the helper need at least one
|
||||
entry; pass ``extras`` for multi-alias scenarios (e.g. delete-by-alias).
|
||||
"""
|
||||
configs = {alias: _make_config(alias, model)}
|
||||
for extra_alias, extra_model in (extras or {}).items():
|
||||
configs[extra_alias] = _make_config(extra_alias, extra_model)
|
||||
return ModelRegistry(configs, default=alias)
|
||||
|
||||
|
||||
class _AppState:
|
||||
"""Shim mirroring Starlette's ``app.state`` for direct helper tests."""
|
||||
|
||||
coord_registry: ModelRegistry | None = None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helper-level tests — ``_refresh_coord_registry`` semantics
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_helper_rebuilds_registry_from_db(storage: SQLiteBackend) -> None:
|
||||
"""Helper pulls the latest DB rows into the existing registry."""
|
||||
_seed_model_def(storage, definition_id="m1", alias="local", model="new-model")
|
||||
state = _AppState()
|
||||
state.coord_registry = _make_registry(alias="local", model="old-model")
|
||||
|
||||
_refresh_coord_registry(state, storage)
|
||||
|
||||
assert state.coord_registry is not None
|
||||
assert state.coord_registry.get_config("local").model == "new-model"
|
||||
|
||||
|
||||
def test_helper_preserves_object_identity(storage: SQLiteBackend) -> None:
|
||||
"""The factory closes over the registry object — refresh must mutate
|
||||
in place rather than swap the attribute."""
|
||||
_seed_model_def(storage, definition_id="m1", alias="local", model="new-model")
|
||||
state = _AppState()
|
||||
state.coord_registry = _make_registry()
|
||||
before = id(state.coord_registry)
|
||||
|
||||
_refresh_coord_registry(state, storage)
|
||||
|
||||
assert id(state.coord_registry) == before
|
||||
|
||||
|
||||
def test_helper_noop_when_coord_registry_none(storage: SQLiteBackend) -> None:
|
||||
"""Console boot with no model rows leaves coord_registry = None.
|
||||
The helper must not 500 in that state — CRUD that lands the FIRST
|
||||
row would otherwise fail before the operator can recover."""
|
||||
_seed_model_def(storage, definition_id="m1", alias="local", model="m")
|
||||
state = _AppState()
|
||||
state.coord_registry = None
|
||||
|
||||
_refresh_coord_registry(state, storage) # must not raise
|
||||
|
||||
assert state.coord_registry is None
|
||||
|
||||
|
||||
def test_helper_preserves_registry_when_load_fails(
|
||||
storage: SQLiteBackend, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""An unexpected error from ``load_model_registry`` (e.g. config.toml
|
||||
parse failure, programming bug) must not tear down a working
|
||||
registry — log + leave the existing instance intact."""
|
||||
state = _AppState()
|
||||
state.coord_registry = _make_registry(alias="local", model="old-model")
|
||||
|
||||
def _boom(**_kw: Any) -> ModelRegistry:
|
||||
raise RuntimeError("simulated loader failure")
|
||||
|
||||
monkeypatch.setattr("turnstone.core.model_registry.load_model_registry", _boom)
|
||||
_refresh_coord_registry(state, storage)
|
||||
|
||||
assert state.coord_registry is not None
|
||||
assert state.coord_registry.get_config("local").model == "old-model"
|
||||
|
||||
|
||||
def test_helper_preserves_registry_when_strict_load_fails(
|
||||
storage: SQLiteBackend, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""``load_model_registry`` normally swallows storage read errors and
|
||||
would return a config.toml-only registry on a transient DB outage —
|
||||
applying that via ``reload()`` would silently drop every DB-sourced
|
||||
alias. The helper passes ``strict=True`` so the loader re-raises
|
||||
instead, the helper's outer except catches it, and the existing
|
||||
registry survives intact."""
|
||||
_seed_model_def(storage, definition_id="m1", alias="local", model="db-model")
|
||||
state = _AppState()
|
||||
state.coord_registry = _make_registry(alias="local", model="db-model")
|
||||
|
||||
def _broken(**_kw: Any) -> Any:
|
||||
raise RuntimeError("simulated transient DB outage")
|
||||
|
||||
monkeypatch.setattr(storage, "list_model_definitions", _broken)
|
||||
_refresh_coord_registry(state, storage)
|
||||
|
||||
assert state.coord_registry is not None
|
||||
# Existing registry untouched — strict=True surfaced the storage
|
||||
# error to the helper before the loader's silent fallback could
|
||||
# produce a truncated registry for reload().
|
||||
assert state.coord_registry.get_config("local").model == "db-model"
|
||||
|
||||
|
||||
def test_helper_preserves_registry_when_no_enabled_rows(storage: SQLiteBackend) -> None:
|
||||
"""All rows disabled/deleted: ModelRegistry.__init__ rejects an empty
|
||||
model dict (raises ValueError). Helper must catch and preserve the
|
||||
existing registry so coord stays usable while admin restores rows."""
|
||||
_seed_model_def(storage, definition_id="m1", alias="local", model="m", enabled=False)
|
||||
state = _AppState()
|
||||
state.coord_registry = _make_registry(alias="local", model="cached-model")
|
||||
|
||||
_refresh_coord_registry(state, storage)
|
||||
|
||||
assert state.coord_registry is not None
|
||||
assert state.coord_registry.get_config("local").model == "cached-model"
|
||||
|
||||
|
||||
def test_helper_preserves_registry_on_reload_validation_error(
|
||||
storage: SQLiteBackend, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""A reload that raises mid-mutation (e.g. validation guard) must
|
||||
leave the existing registry instance functional."""
|
||||
_seed_model_def(storage, definition_id="m1", alias="local", model="new-model")
|
||||
state = _AppState()
|
||||
state.coord_registry = _make_registry(alias="local", model="old-model")
|
||||
|
||||
def _broken_reload(*_a: Any, **_kw: Any) -> None:
|
||||
raise ValueError("simulated reload validation failure")
|
||||
|
||||
monkeypatch.setattr(state.coord_registry, "reload", _broken_reload)
|
||||
_refresh_coord_registry(state, storage)
|
||||
|
||||
# Existing registry still reachable; the broken reload was a no-op
|
||||
# at the public-facing level.
|
||||
assert state.coord_registry is not None
|
||||
assert state.coord_registry.get_config("local").model == "old-model"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Endpoint-level integration tests — verify wiring
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _make_client(storage: SQLiteBackend, registry: ModelRegistry | None) -> TestClient:
|
||||
"""Build a TestClient wired to the four model-definition endpoints.
|
||||
|
||||
Uses the shared header-driven ``_AuthMiddleware`` from
|
||||
``tests/_coord_test_helpers``; default headers below grant
|
||||
``admin.models`` permission so the endpoint gate passes.
|
||||
"""
|
||||
app = Starlette(
|
||||
routes=[
|
||||
Route(
|
||||
"/v1/api/admin/model-definitions",
|
||||
admin_create_model_definition,
|
||||
methods=["POST"],
|
||||
),
|
||||
Route(
|
||||
"/v1/api/admin/model-definitions/reload",
|
||||
admin_model_reload,
|
||||
methods=["POST"],
|
||||
),
|
||||
Route(
|
||||
"/v1/api/admin/model-definitions/{definition_id}",
|
||||
admin_update_model_definition,
|
||||
methods=["PUT"],
|
||||
),
|
||||
Route(
|
||||
"/v1/api/admin/model-definitions/{definition_id}",
|
||||
admin_delete_model_definition,
|
||||
methods=["DELETE"],
|
||||
),
|
||||
],
|
||||
middleware=[Middleware(_AuthMiddleware)],
|
||||
)
|
||||
app.state.auth_storage = storage
|
||||
app.state.coord_registry = registry
|
||||
# Reload endpoint also touches these — stub them so the test focuses
|
||||
# on the registry-refresh behaviour without dragging in a full
|
||||
# collector / proxy_client wiring.
|
||||
app.state.collector = MagicMock()
|
||||
app.state.collector.get_all_nodes.return_value = []
|
||||
app.state.proxy_client = MagicMock()
|
||||
app.state.config_store = MagicMock()
|
||||
client = TestClient(app)
|
||||
client.headers.update({"X-Test-User": "admin", "X-Test-Perms": "admin.models"})
|
||||
return client
|
||||
|
||||
|
||||
def test_create_endpoint_refreshes_registry(storage: SQLiteBackend) -> None:
|
||||
"""POST /api/admin/model-definitions bumps the in-process registry
|
||||
so newly-spawned coord sessions see the new alias immediately."""
|
||||
# Pre-existing alias (registry needs at least one row)
|
||||
_seed_model_def(storage, definition_id="m1", alias="local", model="m")
|
||||
registry = _make_registry(alias="local", model="m")
|
||||
client = _make_client(storage, registry)
|
||||
|
||||
resp = client.post(
|
||||
"/v1/api/admin/model-definitions",
|
||||
json={
|
||||
"alias": "fast",
|
||||
"model": "fast-model",
|
||||
"provider": "openai-compatible",
|
||||
"base_url": "http://localhost:9000/v1",
|
||||
"api_key": "sk-x",
|
||||
"context_window": 4096,
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 200, resp.text
|
||||
assert registry.has_alias("fast")
|
||||
assert registry.get_config("fast").model == "fast-model"
|
||||
|
||||
|
||||
def test_update_endpoint_refreshes_registry(storage: SQLiteBackend) -> None:
|
||||
"""PUT swaps the underlying model name behind a stable alias — the
|
||||
user's reported regression."""
|
||||
_seed_model_def(storage, definition_id="m1", alias="local", model="old-model")
|
||||
registry = _make_registry(alias="local", model="old-model")
|
||||
client = _make_client(storage, registry)
|
||||
|
||||
resp = client.put(
|
||||
"/v1/api/admin/model-definitions/m1",
|
||||
json={"model": "new-model"},
|
||||
)
|
||||
assert resp.status_code == 200, resp.text
|
||||
assert registry.get_config("local").model == "new-model"
|
||||
|
||||
|
||||
def test_update_endpoint_skips_refresh_on_empty_body(
|
||||
storage: SQLiteBackend, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""An empty PUT body must skip the registry refresh — the
|
||||
``if updates:`` gate exists because ``load_model_registry`` is
|
||||
non-trivial and a no-op refresh on every PUT would burn cycles
|
||||
rebuilding state that hasn't changed. Spy on the helper to lock
|
||||
the gate down: a regression that drops the conditional would
|
||||
register a call here and trip the assertion.
|
||||
"""
|
||||
from turnstone.console import server as server_module
|
||||
|
||||
_seed_model_def(storage, definition_id="m1", alias="local", model="locked-in")
|
||||
registry = _make_registry(alias="local", model="locked-in")
|
||||
client = _make_client(storage, registry)
|
||||
|
||||
calls: list[tuple[Any, Any]] = []
|
||||
|
||||
def _spy(app_state: Any, storage: Any) -> None:
|
||||
calls.append((app_state, storage))
|
||||
|
||||
monkeypatch.setattr(server_module, "_refresh_coord_registry", _spy)
|
||||
|
||||
resp = client.put("/v1/api/admin/model-definitions/m1", json={})
|
||||
assert resp.status_code == 200, resp.text
|
||||
assert calls == [] # gate held: empty body did not trigger a refresh
|
||||
|
||||
|
||||
def test_delete_endpoint_refreshes_registry(storage: SQLiteBackend) -> None:
|
||||
"""DELETE drops the alias from the in-process registry too — a
|
||||
coord session that tried to resolve the deleted alias would
|
||||
otherwise hit a stale cached client."""
|
||||
_seed_model_def(storage, definition_id="m1", alias="local", model="m")
|
||||
_seed_model_def(storage, definition_id="m2", alias="extra", model="x")
|
||||
registry = _make_registry(alias="local", model="m", extras={"extra": "x"})
|
||||
client = _make_client(storage, registry)
|
||||
|
||||
resp = client.delete("/v1/api/admin/model-definitions/m2")
|
||||
assert resp.status_code == 200, resp.text
|
||||
assert not registry.has_alias("extra")
|
||||
assert registry.has_alias("local") # default alias unaffected
|
||||
|
||||
|
||||
def test_reload_endpoint_refreshes_registry(
|
||||
storage: SQLiteBackend, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""The explicit reload button must refresh the console's own
|
||||
registry — until this PR it only fanned out to nodes."""
|
||||
_seed_model_def(storage, definition_id="m1", alias="local", model="initial")
|
||||
registry = _make_registry(alias="local", model="initial")
|
||||
client = _make_client(storage, registry)
|
||||
|
||||
# Bypass the CRUD endpoints to mimic an out-of-band DB change (e.g.
|
||||
# an operator psql session) and verify the explicit reload path
|
||||
# still pulls the change in.
|
||||
storage.update_model_definition("m1", model="reloaded-model")
|
||||
|
||||
# Stub the async cluster fan-out helpers — they require a fully-wired
|
||||
# collector / proxy_client which is orthogonal to the helper under test.
|
||||
async def _noop_publish(_request: Any) -> None:
|
||||
return None
|
||||
|
||||
async def _noop_notify(_request: Any) -> dict[str, Any]:
|
||||
return {}
|
||||
|
||||
monkeypatch.setattr("turnstone.console.server._publish_config_change", _noop_publish)
|
||||
monkeypatch.setattr("turnstone.console.server._notify_nodes_model_reload", _noop_notify)
|
||||
|
||||
resp = client.post("/v1/api/admin/model-definitions/reload")
|
||||
assert resp.status_code == 200, resp.text
|
||||
assert registry.get_config("local").model == "reloaded-model"
|
||||
@@ -314,6 +314,48 @@ class TestCollectorSnapshot:
|
||||
assert event["ws_id"] == "ws1"
|
||||
assert event["state"] == "running"
|
||||
|
||||
def test_apply_snapshot_state_change_forwards_pending_approval_detail(self):
|
||||
"""Reconnect-via-snapshot is the resync path after every console
|
||||
restart or network blip. Without forwarding the field here,
|
||||
a child sitting in approval-pending across the gap renders as
|
||||
``activity_state=approval`` with no buttons until the next
|
||||
state change — broken UX during the most common re-sync event."""
|
||||
c = _make_collector()
|
||||
c._nodes["node-a"] = NodeSnapshot(
|
||||
node_id="node-a",
|
||||
server_url="http://a:8080",
|
||||
workstreams={"ws1": {"id": "ws1", "name": "same", "state": "idle"}},
|
||||
)
|
||||
q: queue.Queue[dict] = queue.Queue()
|
||||
c.register_listener(q)
|
||||
|
||||
detail = {
|
||||
"items": [{"call_id": "c1", "header": "tool x"}],
|
||||
"judge_pending": False,
|
||||
}
|
||||
c._apply_snapshot(
|
||||
"node-a",
|
||||
{
|
||||
"type": "node_snapshot",
|
||||
"node_id": "node-a",
|
||||
"workstreams": [
|
||||
{
|
||||
"id": "ws1",
|
||||
"name": "same",
|
||||
"state": "running",
|
||||
"activity_state": "approval",
|
||||
"pending_approval_detail": detail,
|
||||
}
|
||||
],
|
||||
"health": {},
|
||||
"aggregate": {},
|
||||
},
|
||||
)
|
||||
|
||||
event = q.get_nowait()
|
||||
assert event["type"] == "cluster_state"
|
||||
assert event["pending_approval_detail"] == detail
|
||||
|
||||
def test_apply_snapshot_skips_empty_id_workstream(self):
|
||||
c = _make_collector()
|
||||
c._nodes["node-a"] = NodeSnapshot(node_id="node-a", server_url="http://a:8080")
|
||||
@@ -359,6 +401,40 @@ class TestCollectorDelta:
|
||||
# Verify in-memory state was updated
|
||||
assert c._nodes["node-a"].workstreams["ws1"]["state"] == "running"
|
||||
|
||||
def test_apply_delta_ws_state_forwards_pending_approval_detail(self):
|
||||
"""The rich approval payload now travels on the cluster bus so
|
||||
coord tabs can render inline approve/deny buttons in lockstep
|
||||
with the activity_state transition. Collector must forward
|
||||
the field verbatim — the adapter does the child-routing on
|
||||
top, but the bus carries the data."""
|
||||
c = _make_collector()
|
||||
c._nodes["node-a"] = NodeSnapshot(
|
||||
node_id="node-a",
|
||||
server_url="http://a:8080",
|
||||
workstreams={"ws1": {"id": "ws1", "name": "test", "state": "idle"}},
|
||||
)
|
||||
q: queue.Queue[dict] = queue.Queue()
|
||||
c.register_listener(q)
|
||||
|
||||
detail = {
|
||||
"items": [{"call_id": "c1", "header": "tool x"}],
|
||||
"judge_pending": False,
|
||||
}
|
||||
c._apply_delta(
|
||||
"node-a",
|
||||
{
|
||||
"type": "ws_state",
|
||||
"ws_id": "ws1",
|
||||
"state": "running",
|
||||
"activity_state": "approval",
|
||||
"pending_approval_detail": detail,
|
||||
},
|
||||
)
|
||||
|
||||
event = q.get_nowait()
|
||||
assert event["type"] == "cluster_state"
|
||||
assert event["pending_approval_detail"] == detail
|
||||
|
||||
def test_apply_delta_ws_created(self):
|
||||
c = _make_collector()
|
||||
c._nodes["node-a"] = NodeSnapshot(node_id="node-a", server_url="http://a:8080")
|
||||
|
||||
@@ -0,0 +1,106 @@
|
||||
"""Tests for the console's coordinator idle-cleanup thread helper.
|
||||
|
||||
The helper itself is a tiny loop wrapping ``mgr.close_idle``; the heavy
|
||||
lifting is in ``SessionManager.close_idle`` (covered in
|
||||
``test_session_manager.py``) and ``bulk_close_stale_orphans`` (covered
|
||||
in ``test_storage_sqlite.py``). These tests verify the glue:
|
||||
|
||||
- the helper runs an initial sweep BEFORE its first sleep (cold-start
|
||||
cleanup without blocking the lifespan),
|
||||
- the helper swallows exceptions so a transient DB blip can't kill the
|
||||
daemon thread,
|
||||
- the helper exits cleanly when ``stop_event`` is set.
|
||||
|
||||
The ``stop_event`` parameter is exclusively for tests — production
|
||||
callers pass ``None`` and the daemon runs for process lifetime.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import threading
|
||||
from unittest.mock import patch
|
||||
|
||||
from turnstone.console.server import _coord_idle_cleanup_thread
|
||||
|
||||
|
||||
class _StubMgr:
|
||||
def __init__(
|
||||
self, *, stop_event: threading.Event, expected_calls: int, raise_after: int = -1
|
||||
) -> None:
|
||||
self.calls: list[float] = []
|
||||
self.sleep_calls_at_each_close: list[int] = []
|
||||
self._stop_event = stop_event
|
||||
self._expected = expected_calls
|
||||
self._raise_after = raise_after
|
||||
self._sleep_count = 0
|
||||
|
||||
def close_idle(self, timeout_sec: float) -> list[str]:
|
||||
# Snapshot how many sleeps preceded this close — lets the
|
||||
# "initial sweep" test verify the first close_idle ran with
|
||||
# zero preceding sleeps.
|
||||
self.sleep_calls_at_each_close.append(self._sleep_count)
|
||||
self.calls.append(timeout_sec)
|
||||
try:
|
||||
if 0 <= self._raise_after < len(self.calls):
|
||||
raise RuntimeError("simulated DB blip")
|
||||
finally:
|
||||
# Set stop after the helper has been exercised enough,
|
||||
# regardless of whether this call raised.
|
||||
if len(self.calls) >= self._expected:
|
||||
self._stop_event.set()
|
||||
return []
|
||||
|
||||
def record_sleep(self, _seconds: float) -> None:
|
||||
self._sleep_count += 1
|
||||
|
||||
|
||||
def _run_until_done(mgr: _StubMgr, stop_event: threading.Event, timeout_sec: float) -> None:
|
||||
with patch("turnstone.console.server.time.sleep", mgr.record_sleep):
|
||||
thread = threading.Thread(
|
||||
target=_coord_idle_cleanup_thread,
|
||||
args=(mgr, timeout_sec, stop_event),
|
||||
daemon=True,
|
||||
)
|
||||
thread.start()
|
||||
thread.join(timeout=2.0)
|
||||
assert not thread.is_alive(), "helper failed to exit on stop_event"
|
||||
|
||||
|
||||
def test_coord_idle_cleanup_runs_initial_sweep_before_sleep() -> None:
|
||||
"""The first close_idle call must happen BEFORE the first time.sleep —
|
||||
otherwise cold-start orphans wait one ``check_every`` interval (~30 min
|
||||
on default 2h timeout) for the first reap. Crucial because the
|
||||
lifespan no longer does a synchronous initial sweep."""
|
||||
stop_event = threading.Event()
|
||||
mgr = _StubMgr(stop_event=stop_event, expected_calls=1)
|
||||
_run_until_done(mgr, stop_event, timeout_sec=120.0)
|
||||
assert mgr.sleep_calls_at_each_close == [0], "first close_idle should run before any sleep"
|
||||
|
||||
|
||||
def test_coord_idle_cleanup_calls_close_idle_each_tick() -> None:
|
||||
stop_event = threading.Event()
|
||||
mgr = _StubMgr(stop_event=stop_event, expected_calls=3)
|
||||
_run_until_done(mgr, stop_event, timeout_sec=120.0)
|
||||
assert len(mgr.calls) == 3
|
||||
assert all(t == 120.0 for t in mgr.calls)
|
||||
|
||||
|
||||
def test_coord_idle_cleanup_survives_close_idle_exceptions() -> None:
|
||||
"""A transient DB error must not kill the daemon thread — the next
|
||||
tick should still fire close_idle. Without the try/except, a single
|
||||
blip would silently leak orphans forever."""
|
||||
stop_event = threading.Event()
|
||||
mgr = _StubMgr(stop_event=stop_event, expected_calls=4, raise_after=1)
|
||||
_run_until_done(mgr, stop_event, timeout_sec=120.0)
|
||||
# All four calls must have fired despite calls 2-4 raising.
|
||||
assert len(mgr.calls) == 4
|
||||
|
||||
|
||||
def test_coord_idle_cleanup_exits_cleanly_on_stop_event() -> None:
|
||||
"""The stop_event mechanism is the test contract; verify the thread
|
||||
actually exits when the event is set, without needing exceptions or
|
||||
daemon-process termination."""
|
||||
stop_event = threading.Event()
|
||||
mgr = _StubMgr(stop_event=stop_event, expected_calls=2)
|
||||
_run_until_done(mgr, stop_event, timeout_sec=120.0)
|
||||
assert stop_event.is_set()
|
||||
@@ -558,3 +558,54 @@ class TestCoordinatorAdapterDispatchChildEvent:
|
||||
}
|
||||
)
|
||||
assert recorder.enqueued[0]["ws_id"] == "coord-a"
|
||||
|
||||
def test_dispatch_cluster_state_forwards_pending_approval_detail(self) -> None:
|
||||
"""The rich approval payload now rides on child_ws_state directly so
|
||||
the browser can mutate liveBadgeCache without a separate live-bulk
|
||||
fetch. Drift here means the inline approve/deny buttons would
|
||||
regress to chasing the dashboard cache (the load-storm pattern
|
||||
Shape A is unwinding)."""
|
||||
adapter, recorder, _ = self._setup()
|
||||
with adapter._children_lock:
|
||||
adapter._merge_child_ids_locked("coord-a", ["child-a1"])
|
||||
detail = {
|
||||
"items": [{"call_id": "c1", "header": "tool x"}],
|
||||
"judge_pending": False,
|
||||
}
|
||||
adapter._dispatch_child_event(
|
||||
{
|
||||
"type": "cluster_state",
|
||||
"ws_id": "child-a1",
|
||||
"state": "running",
|
||||
"activity_state": "approval",
|
||||
"pending_approval_detail": detail,
|
||||
}
|
||||
)
|
||||
assert len(recorder.enqueued) == 1
|
||||
payload = recorder.enqueued[0]
|
||||
assert payload["type"] == "child_ws_state"
|
||||
assert payload["activity_state"] == "approval"
|
||||
assert payload["pending_approval_detail"] == detail
|
||||
|
||||
def test_dispatch_cluster_state_pending_approval_detail_none_passes_through(
|
||||
self,
|
||||
) -> None:
|
||||
"""Missing pending_approval_detail (no approval pending, or pre-fix
|
||||
node mid-rolling-upgrade) must forward as None — not raise, not
|
||||
omit — so the browser's handleChildState treats it as "no SSE-
|
||||
supplied detail, fall back to cached value"."""
|
||||
adapter, recorder, _ = self._setup()
|
||||
with adapter._children_lock:
|
||||
adapter._merge_child_ids_locked("coord-a", ["child-a1"])
|
||||
adapter._dispatch_child_event(
|
||||
{
|
||||
"type": "cluster_state",
|
||||
"ws_id": "child-a1",
|
||||
"state": "running",
|
||||
"activity_state": "tool",
|
||||
}
|
||||
)
|
||||
assert len(recorder.enqueued) == 1
|
||||
payload = recorder.enqueued[0]
|
||||
assert "pending_approval_detail" in payload
|
||||
assert payload["pending_approval_detail"] is None
|
||||
|
||||
@@ -152,3 +152,95 @@ def test_coordinator_js_exposes_inline_approval_helpers():
|
||||
# any prior denial. bug-1 / bug-3 from the second /review pass.
|
||||
assert "Denied by user" in body
|
||||
assert "callOutcomes" in body
|
||||
|
||||
|
||||
def test_coordinator_js_handle_child_state_reads_sse_pending_approval_detail():
|
||||
"""Lock the Shape A behavior change: child_ws_state SSE events now
|
||||
carry ``pending_approval_detail`` directly so the browser mutates
|
||||
``liveBadgeCache`` without firing an urgent live-bulk fetch on
|
||||
every activity_state transition into/out of approval. A refactor
|
||||
that re-introduces the urgent-fetch path on routine transitions
|
||||
(or drops the SSE-source merge guard in flushLiveFetches) would
|
||||
re-open the load-storm pattern this PR is fixing.
|
||||
|
||||
Structural assertions (regex against multi-line source) — symbol-
|
||||
presence alone wouldn't catch a guard that keeps the names but
|
||||
inverts the comparison or drops the ``prev.live`` check. This
|
||||
codebase has no JS test framework, so locking the guard's shape
|
||||
here is the next-best thing to a behavioral test."""
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
coord_js = Path(__file__).resolve().parent.parent / (
|
||||
"turnstone/console/static/coordinator/coordinator.js"
|
||||
)
|
||||
body = coord_js.read_text(encoding="utf-8")
|
||||
|
||||
# handleChildState now reads the SSE-supplied detail.
|
||||
assert "ev.pending_approval_detail" in body
|
||||
# The pre-fix urgent-fetch on activity_state transitions is
|
||||
# gone (the 409 retry path keeps its own ``{ urgent: true }``
|
||||
# for stale-call_id refresh — that's a different scenario).
|
||||
assert "enteredApproval" not in body
|
||||
assert "leftApproval" not in body
|
||||
|
||||
# SSE-authoritative window constant is defined and used.
|
||||
assert re.search(r"\bconst\s+SSE_AUTHORITATIVE_MS\s*=\s*\d+", body), (
|
||||
"SSE_AUTHORITATIVE_MS constant must be defined as a numeric literal"
|
||||
)
|
||||
|
||||
# handleChildState writes sseUpdatedAt = Date.now() into the cache
|
||||
# entry it sets. This is the SSE-source tag; without it, the
|
||||
# merge guard in flushLiveFetches has nothing to gate on.
|
||||
assert re.search(
|
||||
r"sseUpdatedAt:\s*Date\.now\(\)",
|
||||
body,
|
||||
), "handleChildState must write sseUpdatedAt: Date.now() onto liveBadgeCache entries"
|
||||
|
||||
# flushLiveFetches' merge guard structure: SSE-set pending_approval
|
||||
# / _detail wins over a stale bulk-poll snapshot when (live) AND
|
||||
# (prev exists) AND (prev.sseUpdatedAt set) AND (within window)
|
||||
# AND (prev.live exists). Inverting the comparison or dropping
|
||||
# any of these guards reopens the clobber bug.
|
||||
merge_guard = re.search(
|
||||
r"if\s*\(\s*live\s*&&\s*prev\s*&&\s*prev\.sseUpdatedAt\s*&&\s*"
|
||||
r"now\s*-\s*prev\.sseUpdatedAt\s*<\s*SSE_AUTHORITATIVE_MS\s*&&\s*"
|
||||
r"prev\.live\s*\)",
|
||||
body,
|
||||
)
|
||||
assert merge_guard is not None, (
|
||||
"flushLiveFetches merge guard must be the conjunction "
|
||||
"(live && prev && prev.sseUpdatedAt && now - prev.sseUpdatedAt < "
|
||||
"SSE_AUTHORITATIVE_MS && prev.live). An inverted comparison or "
|
||||
"missing prev.live check would let a stale bulk-poll clobber a "
|
||||
"fresh SSE-set approval."
|
||||
)
|
||||
|
||||
# The merge body must preserve BOTH pending_approval and
|
||||
# pending_approval_detail from prev — preserving only one would
|
||||
# render a row with a phantom badge but no buttons (or vice versa).
|
||||
merge_body = re.search(
|
||||
r"mergedLive\s*=\s*Object\.assign\(\s*\{\}\s*,\s*live\s*,\s*\{"
|
||||
r"[^}]*pending_approval:\s*prev\.live\.pending_approval[^}]*"
|
||||
r"pending_approval_detail:\s*prev\.live\.pending_approval_detail",
|
||||
body,
|
||||
)
|
||||
assert merge_body is not None, (
|
||||
"Merge body must preserve both pending_approval AND "
|
||||
"pending_approval_detail from prev.live — preserving only one "
|
||||
"creates a half-rendered approval row."
|
||||
)
|
||||
|
||||
# flushLiveFetches must forward sseUpdatedAt onto the new cache
|
||||
# entry so the SSE-source tag survives the bulk-poll write back —
|
||||
# without this, every bulk-poll resets the window and the next
|
||||
# late-arriving poll silently clobbers.
|
||||
assert re.search(
|
||||
r"sseUpdatedAt:\s*prev\s*\?\s*prev\.sseUpdatedAt",
|
||||
body,
|
||||
), (
|
||||
"flushLiveFetches must forward prev.sseUpdatedAt onto the new "
|
||||
"cache entry (preserving the SSE-source window across bulk-poll "
|
||||
"cycles) — without this, the second bulk-poll after an SSE "
|
||||
"transition silently clobbers."
|
||||
)
|
||||
|
||||
@@ -8,6 +8,7 @@ from turnstone.core.metacognition import (
|
||||
NUDGE_RESUME,
|
||||
NUDGE_START,
|
||||
NUDGE_TOOL_ERROR,
|
||||
RepeatDetector,
|
||||
detect_completion,
|
||||
detect_correction,
|
||||
format_nudge,
|
||||
@@ -308,3 +309,70 @@ class TestRepeatNudge:
|
||||
"""Repeat nudge should fire even with zero memories."""
|
||||
state: dict[str, float] = {}
|
||||
assert should_nudge("repeat", state, message_count=5, memory_count=0) is True
|
||||
|
||||
|
||||
class TestRepeatDetector:
|
||||
"""Repeat-detection streak machine — fires only when the same signature
|
||||
is recorded ``threshold`` times *consecutively* (default 3). Recording
|
||||
any different signature resets the streak, so an interrupted repeat
|
||||
isn't flagged as a stuck loop."""
|
||||
|
||||
def test_below_threshold_does_not_fire(self):
|
||||
det = RepeatDetector()
|
||||
assert det.record("a") is False
|
||||
assert det.record("a") is False # second call still under threshold
|
||||
|
||||
def test_at_threshold_fires(self):
|
||||
det = RepeatDetector()
|
||||
det.record("a")
|
||||
det.record("a")
|
||||
assert det.record("a") is True
|
||||
|
||||
def test_continues_to_fire_past_threshold(self):
|
||||
# Caller is responsible for clearing after a fire — until they do,
|
||||
# subsequent identical calls keep returning True.
|
||||
det = RepeatDetector()
|
||||
det.record("a")
|
||||
det.record("a")
|
||||
assert det.record("a") is True
|
||||
assert det.record("a") is True
|
||||
|
||||
def test_clear_resets_count(self):
|
||||
det = RepeatDetector()
|
||||
det.record("a")
|
||||
det.record("a")
|
||||
det.clear()
|
||||
assert det.record("a") is False # back to 1 after clear
|
||||
|
||||
def test_intervening_sig_resets_streak(self):
|
||||
# The streak is consecutive: recording any other sig mid-streak
|
||||
# discards the in-progress count. An alternating pattern like
|
||||
# [A, A, B, A, A] is two short streaks of 2, not a streak of 4.
|
||||
det = RepeatDetector()
|
||||
det.record("a")
|
||||
det.record("a")
|
||||
assert det.record("b") is False # b at count 1; a's streak is gone
|
||||
assert det.record("a") is False # a starts fresh at 1
|
||||
assert det.record("a") is False # a at 2
|
||||
assert det.record("a") is True # a hits 3 — fresh streak completes
|
||||
|
||||
def test_errored_signature_counts_toward_repeat(self):
|
||||
# Regression: when metacog was split out of the system message,
|
||||
# the error-output skip got reintroduced and stuck-loop detection
|
||||
# silently broke for tools that kept failing. Detector itself is
|
||||
# signature-only — error vs. success is the caller's policy.
|
||||
det = RepeatDetector()
|
||||
# Caller records an errored call's sig the same as a successful one;
|
||||
# the streak is what matters.
|
||||
for _ in range(3):
|
||||
last = det.record("bash:ls /nonexistent")
|
||||
assert last is True
|
||||
|
||||
def test_custom_threshold(self):
|
||||
det = RepeatDetector(threshold=2)
|
||||
assert det.record("a") is False
|
||||
assert det.record("a") is True
|
||||
|
||||
def test_threshold_one_fires_immediately(self):
|
||||
det = RepeatDetector(threshold=1)
|
||||
assert det.record("a") is True
|
||||
|
||||
@@ -770,16 +770,76 @@ class TestRegistryReload:
|
||||
assert reg.has_alias("b")
|
||||
assert reg.default == "b"
|
||||
|
||||
def test_reload_clears_clients(self) -> None:
|
||||
models = {"a": ModelConfig("a", "http://x/v1", "key", "m")}
|
||||
def test_reload_keeps_clients_when_connection_target_unchanged(self) -> None:
|
||||
"""Selective teardown: a model edit that leaves base_url / api_key /
|
||||
provider intact (e.g. admin tweaks the underlying ``model`` name or
|
||||
``temperature``) keeps the cached HTTP client warm — no need to
|
||||
re-establish TLS+pool when the endpoint is the same."""
|
||||
models = {"a": ModelConfig("a", "http://x/v1", "key", "m1", provider="openai")}
|
||||
reg = ModelRegistry(models=models, default="a")
|
||||
# Force client creation
|
||||
reg.get_client("a")
|
||||
assert "a" in reg._clients
|
||||
client_before = reg._clients["a"]
|
||||
provider_before = reg.get_provider("a")
|
||||
|
||||
# Same endpoint (base_url, api_key, provider), only ``model`` changed.
|
||||
new_models = {"a": ModelConfig("a", "http://x/v1", "key", "m2", provider="openai")}
|
||||
reg.reload(new_models, "a")
|
||||
|
||||
assert "a" in reg._clients
|
||||
assert reg._clients["a"] is client_before
|
||||
assert "a" in reg._providers
|
||||
assert reg._providers["a"] is provider_before
|
||||
|
||||
def test_reload_drops_client_when_base_url_changes(self) -> None:
|
||||
"""A ``base_url`` change drops the cached client (different
|
||||
endpoint = new connection) but keeps the cached provider —
|
||||
``LLMProvider`` is keyed only on the provider string, which
|
||||
didn't change."""
|
||||
models = {"a": ModelConfig("a", "http://x/v1", "key", "m", provider="openai")}
|
||||
reg = ModelRegistry(models=models, default="a")
|
||||
reg.get_client("a")
|
||||
provider_before = reg.get_provider("a")
|
||||
|
||||
new_models = {"a": ModelConfig("a", "http://y/v1", "key", "m", provider="openai")}
|
||||
reg.reload(new_models, "a")
|
||||
|
||||
# Reload with same models — clients should be cleared
|
||||
reg.reload(dict(models), "a")
|
||||
assert "a" not in reg._clients
|
||||
assert "a" in reg._providers
|
||||
assert reg._providers["a"] is provider_before
|
||||
|
||||
def test_reload_drops_provider_when_provider_string_changes(self) -> None:
|
||||
"""A provider-type swap (e.g. openai → anthropic) drops both the
|
||||
client AND the provider so the next resolve picks up the right
|
||||
``LLMProvider`` implementation against the new SDK."""
|
||||
models = {"a": ModelConfig("a", "http://x/v1", "key", "m", provider="openai")}
|
||||
reg = ModelRegistry(models=models, default="a")
|
||||
reg.get_client("a")
|
||||
reg.get_provider("a")
|
||||
|
||||
new_models = {"a": ModelConfig("a", "http://x/v1", "key", "m", provider="anthropic")}
|
||||
reg.reload(new_models, "a")
|
||||
|
||||
assert "a" not in reg._clients
|
||||
assert "a" not in reg._providers
|
||||
|
||||
def test_reload_drops_clients_for_removed_aliases(self) -> None:
|
||||
"""Aliases removed from the registry must release their cached
|
||||
clients — otherwise a deleted endpoint's connection pool would
|
||||
outlive the alias indefinitely."""
|
||||
models = {
|
||||
"a": ModelConfig("a", "http://x/v1", "key", "m"),
|
||||
"b": ModelConfig("b", "http://y/v1", "key", "m"),
|
||||
}
|
||||
reg = ModelRegistry(models=models, default="a")
|
||||
reg.get_client("a")
|
||||
reg.get_client("b")
|
||||
|
||||
# Drop "b" entirely.
|
||||
new_models = {"a": ModelConfig("a", "http://x/v1", "key", "m")}
|
||||
reg.reload(new_models, "a")
|
||||
|
||||
assert "a" in reg._clients # unchanged endpoint, kept warm
|
||||
assert "b" not in reg._clients
|
||||
|
||||
def test_reload_validates_default(self) -> None:
|
||||
models_a = {"a": ModelConfig("a", "x", "x", "m")}
|
||||
|
||||
@@ -224,6 +224,25 @@ class TestOpenAIProvider:
|
||||
sanitize_messages([original])
|
||||
assert original["content"] is None
|
||||
|
||||
def test_sanitize_messages_strips_underscore_sibling_keys(self) -> None:
|
||||
"""Internal sibling metadata (``_reminders``, ``_reminders_delivered``,
|
||||
``_attachments_meta``, ``_provider_content``) must be stripped
|
||||
before the wire — the OpenAI-compat APIs reject unknown fields."""
|
||||
msgs = [
|
||||
{
|
||||
"role": "user",
|
||||
"content": "hi",
|
||||
"_reminders": [{"type": "correction", "text": "watch"}],
|
||||
"_reminders_delivered": True,
|
||||
"_attachments_meta": [{"kind": "image"}],
|
||||
}
|
||||
]
|
||||
result = sanitize_messages(msgs)
|
||||
assert result == [{"role": "user", "content": "hi"}]
|
||||
assert "_reminders" not in result[0]
|
||||
assert "_reminders_delivered" not in result[0]
|
||||
assert "_attachments_meta" not in result[0]
|
||||
|
||||
# -- sanitize_messages: orphan detection -----------------------------------
|
||||
|
||||
def test_sanitize_orphaned_tool_call_synthesized(self) -> None:
|
||||
|
||||
+837
-114
File diff suppressed because it is too large
Load Diff
@@ -18,13 +18,19 @@ from __future__ import annotations
|
||||
import threading
|
||||
import time
|
||||
from dataclasses import dataclass
|
||||
from datetime import UTC, datetime
|
||||
from typing import Any
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
from turnstone.core.session_manager import SessionKindAdapter, SessionManager
|
||||
from turnstone.core.workstream import Workstream, WorkstreamKind, WorkstreamState
|
||||
from turnstone.core.workstream import (
|
||||
BULK_CLOSE_STATE_VALUES,
|
||||
Workstream,
|
||||
WorkstreamKind,
|
||||
WorkstreamState,
|
||||
)
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Test fixtures
|
||||
@@ -156,6 +162,8 @@ class _Row:
|
||||
kind: str
|
||||
state: str = "idle"
|
||||
parent_ws_id: str | None = None
|
||||
updated: str = ""
|
||||
node_id: str | None = None
|
||||
|
||||
|
||||
class FakeStorage:
|
||||
@@ -164,8 +172,19 @@ class FakeStorage:
|
||||
def __init__(self) -> None:
|
||||
self.rows: dict[str, _Row] = {}
|
||||
self.state_updates: list[tuple[str, str]] = []
|
||||
self.touch_calls: list[str] = []
|
||||
self.register_raises = False
|
||||
self.lock = threading.Lock()
|
||||
# Live-services lookup target for close_idle pass 2. Map
|
||||
# service_type → list of live service_ids. Tests that exercise
|
||||
# liveness scoping populate this directly; default empty means
|
||||
# "no peers alive" (every row unprotected by liveness).
|
||||
self.live_services: dict[str, list[str]] = {}
|
||||
self.list_services_raises = False
|
||||
|
||||
@staticmethod
|
||||
def _now_iso() -> str:
|
||||
return datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%S")
|
||||
|
||||
def register_workstream(
|
||||
self,
|
||||
@@ -178,6 +197,8 @@ class FakeStorage:
|
||||
parent_ws_id: str | None = None,
|
||||
skill_id: str = "",
|
||||
skill_version: int = 0,
|
||||
state: str = "idle",
|
||||
updated: str | None = None,
|
||||
) -> None:
|
||||
if self.register_raises:
|
||||
raise RuntimeError("register forced failure")
|
||||
@@ -188,14 +209,66 @@ class FakeStorage:
|
||||
user_id=user_id or "",
|
||||
name=name,
|
||||
kind=kind_str,
|
||||
state=state,
|
||||
parent_ws_id=parent_ws_id,
|
||||
updated=updated if updated is not None else self._now_iso(),
|
||||
node_id=node_id,
|
||||
)
|
||||
|
||||
def touch_workstream(self, ws_id: str) -> None:
|
||||
with self.lock:
|
||||
self.touch_calls.append(ws_id)
|
||||
if ws_id in self.rows:
|
||||
self.rows[ws_id].updated = self._now_iso()
|
||||
|
||||
def update_workstream_state(self, ws_id: str, state: str) -> None:
|
||||
with self.lock:
|
||||
self.state_updates.append((ws_id, state))
|
||||
if ws_id in self.rows:
|
||||
self.rows[ws_id].state = state
|
||||
self.rows[ws_id].updated = self._now_iso()
|
||||
|
||||
def bulk_close_stale_orphans(
|
||||
self,
|
||||
kind: WorkstreamKind | str,
|
||||
cutoff: str,
|
||||
exclude_ws_ids: list[str],
|
||||
live_node_ids: list[str] | None = None,
|
||||
) -> list[str]:
|
||||
kind_str = kind.value if isinstance(kind, WorkstreamKind) else str(kind)
|
||||
excluded = set(exclude_ws_ids)
|
||||
live_set = set(live_node_ids) if live_node_ids else set()
|
||||
now = self._now_iso()
|
||||
closed: list[str] = []
|
||||
with self.lock:
|
||||
for ws_id, row in self.rows.items():
|
||||
if (
|
||||
row.kind == kind_str
|
||||
and row.state in BULK_CLOSE_STATE_VALUES
|
||||
and row.updated < cutoff
|
||||
and ws_id not in excluded
|
||||
):
|
||||
# Liveness gate: when live_node_ids was provided AND
|
||||
# non-empty, protect rows whose owner is in the live
|
||||
# set. NULL node_id is always eligible. When
|
||||
# live_node_ids is None or empty, no protection
|
||||
# (mirror of the real backends).
|
||||
if live_node_ids and row.node_id is not None and row.node_id in live_set:
|
||||
continue
|
||||
row.state = "closed"
|
||||
row.updated = now
|
||||
self.state_updates.append((ws_id, "closed"))
|
||||
closed.append(ws_id)
|
||||
return closed
|
||||
|
||||
def list_services(self, service_type: str, max_age_seconds: int = 120) -> list[dict[str, str]]:
|
||||
if self.list_services_raises:
|
||||
raise RuntimeError("list_services forced failure")
|
||||
with self.lock:
|
||||
return [
|
||||
{"service_id": sid, "service_type": service_type}
|
||||
for sid in self.live_services.get(service_type, [])
|
||||
]
|
||||
|
||||
def get_workstream(self, ws_id: str) -> dict[str, Any] | None:
|
||||
with self.lock:
|
||||
@@ -228,6 +301,7 @@ def _make_manager(
|
||||
max_active: int = 5,
|
||||
storage: FakeStorage | None = None,
|
||||
event_emitter: Any = _EMITTER_DEFAULT,
|
||||
node_id: str | None = None,
|
||||
) -> tuple[SessionManager, FakeAdapter, FakeStorage]:
|
||||
"""Build a SessionManager wired to a FakeAdapter for both Protocols.
|
||||
|
||||
@@ -246,6 +320,7 @@ def _make_manager(
|
||||
storage=storage,
|
||||
max_active=max_active,
|
||||
event_emitter=emitter,
|
||||
node_id=node_id,
|
||||
)
|
||||
return mgr, adapter, storage
|
||||
|
||||
@@ -579,6 +654,24 @@ def test_open_resurrects_closed_state() -> None:
|
||||
assert ws_id in [e.ws_id for e in adapter.events_of("rehydrated")]
|
||||
|
||||
|
||||
def test_open_touches_workstream_on_rehydrate() -> None:
|
||||
"""Rehydrating a workstream must bump its ``updated`` so a concurrent
|
||||
close_idle pass-2 in this same process can't clobber the freshly-loaded
|
||||
row to ``closed`` because its DB ``updated`` is older than the cutoff.
|
||||
The touch is best-effort (try/except in open()) but must fire on the
|
||||
happy path."""
|
||||
mgr, _, storage = _make_manager()
|
||||
ws = mgr.create(user_id="u1")
|
||||
ws_id = ws.id
|
||||
mgr.close(ws_id)
|
||||
storage.touch_calls.clear() # only care about touches from rehydrate
|
||||
|
||||
reopened = mgr.open(ws_id)
|
||||
|
||||
assert reopened is not None
|
||||
assert ws_id in storage.touch_calls
|
||||
|
||||
|
||||
def test_open_ignores_owner_mismatch() -> None:
|
||||
# Turnstone is a trusted-team tool; row-level ownership is
|
||||
# metadata, not an access boundary. ``open`` no longer cares
|
||||
@@ -827,6 +920,197 @@ def test_close_idle_on_empty_manager_returns_empty_list() -> None:
|
||||
assert mgr.close_idle(max_age_seconds=1.0) == []
|
||||
|
||||
|
||||
def test_close_idle_runs_db_orphan_pass() -> None:
|
||||
"""DB rows of this kind that aren't loaded into the manager get
|
||||
bulk-closed when their ``updated`` is older than the cutoff. Catches
|
||||
the orphan-after-process-restart case the original close_idle missed."""
|
||||
mgr, _, storage = _make_manager()
|
||||
# Orphan rows live in storage but were never loaded via mgr.create.
|
||||
storage.register_workstream(
|
||||
"orphan-1",
|
||||
kind=WorkstreamKind.INTERACTIVE,
|
||||
updated="2020-01-01T00:00:00",
|
||||
)
|
||||
storage.register_workstream(
|
||||
"orphan-2",
|
||||
kind=WorkstreamKind.INTERACTIVE,
|
||||
state="thinking",
|
||||
updated="2020-01-01T00:00:00",
|
||||
)
|
||||
|
||||
closed = mgr.close_idle(max_age_seconds=0.0)
|
||||
|
||||
assert set(closed) == {"orphan-1", "orphan-2"}
|
||||
assert ("orphan-1", "closed") in storage.state_updates
|
||||
assert ("orphan-2", "closed") in storage.state_updates
|
||||
assert storage.rows["orphan-1"].state == "closed"
|
||||
assert storage.rows["orphan-2"].state == "closed"
|
||||
|
||||
|
||||
def test_close_idle_excludes_loaded_workstreams_from_db_pass() -> None:
|
||||
"""A workstream loaded into memory must NOT be reaped by the DB
|
||||
orphan pass even when its storage ``updated`` is stale — the
|
||||
in-memory pass owns those. Verifies the exclude_ws_ids plumbing."""
|
||||
mgr, _, storage = _make_manager()
|
||||
ws = mgr.create(user_id="u1")
|
||||
# Force the storage row's ``updated`` to look stale. In practice
|
||||
# ``set_state`` would bump it, but we're simulating a long-running
|
||||
# active workstream whose updated drifted older than the cutoff.
|
||||
storage.rows[ws.id].updated = "2020-01-01T00:00:00"
|
||||
|
||||
# Huge timeout so the in-memory IDLE pass skips it (stays loaded).
|
||||
closed = mgr.close_idle(max_age_seconds=10_000.0)
|
||||
|
||||
assert ws.id not in closed
|
||||
assert mgr.get(ws.id) is not None
|
||||
assert storage.rows[ws.id].state == "idle"
|
||||
|
||||
|
||||
def test_close_idle_filters_db_orphans_by_kind() -> None:
|
||||
"""An interactive manager's close_idle must not touch coordinator
|
||||
rows in storage and vice versa. Without this filter, both managers
|
||||
would race to close each other's rows."""
|
||||
mgr, _, storage = _make_manager() # interactive by default
|
||||
storage.register_workstream(
|
||||
"coord-orphan",
|
||||
kind=WorkstreamKind.COORDINATOR,
|
||||
updated="2020-01-01T00:00:00",
|
||||
)
|
||||
storage.register_workstream(
|
||||
"interactive-orphan",
|
||||
kind=WorkstreamKind.INTERACTIVE,
|
||||
updated="2020-01-01T00:00:00",
|
||||
)
|
||||
|
||||
closed = mgr.close_idle(max_age_seconds=0.0)
|
||||
|
||||
assert "interactive-orphan" in closed
|
||||
assert "coord-orphan" not in closed
|
||||
assert storage.rows["coord-orphan"].state == "idle"
|
||||
assert storage.rows["interactive-orphan"].state == "closed"
|
||||
|
||||
|
||||
def test_close_idle_protects_rows_owned_by_live_services() -> None:
|
||||
"""Multi-node correctness: rows whose ``node_id`` matches a service
|
||||
with a recent heartbeat must NOT be reaped, even when *this* manager
|
||||
is on a different node — the alive peer may legitimately have them
|
||||
loaded. Liveness is the rendezvous router's primitive (post-PR-#384);
|
||||
using it here keeps reap scoping aligned with routing.
|
||||
|
||||
Default ``_make_manager`` uses an INTERACTIVE adapter, which derives
|
||||
``service_type='server'`` — so live_services seeded under "server"
|
||||
are what the manager queries."""
|
||||
mgr, _, storage = _make_manager()
|
||||
storage.live_services["server"] = ["node-b"] # only node-b is alive
|
||||
storage.register_workstream(
|
||||
"ours-from-dead-node",
|
||||
node_id="node-a", # dead pod (not in live_services)
|
||||
kind=WorkstreamKind.INTERACTIVE,
|
||||
updated="2020-01-01T00:00:00",
|
||||
)
|
||||
storage.register_workstream(
|
||||
"theirs-still-alive",
|
||||
node_id="node-b",
|
||||
kind=WorkstreamKind.INTERACTIVE,
|
||||
updated="2020-01-01T00:00:00",
|
||||
)
|
||||
|
||||
closed = mgr.close_idle(max_age_seconds=0.0)
|
||||
|
||||
assert closed == ["ours-from-dead-node"]
|
||||
assert storage.rows["ours-from-dead-node"].state == "closed"
|
||||
assert storage.rows["theirs-still-alive"].state == "idle"
|
||||
|
||||
|
||||
def test_close_idle_protects_live_services_for_coordinator_kind() -> None:
|
||||
"""Coord-side parity: a coordinator manager derives
|
||||
``service_type='console'``, so live_services seeded under "console"
|
||||
are what gets queried. Mirrors the interactive test to ensure both
|
||||
halves of the production wiring are exercised."""
|
||||
coord_adapter = FakeAdapter(kind=WorkstreamKind.COORDINATOR)
|
||||
mgr, _, storage = _make_manager(coord_adapter)
|
||||
storage.live_services["console"] = ["console"] # console is alive
|
||||
storage.register_workstream(
|
||||
"alive-console-coord",
|
||||
node_id="console",
|
||||
kind=WorkstreamKind.COORDINATOR,
|
||||
updated="2020-01-01T00:00:00",
|
||||
)
|
||||
storage.register_workstream(
|
||||
"dead-console-coord",
|
||||
node_id="dead-console-instance", # not in live set
|
||||
kind=WorkstreamKind.COORDINATOR,
|
||||
updated="2020-01-01T00:00:00",
|
||||
)
|
||||
|
||||
closed = mgr.close_idle(max_age_seconds=0.0)
|
||||
|
||||
assert closed == ["dead-console-coord"]
|
||||
assert storage.rows["alive-console-coord"].state == "idle"
|
||||
assert storage.rows["dead-console-coord"].state == "closed"
|
||||
|
||||
|
||||
def test_close_idle_reaps_rows_with_null_node_id() -> None:
|
||||
"""A row with no ``node_id`` has no owner identity — age alone gates
|
||||
the reap. Defends against a NULL silently propagating through ``NOT
|
||||
IN (live)`` and protecting orphans forever."""
|
||||
mgr, _, storage = _make_manager()
|
||||
storage.live_services["server"] = ["node-a"]
|
||||
storage.register_workstream(
|
||||
"no-owner",
|
||||
node_id=None,
|
||||
kind=WorkstreamKind.INTERACTIVE,
|
||||
updated="2020-01-01T00:00:00",
|
||||
)
|
||||
|
||||
closed = mgr.close_idle(max_age_seconds=0.0)
|
||||
|
||||
assert closed == ["no-owner"]
|
||||
|
||||
|
||||
def test_close_idle_reaps_all_orphans_when_no_peers_alive() -> None:
|
||||
"""When ``list_services`` returns an empty list (no heartbeating
|
||||
peers), every stale orphan is unprotected and gets reaped. This is
|
||||
the cold-start / single-process / dead-cluster-recovery case."""
|
||||
mgr, _, storage = _make_manager()
|
||||
# storage.live_services["server"] left empty — no peers heartbeating
|
||||
storage.register_workstream(
|
||||
"any-node-1",
|
||||
node_id="node-a",
|
||||
kind=WorkstreamKind.INTERACTIVE,
|
||||
updated="2020-01-01T00:00:00",
|
||||
)
|
||||
storage.register_workstream(
|
||||
"any-node-2",
|
||||
node_id="node-b",
|
||||
kind=WorkstreamKind.INTERACTIVE,
|
||||
updated="2020-01-01T00:00:00",
|
||||
)
|
||||
|
||||
closed = mgr.close_idle(max_age_seconds=0.0)
|
||||
|
||||
assert set(closed) == {"any-node-1", "any-node-2"}
|
||||
|
||||
|
||||
def test_close_idle_skips_pass_2_when_list_services_fails() -> None:
|
||||
"""Conservative fallback: if list_services fails we can't enumerate
|
||||
live owners safely, so pass 2 must skip rather than reap blind. Pass
|
||||
1 (in-memory IDLE) still runs."""
|
||||
mgr, _, storage = _make_manager()
|
||||
storage.list_services_raises = True
|
||||
storage.register_workstream(
|
||||
"would-be-orphan",
|
||||
node_id="node-a",
|
||||
kind=WorkstreamKind.INTERACTIVE,
|
||||
updated="2020-01-01T00:00:00",
|
||||
)
|
||||
|
||||
closed = mgr.close_idle(max_age_seconds=0.0)
|
||||
|
||||
assert closed == []
|
||||
assert storage.rows["would-be-orphan"].state == "idle"
|
||||
|
||||
|
||||
def test_list_all_returns_creation_order() -> None:
|
||||
mgr, _, _ = _make_manager()
|
||||
a = mgr.create(user_id="u1")
|
||||
|
||||
@@ -4,6 +4,10 @@ from __future__ import annotations
|
||||
|
||||
from typing import Any
|
||||
|
||||
import sqlalchemy as sa
|
||||
|
||||
from turnstone.core.storage._schema import workstreams
|
||||
|
||||
# -- Workstream registration ---------------------------------------------------
|
||||
|
||||
|
||||
@@ -664,6 +668,277 @@ class TestBatchPrimitives:
|
||||
assert result == {"never-seen": 0}
|
||||
|
||||
|
||||
# -- bulk_close_stale_orphans --------------------------------------------------
|
||||
|
||||
|
||||
def _force_updated(backend: Any, ws_id: str, updated: str) -> None:
|
||||
"""Stamp a workstream row's ``updated`` column directly.
|
||||
|
||||
The public surface only sets ``updated`` to ``now``, which makes it
|
||||
impossible to fabricate a stale row through register/update calls.
|
||||
Reaches into ``backend._engine`` — same access pattern conftest uses
|
||||
for cross-backend cleanup.
|
||||
"""
|
||||
with backend._engine.connect() as conn:
|
||||
conn.execute(
|
||||
sa.update(workstreams).where(workstreams.c.ws_id == ws_id).values(updated=updated)
|
||||
)
|
||||
conn.commit()
|
||||
|
||||
|
||||
class TestBulkCloseStaleOrphans:
|
||||
def test_closes_stale_non_terminal_rows_of_kind(self, backend):
|
||||
backend.register_workstream("stale-idle", kind="interactive")
|
||||
backend.register_workstream("stale-thinking", kind="interactive")
|
||||
backend.update_workstream_state("stale-thinking", "thinking")
|
||||
backend.register_workstream("fresh-idle", kind="interactive")
|
||||
_force_updated(backend, "stale-idle", "2020-01-01T00:00:00")
|
||||
_force_updated(backend, "stale-thinking", "2020-01-01T00:00:00")
|
||||
# fresh-idle stays at registration time (effectively now)
|
||||
|
||||
closed = backend.bulk_close_stale_orphans(
|
||||
"interactive", cutoff="2024-01-01T00:00:00", exclude_ws_ids=[]
|
||||
)
|
||||
|
||||
assert set(closed) == {"stale-idle", "stale-thinking"}
|
||||
rows = backend.get_workstreams_batch(["stale-idle", "stale-thinking", "fresh-idle"])
|
||||
assert rows["stale-idle"]["state"] == "closed"
|
||||
assert rows["stale-thinking"]["state"] == "closed"
|
||||
assert rows["fresh-idle"]["state"] == "idle"
|
||||
|
||||
def test_skips_already_closed(self, backend):
|
||||
backend.register_workstream("already-closed", kind="interactive")
|
||||
backend.update_workstream_state("already-closed", "closed")
|
||||
_force_updated(backend, "already-closed", "2020-01-01T00:00:00")
|
||||
|
||||
closed = backend.bulk_close_stale_orphans(
|
||||
"interactive", cutoff="2024-01-01T00:00:00", exclude_ws_ids=[]
|
||||
)
|
||||
|
||||
assert closed == []
|
||||
|
||||
def test_filters_by_kind(self, backend):
|
||||
backend.register_workstream("interactive-stale", kind="interactive")
|
||||
backend.register_workstream("coord-stale", kind="coordinator")
|
||||
_force_updated(backend, "interactive-stale", "2020-01-01T00:00:00")
|
||||
_force_updated(backend, "coord-stale", "2020-01-01T00:00:00")
|
||||
|
||||
closed = backend.bulk_close_stale_orphans(
|
||||
"interactive", cutoff="2024-01-01T00:00:00", exclude_ws_ids=[]
|
||||
)
|
||||
|
||||
assert closed == ["interactive-stale"]
|
||||
rows = backend.get_workstreams_batch(["interactive-stale", "coord-stale"])
|
||||
assert rows["interactive-stale"]["state"] == "closed"
|
||||
assert rows["coord-stale"]["state"] == "idle"
|
||||
|
||||
def test_excludes_loaded_ws_ids(self, backend):
|
||||
backend.register_workstream("ws-keep", kind="interactive")
|
||||
backend.register_workstream("ws-close", kind="interactive")
|
||||
_force_updated(backend, "ws-keep", "2020-01-01T00:00:00")
|
||||
_force_updated(backend, "ws-close", "2020-01-01T00:00:00")
|
||||
|
||||
closed = backend.bulk_close_stale_orphans(
|
||||
"interactive", cutoff="2024-01-01T00:00:00", exclude_ws_ids=["ws-keep"]
|
||||
)
|
||||
|
||||
assert closed == ["ws-close"]
|
||||
rows = backend.get_workstreams_batch(["ws-keep", "ws-close"])
|
||||
assert rows["ws-keep"]["state"] == "idle"
|
||||
assert rows["ws-close"]["state"] == "closed"
|
||||
|
||||
def test_empty_exclude_list_does_not_break_sql(self, backend):
|
||||
backend.register_workstream("orphan", kind="interactive")
|
||||
_force_updated(backend, "orphan", "2020-01-01T00:00:00")
|
||||
|
||||
closed = backend.bulk_close_stale_orphans(
|
||||
"interactive", cutoff="2024-01-01T00:00:00", exclude_ws_ids=[]
|
||||
)
|
||||
|
||||
assert closed == ["orphan"]
|
||||
|
||||
def test_no_orphans_returns_empty(self, backend):
|
||||
backend.register_workstream("fresh", kind="interactive")
|
||||
|
||||
closed = backend.bulk_close_stale_orphans(
|
||||
"interactive", cutoff="2024-01-01T00:00:00", exclude_ws_ids=[]
|
||||
)
|
||||
|
||||
assert closed == []
|
||||
|
||||
def test_closes_all_non_terminal_states(self, backend):
|
||||
for ws_id, state in [
|
||||
("o-idle", "idle"),
|
||||
("o-thinking", "thinking"),
|
||||
("o-attention", "attention"),
|
||||
("o-running", "running"),
|
||||
]:
|
||||
backend.register_workstream(ws_id, kind="interactive")
|
||||
if state != "idle":
|
||||
backend.update_workstream_state(ws_id, state)
|
||||
_force_updated(backend, ws_id, "2020-01-01T00:00:00")
|
||||
|
||||
closed = backend.bulk_close_stale_orphans(
|
||||
"interactive", cutoff="2024-01-01T00:00:00", exclude_ws_ids=[]
|
||||
)
|
||||
|
||||
assert set(closed) == {"o-idle", "o-thinking", "o-attention", "o-running"}
|
||||
|
||||
def test_bumps_updated_on_close(self, backend):
|
||||
stale_updated = "2020-01-01T00:00:00"
|
||||
backend.register_workstream("orphan", kind="interactive")
|
||||
_force_updated(backend, "orphan", stale_updated)
|
||||
|
||||
backend.bulk_close_stale_orphans(
|
||||
"interactive", cutoff="2024-01-01T00:00:00", exclude_ws_ids=[]
|
||||
)
|
||||
|
||||
# ``updated`` must change away from the forced stale value. Asserting
|
||||
# inequality from the seed (rather than ``> "2024-01-01..."``) keeps
|
||||
# the test independent of wall-clock date.
|
||||
with backend._engine.connect() as conn:
|
||||
row = conn.execute(
|
||||
sa.select(workstreams.c.updated).where(workstreams.c.ws_id == "orphan")
|
||||
).one()
|
||||
assert row[0] != stale_updated
|
||||
|
||||
def test_protects_rows_owned_by_live_services(self, backend):
|
||||
"""Liveness scoping (post-#384 rendezvous-routing world): rows
|
||||
whose ``node_id`` matches a heartbeating service must NOT be
|
||||
reaped, because that owner may legitimately have them loaded on
|
||||
another worker. Rows whose ``node_id`` matches a dead service
|
||||
ARE eligible — that's how dead-pod orphans get reclaimed in
|
||||
containerized deployments with dynamic hostnames."""
|
||||
backend.register_workstream("dead-node", node_id="dead-pod-x4k2", kind="interactive")
|
||||
backend.register_workstream("alive-node", node_id="alive-pod-y9p3", kind="interactive")
|
||||
_force_updated(backend, "dead-node", "2020-01-01T00:00:00")
|
||||
_force_updated(backend, "alive-node", "2020-01-01T00:00:00")
|
||||
|
||||
closed = backend.bulk_close_stale_orphans(
|
||||
"interactive",
|
||||
cutoff="2024-01-01T00:00:00",
|
||||
exclude_ws_ids=[],
|
||||
live_node_ids=["alive-pod-y9p3"],
|
||||
)
|
||||
|
||||
assert closed == ["dead-node"]
|
||||
rows = backend.get_workstreams_batch(["dead-node", "alive-node"])
|
||||
assert rows["dead-node"]["state"] == "closed"
|
||||
assert rows["alive-node"]["state"] == "idle"
|
||||
|
||||
def test_null_node_id_always_eligible(self, backend):
|
||||
"""A row with NULL ``node_id`` has no owner identity — age alone
|
||||
gates the reap. Belt-and-suspenders against ``NULL NOT IN (...)``
|
||||
evaluating to NULL (not TRUE) and silently protecting orphans
|
||||
forever."""
|
||||
backend.register_workstream("no-owner", node_id=None, kind="interactive")
|
||||
_force_updated(backend, "no-owner", "2020-01-01T00:00:00")
|
||||
|
||||
closed = backend.bulk_close_stale_orphans(
|
||||
"interactive",
|
||||
cutoff="2024-01-01T00:00:00",
|
||||
exclude_ws_ids=[],
|
||||
live_node_ids=["some-other-node"],
|
||||
)
|
||||
|
||||
assert closed == ["no-owner"]
|
||||
|
||||
def test_live_node_ids_none_skips_filter(self, backend):
|
||||
"""``live_node_ids=None`` is the single-process / operator-backfill
|
||||
mode — all rows of *kind* are eligible regardless of node_id."""
|
||||
backend.register_workstream("node-a", node_id="node-a", kind="interactive")
|
||||
backend.register_workstream("node-b", node_id="node-b", kind="interactive")
|
||||
_force_updated(backend, "node-a", "2020-01-01T00:00:00")
|
||||
_force_updated(backend, "node-b", "2020-01-01T00:00:00")
|
||||
|
||||
closed = backend.bulk_close_stale_orphans(
|
||||
"interactive", cutoff="2024-01-01T00:00:00", exclude_ws_ids=[]
|
||||
)
|
||||
|
||||
assert set(closed) == {"node-a", "node-b"}
|
||||
|
||||
def test_empty_live_node_ids_treats_all_as_dead(self, backend):
|
||||
"""Empty list ``live_node_ids=[]`` means "no nodes alive" — every
|
||||
row's owner is unprotected. Useful for operator scripts that
|
||||
want to reap regardless of liveness."""
|
||||
backend.register_workstream("any", node_id="node-a", kind="interactive")
|
||||
_force_updated(backend, "any", "2020-01-01T00:00:00")
|
||||
|
||||
closed = backend.bulk_close_stale_orphans(
|
||||
"interactive",
|
||||
cutoff="2024-01-01T00:00:00",
|
||||
exclude_ws_ids=[],
|
||||
live_node_ids=[],
|
||||
)
|
||||
|
||||
assert closed == ["any"]
|
||||
|
||||
def test_combines_live_node_ids_and_exclude_ws_ids(self, backend):
|
||||
"""Both filters stack as AND clauses on the UPDATE. Covers the
|
||||
full 2x2 matrix to catch a future edit that replaces an AND with
|
||||
an OR or drops one of the filters: only the (orphan + dead-node)
|
||||
cell should be reaped."""
|
||||
# All four registered with the same stale ``updated``.
|
||||
for ws_id, node in [
|
||||
("loaded-alive", "alive-node"),
|
||||
("loaded-dead", "dead-node"),
|
||||
("orphan-alive", "alive-node"),
|
||||
("orphan-dead", "dead-node"),
|
||||
]:
|
||||
backend.register_workstream(ws_id, node_id=node, kind="interactive")
|
||||
_force_updated(backend, ws_id, "2020-01-01T00:00:00")
|
||||
|
||||
closed = backend.bulk_close_stale_orphans(
|
||||
"interactive",
|
||||
cutoff="2024-01-01T00:00:00",
|
||||
exclude_ws_ids=["loaded-alive", "loaded-dead"],
|
||||
live_node_ids=["alive-node"],
|
||||
)
|
||||
|
||||
# Only orphan-dead is unprotected by both filters.
|
||||
assert closed == ["orphan-dead"]
|
||||
rows = backend.get_workstreams_batch(
|
||||
["loaded-alive", "loaded-dead", "orphan-alive", "orphan-dead"]
|
||||
)
|
||||
assert rows["loaded-alive"]["state"] == "idle"
|
||||
assert rows["loaded-dead"]["state"] == "idle"
|
||||
assert rows["orphan-alive"]["state"] == "idle"
|
||||
assert rows["orphan-dead"]["state"] == "closed"
|
||||
|
||||
|
||||
# -- touch_workstream ----------------------------------------------------------
|
||||
|
||||
|
||||
class TestTouchWorkstream:
|
||||
def test_bumps_updated_only(self, backend):
|
||||
"""Used by ``open()`` on rehydrate to defend against the orphan
|
||||
reaper clobbering a freshly-loaded row. Must not change ``state``
|
||||
(the open() path explicitly avoids state writes to dodge a race
|
||||
with concurrent close())."""
|
||||
stale_updated = "2020-01-01T00:00:00"
|
||||
backend.register_workstream("ws-touch", kind="interactive")
|
||||
backend.update_workstream_state("ws-touch", "closed") # simulate prior close
|
||||
_force_updated(backend, "ws-touch", stale_updated)
|
||||
|
||||
backend.touch_workstream("ws-touch")
|
||||
|
||||
with backend._engine.connect() as conn:
|
||||
row = conn.execute(
|
||||
sa.select(workstreams.c.state, workstreams.c.updated).where(
|
||||
workstreams.c.ws_id == "ws-touch"
|
||||
)
|
||||
).one()
|
||||
assert row[0] == "closed", "state must not be modified by touch"
|
||||
# Compare against the forced stale value rather than a fixed calendar
|
||||
# date so the test is independent of wall-clock time.
|
||||
assert row[1] != stale_updated, "updated must be bumped"
|
||||
|
||||
def test_unknown_id_is_noop(self, backend):
|
||||
"""Touch on a missing id must not raise — open()'s exception
|
||||
handler is best-effort."""
|
||||
backend.touch_workstream("nonexistent") # must not raise
|
||||
|
||||
|
||||
# -- Lifecycle -----------------------------------------------------------------
|
||||
|
||||
|
||||
|
||||
@@ -155,3 +155,87 @@ class TestContentAccumulation:
|
||||
assert len(idle_events) == 1
|
||||
# Content should be capped, not contain everything
|
||||
assert len(idle_events[0]["content"]) <= _MAX_TURN_CONTENT_CHARS + 1024
|
||||
|
||||
|
||||
class TestPendingApprovalDetailGate:
|
||||
"""The Shape A SSE plumbing carries ``pending_approval_detail`` on the
|
||||
``ws_state`` event so the coord tree UI can render inline approve/deny
|
||||
buttons in lockstep with the activity_state transition. The gate
|
||||
(``if self._pending_approval is not None``) keeps the per-broadcast
|
||||
serializer cost off the common no-approval-pending path — these tests
|
||||
lock both branches down."""
|
||||
|
||||
def test_state_broadcast_omits_field_when_no_approval_pending(self):
|
||||
"""Common case: no approval pending → field absent from event so the
|
||||
per-broadcast verdict-cache deepcopy in
|
||||
``serialize_pending_approval_detail`` never runs. A regression
|
||||
that drops the gate would silently 10x the cost of every state
|
||||
broadcast in the steady state."""
|
||||
ui = _make_ui()
|
||||
assert ui._pending_approval is None
|
||||
ui._broadcast_state("running")
|
||||
|
||||
events = _drain_global()
|
||||
running_events = [e for e in events if e.get("state") == "running"]
|
||||
assert len(running_events) == 1
|
||||
assert "pending_approval_detail" not in running_events[0]
|
||||
|
||||
def test_state_broadcast_includes_field_when_approval_pending(self):
|
||||
"""When an approval is pending the broadcast must carry the rich
|
||||
payload — the coord tree UI reads it directly to render inline
|
||||
approve/deny buttons. Without this, a coord browser would have
|
||||
to chase a separate ``cluster/ws/live`` fetch on every
|
||||
activity_state transition (the load-storm pattern Shape A is
|
||||
unwinding)."""
|
||||
ui = _make_ui()
|
||||
# Mirror the shape ``pause_for_approval`` writes (session_ui_base
|
||||
# lines 576-580) — items with call_id + header is the minimum
|
||||
# the serializer needs to project.
|
||||
ui._pending_approval = {
|
||||
"type": "approve_request",
|
||||
"items": [
|
||||
{
|
||||
"call_id": "c1",
|
||||
"header": "tool x",
|
||||
"func_args": "{}",
|
||||
"intent_summary": "do x",
|
||||
"needs_approval": True,
|
||||
}
|
||||
],
|
||||
"judge_pending": False,
|
||||
}
|
||||
ui._broadcast_state("attention")
|
||||
|
||||
events = _drain_global()
|
||||
attn = [e for e in events if e.get("state") == "attention"]
|
||||
assert len(attn) == 1
|
||||
# Field present and structurally sound — the serializer's
|
||||
# full shape is covered by tests/test_session_ui_base.py;
|
||||
# here we only need to confirm the gate fires and the
|
||||
# serializer's output is what lands on the event.
|
||||
assert "pending_approval_detail" in attn[0]
|
||||
detail = attn[0]["pending_approval_detail"]
|
||||
assert detail is not None
|
||||
assert detail.get("items")
|
||||
assert detail["items"][0]["call_id"] == "c1"
|
||||
|
||||
def test_field_cleared_after_approval_resolves(self):
|
||||
"""Once ``_pending_approval`` is cleared, subsequent state
|
||||
broadcasts must drop the field again — without this, the
|
||||
browser would render stale approve/deny buttons until the
|
||||
next bulk-poll TTL window expired."""
|
||||
ui = _make_ui()
|
||||
ui._pending_approval = {
|
||||
"type": "approve_request",
|
||||
"items": [{"call_id": "c1", "header": "x"}],
|
||||
"judge_pending": False,
|
||||
}
|
||||
ui._broadcast_state("attention")
|
||||
_drain_global() # discard the with-detail event
|
||||
|
||||
ui._pending_approval = None
|
||||
ui._broadcast_state("running")
|
||||
events = _drain_global()
|
||||
running = [e for e in events if e.get("state") == "running"]
|
||||
assert len(running) == 1
|
||||
assert "pending_approval_detail" not in running[0]
|
||||
|
||||
@@ -899,6 +899,138 @@ class TestHistoryInteractive:
|
||||
assert client.get(base, params={"limit": 999}).status_code == 200
|
||||
|
||||
|
||||
class TestBuildHistoryReminderPropagation:
|
||||
"""``_build_history`` must surface the ``_reminders`` side-channel on
|
||||
each entry so a tab reconnecting via ``/history`` renders the same
|
||||
metacognitive nudge bubble the originating tab saw via the live
|
||||
``user_reminder`` SSE event.
|
||||
"""
|
||||
|
||||
def _session_with_messages(self, messages: list[dict]) -> MagicMock:
|
||||
session = MagicMock()
|
||||
session.messages = messages
|
||||
return session
|
||||
|
||||
def test_reminders_sidechannel_surfaces_on_entry(self):
|
||||
from turnstone.server import _build_history
|
||||
|
||||
session = self._session_with_messages(
|
||||
[
|
||||
{
|
||||
"role": "user",
|
||||
"content": "ah no",
|
||||
"_reminders": [{"type": "correction", "text": "watch out"}],
|
||||
}
|
||||
]
|
||||
)
|
||||
history = _build_history(session)
|
||||
assert history[0]["content"] == "ah no"
|
||||
assert history[0]["reminders"] == [{"type": "correction", "text": "watch out"}]
|
||||
|
||||
def test_no_reminders_key_when_sidechannel_absent(self):
|
||||
from turnstone.server import _build_history
|
||||
|
||||
session = self._session_with_messages([{"role": "user", "content": "just a message"}])
|
||||
history = _build_history(session)
|
||||
assert "reminders" not in history[0]
|
||||
|
||||
def test_no_reminders_key_when_sidechannel_empty(self):
|
||||
from turnstone.server import _build_history
|
||||
|
||||
session = self._session_with_messages([{"role": "user", "content": "hi", "_reminders": []}])
|
||||
history = _build_history(session)
|
||||
assert "reminders" not in history[0]
|
||||
|
||||
def test_multiple_reminders_preserved_in_order(self):
|
||||
from turnstone.server import _build_history
|
||||
|
||||
session = self._session_with_messages(
|
||||
[
|
||||
{
|
||||
"role": "user",
|
||||
"content": "x",
|
||||
"_reminders": [
|
||||
{"type": "denial", "text": "FIRST"},
|
||||
{"type": "correction", "text": "SECOND"},
|
||||
],
|
||||
}
|
||||
]
|
||||
)
|
||||
history = _build_history(session)
|
||||
assert history[0]["reminders"] == [
|
||||
{"type": "denial", "text": "FIRST"},
|
||||
{"type": "correction", "text": "SECOND"},
|
||||
]
|
||||
|
||||
def test_reminders_coexist_with_attachments(self):
|
||||
from turnstone.server import _build_history
|
||||
|
||||
session = self._session_with_messages(
|
||||
[
|
||||
{
|
||||
"role": "user",
|
||||
"content": [
|
||||
{"type": "text", "text": "look"},
|
||||
{"type": "image_url", "image_url": {"url": "data:..."}},
|
||||
],
|
||||
"_reminders": [{"type": "correction", "text": "watch"}],
|
||||
}
|
||||
]
|
||||
)
|
||||
history = _build_history(session)
|
||||
assert history[0]["content"] == "look"
|
||||
assert history[0]["attachments"] == [{"kind": "image", "filename": "", "mime_type": ""}]
|
||||
assert history[0]["reminders"] == [{"type": "correction", "text": "watch"}]
|
||||
|
||||
def test_malformed_reminders_filtered_out(self):
|
||||
"""Defensive: a non-dict element in the list (corruption / bug)
|
||||
is dropped rather than crashing the history serialisation."""
|
||||
from turnstone.server import _build_history
|
||||
|
||||
session = self._session_with_messages(
|
||||
[
|
||||
{
|
||||
"role": "user",
|
||||
"content": "x",
|
||||
"_reminders": [
|
||||
{"type": "correction", "text": "ok"},
|
||||
"not-a-dict",
|
||||
{"type": "denial"}, # missing text
|
||||
],
|
||||
}
|
||||
]
|
||||
)
|
||||
history = _build_history(session)
|
||||
# Non-dicts dropped; missing-text fills with empty string.
|
||||
assert history[0]["reminders"] == [
|
||||
{"type": "correction", "text": "ok"},
|
||||
{"type": "denial", "text": ""},
|
||||
]
|
||||
|
||||
def test_clean_message_passes_through_unchanged(self):
|
||||
"""No reminders, plain content — _build_history is a no-op for the
|
||||
reminder field and ``content`` rides through verbatim."""
|
||||
from turnstone.server import _build_history
|
||||
|
||||
session = self._session_with_messages(
|
||||
[{"role": "user", "content": "just a normal message"}]
|
||||
)
|
||||
history = _build_history(session)
|
||||
assert history[0]["content"] == "just a normal message"
|
||||
assert "reminders" not in history[0]
|
||||
|
||||
def test_assistant_content_with_literal_reminder_tag_unchanged(self):
|
||||
"""Assistant output may legitimately reference the tag (e.g. when
|
||||
the model is explaining the reminder system itself). No
|
||||
transformation should ever apply to assistant content."""
|
||||
from turnstone.server import _build_history
|
||||
|
||||
content = "Here is a <system-reminder> tag in assistant output."
|
||||
session = self._session_with_messages([{"role": "assistant", "content": content}])
|
||||
history = _build_history(session)
|
||||
assert history[0]["content"] == content
|
||||
|
||||
|
||||
class TestDetailInteractive:
|
||||
"""Interactive parity for the lifted ``GET /v1/api/workstreams/{ws_id}``.
|
||||
|
||||
|
||||
@@ -1,3 +1,3 @@
|
||||
"""turnstone - Multi-node AI orchestration platform with tool use, agent routing, and cluster simulation."""
|
||||
|
||||
__version__ = "1.5.0"
|
||||
__version__ = "1.5.3"
|
||||
|
||||
@@ -312,6 +312,29 @@ class TerminalUI(SessionUI):
|
||||
sys.stdout.write(f"{RED}{message}{RESET}\n")
|
||||
sys.stdout.flush()
|
||||
|
||||
def _print_reminder(self, reminders: list[dict[str, str]]) -> None:
|
||||
"""Render a metacognitive reminder list as ``[metacognition · type] text``
|
||||
lines in the terminal — the CLI's equivalent of the web UI's
|
||||
yellow themed bubble. Used by both ``on_user_reminder`` and
|
||||
``on_tool_reminder``; the rendering is identical because
|
||||
terminal output is anchor-by-flow rather than DOM-by-anchor.
|
||||
"""
|
||||
for r in reminders:
|
||||
nt = str(r.get("type", "") or "")
|
||||
text = str(r.get("text", "") or "")
|
||||
label = "metacognition" + (f" · {nt}" if nt else "")
|
||||
sys.stdout.write(f"{YELLOW}[{label}]{RESET} {text}\n")
|
||||
sys.stdout.flush()
|
||||
|
||||
def on_user_reminder(self, reminders: list[dict[str, str]]) -> None:
|
||||
self._print_reminder(reminders)
|
||||
|
||||
def on_tool_reminder(self, reminders: list[dict[str, str]], tool_call_id: str) -> None:
|
||||
# tool_call_id ignored — the CLI anchors by output sequence
|
||||
# (the line lands directly after the tool result that
|
||||
# triggered the batch's reminder).
|
||||
self._print_reminder(reminders)
|
||||
|
||||
def on_state_change(self, state: str) -> None:
|
||||
pass # base TerminalUI ignores state changes
|
||||
|
||||
|
||||
@@ -498,6 +498,7 @@ class ClusterCollector:
|
||||
"kind": WorkstreamKind.from_raw(new_w.get("kind")),
|
||||
"parent_ws_id": new_w.get("parent_ws_id"),
|
||||
"activity_state": new_w.get("activity_state", ""),
|
||||
"pending_approval_detail": new_w.get("pending_approval_detail"),
|
||||
}
|
||||
)
|
||||
old_name = old_ws.get("title", "") or old_ws.get("name", "")
|
||||
@@ -557,6 +558,18 @@ class ClusterCollector:
|
||||
ws["kind"] = data["kind"]
|
||||
if "parent_ws_id" in data:
|
||||
ws["parent_ws_id"] = data["parent_ws_id"]
|
||||
# ``pending_approval_detail`` overwrites (no
|
||||
# ``ws.get`` fallback): the node's broadcast gate
|
||||
# on ``_pending_approval is not None`` means the
|
||||
# field is absent from ``data`` exactly when no
|
||||
# approval is pending — falling back to the cached
|
||||
# value would resurrect a stale detail after the
|
||||
# approval resolved. Without this assignment the
|
||||
# cached ``node.workstreams`` dict served by
|
||||
# ``get_node_detail`` / ``get_snapshot`` between
|
||||
# reconciliations would render stale approve/deny
|
||||
# buttons on closed approvals.
|
||||
ws["pending_approval_detail"] = data.get("pending_approval_detail")
|
||||
pending_events.append(
|
||||
{
|
||||
"type": "cluster_state",
|
||||
@@ -568,6 +581,7 @@ class ClusterCollector:
|
||||
"kind": WorkstreamKind.from_raw(ws.get("kind")),
|
||||
"parent_ws_id": ws.get("parent_ws_id"),
|
||||
"activity_state": ws.get("activity_state", ""),
|
||||
"pending_approval_detail": data.get("pending_approval_detail"),
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
@@ -685,11 +685,13 @@ class CoordinatorAdapter:
|
||||
"tokens": event.get("tokens", 0),
|
||||
"node_id": event.get("node_id", ""),
|
||||
# activity_state lets the JS detect approval-state
|
||||
# transitions and fire urgent live-bulk fetches so
|
||||
# inline approve/deny buttons render in lockstep
|
||||
# with the child entering attention (instead of
|
||||
# waiting up to 5s for the next TTL window).
|
||||
# transitions; pending_approval_detail rides on
|
||||
# the same event so the browser can mutate
|
||||
# liveBadgeCache directly and render inline
|
||||
# approve/deny buttons in lockstep with the
|
||||
# transition, no separate dashboard refetch.
|
||||
"activity_state": event.get("activity_state", ""),
|
||||
"pending_approval_detail": event.get("pending_approval_detail"),
|
||||
}
|
||||
elif etype == "ws_closed":
|
||||
child_event = {
|
||||
|
||||
@@ -81,13 +81,13 @@ WAIT_MAX_TIMEOUT: float = 600.0
|
||||
WAIT_POLL_INTERVAL: float = 0.5
|
||||
|
||||
# Per-ws cap on the inline ``message`` field bundled into wait_for_workstream
|
||||
# results. Sized so a fan-out of 32 children at the cap is ~192 KiB of
|
||||
# results. Sized so a fan-out of 32 children at the cap is ~320 KiB of
|
||||
# tool output — large but not catastrophic on commercial models, and
|
||||
# typical waits run with a handful of children. Truncation is from the
|
||||
# END (the lead is usually more informative than the tail) and sets a
|
||||
# ``truncated=True`` flag so the model can opt into a follow-up read if
|
||||
# the trailing bytes matter.
|
||||
WAIT_MESSAGE_MAX_BYTES: int = 6 * 1024
|
||||
WAIT_MESSAGE_MAX_BYTES: int = 10 * 1024
|
||||
|
||||
# How many tail messages ``wait_for_workstream`` reads when extracting a
|
||||
# child's last assistant turn. The conversation tail almost always
|
||||
|
||||
+199
-8
@@ -23,6 +23,7 @@ import queue
|
||||
import re
|
||||
import secrets
|
||||
import textwrap
|
||||
import threading
|
||||
import time
|
||||
import urllib.parse
|
||||
from concurrent.futures import ThreadPoolExecutor
|
||||
@@ -89,6 +90,7 @@ if TYPE_CHECKING:
|
||||
from starlette.requests import Request
|
||||
|
||||
from turnstone.core.session import ChatSession
|
||||
from turnstone.core.session_manager import SessionManager
|
||||
from turnstone.core.storage._protocol import StorageBackend
|
||||
|
||||
log = logging.getLogger("turnstone.console.server")
|
||||
@@ -2453,7 +2455,7 @@ def _require_admin_coordinator(
|
||||
)
|
||||
|
||||
|
||||
def _resolve_coordinator_or_404(
|
||||
async def _resolve_coordinator_or_404(
|
||||
request: Request,
|
||||
coord_mgr: Any,
|
||||
storage: Any,
|
||||
@@ -2484,7 +2486,11 @@ def _resolve_coordinator_or_404(
|
||||
if storage is None:
|
||||
return None, miss
|
||||
try:
|
||||
row = storage.get_workstream(ws_id)
|
||||
# Cold-cache path (every console restart, eviction, console
|
||||
# proxy hop) — offload the sync DB call so the coord
|
||||
# children/tasks handlers don't block the event loop on the
|
||||
# same DB the rest of the handler is unblocking.
|
||||
row = await asyncio.to_thread(storage.get_workstream, ws_id)
|
||||
except Exception:
|
||||
log.debug("resolve_coordinator.storage_failed ws=%s", ws_id[:8], exc_info=True)
|
||||
return None, miss
|
||||
@@ -2910,7 +2916,7 @@ async def coordinator_children(request: Request) -> JSONResponse:
|
||||
if not _VALID_WS_ID_RE.match(ws_id):
|
||||
return JSONResponse({"error": "invalid ws_id"}, status_code=400)
|
||||
user_id = _auth_user_id(request)
|
||||
_ws, err404 = _resolve_coordinator_or_404(request, coord_mgr, storage, ws_id, user_id)
|
||||
_ws, err404 = await _resolve_coordinator_or_404(request, coord_mgr, storage, ws_id, user_id)
|
||||
if err404 is not None:
|
||||
return err404
|
||||
|
||||
@@ -2918,7 +2924,8 @@ async def coordinator_children(request: Request) -> JSONResponse:
|
||||
# the full child subtree. ``user_id`` stays on each row as
|
||||
# metadata, not a filter.
|
||||
try:
|
||||
raw = storage.list_workstreams(
|
||||
raw = await asyncio.to_thread(
|
||||
storage.list_workstreams,
|
||||
limit=_CHILDREN_PAGE_LIMIT + 1,
|
||||
parent_ws_id=ws_id,
|
||||
kind=None,
|
||||
@@ -3033,7 +3040,7 @@ async def coordinator_metrics(request: Request) -> JSONResponse:
|
||||
if not _VALID_WS_ID_RE.match(ws_id):
|
||||
return JSONResponse({"error": "invalid ws_id"}, status_code=400)
|
||||
user_id = _auth_user_id(request)
|
||||
_ws, err404 = _resolve_coordinator_or_404(request, coord_mgr, storage, ws_id, user_id)
|
||||
_ws, err404 = await _resolve_coordinator_or_404(request, coord_mgr, storage, ws_id, user_id)
|
||||
if err404 is not None:
|
||||
return err404
|
||||
|
||||
@@ -3233,7 +3240,7 @@ async def _resolve_coord_session(
|
||||
if not _VALID_WS_ID_RE.match(ws_id):
|
||||
return JSONResponse({"error": "invalid ws_id"}, status_code=400)
|
||||
user_id = _auth_user_id(request)
|
||||
ws, err404 = _resolve_coordinator_or_404(request, coord_mgr, storage, ws_id, user_id)
|
||||
ws, err404 = await _resolve_coordinator_or_404(request, coord_mgr, storage, ws_id, user_id)
|
||||
if err404 is not None:
|
||||
return err404
|
||||
if ws is None or ws.session is None:
|
||||
@@ -3523,11 +3530,11 @@ async def coordinator_tasks(request: Request) -> JSONResponse:
|
||||
if not _VALID_WS_ID_RE.match(ws_id):
|
||||
return JSONResponse({"error": "invalid ws_id"}, status_code=400)
|
||||
user_id = _auth_user_id(request)
|
||||
_ws, err404 = _resolve_coordinator_or_404(request, coord_mgr, storage, ws_id, user_id)
|
||||
_ws, err404 = await _resolve_coordinator_or_404(request, coord_mgr, storage, ws_id, user_id)
|
||||
if err404 is not None:
|
||||
return err404
|
||||
|
||||
envelope, _corrupt = load_task_envelope(storage, ws_id)
|
||||
envelope, _corrupt = await asyncio.to_thread(load_task_envelope, storage, ws_id)
|
||||
return JSONResponse(envelope)
|
||||
|
||||
|
||||
@@ -3682,6 +3689,50 @@ async def _verify_collector_service_scope(app: Starlette, client: httpx.AsyncCli
|
||||
)
|
||||
|
||||
|
||||
def _coord_idle_cleanup_thread(
|
||||
mgr: SessionManager,
|
||||
timeout_sec: float,
|
||||
stop_event: threading.Event | None = None,
|
||||
) -> None:
|
||||
"""Periodically reap idle + DB-orphan coordinator workstreams.
|
||||
|
||||
Mirrors the regular server's ``_idle_cleanup_thread`` (turnstone/server.py)
|
||||
but skips the rate-limiter / global-queue arms — the console doesn't have
|
||||
those. ``mgr.close_idle`` does the work: closes loaded IDLE rows AND
|
||||
bulk-closes DB rows of this kind whose ``updated`` is past the cutoff
|
||||
and which aren't currently loaded. The latter pass catches coords left
|
||||
behind by prior console process incarnations.
|
||||
|
||||
Runs an initial sweep BEFORE the first sleep so cold-start orphans are
|
||||
reaped immediately rather than waiting one ``check_every`` interval (~30
|
||||
min on default 2h timeout). This intentionally diverges from the regular
|
||||
server pattern, which has no initial sweep — the regular server runs
|
||||
inside a normal request-handling lifecycle, the console-side coord pool
|
||||
is a small fixed-size cache where orphans dominate the row count after
|
||||
a cold boot.
|
||||
|
||||
``stop_event`` is for tests — when set, the thread exits cleanly after
|
||||
the next loop check. Production callers pass ``None`` (the daemon is
|
||||
process-lifetime).
|
||||
"""
|
||||
check_every = min(300.0, timeout_sec / 4)
|
||||
# Initial sweep — runs once before entering the sleep loop.
|
||||
try:
|
||||
mgr.close_idle(timeout_sec)
|
||||
except Exception:
|
||||
log.debug("console.coord_idle_cleanup_initial_failed", exc_info=True)
|
||||
while True:
|
||||
if stop_event is not None and stop_event.is_set():
|
||||
return
|
||||
time.sleep(check_every)
|
||||
if stop_event is not None and stop_event.is_set():
|
||||
return
|
||||
try:
|
||||
mgr.close_idle(timeout_sec)
|
||||
except Exception:
|
||||
log.debug("console.coord_idle_cleanup_failed", exc_info=True)
|
||||
|
||||
|
||||
@asynccontextmanager
|
||||
async def _lifespan(app: Starlette) -> AsyncGenerator[None, None]:
|
||||
# Create async HTTP clients for proxy routes. Auth headers are NOT baked
|
||||
@@ -3750,6 +3801,18 @@ async def _lifespan(app: Starlette) -> AsyncGenerator[None, None]:
|
||||
audit_exec = ThreadPoolExecutor(max_workers=4, thread_name_prefix="coord-audit")
|
||||
app.state.audit_executor = audit_exec
|
||||
_set_audit_executor(audit_exec)
|
||||
# Dedicated executor for coord SSE queue polling, mirroring the
|
||||
# interactive-side ``sse_executor`` in ``turnstone/server.py``.
|
||||
# Each coord ``events`` SSE listener parks a thread on
|
||||
# ``client_queue.get(timeout=5)`` for the connection lifetime.
|
||||
# Without this pool, those parks land on Python's default
|
||||
# ThreadPoolExecutor (~min(32, cpu_count+4)) and compete with
|
||||
# every other ``asyncio.to_thread`` caller — a few coord tabs
|
||||
# against a multi-child workstream are enough to stall new
|
||||
# request handlers.
|
||||
app.state.coord_sse_executor = ThreadPoolExecutor(
|
||||
max_workers=200, thread_name_prefix="coord-sse"
|
||||
)
|
||||
# Populate the router's services cache if a router is configured
|
||||
_router: ConsoleRouter | None = getattr(app.state, "router", None)
|
||||
if _router is not None:
|
||||
@@ -3975,6 +4038,27 @@ async def _lifespan(app: Starlette) -> AsyncGenerator[None, None]:
|
||||
coord_adapter.start_child_event_fanout(app.state.collector)
|
||||
except Exception:
|
||||
log.warning("console.coordinator_child_fanout_init_failed", exc_info=True)
|
||||
# Idle cleanup: closes loaded-but-stale coords AND DB orphans
|
||||
# left behind by prior console processes. The thread runs an
|
||||
# initial sweep on entry (no synchronous lifespan call needed —
|
||||
# see ``_coord_idle_cleanup_thread``) so cold-start cleanup
|
||||
# doesn't block startup. Reuses the regular-server
|
||||
# ``server.workstream_idle_timeout`` setting — the same cadence
|
||||
# makes sense for both kinds and avoids a redundant config knob.
|
||||
try:
|
||||
idle_minutes = int(config_store.get("server.workstream_idle_timeout"))
|
||||
except Exception:
|
||||
idle_minutes = 0
|
||||
if idle_minutes > 0:
|
||||
timeout_sec = float(idle_minutes * 60)
|
||||
cleanup_thread = threading.Thread(
|
||||
target=_coord_idle_cleanup_thread,
|
||||
args=(coord_mgr, timeout_sec),
|
||||
name="coord-idle-cleanup",
|
||||
daemon=True,
|
||||
)
|
||||
cleanup_thread.start()
|
||||
app.state.coord_idle_cleanup_thread = cleanup_thread
|
||||
log.info(
|
||||
"console.coordinator_mgr_ready max_active=%s",
|
||||
config_store.get("coordinator.max_active"),
|
||||
@@ -4029,6 +4113,15 @@ async def _lifespan(app: Starlette) -> AsyncGenerator[None, None]:
|
||||
if audit_exec_shutdown is not None:
|
||||
_set_audit_executor(None)
|
||||
audit_exec_shutdown.shutdown(wait=True)
|
||||
# Drain the coord SSE pool AFTER ``coord_adapter.shutdown()`` above:
|
||||
# adapter shutdown deregisters listeners so no new events handlers
|
||||
# arrive at this pool; in-flight ``client_queue.get`` futures
|
||||
# already running are bounded by their 5s timeout and finish
|
||||
# naturally. ``cancel_futures=True`` discards any queued-but-not-
|
||||
# started futures so we don't block lifespan teardown on them.
|
||||
coord_sse_exec_shutdown = getattr(app.state, "coord_sse_executor", None)
|
||||
if coord_sse_exec_shutdown is not None:
|
||||
coord_sse_exec_shutdown.shutdown(wait=True, cancel_futures=True)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -7996,6 +8089,82 @@ async def _collect_model_status(
|
||||
return {nid: models for nid, models in results if models is not None}
|
||||
|
||||
|
||||
def _refresh_coord_registry(app_state: Any, storage: Any) -> None:
|
||||
"""Rebuild ``app_state.coord_registry`` in place from DB model definitions.
|
||||
|
||||
The console-side coordinator session factory closes over the
|
||||
``coord_registry`` instance built at lifespan startup
|
||||
(see this module's lifespan setup and ``console/session_factory.py``).
|
||||
Replacing the attribute would orphan the closure — new sessions would
|
||||
still resolve through the stale object. Mutating in place via
|
||||
``ModelRegistry.reload()`` preserves identity, so:
|
||||
|
||||
- new coordinator sessions see the new state at create-time;
|
||||
- active coordinator sessions auto-pick up the swap at next ``send()``
|
||||
via ``ChatSession._refresh_model_from_registry`` (the per-send
|
||||
check compares ``cfg.model`` against ``self.model`` and re-resolves
|
||||
on mismatch).
|
||||
|
||||
Errors are logged + swallowed. The DB write that triggered this
|
||||
refresh has already succeeded, and the explicit reload button
|
||||
remains the user-facing recovery path. Validation failures
|
||||
(e.g. admin deleted the alias that ``registry.default`` points at)
|
||||
leave the existing registry intact rather than tearing down a
|
||||
working coordinator.
|
||||
"""
|
||||
from turnstone.core.model_registry import load_model_registry
|
||||
|
||||
existing = getattr(app_state, "coord_registry", None)
|
||||
if existing is None:
|
||||
# Lifespan didn't build a coord_registry (no DB model rows at boot)
|
||||
# — the entire coord subsystem stayed uninitialized, so a console
|
||||
# restart is required after the operator adds the first row.
|
||||
return
|
||||
try:
|
||||
# ``strict=True`` so a transient DB read error surfaces here.
|
||||
# Without it, the loader degrades to a config.toml-only registry
|
||||
# and ``existing.reload()`` would silently drop every DB-sourced
|
||||
# alias.
|
||||
new_registry = load_model_registry(storage=storage, strict=True)
|
||||
except ValueError as exc:
|
||||
# ModelRegistry.__init__ raises ValueError for several distinct
|
||||
# config issues — empty models, default/fallback/agent/plan/task
|
||||
# alias not present in the loaded set. Log the actual reason so
|
||||
# operators can tell "no enabled rows" from "default alias typo
|
||||
# in config.toml". Existing registry stays in place either way.
|
||||
log.warning("console.coord_registry_refresh_skipped reason=%s", exc)
|
||||
return
|
||||
except Exception:
|
||||
log.warning("console.coord_registry_refresh_load_failed", exc_info=True)
|
||||
return
|
||||
try:
|
||||
existing.reload(
|
||||
new_registry.models,
|
||||
new_registry.default,
|
||||
new_registry.fallback,
|
||||
new_registry.agent_model,
|
||||
plan_model=new_registry.plan_model,
|
||||
task_model=new_registry.task_model,
|
||||
plan_effort=new_registry.plan_effort,
|
||||
task_effort=new_registry.task_effort,
|
||||
)
|
||||
except Exception:
|
||||
log.warning("console.coord_registry_refresh_reload_failed", exc_info=True)
|
||||
finally:
|
||||
# Defensive — load_model_registry doesn't eagerly create clients
|
||||
# (ModelRegistry.__init__ leaves _clients/_providers empty; they
|
||||
# populate lazily on first resolve), so shutdown() iterates empty
|
||||
# dicts in practice. Kept against the day the loader grows
|
||||
# eager-init or a future caller pre-warms the throwaway, and
|
||||
# wrapped because shutdown() in finally would otherwise escape
|
||||
# after a successful in-place reload — surfacing as 500 with the
|
||||
# registry actually mutated and the audit row recording success.
|
||||
try:
|
||||
new_registry.shutdown()
|
||||
except Exception:
|
||||
log.warning("console.coord_registry_refresh_shutdown_failed", exc_info=True)
|
||||
|
||||
|
||||
async def _notify_nodes_model_reload(request: Request) -> dict[str, Any]:
|
||||
"""Tell all nodes to re-read model definitions from DB and rebuild registry."""
|
||||
collector: ClusterCollector = request.app.state.collector
|
||||
@@ -8232,6 +8401,8 @@ async def admin_create_model_definition(request: Request) -> JSONResponse:
|
||||
ip,
|
||||
)
|
||||
|
||||
await asyncio.to_thread(_refresh_coord_registry, request.app.state, storage)
|
||||
|
||||
created = storage.get_model_definition(definition_id)
|
||||
if created is None:
|
||||
return JSONResponse(
|
||||
@@ -8389,6 +8560,9 @@ async def admin_update_model_definition(request: Request) -> JSONResponse:
|
||||
ip,
|
||||
)
|
||||
|
||||
if updates:
|
||||
await asyncio.to_thread(_refresh_coord_registry, request.app.state, storage)
|
||||
|
||||
model_def = storage.get_model_definition(definition_id)
|
||||
return JSONResponse(_mask_model_secrets(model_def or {}))
|
||||
|
||||
@@ -8424,6 +8598,8 @@ async def admin_delete_model_definition(request: Request) -> JSONResponse:
|
||||
ip,
|
||||
)
|
||||
|
||||
await asyncio.to_thread(_refresh_coord_registry, request.app.state, storage)
|
||||
|
||||
return JSONResponse({"status": "ok", "definition_id": definition_id})
|
||||
|
||||
|
||||
@@ -8443,6 +8619,13 @@ async def admin_model_reload(request: Request) -> JSONResponse:
|
||||
# before they rebuild their model registries.
|
||||
await _publish_config_change(request)
|
||||
|
||||
# Refresh the console's own coord_registry first. The node fan-out
|
||||
# below carries DB→nodes propagation; the console hosts coordinator
|
||||
# sessions itself and must mutate its in-process registry too —
|
||||
# otherwise the coord LLM keeps calling the prior model name even
|
||||
# after a successful reload.
|
||||
await asyncio.to_thread(_refresh_coord_registry, request.app.state, storage)
|
||||
|
||||
results = await _notify_nodes_model_reload(request)
|
||||
return JSONResponse({"status": "ok", "results": results})
|
||||
|
||||
@@ -10196,6 +10379,14 @@ def create_app(
|
||||
# tombstones are non-resurrectable.
|
||||
saved_state_filter="closed",
|
||||
saved_loaded_lookup=_coord_saved_loaded_lookup,
|
||||
# Isolate coord SSE polling on its own 200-thread pool so a
|
||||
# handful of coord tabs (each parking a thread on
|
||||
# ``client_queue.get``) can't starve the default executor and
|
||||
# stall every other ``asyncio.to_thread`` caller (storage,
|
||||
# router, audit). Mirrors the interactive endpoint's
|
||||
# ``sse_executor_lookup`` wiring on ``interactive_endpoint_config``
|
||||
# in ``turnstone/server.py``.
|
||||
sse_executor_lookup=lambda request: request.app.state.coord_sse_executor,
|
||||
)
|
||||
coord_workstream_routes: list[Any] = []
|
||||
register_session_routes(
|
||||
|
||||
@@ -335,6 +335,73 @@
|
||||
return appendMsg(role, esc(text), opts);
|
||||
}
|
||||
|
||||
// Metacognitive reminder bubble (user-channel correction / denial /
|
||||
// resume / start / completion AND tool-channel tool_error / repeat).
|
||||
// Mirrors Pane.prototype.addUserReminder / addToolReminder in the
|
||||
// interactive UI — yellow themed bubble slotted directly below the
|
||||
// message it advises. ``anchor`` is the DOM element to anchor below;
|
||||
// when null, append at the bottom of messagesEl.
|
||||
function appendReminderBubble(reminders, anchor) {
|
||||
if (!Array.isArray(reminders) || !reminders.length) return;
|
||||
let cursor = anchor;
|
||||
for (let i = 0; i < reminders.length; i++) {
|
||||
const r = reminders[i] || {};
|
||||
const el = document.createElement("div");
|
||||
el.className = "msg user-reminder";
|
||||
el.setAttribute("role", "article");
|
||||
el.setAttribute("data-ts-role", "metacognition");
|
||||
el.setAttribute("aria-label", "metacognition");
|
||||
const body = document.createElement("div");
|
||||
body.className = "msg-body";
|
||||
const labelEl = document.createElement("span");
|
||||
labelEl.className = "msg-user-reminder-label";
|
||||
labelEl.textContent =
|
||||
"metacognition" + (r.type ? " · " + String(r.type) : "");
|
||||
const textEl = document.createElement("span");
|
||||
textEl.className = "msg-user-reminder-text";
|
||||
textEl.textContent = r.text || "";
|
||||
body.appendChild(labelEl);
|
||||
body.appendChild(textEl);
|
||||
el.appendChild(body);
|
||||
if (cursor) {
|
||||
cursor.insertAdjacentElement("afterend", el);
|
||||
cursor = el;
|
||||
} else {
|
||||
messagesEl.appendChild(el);
|
||||
}
|
||||
}
|
||||
_scheduleScroll();
|
||||
}
|
||||
|
||||
// Live SSE for user-channel reminders — anchors below the most
|
||||
// recent user message. On a non-originating tab there may be no
|
||||
// user message rendered yet; we append and the next /history reload
|
||||
// corrects. (Same caveat as the interactive UI; tracked there.)
|
||||
function appendUserReminderLive(reminders) {
|
||||
const userMsgs = messagesEl.querySelectorAll(".msg.user");
|
||||
const anchor = userMsgs.length ? userMsgs[userMsgs.length - 1] : null;
|
||||
appendReminderBubble(reminders, anchor);
|
||||
}
|
||||
|
||||
// Live SSE for tool-channel reminders — anchors below the
|
||||
// .coord-tool-batch construct that produced the tool result. Looks
|
||||
// up the row by data-call-id and walks to the parent batch; falls
|
||||
// back to the most recent batch if not found.
|
||||
function appendToolReminderLive(reminders, toolCallId) {
|
||||
let anchor = null;
|
||||
if (toolCallId) {
|
||||
const entry = toolRows.get(toolCallId);
|
||||
if (entry && entry.batch) {
|
||||
anchor = entry.batch;
|
||||
}
|
||||
}
|
||||
if (!anchor) {
|
||||
const batches = messagesEl.querySelectorAll(".coord-tool-batch");
|
||||
if (batches.length) anchor = batches[batches.length - 1];
|
||||
}
|
||||
appendReminderBubble(reminders, anchor);
|
||||
}
|
||||
|
||||
// Build a tool-batch item from a persisted assistant
|
||||
// tool_call. Live calls land here with header / preview already
|
||||
// computed by ChatSession._prepare_tool; history replay never sees
|
||||
@@ -1844,6 +1911,22 @@
|
||||
// styling which mis-categorised them as tool calls.
|
||||
appendText("info", ev.message || "", { label: "info" });
|
||||
break;
|
||||
case "user_reminder":
|
||||
// Metacognitive user-channel nudge — render below the most
|
||||
// recent user message as a yellow themed bubble. Same shape
|
||||
// as the interactive UI's case.
|
||||
if (Array.isArray(ev.reminders) && ev.reminders.length) {
|
||||
appendUserReminderLive(ev.reminders);
|
||||
}
|
||||
break;
|
||||
case "tool_reminder":
|
||||
// Metacognitive tool-channel nudge — render below the
|
||||
// .coord-tool-batch that produced the tool result identified
|
||||
// by ev.tool_call_id.
|
||||
if (Array.isArray(ev.reminders) && ev.reminders.length) {
|
||||
appendToolReminderLive(ev.reminders, ev.tool_call_id || "");
|
||||
}
|
||||
break;
|
||||
case "connected":
|
||||
// First yield from _coord_events_replay — populates the
|
||||
// status bar's model cell before any history arrives. Also
|
||||
@@ -2085,6 +2168,14 @@
|
||||
const TERMINAL_CHILD_STATES = new Set(["closed", "deleted"]);
|
||||
const LIVE_BADGE_TTL_MS = 5000;
|
||||
const LIVE_BADGE_DEBOUNCE_MS = 250;
|
||||
// After handleChildState mutates liveBadgeCache from a child_ws_state
|
||||
// SSE event, a bulk-poll landing within this window must NOT
|
||||
// overwrite the SSE-supplied pending_approval / _detail fields with
|
||||
// its own (potentially stale) snapshot — the upstream node
|
||||
// /dashboard cache has its own ~2s TTL so a poll right after a
|
||||
// transition can carry pre-transition state. 3s covers the worst
|
||||
// case (upstream TTL + console TTL minus a margin).
|
||||
const SSE_AUTHORITATIVE_MS = 3000;
|
||||
// Debounce window for /tasks refreshes triggered by ``tasks``
|
||||
// tool_result SSE events. Without it, a model that runs
|
||||
// ``add → list`` (or any back-to-back mutation pair) double-fetches
|
||||
@@ -2192,9 +2283,11 @@
|
||||
// Inline approve/deny block \u2014 shown only when the live block
|
||||
// carries pending_approval_detail (the rich payload added by the
|
||||
// server-side dashboard projection). A "\u2691 approval" badge alone
|
||||
// means the child is in attention state but the rich detail hasn't
|
||||
// arrived yet (urgent live-bulk fetch is in flight); the row gets
|
||||
// re-rendered when it lands.
|
||||
// means the child is in attention state but the rich detail
|
||||
// hasn't arrived on the cache yet \u2014 a rare cross-version race
|
||||
// (e.g. a node mid-rolling-upgrade emitted ws_state without
|
||||
// pending_approval_detail before this PR landed). The next SSE
|
||||
// tick or the 5s TTL bulk-poll catches up and re-renders.
|
||||
if (cached && cached.live && cached.live.pending_approval_detail) {
|
||||
const detail = cached.live.pending_approval_detail;
|
||||
const block = renderApprovalBlock(child, detail);
|
||||
@@ -3066,12 +3159,35 @@
|
||||
? results[id]
|
||||
: null;
|
||||
const wasDenied = denied.indexOf(id) !== -1;
|
||||
const prev = liveBadgeCache.get(id);
|
||||
// SSE-set pending_approval / _detail wins over a stale
|
||||
// bulk-poll snapshot for SSE_AUTHORITATIVE_MS after the
|
||||
// SSE update. Without this guard, a poll landing right
|
||||
// after a child_ws_state transition can clobber freshly-
|
||||
// mutated approval state with pre-transition data from
|
||||
// the upstream /dashboard cache (which has its own ~2s
|
||||
// TTL). Other fields (tokens, context_ratio) still track
|
||||
// the bulk response — only the approval surface is gated.
|
||||
let mergedLive = live;
|
||||
if (
|
||||
live &&
|
||||
prev &&
|
||||
prev.sseUpdatedAt &&
|
||||
now - prev.sseUpdatedAt < SSE_AUTHORITATIVE_MS &&
|
||||
prev.live
|
||||
) {
|
||||
mergedLive = Object.assign({}, live, {
|
||||
pending_approval: prev.live.pending_approval,
|
||||
pending_approval_detail: prev.live.pending_approval_detail,
|
||||
});
|
||||
}
|
||||
liveBadgeCache.set(id, {
|
||||
live: live,
|
||||
live: mergedLive,
|
||||
fetched: now,
|
||||
// Denied ids are permission/identity misses — mark permanent
|
||||
// so SSE state ticks on those rows don't retry every window.
|
||||
permanent: wasDenied,
|
||||
sseUpdatedAt: prev ? prev.sseUpdatedAt || 0 : 0,
|
||||
});
|
||||
const row = childrenTreeEl.querySelector(
|
||||
'.ch-row[data-ws-id="' + cssEscape(id) + '"]',
|
||||
@@ -3092,10 +3208,12 @@
|
||||
const isPermanent = e && /HTTP 403/.test(e.message || "");
|
||||
const now = Date.now();
|
||||
ids.forEach((id) => {
|
||||
const prev = liveBadgeCache.get(id);
|
||||
liveBadgeCache.set(id, {
|
||||
live: null,
|
||||
fetched: now,
|
||||
permanent: isPermanent,
|
||||
sseUpdatedAt: prev ? prev.sseUpdatedAt || 0 : 0,
|
||||
});
|
||||
});
|
||||
if (!isPermanent) console.warn("flushLiveFetches failed", e);
|
||||
@@ -3135,7 +3253,6 @@
|
||||
ws_id: childId,
|
||||
name: "",
|
||||
};
|
||||
const prevActivity = existing.activity_state || "";
|
||||
existing.state = ev.state || existing.state;
|
||||
existing.activity_state =
|
||||
typeof ev.activity_state === "string"
|
||||
@@ -3144,32 +3261,60 @@
|
||||
if (ev.node_id) existing.node_id = ev.node_id;
|
||||
childrenState.set(childId, existing);
|
||||
_touchChild(childId);
|
||||
renderChildren();
|
||||
// Do NOT invalidateLiveBadge on routine state ticks — that defeats
|
||||
// the 5s TTL cache and devolves rate-limiting to the 250ms
|
||||
// debouncer, hitting cluster_ws_detail ~4 req/s per chatty child.
|
||||
// The TTL check in scheduleLiveFetch will refresh the badge on its
|
||||
// own schedule; identity-changing events (created/rename/closed)
|
||||
// still invalidate below.
|
||||
//
|
||||
// Two activity_state transitions warrant an *urgent* (TTL-bypassing)
|
||||
// fetch so the row carries pending_approval_detail in lockstep with
|
||||
// the child's true state:
|
||||
// - "" / "tool" / "thinking" → "approval" (need rich payload now
|
||||
// so the inline approve/deny buttons can render)
|
||||
// - "approval" → anything else (need to drop the
|
||||
// stale payload so the buttons disappear; without this the
|
||||
// 5s TTL leaves stale buttons on a row whose approval was
|
||||
// resolved elsewhere — e.g. the child's own UI tab)
|
||||
const enteredApproval =
|
||||
existing.activity_state === "approval" && prevActivity !== "approval";
|
||||
const leftApproval =
|
||||
prevActivity === "approval" && existing.activity_state !== "approval";
|
||||
if (enteredApproval || leftApproval) {
|
||||
scheduleLiveFetch(childId, { urgent: true });
|
||||
// pending_approval_detail rides on the ws_state event when an
|
||||
// approval is pending (see turnstone/server.py
|
||||
// WebUI._broadcast_state — gated on ``_pending_approval is not
|
||||
// None``, so absent on the steady state and possibly null on a
|
||||
// node mid-rolling-upgrade). When present we mutate
|
||||
// liveBadgeCache directly here — inline approve/deny buttons
|
||||
// render in lockstep with the activity_state transition without
|
||||
// a separate live-bulk fetch. The cache entry is tagged
|
||||
// ``sseUpdatedAt`` so a bulk-poll landing within
|
||||
// SSE_AUTHORITATIVE_MS preserves the SSE-supplied fields
|
||||
// (the upstream /dashboard cache's ~2s TTL would otherwise
|
||||
// clobber a fresh transition with pre-transition state).
|
||||
const pendingApproval = existing.activity_state === "approval";
|
||||
const evDetail =
|
||||
ev.pending_approval_detail !== undefined
|
||||
? ev.pending_approval_detail
|
||||
: null;
|
||||
const cached = liveBadgeCache.get(childId);
|
||||
// When pending, prefer SSE-supplied detail. If SSE didn't
|
||||
// carry it (rare race; e.g. a node mid-rolling-upgrade), keep
|
||||
// any existing cached detail rather than blanking the row —
|
||||
// the next bulk-poll catches up. When not pending, hard-clear.
|
||||
let nextDetail;
|
||||
if (pendingApproval) {
|
||||
nextDetail =
|
||||
evDetail !== null
|
||||
? evDetail
|
||||
: cached && cached.live
|
||||
? cached.live.pending_approval_detail
|
||||
: null;
|
||||
} else {
|
||||
scheduleLiveFetch(childId);
|
||||
nextDetail = null;
|
||||
}
|
||||
const nextLive = Object.assign({}, (cached && cached.live) || {}, {
|
||||
pending_approval: pendingApproval,
|
||||
pending_approval_detail: nextDetail,
|
||||
});
|
||||
liveBadgeCache.set(childId, {
|
||||
live: nextLive,
|
||||
// Preserve prior bulk-poll fetched timestamp so a fresh SSE
|
||||
// tick doesn't artificially extend the 5s TTL gate in
|
||||
// scheduleLiveFetch — the bulk-poll still drives slower-
|
||||
// moving fields (tokens, context_ratio) on its own schedule.
|
||||
fetched: cached ? cached.fetched : 0,
|
||||
permanent: !!(cached && cached.permanent),
|
||||
sseUpdatedAt: Date.now(),
|
||||
});
|
||||
renderChildren();
|
||||
// Do NOT invalidateLiveBadge on routine state ticks — that
|
||||
// defeats the 5s TTL cache and devolves rate-limiting to the
|
||||
// 250ms debouncer. The TTL check in scheduleLiveFetch handles
|
||||
// refresh cadence for slower-moving fields; identity-changing
|
||||
// events (created/rename/closed) still invalidate below.
|
||||
scheduleLiveFetch(childId);
|
||||
}
|
||||
|
||||
function handleChildClosed(ev) {
|
||||
@@ -3481,6 +3626,12 @@
|
||||
(callId && toolNameByCallId.get(callId)) || m.tool_name || "tool";
|
||||
const isError = callOutcomes.get(callId) === "error";
|
||||
appendToolResult(toolName, callId, content || "", isError);
|
||||
// Tool-channel metacog reminders ride the same _reminders
|
||||
// side-channel as the user channel; surface as a themed
|
||||
// bubble below the .coord-tool-batch construct.
|
||||
if (Array.isArray(m.reminders) && m.reminders.length) {
|
||||
appendToolReminderLive(m.reminders, callId);
|
||||
}
|
||||
} else if (role === "assistant") {
|
||||
// Empty content with tool_calls only means the assistant
|
||||
// turn was just tool dispatch — the synthesized tool-call
|
||||
@@ -3511,6 +3662,15 @@
|
||||
// (appendReasoningToken uses textContent; user/system are
|
||||
// typed verbatim and don't carry markdown structure).
|
||||
appendText(role, content, { label: role });
|
||||
// User-channel metacog reminders attach to the just-appended
|
||||
// user bubble (the most recent .msg.user in messagesEl).
|
||||
if (
|
||||
role === "user" &&
|
||||
Array.isArray(m.reminders) &&
|
||||
m.reminders.length
|
||||
) {
|
||||
appendUserReminderLive(m.reminders);
|
||||
}
|
||||
}
|
||||
});
|
||||
// History alone can't tell whether an orphaned assistant
|
||||
|
||||
@@ -432,7 +432,7 @@ LAST_ERROR_CONFIG_KEY = "last_error"
|
||||
# such error per workstream would bloat workstream_config and the model
|
||||
# prompt the coord LLM ingests on inspect. 1024 chars matches the
|
||||
# practical "useful for triage" length while staying well under the
|
||||
# WAIT_MESSAGE_MAX_BYTES (6 KiB) cap so the truncate happens here at
|
||||
# WAIT_MESSAGE_MAX_BYTES (10 KiB) cap so the truncate happens here at
|
||||
# write time, not later at the wait surface.
|
||||
LAST_ERROR_MAX_LEN = 1024
|
||||
|
||||
|
||||
@@ -5,7 +5,50 @@ from __future__ import annotations
|
||||
import re
|
||||
import time
|
||||
|
||||
_COOLDOWN_SECS = 300 # 5 minutes between nudges of the same type
|
||||
# Default cooldown (s) between nudges of the same type. Production
|
||||
# paths pass ``cooldown_secs`` explicitly from
|
||||
# ``MemoryConfig.nudge_cooldown`` (config-store ``memory.nudge_cooldown``,
|
||||
# default 300); this constant is the fallback for tests and unit-style
|
||||
# callers without a ``MemoryConfig`` and is kept aligned with that
|
||||
# canonical default so both paths behave the same.
|
||||
_COOLDOWN_SECS = 300
|
||||
|
||||
# Repeat-detection threshold — number of *consecutive* identical tool
|
||||
# calls (same name + same arguments) before a repeat warning fires.
|
||||
# Two-in-a-row is too noisy because legitimate retries on transient
|
||||
# failures look identical; three-in-a-row is the cheapest signal that
|
||||
# the model is stuck on the same call.
|
||||
_REPEAT_THRESHOLD = 3
|
||||
|
||||
|
||||
class RepeatDetector:
|
||||
"""Detect a streak of identical tool-call signatures.
|
||||
|
||||
``record(sig)`` returns ``True`` once *sig* has been recorded
|
||||
``threshold`` times in a row (default 3). Recording a different
|
||||
signature resets the streak — interleaved tool calls aren't a
|
||||
stuck loop, only repeated identical ones are. After a fire, the
|
||||
caller is expected to call ``clear()`` to start a fresh streak.
|
||||
"""
|
||||
|
||||
def __init__(self, threshold: int = _REPEAT_THRESHOLD) -> None:
|
||||
self._threshold = threshold
|
||||
self._sig: str | None = None
|
||||
self._count = 0
|
||||
|
||||
def record(self, sig: str) -> bool:
|
||||
"""Record *sig*; return ``True`` when the streak hits the threshold."""
|
||||
if sig == self._sig:
|
||||
self._count += 1
|
||||
else:
|
||||
self._sig = sig
|
||||
self._count = 1
|
||||
return self._count >= self._threshold
|
||||
|
||||
def clear(self) -> None:
|
||||
self._sig = None
|
||||
self._count = 0
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Nudge messages (brief, model-facing hints)
|
||||
|
||||
@@ -230,6 +230,7 @@ class ModelRegistry:
|
||||
if task_model and task_model not in models:
|
||||
raise ValueError(f"Task model '{task_model}' not found in registry")
|
||||
with self._client_lock:
|
||||
old_models = self._models
|
||||
self._models = dict(models)
|
||||
self.default = default
|
||||
self.fallback = list(fallback) if fallback else []
|
||||
@@ -238,11 +239,32 @@ class ModelRegistry:
|
||||
self.task_model = task_model
|
||||
self.plan_effort = plan_effort
|
||||
self.task_effort = task_effort
|
||||
for client in self._clients.values():
|
||||
if hasattr(client, "close"):
|
||||
client.close()
|
||||
self._clients.clear()
|
||||
self._providers.clear()
|
||||
# Selective teardown — close + drop only clients whose
|
||||
# connection target actually changed (alias removed, or
|
||||
# base_url / api_key / provider differs). Keeps connection
|
||||
# pools warm for the common admin-edit case where only
|
||||
# ``model`` / ``temperature`` / ``context_window`` changed.
|
||||
for alias, client in list(self._clients.items()):
|
||||
old_cfg = old_models.get(alias)
|
||||
new_cfg = self._models.get(alias)
|
||||
if (
|
||||
new_cfg is None
|
||||
or old_cfg is None
|
||||
or old_cfg.base_url != new_cfg.base_url
|
||||
or old_cfg.api_key != new_cfg.api_key
|
||||
or old_cfg.provider != new_cfg.provider
|
||||
):
|
||||
if hasattr(client, "close"):
|
||||
client.close()
|
||||
del self._clients[alias]
|
||||
# Providers are keyed on alias but only depend on
|
||||
# ``cfg.provider`` — drop only when the provider string
|
||||
# changed or the alias was removed.
|
||||
for alias in list(self._providers.keys()):
|
||||
old_cfg = old_models.get(alias)
|
||||
new_cfg = self._models.get(alias)
|
||||
if new_cfg is None or old_cfg is None or old_cfg.provider != new_cfg.provider:
|
||||
del self._providers[alias]
|
||||
|
||||
def shutdown(self) -> None:
|
||||
"""Close all cached client connections."""
|
||||
@@ -300,6 +322,7 @@ def load_model_registry(
|
||||
context_window: int = 32768,
|
||||
provider: str = "openai",
|
||||
storage: Any | None = None,
|
||||
strict: bool = False,
|
||||
) -> ModelRegistry:
|
||||
"""Build a ModelRegistry from CLI args, ``config.toml``, and database.
|
||||
|
||||
@@ -317,6 +340,15 @@ def load_model_registry(
|
||||
``[model].plan_effort``, ``[model].task_effort`` control routing.
|
||||
``plan_model``/``task_model`` override ``agent_model`` per sub-agent
|
||||
role; both fall back to it when unset.
|
||||
|
||||
``strict``: when True, a storage read failure during the DB-rows step
|
||||
re-raises instead of degrading to a config.toml-only registry.
|
||||
Callers that hot-reload an existing registry need this so a transient
|
||||
DB outage doesn't silently drop every DB-sourced alias when the
|
||||
truncated result is applied via ``ModelRegistry.reload``. Callers
|
||||
that build a fresh registry from scratch (CLI, lifespan startup) want
|
||||
the default behaviour — boot succeeds with a config-only fallback
|
||||
rather than crashing on a flaky DB.
|
||||
"""
|
||||
import json as _json
|
||||
|
||||
@@ -370,6 +402,8 @@ def load_model_registry(
|
||||
server_compat=row_server_compat,
|
||||
)
|
||||
except Exception:
|
||||
if strict:
|
||||
raise
|
||||
log.warning("Failed to load model definitions from storage", exc_info=True)
|
||||
|
||||
# 2. Build configs from [models.*] sections (overrides DB for same alias)
|
||||
|
||||
+390
-176
@@ -81,6 +81,7 @@ from turnstone.core.memory_relevance import (
|
||||
score_memories,
|
||||
)
|
||||
from turnstone.core.metacognition import (
|
||||
RepeatDetector,
|
||||
detect_completion,
|
||||
detect_correction,
|
||||
format_nudge,
|
||||
@@ -90,6 +91,7 @@ from turnstone.core.providers import create_provider
|
||||
from turnstone.core.safety import is_command_blocked, sanitize_command
|
||||
from turnstone.core.sandbox import execute_math_sandboxed
|
||||
from turnstone.core.storage._registry import get_storage
|
||||
from turnstone.core.tool_advisory import escape_wrapper_tags, render_system_reminder
|
||||
from turnstone.core.tool_search import ToolSearchManager
|
||||
from turnstone.core.tools import (
|
||||
AGENT_AUTO_TOOLS,
|
||||
@@ -275,6 +277,8 @@ class SessionUI(Protocol):
|
||||
def on_plan_review(self, content: str) -> str: ...
|
||||
def on_info(self, message: str) -> None: ...
|
||||
def on_error(self, message: str) -> None: ...
|
||||
def on_user_reminder(self, reminders: list[dict[str, str]]) -> None: ...
|
||||
def on_tool_reminder(self, reminders: list[dict[str, str]], tool_call_id: str) -> None: ...
|
||||
def on_state_change(self, state: str) -> None: ...
|
||||
def on_rename(self, name: str) -> None: ...
|
||||
def on_intent_verdict(self, verdict: dict[str, Any]) -> None:
|
||||
@@ -474,8 +478,12 @@ class ChatSession:
|
||||
collections.OrderedDict()
|
||||
)
|
||||
self._queued_lock = threading.Lock()
|
||||
# Repeat detection: track recent tool call signatures
|
||||
self._recent_tool_sigs: set[str] = set()
|
||||
# Repeat detection: streak counter over tool-call signatures.
|
||||
# Fires when a (name, args) signature has been seen N times in
|
||||
# a row; recording any different signature resets the streak.
|
||||
# Also cleared after a write tool succeeds (state changed) or
|
||||
# after a warning fires (clean slate, re-fire on the next streak).
|
||||
self._repeat_detector = RepeatDetector()
|
||||
# Tool error tracking: call_id → is_error for message persistence
|
||||
self._tool_error_flags: dict[str, bool] = {}
|
||||
# Cooperative cancellation: set from outside to stop generation
|
||||
@@ -1293,7 +1301,7 @@ class ChatSession:
|
||||
self._ws_id = ws_id
|
||||
self.messages = messages
|
||||
self._read_files.clear()
|
||||
self._recent_tool_sigs.clear()
|
||||
self._repeat_detector.clear()
|
||||
self._last_usage = None
|
||||
self._calibrated_msg_count = 0
|
||||
self._title_generated = True # don't re-title resumed workstreams
|
||||
@@ -1638,6 +1646,124 @@ class ChatSession:
|
||||
"""System messages + conversation history."""
|
||||
return self.system_messages + self.messages
|
||||
|
||||
def _apply_reminders_for_provider(
|
||||
self,
|
||||
messages: list[dict[str, Any]],
|
||||
) -> list[dict[str, Any]]:
|
||||
"""Return a transient copy of *messages* with ``_reminders`` rendered
|
||||
inline for the model.
|
||||
|
||||
Metacognitive nudges live on the message dict's ``_reminders``
|
||||
side-channel regardless of role — user messages carry
|
||||
user-channel nudges (correction / denial / resume / start /
|
||||
completion), tool messages carry tool-channel nudges
|
||||
(tool_error / repeat). Both ride the same side-channel so
|
||||
``self.messages`` and every downstream consumer (UI replay,
|
||||
compaction, title gen, channel adapters, DB) see clean
|
||||
``content``; only the wire-bound copy here carries the
|
||||
rendered reminder.
|
||||
|
||||
For each message that has ``_reminders`` AND has not yet been
|
||||
flagged delivered, build a shallow copy and splice the reminders
|
||||
as ``<system-reminder>`` blocks onto the trailing edge of the
|
||||
copy's ``content`` — string content gets a tail block, list
|
||||
content gets the block on the trailing text part (or a new text
|
||||
part if there isn't one). Messages without ``_reminders`` (or
|
||||
already delivered) pass through unchanged (same object
|
||||
reference) so the common case is allocation-free.
|
||||
|
||||
**Reminder lifecycle.** After a successful provider stream
|
||||
``_mark_reminders_delivered`` flips ``_reminders_delivered`` to
|
||||
``True`` on every message (user or tool) that carried reminders
|
||||
into that call, so subsequent provider calls skip them — the
|
||||
model sees each reminder once, the turn it advised. The
|
||||
``_reminders`` key itself stays on the message dict for the
|
||||
lifetime of the in-memory session so ``/history`` (reconnecting
|
||||
tabs, multi-tab live mirrors) still renders the same nudge
|
||||
bubbles the originating tab saw; only the wire-side replay is
|
||||
suppressed. Compaction is the natural full drain (it replaces
|
||||
``self.messages`` wholesale).
|
||||
|
||||
``sanitize_messages`` later drops both leading-underscore sibling
|
||||
keys (``_reminders`` and ``_reminders_delivered``) on the way to
|
||||
the wire, so the provider sees only ``content`` with the
|
||||
reminder spliced in.
|
||||
|
||||
**Read-only contract on the returned list.** The pass-through
|
||||
path returns the original ``msg`` by reference; callers must
|
||||
not mutate the returned dicts in place (today's only callers —
|
||||
``sanitize_messages`` + provider conversion — construct new
|
||||
dicts, so the contract holds). Mutations on the spliced copy
|
||||
are safe; mutations on a pass-through reference would bleed
|
||||
back into ``self.messages``.
|
||||
"""
|
||||
out: list[dict[str, Any]] = []
|
||||
for msg in messages:
|
||||
raw_reminders = msg.get("_reminders")
|
||||
if not raw_reminders or msg.get("_reminders_delivered"):
|
||||
out.append(msg)
|
||||
continue
|
||||
# Defensive filter — only dict entries are valid; a string /
|
||||
# None / other shape from corruption or partial state must
|
||||
# not abort the whole send via an AttributeError on .get().
|
||||
# Mirrors the same filter ``_build_history`` applies on the
|
||||
# wire-out side.
|
||||
reminders = [r for r in raw_reminders if isinstance(r, dict)]
|
||||
if not reminders:
|
||||
out.append(msg)
|
||||
continue
|
||||
block = "\n\n" + "\n\n".join(
|
||||
render_system_reminder(r.get("text", "")) for r in reminders
|
||||
)
|
||||
copy = dict(msg)
|
||||
content = copy.get("content")
|
||||
if isinstance(content, str):
|
||||
copy["content"] = escape_wrapper_tags(content) + block
|
||||
elif isinstance(content, list):
|
||||
# Shallow-copy the parts list and any text parts we'll
|
||||
# mutate so the original list/dicts in self.messages stay
|
||||
# untouched.
|
||||
new_parts = [
|
||||
dict(p) if isinstance(p, dict) and p.get("type") == "text" else p
|
||||
for p in content
|
||||
]
|
||||
text_parts = [
|
||||
p for p in new_parts if isinstance(p, dict) and p.get("type") == "text"
|
||||
]
|
||||
for part in text_parts:
|
||||
part["text"] = escape_wrapper_tags(part.get("text", ""))
|
||||
if text_parts:
|
||||
text_parts[-1]["text"] = text_parts[-1]["text"] + block
|
||||
else:
|
||||
new_parts.append({"type": "text", "text": block})
|
||||
copy["content"] = new_parts
|
||||
else:
|
||||
# Unexpected shape (None, etc.) — attach as a text-only
|
||||
# content rather than dropping the reminder silently.
|
||||
copy["content"] = block.lstrip()
|
||||
out.append(copy)
|
||||
return out
|
||||
|
||||
def _mark_reminders_delivered(self) -> None:
|
||||
"""Flag every message's ``_reminders`` as delivered.
|
||||
|
||||
Role-agnostic — both user-channel reminders (set by
|
||||
``_attach_pending_user_reminders`` on user messages) and
|
||||
tool-channel reminders (set by the per-result loop on tool
|
||||
messages) ride the same ``_reminders`` side-channel and the
|
||||
same delivered flag. Called after a successful provider
|
||||
stream; subsequent calls to ``_apply_reminders_for_provider``
|
||||
skip messages with this flag, so the model sees each reminder
|
||||
exactly once (the turn it advised). The flag is a sibling key
|
||||
like ``_reminders`` itself; ``sanitize_messages`` strips both
|
||||
before the wire and ``_build_history`` ignores the delivered
|
||||
flag entirely so UI replay parity is preserved across
|
||||
reconnects.
|
||||
"""
|
||||
for msg in self.messages:
|
||||
if msg.get("_reminders") and not msg.get("_reminders_delivered"):
|
||||
msg["_reminders_delivered"] = True
|
||||
|
||||
def _emit_state(self, state: str) -> None:
|
||||
"""Notify UI of a workstream state transition.
|
||||
|
||||
@@ -2113,12 +2239,13 @@ class ChatSession:
|
||||
# Metacognitive user-channel drain: any nudges queued via
|
||||
# _queue_user_advisory (correction/start/completion from this
|
||||
# turn, denial from the previous tool batch, resume from
|
||||
# rehydrate) splice in as <system-reminder> blocks at the
|
||||
# trailing edge of the user content. The DB row stores
|
||||
# ``user_input`` only (line below) so these blocks stay
|
||||
# ephemeral — they advise the next assistant turn and do not
|
||||
# persist across reloads.
|
||||
self._splice_pending_user_advisories(user_msg)
|
||||
# rehydrate) attach to the user message dict's ``_reminders``
|
||||
# side-channel — content stays clean. The wire-side splice
|
||||
# happens later in _apply_reminders_for_provider against a
|
||||
# transient copy. The DB row stores ``user_input`` only (line
|
||||
# below) so reminders stay in-memory only and don't persist
|
||||
# across reloads.
|
||||
self._attach_pending_user_reminders(user_msg)
|
||||
self.messages.append(user_msg)
|
||||
self._msg_tokens.append(max(1, int(self._msg_char_count(user_msg) / self._chars_per_token)))
|
||||
# DB row stores the raw text only; attachments are joined back in
|
||||
@@ -2198,7 +2325,7 @@ class ChatSession:
|
||||
try:
|
||||
while True:
|
||||
self._check_cancelled(my_generation)
|
||||
msgs = self._full_messages()
|
||||
msgs = self._apply_reminders_for_provider(self._full_messages())
|
||||
|
||||
if self.debug:
|
||||
self._debug_print_request(msgs)
|
||||
@@ -2235,7 +2362,7 @@ class ChatSession:
|
||||
self.ui.on_thinking_stop()
|
||||
try:
|
||||
self._compact_messages(auto=True)
|
||||
msgs = self._full_messages()
|
||||
msgs = self._apply_reminders_for_provider(self._full_messages())
|
||||
self.ui.on_thinking_start()
|
||||
stream = self._create_stream_with_retry(msgs)
|
||||
except Exception:
|
||||
@@ -2257,7 +2384,19 @@ class ChatSession:
|
||||
if self._generation != my_generation:
|
||||
return
|
||||
|
||||
self._update_token_table(assistant_msg)
|
||||
# Reuse the wire-bound ``msgs`` we already built for the
|
||||
# stream call instead of re-applying the reminder splice
|
||||
# (perf-2). After mark-delivered runs below, a fresh
|
||||
# _apply_reminders_for_provider would skip the
|
||||
# just-rendered reminders and undercount; passing the
|
||||
# already-rendered list keeps calibration char count
|
||||
# aligned with what the provider actually counted.
|
||||
self._update_token_table(assistant_msg, msgs=msgs)
|
||||
# Reminders that rode this stream have now reached the
|
||||
# model; flag delivered so the next provider call skips
|
||||
# them (one-shot semantics for the wire; UI replay still
|
||||
# surfaces ``_reminders`` for reconnect parity).
|
||||
self._mark_reminders_delivered()
|
||||
self._print_status_line() # Report usage for EVERY API call
|
||||
self.messages.append(assistant_msg)
|
||||
self._msg_tokens.append(
|
||||
@@ -2332,92 +2471,10 @@ class ChatSession:
|
||||
if self._generation != my_generation:
|
||||
return
|
||||
|
||||
# Repeat detection: warn when a tool is called with identical args.
|
||||
# Skip error outputs — retrying a failed tool is valid.
|
||||
# Skip JSON outputs (MCP structured results) — appending
|
||||
# text would corrupt the payload.
|
||||
_tc_by_id = {c["id"]: c for c in tool_calls}
|
||||
_repeat_detected = False
|
||||
_error_prefixes = (
|
||||
"Error",
|
||||
"JSON parse error",
|
||||
"Unknown tool",
|
||||
"Command timed out",
|
||||
"Blocked:",
|
||||
"Denied",
|
||||
)
|
||||
|
||||
# Clear dedup sigs when a write tool executed successfully —
|
||||
# the state has changed so re-running a read tool is valid.
|
||||
_write_tools = frozenset({"write_file", "edit_file", "bash"})
|
||||
if any(
|
||||
tc["function"]["name"] in _write_tools
|
||||
and not any(
|
||||
cid == tc["id"] and isinstance(out, str) and out.startswith(_error_prefixes)
|
||||
for cid, out in results
|
||||
)
|
||||
for tc in tool_calls
|
||||
):
|
||||
self._recent_tool_sigs.clear()
|
||||
for i, (tc_id, output) in enumerate(results):
|
||||
tc = _tc_by_id.get(tc_id)
|
||||
if tc and isinstance(output, str) and not output.startswith(_error_prefixes):
|
||||
raw = tc["function"]["name"] + ":" + tc["function"]["arguments"]
|
||||
sig = hashlib.sha256(raw.encode()).hexdigest()
|
||||
is_json = output.lstrip().startswith(("{", "["))
|
||||
if sig in self._recent_tool_sigs:
|
||||
_repeat_detected = True
|
||||
if not is_json:
|
||||
output += (
|
||||
"\n\n⚠ Warning: this is an identical repeat of a "
|
||||
"previous tool call. The result is the same. "
|
||||
"Try a different approach."
|
||||
)
|
||||
results[i] = (tc_id, output)
|
||||
self.ui.on_info(
|
||||
f"{GRAY}[repeat: {tc['function']['name']}() "
|
||||
f"called with same arguments]{RESET}"
|
||||
)
|
||||
self._recent_tool_sigs.add(sig)
|
||||
if _repeat_detected:
|
||||
# Reset so the model gets a clean slate after the warning.
|
||||
# If it repeats again, a new warning fires.
|
||||
self._recent_tool_sigs.clear()
|
||||
if self._mem_cfg.nudges and should_nudge(
|
||||
"repeat",
|
||||
self._metacog_state,
|
||||
message_count=len(self.messages),
|
||||
cooldown_secs=self._mem_cfg.nudge_cooldown,
|
||||
):
|
||||
self._queue_tool_advisory("repeat", format_nudge("repeat"))
|
||||
|
||||
# Tool-error nudge — checked here (pre-iteration) so the
|
||||
# MetacognitiveAdvisory rides the same _collect_advisories
|
||||
# drain pass that handles guard findings and user
|
||||
# interjections. Cooldown gating in should_nudge keeps
|
||||
# this to one nudge per batch even with many failing
|
||||
# tools.
|
||||
if (
|
||||
self._mem_cfg.nudges
|
||||
and any(
|
||||
isinstance(out, str)
|
||||
and (
|
||||
out.startswith("Error")
|
||||
or " error: " in out[:50]
|
||||
or out.startswith("Command timed out")
|
||||
or out.startswith("Unknown tool:")
|
||||
)
|
||||
for _, out in results
|
||||
)
|
||||
and should_nudge(
|
||||
"tool_error",
|
||||
self._metacog_state,
|
||||
message_count=len(self.messages),
|
||||
memory_count=self._visible_memory_count(),
|
||||
cooldown_secs=self._mem_cfg.nudge_cooldown,
|
||||
)
|
||||
):
|
||||
self._queue_tool_advisory("tool_error", format_nudge("tool_error"))
|
||||
# Repeat-detection + tool-error nudge. Mutates *results*
|
||||
# in place to inject inline warning text on identical
|
||||
# repeats; queues advisories for the next drain pass.
|
||||
self._apply_post_execute_advisories(tool_calls, results)
|
||||
|
||||
# Map tool_call_id → tool name for logging
|
||||
from turnstone.core.tool_advisory import wrap_tool_result
|
||||
@@ -2456,19 +2513,30 @@ class ChatSession:
|
||||
# Capture raw output for DB storage before advisory wrapping
|
||||
raw_output = output
|
||||
|
||||
# Advisory injection: wrap tool output with advisories
|
||||
# (output guard findings, queued user messages, etc.)
|
||||
advisories = self._collect_advisories(
|
||||
# Advisory injection: persistent advisories (output
|
||||
# guard findings, queued user interjections) wrap
|
||||
# into the tool-result envelope and stay in
|
||||
# self.messages. Metacognitive tool-channel
|
||||
# reminders (tool_error / repeat) ride a side-channel
|
||||
# — never inside content — so the model sees the
|
||||
# splice only at the wire boundary via
|
||||
# _apply_reminders_for_provider, while UI/replay
|
||||
# surfaces them as a themed bubble below the tool
|
||||
# result.
|
||||
persistent_advisories, metacog_reminders = self._collect_advisories(
|
||||
assessment, _tc_names.get(tc_id, ""), _ri == _last_idx
|
||||
)
|
||||
if isinstance(output, str):
|
||||
output = wrap_tool_result(output, advisories)
|
||||
elif isinstance(output, list) and advisories:
|
||||
output = wrap_tool_result(output, persistent_advisories)
|
||||
elif isinstance(output, list) and persistent_advisories:
|
||||
# Structured/image output — append advisories as a
|
||||
# text part so they aren't silently dropped.
|
||||
output = [
|
||||
*output,
|
||||
{"type": "text", "text": wrap_tool_result("", advisories)},
|
||||
{
|
||||
"type": "text",
|
||||
"text": wrap_tool_result("", persistent_advisories),
|
||||
},
|
||||
]
|
||||
|
||||
tool_msg: dict[str, Any] = {
|
||||
@@ -2478,6 +2546,15 @@ class ChatSession:
|
||||
}
|
||||
if self._tool_error_flags.pop(tc_id, False):
|
||||
tool_msg["is_error"] = True
|
||||
if metacog_reminders:
|
||||
tool_msg["_reminders"] = metacog_reminders
|
||||
try:
|
||||
self.ui.on_tool_reminder(metacog_reminders, tc_id)
|
||||
except Exception:
|
||||
log.warning(
|
||||
"ui.on_tool_reminder failed; reminder still attached",
|
||||
exc_info=True,
|
||||
)
|
||||
self.messages.append(tool_msg)
|
||||
|
||||
# Token estimation — image content uses a fixed heuristic
|
||||
@@ -2583,10 +2660,7 @@ class ChatSession:
|
||||
# Drain any queued user messages so they appear in the
|
||||
# conversation and are visible on the next send().
|
||||
self._flush_queued_messages()
|
||||
# Tool-channel nudges queued earlier in this generation
|
||||
# (tool_error, repeat) belong to the abandoned batch — drop
|
||||
# them so they don't bleed into the next send()'s tool loop.
|
||||
self._pending_tool_advisories.clear()
|
||||
self._drain_pending_advisories()
|
||||
# No need to clear _cancel_event — it's replaced per-generation
|
||||
# in send(), so this generation's event is simply discarded.
|
||||
self.ui.on_info("[Generation cancelled]")
|
||||
@@ -2596,15 +2670,29 @@ class ChatSession:
|
||||
except KeyboardInterrupt as exc:
|
||||
self._synthesize_cancelled_results("Interrupted by user.")
|
||||
self._flush_queued_messages()
|
||||
self._pending_tool_advisories.clear()
|
||||
self._drain_pending_advisories()
|
||||
self._record_fatal_error(exc)
|
||||
raise
|
||||
except Exception as exc:
|
||||
self._flush_queued_messages()
|
||||
self._pending_tool_advisories.clear()
|
||||
self._drain_pending_advisories()
|
||||
self._record_fatal_error(exc)
|
||||
raise
|
||||
|
||||
def _drain_pending_advisories(self) -> None:
|
||||
"""Drop both advisory channels' pending buffers.
|
||||
|
||||
Both channels are scoped to the current generation: tool-channel
|
||||
nudges (``tool_error``, ``repeat``) queued earlier in this batch
|
||||
and user-channel nudges (``correction``, ``denial``, …) queued
|
||||
during ``_check_metacognitive_nudge`` but not yet drained. When
|
||||
a generation is abandoned (cancel, KeyboardInterrupt, unexpected
|
||||
exception) both must drop so they don't bleed into the next
|
||||
send's tool loop or next user turn.
|
||||
"""
|
||||
self._pending_tool_advisories.clear()
|
||||
self._pending_user_advisories.clear()
|
||||
|
||||
def _synthesize_cancelled_results(self, reason: str) -> None:
|
||||
"""Synthesize tool_result messages for orphaned tool_calls after cancel.
|
||||
|
||||
@@ -3137,8 +3225,24 @@ class ChatSession:
|
||||
text_chars, images, doc_chars = self._msg_text_chars(msg)
|
||||
return text_chars + doc_chars + int(images * self._IMAGE_TOKENS * self._chars_per_token)
|
||||
|
||||
def _update_token_table(self, assistant_msg: dict[str, Any]) -> None:
|
||||
"""Update per-message token estimates using API usage data."""
|
||||
def _update_token_table(
|
||||
self,
|
||||
assistant_msg: dict[str, Any],
|
||||
*,
|
||||
msgs: list[dict[str, Any]] | None = None,
|
||||
) -> None:
|
||||
"""Update per-message token estimates using API usage data.
|
||||
|
||||
*msgs* (optional) is the wire-bound message list already built
|
||||
for the stream call — passing it avoids a redundant
|
||||
``_apply_reminders_for_provider`` walk and, more importantly,
|
||||
ensures the char count matches the bytes the provider counted
|
||||
even after ``_mark_reminders_delivered`` has flipped the flag
|
||||
on the reminders that rode the stream. When *msgs* is None the
|
||||
caller didn't pre-build (rare path) — fall back to applying
|
||||
the splice on the fly, but be aware the result will be
|
||||
reminder-free if delivered flags are already set.
|
||||
"""
|
||||
if not self._last_usage:
|
||||
return
|
||||
|
||||
@@ -3148,8 +3252,16 @@ class ChatSession:
|
||||
# Calibrate chars_per_token ratio from actual usage.
|
||||
# Images get a fixed token budget (subtracted). Documents
|
||||
# tokenize non-linearly depending on provider — excluded from
|
||||
# calibration so they don't skew the text ratio.
|
||||
all_msgs = self._full_messages() # system + self.messages (before append)
|
||||
# calibration so they don't skew the text ratio. ``all_msgs``
|
||||
# must reflect what the provider actually counted in
|
||||
# ``prompt_tokens``: when called from the loop with the
|
||||
# pre-built ``msgs``, that's exact; without it, fall back to
|
||||
# applying the splice fresh (post-mark-delivered the result
|
||||
# may undercount, but no caller currently takes this path
|
||||
# after a successful stream).
|
||||
all_msgs = (
|
||||
msgs if msgs is not None else self._apply_reminders_for_provider(self._full_messages())
|
||||
) # system + self.messages (before append)
|
||||
active_tools = self._get_active_tools() or []
|
||||
tool_def_chars = sum(len(json.dumps(t)) for t in active_tools)
|
||||
text_chars = 0
|
||||
@@ -3391,7 +3503,7 @@ class ChatSession:
|
||||
self.messages = [summary_user, summary_asst]
|
||||
# File contents are gone after compaction — force re-read before edit_file
|
||||
self._read_files.clear()
|
||||
self._recent_tool_sigs.clear()
|
||||
self._repeat_detector.clear()
|
||||
|
||||
# Rebuild token table
|
||||
su_tok = max(1, int(self._msg_char_count(summary_user) / self._chars_per_token))
|
||||
@@ -3780,12 +3892,26 @@ class ChatSession:
|
||||
assessment: OutputAssessment | None,
|
||||
func_name: str,
|
||||
is_last_in_batch: bool,
|
||||
) -> list[ToolAdvisory]:
|
||||
) -> tuple[list[ToolAdvisory], list[dict[str, str]]]:
|
||||
"""Gather advisories to attach to a tool result message.
|
||||
|
||||
Returns an empty list when no advisories apply (common case).
|
||||
Guard advisories attach per-result; user messages drain on the
|
||||
last result in the batch only.
|
||||
Returns ``(persistent, metacog_reminders)``:
|
||||
|
||||
- ``persistent`` — guard findings + user interjections that ride
|
||||
inside the tool-result envelope via ``wrap_tool_result``.
|
||||
These are conversation history and must persist in
|
||||
``self.messages``.
|
||||
- ``metacog_reminders`` — list of ``{"type", "text"}`` dicts for
|
||||
``tool_error`` / ``repeat`` nudges that the caller attaches to
|
||||
the tool message dict's ``_reminders`` side-channel. Like
|
||||
user-channel reminders, they are spliced into ``content`` only
|
||||
at the wire boundary by ``_apply_reminders_for_provider`` and
|
||||
surfaced separately on the UI as a themed bubble below the
|
||||
tool result.
|
||||
|
||||
Both lists are empty when no advisories apply (common case).
|
||||
Guard advisories attach per-result; user messages and
|
||||
metacognitive nudges drain on the last result in the batch only.
|
||||
"""
|
||||
from turnstone.core.tool_advisory import GuardAdvisory, UserInterjection
|
||||
|
||||
@@ -3801,26 +3927,25 @@ class ChatSession:
|
||||
if is_last_in_batch:
|
||||
self._pending_tool_advisories.clear()
|
||||
self._flush_queued_messages()
|
||||
return []
|
||||
return [], []
|
||||
|
||||
advisories: list[ToolAdvisory] = []
|
||||
persistent: list[ToolAdvisory] = []
|
||||
metacog_reminders: list[dict[str, str]] = []
|
||||
|
||||
# Output guard advisory
|
||||
# Output guard advisory — persists with the tool result.
|
||||
if assessment is not None:
|
||||
advisories.append(GuardAdvisory(assessment=assessment, func_name=func_name))
|
||||
persistent.append(GuardAdvisory(assessment=assessment, func_name=func_name))
|
||||
|
||||
# Metacognitive tool-channel drain — fires once per batch on the
|
||||
# last result. Queued by _queue_tool_advisory from the
|
||||
# tool_error and repeat detection paths just before this loop.
|
||||
# last result. Queued by _queue_tool_advisory from the
|
||||
# tool_error / repeat detection paths just before this loop.
|
||||
# Lands on the tool message dict's ``_reminders`` side-channel
|
||||
# (caller's responsibility) so it stays out of persisted content
|
||||
# and rides the wire only via the transient-copy splice.
|
||||
if is_last_in_batch and self._pending_tool_advisories:
|
||||
from turnstone.core.tool_advisory import MetacognitiveAdvisory
|
||||
|
||||
drained = list(self._pending_tool_advisories)
|
||||
self._pending_tool_advisories.clear()
|
||||
advisories.extend(
|
||||
MetacognitiveAdvisory(nudge_type=nt, message=text) for nt, text in drained
|
||||
)
|
||||
self._emit_nudge_ping(nt for nt, _ in drained)
|
||||
metacog_reminders.extend({"type": nt, "text": text} for nt, text in drained)
|
||||
|
||||
# Drain queued user messages on the last result in the batch.
|
||||
# Attachment-bearing items fall back to a full multipart user
|
||||
@@ -3834,7 +3959,7 @@ class ChatSession:
|
||||
if att_ids:
|
||||
attachment_items.append((queue_msg_id, msg, priority, att_ids))
|
||||
else:
|
||||
advisories.append(UserInterjection(message=msg, priority=priority))
|
||||
persistent.append(UserInterjection(message=msg, priority=priority))
|
||||
if attachment_items:
|
||||
from turnstone.core.tool_advisory import PRIORITY_IMPORTANT
|
||||
|
||||
@@ -3845,7 +3970,7 @@ class ChatSession:
|
||||
)
|
||||
self._append_user_turn(text, resolved, send_id=queue_msg_id)
|
||||
|
||||
return advisories
|
||||
return persistent, metacog_reminders
|
||||
|
||||
# -- Two-phase tool execution -----------------------------------------------
|
||||
#
|
||||
@@ -3974,7 +4099,14 @@ class ChatSession:
|
||||
)
|
||||
return item["call_id"], item["error"]
|
||||
if item.get("denied"):
|
||||
return item["call_id"], item.get("denial_msg", "Denied by user")
|
||||
msg = item.get("denial_msg", "Denied by user")
|
||||
self._report_tool_result(
|
||||
item["call_id"],
|
||||
item.get("func_name", "unknown"),
|
||||
msg,
|
||||
is_error=True,
|
||||
)
|
||||
return item["call_id"], msg
|
||||
try:
|
||||
result: tuple[str, str | list[dict[str, Any]]] = item["execute"](item)
|
||||
return result
|
||||
@@ -5377,59 +5509,52 @@ class ChatSession:
|
||||
def _queue_user_advisory(self, nudge_type: str, text: str) -> None:
|
||||
"""Queue a metacognitive nudge for the next user turn.
|
||||
|
||||
Drains in ``_append_user_turn`` as a ``<system-reminder>`` block
|
||||
appended to the user message body. Used for nudges that respond
|
||||
to user behaviour: ``correction``, ``denial``, ``resume``,
|
||||
Drains in ``_append_user_turn`` onto the user message dict's
|
||||
``_reminders`` side-channel. Used for nudges that respond to
|
||||
user behaviour: ``correction``, ``denial``, ``resume``,
|
||||
``start``, ``completion``.
|
||||
"""
|
||||
self._pending_user_advisories.append((nudge_type, text))
|
||||
|
||||
def _splice_pending_user_advisories(self, user_msg: dict[str, Any]) -> None:
|
||||
"""Drain ``_pending_user_advisories`` into *user_msg*'s content.
|
||||
def _attach_pending_user_reminders(self, user_msg: dict[str, Any]) -> None:
|
||||
"""Drain ``_pending_user_advisories`` onto *user_msg*'s ``_reminders``
|
||||
sibling key (a side-channel, never inside ``content``).
|
||||
|
||||
Mutates *user_msg* in place — the caller appends it after.
|
||||
Renders each queued nudge as a ``<system-reminder>`` block
|
||||
(same envelope as ``wrap_tool_result``) and attaches them to
|
||||
the trailing edge of the user content. Every text segment in
|
||||
the user content is passed through ``escape_wrapper_tags``
|
||||
first so a user typing literal ``<system-reminder>`` cannot
|
||||
fabricate an envelope the model would treat as a
|
||||
Turnstone-issued reminder. For attachment-bearing turns the
|
||||
blocks land on the trailing text part so they stay glued to
|
||||
the same multipart turn.
|
||||
Mutates *user_msg* in place — the caller appends it after. The
|
||||
rendered ``<system-reminder>`` envelope is built later in
|
||||
``_apply_reminders_for_provider`` against a transient copy, so
|
||||
the model still sees the reminder spliced into ``content`` at
|
||||
the wire boundary while ``self.messages`` and every downstream
|
||||
consumer (UI replay, compaction, title gen, channel adapters,
|
||||
DB) see clean user text.
|
||||
|
||||
``_reminders`` rides the leading-underscore convention used by
|
||||
other internal sibling metadata (``_attachments_meta``,
|
||||
``_provider_content``); ``sanitize_messages`` strips it before
|
||||
the wire on its own pass.
|
||||
|
||||
Also fires the live ``on_user_reminder`` UI hook so any open
|
||||
SSE consumers (other browser tabs, CLI mirrors, eventual
|
||||
channel adapters) can render the reminder bubble in lockstep
|
||||
with the originating tab's optimistic render. Hook failures
|
||||
are logged and swallowed: the side-channel write is the
|
||||
load-bearing op, and a UI implementation throwing here must
|
||||
not abort the user's send (which would otherwise drop both the
|
||||
user message and the queued nudges).
|
||||
"""
|
||||
if not self._pending_user_advisories:
|
||||
return
|
||||
from turnstone.core.tool_advisory import escape_wrapper_tags, render_system_reminder
|
||||
|
||||
items = list(self._pending_user_advisories)
|
||||
self._pending_user_advisories.clear()
|
||||
|
||||
block = "\n\n" + "\n\n".join(render_system_reminder(text) for _, text in items)
|
||||
content = user_msg["content"]
|
||||
if isinstance(content, str):
|
||||
user_msg["content"] = escape_wrapper_tags(content) + block
|
||||
else:
|
||||
text_parts = [p for p in content if isinstance(p, dict) and p.get("type") == "text"]
|
||||
for part in text_parts:
|
||||
part["text"] = escape_wrapper_tags(part.get("text", ""))
|
||||
if text_parts:
|
||||
text_parts[-1]["text"] = text_parts[-1]["text"] + block
|
||||
else:
|
||||
content.append({"type": "text", "text": block})
|
||||
reminders = [{"type": nudge_type, "text": text} for nudge_type, text in items]
|
||||
user_msg["_reminders"] = reminders
|
||||
|
||||
self._emit_nudge_ping(nudge_type for nudge_type, _ in items)
|
||||
|
||||
def _emit_nudge_ping(self, types: Iterable[str]) -> None:
|
||||
"""Surface the ``[metacognition: nudge injected — …]`` UI line.
|
||||
|
||||
Centralised so both drain sites (tool channel via
|
||||
``_collect_advisories``, user channel via
|
||||
``_splice_pending_user_advisories``) emit the same wording.
|
||||
"""
|
||||
joined = ", ".join(types)
|
||||
if joined:
|
||||
self.ui.on_info(f"{GRAY}[metacognition: nudge injected — {joined}]{RESET}")
|
||||
try:
|
||||
self.ui.on_user_reminder(reminders)
|
||||
except Exception:
|
||||
log.warning("ui.on_user_reminder failed; reminder still attached", exc_info=True)
|
||||
|
||||
def _queue_tool_advisory(self, nudge_type: str, text: str) -> None:
|
||||
"""Queue a metacognitive nudge for the next tool-result batch.
|
||||
@@ -5441,6 +5566,95 @@ class ChatSession:
|
||||
"""
|
||||
self._pending_tool_advisories.append((nudge_type, text))
|
||||
|
||||
def _apply_post_execute_advisories(
|
||||
self,
|
||||
tool_calls: list[dict[str, Any]],
|
||||
results: list[tuple[str, str | list[dict[str, Any]]]],
|
||||
) -> None:
|
||||
"""Run repeat detection + tool-error nudge over a freshly-executed batch.
|
||||
|
||||
Mutates *results* in place when an identical-repeat warning is
|
||||
appended to a tool's text output. Updates ``self._repeat_detector``,
|
||||
``self._pending_tool_advisories``, and ``self._metacog_state``
|
||||
(cooldown timestamp via ``should_nudge``). The operator-visible
|
||||
signal is the themed ``tool_reminder`` bubble below the tool
|
||||
block — emitted by the per-result loop downstream when the
|
||||
drained metacog reminders attach to the tool message dict's
|
||||
``_reminders`` side-channel.
|
||||
|
||||
Repeat detection's job is to nudge a flaky local model out of a
|
||||
loop where it keeps making the same tool call ("``bash(cmd='echo
|
||||
test')`` × 3" being the canonical example). It fires on the
|
||||
consecutive-streak signal alone, with no regard for the tool's
|
||||
success / failure / output content — same (name, args) for N
|
||||
turns in a row is by definition stuck. ``RepeatDetector.record``
|
||||
already resets the streak on any different signature, so an
|
||||
intervening tool call (read, write, anything different) breaks
|
||||
the streak naturally without an explicit clear here.
|
||||
|
||||
``_tool_error_flags`` is the authoritative is_error signal —
|
||||
consumed below for the tool-error nudge gate; the per-result
|
||||
loop in ``_run_loop`` ``.pop``s it after this returns.
|
||||
"""
|
||||
# Repeat detection: warn when a tool is called with identical
|
||||
# args N times in a row. Independent of success/failure — the
|
||||
# stuck-loop pattern is sig-driven, not state-driven. JSON
|
||||
# outputs (MCP structured results) are tracked but exempt from
|
||||
# the inline warning text (appending text would corrupt the
|
||||
# payload).
|
||||
_tc_by_id = {c["id"]: c for c in tool_calls}
|
||||
_repeat_detected = False
|
||||
|
||||
for i, (tc_id, output) in enumerate(results):
|
||||
tc = _tc_by_id.get(tc_id)
|
||||
if tc and isinstance(output, str):
|
||||
raw = tc["function"]["name"] + ":" + tc["function"]["arguments"]
|
||||
sig = hashlib.sha256(raw.encode()).hexdigest()
|
||||
is_json = output.lstrip().startswith(("{", "["))
|
||||
if self._repeat_detector.record(sig):
|
||||
_repeat_detected = True
|
||||
if not is_json:
|
||||
output += (
|
||||
"\n\n⚠ Warning: this is an identical repeat of a "
|
||||
"previous tool call. The result is the same. "
|
||||
"Try a different approach."
|
||||
)
|
||||
results[i] = (tc_id, output)
|
||||
# The themed ``tool_reminder`` bubble below the tool
|
||||
# block carries the operator-visible signal; the
|
||||
# tool-name context comes from the visible tool
|
||||
# block immediately above the bubble, so a separate
|
||||
# diagnostic info line would just duplicate it.
|
||||
|
||||
if _repeat_detected:
|
||||
# Reset so the model gets a clean slate after the warning.
|
||||
# If it repeats again, a new warning fires.
|
||||
self._repeat_detector.clear()
|
||||
if self._mem_cfg.nudges and should_nudge(
|
||||
"repeat",
|
||||
self._metacog_state,
|
||||
message_count=len(self.messages),
|
||||
cooldown_secs=self._mem_cfg.nudge_cooldown,
|
||||
):
|
||||
self._queue_tool_advisory("repeat", format_nudge("repeat"))
|
||||
|
||||
# Tool-error nudge — queued so the MetacognitiveAdvisory rides
|
||||
# the same _collect_advisories drain pass as guard findings and
|
||||
# user interjections. Cooldown gating in should_nudge keeps
|
||||
# this to one nudge per batch even with many failing tools.
|
||||
if (
|
||||
self._mem_cfg.nudges
|
||||
and any(self._tool_error_flags.get(tc_id) for tc_id, _ in results)
|
||||
and should_nudge(
|
||||
"tool_error",
|
||||
self._metacog_state,
|
||||
message_count=len(self.messages),
|
||||
memory_count=self._visible_memory_count(),
|
||||
cooldown_secs=self._mem_cfg.nudge_cooldown,
|
||||
)
|
||||
):
|
||||
self._queue_tool_advisory("tool_error", format_nudge("tool_error"))
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# Coordinator tools — reachable only when ``kind == "coordinator"``.
|
||||
# All six dispatch through ``self._coord_client`` which is None when
|
||||
@@ -9203,7 +9417,7 @@ class ChatSession:
|
||||
elif cmd == "/clear":
|
||||
self.messages.clear()
|
||||
self._read_files.clear()
|
||||
self._recent_tool_sigs.clear()
|
||||
self._repeat_detector.clear()
|
||||
self._last_usage = None
|
||||
self._calibrated_msg_count = 0
|
||||
self._msg_tokens = []
|
||||
@@ -9214,7 +9428,7 @@ class ChatSession:
|
||||
|
||||
self.messages.clear()
|
||||
self._read_files.clear()
|
||||
self._recent_tool_sigs.clear()
|
||||
self._repeat_detector.clear()
|
||||
self._last_usage = None
|
||||
self._calibrated_msg_count = 0
|
||||
self._msg_tokens = []
|
||||
|
||||
@@ -13,6 +13,7 @@ import contextlib
|
||||
import threading
|
||||
import time
|
||||
import uuid
|
||||
from datetime import UTC, datetime, timedelta
|
||||
from typing import TYPE_CHECKING, Any, Protocol
|
||||
|
||||
from turnstone.core.log import get_logger
|
||||
@@ -28,6 +29,22 @@ if TYPE_CHECKING:
|
||||
log = get_logger(__name__)
|
||||
|
||||
|
||||
# Maps each workstream kind to the ``services.service_type`` its hosting
|
||||
# process registers under. Used by ``SessionManager.close_idle`` pass 2
|
||||
# to enumerate live peer processes for orphan-reaper liveness scoping.
|
||||
# Server processes register as ``("server", node_id, ...)`` (see
|
||||
# ``turnstone/server.py``); the console process as ``("console",
|
||||
# "console", ...)`` (see ``turnstone/console/server.py``). Deriving from
|
||||
# kind here removes a duplicated-config footgun: any caller that builds
|
||||
# a ``SessionManager`` automatically gets the correct service_type for
|
||||
# its kind, with no risk of miswiring INTERACTIVE→"console" or vice
|
||||
# versa.
|
||||
_KIND_SERVICE_TYPE: dict[WorkstreamKind, str] = {
|
||||
WorkstreamKind.INTERACTIVE: "server",
|
||||
WorkstreamKind.COORDINATOR: "console",
|
||||
}
|
||||
|
||||
|
||||
class SessionKindAdapter(Protocol):
|
||||
"""Per-kind construction + cleanup policies the shared ``SessionManager`` delegates to.
|
||||
|
||||
@@ -217,6 +234,16 @@ class SessionManager:
|
||||
def kind(self) -> WorkstreamKind:
|
||||
return self._adapter.kind
|
||||
|
||||
@property
|
||||
def _service_type(self) -> str | None:
|
||||
"""``services.service_type`` this manager's hosting process registers
|
||||
under, derived from its ``kind``. Used by ``close_idle`` pass 2 to
|
||||
enumerate live peer processes. Returns ``None`` for kinds that have
|
||||
no production service mapping (only the two existing kinds map
|
||||
today; ``None`` would be a marker for a future kind without a
|
||||
clustered hosting model)."""
|
||||
return _KIND_SERVICE_TYPE.get(self.kind)
|
||||
|
||||
@property
|
||||
def count(self) -> int:
|
||||
with self._lock:
|
||||
@@ -604,6 +631,22 @@ class SessionManager:
|
||||
# last close(). The next set_state() call syncs it
|
||||
# naturally; writing 'idle' here could race a concurrent
|
||||
# close() that writes 'closed' under self._lock.
|
||||
#
|
||||
# Bump only ``updated`` (no state write) so this row's
|
||||
# timestamp is fresh against the orphan-reaper cutoff —
|
||||
# otherwise a concurrent close_idle pass-2 in this same
|
||||
# process could clobber a freshly-rehydrated row whose
|
||||
# ``updated`` is older than the cutoff. The pure-
|
||||
# timestamp write is safe against concurrent close()
|
||||
# because close still wins on the state column.
|
||||
try:
|
||||
self._storage.touch_workstream(ws_id)
|
||||
except Exception:
|
||||
log.debug(
|
||||
"session_mgr.touch_workstream_failed ws=%s",
|
||||
ws_id[:8],
|
||||
exc_info=True,
|
||||
)
|
||||
if self._event_emitter is not None:
|
||||
self._event_emitter.emit_rehydrated(ws)
|
||||
return ws
|
||||
@@ -804,15 +847,52 @@ class SessionManager:
|
||||
def close_idle(self, max_age_seconds: float) -> list[str]:
|
||||
"""Close IDLE workstreams inactive for more than ``max_age_seconds``.
|
||||
|
||||
Returns the list of closed ws_ids. Unlike the old WSM version,
|
||||
this does NOT skip the last workstream — the default-startup
|
||||
relic is gone, callers can handle the 0-workstream case.
|
||||
Two-pass shape:
|
||||
|
||||
- Pass 1 (in-memory): close loaded ``IDLE`` rows whose
|
||||
``ws.last_active`` (monotonic) is past timeout. Closes only
|
||||
``IDLE`` so legitimately-attentive rows (waiting for user
|
||||
response) stay live.
|
||||
- Pass 2 (DB orphans): bulk-close DB rows of this manager's
|
||||
kind whose ``updated`` is past the wall-clock cutoff and
|
||||
which are not currently loaded. This catches workstreams
|
||||
left behind by prior process incarnations — a process crash
|
||||
/restart leaves rows in non-terminal states forever
|
||||
otherwise. Closes ``idle/thinking/attention/running``
|
||||
because any matching row is by definition not loaded by any
|
||||
live process and cannot be in a live interaction.
|
||||
|
||||
**Liveness scoping** (the rendezvous router's primitive
|
||||
since PR #384): when ``self._service_type`` resolves to a
|
||||
known service type — both production kinds do — pass 2
|
||||
calls ``storage.list_services`` to enumerate peer processes
|
||||
with recent heartbeats and protects rows whose ``node_id``
|
||||
matches a live ``service_id`` from reap, even when *this*
|
||||
manager is on a different node. This is essential for
|
||||
containerized deployments with dynamic hostnames: dead-pod
|
||||
rows fall out of the live set after the heartbeat window
|
||||
and become reapable; alive-pod rows stay protected as long
|
||||
as the owner heartbeats. A future kind with no service
|
||||
registration would resolve ``_service_type`` to ``None``
|
||||
and skip the live-services lookup (single-process / CLI).
|
||||
|
||||
**Conservative fallback**: if ``list_services`` raises,
|
||||
pass 2 is skipped entirely this tick — never reap when
|
||||
liveness state is unknown. Pass 1 still runs. Next tick
|
||||
retries the lookup.
|
||||
|
||||
Returns the combined list of closed ws_ids (in-memory first,
|
||||
then DB orphans). Pass 1 emits ``ws_closed``; pass 2 does
|
||||
not, because never-loaded rows have no SSE listeners
|
||||
expecting them.
|
||||
|
||||
Atomic pop per victim under ``self._lock`` (bug-5): a pending
|
||||
tool result can flip state IDLE→RUNNING between the snapshot
|
||||
and the close, so the state test + pop must run together.
|
||||
Batches every pop under one ``self._lock`` acquisition (perf-5)
|
||||
rather than locking once per victim.
|
||||
rather than locking once per victim. The DB pass runs OUTSIDE
|
||||
``self._lock`` — only a brief lock to snapshot loaded keys —
|
||||
so a slow UPDATE doesn't block create/get/set_state.
|
||||
"""
|
||||
now = time.monotonic()
|
||||
popped: list[Workstream] = []
|
||||
@@ -853,6 +933,63 @@ class SessionManager:
|
||||
if self._event_emitter is not None:
|
||||
self._event_emitter.emit_closed(ws.id, name=ws.name)
|
||||
closed_ids.append(ws.id)
|
||||
|
||||
# Pass 2: reap DB orphans of this kind older than the cutoff.
|
||||
# Snapshot loaded keys under self._lock briefly so a concurrent
|
||||
# create/load doesn't get its row clobbered by the UPDATE; release
|
||||
# before the DB call.
|
||||
#
|
||||
# Liveness scoping uses ``services.last_heartbeat`` — the same
|
||||
# primitive the rendezvous router (PR #384) uses for routing. A
|
||||
# row's ``node_id`` is stamped at create time and never updated;
|
||||
# in containerized deployments with dynamic hostnames the dead
|
||||
# pod's ``node_id`` points at a service that's no longer
|
||||
# heartbeating, so the row falls through to reap. Conversely,
|
||||
# rows whose ``node_id`` matches a heartbeating service are
|
||||
# protected even when *this* manager is on a different node —
|
||||
# the alive peer may legitimately have them loaded.
|
||||
with self._lock:
|
||||
loaded = list(self._workstreams.keys())
|
||||
cutoff = (datetime.now(UTC) - timedelta(seconds=max_age_seconds)).strftime(
|
||||
"%Y-%m-%dT%H:%M:%S"
|
||||
)
|
||||
live_node_ids: list[str] | None = None
|
||||
skip_pass_2 = False
|
||||
if self._service_type is not None:
|
||||
try:
|
||||
live_services = self._storage.list_services(self._service_type)
|
||||
live_node_ids = [
|
||||
str(svc["service_id"]) for svc in live_services if svc.get("service_id")
|
||||
]
|
||||
except Exception:
|
||||
# Conservative fallback: skip pass 2 entirely this tick
|
||||
# so we can't accidentally reap rows whose owners we
|
||||
# failed to enumerate. Next tick retries.
|
||||
log.debug(
|
||||
"session_mgr.list_services_failed kind=%s",
|
||||
self.kind.value,
|
||||
exc_info=True,
|
||||
)
|
||||
skip_pass_2 = True
|
||||
orphans: list[str] = []
|
||||
if not skip_pass_2:
|
||||
try:
|
||||
orphans = self._storage.bulk_close_stale_orphans(
|
||||
self.kind, cutoff, loaded, live_node_ids=live_node_ids
|
||||
)
|
||||
except Exception:
|
||||
log.debug(
|
||||
"session_mgr.bulk_close_orphans_failed kind=%s",
|
||||
self.kind.value,
|
||||
exc_info=True,
|
||||
)
|
||||
if orphans:
|
||||
log.info(
|
||||
"session_mgr.bulk_close_orphans count=%d kind=%s",
|
||||
len(orphans),
|
||||
self.kind.value,
|
||||
)
|
||||
closed_ids.extend(orphans)
|
||||
return closed_ids
|
||||
|
||||
def _close_if_idle_locked(self, ws_id: str) -> Workstream | None:
|
||||
|
||||
@@ -1293,6 +1293,42 @@ class SessionUIBase:
|
||||
def on_error(self, message: str) -> None:
|
||||
self._enqueue({"type": "error", "message": message})
|
||||
|
||||
def on_user_reminder(self, reminders: list[dict[str, str]]) -> None:
|
||||
"""Surface a metacognitive user-channel nudge as its own UI
|
||||
element.
|
||||
|
||||
Reminders live on the user message dict's ``_reminders``
|
||||
side-channel and are spliced into ``content`` only at the
|
||||
provider boundary; this event is what lets every connected
|
||||
SSE consumer (other browser tabs, CLI mirrors, future channel
|
||||
adapters) render the reminder bubble in lockstep with the
|
||||
originating tab. The history-replay path surfaces the same
|
||||
shape via ``_build_history`` so a tab reconnecting later
|
||||
renders the same bubble.
|
||||
"""
|
||||
self._enqueue({"type": "user_reminder", "reminders": reminders})
|
||||
|
||||
def on_tool_reminder(self, reminders: list[dict[str, str]], tool_call_id: str) -> None:
|
||||
"""Surface a metacognitive tool-channel nudge (``tool_error`` /
|
||||
``repeat``) as its own UI element below the tool result that
|
||||
triggered it.
|
||||
|
||||
Tool-channel reminders ride the same ``_reminders``
|
||||
side-channel pattern as the user channel — kept out of
|
||||
``content`` so compaction / title-gen / channel adapters never
|
||||
see the nudge text, spliced into the wire only via
|
||||
``_apply_reminders_for_provider``. ``tool_call_id`` is the
|
||||
anchor the frontend uses to render the bubble below the
|
||||
specific tool result that triggered the batch's reminder.
|
||||
"""
|
||||
self._enqueue(
|
||||
{
|
||||
"type": "tool_reminder",
|
||||
"reminders": reminders,
|
||||
"tool_call_id": tool_call_id,
|
||||
}
|
||||
)
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# Broadcast hooks — kind-specific transport.
|
||||
#
|
||||
|
||||
@@ -97,7 +97,7 @@ from turnstone.core.storage._utils import sanitize_text
|
||||
from turnstone.core.storage._utils import (
|
||||
scan_skill_content as _scan_skill_content,
|
||||
)
|
||||
from turnstone.core.workstream import WorkstreamKind
|
||||
from turnstone.core.workstream import BULK_CLOSE_STATE_VALUES, WorkstreamKind
|
||||
|
||||
log = get_logger(__name__)
|
||||
|
||||
@@ -595,6 +595,57 @@ class PostgreSQLBackend:
|
||||
)
|
||||
conn.commit()
|
||||
|
||||
def bulk_close_stale_orphans(
|
||||
self,
|
||||
kind: WorkstreamKind | str,
|
||||
cutoff: str,
|
||||
exclude_ws_ids: list[str],
|
||||
live_node_ids: list[str] | None = None,
|
||||
) -> list[str]:
|
||||
norm_kind = WorkstreamKind(kind).value
|
||||
now = datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%S")
|
||||
stmt = (
|
||||
sa.update(workstreams)
|
||||
.where(
|
||||
workstreams.c.kind == norm_kind,
|
||||
workstreams.c.state.in_(BULK_CLOSE_STATE_VALUES),
|
||||
workstreams.c.updated < cutoff,
|
||||
)
|
||||
.values(state="closed", updated=now)
|
||||
.returning(workstreams.c.ws_id)
|
||||
)
|
||||
# Protect rows whose owning process is still heartbeating in the
|
||||
# services table (rendezvous router's liveness primitive). NULL
|
||||
# node_id rows have no owner identity — always eligible. The
|
||||
# ``and live_node_ids`` short-circuits both ``None`` (skip the
|
||||
# filter entirely — single-process / operator backfill) and ``[]``
|
||||
# (no nodes alive — every row unprotected, no extra predicate
|
||||
# needed since absence equals match-all).
|
||||
if live_node_ids is not None and live_node_ids:
|
||||
stmt = stmt.where(
|
||||
sa.or_(
|
||||
workstreams.c.node_id.is_(None),
|
||||
~workstreams.c.node_id.in_(live_node_ids),
|
||||
)
|
||||
)
|
||||
if exclude_ws_ids:
|
||||
# Skip ``NOT IN ()`` when nothing to exclude — keeps the SQL clean
|
||||
# and avoids SQLAlchemy's empty-collection warning.
|
||||
stmt = stmt.where(~workstreams.c.ws_id.in_(exclude_ws_ids))
|
||||
with self._conn() as conn:
|
||||
result = conn.execute(stmt)
|
||||
ids = [row[0] for row in result]
|
||||
conn.commit()
|
||||
return ids
|
||||
|
||||
def touch_workstream(self, ws_id: str) -> None:
|
||||
now = datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%S")
|
||||
with self._conn() as conn:
|
||||
conn.execute(
|
||||
sa.update(workstreams).where(workstreams.c.ws_id == ws_id).values(updated=now)
|
||||
)
|
||||
conn.commit()
|
||||
|
||||
def update_workstream_name(self, ws_id: str, name: str) -> None:
|
||||
now = datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%S")
|
||||
with self._conn() as conn:
|
||||
|
||||
@@ -431,6 +431,68 @@ class StorageBackend(Protocol):
|
||||
"""Update a workstream's state and bump updated timestamp."""
|
||||
...
|
||||
|
||||
def bulk_close_stale_orphans(
|
||||
self,
|
||||
kind: WorkstreamKind | str,
|
||||
cutoff: str,
|
||||
exclude_ws_ids: list[str],
|
||||
live_node_ids: list[str] | None = None,
|
||||
) -> list[str]:
|
||||
"""Close DB-side workstream rows of *kind* whose state is in
|
||||
``BULK_CLOSE_STATE_VALUES`` and whose ``updated`` is lex-older than
|
||||
*cutoff*, excluding rows currently loaded in memory. Sets
|
||||
``state='closed'`` and bumps ``updated``. Returns the list of ws_ids
|
||||
actually transitioned.
|
||||
|
||||
``cutoff`` is a UTC ``YYYY-MM-DDTHH:MM:SS`` string matching the on-disk
|
||||
format ``update_workstream_state`` writes — lex compare is safe for
|
||||
same-offset timestamps. Empty ``exclude_ws_ids`` means no exclusion.
|
||||
|
||||
``live_node_ids`` is the set of ``services.service_id`` values whose
|
||||
``last_heartbeat`` is recent (i.e. owning processes still alive);
|
||||
rows whose ``node_id`` matches one of these are protected because
|
||||
their owning process may legitimately have them loaded on another
|
||||
worker. ``None`` skips the filter entirely (single-process / tests
|
||||
/ operator backfill). Empty list ``[]`` treats every node as dead —
|
||||
useful when operator scripts want to reap regardless of liveness.
|
||||
|
||||
Rows with ``NULL`` ``node_id`` are always eligible: they have no
|
||||
meaningful owner identity, so age alone gates the reap.
|
||||
|
||||
Liveness scoping replaces an earlier ``node_id == self`` heuristic.
|
||||
That heuristic broke in the post-rendezvous-routing world (PR #384):
|
||||
``workstreams.node_id`` is stamped at create time and never updated,
|
||||
so dead-pod orphans in containerized deployments with dynamic
|
||||
hostnames couldn't be reclaimed. ``services.last_heartbeat`` is the
|
||||
rendezvous router's authoritative liveness primitive — using it here
|
||||
keeps reap scoping aligned with routing.
|
||||
|
||||
Asymmetric with ``SessionManager.close_idle``'s in-memory pass on
|
||||
purpose: that pass closes only ``IDLE`` (legitimately-attentive rows
|
||||
stay), this method closes the broader ``BULK_CLOSE_STATE_VALUES`` set
|
||||
because any row matching here is by definition not loaded by any
|
||||
live process and cannot be in a live interaction.
|
||||
"""
|
||||
...
|
||||
|
||||
def touch_workstream(self, ws_id: str) -> None:
|
||||
"""Bump a workstream row's ``updated`` timestamp without touching its
|
||||
state.
|
||||
|
||||
Used by ``SessionManager.open()`` on cold rehydrate so a freshly-
|
||||
loaded row's ``updated`` can't be older than the orphan-reaper cutoff
|
||||
— protects against a same-process race where a parallel
|
||||
``close_idle`` pass-2 snapshots loaded keys after the storage read
|
||||
but before the in-memory install. Distinct from
|
||||
``update_workstream_state(ws_id, current_state)`` because the
|
||||
rehydrate path explicitly avoids a state write (see the
|
||||
``open()`` no-DB-state-flip-on-resurrect comment): a state write
|
||||
could race a concurrent ``close()`` and resurrect a closed row.
|
||||
Bumping only ``updated`` is safe — close still wins on the state
|
||||
column.
|
||||
"""
|
||||
...
|
||||
|
||||
def update_workstream_name(self, ws_id: str, name: str) -> None:
|
||||
"""Update a workstream's display name."""
|
||||
...
|
||||
|
||||
@@ -97,7 +97,7 @@ from turnstone.core.storage._utils import sanitize_text
|
||||
from turnstone.core.storage._utils import (
|
||||
scan_skill_content as _scan_skill_content,
|
||||
)
|
||||
from turnstone.core.workstream import WorkstreamKind
|
||||
from turnstone.core.workstream import BULK_CLOSE_STATE_VALUES, WorkstreamKind
|
||||
|
||||
log = get_logger(__name__)
|
||||
|
||||
@@ -707,6 +707,87 @@ class SQLiteBackend:
|
||||
)
|
||||
conn.commit()
|
||||
|
||||
def bulk_close_stale_orphans(
|
||||
self,
|
||||
kind: WorkstreamKind | str,
|
||||
cutoff: str,
|
||||
exclude_ws_ids: list[str],
|
||||
live_node_ids: list[str] | None = None,
|
||||
) -> list[str]:
|
||||
norm_kind = WorkstreamKind(kind).value
|
||||
now = datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%S")
|
||||
# SQLite has no RETURNING precedent in this file — do SELECT-then-
|
||||
# UPDATE in one transaction, with the SAME WHERE predicates re-applied
|
||||
# to the UPDATE. Re-application defends against a same-process race:
|
||||
# ``SessionManager.open()`` calls ``touch_workstream`` between the
|
||||
# SELECT and the UPDATE could have bumped a row's ``updated`` past
|
||||
# ``cutoff`` (or ``set_state`` could have flipped its state out of
|
||||
# the bulk-close set). Without the re-applied WHERE the UPDATE
|
||||
# closes those rows anyway; with it, the UPDATE skips rows that
|
||||
# became ineligible after the SELECT and the row stays open.
|
||||
# Chunked through ``_in_chunks`` so the ``IN`` clause never exceeds
|
||||
# SQLite's bind-parameter limit (default 999) on a large reap.
|
||||
candidate_conditions = [
|
||||
workstreams.c.kind == norm_kind,
|
||||
workstreams.c.state.in_(BULK_CLOSE_STATE_VALUES),
|
||||
workstreams.c.updated < cutoff,
|
||||
]
|
||||
if live_node_ids is not None and live_node_ids:
|
||||
# Protect rows owned by heartbeating services. NULL node_id is
|
||||
# always eligible. Empty list means "no nodes alive" — every
|
||||
# row is unprotected; the absence of this predicate is
|
||||
# equivalent to "match all," so we just skip it.
|
||||
candidate_conditions.append(
|
||||
sa.or_(
|
||||
workstreams.c.node_id.is_(None),
|
||||
~workstreams.c.node_id.in_(live_node_ids),
|
||||
)
|
||||
)
|
||||
if exclude_ws_ids:
|
||||
candidate_conditions.append(~workstreams.c.ws_id.in_(exclude_ws_ids))
|
||||
select_stmt = sa.select(workstreams.c.ws_id).where(*candidate_conditions)
|
||||
closed: list[str] = []
|
||||
# Match the chunk size used by ``prune_workstreams`` (line 453) — keeps
|
||||
# ``IN`` clauses well below SQLite's default 999-bind-param limit even
|
||||
# on very large reaps.
|
||||
chunk_size = 500
|
||||
with self._conn() as conn:
|
||||
candidate_ids = [row[0] for row in conn.execute(select_stmt)]
|
||||
for i in range(0, len(candidate_ids), chunk_size):
|
||||
chunk = candidate_ids[i : i + chunk_size]
|
||||
# Re-apply the eligibility predicates on the UPDATE so a row
|
||||
# that became fresh between the SELECT and the UPDATE is not
|
||||
# clobbered. Then SELECT back by ``state='closed' AND updated=now``
|
||||
# to determine which rows actually transitioned this commit —
|
||||
# the returned list reflects reality even when re-application
|
||||
# filters out some candidates.
|
||||
conn.execute(
|
||||
sa.update(workstreams)
|
||||
.where(workstreams.c.ws_id.in_(chunk), *candidate_conditions)
|
||||
.values(state="closed", updated=now)
|
||||
)
|
||||
actually_closed = [
|
||||
row[0]
|
||||
for row in conn.execute(
|
||||
sa.select(workstreams.c.ws_id).where(
|
||||
workstreams.c.ws_id.in_(chunk),
|
||||
workstreams.c.state == "closed",
|
||||
workstreams.c.updated == now,
|
||||
)
|
||||
)
|
||||
]
|
||||
closed.extend(actually_closed)
|
||||
conn.commit()
|
||||
return closed
|
||||
|
||||
def touch_workstream(self, ws_id: str) -> None:
|
||||
now = datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%S")
|
||||
with self._conn() as conn:
|
||||
conn.execute(
|
||||
sa.update(workstreams).where(workstreams.c.ws_id == ws_id).values(updated=now)
|
||||
)
|
||||
conn.commit()
|
||||
|
||||
def update_workstream_name(self, ws_id: str, name: str) -> None:
|
||||
now = datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%S")
|
||||
with self._conn() as conn:
|
||||
|
||||
@@ -0,0 +1,88 @@
|
||||
"""Add a partial composite index for the orphan-reaper query.
|
||||
|
||||
``StorageBackend.bulk_close_stale_orphans`` (introduced alongside the
|
||||
workstream-lifecycle leak fix) runs every ``min(300s, idle_timeout/4)``
|
||||
on every server and console process. Its WHERE shape is:
|
||||
|
||||
WHERE kind = ?
|
||||
AND state IN ('idle', 'thinking', 'attention', 'running')
|
||||
AND updated < ?
|
||||
AND (node_id IS NULL OR node_id NOT IN (alive_service_ids))
|
||||
|
||||
At current scale (low-thousands of workstream rows) the existing single-
|
||||
column indexes are sufficient — ``idx_workstreams_state`` prunes to the
|
||||
non-closed subset, and the planner filters the rest sequentially. At
|
||||
100k+ rows that filter becomes a tablescan-shaped cost on the reaper's
|
||||
periodic run.
|
||||
|
||||
A **partial** index covering only ``BULK_CLOSE_STATE_VALUES`` rows
|
||||
matches the reaper's query exactly while staying tiny — closed rows
|
||||
(typically 95%+ of the table per empirical diagnosis) and ``error``
|
||||
rows are excluded, so the index is roughly 5% the size a full multi-
|
||||
column index would be. Write amplification only kicks in for
|
||||
transitions that touch one of the four covered states.
|
||||
|
||||
Column order ``(kind, updated)``:
|
||||
|
||||
- ``kind`` first because the reaper always supplies it as an equality
|
||||
predicate; partitions the partial index into interactive vs
|
||||
coordinator subtrees.
|
||||
- ``updated`` last so the range comparison rides the trailing column —
|
||||
classic composite-index pattern for ``WHERE eq AND range``.
|
||||
|
||||
``node_id`` is intentionally NOT in the index. The reaper's predicate
|
||||
on it is ``NOT IN (small list)`` against an unbounded-cardinality
|
||||
column, which planners don't index well; including it would just add
|
||||
write cost for negligible read benefit.
|
||||
|
||||
PostgreSQL uses ``CREATE INDEX CONCURRENTLY`` so the build is
|
||||
non-blocking on a live system; SQLite has no concurrent concept and
|
||||
the table-level write lock already serializes, so a plain
|
||||
``CREATE INDEX`` is fine.
|
||||
|
||||
Revision ID: 048
|
||||
Revises: 047
|
||||
Create Date: 2026-04-30
|
||||
"""
|
||||
|
||||
import sqlalchemy as sa
|
||||
from alembic import op
|
||||
|
||||
revision = "048"
|
||||
down_revision = "047"
|
||||
branch_labels = None
|
||||
depends_on = None
|
||||
|
||||
|
||||
_REAPER_PARTIAL_WHERE = "state IN ('idle', 'thinking', 'attention', 'running')"
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
bind = op.get_bind()
|
||||
dialect = bind.dialect.name
|
||||
|
||||
if dialect == "postgresql":
|
||||
with op.get_context().autocommit_block():
|
||||
op.execute(
|
||||
"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_workstreams_reaper "
|
||||
"ON workstreams (kind, updated) "
|
||||
f"WHERE {_REAPER_PARTIAL_WHERE}"
|
||||
)
|
||||
else:
|
||||
op.create_index(
|
||||
"idx_workstreams_reaper",
|
||||
"workstreams",
|
||||
["kind", "updated"],
|
||||
sqlite_where=sa.text(_REAPER_PARTIAL_WHERE),
|
||||
)
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
bind = op.get_bind()
|
||||
dialect = bind.dialect.name
|
||||
|
||||
if dialect == "postgresql":
|
||||
with op.get_context().autocommit_block():
|
||||
op.execute("DROP INDEX CONCURRENTLY IF EXISTS idx_workstreams_reaper")
|
||||
else:
|
||||
op.drop_index("idx_workstreams_reaper", table_name="workstreams")
|
||||
@@ -77,6 +77,25 @@ class WorkstreamState(enum.Enum):
|
||||
ERROR = "error" # last operation failed
|
||||
|
||||
|
||||
# States the orphan reaper (``SessionManager.close_idle`` pass 2 +
|
||||
# ``StorageBackend.bulk_close_stale_orphans``) is allowed to flip to
|
||||
# ``closed`` for rows past the staleness cutoff. Excludes ``ERROR``
|
||||
# deliberately — error rows are user-investigatable and shouldn't be
|
||||
# auto-reaped — and excludes ``CLOSED`` (terminal). Centralized here
|
||||
# so the storage backends and FakeStorage all agree; if a new transient
|
||||
# state is added to ``WorkstreamState``, deciding whether it joins
|
||||
# this set is part of the change rather than an after-the-fact
|
||||
# audit across three files.
|
||||
BULK_CLOSE_STATE_VALUES: frozenset[str] = frozenset(
|
||||
{
|
||||
WorkstreamState.IDLE.value,
|
||||
WorkstreamState.THINKING.value,
|
||||
WorkstreamState.RUNNING.value,
|
||||
WorkstreamState.ATTENTION.value,
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Workstream dataclass
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -139,6 +139,12 @@ class NullUI:
|
||||
def on_error(self, message: str) -> None:
|
||||
pass
|
||||
|
||||
def on_user_reminder(self, reminders: list[dict[str, str]]) -> None:
|
||||
pass
|
||||
|
||||
def on_tool_reminder(self, reminders: list[dict[str, str]], tool_call_id: str) -> None:
|
||||
pass
|
||||
|
||||
def on_state_change(self, state: str) -> None:
|
||||
pass
|
||||
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
You are a coordinator on a small, focused infrastructure team. Your role is to orchestrate work across the cluster: you decompose a user's request into tasks, spawn child workstreams on appropriate nodes with the right skills, monitor their progress, synthesise their results, and surface the outcome back to the user.
|
||||
|
||||
You do not edit files, run shell commands, browse the web, or manipulate the codebase directly. Children do that. Your job is to pick the right child, give it a well-formed brief, and keep the plan coherent while multiple children run in parallel.
|
||||
You do not edit files, run shells, or browse the web — children do. You pick the right child, give a well-formed brief, and keep the plan coherent while multiple children run.
|
||||
|
||||
You think in plans: a tasks entry, a child to own it, a way to know when it's done. When a child reports back, you read what it said, decide whether the goal is met, and either close it out, push a follow-up message, or spawn another child to cover the gap.
|
||||
You think in plans: enumerate the independent units of work, spawn one child per unit, run them in parallel by default. Sequential only when one child's output feeds the next. When a child reports back, you decide whether the goal is met, then close it out, push a follow-up, or spawn another child to cover the gap.
|
||||
|
||||
You are precise about what you delegate. A child gets the minimum context it needs — skill, initial_message, maybe a node_id. You don't paste whole files into its prompt; children have their own tools for that.
|
||||
|
||||
|
||||
@@ -1,7 +1,5 @@
|
||||
TOOL PATTERNS:
|
||||
|
||||
You are a coordinator. You do not edit files, run shell commands, or browse the web directly. You delegate work by spawning child workstreams on cluster nodes, monitoring their progress, and synthesising their results. Every tool below is in your schema; nothing else is.
|
||||
|
||||
Discover available capacity → list_nodes / list_skills:
|
||||
list_nodes(filters={'capability': 'gpu'})
|
||||
list_skills(category='engineering')
|
||||
@@ -10,11 +8,11 @@ Delegate a task → spawn_workstream:
|
||||
spawn_workstream(initial_message='audit auth.py for CSRF handling', name='csrf-audit')
|
||||
spawn_workstream(initial_message='compare FastAPI vs Starlette for async websockets', target_node='flat-blck-io_43a3')
|
||||
|
||||
Fan out to multiple children in one approval → spawn_batch (up to 10):
|
||||
Fan out across independent inputs → spawn_batch:
|
||||
spawn_batch(children=[
|
||||
{'initial_message': 'benchmark A'},
|
||||
{'initial_message': 'benchmark B'},
|
||||
{'initial_message': 'prototype the winner'},
|
||||
{'initial_message': 'top stories on Hacker News'},
|
||||
{'initial_message': 'top stories on Lobsters'},
|
||||
{'initial_message': 'top stories on r/programming'},
|
||||
])
|
||||
|
||||
Check on a child → inspect_workstream:
|
||||
@@ -24,8 +22,9 @@ Wait for spawned children to finish → wait_for_workstream (PREFER over busy-po
|
||||
wait_for_workstream(ws_ids=['a1b2c3d4'], timeout=120)
|
||||
wait_for_workstream(ws_ids=['a1b2c3d4', 'e5f6g7h8', 'i9j0k1l2'], mode='all', timeout=300)
|
||||
|
||||
Push a follow-up message to a running child → send_to_workstream:
|
||||
Push a follow-up message to a child → send_to_workstream (mid-run nudge, or course-correct a child that drifted off-brief):
|
||||
send_to_workstream(ws_id='a1b2c3d4', message='also capture the test-coverage delta')
|
||||
send_to_workstream(ws_id='a1b2c3d4', message='stop — you are editing auth_legacy.py, the active path is auth.py')
|
||||
|
||||
List what you've spawned → list_workstreams:
|
||||
list_workstreams()
|
||||
@@ -38,19 +37,10 @@ Wind a child down → close_workstream (soft; session stops, storage kept) or de
|
||||
close_workstream(ws_id='a1b2c3d4', reason='task complete')
|
||||
delete_workstream(ws_id='a1b2c3d4')
|
||||
|
||||
Wind all direct children down at once → close_all_children (soft-close cascade, single approval):
|
||||
Wind all direct children down at once → close_all_children (soft-close cascade):
|
||||
close_all_children(reason='batch complete, synthesising results')
|
||||
|
||||
Plan and track work → tasks (your scratchpad; children don't see it):
|
||||
tasks(action='add', title='audit auth.py for CSRF')
|
||||
tasks(action='update', task_id='t_03', status='in_progress')
|
||||
tasks(action='list')
|
||||
tasks(action='remove', task_id='t_03')
|
||||
|
||||
## Workflow shape
|
||||
|
||||
Prefer: tasks to plan → spawn_workstream to delegate → wait_for_workstream to block on completion → inspect_workstream to read the final message → synthesise → close_workstream.
|
||||
|
||||
Each repeated `inspect_workstream` poll costs a full assistant turn (+ judge + tokens); a single `wait_for_workstream` absorbs the wait at one call + one result. The cost gap widens fast on fan-outs of 3+ children.
|
||||
|
||||
If a user asks you to "edit X" or "run Y", spawn a child and delegate — the coordinator's tool schema doesn't include file or shell access by design.
|
||||
|
||||
+61
-4
@@ -194,6 +194,18 @@ class WebUI(SessionUIBase):
|
||||
}
|
||||
if state == "idle":
|
||||
event["content"] = payload["content"]
|
||||
# Coord tree-UI renders inline approve/deny buttons off
|
||||
# ``pending_approval_detail``; carrying it on the
|
||||
# state-change broadcast lets the cluster bus update those
|
||||
# buttons in lockstep with ``activity_state`` instead of
|
||||
# forcing the browser to chase a separate dashboard fetch.
|
||||
# Gated on existence so we don't pay the serializer's
|
||||
# per-broadcast verdict-cache deepcopy on the common
|
||||
# no-approval-pending path.
|
||||
if self._pending_approval is not None:
|
||||
detail = self.serialize_pending_approval_detail()
|
||||
if detail is not None:
|
||||
event["pending_approval_detail"] = detail
|
||||
try:
|
||||
WebUI._global_queue.put_nowait(event)
|
||||
except queue.Full:
|
||||
@@ -347,6 +359,16 @@ def _build_history(
|
||||
issued the tool calls is also marked ``"denied": True`` so the
|
||||
client can render the correct badge.
|
||||
"""
|
||||
# Metacognitive nudges live on the message dict's ``_reminders``
|
||||
# side-channel — user messages carry user-channel nudges
|
||||
# (correction / denial / resume / start / completion), tool
|
||||
# messages carry tool-channel nudges (tool_error / repeat). Both
|
||||
# are surfaced separately on each entry so the UI can render them
|
||||
# as their own bubble (live via ``user_reminder`` /
|
||||
# ``tool_reminder`` SSE events; replay via this propagation).
|
||||
# ``content`` never carries the ``<system-reminder>`` envelope —
|
||||
# that splice is transient, applied to a wire-bound copy in
|
||||
# ``ChatSession._apply_reminders_for_provider``.
|
||||
history = []
|
||||
for msg in session.messages:
|
||||
content = msg.get("content")
|
||||
@@ -392,6 +414,24 @@ def _build_history(
|
||||
entry = {"role": msg["role"], "content": content}
|
||||
if attachments_meta:
|
||||
entry["attachments"] = attachments_meta
|
||||
# Surface the ``_reminders`` side-channel so a tab reconnecting
|
||||
# via /history renders the same metacognitive nudge bubble the
|
||||
# originating tab saw live (user-channel reminders via
|
||||
# ``user_reminder`` SSE; tool-channel via ``tool_reminder``).
|
||||
# Reminders are in-memory only (not persisted to DB), so this
|
||||
# only fires for the originating session.
|
||||
reminders = msg.get("_reminders")
|
||||
if isinstance(reminders, list):
|
||||
# Filter first so an all-malformed _reminders doesn't set the
|
||||
# field to []; absent vs. empty-list should mean the same
|
||||
# thing on the wire.
|
||||
clean_reminders = [
|
||||
{"type": str(r.get("type") or ""), "text": str(r.get("text") or "")}
|
||||
for r in reminders
|
||||
if isinstance(r, dict)
|
||||
]
|
||||
if clean_reminders:
|
||||
entry["reminders"] = clean_reminders
|
||||
if msg.get("tool_calls"):
|
||||
entry["tool_calls"] = [
|
||||
{
|
||||
@@ -823,6 +863,16 @@ def _build_node_snapshot(app_state: Any) -> dict[str, Any]:
|
||||
title = ""
|
||||
if ws.session:
|
||||
title = get_workstream_display_name(ws.session.ws_id) or ""
|
||||
# ``pending_approval_detail`` mirrors the dashboard handler's
|
||||
# projection so the console collector's reconnect-via-snapshot
|
||||
# path (``_reconcile_node``) can carry the rich approval payload
|
||||
# across reconnects — without it, a child sitting in approval-
|
||||
# pending across a console restart or network blip would render
|
||||
# with no buttons until the next state change. Same data, same
|
||||
# ``read`` scope as ``/v1/api/dashboard``.
|
||||
approval_detail: dict[str, Any] | None = None
|
||||
if ui is not None and hasattr(ui, "serialize_pending_approval_detail"):
|
||||
approval_detail = ui.serialize_pending_approval_detail()
|
||||
ws_list.append(
|
||||
{
|
||||
"id": ws.id,
|
||||
@@ -839,6 +889,7 @@ def _build_node_snapshot(app_state: Any) -> dict[str, Any]:
|
||||
"kind": ws.kind,
|
||||
"parent_ws_id": ws.parent_ws_id,
|
||||
"user_id": ws.user_id,
|
||||
"pending_approval_detail": approval_detail,
|
||||
}
|
||||
)
|
||||
return {
|
||||
@@ -3323,8 +3374,9 @@ def create_app(
|
||||
# ``sse_executor`` so SSE polling stayed isolated from
|
||||
# every other ``asyncio.to_thread`` caller in the process
|
||||
# (storage, router, audit). Restore that isolation under
|
||||
# the lifted contract — coord wires ``None`` and falls
|
||||
# back to the default executor.
|
||||
# the lifted contract. The console's coord endpoint wires
|
||||
# its own ``coord_sse_executor`` on the same lookup hook —
|
||||
# see ``turnstone/console/server.py``.
|
||||
sse_executor_lookup=lambda request: request.app.state.sse_executor,
|
||||
create_supports_attachments=True,
|
||||
create_supports_user_id_override=True,
|
||||
@@ -3544,6 +3596,11 @@ def main() -> None:
|
||||
default=8080,
|
||||
help="Port to listen on (default: 8080)",
|
||||
)
|
||||
parser.add_argument(
|
||||
"--skip-permissions",
|
||||
action="store_true",
|
||||
help="Auto-approve all tool calls (no confirmation prompts)",
|
||||
)
|
||||
# MCP config path is bootstrap-critical (needed before ConfigStore for tool loading)
|
||||
parser.add_argument(
|
||||
"--mcp-config",
|
||||
@@ -3971,7 +4028,7 @@ def main() -> None:
|
||||
ws = manager.create(user_id="", name="resumed")
|
||||
if not isinstance(ws.ui, WebUI):
|
||||
raise TypeError(f"Expected WebUI, got {type(ws.ui).__name__}")
|
||||
if config_store.get("tools.skip_permissions"):
|
||||
if args.skip_permissions or config_store.get("tools.skip_permissions"):
|
||||
ws.ui.auto_approve = True
|
||||
assert ws.session is not None
|
||||
ws.session.set_watch_runner(
|
||||
@@ -4009,7 +4066,7 @@ def main() -> None:
|
||||
_advertise_host = args.host if args.host not in ("0.0.0.0", "::") else socket.gethostname()
|
||||
_advertise_url = f"http://{_advertise_host}:{args.port}"
|
||||
|
||||
_skip_perms = config_store.get("tools.skip_permissions")
|
||||
_skip_perms = args.skip_permissions or config_store.get("tools.skip_permissions")
|
||||
app = create_app(
|
||||
workstreams=manager,
|
||||
global_queue=global_queue,
|
||||
|
||||
@@ -885,6 +885,27 @@
|
||||
.msg.user {
|
||||
border-left-color: var(--accent);
|
||||
}
|
||||
/* Metacognitive reminder — slotted directly below the message it
|
||||
advises (user message for correction/denial/etc., tool result for
|
||||
tool_error/repeat). Yellow accent reads as "advisory metadata"
|
||||
against the amber-ish user colour and the cyan tool cards;
|
||||
deliberately quieter than the surrounding bubbles so it doesn't
|
||||
compete for attention. Lives in the shared stylesheet so both
|
||||
the interactive UI and the console coord viewer render the same
|
||||
themed bubble. */
|
||||
.msg.user-reminder {
|
||||
border-left-color: var(--yellow);
|
||||
color: var(--fg-dim);
|
||||
font-size: 12px;
|
||||
padding: 6px 10px;
|
||||
white-space: pre-wrap;
|
||||
}
|
||||
.msg.user-reminder .msg-user-reminder-label {
|
||||
color: var(--yellow);
|
||||
font-weight: 600;
|
||||
margin-right: 6px;
|
||||
text-transform: lowercase;
|
||||
}
|
||||
.msg.assistant {
|
||||
border-left-color: var(--hair-2);
|
||||
}
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "wait_for_workstream",
|
||||
"description": "Block until one or all named child workstreams reach a terminal state. Prefer this over busy-polling inspect_workstream after a fan-out: the tool absorbs the wait, so you get one call + one result regardless of duration. Returns `{results: {ws_id: {state, tokens, updated, message, truncated}}, elapsed, complete, mode}` — `results` is keyed by ws_id (not top-level), `complete` is true when the wait condition fired before the timeout. `message` is the child's last assistant text for `idle`/`error`, a short sentinel for `closed`/`denied`, and `null` for non-terminal rows so a follow-up read knows which children still need work; capped at 6 KiB UTF-8, with `truncated=true` when the cap fires (call inspect_workstream for the rest). Real terminal states: `idle`, `error`, `closed`, `deleted` (the last is unobservable since hard-delete cascades the row out of storage). `mode='any'` returns when the first child hits a real terminal — a `denied` id alone never satisfies the condition, so a typo'd / foreign / nonexistent id can't false-positive a wait. `mode='all'` returns once every id has settled (real terminal OR denied). Cross-tenant guard: only the coordinator's own children (or itself) are visible; everything else is reported `state='denied'`. Capped at 32 ws_ids and 600s; both overflows error rather than silently truncating.",
|
||||
"description": "Block until one or all named child workstreams reach a terminal state. Prefer this over busy-polling inspect_workstream after a fan-out: the tool absorbs the wait, so you get one call + one result regardless of duration. Returns `{results: {ws_id: {state, tokens, updated, message, truncated}}, elapsed, complete, mode}` — `results` is keyed by ws_id (not top-level), `complete` is true when the wait condition fired before the timeout. `message` is the child's last assistant text for `idle`/`error`, a short sentinel for `closed`/`denied`, and `null` for non-terminal rows so a follow-up read knows which children still need work; capped at 10 KiB UTF-8, with `truncated=true` when the cap fires (call inspect_workstream for the rest). Real terminal states: `idle`, `error`, `closed`, `deleted` (the last is unobservable since hard-delete cascades the row out of storage). `mode='any'` returns when the first child hits a real terminal — a `denied` id alone never satisfies the condition, so a typo'd / foreign / nonexistent id can't false-positive a wait. `mode='all'` returns once every id has settled (real terminal OR denied). Cross-tenant guard: only the coordinator's own children (or itself) are visible; everything else is reported `state='denied'`. Capped at 32 ws_ids and 600s; both overflows error rather than silently truncating.",
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
|
||||
+163
-6
@@ -557,6 +557,44 @@ Pane.prototype.handleEvent = function (evt) {
|
||||
this.addErrorMessage(evt.message);
|
||||
break;
|
||||
|
||||
case "user_reminder":
|
||||
// Metacognitive nudges — render as their own bubble below the
|
||||
// user message they advise (semantically: a hint to the model
|
||||
// right before its turn). The originating tab's optimistic
|
||||
// addUserMessage already ran when the user clicked send, so by
|
||||
// the time this SSE event arrives the just-sent user bubble is
|
||||
// at the bottom of messagesEl and addUserReminder's "anchor to
|
||||
// most recent .msg.user" lookup finds it correctly; the
|
||||
// insertAdjacentElement('afterend', el) call drops the bubble
|
||||
// immediately below.
|
||||
//
|
||||
// Multi-tab caveat: the server emits no user_message SSE event
|
||||
// today, so a non-originating tab open on the same workstream
|
||||
// sees the reminder without a paired user-message render — the
|
||||
// anchor falls on a stale prior user bubble, mis-positioning
|
||||
// the reminder. The next /history reload corrects it (the
|
||||
// entry["reminders"] propagation in _build_history is
|
||||
// anchor-stable because replayHistory runs addUserMessage first
|
||||
// for every turn). Acceptable cost for stage 1; closing the
|
||||
// gap is a follow-up that adds a user_message SSE event.
|
||||
if (Array.isArray(evt.reminders) && evt.reminders.length) {
|
||||
this.addUserReminder(evt.reminders);
|
||||
}
|
||||
break;
|
||||
|
||||
case "tool_reminder":
|
||||
// Metacognitive tool-channel nudge (tool_error / repeat) —
|
||||
// render as the same yellow themed bubble used for user-channel
|
||||
// reminders, anchored below the .ts-approval block whose tool
|
||||
// result triggered the batch's reminder. evt.tool_call_id
|
||||
// identifies the specific tool element; addToolReminder walks
|
||||
// up to its parent approval block and inserts the bubble
|
||||
// immediately after.
|
||||
if (Array.isArray(evt.reminders) && evt.reminders.length) {
|
||||
this.addToolReminder(evt.reminders, evt.tool_call_id || "");
|
||||
}
|
||||
break;
|
||||
|
||||
case "message_queued":
|
||||
// Confirmation from server that a queued message was accepted.
|
||||
// The UI already showed the message optimistically in addQueuedMessage.
|
||||
@@ -669,6 +707,97 @@ Pane.prototype.removeThinkingIndicator = function () {
|
||||
if (el) el.remove();
|
||||
};
|
||||
|
||||
Pane.prototype.addUserReminder = function (reminders) {
|
||||
// Render each metacognitive reminder as its own bubble immediately
|
||||
// BELOW the user message it advises — semantically the reminder is
|
||||
// a hint to the model right before the assistant turn. Always
|
||||
// called AFTER the corresponding addUserMessage (live: optimistic
|
||||
// local render ran before the SSE event arrived; replay:
|
||||
// replayHistory renders the user message first), so "most recent
|
||||
// .msg.user" is always THIS turn's bubble — insertAdjacentElement
|
||||
// afterend drops the reminder directly below it. When no .msg.user
|
||||
// exists at all (e.g. a non-originating tab receiving a reminder
|
||||
// before any user turn has rendered) we append; the next /history
|
||||
// reload corrects any anchor anomaly.
|
||||
this.removeEmptyState();
|
||||
var userBubbles = this.messagesEl.querySelectorAll(".msg.user");
|
||||
var anchor = userBubbles.length ? userBubbles[userBubbles.length - 1] : null;
|
||||
for (var i = 0; i < reminders.length; i++) {
|
||||
var r = reminders[i] || {};
|
||||
var el = document.createElement("div");
|
||||
el.className = "msg user-reminder";
|
||||
var labelEl = document.createElement("span");
|
||||
labelEl.className = "msg-user-reminder-label";
|
||||
labelEl.textContent =
|
||||
"metacognition" + (r.type ? " · " + String(r.type) : "");
|
||||
var textEl = document.createElement("span");
|
||||
textEl.className = "msg-user-reminder-text";
|
||||
textEl.textContent = r.text || "";
|
||||
el.appendChild(labelEl);
|
||||
el.appendChild(textEl);
|
||||
if (anchor) {
|
||||
anchor.insertAdjacentElement("afterend", el);
|
||||
// Anchor advances so multiple reminders stack below the user
|
||||
// message in queued order (rather than each landing
|
||||
// immediately-after the user msg, which would reverse them).
|
||||
anchor = el;
|
||||
} else {
|
||||
this.messagesEl.appendChild(el);
|
||||
}
|
||||
}
|
||||
this.scrollToBottom(true);
|
||||
};
|
||||
|
||||
Pane.prototype.addToolReminder = function (reminders, toolCallId) {
|
||||
// Render each metacognitive tool-channel reminder (tool_error /
|
||||
// repeat) as the same yellow themed bubble used for user-channel
|
||||
// reminders, anchored below the .ts-approval block that produced
|
||||
// the tool result. toolCallId is the live-path anchor (SSE event
|
||||
// carries it); during replay it's an empty string and we fall back
|
||||
// to "last .ts-approval block in messagesEl", which is correct
|
||||
// because messages render in order — the assistant block carrying
|
||||
// the tool batch is always the most recent approval block by the
|
||||
// time we hit the tool message that owns the reminder.
|
||||
this.removeEmptyState();
|
||||
var anchor = null;
|
||||
if (toolCallId) {
|
||||
var escapedId = CSS.escape(toolCallId);
|
||||
var toolEl = this.messagesEl.querySelector(
|
||||
'.ts-approval-tool[data-call-id="' + escapedId + '"]',
|
||||
);
|
||||
if (toolEl) {
|
||||
anchor = toolEl.closest(".ts-approval");
|
||||
}
|
||||
}
|
||||
if (!anchor) {
|
||||
var blocks = this.messagesEl.querySelectorAll(".ts-approval");
|
||||
if (blocks.length) anchor = blocks[blocks.length - 1];
|
||||
}
|
||||
for (var i = 0; i < reminders.length; i++) {
|
||||
var r = reminders[i] || {};
|
||||
var el = document.createElement("div");
|
||||
// Same .msg.user-reminder class — visual treatment is shared
|
||||
// across user and tool channels (both are metacog nudges).
|
||||
el.className = "msg user-reminder";
|
||||
var labelEl = document.createElement("span");
|
||||
labelEl.className = "msg-user-reminder-label";
|
||||
labelEl.textContent =
|
||||
"metacognition" + (r.type ? " · " + String(r.type) : "");
|
||||
var textEl = document.createElement("span");
|
||||
textEl.className = "msg-user-reminder-text";
|
||||
textEl.textContent = r.text || "";
|
||||
el.appendChild(labelEl);
|
||||
el.appendChild(textEl);
|
||||
if (anchor) {
|
||||
anchor.insertAdjacentElement("afterend", el);
|
||||
anchor = el;
|
||||
} else {
|
||||
this.messagesEl.appendChild(el);
|
||||
}
|
||||
}
|
||||
this.scrollToBottom(true);
|
||||
};
|
||||
|
||||
Pane.prototype.addUserMessage = function (text, attachments) {
|
||||
this.removeEmptyState();
|
||||
var el = document.createElement("div");
|
||||
@@ -911,7 +1040,16 @@ Pane.prototype.replayHistory = function (messages) {
|
||||
for (var i = 0; i < messages.length; i++) {
|
||||
var msg = messages[i];
|
||||
if (msg.role === "user") {
|
||||
// addUserMessage first so addUserReminder's "anchor to most
|
||||
// recent .msg.user" lookup finds THIS message's bubble (not the
|
||||
// previous user message's, which would associate the reminder
|
||||
// with the wrong turn). addUserReminder then drops the bubble
|
||||
// immediately below the just-rendered user message via
|
||||
// insertAdjacentElement('afterend', el).
|
||||
this.addUserMessage(msg.content || "", msg.attachments || null);
|
||||
if (Array.isArray(msg.reminders) && msg.reminders.length) {
|
||||
this.addUserReminder(msg.reminders);
|
||||
}
|
||||
lastToolBlock = null;
|
||||
} else if (msg.role === "assistant") {
|
||||
if (msg.tool_calls && msg.tool_calls.length) {
|
||||
@@ -1016,6 +1154,15 @@ Pane.prototype.replayHistory = function (messages) {
|
||||
appendToolErrorBadge(lastToolBlock);
|
||||
}
|
||||
}
|
||||
// Tool-channel metacog reminders (tool_error / repeat) attach
|
||||
// to the LAST tool message in a batch; on replay we render the
|
||||
// bubble immediately below the .ts-approval block that owns
|
||||
// the tool result. addToolReminder's empty-toolCallId fallback
|
||||
// resolves to "last .ts-approval block" — which is exactly
|
||||
// lastToolBlock here.
|
||||
if (Array.isArray(msg.reminders) && msg.reminders.length) {
|
||||
this.addToolReminder(msg.reminders, "");
|
||||
}
|
||||
}
|
||||
}
|
||||
this._attachRetryToLastAssistant();
|
||||
@@ -1318,6 +1465,19 @@ Pane.prototype.appendToolOutput = function (callId, name, output, isError) {
|
||||
var stripped = stripAnsi(output || "").trim();
|
||||
if (!stripped) return;
|
||||
|
||||
// Skip rendering for denied/blocked tool results — the ✗ denied
|
||||
// badge from resolveApproval already shows the denial reason; the
|
||||
// SSE tool_result event would otherwise duplicate the text. Mirror
|
||||
// the guard in the history-replay path (the live path used to be
|
||||
// safe because no tool_result event was ever emitted for denied
|
||||
// items, but we now emit one so _tool_error_flags gets set).
|
||||
var parentBlock = target.closest(".ts-approval");
|
||||
var isDenied =
|
||||
(parentBlock && parentBlock.classList.contains("denied")) ||
|
||||
/^Denied by user/.test(stripped) ||
|
||||
/^Blocked/.test(stripped);
|
||||
if (isDenied) return;
|
||||
|
||||
// Detect structured media output and render interactive embed
|
||||
if (!isError) {
|
||||
var media = tryParseMedia(stripped);
|
||||
@@ -1332,12 +1492,9 @@ Pane.prototype.appendToolOutput = function (callId, name, output, isError) {
|
||||
var out = renderToolOutput(stripped, isError);
|
||||
|
||||
// Mark the parent approval block as errored
|
||||
if (isError) {
|
||||
var parentBlock = target.closest(".ts-approval");
|
||||
if (parentBlock && !parentBlock.classList.contains("denied")) {
|
||||
parentBlock.classList.add("error");
|
||||
appendToolErrorBadge(parentBlock);
|
||||
}
|
||||
if (isError && parentBlock && !parentBlock.classList.contains("denied")) {
|
||||
parentBlock.classList.add("error");
|
||||
appendToolErrorBadge(parentBlock);
|
||||
}
|
||||
|
||||
if (out.textContent.split("\n").length > 10) {
|
||||
|
||||
@@ -642,6 +642,9 @@
|
||||
.msg.user {
|
||||
color: var(--fg-bright);
|
||||
}
|
||||
/* .msg.user-reminder lives in shared_static/chat.css so both the
|
||||
interactive UI and the console coord viewer pick up the same
|
||||
yellow themed bubble. */
|
||||
/* .msg.assistant / .msg.info / .msg.error alignment + baseline visuals
|
||||
come from shared_static/chat.css. Interactive UI adds a pre-wrap
|
||||
override for info messages and a tightened tool-message shape with
|
||||
|
||||
Reference in New Issue
Block a user