mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-12 23:12:23 -06:00
9a30530d41
* feat(coord): surface child errors, isolate tool exceptions, add memory tool
Closes four coordinator gaps identified during operator triage:
1. Child workstream errors now surface in inspect/wait. Worker-thread
exception text is sanitized (URL userinfo masked, sk-/Bearer/ghp_/
github_pat_/AKIA tokens redacted, capped at 1024 chars) and persisted
to workstream_config.last_error before _emit_state("error") fires, so
coord polling never sees state=error with a missing cause. The row
is cleared on recovery transitions (idle/running) so a once-leaked
exception body doesn't outlive the failure. inspect_workstream and
wait_for_workstream return last_error for state=error rows; the
wait surface prefers it over the assistant-tail walk.
2. Tool exceptions now return as tool_results with sibling-aware
guidance. ChatSession._safe_prepare_tool wraps every per-call
_prepare_tool invocation; a buggy preparer becomes an error item
for that call only — sibling parallel tool_calls keep going,
never orphaning the assistant message's tool_calls block.
run_one's runtime exception path includes the exception class
and a short note that other tool calls in the batch completed
independently so the model can recover.
3. Memory tool exposed to coordinator with a coord-only scope.
memory.json gains coordinator: true + interactive: true + per-kind
kind_variants. Coord sessions see scope enum ["coordinator"] and
an orchestration-flavored description; IC sessions see ["global",
"workstream", "user"] and the existing flavor. Coord-scope rows
are private to the coordinator session (children cannot read or
write them), closing the cross-session prompt-injection lane that
an adversarially-steered child would otherwise have. Coord
visibility is also restricted to coord-scope only — coords no
longer see global / workstream / user memories that belong to the
user's interactive sessions.
4. Per-call exception isolation in tool batches. _safe_prepare_tool
was previously the implicit shield; now it's an explicit method
with documented invariants. KeyboardInterrupt / GenerationCancelled
re-raise so the cooperative cancel path still works.
Other notable changes:
- LAST_ERROR_CONFIG_KEY + persist_last_error / clear_last_error /
load_last_error / sanitize_error_text moved to turnstone.core.memory
(the storage facade hub) — readers in coordinator_client.py import
the constant.
- Memory scope tuples extracted to module constants
_VALID_MEMORY_SCOPES and _IMPLICIT_SCOPE_WALK; seven inline
duplicates collapsed.
- tools.py grows _apply_kind_variant for the per-kind tool surface;
tools without kind_variants pass through unchanged (no spurious
deep-copies).
- Session adds _coordinator_scope_id, _default_memory_scope,
_implicit_scope_walk, and _record_fatal_error chokepoints so the
worker-thread fatal path is one site rather than three.
- Removed duplicate on_error / on_state_change emits from
session_routes.py and coordinator_adapter.py — session.send()'s
_record_fatal_error owns the sequence now.
Tests: 4742 pass (no live), +30 net since the baseline. Ruff + mypy
clean on every modified production file.
* fix(coord): redact secrets in tool error paths via output_guard
Copilot review flagged two paths where ``str(exc)`` flowed back into
the model-facing tool_result without going through the credential-
redaction the new fatal-error path applies:
- ``ChatSession._safe_prepare_tool``: a preparer-side exception
becomes an error item whose ``error`` field embedded the raw
exception text.
- ``ChatSession._execute_tools.run_one``: a runtime tool exception
became an ``Error executing X: <e>`` tool_result, again with
the raw exception text.
Both now route through ``sanitize_error_text`` (sanitised log line +
sanitised tool_result), and ``sanitize_error_text`` itself was
refactored to delegate to ``output_guard.redact_credentials`` instead
of carrying its own parallel regex catalog — the audit log + post-tool
guard already use that pattern set, so the credential definition
stays in one place.
Also extended ``_RE_CONNECTION_STRING`` in ``output_guard`` to cover
``http(s)://user:pass@host`` so a misconfigured ``OPENAI_BASE_URL``
that lands in an httpx ``ConnectError.__str__`` is redacted by every
caller of ``redact_credentials`` (audit details, close-reason
persistence, last_error, the two tool error paths). The
host (useful for triage) survives; only the password is replaced
with the standard ``[REDACTED:password]`` marker.
Tests: full suite (4745 pass), ruff + mypy clean. Two new tests pin
the redaction behaviour in both tool error paths so a future refactor
can't drift back to leaking ``str(exc)`` verbatim.
173 lines
5.9 KiB
Python
173 lines
5.9 KiB
Python
"""Tests for turnstone.core.tools — JSON auto-loading and schema validation."""
|
|
|
|
from turnstone.core.tools import (
|
|
_META,
|
|
AGENT_AUTO_TOOLS,
|
|
AGENT_TOOLS,
|
|
PRIMARY_KEY_MAP,
|
|
TASK_AGENT_TOOLS,
|
|
TASK_AUTO_TOOLS,
|
|
TOOLS,
|
|
)
|
|
|
|
|
|
class TestToolsSchema:
|
|
def test_all_tools_have_function_type(self):
|
|
for tool in TOOLS:
|
|
assert tool["type"] == "function", f"Tool missing type='function': {tool}"
|
|
|
|
def test_all_tools_have_name(self):
|
|
for tool in TOOLS:
|
|
assert "name" in tool["function"], f"Tool missing name: {tool}"
|
|
assert isinstance(tool["function"]["name"], str)
|
|
|
|
def test_all_tools_have_description(self):
|
|
for tool in TOOLS:
|
|
assert "description" in tool["function"], f"Tool missing description: {tool}"
|
|
assert len(tool["function"]["description"]) > 0
|
|
|
|
def test_all_tools_have_parameters(self):
|
|
for tool in TOOLS:
|
|
params = tool["function"]["parameters"]
|
|
assert params["type"] == "object"
|
|
assert "properties" in params
|
|
|
|
def test_required_fields_exist_in_properties(self):
|
|
for tool in TOOLS:
|
|
func = tool["function"]
|
|
params = func["parameters"]
|
|
required = params.get("required", [])
|
|
properties = params["properties"]
|
|
for field in required:
|
|
assert field in properties, (
|
|
f"Tool '{func['name']}': required field '{field}' not in properties"
|
|
)
|
|
|
|
def test_tool_names_unique(self):
|
|
names = [t["function"]["name"] for t in TOOLS]
|
|
assert len(names) == len(set(names)), f"Duplicate tool names: {names}"
|
|
|
|
def test_agent_tools_subset(self):
|
|
tool_names = {t["function"]["name"] for t in TOOLS}
|
|
agent_names = {t["function"]["name"] for t in AGENT_TOOLS}
|
|
assert agent_names.issubset(tool_names), (
|
|
f"AGENT_TOOLS has names not in TOOLS: {agent_names - tool_names}"
|
|
)
|
|
|
|
def test_task_agent_tools_subset(self):
|
|
tool_names = {t["function"]["name"] for t in TOOLS}
|
|
task_names = {t["function"]["name"] for t in TASK_AGENT_TOOLS}
|
|
assert task_names.issubset(tool_names), (
|
|
f"TASK_AGENT_TOOLS has names not in TOOLS: {task_names - tool_names}"
|
|
)
|
|
|
|
def test_agent_tools_not_empty(self):
|
|
assert len(AGENT_TOOLS) > 0
|
|
|
|
def test_task_agent_tools_not_empty(self):
|
|
assert len(TASK_AGENT_TOOLS) > 0
|
|
|
|
|
|
class TestToolsMetadata:
|
|
"""Validate the metadata extracted from JSON files."""
|
|
|
|
def test_tool_count(self):
|
|
# 19 interactive tools + 13 coordinator tools
|
|
assert len(TOOLS) == 32
|
|
|
|
def test_agent_tools_count(self):
|
|
assert len(AGENT_TOOLS) == 10
|
|
|
|
def test_task_agent_tools_count(self):
|
|
assert len(TASK_AGENT_TOOLS) == 13
|
|
|
|
def test_coordinator_tools_count(self):
|
|
from turnstone.core.tools import COORDINATOR_TOOLS
|
|
|
|
assert len(COORDINATOR_TOOLS) == 14
|
|
assert {t["function"]["name"] for t in COORDINATOR_TOOLS} == {
|
|
"spawn_workstream",
|
|
"spawn_batch",
|
|
"close_all_children",
|
|
"inspect_workstream",
|
|
"send_to_workstream",
|
|
"close_workstream",
|
|
"cancel_workstream",
|
|
"delete_workstream",
|
|
"list_workstreams",
|
|
"list_nodes",
|
|
"list_skills",
|
|
"tasks",
|
|
"wait_for_workstream",
|
|
# ``memory`` is dual-kind (coordinator: true + interactive: true)
|
|
# so coords can persist orchestration context for their children
|
|
# via the new ``coordinator`` scope.
|
|
"memory",
|
|
}
|
|
|
|
def test_auto_approve_sets_match(self):
|
|
expected = {
|
|
"read_file",
|
|
"search",
|
|
"diff_file",
|
|
"math",
|
|
"man",
|
|
"web_fetch",
|
|
"web_search",
|
|
"notify",
|
|
# Coordinator read-only tools (no-mutation, safe to auto-approve):
|
|
"inspect_workstream",
|
|
"list_workstreams",
|
|
"list_nodes",
|
|
"list_skills",
|
|
"wait_for_workstream",
|
|
}
|
|
assert expected == AGENT_AUTO_TOOLS
|
|
assert expected == TASK_AUTO_TOOLS
|
|
|
|
def test_primary_key_map(self):
|
|
expected = {
|
|
"bash": "command",
|
|
"math": "code",
|
|
"read_file": "path",
|
|
"search": "query",
|
|
"write_file": "content",
|
|
"edit_file": "old_string",
|
|
"man": "page",
|
|
"web_fetch": "url",
|
|
"web_search": "query",
|
|
"task_agent": "prompt",
|
|
"plan_agent": "goal",
|
|
"memory": "name",
|
|
"recall": "query",
|
|
"notify": "message",
|
|
"watch": "command",
|
|
"read_resource": "uri",
|
|
"use_prompt": "name",
|
|
"skill": "name",
|
|
"diff_file": "path_a",
|
|
# Coordinator tools:
|
|
"spawn_workstream": "initial_message",
|
|
"spawn_batch": "children",
|
|
"close_all_children": "reason",
|
|
"inspect_workstream": "ws_id",
|
|
"send_to_workstream": "message",
|
|
"close_workstream": "ws_id",
|
|
"cancel_workstream": "ws_id",
|
|
"delete_workstream": "ws_id",
|
|
"tasks": "action",
|
|
}
|
|
assert expected == PRIMARY_KEY_MAP
|
|
|
|
def test_no_metadata_in_function_dicts(self):
|
|
"""Ensure turnstone metadata keys are stripped from the OpenAI schema."""
|
|
meta_keys = {"agent", "task_agent", "coordinator", "auto_approve", "primary_key"}
|
|
for tool in TOOLS:
|
|
func = tool["function"]
|
|
leaked = meta_keys & set(func)
|
|
assert not leaked, f"Tool '{func['name']}' leaks metadata into function dict: {leaked}"
|
|
|
|
def test_meta_has_all_tools(self):
|
|
tool_names = {t["function"]["name"] for t in TOOLS}
|
|
assert set(_META.keys()) == tool_names
|