Files
turnstone/tests/test_tools_schema.py
Patrick Buckley 9a30530d41 feat(coord): surface child errors, isolate tool exceptions, add memory tool (#443)
* 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.
2026-04-28 12:19:34 -07:00

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