diff --git a/docs/api-reference.md b/docs/api-reference.md index 0bd47559..56a401b9 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -386,10 +386,10 @@ Each item in `items` (shared by `tool_info` and `approve_request`): {"type": "tool_output_chunk", "call_id": "call_abc123", "chunk": "Building project...\n"} ``` -**`tool_result`** -- final output from a completed tool execution. The `call_id` matches the corresponding `tool_info`/`approve_request` item and any preceding `tool_output_chunk` events. For bash tools, this arrives after all streaming chunks and includes both stdout and stderr. +**`tool_result`** -- final output from a completed tool execution. The `call_id` matches the corresponding `tool_info`/`approve_request` item and any preceding `tool_output_chunk` events. For bash tools, this arrives after all streaming chunks and includes both stdout and stderr. The `is_error` field is `true` when the tool execution failed (e.g. bash exit code >= 2 or signal, file not found, timeout). Exit code 1 is ambiguous (e.g. `grep` no-match) and is not flagged. User denials are tracked separately via a `denied` flag. Clients should use `is_error` instead of text-prefix heuristics. ```json -{"type": "tool_result", "call_id": "call_abc123", "name": "bash", "output": "file1.py\nfile2.py\n"} +{"type": "tool_result", "call_id": "call_abc123", "name": "bash", "output": "file1.py\nfile2.py\n", "is_error": false} ``` **`status`** -- token usage statistics, sent after each model turn. diff --git a/docs/architecture.md b/docs/architecture.md index 5663cdf9..df6b3eeb 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -242,7 +242,7 @@ class SessionUI(Protocol): def on_content_token(self, text: str) -> None: ... def on_stream_end(self) -> None: ... def approve_tools(self, items: list[dict]) -> tuple[bool, str | None]: ... - def on_tool_result(self, call_id: str, name: str, output: str) -> None: ... + def on_tool_result(self, call_id: str, name: str, output: str, *, is_error: bool = False) -> None: ... def on_tool_output_chunk(self, call_id: str, chunk: str) -> None: ... def on_status(self, usage: dict, context_window: int, effort: str) -> None: ... def on_plan_review(self, content: str) -> str: ... diff --git a/docs/diagrams/03-core-engine-classes.puml b/docs/diagrams/03-core-engine-classes.puml index 6a7cd053..4c92ba15 100644 --- a/docs/diagrams/03-core-engine-classes.puml +++ b/docs/diagrams/03-core-engine-classes.puml @@ -12,7 +12,7 @@ interface "SessionUI" as SessionUI <> { + on_content_token(text: str) + on_stream_end() + approve_tools(items: list) → (bool, str|None) - + on_tool_result(call_id: str, name: str, output: str) + + on_tool_result(call_id: str, name: str, output: str, *, is_error: bool = False) + on_tool_output_chunk(call_id: str, chunk: str) + on_status(usage: dict, ctx_window: int, effort: str) + on_plan_review(content: str) → str diff --git a/docs/diagrams/04-conversation-turn.puml b/docs/diagrams/04-conversation-turn.puml index 1a1b0305..8426a995 100644 --- a/docs/diagrams/04-conversation-turn.puml +++ b/docs/diagrams/04-conversation-turn.puml @@ -133,7 +133,8 @@ group loop [while tool_calls present] note right of TP bash: on_tool_output_chunk(call_id, line) called per stdout line, - then on_tool_result(call_id, name, output). + then on_tool_result(call_id, name, output, is_error). + is_error=True when execution failed. call_id routes chunks/results to correct tool div during parallel execution. Other tools: on_tool_result() only. diff --git a/docs/diagrams/05-tool-pipeline.puml b/docs/diagrams/05-tool-pipeline.puml index cb56abbf..a903b2bc 100644 --- a/docs/diagrams/05-tool-pipeline.puml +++ b/docs/diagrams/05-tool-pipeline.puml @@ -127,7 +127,7 @@ partition "Phase 3: Execute" #E3F2FD { :_truncate_output() on each result\n(max context_window × chars_per_token × 0.5 chars\ndefault: ~context_window × 2 chars); :bash: ui.on_tool_output_chunk(call_id, line) per stdout line; - :ui.on_tool_result(call_id, name, output) for each; + :ui.on_tool_result(call_id, name, output, is_error) for each; if (plan tool was executed?) then (yes) :ui.on_plan_review(output); diff --git a/docs/diagrams/06-mq-protocol.puml b/docs/diagrams/06-mq-protocol.puml index 5d188813..1fd96cc9 100644 --- a/docs/diagrams/06-mq-protocol.puml +++ b/docs/diagrams/06-mq-protocol.puml @@ -147,6 +147,7 @@ package "Outbound Events (Bridge → Client)" as OutPkg #E3F2FD { + call_id: str + name: str + output: str + + is_error: bool } class PlanReviewEvent { type = "plan_review" diff --git a/docs/diagrams/png/03-core-engine-classes.png b/docs/diagrams/png/03-core-engine-classes.png index 7c661874..330e398d 100644 --- a/docs/diagrams/png/03-core-engine-classes.png +++ b/docs/diagrams/png/03-core-engine-classes.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:0e605963c649574c7bf987b2d338257c78bc1fc68b524ac8276bb25365035e06 -size 594096 +oid sha256:6471e611beebf647f3a191eb16588571a404cc52a43067883a2b6f06dd936376 +size 594676 diff --git a/docs/diagrams/png/04-conversation-turn.png b/docs/diagrams/png/04-conversation-turn.png index 273f079a..77ce41ee 100644 --- a/docs/diagrams/png/04-conversation-turn.png +++ b/docs/diagrams/png/04-conversation-turn.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:da9d32000e3d92d92ce621661ced60f276f9b5be652f5ed6123b400505415f4a -size 319702 +oid sha256:3aa8d972bba40d78152f9f0c762b9f5ec616d8052c45fa52b7dd1c679ed81d61 +size 325245 diff --git a/docs/diagrams/png/05-tool-pipeline.png b/docs/diagrams/png/05-tool-pipeline.png index daef54ff..aadf5ca6 100644 --- a/docs/diagrams/png/05-tool-pipeline.png +++ b/docs/diagrams/png/05-tool-pipeline.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:43844b07d36beb04db871f6795a3f3be17852a6a484fdc0ea207403bd7f512a6 -size 274286 +oid sha256:72b3932ce99a860f5069544cd8423d3cdae3a51f6566db262b19ced19c780eb0 +size 274374 diff --git a/docs/diagrams/png/06-mq-protocol.png b/docs/diagrams/png/06-mq-protocol.png index 5536cf0e..43ddbf9f 100644 --- a/docs/diagrams/png/06-mq-protocol.png +++ b/docs/diagrams/png/06-mq-protocol.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:2636e2d4d2f84f26f93de6e984b780f6ad5bf8d3618a0202ea0a2ee80859c5b5 -size 312409 +oid sha256:d831c5e10266f0232262b6a29b0ea8e45b3cc63df75f716a80f18462b4f85e66 +size 319125 diff --git a/docs/sdk.md b/docs/sdk.md index 4e0e8d81..4387f30b 100644 --- a/docs/sdk.md +++ b/docs/sdk.md @@ -127,7 +127,7 @@ SSE events are deserialized into typed dataclasses. Use `event.type` to discrimi | `reasoning` | `ReasoningEvent` | `text` | | `tool_info` | `ToolInfoEvent` | `items` | | `approve_request` | `ApproveRequestEvent` | `items` | -| `tool_result` | `ToolResultEvent` | `call_id`, `name`, `output` | +| `tool_result` | `ToolResultEvent` | `call_id`, `name`, `output`, `is_error` | | `tool_output_chunk` | `ToolOutputChunkEvent` | `call_id`, `chunk` | | `status` | `StatusEvent` | `prompt_tokens`, `total_tokens`, `pct`, `effort`, `cache_creation_tokens`, `cache_read_tokens` | | `plan_review` | `PlanReviewEvent` | `content` | diff --git a/docs/tools.md b/docs/tools.md index 34b6dd8d..2e262ad2 100644 --- a/docs/tools.md +++ b/docs/tools.md @@ -102,10 +102,15 @@ Each item's `execute` callable is invoked: - Errored or denied items return their error/denial message without executing. - The `bash` tool streams stdout incrementally: each line calls `ui.on_tool_output_chunk(call_id, line)` as it is produced, then the final - combined output (stdout + stderr) is delivered via `ui.on_tool_result(call_id, name, output)`. + combined output (stdout + stderr) is delivered via + `ui.on_tool_result(call_id, name, output, is_error=...)`. The `call_id` links `tool_info`/`approve_request` items to their streaming chunks and final result, enabling correct routing when multiple bash tools run in parallel. - Other tools deliver results atomically via `ui.on_tool_result(call_id, name, output)` only. + The `is_error` flag is `True` when the tool execution failed (e.g. bash exit code >= 2 + or signal, file not found, timeout). Exit code 1 is ambiguous and not flagged; user + denials are tracked separately. This removes the need for text-prefix heuristics. + Other tools deliver results atomically via + `ui.on_tool_result(call_id, name, output, is_error=...)` only. - Special post-execution gate for `plan`: the plan output is shown to the user for review, and the user can reject or annotate it. diff --git a/sdk/typescript/src/events.ts b/sdk/typescript/src/events.ts index 68af7404..fffac0f8 100644 --- a/sdk/typescript/src/events.ts +++ b/sdk/typescript/src/events.ts @@ -59,6 +59,7 @@ export interface ToolResultEvent { call_id: string; name: string; output: string; + is_error?: boolean; } export interface ToolOutputChunkEvent { diff --git a/tests/test_cancel.py b/tests/test_cancel.py index 4d317c87..4da0bee9 100644 --- a/tests/test_cancel.py +++ b/tests/test_cancel.py @@ -37,7 +37,7 @@ class NullUI: def approve_tools(self, items): return True, None - def on_tool_result(self, call_id, name, output): + def on_tool_result(self, call_id, name, output, **kwargs): pass def on_tool_output_chunk(self, call_id, chunk): diff --git a/tests/test_load_skill.py b/tests/test_load_skill.py index 0ec328eb..960cb530 100644 --- a/tests/test_load_skill.py +++ b/tests/test_load_skill.py @@ -54,6 +54,7 @@ def _make_session(skills: list[dict[str, Any]] | None = None): session._notify_on_complete = "{}" session.messages = [] session._config = {} + session._tool_error_flags = {} # Stub set_skill to just record the call session._set_skill_called: list[str | None] = [] @@ -423,6 +424,7 @@ class TestSkillCatalogDisclosure: session._tool_search = None session._mcp_client = None session._notify_on_complete = "{}" + session._tool_error_flags = {} # Memory stubs session._memory_config = MagicMock() diff --git a/tests/test_model_registry.py b/tests/test_model_registry.py index bf91d301..5338eb8e 100644 --- a/tests/test_model_registry.py +++ b/tests/test_model_registry.py @@ -339,7 +339,7 @@ class _FakeUI: def approve_tools(self, items: list[dict[str, Any]]) -> tuple[bool, str | None]: return True, None - def on_tool_result(self, call_id: str, name: str, output: str) -> None: ... + def on_tool_result(self, call_id: str, name: str, output: str, **kwargs: Any) -> None: ... def on_tool_output_chunk(self, call_id: str, chunk: str) -> None: ... def on_status(self, usage: dict[str, Any], context_window: int, effort: str) -> None: ... def on_plan_review(self, content: str) -> str: diff --git a/tests/test_prompt_templates_runtime.py b/tests/test_prompt_templates_runtime.py index 73e106ae..c3722dc8 100644 --- a/tests/test_prompt_templates_runtime.py +++ b/tests/test_prompt_templates_runtime.py @@ -29,7 +29,7 @@ class NullUI: def approve_tools(self, items): return True, None - def on_tool_result(self, call_id, name, output): + def on_tool_result(self, call_id, name, output, **kwargs): pass def on_tool_output_chunk(self, call_id, chunk): diff --git a/tests/test_server_live.py b/tests/test_server_live.py index 3a6165d5..95809361 100644 --- a/tests/test_server_live.py +++ b/tests/test_server_live.py @@ -88,7 +88,7 @@ class RecordingUI: def approve_tools(self, items): return True, None # auto-approve everything - def on_tool_result(self, call_id, name, output): + def on_tool_result(self, call_id, name, output, **kwargs): self.tool_results.append((call_id, name, output)) def on_tool_output_chunk(self, call_id, chunk): diff --git a/tests/test_session.py b/tests/test_session.py index 95cf0b8b..eb20bc99 100644 --- a/tests/test_session.py +++ b/tests/test_session.py @@ -28,7 +28,7 @@ class NullUI: def approve_tools(self, items): return True, None - def on_tool_result(self, call_id, name, output): + def on_tool_result(self, call_id, name, output, **kwargs): pass def on_tool_output_chunk(self, call_id, chunk): diff --git a/tests/test_skills.py b/tests/test_skills.py index 2fbea97a..03a523fb 100644 --- a/tests/test_skills.py +++ b/tests/test_skills.py @@ -52,7 +52,7 @@ class NullUI: def approve_tools(self, items): return True, None - def on_tool_result(self, call_id, name, output): + def on_tool_result(self, call_id, name, output, **kwargs): pass def on_tool_output_chunk(self, call_id, chunk): diff --git a/tests/test_workstream.py b/tests/test_workstream.py index 7f0137fe..dbc07618 100644 --- a/tests/test_workstream.py +++ b/tests/test_workstream.py @@ -54,7 +54,7 @@ class FakeUI: def approve_tools(self, items): return True, None - def on_tool_result(self, call_id, name, output): + def on_tool_result(self, call_id, name, output, **kwargs): pass def on_tool_output_chunk(self, call_id, chunk): diff --git a/turnstone/cli.py b/turnstone/cli.py index 7c8b0c04..f55e4b3a 100644 --- a/turnstone/cli.py +++ b/turnstone/cli.py @@ -251,7 +251,14 @@ class TerminalUI(SessionUI): item["denial_msg"] = denial_msg return False, None - def on_tool_result(self, call_id: str, name: str, output: str) -> None: + def on_tool_result( + self, + call_id: str, + name: str, + output: str, + *, + is_error: bool = False, + ) -> None: pass # Optional: display summary def on_tool_output_chunk(self, call_id: str, chunk: str) -> None: @@ -424,9 +431,16 @@ class WorkstreamTerminalUI(TerminalUI): else: self._buffer("error", message) - def on_tool_result(self, call_id: str, name: str, output: str) -> None: + def on_tool_result( + self, + call_id: str, + name: str, + output: str, + *, + is_error: bool = False, + ) -> None: if self.is_foreground: - super().on_tool_result(call_id, name, output) + super().on_tool_result(call_id, name, output, is_error=is_error) def on_tool_output_chunk(self, call_id: str, chunk: str) -> None: if self.is_foreground: diff --git a/turnstone/core/session.py b/turnstone/core/session.py index 16d7e78a..753b7363 100644 --- a/turnstone/core/session.py +++ b/turnstone/core/session.py @@ -190,7 +190,14 @@ class SessionUI(Protocol): def on_content_token(self, text: str) -> None: ... def on_stream_end(self) -> None: ... def approve_tools(self, items: list[dict[str, Any]]) -> tuple[bool, str | None]: ... - def on_tool_result(self, call_id: str, name: str, output: str) -> None: ... + def on_tool_result( + self, + call_id: str, + name: str, + output: str, + *, + is_error: bool = False, + ) -> None: ... def on_tool_output_chunk(self, call_id: str, chunk: str) -> None: ... def on_status(self, usage: dict[str, Any], context_window: int, effort: str) -> None: ... def on_plan_review(self, content: str) -> str: ... @@ -344,6 +351,8 @@ class ChatSession: self._pending_nudge: list[tuple[str, str]] = [] # (type, text) # Repeat detection: track recent tool call signatures self._recent_tool_sigs: set[str] = set() + # 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 self._cancel_event = threading.Event() self._cancel_ref: _CancelRef = _CancelRef(self) # provider appends SDK stream here @@ -728,6 +737,19 @@ class ChatSession: "\n".join([header, *lines]) if lines else "MCP refresh complete: no servers to refresh." ) + def _report_tool_result( + self, + call_id: str, + name: str, + output: str, + *, + is_error: bool = False, + ) -> None: + """Notify the UI and record error flag for message persistence.""" + if is_error: + self._tool_error_flags[call_id] = True + self.ui.on_tool_result(call_id, name, output, is_error=is_error) + def _truncate_output(self, output: str) -> str: """Truncate tool output to self.tool_truncation chars, keeping head + tail.""" limit = self.tool_truncation @@ -1474,6 +1496,8 @@ class ChatSession: "tool_call_id": tc_id, "content": output, } + if self._tool_error_flags.pop(tc_id, False): + tool_msg["is_error"] = True self.messages.append(tool_msg) # Token estimation — image content uses a fixed heuristic @@ -2411,8 +2435,11 @@ class ChatSession: ) -> tuple[str, str | list[dict[str, Any]]]: self._check_cancelled() if item.get("error"): - self.ui.on_tool_result( - item["call_id"], item.get("func_name", "unknown"), item["error"] + self._report_tool_result( + item["call_id"], + item.get("func_name", "unknown"), + item["error"], + is_error=True, ) return item["call_id"], item["error"] if item.get("denied"): @@ -2426,7 +2453,7 @@ class ChatSession: func = item.get("func_name", "unknown") msg = f"Error executing {func}: {e}" log.warning("tool_exec.failed", tool=func, error=str(e), exc_info=True) - self.ui.on_tool_result(item["call_id"], func, msg) + self._report_tool_result(item["call_id"], func, msg, is_error=True) return item["call_id"], msg if len(items) == 1: @@ -3504,12 +3531,12 @@ class ChatSession: skill_data = get_skill_by_name(name) if not skill_data or not skill_data.get("enabled", True): msg = f"Error: skill '{name}' not found" - self.ui.on_tool_result(call_id, "skill", msg) + self._report_tool_result(call_id, "skill", msg, is_error=True) return call_id, msg if self._skill_name == name: msg = f"Skill '{name}' is already active" - self.ui.on_tool_result(call_id, "skill", msg) + self._report_tool_result(call_id, "skill", msg) return call_id, msg self.set_skill(name) @@ -3522,7 +3549,7 @@ class ChatSession: if scan: parts.append(f"Security tier: {scan}") msg = "\n".join(parts) - self.ui.on_tool_result(call_id, "skill", msg) + self._report_tool_result(call_id, "skill", msg) return call_id, msg # action == "search" @@ -3576,7 +3603,7 @@ class ChatSession: if not rows: msg = "No skills found" + (f" matching '{query}'" if query else "") - self.ui.on_tool_result(call_id, "skill", msg) + self._report_tool_result(call_id, "skill", msg) return call_id, msg lines = [f"Found {len(rows)} skill(s):", ""] @@ -3598,7 +3625,7 @@ class ChatSession: lines.append(line) msg = "\n".join(lines) - self.ui.on_tool_result(call_id, "skill", msg) + self._report_tool_result(call_id, "skill", msg) return call_id, msg # -- MCP tool prepare/execute ---------------------------------------------- @@ -3639,17 +3666,20 @@ class ChatSession: args: dict[str, Any] = item["mcp_args"] assert self._mcp_client is not None + mcp_error = False try: output = self._mcp_client.call_tool_sync(func_name, args, timeout=self.tool_timeout) except TimeoutError: output = f"MCP tool timed out after {self.tool_timeout}s" + mcp_error = True self.ui.on_error(output) except Exception as e: output = f"MCP tool error: {e}" + mcp_error = True self.ui.on_error(output) output = self._truncate_output(output) - self.ui.on_tool_result(call_id, func_name, output) + self._report_tool_result(call_id, func_name, output, is_error=mcp_error) return call_id, output @staticmethod @@ -3712,18 +3742,21 @@ class ChatSession: uri: str = item["resource_uri"] assert self._mcp_client is not None + mcp_error = False try: output = self._mcp_client.read_resource_sync(uri, timeout=self.tool_timeout) except TimeoutError: output = f"MCP resource read timed out after {self.tool_timeout}s" + mcp_error = True self.ui.on_error(output) except Exception: log.warning("MCP resource read failed for %s", uri, exc_info=True) output = "MCP resource error: failed to read resource" + mcp_error = True self.ui.on_error(output) output = self._truncate_output(output) - self.ui.on_tool_result(call_id, "read_resource", output) + self._report_tool_result(call_id, "read_resource", output, is_error=mcp_error) return call_id, output def _prepare_use_prompt(self, call_id: str, args: dict[str, Any]) -> dict[str, Any]: @@ -3791,6 +3824,7 @@ class ChatSession: arguments: dict[str, str] = item["prompt_arguments"] assert self._mcp_client is not None + mcp_error = False try: messages = self._mcp_client.get_prompt_sync( name, arguments or None, timeout=self.tool_timeout @@ -3798,14 +3832,16 @@ class ChatSession: output = "\n\n".join(f"[{m['role']}]: {m['content']}" for m in messages) except TimeoutError: output = f"MCP prompt timed out after {self.tool_timeout}s" + mcp_error = True self.ui.on_error(output) except Exception: log.warning("MCP prompt invocation failed for %s", name, exc_info=True) output = "MCP prompt error: failed to invoke prompt" + mcp_error = True self.ui.on_error(output) output = self._truncate_output(output) - self.ui.on_tool_result(call_id, "use_prompt", output) + self._report_tool_result(call_id, "use_prompt", output, is_error=mcp_error) return call_id, output # -- Execute methods (do the work, report output via UI) ------------------- @@ -3901,20 +3937,21 @@ class ChatSession: output = output.strip() output = self._truncate_output(output) + bash_error = proc.returncode not in (0, 1) if proc.returncode != 0: output += f"\n[exit code: {proc.returncode}]" - self.ui.on_tool_result(call_id, "bash", output) + self._report_tool_result(call_id, "bash", output, is_error=bash_error) return call_id, output if output else "(no output)" except subprocess.TimeoutExpired: msg = f"Command timed out after {self.tool_timeout}s" - self.ui.on_tool_result(call_id, "bash", msg) + self._report_tool_result(call_id, "bash", msg, is_error=True) return call_id, msg except Exception as e: msg = f"Error executing command: {e}" - self.ui.on_tool_result(call_id, "bash", msg) + self._report_tool_result(call_id, "bash", msg, is_error=True) return call_id, msg def _exec_read_file(self, item: dict[str, Any]) -> tuple[str, str | list[dict[str, Any]]]: @@ -3935,12 +3972,12 @@ class ChatSession: except FileNotFoundError: self._read_files.discard(resolved) msg = f"Error: {path} not found" - self.ui.on_tool_result(call_id, "read_file", msg) + self._report_tool_result(call_id, "read_file", msg, is_error=True) return call_id, msg except Exception as e: self._read_files.discard(resolved) msg = f"Error reading {path}: {e}" - self.ui.on_tool_result(call_id, "read_file", msg) + self._report_tool_result(call_id, "read_file", msg, is_error=True) return call_id, msg self._read_files.add(resolved) @@ -3963,7 +4000,7 @@ class ChatSession: if offset is not None or limit is not None: end = start + len(lines) - 1 desc += f" (lines {start}-{end} of {total_lines})" - self.ui.on_tool_result(call_id, "read_file", desc) + self._report_tool_result(call_id, "read_file", desc) return call_id, output if output else "(empty file)" @@ -3978,11 +4015,11 @@ class ChatSession: except OSError as e: self._read_files.discard(resolved) msg = f"Error: {path}: {e}" - self.ui.on_tool_result(call_id, "read_file", msg) + self._report_tool_result(call_id, "read_file", msg, is_error=True) return call_id, msg self._read_files.add(resolved) desc = f"image (no vision, {size:,} bytes)" - self.ui.on_tool_result(call_id, "read_file", desc) + self._report_tool_result(call_id, "read_file", desc) return call_id, ( f"Binary image file: {path} ({size:,} bytes). " "Current model does not support vision." @@ -3994,12 +4031,12 @@ class ChatSession: except FileNotFoundError: self._read_files.discard(resolved) msg = f"Error: {path} not found" - self.ui.on_tool_result(call_id, "read_file", msg) + self._report_tool_result(call_id, "read_file", msg, is_error=True) return call_id, msg except Exception as e: self._read_files.discard(resolved) msg = f"Error reading {path}: {e}" - self.ui.on_tool_result(call_id, "read_file", msg) + self._report_tool_result(call_id, "read_file", msg, is_error=True) return call_id, msg if len(raw) > _IMAGE_SIZE_CAP: @@ -4010,7 +4047,7 @@ class ChatSession: f"Error: image {path} is {size_mb:.1f} MB, " f"exceeds {cap_mb:.0f} MB limit for vision." ) - self.ui.on_tool_result(call_id, "read_file", msg) + self._report_tool_result(call_id, "read_file", msg, is_error=True) return call_id, msg self._read_files.add(resolved) @@ -4027,7 +4064,7 @@ class ChatSession: }, ] - self.ui.on_tool_result(call_id, "read_file", f"image ({len(raw):,} bytes)") + self._report_tool_result(call_id, "read_file", f"image ({len(raw):,} bytes)") return call_id, content_parts def _exec_search(self, item: dict[str, Any]) -> tuple[str, str]: @@ -4070,17 +4107,17 @@ class ChatSession: desc = f"{match_count} matches" if match_count else "no matches" if original_len > 500: desc += f" ({original_len} chars)" - self.ui.on_tool_result(call_id, "search", desc) + self._report_tool_result(call_id, "search", desc) return call_id, output except subprocess.TimeoutExpired: msg = f"Search timed out after {self.tool_timeout}s" - self.ui.on_tool_result(call_id, "search", msg) + self._report_tool_result(call_id, "search", msg, is_error=True) return call_id, msg except Exception as e: msg = f"Error: search failed: {e}" - self.ui.on_tool_result(call_id, "search", msg) + self._report_tool_result(call_id, "search", msg, is_error=True) return call_id, msg def _run_agent( @@ -4561,24 +4598,25 @@ class ChatSession: ) if not memory_id: msg = f"Error: failed to save memory '{item['name']}'" - self.ui.on_tool_result(call_id, "memory", msg) + self._report_tool_result(call_id, "memory", msg, is_error=True) return call_id, msg self._init_system_messages() if old is not None: msg = f"Updated memory '{item['name']}' (type={item['mem_type']}, scope={item['scope']})" else: msg = f"Saved memory '{item['name']}' (type={item['mem_type']}, scope={item['scope']})" - self.ui.on_tool_result(call_id, "memory", msg) + self._report_tool_result(call_id, "memory", msg) return call_id, msg if action == "delete": deleted = delete_structured_memory(item["name"], item["scope"], item["scope_id"]) if not deleted: msg = f"Error: memory '{item['name']}' not found (scope={item['scope']})" + self._report_tool_result(call_id, "memory", msg, is_error=True) else: self._init_system_messages() msg = f"Deleted memory '{item['name']}'" - self.ui.on_tool_result(call_id, "memory", msg) + self._report_tool_result(call_id, "memory", msg) return call_id, msg if action == "search": @@ -4604,7 +4642,7 @@ class ChatSession: if item["query"] else "No memories stored." ) - self.ui.on_tool_result(call_id, "memory", msg) + self._report_tool_result(call_id, "memory", msg) return call_id, msg if action == "list": @@ -4625,16 +4663,16 @@ class ChatSession: msg = f"Memories ({len(rows)}):\n" + "\n".join(lines) else: msg = "No memories stored." - self.ui.on_tool_result(call_id, "memory", msg) + self._report_tool_result(call_id, "memory", msg) return call_id, msg except Exception as e: msg = f"Error: {e}" - self.ui.on_tool_result(call_id, "memory", msg) + self._report_tool_result(call_id, "memory", msg, is_error=True) return call_id, msg msg = "Error: unexpected action" - self.ui.on_tool_result(call_id, "memory", msg) + self._report_tool_result(call_id, "memory", msg, is_error=True) return call_id, msg def _exec_recall(self, item: dict[str, Any]) -> tuple[str, str]: @@ -4655,7 +4693,7 @@ class ChatSession: else: output = f"No conversation history found for '{query}'." - self.ui.on_tool_result(call_id, "recall", output) + self._report_tool_result(call_id, "recall", output) return call_id, output # -- Notify tool ----------------------------------------------------------- @@ -4754,7 +4792,7 @@ class ChatSession: if self._notify_count >= 5: msg = "Error: notification rate limit exceeded (max 5 per turn)" - self.ui.on_tool_result(call_id, "notify", msg) + self._report_tool_result(call_id, "notify", msg, is_error=True) return call_id, msg target: dict[str, str] = {} @@ -4792,7 +4830,7 @@ class ChatSession: continue log.warning("notify.no_services_exhausted") msg = "Error: no channel gateway services available" - self.ui.on_tool_result(call_id, "notify", msg) + self._report_tool_result(call_id, "notify", msg, is_error=True) return call_id, msg # Try first healthy gateway, fall back to next @@ -4817,7 +4855,7 @@ class ChatSession: ): self._notify_count += 1 msg = "Notification sent successfully" - self.ui.on_tool_result(call_id, "notify", msg) + self._report_tool_result(call_id, "notify", msg) return call_id, msg last_error = "no successful deliveries" continue @@ -4846,7 +4884,7 @@ class ChatSession: ) msg = "Error: notification delivery failed" - self.ui.on_tool_result(call_id, "notify", msg) + self._report_tool_result(call_id, "notify", msg, is_error=True) return call_id, msg # -- Watch tool ---------------------------------------------------------- @@ -5031,12 +5069,12 @@ class ChatSession: if action == "list": if not storage: msg = "No watches (storage unavailable)" - self.ui.on_tool_result(call_id, "watch", msg) + self._report_tool_result(call_id, "watch", msg) return call_id, msg watches = storage.list_watches_for_ws(self._ws_id) if not watches: msg = "No active watches." - self.ui.on_tool_result(call_id, "watch", msg) + self._report_tool_result(call_id, "watch", msg) return call_id, msg from turnstone.core.watch import format_interval @@ -5051,14 +5089,14 @@ class ChatSession: f"cmd: {w['command'][:60]}" ) msg = "Active watches:\n" + "\n".join(lines) - self.ui.on_tool_result(call_id, "watch", msg) + self._report_tool_result(call_id, "watch", msg) return call_id, msg if action == "cancel": name = item.get("watch_name", "") if not storage: msg = "Error: storage unavailable" - self.ui.on_tool_result(call_id, "watch", msg) + self._report_tool_result(call_id, "watch", msg, is_error=True) return call_id, msg watches = storage.list_watches_for_ws(self._ws_id) target = None @@ -5068,17 +5106,17 @@ class ChatSession: break if target is None: msg = f'Watch "{name}" not found.' - self.ui.on_tool_result(call_id, "watch", msg) + self._report_tool_result(call_id, "watch", msg, is_error=True) return call_id, msg storage.update_watch(target["watch_id"], active=False, next_poll="") msg = f'Watch "{target["name"]}" cancelled.' - self.ui.on_tool_result(call_id, "watch", msg) + self._report_tool_result(call_id, "watch", msg) return call_id, msg # action == "create" if not storage: msg = "Error: storage unavailable" - self.ui.on_tool_result(call_id, "watch", msg) + self._report_tool_result(call_id, "watch", msg, is_error=True) return call_id, msg watch_id = uuid.uuid4().hex @@ -5107,7 +5145,7 @@ class ChatSession: f" Command: {item['command']}\n" f" Condition: {stop_desc}" ) - self.ui.on_tool_result(call_id, "watch", msg) + self._report_tool_result(call_id, "watch", msg) return call_id, msg _MAX_WATCH_CHAIN = 5 # max consecutive watch dispatches per worker thread @@ -5143,11 +5181,11 @@ class ChatSession: f.write(content) self._read_files.add(resolved) msg = f"Wrote {len(content)} chars to {path}" - self.ui.on_tool_result(call_id, "write_file", msg) + self._report_tool_result(call_id, "write_file", msg) return call_id, msg except Exception as e: msg = f"Error writing {path}: {e}" - self.ui.on_tool_result(call_id, "write_file", msg) + self._report_tool_result(call_id, "write_file", msg, is_error=True) return call_id, msg def _exec_edit_file(self, item: dict[str, Any]) -> tuple[str, str]: @@ -5170,7 +5208,7 @@ class ChatSession: occurrences = find_occurrences(content, old_string) if len(occurrences) == 0: msg = f"Error: old_string no longer found in {path} (file changed)" - self.ui.on_tool_result(call_id, "edit_file", msg) + self._report_tool_result(call_id, "edit_file", msg, is_error=True) return call_id, msg if len(occurrences) > 1 and near_line is None: line_list = ", ".join(str(ln) for ln in occurrences) @@ -5178,7 +5216,7 @@ class ChatSession: f"Error: old_string found {len(occurrences)} times " f"at lines {line_list} (file changed)" ) - self.ui.on_tool_result(call_id, "edit_file", msg) + self._report_tool_result(call_id, "edit_file", msg, is_error=True) return call_id, msg if near_line is not None and len(occurrences) > 1: # Replace only the occurrence nearest to near_line @@ -5189,11 +5227,11 @@ class ChatSession: with open(path, "w") as f: f.write(content) msg = f"Edited {path}: replaced 1 occurrence" - self.ui.on_tool_result(call_id, "edit_file", msg) + self._report_tool_result(call_id, "edit_file", msg) return call_id, msg except Exception as e: msg = f"Error writing {path}: {e}" - self.ui.on_tool_result(call_id, "edit_file", msg) + self._report_tool_result(call_id, "edit_file", msg, is_error=True) return call_id, msg def _exec_math(self, item: dict[str, Any]) -> tuple[str, str]: @@ -5203,7 +5241,7 @@ class ChatSession: output = self._truncate_output(output) result_msg = f"Error:\n{output}" if is_error else output if output else "(no output)" - self.ui.on_tool_result(call_id, "math", result_msg) + self._report_tool_result(call_id, "math", result_msg, is_error=is_error) return call_id, result_msg def _exec_man(self, item: dict[str, Any]) -> tuple[str, str]: @@ -5247,20 +5285,20 @@ class ChatSession: text = result.stdout else: msg = f"No man or info page found for '{page}'" - self.ui.on_tool_result(call_id, "man", msg) + self._report_tool_result(call_id, "man", msg) return call_id, msg except FileNotFoundError: msg = "Error: man command not available" - self.ui.on_tool_result(call_id, "man", msg) + self._report_tool_result(call_id, "man", msg, is_error=True) return call_id, msg except subprocess.TimeoutExpired: msg = "Error: man page lookup timed out" - self.ui.on_tool_result(call_id, "man", msg) + self._report_tool_result(call_id, "man", msg, is_error=True) return call_id, msg text = self._truncate_output(text) - self.ui.on_tool_result(call_id, "man", f"{len(text)} chars") + self._report_tool_result(call_id, "man", f"{len(text)} chars") return call_id, text @@ -5289,15 +5327,15 @@ class ChatSession: except httpx.HTTPStatusError as e: msg = f"Error: fetch failed: HTTP {e.response.status_code}" - self.ui.on_tool_result(call_id, "web_fetch", msg) + self._report_tool_result(call_id, "web_fetch", msg, is_error=True) return call_id, msg except (httpx.RequestError, ValueError) as e: msg = f"Error: fetch failed: {e}" - self.ui.on_tool_result(call_id, "web_fetch", msg) + self._report_tool_result(call_id, "web_fetch", msg, is_error=True) return call_id, msg except Exception as e: msg = f"Error fetching URL: {e}" - self.ui.on_tool_result(call_id, "web_fetch", msg) + self._report_tool_result(call_id, "web_fetch", msg, is_error=True) return call_id, msg if not text.strip(): @@ -5348,7 +5386,12 @@ class ChatSession: except Exception as e: answer = f"Extraction failed (page was fetched but summarization errored): {e}" - self.ui.on_tool_result(call_id, "web_fetch", answer) + self._report_tool_result( + call_id, + "web_fetch", + answer, + is_error=answer.startswith("Extraction failed"), + ) return call_id, answer @@ -5363,17 +5406,17 @@ class ChatSession: client = self._resolve_search_client() if not client: msg = "Error: web search backend not available" - self.ui.on_tool_result(call_id, "web_search", msg) + self._report_tool_result(call_id, "web_search", msg, is_error=True) return call_id, msg try: output = client.search(query, max_results=max_results, topic=topic) except Exception as e: msg = f"Error: web search failed: {e}" - self.ui.on_tool_result(call_id, "web_search", msg) + self._report_tool_result(call_id, "web_search", msg, is_error=True) return call_id, msg - self.ui.on_tool_result(call_id, "web_search", output) + self._report_tool_result(call_id, "web_search", output) return call_id, output def handle_command(self, cmd_line: str) -> bool: diff --git a/turnstone/eval.py b/turnstone/eval.py index ac252a01..fc9bad60 100644 --- a/turnstone/eval.py +++ b/turnstone/eval.py @@ -105,7 +105,14 @@ class NullUI: def approve_tools(self, items: list[dict[str, Any]]) -> tuple[bool, str | None]: return True, None - def on_tool_result(self, call_id: str, name: str, output: str) -> None: + def on_tool_result( + self, + call_id: str, + name: str, + output: str, + *, + is_error: bool = False, + ) -> None: pass def on_tool_output_chunk(self, call_id: str, chunk: str) -> None: diff --git a/turnstone/sdk/events.py b/turnstone/sdk/events.py index d86b3896..e99081f7 100644 --- a/turnstone/sdk/events.py +++ b/turnstone/sdk/events.py @@ -107,6 +107,7 @@ class ToolResultEvent(ServerEvent): call_id: str = "" name: str = "" output: str = "" + is_error: bool = False @dataclass diff --git a/turnstone/server.py b/turnstone/server.py index bf65cd63..588040b2 100644 --- a/turnstone/server.py +++ b/turnstone/server.py @@ -382,22 +382,20 @@ class WebUI: return approved, feedback - def on_tool_result(self, call_id: str, name: str, output: str) -> None: + def on_tool_result( + self, + call_id: str, + name: str, + output: str, + *, + is_error: bool = False, + ) -> None: _metrics.record_tool_call(name) with self._ws_lock: self._ws_tool_calls[name] = self._ws_tool_calls.get(name, 0) + 1 self._ws_current_activity = "" self._ws_activity_state = "" self._broadcast_activity() - is_error = isinstance(output, str) and ( - output.startswith("Error") - or output.startswith("Command timed out") - or output.startswith("Search timed out") - or output.startswith("Unknown tool:") - or output.startswith("JSON parse error:") - or output.startswith("MCP prompt timed out") - or output.startswith("MCP prompt error") - ) event: dict[str, Any] = { "type": "tool_result", "call_id": call_id, @@ -647,8 +645,11 @@ def _build_history( if isinstance(content, str): if content.startswith("Denied by user") or content.startswith("Blocked"): entry["denied"] = True - elif ( - content.startswith("Error") + # Use persisted flag if available, fall back to text + # heuristic for historical data that predates is_error. + if ( + msg.get("is_error") + or content.startswith("Error") or content.startswith("Command timed out") or content.startswith("Search timed out") or content.startswith("Unknown tool:") diff --git a/turnstone/ui/static/app.js b/turnstone/ui/static/app.js index 84607e35..d88ca858 100644 --- a/turnstone/ui/static/app.js +++ b/turnstone/ui/static/app.js @@ -619,11 +619,7 @@ Pane.prototype.replayHistory = function (messages) { msg.denied || /^Denied by user/.test(stripped) || /^Blocked/.test(stripped); - var isToolError = - msg.is_error || - /^Error[:. \n]|^Command timed out|^Search timed out|^Unknown tool:|^JSON parse error:|^MCP prompt /.test( - stripped, - ); + var isToolError = !!msg.is_error; if (stripped && !isDenied) { var out = document.createElement("div"); out.className = @@ -924,19 +920,13 @@ Pane.prototype.appendToolOutput = function (callId, name, output, isError) { var stripped = stripAnsi(output || "").trim(); if (!stripped) return; - // Detect error by flag or content prefix - var hasError = - isError || - /^Error[:. \n]|^Command timed out|^Search timed out|^Unknown tool:|^JSON parse error:|^MCP prompt /.test( - stripped, - ); - + // Style tool output as error when indicated by isError flag var out = document.createElement("div"); - out.className = "tool-output" + (hasError ? " tool-output-error" : ""); + out.className = "tool-output" + (isError ? " tool-output-error" : ""); out.textContent = stripped; // Mark the parent approval block as errored - if (hasError) { + if (isError) { var parentBlock = target.closest(".approval-block"); if (parentBlock && !parentBlock.classList.contains("denied")) { parentBlock.classList.add("error");