ewilliams28/turnstone
1
0
Fork 0
mirror of https://github.com/turnstonelabs/turnstone.git synced 2026-08-28 06:44:51 -06:00
Code Issues Packages Projects Releases Wiki Activity

Fix/codeql quality findings (#299)

Browse Source 
* fix: replace empty except blocks with diagnostic logging

Add log.debug/warning to 7 bare except-pass blocks that silenced
failures in security-relevant or operationally-important paths:
- Channel route lookup, CLI policy evaluation, OIDC JWKS fetch,
  prompt policy loading, plan file write, routing override, username
  resolution.

Plan write now reports failure to user instead of falsely claiming
"Plan saved."

* fix: replace assert-with-side-effect and narrow BaseException catch

- Convert 4 assert isinstance() to explicit TypeError raises — assertions
  are stripped under python -O, removing runtime type checks
- Narrow except BaseException to except Exception in fallback handler —
  KeyboardInterrupt/SystemExit should not record as health failures
- Plan write failure now reports error to user instead of "Plan saved"

* fix: wire up toast error type and remove useless conditional

- showToast() now accepts optional type param ("error") with red border
  styling — 3 call sites were passing "error" that was silently ignored
- Remove always-true if (q) guard after early-return on empty query

* fix: remove unreachable return None after return self._judge

* fix: parenthesize multi-line string concatenations in dev_parts list

Explicit parens make intentional concatenation unambiguous to static
analysis (CodeQL implicit-string-concatenation-in-list rule).

* fix: remove constant-true filter in test mock — return list directly

* fix: extract side-effecting calls from assert in tests

store.delete() and mgr.close() have side effects that would be
stripped under python -O. Assign to variable first, then assert.

* fix: remove unused local variables in tests

Drop assignments to unused workstream/variable references created
solely for side effects. Use _ for unused tuple unpacking.

* fix: use admin.prompt_policies permission for prompt policy endpoints

All 5 prompt-policy endpoints (list, create, get, update, delete)
were checking admin.policies (the tool-policy permission) instead of
admin.prompt_policies. This caused a mismatch with the admin UI which
gates the tab on admin.prompt_policies — users could see the tab but
get 403, or reach the endpoint but never see the tab.

* fix: use caplog instead of capsys for structlog warning assertion

structlog output goes through the logging system, not stdout/stderr.

* fix: address review — remove dead isinstance, module-level import, unnecessary lambdas

- session.py: remove unreachable isinstance check (has_batch already
  validates raw_edits is a list)
- cli.py: move logging import to module level
- test_workstream.py: replace lambda wid: FakeUI(wid) with FakeUI
This commit is contained in:
Patrick Buckley
2026-04-04 17:53:20 -07:00
committed by GitHub
parent caf449e048
commit 205e7818f8
16 changed files with 142 additions and 121 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/test_config_store.py
+2 -1
View File
@@ -105,7 +105,8 @@ class TestDelete:
assert store.get("tools.timeout") == defn.default
def test_returns_false_for_non_existent(self, store):
assert store.delete("tools.timeout") is False
result = store.delete("tools.timeout")
assert result is False
def test_rejects_unknown_key(self, store):
with pytest.raises(ValueError, match="Unknown setting"):
tests/test_console.py
+1 -1
View File
@@ -39,7 +39,7 @@ class MockStorage:
self.services: list[dict[str, str]] = []
def list_services(self, service_type: str, max_age_seconds: int = 120) -> list[dict[str, str]]:
return [s for s in self.services if True] # all services match
return list(self.services)
# ---------------------------------------------------------------------------
tests/test_edit_file.py
+11 -11
View File
@@ -154,7 +154,7 @@ class TestSingleEdit:
)
assert result["needs_approval"]
call_id, msg = session._exec_edit_file(result)
_, msg = session._exec_edit_file(result)
assert "applied 1 edit" in msg
with open(path) as f:
assert f.read() == "foo\nbar\nbaz\n"
@@ -172,7 +172,7 @@ class TestSingleEdit:
assert result["needs_approval"]
assert "deletion" in result["preview"]
call_id, msg = session._exec_edit_file(result)
_, msg = session._exec_edit_file(result)
with open(sample_file) as f:
assert f.read() == "line1\nline2\nline4\nline5\n"
@@ -196,7 +196,7 @@ class TestBatchEdit:
assert result["needs_approval"]
assert "2 edits" in result["header"]
call_id, msg = session._exec_edit_file(result)
_, msg = session._exec_edit_file(result)
assert "applied 2 edits" in msg
with open(sample_file) as f:
assert f.read() == "first\nline2\nline3\nline4\nlast\n"
@@ -216,7 +216,7 @@ class TestBatchEdit:
)
assert result["needs_approval"]
call_id, msg = session._exec_edit_file(result)
_, msg = session._exec_edit_file(result)
assert "applied 3 edits" in msg
with open(sample_file) as f:
assert f.read() == "line1\nsecond\nthird\nfourth\nline5\n"
@@ -238,7 +238,7 @@ class TestBatchEdit:
)
assert result["needs_approval"]
call_id, msg = session._exec_edit_file(result)
_, msg = session._exec_edit_file(result)
assert "overlap" in msg.lower()
# File should be untouched
with open(path) as f:
@@ -305,7 +305,7 @@ class TestBatchEdit:
)
assert result["needs_approval"]
call_id, msg = session._exec_edit_file(result)
_, msg = session._exec_edit_file(result)
assert "applied 2 edits" in msg
with open(path) as f:
assert f.read() == "first_foo\nbar\nsecond_foo\nbaz\n"
@@ -324,7 +324,7 @@ class TestBatchEdit:
)
assert result["needs_approval"]
call_id, msg = session._exec_edit_file(result)
_, msg = session._exec_edit_file(result)
with open(sample_file) as f:
assert f.read() == "line1\nline3\nline5\n"
@@ -344,7 +344,7 @@ class TestBatchEdit:
# Single edit — no "(N edits)" count in header
assert "edits)" not in result["header"]
call_id, msg = session._exec_edit_file(result)
_, msg = session._exec_edit_file(result)
assert "applied 1 edit" in msg
with open(sample_file) as f:
assert f.read() == "line1\nline2\nmiddle\nline4\nline5\n"
@@ -414,7 +414,7 @@ class TestExecEdgeCases:
with open(sample_file, "w") as f:
f.write("completely different content\n")
call_id, msg = session._exec_edit_file(result)
_, msg = session._exec_edit_file(result)
assert "no longer found" in msg
def test_file_deleted_between_prepare_and_exec(self, session, sample_file):
@@ -431,7 +431,7 @@ class TestExecEdgeCases:
os.unlink(sample_file)
call_id, msg = session._exec_edit_file(result)
_, msg = session._exec_edit_file(result)
assert "Error" in msg
def test_batch_file_changed_partial_match(self, session, sample_file):
@@ -453,7 +453,7 @@ class TestExecEdgeCases:
with open(sample_file, "w") as f:
f.write("line1\nline2\nline3\nline4\n")
call_id, msg = session._exec_edit_file(result)
_, msg = session._exec_edit_file(result)
assert "no longer found" in msg
# line1 should NOT have been edited (atomic failure)
with open(sample_file) as f:
tests/test_prompt_templates_runtime.py
+2 -3
View File
@@ -403,7 +403,7 @@ class TestMCPTemplates:
class TestResumeDeletedTemplate:
def test_resume_with_deleted_template_degrades_gracefully(self, tmp_db, capsys):
def test_resume_with_deleted_template_degrades_gracefully(self, tmp_db, caplog):
from turnstone.core.memory import save_message
from turnstone.core.storage import get_storage
@@ -430,8 +430,7 @@ class TestResumeDeletedTemplate:
content = _sys_content(session2)
assert "EPHEMERAL_CONTENT" not in content
# Warning should be logged via structlog
captured = capsys.readouterr()
assert "not_found" in captured.out or "not_found" in captured.err
assert "not_found" in caplog.text
# ---------------------------------------------------------------------------
tests/test_sessions.py
-2
View File
@@ -335,8 +335,6 @@ class TestSaveMessageUpdatesWorkstream:
def test_updated_timestamp_bumped(self, tmp_db):
register_workstream("s1")
save_message("s1", "user", "first")
rows = list_workstreams_with_history()
_original_updated = rows[0][4]
import time
tests/test_storage_registry.py
-1
View File
@@ -57,7 +57,6 @@ class TestResetStorage:
s1 = get_storage()
reset_storage()
# After reset, get_storage() auto-inits a new instance
monkeypatch_not_needed = True # noqa: F841
init_storage("sqlite", path=str(tmp_path / "test2.db"), run_migrations=False)
s2 = get_storage()
assert s1 is not s2
tests/test_workstream.py
+66 -63
View File
@@ -130,54 +130,54 @@ class TestWorkstream:
class TestManagerCreation:
def test_create_first_sets_active(self):
mgr = WorkstreamManager(_fake_factory)
ws = mgr.create(ui_factory=lambda wid: FakeUI(wid))
ws = mgr.create(ui_factory=FakeUI)
assert mgr.active_id == ws.id
assert mgr.get_active() is ws
def test_create_second_does_not_change_active(self):
mgr = WorkstreamManager(_fake_factory)
ws1 = mgr.create(ui_factory=lambda wid: FakeUI(wid))
_ws2 = mgr.create(ui_factory=lambda wid: FakeUI(wid))
ws1 = mgr.create(ui_factory=FakeUI)
mgr.create(ui_factory=FakeUI)
assert mgr.active_id == ws1.id
def test_create_assigns_session(self):
mgr = WorkstreamManager(_fake_factory)
ws = mgr.create(ui_factory=lambda wid: FakeUI(wid))
ws = mgr.create(ui_factory=FakeUI)
assert isinstance(ws.session, FakeSession)
def test_create_assigns_ui(self):
mgr = WorkstreamManager(_fake_factory)
ws = mgr.create(ui_factory=lambda wid: FakeUI(wid))
ws = mgr.create(ui_factory=FakeUI)
assert isinstance(ws.ui, FakeUI)
assert ws.ui.ws_id == ws.id
def test_create_custom_name(self):
mgr = WorkstreamManager(_fake_factory)
ws = mgr.create(name="research", ui_factory=lambda wid: FakeUI(wid))
ws = mgr.create(name="research", ui_factory=FakeUI)
assert ws.name == "research"
def test_create_default_name(self):
mgr = WorkstreamManager(_fake_factory)
ws = mgr.create(ui_factory=lambda wid: FakeUI(wid))
ws = mgr.create(ui_factory=FakeUI)
assert ws.name.startswith("ws-")
def test_create_max_workstreams_all_active(self):
mgr = WorkstreamManager(_fake_factory, max_workstreams=3)
ws1 = mgr.create(ui_factory=lambda wid: FakeUI(wid))
ws2 = mgr.create(ui_factory=lambda wid: FakeUI(wid))
ws3 = mgr.create(ui_factory=lambda wid: FakeUI(wid))
ws1 = mgr.create(ui_factory=FakeUI)
ws2 = mgr.create(ui_factory=FakeUI)
ws3 = mgr.create(ui_factory=FakeUI)
# Mark all as non-idle so eviction cannot help
mgr.set_state(ws1.id, WorkstreamState.THINKING)
mgr.set_state(ws2.id, WorkstreamState.RUNNING)
mgr.set_state(ws3.id, WorkstreamState.ATTENTION)
with pytest.raises(RuntimeError, match="All 3 workstreams are active"):
mgr.create(ui_factory=lambda wid: FakeUI(wid))
mgr.create(ui_factory=FakeUI)
class TestManagerLookup:
def test_get_existing(self):
mgr = WorkstreamManager(_fake_factory)
ws = mgr.create(ui_factory=lambda wid: FakeUI(wid))
ws = mgr.create(ui_factory=FakeUI)
assert mgr.get(ws.id) is ws
def test_get_nonexistent(self):
@@ -186,16 +186,16 @@ class TestManagerLookup:
def test_list_all_creation_order(self):
mgr = WorkstreamManager(_fake_factory)
_ws1 = mgr.create(name="a", ui_factory=lambda wid: FakeUI(wid))
_ws2 = mgr.create(name="b", ui_factory=lambda wid: FakeUI(wid))
_ws3 = mgr.create(name="c", ui_factory=lambda wid: FakeUI(wid))
mgr.create(name="a", ui_factory=FakeUI)
mgr.create(name="b", ui_factory=FakeUI)
mgr.create(name="c", ui_factory=FakeUI)
result = mgr.list_all()
assert [w.name for w in result] == ["a", "b", "c"]
def test_index_of(self):
mgr = WorkstreamManager(_fake_factory)
ws1 = mgr.create(ui_factory=lambda wid: FakeUI(wid))
ws2 = mgr.create(ui_factory=lambda wid: FakeUI(wid))
ws1 = mgr.create(ui_factory=FakeUI)
ws2 = mgr.create(ui_factory=FakeUI)
assert mgr.index_of(ws1.id) == 1
assert mgr.index_of(ws2.id) == 2
assert mgr.index_of("nonexistent") == 0
@@ -203,9 +203,9 @@ class TestManagerLookup:
def test_count(self):
mgr = WorkstreamManager(_fake_factory)
assert mgr.count == 0
mgr.create(ui_factory=lambda wid: FakeUI(wid))
mgr.create(ui_factory=FakeUI)
assert mgr.count == 1
mgr.create(ui_factory=lambda wid: FakeUI(wid))
mgr.create(ui_factory=FakeUI)
assert mgr.count == 2
@@ -217,8 +217,8 @@ class TestManagerLookup:
class TestManagerSwitching:
def test_switch_by_id(self):
mgr = WorkstreamManager(_fake_factory)
ws1 = mgr.create(ui_factory=lambda wid: FakeUI(wid))
ws2 = mgr.create(ui_factory=lambda wid: FakeUI(wid))
ws1 = mgr.create(ui_factory=FakeUI)
ws2 = mgr.create(ui_factory=FakeUI)
assert mgr.active_id == ws1.id
result = mgr.switch(ws2.id)
@@ -227,13 +227,13 @@ class TestManagerSwitching:
def test_switch_nonexistent_returns_none(self):
mgr = WorkstreamManager(_fake_factory)
mgr.create(ui_factory=lambda wid: FakeUI(wid))
mgr.create(ui_factory=FakeUI)
assert mgr.switch("bad-id") is None
def test_switch_by_index(self):
mgr = WorkstreamManager(_fake_factory)
_ws1 = mgr.create(ui_factory=lambda wid: FakeUI(wid))
ws2 = mgr.create(ui_factory=lambda wid: FakeUI(wid))
mgr.create(ui_factory=FakeUI)
ws2 = mgr.create(ui_factory=FakeUI)
result = mgr.switch_by_index(2)
assert result is ws2
@@ -241,7 +241,7 @@ class TestManagerSwitching:
def test_switch_by_index_out_of_range(self):
mgr = WorkstreamManager(_fake_factory)
mgr.create(ui_factory=lambda wid: FakeUI(wid))
mgr.create(ui_factory=FakeUI)
assert mgr.switch_by_index(0) is None
assert mgr.switch_by_index(5) is None
@@ -254,29 +254,32 @@ class TestManagerSwitching:
class TestManagerClose:
def test_close_removes_workstream(self):
mgr = WorkstreamManager(_fake_factory)
_ws1 = mgr.create(ui_factory=lambda wid: FakeUI(wid))
ws2 = mgr.create(ui_factory=lambda wid: FakeUI(wid))
mgr.create(ui_factory=FakeUI)
ws2 = mgr.create(ui_factory=FakeUI)
assert mgr.close(ws2.id) is True
closed = mgr.close(ws2.id)
assert closed is True
assert mgr.count == 1
assert mgr.get(ws2.id) is None
def test_close_last_returns_false(self):
mgr = WorkstreamManager(_fake_factory)
ws = mgr.create(ui_factory=lambda wid: FakeUI(wid))
assert mgr.close(ws.id) is False
ws = mgr.create(ui_factory=FakeUI)
closed = mgr.close(ws.id)
assert closed is False
assert mgr.count == 1
def test_close_nonexistent_returns_false(self):
mgr = WorkstreamManager(_fake_factory)
mgr.create(ui_factory=lambda wid: FakeUI(wid))
mgr.create(ui_factory=lambda wid: FakeUI(wid))
assert mgr.close("nonexistent") is False
mgr.create(ui_factory=FakeUI)
mgr.create(ui_factory=FakeUI)
closed = mgr.close("nonexistent")
assert closed is False
def test_close_active_switches_to_first(self):
mgr = WorkstreamManager(_fake_factory)
ws1 = mgr.create(ui_factory=lambda wid: FakeUI(wid))
ws2 = mgr.create(ui_factory=lambda wid: FakeUI(wid))
ws1 = mgr.create(ui_factory=FakeUI)
ws2 = mgr.create(ui_factory=FakeUI)
mgr.switch(ws2.id)
mgr.close(ws2.id)
@@ -284,9 +287,9 @@ class TestManagerClose:
def test_close_updates_order(self):
mgr = WorkstreamManager(_fake_factory)
_ws1 = mgr.create(name="a", ui_factory=lambda wid: FakeUI(wid))
ws2 = mgr.create(name="b", ui_factory=lambda wid: FakeUI(wid))
_ws3 = mgr.create(name="c", ui_factory=lambda wid: FakeUI(wid))
mgr.create(name="a", ui_factory=FakeUI)
ws2 = mgr.create(name="b", ui_factory=FakeUI)
mgr.create(name="c", ui_factory=FakeUI)
mgr.close(ws2.id)
names = [w.name for w in mgr.list_all()]
@@ -295,7 +298,7 @@ class TestManagerClose:
def test_close_unblocks_approval_event(self):
"""Closing a workstream whose UI has a pending approval should unblock it."""
mgr = WorkstreamManager(_fake_factory)
_ws1 = mgr.create(ui_factory=lambda wid: FakeUI(wid))
mgr.create(ui_factory=FakeUI)
# Create a workstream with a WebUI-like approval mechanism
from turnstone.server import WebUI
@@ -310,7 +313,7 @@ class TestManagerClose:
def test_close_unblocks_plan_event(self):
"""Closing a workstream with pending plan review should unblock it."""
mgr = WorkstreamManager(_fake_factory)
_ws1 = mgr.create(ui_factory=lambda wid: FakeUI(wid))
mgr.create(ui_factory=FakeUI)
from turnstone.server import WebUI
@@ -331,13 +334,13 @@ class TestManagerEviction:
def test_evict_oldest_idle_on_create(self):
"""At capacity with idle workstreams, create() succeeds by evicting the oldest idle."""
mgr = WorkstreamManager(_fake_factory, max_workstreams=3)
ws1 = mgr.create(name="oldest", ui_factory=lambda wid: FakeUI(wid))
ws2 = mgr.create(name="middle", ui_factory=lambda wid: FakeUI(wid))
_ws3 = mgr.create(name="newest", ui_factory=lambda wid: FakeUI(wid))
ws1 = mgr.create(name="oldest", ui_factory=FakeUI)
ws2 = mgr.create(name="middle", ui_factory=FakeUI)
mgr.create(name="newest", ui_factory=FakeUI)
# All three are IDLE. Mark ws2 as RUNNING so it won't be evicted.
mgr.set_state(ws2.id, WorkstreamState.RUNNING)
# ws1 is oldest idle, ws3 is newer idle. Creating should evict ws1.
ws4 = mgr.create(name="four", ui_factory=lambda wid: FakeUI(wid))
ws4 = mgr.create(name="four", ui_factory=FakeUI)
assert mgr.count == 3
assert mgr.get(ws1.id) is None, "oldest idle should have been evicted"
assert mgr.get(ws4.id) is ws4
@@ -349,35 +352,35 @@ class TestManagerEviction:
def test_create_fails_when_all_active(self):
"""At capacity with ALL non-idle workstreams, create() raises RuntimeError."""
mgr = WorkstreamManager(_fake_factory, max_workstreams=2)
ws1 = mgr.create(ui_factory=lambda wid: FakeUI(wid))
ws2 = mgr.create(ui_factory=lambda wid: FakeUI(wid))
ws1 = mgr.create(ui_factory=FakeUI)
ws2 = mgr.create(ui_factory=FakeUI)
mgr.set_state(ws1.id, WorkstreamState.THINKING)
mgr.set_state(ws2.id, WorkstreamState.RUNNING)
with pytest.raises(RuntimeError, match="All 2 workstreams are active"):
mgr.create(ui_factory=lambda wid: FakeUI(wid))
mgr.create(ui_factory=FakeUI)
def test_configurable_max(self):
"""Constructor accepts max_workstreams param and respects it."""
mgr = WorkstreamManager(_fake_factory, max_workstreams=2)
ws1 = mgr.create(ui_factory=lambda wid: FakeUI(wid))
ws2 = mgr.create(ui_factory=lambda wid: FakeUI(wid))
ws1 = mgr.create(ui_factory=FakeUI)
ws2 = mgr.create(ui_factory=FakeUI)
mgr.set_state(ws1.id, WorkstreamState.RUNNING)
mgr.set_state(ws2.id, WorkstreamState.RUNNING)
with pytest.raises(RuntimeError):
mgr.create(ui_factory=lambda wid: FakeUI(wid))
mgr.create(ui_factory=FakeUI)
assert mgr.count == 2
def test_eviction_counter(self):
"""eviction_count increments on each auto-eviction."""
mgr = WorkstreamManager(_fake_factory, max_workstreams=2)
assert mgr.eviction_count == 0
mgr.create(ui_factory=lambda wid: FakeUI(wid))
mgr.create(ui_factory=lambda wid: FakeUI(wid))
mgr.create(ui_factory=FakeUI)
mgr.create(ui_factory=FakeUI)
# Both IDLE — create should evict the oldest
mgr.create(ui_factory=lambda wid: FakeUI(wid))
mgr.create(ui_factory=FakeUI)
assert mgr.eviction_count == 1
# Again — evict another idle one
mgr.create(ui_factory=lambda wid: FakeUI(wid))
mgr.create(ui_factory=FakeUI)
assert mgr.eviction_count == 2
assert mgr.count == 2
@@ -390,7 +393,7 @@ class TestManagerEviction:
class TestManagerState:
def test_set_state(self):
mgr = WorkstreamManager(_fake_factory)
ws = mgr.create(ui_factory=lambda wid: FakeUI(wid))
ws = mgr.create(ui_factory=FakeUI)
assert ws.state == WorkstreamState.IDLE
mgr.set_state(ws.id, WorkstreamState.THINKING)
@@ -398,7 +401,7 @@ class TestManagerState:
def test_set_state_with_error(self):
mgr = WorkstreamManager(_fake_factory)
ws = mgr.create(ui_factory=lambda wid: FakeUI(wid))
ws = mgr.create(ui_factory=FakeUI)
mgr.set_state(ws.id, WorkstreamState.ERROR, error_msg="API timeout")
assert ws.state == WorkstreamState.ERROR
@@ -410,7 +413,7 @@ class TestManagerState:
def test_on_state_change_callback(self):
mgr = WorkstreamManager(_fake_factory)
ws = mgr.create(ui_factory=lambda wid: FakeUI(wid))
ws = mgr.create(ui_factory=FakeUI)
changes = []
mgr._on_state_change = lambda wid, state: changes.append((wid, state))
@@ -433,7 +436,7 @@ class TestManagerThreadSafety:
def do_create():
try:
ws = mgr.create(ui_factory=lambda wid: FakeUI(wid))
ws = mgr.create(ui_factory=FakeUI)
# Mark as non-idle immediately so auto-eviction cannot reclaim it
mgr.set_state(ws.id, WorkstreamState.RUNNING)
created.append(ws.id)
@@ -456,7 +459,7 @@ class TestManagerThreadSafety:
mgr = WorkstreamManager(_fake_factory)
ids = []
for _ in range(5):
ws = mgr.create(ui_factory=lambda wid: FakeUI(wid))
ws = mgr.create(ui_factory=FakeUI)
ids.append(ws.id)
def do_switch(wid):
@@ -476,10 +479,10 @@ class TestManagerThreadSafety:
"""close() and list_all() running concurrently should not crash."""
mgr = WorkstreamManager(_fake_factory)
# Keep one alive to prevent closing the last
anchor = mgr.create(ui_factory=lambda wid: FakeUI(wid))
anchor = mgr.create(ui_factory=FakeUI)
targets = []
for _ in range(5):
ws = mgr.create(ui_factory=lambda wid: FakeUI(wid))
ws = mgr.create(ui_factory=FakeUI)
targets.append(ws.id)
def do_close():
@@ -878,7 +881,7 @@ class TestStateTransitions:
def test_full_lifecycle(self):
"""Verify the expected state transition sequence."""
mgr = WorkstreamManager(_fake_factory)
ws = mgr.create(ui_factory=lambda wid: FakeUI(wid))
ws = mgr.create(ui_factory=FakeUI)
# Simulate the state transitions that ChatSession.send() would emit
mgr.set_state(ws.id, WorkstreamState.THINKING)
@@ -899,7 +902,7 @@ class TestStateTransitions:
def test_error_recovery(self):
"""After an error, sending again should transition back to thinking."""
mgr = WorkstreamManager(_fake_factory)
ws = mgr.create(ui_factory=lambda wid: FakeUI(wid))
ws = mgr.create(ui_factory=FakeUI)
mgr.set_state(ws.id, WorkstreamState.ERROR, "API failed")
assert ws.state == WorkstreamState.ERROR
turnstone/channels/_routing.py
+1 -1
View File
@@ -244,7 +244,7 @@ class ChannelRouter:
self._node_urls[ws_id] = node_url.rstrip("/")
return self._node_urls[ws_id]
except Exception:
pass
log.debug("Console route lookup failed for ws %s", ws_id, exc_info=True)
return self._server_url
# -- user resolution -----------------------------------------------------
turnstone/cli.py
+2 -1
View File
@@ -7,6 +7,7 @@ model auto-detection, workstream management, and the main() REPL entry point.
from __future__ import annotations
import argparse
import logging
import os
import readline
import sys
@@ -165,7 +166,7 @@ class TerminalUI(SessionUI):
it for it in items if it.get("needs_approval") and not it.get("error")
]
except Exception:
pass # Best-effort — no policy enforcement on error
logging.getLogger(__name__).debug("Policy evaluation unavailable", exc_info=True)
with self._print_lock:
# Print all headers, previews, and heuristic verdicts
turnstone/console/server.py
+5 -5
View File
@@ -5582,7 +5582,7 @@ async def admin_list_prompt_policies(request: Request) -> JSONResponse:
storage, err = require_storage_or_503(request)
if err:
return err
err = require_permission(request, "admin.policies")
err = require_permission(request, "admin.prompt_policies")
if err:
return err
@@ -5601,7 +5601,7 @@ async def admin_create_prompt_policy(request: Request) -> JSONResponse:
storage, err = require_storage_or_503(request)
if err:
return err
err = require_permission(request, "admin.policies")
err = require_permission(request, "admin.prompt_policies")
if err:
return err
@@ -5658,7 +5658,7 @@ async def admin_get_prompt_policy(request: Request) -> JSONResponse:
storage, err = require_storage_or_503(request)
if err:
return err
err = require_permission(request, "admin.policies")
err = require_permission(request, "admin.prompt_policies")
if err:
return err
@@ -5678,7 +5678,7 @@ async def admin_update_prompt_policy(request: Request) -> JSONResponse:
storage, err = require_storage_or_503(request)
if err:
return err
err = require_permission(request, "admin.policies")
err = require_permission(request, "admin.prompt_policies")
if err:
return err
@@ -5731,7 +5731,7 @@ async def admin_delete_prompt_policy(request: Request) -> JSONResponse:
storage, err = require_storage_or_503(request)
if err:
return err
err = require_permission(request, "admin.policies")
err = require_permission(request, "admin.prompt_policies")
if err:
return err
turnstone/console/static/governance.js
+1 -2
View File
@@ -2185,8 +2185,7 @@ function searchSkillDiscover() {
var searchBtn = document.getElementById("skill-discover-search-btn");
if (searchBtn) searchBtn.disabled = true;
var url = "/v1/api/admin/skills/discover?limit=20";
if (q) url += "&q=" + encodeURIComponent(q);
var url = "/v1/api/admin/skills/discover?limit=20&q=" + encodeURIComponent(q);
authFetch(url)
.then(function (r) {
turnstone/core/auth.py
+1 -1
View File
@@ -1192,7 +1192,7 @@ async def handle_oidc_callback(request: Request, audience: str) -> Response:
jwks_data = await fetch_jwks(oidc_config.jwks_uri)
request.app.state.jwks_data = jwks_data
except OIDCError:
pass
log.warning("JWKS fetch failed from %s", oidc_config.jwks_uri, exc_info=True)
if jwks_data is None:
return RedirectResponse("/?oidc_error=OIDC+temporarily+unavailable", status_code=302)
turnstone/core/session.py
+32 -20
View File
@@ -1078,22 +1078,32 @@ class ChatSession:
dev_parts = [
"# Instructions",
"",
"You are a creative writing partner. Use the analysis channel to "
"think through structure, voice, and intent before drafting.",
(
"You are a creative writing partner. Use the analysis channel to "
"think through structure, voice, and intent before drafting."
),
"",
"Craft principles:",
"- Ground scenes in concrete sensory detail — what is seen, heard, felt.",
"- Vary rhythm. Short sentences hit hard. Longer ones carry the reader "
"through texture and nuance, building toward something.",
"- Dialogue should do at least two things: reveal character AND advance "
"plot or tension. Cut anything that's just exchanging information.",
"- Earn your abstractions. Don't say 'she felt sad' — show the thing "
"that makes the reader feel it.",
(
"- Vary rhythm. Short sentences hit hard. Longer ones carry the reader "
"through texture and nuance, building toward something."
),
(
"- Dialogue should do at least two things: reveal character AND advance "
"plot or tension. Cut anything that's just exchanging information."
),
(
"- Earn your abstractions. Don't say 'she felt sad' — show the thing "
"that makes the reader feel it."
),
"- Trust subtext. Leave room for the reader.",
"",
"Match the user's genre and tone. If they want literary fiction, write "
"literary fiction. If they want pulp, write pulp with conviction. "
"Never condescend to the form.",
(
"Match the user's genre and tone. If they want literary fiction, write "
"literary fiction. If they want pulp, write pulp with conviction. "
"Never condescend to the form."
),
]
else:
# Compose system message from modular components
@@ -1105,7 +1115,7 @@ class ChatSession:
if storage:
db_policies = storage.list_prompt_policies()
except Exception:
pass
log.debug("Failed to load prompt policies from storage", exc_info=True)
now = datetime.now().astimezone()
ctx = SessionContext(
current_datetime=now.strftime("%Y-%m-%dT%H:%M"),
@@ -1378,11 +1388,9 @@ class ChatSession:
if self._health_monitor:
self._health_monitor.record_success()
return result
except BaseException as primary_err:
except Exception as primary_err:
if self._health_monitor:
self._health_monitor.record_failure()
if isinstance(primary_err, (KeyboardInterrupt, SystemExit)):
raise
if not self._registry or not self._registry.fallback:
raise
# Try each fallback model. Fallbacks may use different backends;
@@ -2585,7 +2593,6 @@ class ChatSession:
return None
if self._judge is not None:
return self._judge
return None
# Frozen config required for IntentJudge init (LLM client fields).
# _judge_cfg already returns None when _judge_config is None, but
# this guard makes the dependency explicit for type narrowing.
@@ -2804,7 +2811,8 @@ class ChatSession:
continue
cid, output = results[i]
assert isinstance(output, str) # plan always returns text
if not isinstance(output, str):
raise TypeError(f"plan_agent must return str, got {type(output).__name__}")
plan_path = f".plan-{self._ws_id}.md"
if not self.auto_approve:
@@ -2857,7 +2865,10 @@ class ChatSession:
with open(plan_path, "w") as f:
f.write(output)
except OSError:
pass
log.warning("Failed to write plan to %s", plan_path, exc_info=True)
output += "\n\n---\nPlan could not be saved to disk."
results[i] = (cid, output)
continue
# Always include file path in the tool result so the
# outer model knows where the plan lives on disk.
@@ -3326,9 +3337,10 @@ class ChatSession:
"error": "Error: provide old_string/new_string or edits array, not both",
}
if has_batch:
assert isinstance(raw_edits, list)
# raw_edits is guaranteed to be a list by the has_batch check above
batch_edits: list[Any] = raw_edits # type: ignore[assignment]
edits: list[dict[str, Any]] = []
for i, e in enumerate(raw_edits):
for i, e in enumerate(batch_edits):
if not isinstance(e, dict):
return {
"call_id": call_id,
turnstone/server.py
+6 -4
View File
@@ -1652,7 +1652,8 @@ async def create_workstream(request: Request) -> JSONResponse:
ws_id=requested_ws_id,
client_type=body.get("client_type", "") or "",
)
assert isinstance(ws.ui, WebUI)
if not isinstance(ws.ui, WebUI):
raise TypeError(f"Expected WebUI, got {type(ws.ui).__name__}")
if skip or body.get("auto_approve", False):
ws.ui.auto_approve = True
# Register watch runner for this workstream
@@ -1752,7 +1753,7 @@ async def create_workstream(request: Request) -> JSONResponse:
_gs().set_workstream_override(ws.id, node_id, reason="local")
except Exception:
pass # best-effort; routing will still work via resume
log.debug("Failed to set routing override for %s", ws.id, exc_info=True)
# If an initial_message was provided, send it as the first user message.
# This replaces the old bridge behavior where CreateWorkstreamMessage
@@ -2901,7 +2902,7 @@ def main() -> None:
if _u:
_username = _u.get("username", "")
except Exception:
pass
log.debug("Failed to resolve username for uid %s", uid, exc_info=True)
# Re-resolve from ConfigStore so new workstreams pick up hot-reloaded settings.
live_memory_config = _build_memory_config()
@@ -2989,7 +2990,8 @@ def main() -> None:
name="default",
ui_factory=lambda wid: WebUI(ws_id=wid),
)
assert isinstance(ws.ui, WebUI)
if not isinstance(ws.ui, WebUI):
raise TypeError(f"Expected WebUI, got {type(ws.ui).__name__}")
if config_store.get("tools.skip_permissions"):
ws.ui.auto_approve = True
turnstone/shared_static/base.css
+4
View File
@@ -512,6 +512,10 @@ body {
overflow: hidden;
text-overflow: ellipsis;
}
#toast.toast-error {
border-color: var(--red, #c44);
color: var(--red, #c44);
}
#toast.show {
opacity: 1;
transform: translateX(-50%) translateY(0);
turnstone/shared_static/toast.js
+8 -5
View File
@@ -6,18 +6,20 @@ var _toastTimer = null;
var _toastShowing = false;
var _TOAST_TIMEOUT = window.TURNSTONE_TOAST_TIMEOUT || 3000;
function showToast(message) {
function showToast(message, type) {
var el = document.getElementById("toast");
if (!el) return;
if (_toastShowing) {
_toastQueue.push(message);
_toastQueue.push({ message: message, type: type });
return;
}
_displayToast(el, message);
_displayToast(el, message, type);
}
function _displayToast(el, message) {
function _displayToast(el, message, type) {
el.textContent = message;
el.classList.remove("toast-error");
if (type === "error") el.classList.add("toast-error");
el.classList.add("show");
_toastShowing = true;
if (_toastTimer) clearTimeout(_toastTimer);
@@ -27,7 +29,8 @@ function _displayToast(el, message) {
_toastTimer = null;
if (_toastQueue.length) {
setTimeout(function () {
_displayToast(el, _toastQueue.shift());
var item = _toastQueue.shift();
_displayToast(el, item.message, item.type);
}, 300);
}
}, _TOAST_TIMEOUT);