From 110d44b07e8f0bfdb1fc3e980b60a76e5f418cbd Mon Sep 17 00:00:00 2001 From: Patrick Buckley Date: Sun, 31 May 2026 19:26:47 -0700 Subject: [PATCH] refactor(tools): remove man, math, and plan_agent built-in tools `man` and `math` duplicated capabilities already reachable through `bash`; `plan_agent` is better expressed as a `task_agent` running a planning skill, and carried a large amount of special-case machinery (plan-review gate, refinement loop, per-kind model routing). Removing all three shrinks the tool surface and cuts per-call token cost. Also removed, as dead-once-the-tools-are-gone: - the `math` sandbox executor (`turnstone.core.sandbox`) and its `[sandbox]` extra; the eval analyst now runs bash-only - the read-only `AGENT_TOOLS` sub-agent tool set and the `agent` tool-metadata key (`task_agent`/`TASK_AGENT_TOOLS` retained) - the plan-review protocol end to end: the `on_plan_review` UI hook, `resolve_plan`, `POST /v1/api/plan` + `POST /v1/api/route/plan`, the `plan_review`/`plan_resolved` SSE events, and their Python SDK / TypeScript SDK / OpenAPI / frontend / Discord+Slack bindings - the `model.plan_alias` / `model.plan_effort` settings and the registry `plan_model` / `plan_effort` routing fields TOOLS 31->28, TASK_AGENT_TOOLS 13->11; COORDINATOR_TOOLS unchanged. BREAKING CHANGE: removes the `man`, `math`, `plan_agent` tools, the plan-review SSE/HTTP/SDK surface, and the plan_* model-routing settings from the experimental 1.6 line. --- CHANGELOG.md | 13 + docs/api-reference.md | 37 - docs/architecture.md | 64 +- docs/channels.md | 16 +- docs/coordinator-skills.md | 2 +- docs/diagrams/02-package-structure.puml | 2 - docs/diagrams/03-core-engine-classes.puml | 6 +- docs/diagrams/04-conversation-turn.puml | 3 +- docs/diagrams/05-tool-pipeline.puml | 13 +- docs/diagrams/09-workstream-states.puml | 6 +- docs/diagrams/13-sdk-architecture.puml | 1 - docs/diagrams/22-judge-architecture.puml | 2 +- docs/eval.md | 3 +- docs/judge.md | 2 +- docs/sdk.md | 2 - docs/security.md | 2 +- docs/settings.md | 15 +- docs/tools.md | 164 ++--- pyproject.toml | 8 +- sdk/typescript/openapi-server.json | 448 +++--------- sdk/typescript/src/events.ts | 20 - sdk/typescript/src/index.ts | 3 - sdk/typescript/src/server.ts | 9 - sdk/typescript/src/types.ts | 5 - sdk/typescript/tests/events.test.ts | 12 - tests.json | 80 -- tests/_replay_helpers.py | 5 +- tests/test_auth.py | 3 - tests/test_cancel.py | 3 - tests/test_channel_routing.py | 22 - tests/test_channel_slack.py | 82 --- tests/test_config_store.py | 8 +- tests/test_coordinator_adapter.py | 5 - tests/test_coordinator_endpoints.py | 19 +- tests/test_coordinator_tools.py | 3 +- tests/test_interactive_adapter.py | 7 +- tests/test_judge_heuristic.py | 5 - tests/test_mcp_client.py | 8 - tests/test_model_registry.py | 134 +--- tests/test_models_changed_event.py | 8 +- tests/test_openapi.py | 1 - tests/test_prompt_templates_runtime.py | 3 - tests/test_rewind_retry.py | 3 - tests/test_route_proxy_audit.py | 7 +- tests/test_rule_registry.py | 14 +- tests/test_sandbox.py | 99 --- tests/test_sdk_console.py | 17 - tests/test_sdk_events.py | 14 - tests/test_sdk_server.py | 9 - tests/test_server_authz.py | 19 +- tests/test_server_live.py | 32 +- tests/test_session.py | 688 +----------------- tests/test_session_ui_base.py | 35 +- tests/test_settings_registry.py | 17 +- tests/test_skill_resource_materialization.py | 3 - tests/test_skills.py | 3 - tests/test_skills_tool.py | 6 +- tests/test_tools_schema.py | 31 +- tests/test_workstream_kind.py | 3 +- turnstone.example.toml | 16 +- turnstone/api/server_schemas.py | 5 - turnstone/api/server_spec.py | 11 - turnstone/channels/_formatter.py | 4 +- turnstone/channels/_routing.py | 13 - turnstone/channels/discord/bot.py | 28 +- turnstone/channels/discord/views.py | 166 +---- turnstone/channels/slack/bot.py | 217 ------ turnstone/cli.py | 29 - turnstone/console/coordinator_ui.py | 22 +- turnstone/console/server.py | 22 +- turnstone/console/static/admin.js | 17 +- turnstone/core/adapters/_ui_cleanup.py | 3 - .../core/adapters/interactive_adapter.py | 5 +- turnstone/core/auth.py | 1 - turnstone/core/judge.py | 12 +- turnstone/core/model_registry.py | 75 +- turnstone/core/sandbox.py | 345 --------- turnstone/core/session.py | 551 +------------- turnstone/core/session_manager.py | 14 +- turnstone/core/session_routes.py | 56 +- turnstone/core/session_ui_base.py | 49 +- turnstone/core/settings_registry.py | 32 +- turnstone/core/tools.py | 5 +- turnstone/eval.py | 43 +- turnstone/prompts/tools.md | 8 - turnstone/sdk/__init__.py | 2 - turnstone/sdk/console.py | 9 - turnstone/sdk/events.py | 14 - turnstone/sdk/server.py | 11 - turnstone/server.py | 110 +-- turnstone/tools/diff_file.json | 1 - turnstone/tools/man.json | 22 - turnstone/tools/math.json | 18 - turnstone/tools/notify.json | 1 - turnstone/tools/plan_agent.json | 19 - turnstone/tools/read_file.json | 1 - turnstone/tools/read_resource.json | 1 - turnstone/tools/search.json | 1 - turnstone/tools/task_agent.json | 2 +- turnstone/tools/use_prompt.json | 1 - turnstone/tools/web_fetch.json | 1 - turnstone/tools/web_search.json | 1 - turnstone/ui/static/app.js | 236 ------ turnstone/ui/static/index.html | 27 - turnstone/ui/static/style.css | 194 ----- uv.lock | 189 +---- 106 files changed, 471 insertions(+), 4393 deletions(-) delete mode 100644 tests/test_sandbox.py delete mode 100644 turnstone/core/sandbox.py delete mode 100644 turnstone/tools/man.json delete mode 100644 turnstone/tools/math.json delete mode 100644 turnstone/tools/plan_agent.json diff --git a/CHANGELOG.md b/CHANGELOG.md index ad7f9856..8b5507a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,19 @@ Three release tracks are maintained: dependency). Migration: use the bundled SearxNG (it ships in the compose stacks by default) or point `TURNSTONE_SEARXNG_URL` at an existing instance. No database migration required. +- **`man`, `math`, and `plan_agent` built-in tools removed** — `man` and + `math` duplicated capabilities already available through `bash`; `plan_agent` + is better expressed as a `task_agent` running a planning skill. Removing + them simplifies the tool surface and cuts per-call token cost. This release + also removes: the `math` sandbox executor (`turnstone.core.sandbox`) and the + `[sandbox]` extra's role for it; the read-only `AGENT_TOOLS` sub-agent tool + set and the `agent` tool-metadata key; the plan-review protocol + (`/v1/api/plan` endpoint, `plan_review`/`plan_resolved` SSE events, the + `on_plan_review` SDK/UI hook); and the `model.plan_alias` / + `model.plan_effort` ConfigStore settings (and the corresponding + `[model].plan_model` / `[model].plan_effort` config.toml knobs). + **Breaking change** on the experimental 1.6 line. Interactive built-in tool + count moves from 19 → 16; `TASK_AGENT_TOOLS` from 13 → 11. ### Security diff --git a/docs/api-reference.md b/docs/api-reference.md index c5da6cc3..71a7dbdf 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -455,13 +455,6 @@ Each item in `items` (shared by `tool_info` and `approve_request`): | `cache_creation_tokens` | int | Tokens written to prompt cache (Anthropic) | | `cache_read_tokens` | int | Tokens served from prompt cache (Anthropic + OpenAI) | -**`plan_review`** -- the model is proposing a plan and wants feedback. The -client must respond via `POST /v1/api/plan`. - -```json -{"type": "plan_review", "content": "Step 1: ...\nStep 2: ..."} -``` - **`info`** -- an informational message (e.g. command output). ```json @@ -779,36 +772,6 @@ automatically approved without prompting. --- -### `POST /v1/api/plan` - -Responds to a plan review dialog. The SSE stream must have previously sent a -`plan_review` event for the given workstream. - -**Request body:** - -```json -{"feedback": "", "ws_id": "abc123"} -``` - -| Field | Type | Required | Description | -|------------|--------|----------|---------------------------------------------------------| -| `feedback` | string | yes | Feedback text; empty string means approval | -| `ws_id` | string | yes | Target workstream ID | - -To approve the plan, send an empty string for `feedback`. To reject or request -changes, send a non-empty feedback string (e.g. `"reject"` or specific -revision instructions). - -**Response:** - -```json -{"status": "ok"} -``` - -**Error:** `404` with `{"error": "Unknown workstream"}` if `ws_id` is invalid. - ---- - ### `POST /v1/api/command` Executes a slash command in the given workstream. diff --git a/docs/architecture.md b/docs/architecture.md index 47f303a6..24e40e5b 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -3,8 +3,8 @@ Turnstone is an AI orchestration platform with tool use, parallel workstreams, and persistent memory. It connects to any OpenAI-compatible API (local vLLM, OpenAI, etc.) or Anthropic's native Messages API via pluggable provider adapters, and gives the -model 19 built-in tools plus external tools via MCP (Model Context Protocol) for -reading, writing, searching, planning, and executing code. +model 16 built-in tools plus external tools via MCP (Model Context Protocol) for +reading, writing, searching, and executing code. The core design principle is a **UI-agnostic engine with pluggable frontends**. The engine (`ChatSession`) drives the conversation loop -- streaming, tool @@ -61,7 +61,6 @@ turnstone/ ratelimit.py Per-IP token-bucket rate limiter (RateLimiter, TokenBucket) edit.py File edit utilities (find_occurrences, pick_nearest) safety.py Command safety validation (blocked patterns, sanitization) - sandbox.py Math code sandboxing (AST validation, subprocess execution) web.py Web utilities (HTML stripping, SSRF prevention) api/ schemas.py Shared Pydantic v2 models (auth, errors, WorkstreamState) @@ -102,7 +101,7 @@ turnstone/ renderer.js Markdown + LaTeX renderer (tables, nested lists, blockquotes, KaTeX math) app.js Split-pane UI (Pane class, binary layout tree, SSE, tool approval) tools/ - *.json 19 tool schemas (OpenAI function-calling format + turnstone metadata) + *.json 16 tool schemas (OpenAI function-calling format + turnstone metadata) ``` Both UIs share a common design system extracted into `turnstone/shared_static/`: design tokens, login overlay, toast notifications, theme toggle, keyboard shortcuts, and utility functions. Each UI imports `base.css` and the shared JS modules at `/shared/`, then adds only page-specific code at `/static/`. @@ -190,7 +189,6 @@ Phase 3: EXECUTE (parallel) (cancel_event also checked per line — kills process group on cancel) Final output (stdout + stderr) delivered via ui.on_tool_result(call_id, name, output) call_id links tool_info items → streaming chunks → final result - For plan tool: post-execution gate via ui.on_plan_review() ``` ### State Transitions @@ -209,7 +207,7 @@ The engine emits state changes via `_emit_state()` which calls "running" ---> tool execution | v - "attention" ---> waiting for user approval / plan review + "attention" ---> waiting for user approval | v "running" ---> executing approved tools @@ -231,7 +229,7 @@ The engine emits state changes via `_emit_state()` which calls > See also: [Core Engine Classes diagram](diagrams/png/03-core-engine-classes.png) -Defined in `turnstone.core.session.SessionUI` as a `typing.Protocol` with 16 +Defined in `turnstone.core.session.SessionUI` as a `typing.Protocol` with 15 methods. Every frontend must implement all of them. ```python @@ -247,7 +245,6 @@ class SessionUI(Protocol): 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: ... def on_info(self, message: str) -> None: ... def on_error(self, message: str) -> None: ... def on_state_change(self, state: str) -> None: ... @@ -269,7 +266,7 @@ the per-workstream events stream in | Class | Module | Notes | |-------|--------|-------| | `TerminalUI` | `turnstone.cli` | ANSI colors, `MarkdownRenderer`, `Spinner`, readline-based `input()` for approval | -| `WebUI` | `turnstone.server` | SSE event queue per workstream + global broadcast, `threading.Event` for blocking on approval/plan. `on_state_change` sends to both per-workstream and global SSE (the browser UI uses per-workstream `state_change` events to manage busy/idle transitions; `stream_end` only finalizes markdown rendering). | +| `WebUI` | `turnstone.server` | SSE event queue per workstream + global broadcast, `threading.Event` for blocking on approval. `on_state_change` sends to both per-workstream and global SSE (the browser UI uses per-workstream `state_change` events to manage busy/idle transitions; `stream_end` only finalizes markdown rendering). | | `NullUI` | `turnstone.eval` | Discards all output; `approve_tools` always returns `(True, None)` | ### WorkstreamTerminalUI @@ -281,10 +278,9 @@ awareness: are appended to `_output_buffer` instead of written to stdout. When the user switches to this workstream, `flush_buffer()` replays them. -- **Approval blocking**: `approve_tools()` and `on_plan_review()` call - `_fg_event.wait()` when in background, blocking the worker thread until the - workstream is foregrounded. This ensures the user sees the approval prompt - in the correct context. +- **Approval blocking**: `approve_tools()` calls `_fg_event.wait()` when in + background, blocking the worker thread until the workstream is foregrounded. + This ensures the user sees the approval prompt in the correct context. - **Foreground/background toggle**: `set_foreground(bool)` sets or clears `_fg_event` (a `threading.Event`). The manager calls this during `/ws ` @@ -421,7 +417,6 @@ turnstone metadata keys: | Metadata Key | Type | Meaning | |-------------|------|---------| -| `agent` | `bool` | Include this tool when running as a plan/task sub-agent | | `task_agent` | `bool` | Include this tool when running as a task sub-agent | | `auto_approve` | `bool` | Tool is read-only; skip user approval | | `primary_key` | `str` | Fallback argument name for bare-string JSON recovery | @@ -441,7 +436,6 @@ Example (`read_file.json`): }, "required": ["path"] }, - "agent": true, "task_agent": true, "auto_approve": true, "primary_key": "path" @@ -452,19 +446,17 @@ At import time, `turnstone.core.tools._load_tools()` strips the metadata keys from each schema and builds: - `TOOLS` -- list of `{"type": "function", "function": {...}}` dicts for the API -- `AGENT_TOOLS` -- subset with `agent: true` - `TASK_AGENT_TOOLS` -- subset with `task_agent: true` -- `AGENT_AUTO_TOOLS` / `TASK_AUTO_TOOLS` -- sets of tool names with `auto_approve: true` +- `TASK_AUTO_TOOLS` -- set of tool names with `auto_approve: true` - `PRIMARY_KEY_MAP` -- `{name: primary_key}` for JSON fallback recovery - `merge_mcp_tools(builtin, mcp_tools)` -- merges built-in + MCP tools at session init -### 19 Tools by Category +### 16 Tools by Category **Read-only (auto-approve)**: - `read_file` -- read file contents with optional offset/limit - `diff_file` -- show diff between two files / versions - `search` -- ripgrep-based codebase search -- `man` -- read man pages - `recall` -- search conversation history - `read_resource` -- read an MCP resource by URI @@ -472,7 +464,6 @@ from each schema and builds: - `bash` -- execute shell commands (with safety checks via `turnstone.core.safety`) - `write_file` -- create or overwrite a file - `edit_file` -- string replacement in an existing file (requires prior `read_file`) -- `math` -- execute Python in sandboxed subprocess (via `turnstone.core.sandbox`) - `web_fetch` -- fetch a URL (with SSRF protection via `turnstone.core.web`) - `web_search` -- search the web (provider-native for Anthropic/OpenAI, self-hosted SearxNG fallback for local models) - `notify` -- send a user-facing notification (Discord/Slack, optional reply routing) @@ -480,15 +471,14 @@ from each schema and builds: **Agent (delegated sub-sessions)**: - `task_agent` -- delegate to a sub-agent with full tool access (`TASK_AGENT_TOOLS`) -- `plan_agent` -- explore codebase and write a structured plan (`AGENT_TOOLS`) **Memory / skills / prompts**: - `memory` -- save, search, delete, or list memories (typed and scoped) - `skill` -- invoke a skill (governed, versioned procedure) - `use_prompt` -- fetch and apply a prompt template -Tool names are `plan_agent` / `task_agent` (not `plan` / `task`); bare words -collide with chat-template channels on some local models. +The tool name uses the `_agent` suffix — bare `task` collides with +chat-template channels on some local models. ### Prepare / Execute Pattern @@ -507,17 +497,11 @@ separation allows the UI to show previews before any side effects occur. ### Agent Tools -`task_agent` and `plan_agent` invoke `_run_agent()`, which runs a multi-turn -loop with a subset of tools and its own system prompt. The sub-agent runs -independently, then returns the final content as the tool result. +`task_agent` invokes `_run_agent()`, which runs a multi-turn loop with a +subset of tools and its own system prompt. The sub-agent runs independently, +then returns the final content as the tool result. - **task_agent**: uses `self._task_tools` (`TASK_AGENT_TOOLS` + MCP tools) -- **plan_agent**: uses `self._agent_tools` (`AGENT_TOOLS` + MCP tools). Writes output - to `.plan-.md` — unique per `ChatSession` so concurrent workstreams - don't collide. On repeat invocations the prior `plan_agent` tool call and its result - are forwarded from `self.messages` so the agent refines the existing plan rather - than starting over. Planning instructions are injected as a developer message - prepended to the agent's conversation. - **Turn limit**: controlled by `agent_max_turns` (default: `-1`, unlimited). When a limit is set and reached, the agent is forced to synthesize a final response without tools. When unlimited, the loop only exits when the model @@ -564,8 +548,8 @@ adds, removes, or reconnects servers as needed. When tools change, `_rebuild_tools()` creates new `_tools`/`_tool_map` objects (copy-on-write for thread safety) and notifies listener callbacks. Each `ChatSession` -rebuilds its merged tool lists and reconstructs `ToolSearchManager` (preserving -expanded tools). +rebuilds its `_tools` and `_task_tools` lists and reconstructs `ToolSearchManager` +(preserving expanded tools). **Tool naming:** `mcp__{server}__{tool}` — double underscore delimiter, validated at connection time (server names with `__` are rejected). @@ -803,7 +787,7 @@ with the same alias in-memory (the DB rows are never modified). parameters 6. `_create_stream_with_retry()` tries the primary model, then each fallback alias in order if the primary is unreachable -7. `_run_agent()` resolves `registry.agent_model` (if set) for plan/task +7. `_run_agent()` resolves `registry.agent_model` (if set) for task sub-agents, allowing a cheaper model for autonomous loops **Per-workstream selection:** `POST /v1/api/workstreams/new` accepts an optional @@ -812,7 +796,7 @@ which can override the model before workstream creation. ### Tool Output Truncation -Tool execution results (bash, read_file, search, math, man) are truncated by +Tool execution results (bash, read_file, search) are truncated by `_truncate_output()` when they exceed `tool_truncation` characters. Truncation preserves the first half and last half of the output, with a message in between: @@ -1240,7 +1224,6 @@ Starlette ASGI app (served by uvicorn) +-- Async request handlers (all under /v1/ prefix) | POST /v1/api/workstreams/{ws_id}/send -> starts worker thread per workstream | POST /v1/api/workstreams/{ws_id}/approve -> unblocks WebUI._approval_event - | POST /v1/api/plan -> unblocks WebUI._plan_event | POST /v1/api/workstreams/new -> creates workstream + worker | GET /v1/api/workstreams/{ws_id}/events -> SSE via EventSourceResponse (per workstream) | GET /v1/api/events/global -> SSE via EventSourceResponse (fan-out) @@ -1250,7 +1233,7 @@ Starlette ASGI app (served by uvicorn) | +-- Worker thread per workstream (daemon) | Runs session.send() synchronously -- ChatSession is fully blocking - | Blocks on WebUI._approval_event / _plan_event (threading.Event) + | Blocks on WebUI._approval_event (threading.Event) | +-- Background daemon threads Global SSE fan-out: reads global_queue, copies to per-client queues @@ -1274,7 +1257,7 @@ registry). Each workstream's `WebUI` has: - `_listeners` (per-client SSE queues, fan-out on `_enqueue()`) -- `_approval_event` / `_plan_event` (`threading.Event` for blocking) +- `_approval_event` (`threading.Event` for blocking) - `_global_queue` (class variable, shared, for state broadcasts) The SSE handlers bridge these sync queues to async via @@ -1530,8 +1513,7 @@ implemented in `turnstone/core/judge.py`: The judge is session-scoped (`IntentJudge`), lazy-initialized on first approval, and configured via the `[judge]` config section or `--judge` CLI flags. By default it uses self-consistency (same model), but supports -cross-model and cross-provider configurations. Sub-agents (plan, task) -are exempt. All verdicts are persisted to the `intent_verdicts` table +cross-model and cross-provider configurations. Task sub-agents are exempt. All verdicts are persisted to the `intent_verdicts` table (migration 012) with the user's final decision, enabling future calibration. The console exposes `GET /v1/api/admin/verdicts` for audit queries (requires `admin.judge` permission). diff --git a/docs/channels.md b/docs/channels.md index 7c947156..71e5b2e6 100644 --- a/docs/channels.md +++ b/docs/channels.md @@ -179,7 +179,6 @@ both and the gateway hosts both adapters in one process. see starts a per-user channel session. - Tool approvals render as Slack **Block Kit** buttons; only the user who owns the workstream can approve/reject. -- Plan reviews render as a modal with approve / request-changes actions. - Notifications and reply routing work identically to Discord. - Session recovery: persisted channel routes are re-subscribed when the bot restarts, so existing Slack conversations keep flowing. @@ -235,15 +234,6 @@ config, the bot auto-responds with approval and posts a field (useful for allowing specific tools like `bash` or `read_file` while still requiring manual approval for others). -### Plan Reviews - -Plan review requests are displayed as a blue embed with: - -- **Approve Plan** (green) button — approves the plan with empty feedback -- **Request Changes** (gray) button — opens a modal for feedback text - (up to 2000 characters) -- Feedback is forwarded to the server via HTTP - --- ## Configuration Reference @@ -431,9 +421,9 @@ message with a `ws_id` so that user replies can be routed back to the originating workstream. Adapters must track the mapping from outgoing message ID to `(ws_id, target_user_id)` and handle DM replies. -Platform-specific concerns — approval prompts, plan reviews, message -edits, thread creation — live inside the adapter implementation and are -not part of the protocol surface. Each adapter drives those via its +Platform-specific concerns — approval prompts, message edits, thread +creation — live inside the adapter implementation and are not part of +the protocol surface. Each adapter drives those via its own `_on_ws_event` dispatcher using SDK-native APIs. To add a new platform: diff --git a/docs/coordinator-skills.md b/docs/coordinator-skills.md index 80ab8e37..8e586489 100644 --- a/docs/coordinator-skills.md +++ b/docs/coordinator-skills.md @@ -86,7 +86,7 @@ Explicitly **not** in the coordinator set: - `bash` / `edit_file` / `write_file` / `append_file` / `diff_file` — no local FS. - `read_file` / `search` — no local FS reads. - `web_fetch` / `web_search` — no direct web access. -- `task_agent` / `plan_agent` — sub-agent tools are zeroed on coord sessions. +- `task_agent` — sub-agent tool is zeroed on coord sessions. - `recall` / `watch` / `read_resource` / `use_prompt` — UX / persistence tools that belong to interactive sessions. The dual-kind `memory` / `skills` / `notify` tools are available on both kinds (see the table above). If your skill needs a coordinator to "run a command" or "read a diff --git a/docs/diagrams/02-package-structure.puml b/docs/diagrams/02-package-structure.puml index 2597fdc8..d377385e 100644 --- a/docs/diagrams/02-package-structure.puml +++ b/docs/diagrams/02-package-structure.puml @@ -34,7 +34,6 @@ package "turnstone/core/" <> { component [metrics.py\nPrometheus metrics] as metrics <> component [config.py\nTOML config] as config <> component [safety.py\nPath validation] as safety <> - component [sandbox.py\nCommand sandbox] as sandbox <> component [edit.py\nFile editing] as edit <> component [web.py\nWeb helpers] as web <> component [auth.py\nAuthentication] as auth <> @@ -123,7 +122,6 @@ session --> tools session --> memory memory --> storage session --> safety -session --> sandbox session --> edit session --> web session --> healthcheck diff --git a/docs/diagrams/03-core-engine-classes.puml b/docs/diagrams/03-core-engine-classes.puml index fdba3a6b..74aae8a6 100644 --- a/docs/diagrams/03-core-engine-classes.puml +++ b/docs/diagrams/03-core-engine-classes.puml @@ -15,7 +15,6 @@ interface "SessionUI" as SessionUI <> { + 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 + on_info(message: str) + on_error(message: str) + on_state_change(state: str) @@ -43,15 +42,13 @@ class "WorkstreamTerminalUI" as WsTermUI { class "WebUI" as WebUI { - _listeners: list[Queue] - _approval_event: Event - - _plan_event: Event - _ws_prompt_tokens: int - _ws_tool_calls: dict + resolve_approval(approved, feedback) - + resolve_plan(feedback) -- Enqueues JSON events for SSE. Blocks on threading.Event for - approval/plan review. + approval. SSE handlers bridge Queue to async via run_in_executor(). -- @@ -145,7 +142,6 @@ class "ChatSession" as ChatSession { + model_alias: str | None {property} - _tools: list[dict] - _task_tools: list[dict] - - _agent_tools: list[dict] - _read_files: set[str] - system_messages: list[dict] -- diff --git a/docs/diagrams/04-conversation-turn.puml b/docs/diagrams/04-conversation-turn.puml index 09962dd4..4950bbaa 100644 --- a/docs/diagrams/04-conversation-turn.puml +++ b/docs/diagrams/04-conversation-turn.puml @@ -139,8 +139,7 @@ group loop [while tool_calls present] read_file → open().read() or base64 image search → grep subprocess edit_file → string replace - task/plan → _run_agent() sub-loop - math → sandboxed subprocess + task → _run_agent() sub-loop web_fetch → httpx + LLM summarize web_search → provider-native or SearxNG fallback memory/recall → SQLite diff --git a/docs/diagrams/05-tool-pipeline.puml b/docs/diagrams/05-tool-pipeline.puml index f1992b91..4610ba97 100644 --- a/docs/diagrams/05-tool-pipeline.puml +++ b/docs/diagrams/05-tool-pipeline.puml @@ -24,7 +24,7 @@ partition "Phase 1: Prepare" #E8F5E9 { :Dispatch to _prepare_{func_name}(); note right - **Dispatch table (19 built-in + tool_search):** + **Dispatch table (16 built-in + tool_search):** ┌───────────────┬──────────────────┐ │ Tool │ Needs Approval? │ ├───────────────┼──────────────────┤ @@ -34,13 +34,10 @@ partition "Phase 1: Prepare" #E8F5E9 { │ edit_file │ ✓ Yes │ │ search │ ✗ Auto-approve │ │ diff_file │ ✗ Auto-approve │ - │ math │ ✗ Auto-approve │ - │ man │ ✗ Auto-approve │ │ web_fetch │ ✗ Auto-approve │ │ web_search │ ✗ Auto-approve │ │ tool_search │ ✗ Auto-approve │ │ task_agent │ ✓ Yes │ - │ plan_agent │ ✓ Yes │ │ memory │ ✗ Auto-approve │ │ recall │ ✗ Auto-approve │ │ notify │ ✗ Auto-approve │ @@ -110,13 +107,10 @@ partition "Phase 3: Execute" #E3F2FD { ├─ _exec_write_file: makedirs + write ├─ _exec_edit_file: find_occurrences + replace ├─ _exec_search: grep subprocess - ├─ _exec_math: sandboxed subprocess - ├─ _exec_man: man/info subprocess ├─ _exec_web_fetch: httpx.get + LLM summary ├─ _exec_web_search: SearxNG JSON GET (fallback for local models) ├─ _exec_tool_search: BM25 search + expand_visible() ├─ _exec_task: _run_agent(TASK_AGENT_TOOLS) - ├─ _exec_plan: _run_agent(AGENT_TOOLS, read-only) ├─ _exec_notify: HTTP POST to channel gateway ├─ _exec_memory: structured memory save/search/delete/list ├─ _exec_recall: conversation history FTS5 search @@ -131,11 +125,6 @@ partition "Phase 3: Execute" #E3F2FD { :bash: ui.on_tool_output_chunk(call_id, line) per stdout line; :ui.on_tool_result(call_id, name, output, is_error) for each; - - if (plan tool was executed?) then (yes) - :ui.on_plan_review(output); - :Block for user review/feedback; - endif } :Return (results, user_feedback); diff --git a/docs/diagrams/09-workstream-states.puml b/docs/diagrams/09-workstream-states.puml index c4e67b0e..746af964 100644 --- a/docs/diagrams/09-workstream-states.puml +++ b/docs/diagrams/09-workstream-states.puml @@ -13,7 +13,7 @@ skinparam state { state "IDLE" as idle <> : Waiting for user input.\nNo active LLM call or tool execution. state "THINKING" as thinking <> : LLM streaming response.\nTokens flowing (reasoning + content). state "RUNNING" as running <> : Tools executing.\nThreadPoolExecutor active. -state "ATTENTION" as attention <> : Blocked on user action.\nTool approval or plan review needed. +state "ATTENTION" as attention <> : Blocked on user action.\nTool approval needed. state "ERROR" as error <> : Exception occurred.\nRecoverable on next send(). [*] --> idle : Session created @@ -34,8 +34,6 @@ attention --> running : User denies\n(denial recorded)\n_emit_state("running") running --> thinking : Tool results appended,\nnext LLM call\n_emit_state("thinking") -running --> attention : Plan tool complete,\non_plan_review()\n_emit_state("attention") - running --> error : Exception during\ntool execution error --> thinking : New send() call\n_emit_state("thinking") @@ -44,7 +42,7 @@ thinking --> idle : cancel() called\nstream aborted\n_emit_state("idle") running --> idle : cancel() called\n_emit_state("idle") -attention --> idle : cancel() unblocks\napproval/plan wait\n_emit_state("idle") +attention --> idle : cancel() unblocks\napproval wait\n_emit_state("idle") note left of idle **Cancel escalation:** diff --git a/docs/diagrams/13-sdk-architecture.puml b/docs/diagrams/13-sdk-architecture.puml index 7bc3ab3a..6a5c4e3a 100644 --- a/docs/diagrams/13-sdk-architecture.puml +++ b/docs/diagrams/13-sdk-architecture.puml @@ -30,7 +30,6 @@ package "turnstone/sdk/ (Python)" { + close_workstream() + send(message, ws_id) + approve() - + plan_feedback() + command() + cancel(ws_id) + stream_events(ws_id) diff --git a/docs/diagrams/22-judge-architecture.puml b/docs/diagrams/22-judge-architecture.puml index 622373dc..e5c2d3e4 100644 --- a/docs/diagrams/22-judge-architecture.puml +++ b/docs/diagrams/22-judge-architecture.puml @@ -202,7 +202,7 @@ note over Session, Judge Cross-model: separate provider/client from [judge] config. **Sub-agent exemption:** - Plan agent and task agent skip intent validation entirely. + Task sub-agents skip intent validation entirely. **Output guard:** Runs when judge_config.output_guard is true (default). diff --git a/docs/eval.md b/docs/eval.md index 64b8372c..9c41bea9 100644 --- a/docs/eval.md +++ b/docs/eval.md @@ -274,8 +274,7 @@ for iteration in 0..max_iterations: ### Phase 1: Analyst (`_run_analyst`) -A multi-turn agent with `math` (Python) and `bash` tools for computing -statistics. It receives per-case results with failure classifications and +A multi-turn agent with a `bash` tool for computing statistics. It receives per-case results with failure classifications and produces a structured diagnosis: - **Failure patterns**: Shared root causes across failing cases diff --git a/docs/judge.md b/docs/judge.md index a6881413..18e7f214 100644 --- a/docs/judge.md +++ b/docs/judge.md @@ -125,7 +125,7 @@ last) and returns the first matching rule. Each rule has: | Critical | 0.90 | deny | `rm -rf /`, `mkfs`, `dd if=`, pipe-to-shell, chmod 777 on root, write/edit to `/etc/` or `.ssh/`, download-then-execute chains (`curl -o file && chmod +x && bash`) | | High | 0.80 | review | `sudo`, `kill -9`, destructive git, DROP TABLE, write/edit secrets, HTTP mutations, `ssh`/`scp`, credential file access, browser automation + data export, transitive installs (`npx skills add`, `pip install git+`), control plane mutations (`crontab`, `systemctl enable/start/stop`) | | Medium | 0.70 | review | Content ingestion pipelines (`curl \| python3`), interpreter execution (`python3 script.py`, `node build.js`), cloud CLI mutations (`az/gcloud/aws/kubectl/terraform` with create/delete/destroy verbs), package installs, `write_file`, MCP tools, Docker operations | -| Low | 0.85 | approve | `read_file`, `list_directory`, `search`, `recall`, `man`, `use_prompt`, `tool_search`, `read_resource`, `web_search`, read-only bash (`ls`, `cat`, `head`, `grep`, `find`, etc.) | +| Low | 0.85 | approve | `read_file`, `list_directory`, `search`, `recall`, `use_prompt`, `tool_search`, `read_resource`, `web_search`, read-only bash (`ls`, `cat`, `head`, `grep`, `find`, etc.) | When no rule matches, the heuristic returns a default verdict: medium risk, 0.50 confidence, "review" recommendation. diff --git a/docs/sdk.md b/docs/sdk.md index a4c0ae14..884a64be 100644 --- a/docs/sdk.md +++ b/docs/sdk.md @@ -77,7 +77,6 @@ Both `TurnstoneServer` (sync) and `AsyncTurnstoneServer` (async) expose: | | `delete_attachment(ws_id, attachment_id)` | `StatusResponse` | | **Chat** | `send(message, ws_id)` | `SendResponse` | | | `approve(*, ws_id, approved, feedback, always)` | `StatusResponse` | -| | `plan_feedback(*, ws_id, feedback)` | `StatusResponse` | | | `command(*, ws_id, command)` | `StatusResponse` | | | `cancel(ws_id, *, force=False)` | `StatusResponse` | | **Streaming** | `stream_events(ws_id)` | `Iterator[ServerEvent]` | @@ -134,7 +133,6 @@ SSE events are deserialized into typed dataclasses. Use `event.type` to discrimi | `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` | | `error` | `ErrorEvent` | `message` | | `info` | `InfoEvent` | `message` | | `stream_end` | `StreamEndEvent` | — | diff --git a/docs/security.md b/docs/security.md index ba111e93..782d4d02 100644 --- a/docs/security.md +++ b/docs/security.md @@ -67,7 +67,7 @@ Scopes are hierarchical — higher scopes imply all lower ones. | Method | Path pattern | Required scope | |--------|-------------|----------------| | GET | Any protected path | `read` | -| POST | `/api/plan`, `/api/command` | `write` | +| POST | `/api/command` | `write` | | POST | `/api/workstreams/new`, `/api/cluster/workstreams/new` | `write` | | POST | `/api/workstreams/{ws_id}/{send,cancel,close,delete,open,refresh-title,title,attachments}` | `write` | | DELETE | `/api/workstreams/{ws_id}/send` (dequeue), `/api/workstreams/{ws_id}/attachments/{attachment_id}` | `write` | diff --git a/docs/settings.md b/docs/settings.md index ad36742c..07b6badf 100644 --- a/docs/settings.md +++ b/docs/settings.md @@ -74,20 +74,17 @@ provider-side mechanics (Anthropic `thinking`, OpenAI Responses `reasoning` + `include=["reasoning.encrypted_content"]`, synthetic `reasoning_text` for Chat Completions / vLLM / llama.cpp / Gemini-compat). -### Plan / task agent overrides +### Task agent overrides -`plan_agent` and `task_agent` sub-sessions resolve independently from the -conversation model so operators can pick a cheaper/faster model for -autonomous loops: +`task_agent` sub-sessions resolve independently from the conversation model +so operators can pick a cheaper/faster model for autonomous loops: | Setting | Purpose | |---------|---------| -| `model.plan_alias` | Alias used for `plan_agent` sub-sessions. Falls back to `[model].plan_model` in config.toml, then `[model].agent_model`, then the session's active model. | -| `model.task_alias` | Alias used for `task_agent` sub-sessions. Same fallback chain as `plan_alias`. | -| `model.plan_effort` | Reasoning effort for `plan_agent` (`none` / `minimal` / `low` / `medium` / `high` / `xhigh` / `max`). Defaults to `high`. | +| `model.task_alias` | Alias used for `task_agent` sub-sessions. Falls back to `[model].agent_model` in config.toml, then the session's active model. | | `model.task_effort` | Reasoning effort for `task_agent`. Empty string means "inherit from the session". | -All four are live-editable from the Settings tab and take effect on the +Both are live-editable from the Settings tab and take effect on the next sub-agent invocation — no restart required. --- @@ -110,7 +107,7 @@ initialization: | Section | Settings | |---------|----------| -| `model` | default_alias, temperature, max_tokens, reasoning_effort, plan_alias, task_alias, plan_effort, task_effort | +| `model` | default_alias, temperature, max_tokens, reasoning_effort, task_alias, task_effort | | `session` | instructions, retention_days, compact_max_tokens, auto_compact_pct | | `tools` | timeout, truncation, agent_max_turns, skip_permissions, search, search_threshold, search_max_results | | `server` | workstream_idle_timeout, max_workstreams | diff --git a/docs/tools.md b/docs/tools.md index 9497a005..2ea2bbd2 100644 --- a/docs/tools.md +++ b/docs/tools.md @@ -1,6 +1,6 @@ # Tools Reference -turnstone exposes 19 built-in tools plus any number of external MCP tools to the +turnstone exposes 16 built-in tools plus any number of external MCP tools to the LLM via the OpenAI function-calling interface. Built-in tools are defined as JSON files under `turnstone/tools/` and loaded at startup by `turnstone/core/tools.py`. MCP tools are discovered from configured MCP servers at startup by @@ -22,7 +22,6 @@ schema plus turnstone-specific metadata keys: "properties": { ... }, "required": ["param1"] }, - "agent": true, "task_agent": true, "auto_approve": true, "primary_key": "param1" @@ -33,8 +32,7 @@ schema plus turnstone-specific metadata keys: | Key | Type | Meaning | |----------------|------|---------| -| `agent` | bool | Tool is available to plan/task sub-agents (read-only subset). | -| `task_agent` | bool | Tool is available to task sub-agents (broader subset). | +| `task_agent` | bool | Tool is available to task sub-agents. | | `auto_approve` | bool | Tool runs without user confirmation (read-only, safe operations). | | `primary_key` | str | When the model sends a bare string instead of JSON args, map it to this parameter name. | @@ -46,12 +44,10 @@ schema plus turnstone-specific metadata keys: | Name | Description | |---------------------|-------------| -| `TOOLS` | All 19 tool definitions (sent to the model). | -| `AGENT_TOOLS` | Tools with `agent: true` -- available to plan sub-agents. Read-only tools. | +| `TOOLS` | All 28 loaded built-in tool definitions (interactive + coordinator union). Sessions send a kind-specific subset (`INTERACTIVE_TOOLS` or `COORDINATOR_TOOLS`). | | `TASK_AGENT_TOOLS` | Tools with `task_agent: true` -- available to task sub-agents. Includes write operations. | -| `AGENT_AUTO_TOOLS` | Set of tool names with `auto_approve: true` -- no user confirmation needed. | -| `TASK_AUTO_TOOLS` | Same as `AGENT_AUTO_TOOLS` (identical filter). | -| `BUILTIN_TOOL_NAMES`| Frozenset of all 19 built-in tool names. Used by tool search to distinguish always-on tools from deferrable MCP tools. | +| `TASK_AUTO_TOOLS` | Set of all tool names with `auto_approve: true` -- used by task-agent sub-sessions to skip confirmation for matching available tools. | +| `BUILTIN_TOOL_NAMES`| Frozenset of all 28 built-in tool names (interactive + coordinator union). Used by tool search to distinguish always-on tools from deferrable MCP tools. | | `PRIMARY_KEY_MAP` | Dict mapping tool name to its `primary_key` parameter name. | --- @@ -69,7 +65,7 @@ Tool execution follows a three-phase pipeline inside `ChatSession._execute_tools - Parses the JSON arguments (with fallback for malformed JSON). - If JSON parsing fails entirely, uses `PRIMARY_KEY_MAP` to map a bare string to the correct parameter. -- Dispatches to the matching `_prepare_{func_name}()` handler. There are 19 +- Dispatches to the matching `_prepare_{func_name}()` handler. There are 16 built-in tools plus `tool_search` (synthetic, client-side BM25 fallback) and the generic `_prepare_mcp_tool()` handler for MCP tools. - Validates arguments and builds a preview dict containing: @@ -111,9 +107,6 @@ Each item's `execute` callable is invoked: 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. - --- ## Tool Approval Flow @@ -121,7 +114,6 @@ Each item's `execute` callable is invoked: **Auto-approved** (no user confirmation needed at runtime): - `read_file` -- reads files, no side effects - `search` -- grep-style search, no side effects -- `man` -- reads man pages, no side effects - `memory` -- structured persistent memory (save/search/delete/list) - `recall` -- searches conversation history - `notify` -- sends notifications to linked channels (time-sensitive, auto-approved for urgency) @@ -130,16 +122,14 @@ Each item's `execute` callable is invoked: - `bash` -- arbitrary command execution - `write_file` -- creates or overwrites files - `edit_file` -- modifies file content -- `math` -- sandboxed computation (confirmation required despite being sandboxed) - `web_fetch` -- fetches a URL (SSRF-protected, but makes network requests) - `web_search` -- web search via self-hosted SearxNG (makes network requests) -- `task` -- spawns an autonomous sub-agent -- `plan` -- spawns a planning sub-agent, plus post-execution review gate +- `task_agent` -- spawns an autonomous sub-agent Note: The JSON schema metadata key `auto_approve` controls membership in -`AGENT_AUTO_TOOLS`/`TASK_AUTO_TOOLS` (used for agent sub-sessions). The actual -runtime approval behavior is determined by the `needs_approval` field set in -each `_prepare_*` method on `ChatSession`. These two mechanisms can differ. +`TASK_AUTO_TOOLS` (used for task agent sub-sessions). The actual runtime +approval behavior is determined by the `needs_approval` field set in each +`_prepare_*` method on `ChatSession`. These two mechanisms can differ. --- @@ -165,12 +155,9 @@ Every tool defines a `primary_key`. The mapping is: | `write_file` | `content` | | `edit_file` | `old_string`| | `search` | `query` | -| `math` | `code` | -| `man` | `page` | | `web_fetch` | `url` | | `web_search` | `query` | | `task_agent` | `prompt` | -| `plan_agent` | `goal` | | `memory` | `name` | | `recall` | `query` | | `notify` | `message` | @@ -194,7 +181,7 @@ Execute a bash command and return stdout + stderr. - **What it does**: Runs the command in a subprocess with a configurable timeout. Commands are sanitized and checked against a blocklist (e.g. `rm -rf /`). Environment variables containing secrets are scrubbed (`*_KEY`, `*_SECRET`, `*_TOKEN`, etc.). - **Output format**: Stdout is returned directly. Stderr lines are prefixed with `[stderr]` so the model can distinguish them. When the command itself redirects stderr to stdout (`2>&1`), no prefix is added. Output exceeding 256KB is truncated (head + tail preserved, middle replaced with a truncation notice). - **Auto-approve**: No -- requires user confirmation. -- **Agent availability**: `task_agent` only (not available to plan sub-agents). +- **Agent availability**: `task_agent` only. --- @@ -212,7 +199,7 @@ base64-encoded image data for supported image formats. - **What it does**: For text files, reads and returns content with line numbers. For image files (PNG, JPEG, GIF, WebP, BMP, TIFF, ICO), returns image data as multi-part content when the model supports vision, or a text description when it does not. SVG files are read as text. Images larger than 4 MB are rejected. Must be called before `edit_file` on the same path (the session tracks which files have been read). - **Vision support**: Controlled by `ModelCapabilities.supports_vision`. All commercial OpenAI and Anthropic models have vision enabled. Local models (vLLM, llama.cpp, NIM) default to off — enable via `[models.*.capabilities] supports_vision = true` in config.toml. - **Auto-approve**: Yes. -- **Agent availability**: `agent` and `task_agent`. +- **Agent availability**: `task_agent`. --- @@ -268,7 +255,7 @@ Show a unified diff between two files, or between a file and a provided string. - **What it does**: Returns unified diff output using Python's `difflib`. Binary files (containing null bytes) are rejected with a clear error. Files read through `diff_file` satisfy `edit_file`'s read guard — you can diff then edit without a separate `read_file` call. Large diffs are streamed with early cutoff at the tool truncation limit. - **Auto-approve**: Yes (read-only). -- **Agent availability**: `agent` and `task_agent`. +- **Agent availability**: `task_agent`. --- @@ -283,44 +270,12 @@ Search file contents for a regex pattern. - **What it does**: Recursively searches for the pattern using `grep -rn`. Returns matching lines with file paths and line numbers. - **Auto-approve**: Yes. -- **Agent availability**: `agent` and `task_agent`. - ---- - -## Computation - -### math - -Execute Python code for math and computation in a sandbox. - -| Parameter | Type | Required | Description | -|-----------|--------|----------|-------------| -| `code` | string | yes | Python code to execute. Must use `print()` for output. | - -- **What it does**: Runs Python code in a sandboxed environment with pre-imported libraries: `sympy`, `numpy`, `scipy`, `math`, `fractions`, `itertools`, `functools`, `collections`, `decimal`, `operator`, `random`, `re`, `string`. Common sympy names (`symbols`, `solve`, `simplify`, `sqrt`, `Matrix`, etc.) are pre-imported. `pytest` is also available for import. -- **Installation**: `sympy`, `numpy`, `scipy`, and `pytest` require the `[sandbox]` extras group: `pip install turnstone[sandbox]` (included in `[all]`). -- **Auto-approve**: Yes. -- **Agent availability**: `agent` and `task_agent`. +- **Agent availability**: `task_agent`. --- ## Information -### man - -Read a man page. - -| Parameter | Type | Required | Description | -|-----------|--------|----------|-------------| -| `page` | string | yes | The man page name (e.g. `grep`, `socket`, `printf`). | -| `section` | string | no | Manual section (e.g. `1` commands, `2` syscalls, `3` library). | - -- **What it does**: Returns the full formatted manual entry. Preferred over `bash('man ...')` or `web_search` for command/API documentation. -- **Auto-approve**: Yes. -- **Agent availability**: `agent` and `task_agent`. - ---- - ### web_fetch Fetch a URL and extract specific information from it. @@ -332,7 +287,7 @@ Fetch a URL and extract specific information from it. - **What it does**: Fetches the URL, strips HTML to plain text, and uses the LLM to extract the answer to the question from the page content. Protected against SSRF (blocks private/internal IPs). - **Auto-approve**: No -- requires user confirmation (makes network requests). -- **Agent availability**: `agent` and `task_agent`. +- **Agent availability**: `task_agent`. --- @@ -351,13 +306,13 @@ Search the web using a text query. - **OpenAI search models** (`gpt-5-search-api`): Replaced with `web_search_options` parameter. The model always searches and returns `url_citation` annotations. - **Local/vLLM models**: Falls back to a self-hosted [SearxNG](https://searxng.org) instance. Set `searxng_url` in `config.toml` `[tools]` or `$TURNSTONE_SEARXNG_URL` (the docker-compose stack bundles a `searxng` service and points at it by default). Operators with a custom MCP search server can instead set `web_search_backend = "mcp:server:tool"`. - **Auto-approve**: Yes (auto-approved for all tool dispatch paths). -- **Agent availability**: `agent` and `task_agent`. +- **Agent availability**: `task_agent`. --- ## Agent -Tool names use the `_agent` suffix — bare `plan` / `task` collide with +The tool name uses the `_agent` suffix — bare `task` collides with chat-template channel names on some local models. ### task_agent @@ -368,23 +323,9 @@ Delegate a general-purpose task to an autonomous sub-agent. |-----------|--------|----------|-------------| | `prompt` | string | yes | Complete task description for the sub-agent. | -- **What it does**: Spawns a sub-agent that inherits the `TASK_AGENT_TOOLS` set (read, write, edit, search, bash, math, man, web tools, memory tools). The sub-agent runs autonomously to completion. Use for work that requires file modifications or command execution. +- **What it does**: Spawns a sub-agent that inherits the `TASK_AGENT_TOOLS` set (read, write, edit, search, bash, web tools, memory tools). The sub-agent runs autonomously to completion. Use for work that requires file modifications or command execution. - **Auto-approve**: No -- requires user confirmation. -- **Agent availability**: Not available to sub-agents (top-level only). - ---- - -### plan_agent - -Plan before implementing -- an autonomous agent explores the codebase and writes a structured plan. - -| Parameter | Type | Required | Description | -|-----------|--------|----------|-------------| -| `prompt` | string | yes | What to plan -- the goal, constraints, and scope. | - -- **What it does**: Spawns a planning sub-agent with `AGENT_TOOLS` (read-only tools: `read_file`, `search`, `math`, `man`, `web_fetch`, `web_search`). The agent explores the codebase and writes a structured plan to `.plan-.md` (unique per workstream, so concurrent workstreams never collide). If the `plan` tool has been called before in the same session, the prior plan is passed to the agent as context so it refines rather than restarts. After completion, the user is prompted to review and can accept, reject, or annotate the plan. -- **Auto-approve**: No -- requires user confirmation, plus post-execution review gate. -- **Agent availability**: Not available to sub-agents (top-level only). +- **Agent availability**: Top-level only. --- @@ -445,7 +386,7 @@ Provide either `username` for user-based targeting or `channel_type` + - **What it does**: Sends a notification via the channel gateway's HTTP endpoint (`POST /v1/api/notify`). The server queries the `services` table for healthy channel gateways, authenticates with a service JWT (`aud: turnstone-channel`), and delivers to the first healthy gateway. On failure, retries up to 2 additional times with backoff (1s, 3s). Rate-limited to 5 notifications per turn (counter only increments on success). - **Auto-approve**: Yes — notifications are time-sensitive and auto-approved so the model can alert users urgently. -- **Agent availability**: `agent` and `task_agent`. +- **Agent availability**: `task_agent`. > See [Channel Integrations: Notifications](channels.md#notifications) > for the full delivery flow, service registry details, and security @@ -521,7 +462,7 @@ data.get("mergedAt") is not None - Duplicate names rejected within the same workstream. - **Auto-approve**: `create` requires approval; `list` and `cancel` are auto-approved. -- **Agent availability**: Main session only — not available to plan/task sub-agents. +- **Agent availability**: Main session only — not available to task sub-agents. > See [Watch Architecture](diagrams/png/18-watch-architecture.png) for the > full poll → evaluate → dispatch flow. @@ -554,33 +495,30 @@ pre-configure skills at workstream creation. - **Auto-approve**: `load` requires approval (changes session behavior); `search` is auto-approved (read-only). -- **Agent availability**: Main session only — not available to plan/task sub-agents. +- **Agent availability**: Main session only — not available to task sub-agents. --- ## Summary Table -| Tool | Category | Auto-approve | agent | task_agent | primary_key | -|--------------|------------|--------------|-------|------------|-------------| -| `bash` | File Ops | No | No | Yes | `command` | -| `read_file` | File Ops | Yes | Yes | Yes | `path` | -| `write_file` | File Ops | No | No | Yes | `content` | -| `edit_file` | File Ops | No | No | Yes | `old_string`| -| `search` | File Ops | Yes | Yes | Yes | `query` | -| `math` | Compute | No | Yes | Yes | `code` | -| `man` | Info | Yes | Yes | Yes | `page` | -| `web_fetch` | Info | No | Yes | Yes | `url` | -| `web_search` | Info | No | Yes | Yes | `query` | -| `task_agent` | Agent | No | No | No | `prompt` | -| `plan_agent` | Agent | No | No | No | `goal` | -| `memory` | Memory | Yes | No | No | `name` | -| `recall` | Memory | Yes | No | No | `query` | -| `notify` | Notify | Yes | Yes | Yes | `message` | -| `watch` | Monitor | No (create) | No | No | `command` | -| `read_resource`| MCP | No | Yes | Yes | `uri` | -| `use_prompt` | MCP | No | Yes | Yes | `name` | -| `skill` | Skills | No (load) | No | No | `name` | -| `tool_search`| Search | Yes | No | No | `query` | +| Tool | Category | Auto-approve | task_agent | primary_key | +|--------------|------------|--------------|------------|-------------| +| `bash` | File Ops | No | Yes | `command` | +| `read_file` | File Ops | Yes | Yes | `path` | +| `write_file` | File Ops | No | Yes | `content` | +| `edit_file` | File Ops | No | Yes | `old_string`| +| `search` | File Ops | Yes | Yes | `query` | +| `web_fetch` | Info | No | Yes | `url` | +| `web_search` | Info | No | Yes | `query` | +| `task_agent` | Agent | No | No | `prompt` | +| `memory` | Memory | Yes | No | `name` | +| `recall` | Memory | Yes | No | `query` | +| `notify` | Notify | Yes | Yes | `message` | +| `watch` | Monitor | No (create) | No | `command` | +| `read_resource`| MCP | No | Yes | `uri` | +| `use_prompt` | MCP | No | Yes | `name` | +| `skill` | Skills | No (load) | No | `name` | +| `tool_search`| Search | Yes | No | `query` | --- @@ -632,8 +570,9 @@ CLI flags override the config file: directly. 2. **Partitioning**: When active, tools are split into two sets: - - **Always-on** -- the 19 built-in tools (members of `BUILTIN_TOOL_NAMES`). - These are always visible to the model. + - **Always-on** -- the built-in tools present in the current session + (interactive sessions currently have 16; `BUILTIN_TOOL_NAMES` is the + 28-tool built-in union). These are always visible to the model. - **Deferred** -- all MCP tools. These are not sent in the tool list unless the model searches for them. @@ -647,10 +586,10 @@ CLI flags override the config file: ### Agent exemption -Plan and task sub-agents do not use tool search. They operate on scoped tool -sets (`AGENT_TOOLS` for plan agents, `TASK_AGENT_TOOLS` for task agents) with -MCP tools merged in. Tool search is only active for the top-level session, -where the model can interactively search for tools it needs. +Task sub-agents do not use tool search. They operate on the scoped tool set +(`TASK_AGENT_TOOLS`) with MCP tools merged in. Tool search is only active for +the top-level session, where the model can interactively search for tools it +needs. --- @@ -675,7 +614,7 @@ MCP-compatible service. 3. **Schema conversion**: Each MCP tool's `inputSchema` is converted to OpenAI function-calling format. The tool name is prefixed: `mcp__{server}__{tool}`. -4. **Merging**: MCP tools are appended after the 19 built-in tools via +4. **Merging**: MCP tools are appended after the 16 built-in tools via `merge_mcp_tools()`. Built-in tools appear first, giving them natural LLM priority. When dynamic tool search is active, MCP tools are deferred rather than directly visible -- the model discovers them via search as needed (see @@ -701,7 +640,6 @@ gives per-tool-type granularity (e.g., all `use_prompt` calls). MCP tools are available to: - **Main session** — full access - **Task sub-agents** — via `self._task_tools` (merged list) -- **Plan sub-agents** — via `self._agent_tools` (merged list) ### Naming convention @@ -774,7 +712,7 @@ MCP tool lists stay up-to-date without restart through two mechanisms: When tools change, `MCPClientManager` rebuilds its merged tool list using copy-on-write (new list/dict objects assigned atomically) and notifies all active `ChatSession` instances via registered listener callbacks. Each session rebuilds its `_tools`, -`_task_tools`, `_agent_tools`, and reconstructs its `ToolSearchManager` (if active), +`_task_tools`, and reconstructs its `ToolSearchManager` (if active), preserving the set of previously expanded (discovered) tools. ``` @@ -830,7 +768,7 @@ Use read_resource(uri='...') to access the resources listed above. - **What it does**: Reads the resource from its MCP server via `MCPClientManager.read_resource_sync()`. Returns text content for text resources or base64-encoded data for binary resources. Output is truncated by the standard tool output limiter. - **Auto-approve**: No -- requires user confirmation (reads external data). -- **Agent availability**: `agent` and `task_agent`. +- **Agent availability**: `task_agent`. ### Capability guards @@ -872,7 +810,7 @@ the `initialize` handshake. Each prompt is stored with its prefixed name - **What it does**: Invokes an MCP prompt by name via `MCPClientManager.get_prompt_sync()`, expanding it into messages. Returns the expanded prompt content formatted as `[role]: content` blocks joined with blank lines. The prompt catalog is listed in the system message so the model knows which prompts are available. Output is truncated by the standard tool output limiter. - **Auto-approve**: No -- requires user confirmation (invokes external prompt servers). -- **Agent availability**: `agent` and `task_agent`. +- **Agent availability**: `task_agent`. ### Invocation diff --git a/pyproject.toml b/pyproject.toml index b8fb8ff8..ca485ffe 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -52,9 +52,8 @@ anthropic = ["anthropic>=0.39"] postgres = ["psycopg[binary]>=3.2"] discord = ["discord.py>=2.4"] tls = ["lacme>=1.0.5"] -sandbox = ["sympy>=1.13", "numpy>=2.0", "scipy>=1.14", "pytest>=9.0"] slack = ["slack-bolt>=1.18", "aiohttp>=3.9"] -all = ["turnstone[console,anthropic,postgres,discord,tls,sandbox,slack]"] +all = ["turnstone[console,anthropic,postgres,discord,tls,slack]"] [project.scripts] turnstone = "turnstone.cli:main" @@ -102,7 +101,6 @@ line-length = 100 [tool.ruff.lint] select = ["E", "F", "W", "I", "N", "UP", "B", "A", "SIM", "TCH"] ignore = ["E501"] -per-file-ignores = { "turnstone/core/sandbox.py" = ["N802"] } [tool.ruff.format] quote-style = "double" @@ -132,10 +130,6 @@ exclude_lines = [ 'if __name__ == "__main__"', ] -[[tool.mypy.overrides]] -module = ["sympy", "sympy.*", "numpy", "numpy.*"] -ignore_missing_imports = true - [[tool.mypy.overrides]] module = ["mcp", "mcp.*"] ignore_missing_imports = true diff --git a/sdk/typescript/openapi-server.json b/sdk/typescript/openapi-server.json index 5ca4b1ec..9d64288f 100644 --- a/sdk/typescript/openapi-server.json +++ b/sdk/typescript/openapi-server.json @@ -10,9 +10,7 @@ "get": { "summary": "List active workstreams", "operationId": "v1_api_workstreams_get", - "tags": [ - "Workstreams" - ], + "tags": ["Workstreams"], "responses": { "200": { "description": "Success", @@ -31,9 +29,7 @@ "get": { "summary": "Dashboard with workstream details and aggregates", "operationId": "v1_api_dashboard_get", - "tags": [ - "Workstreams" - ], + "tags": ["Workstreams"], "responses": { "200": { "description": "Success", @@ -52,9 +48,7 @@ "post": { "summary": "Create a new workstream", "operationId": "v1_api_workstreams_new_post", - "tags": [ - "Workstreams" - ], + "tags": ["Workstreams"], "description": "Accepts two content types. Default is `application/json` with a `CreateWorkstreamRequest` body. Alternatively, `multipart/form-data` with one `meta` field (JSON-encoded `CreateWorkstreamRequest` shape) plus zero-or-more `file` parts saves each file as an attachment under the new workstream. When `initial_message` is also set, attachments are reserved onto that turn before the worker thread dispatches; otherwise they remain pending for a follow-up `POST /v1/api/workstreams/{ws_id}/send`.", "requestBody": { "required": true, @@ -114,9 +108,7 @@ "post": { "summary": "Close a workstream", "operationId": "v1_api_workstreams_{ws_id}_close_post", - "tags": [ - "Workstreams" - ], + "tags": ["Workstreams"], "parameters": [ { "name": "ws_id", @@ -175,9 +167,7 @@ "post": { "summary": "Send a user message", "operationId": "v1_api_workstreams_{ws_id}_send_post", - "tags": [ - "Chat" - ], + "tags": ["Chat"], "parameters": [ { "name": "ws_id", @@ -234,9 +224,7 @@ "delete": { "summary": "Cancel a queued message", "operationId": "v1_api_workstreams_{ws_id}_send_delete", - "tags": [ - "Chat" - ], + "tags": ["Chat"], "description": "Removes a previously-queued message from the workstream's pending queue. Returns ``status: removed`` when the queue had the entry, ``status: not_found`` otherwise.", "parameters": [ { @@ -296,9 +284,7 @@ "post": { "summary": "Approve or deny a tool call", "operationId": "v1_api_workstreams_{ws_id}_approve_post", - "tags": [ - "Chat" - ], + "tags": ["Chat"], "parameters": [ { "name": "ws_id", @@ -343,54 +329,11 @@ } } }, - "/v1/api/plan": { - "post": { - "summary": "Respond to a plan review", - "operationId": "v1_api_plan_post", - "tags": [ - "Chat" - ], - "requestBody": { - "required": true, - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/PlanFeedbackRequest" - } - } - } - }, - "responses": { - "200": { - "description": "Success", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/StatusResponse" - } - } - } - }, - "404": { - "description": "Error 404", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/ErrorResponse" - } - } - } - } - } - } - }, "/v1/api/command": { "post": { "summary": "Execute a slash command", "operationId": "v1_api_command_post", - "tags": [ - "Chat" - ], + "tags": ["Chat"], "requestBody": { "required": true, "content": { @@ -439,9 +382,7 @@ "post": { "summary": "Cancel the active generation in a workstream", "operationId": "v1_api_workstreams_{ws_id}_cancel_post", - "tags": [ - "Chat" - ], + "tags": ["Chat"], "parameters": [ { "name": "ws_id", @@ -500,9 +441,7 @@ "post": { "summary": "Drop the last N conversation turns (emits clear_ui)", "operationId": "v1_api_workstreams_{ws_id}_rewind_post", - "tags": [ - "Chat" - ], + "tags": ["Chat"], "parameters": [ { "name": "ws_id", @@ -561,9 +500,7 @@ "post": { "summary": "Drop the last response and re-send the last user message", "operationId": "v1_api_workstreams_{ws_id}_retry_post", - "tags": [ - "Chat" - ], + "tags": ["Chat"], "parameters": [ { "name": "ws_id", @@ -612,9 +549,7 @@ "get": { "summary": "Per-workstream SSE event stream", "operationId": "v1_api_workstreams_{ws_id}_events_get", - "tags": [ - "Streaming" - ], + "tags": ["Streaming"], "description": "Opens a Server-Sent Events stream scoped to a single workstream. Returns text/event-stream. See API reference for event types.", "parameters": [ { @@ -647,9 +582,7 @@ "get": { "summary": "Global SSE event stream", "operationId": "v1_api_events_global_get", - "tags": [ - "Streaming" - ], + "tags": ["Streaming"], "description": "Server-Sent Events stream for node-level state broadcasts. Emits a node_snapshot event on connect (workstreams, health, aggregate), followed by real-time delta events (ws_state, ws_activity, ws_created, ws_closed, ws_rename, health_changed, aggregate). Pass ?expected_node_id=X for identity verification (returns 409 on mismatch).", "responses": { "200": { @@ -662,9 +595,7 @@ "post": { "summary": "Permanently delete a saved workstream", "operationId": "v1_api_workstreams_{ws_id}_delete_post", - "tags": [ - "Workstreams" - ], + "tags": ["Workstreams"], "parameters": [ { "name": "ws_id", @@ -716,9 +647,7 @@ "post": { "summary": "Load a saved workstream into memory", "operationId": "v1_api_workstreams_{ws_id}_open_post", - "tags": [ - "Workstreams" - ], + "tags": ["Workstreams"], "parameters": [ { "name": "ws_id", @@ -770,9 +699,7 @@ "post": { "summary": "Set workstream title manually", "operationId": "v1_api_workstreams_{ws_id}_title_post", - "tags": [ - "Workstreams" - ], + "tags": ["Workstreams"], "parameters": [ { "name": "ws_id", @@ -814,9 +741,7 @@ "post": { "summary": "Regenerate workstream title via LLM", "operationId": "v1_api_workstreams_{ws_id}_refresh-title_post", - "tags": [ - "Workstreams" - ], + "tags": ["Workstreams"], "parameters": [ { "name": "ws_id", @@ -848,9 +773,7 @@ "get": { "summary": "Get workstream detail (rehydrates lazily on miss)", "operationId": "v1_api_workstreams_{ws_id}_get", - "tags": [ - "Workstreams" - ], + "tags": ["Workstreams"], "description": "Returns the persisted workstream's display fields. If the session isn't currently in memory the manager rehydrates it before responding; ``500`` on rehydrate failure carries a correlation id matching the server log line. Lifted from the coord-only surface in the Stage 2 history/detail verb lift \u2014 interactive previously had no detail endpoint.", "parameters": [ { @@ -920,9 +843,7 @@ "get": { "summary": "Read the workstream's reconstructed message history", "operationId": "v1_api_workstreams_{ws_id}_history_get", - "tags": [ - "Workstreams" - ], + "tags": ["Workstreams"], "description": "Returns the tail of the conversation in OpenAI-like message format. Persisted-but-not-loaded workstreams (closed / evicted) serve history without rehydrating. Lifted from the coord-only surface in the Stage 2 history/detail verb lift \u2014 interactive previously only exposed history through the SSE replay on ``/events``.", "parameters": [ { @@ -1002,9 +923,7 @@ "post": { "summary": "Upload a file (multipart/form-data, field 'file') and attach it to the caller's next user turn on this workstream. Validates size, MIME, and UTF-8 for text; magic-byte sniff for images. Ownership failures are masked as 404 so non-owners cannot enumerate workstream existence; a 403 indicates a scope/auth failure from the middleware layer.", "operationId": "v1_api_workstreams_{ws_id}_attachments_post", - "tags": [ - "Attachments" - ], + "tags": ["Attachments"], "parameters": [ { "name": "ws_id", @@ -1081,9 +1000,7 @@ "get": { "summary": "List the caller's pending (unconsumed) attachments for this workstream. Ownership failures are masked as 404.", "operationId": "v1_api_workstreams_{ws_id}_attachments_get", - "tags": [ - "Attachments" - ], + "tags": ["Attachments"], "parameters": [ { "name": "ws_id", @@ -1132,9 +1049,7 @@ "get": { "summary": "Return raw bytes of an attachment with its stored Content-Type. Ownership failures are masked as 404.", "operationId": "v1_api_workstreams_{ws_id}_attachments_{attachment_id}_content_get", - "tags": [ - "Attachments" - ], + "tags": ["Attachments"], "parameters": [ { "name": "ws_id", @@ -1184,9 +1099,7 @@ "delete": { "summary": "Remove a pending attachment (consumed attachments return 404). Ownership failures are also masked as 404.", "operationId": "v1_api_workstreams_{ws_id}_attachments_{attachment_id}_delete", - "tags": [ - "Attachments" - ], + "tags": ["Attachments"], "parameters": [ { "name": "ws_id", @@ -1236,9 +1149,7 @@ "get": { "summary": "List saved workstreams", "operationId": "v1_api_workstreams_saved_get", - "tags": [ - "Workstreams" - ], + "tags": ["Workstreams"], "responses": { "200": { "description": "Success", @@ -1257,9 +1168,7 @@ "get": { "summary": "List available skills (summary)", "operationId": "v1_api_skills_get", - "tags": [ - "Skills" - ], + "tags": ["Skills"], "responses": { "200": { "description": "Success", @@ -1278,9 +1187,7 @@ "get": { "summary": "List available model aliases", "operationId": "v1_api_models_get", - "tags": [ - "Models" - ], + "tags": ["Models"], "responses": { "200": { "description": "Success", @@ -1299,9 +1206,7 @@ "post": { "summary": "Authenticate with a token", "operationId": "v1_api_auth_login_post", - "tags": [ - "Auth" - ], + "tags": ["Auth"], "requestBody": { "required": true, "content": { @@ -1340,9 +1245,7 @@ "post": { "summary": "Create first admin user", "operationId": "v1_api_auth_setup_post", - "tags": [ - "Auth" - ], + "tags": ["Auth"], "requestBody": { "required": true, "content": { @@ -1401,9 +1304,7 @@ "get": { "summary": "Return auth state", "operationId": "v1_api_auth_status_get", - "tags": [ - "Auth" - ], + "tags": ["Auth"], "responses": { "200": { "description": "Success", @@ -1422,9 +1323,7 @@ "post": { "summary": "Clear auth cookie", "operationId": "v1_api_auth_logout_post", - "tags": [ - "Auth" - ], + "tags": ["Auth"], "responses": { "200": { "description": "Success", @@ -1443,9 +1342,7 @@ "get": { "summary": "Redirect to OIDC provider for SSO login", "operationId": "v1_api_auth_oidc_authorize_get", - "tags": [ - "Auth" - ], + "tags": ["Auth"], "responses": { "302": { "description": "Success" @@ -1477,9 +1374,7 @@ "get": { "summary": "OIDC callback \u2014 validates code, provisions user, sets JWT cookie, redirects to app", "operationId": "v1_api_auth_oidc_callback_get", - "tags": [ - "Auth" - ], + "tags": ["Auth"], "responses": { "302": { "description": "Success" @@ -1491,9 +1386,7 @@ "get": { "summary": "Return authenticated user info and permissions", "operationId": "v1_api_auth_whoami_get", - "tags": [ - "Auth" - ], + "tags": ["Auth"], "responses": { "200": { "description": "Success", @@ -1522,9 +1415,7 @@ "get": { "summary": "List structured memories", "operationId": "v1_api_memories_get", - "tags": [ - "Memories" - ], + "tags": ["Memories"], "parameters": [ { "name": "type", @@ -1580,9 +1471,7 @@ "post": { "summary": "Save (upsert) a structured memory", "operationId": "v1_api_memories_post", - "tags": [ - "Memories" - ], + "tags": ["Memories"], "requestBody": { "required": true, "content": { @@ -1621,9 +1510,7 @@ "post": { "summary": "Search structured memories by query", "operationId": "v1_api_memories_search_post", - "tags": [ - "Memories" - ], + "tags": ["Memories"], "requestBody": { "required": true, "content": { @@ -1652,9 +1539,7 @@ "delete": { "summary": "Delete a structured memory by name and scope", "operationId": "v1_api_memories_{name}_delete", - "tags": [ - "Memories" - ], + "tags": ["Memories"], "parameters": [ { "name": "name", @@ -1711,9 +1596,7 @@ "get": { "summary": "List interface.* settings with values and sources", "operationId": "v1_api_admin_settings_get", - "tags": [ - "Admin" - ], + "tags": ["Admin"], "responses": { "200": { "description": "Success" @@ -1725,9 +1608,7 @@ "put": { "summary": "Update an interface.* setting", "operationId": "v1_api_admin_settings_{key}_put", - "tags": [ - "Admin" - ], + "tags": ["Admin"], "parameters": [ { "name": "key", @@ -1767,9 +1648,7 @@ "post": { "summary": "Update an interface.* setting (alias for PUT)", "operationId": "v1_api_admin_settings_{key}_post", - "tags": [ - "Admin" - ], + "tags": ["Admin"], "parameters": [ { "name": "key", @@ -1811,9 +1690,7 @@ "get": { "summary": "Server health check", "operationId": "health_get", - "tags": [ - "Observability" - ], + "tags": ["Observability"], "responses": { "200": { "description": "Success", @@ -1840,9 +1717,7 @@ "type": "string" } }, - "required": [ - "error" - ], + "required": ["error"], "title": "ErrorResponse", "type": "object" }, @@ -1851,9 +1726,7 @@ "properties": { "status": { "default": "ok", - "examples": [ - "ok" - ], + "examples": ["ok"], "title": "Status", "type": "string" } @@ -1902,19 +1775,14 @@ }, "role": { "description": "Legacy role", - "examples": [ - "full", - "read" - ], + "examples": ["full", "read"], "title": "Role", "type": "string" }, "scopes": { "default": "", "description": "Comma-separated scopes", - "examples": [ - "read,write,approve" - ], + "examples": ["read,write,approve"], "title": "Scopes", "type": "string" }, @@ -1925,9 +1793,7 @@ "type": "string" } }, - "required": [ - "role" - ], + "required": ["role"], "title": "AuthLoginResponse", "type": "object" }, @@ -1950,11 +1816,7 @@ "type": "string" } }, - "required": [ - "username", - "display_name", - "password" - ], + "required": ["username", "display_name", "password"], "title": "AuthSetupRequest", "type": "object" }, @@ -1991,10 +1853,7 @@ "type": "string" } }, - "required": [ - "user_id", - "username" - ], + "required": ["user_id", "username"], "title": "AuthSetupResponse", "type": "object" }, @@ -2029,11 +1888,7 @@ "type": "boolean" } }, - "required": [ - "auth_enabled", - "has_users", - "setup_required" - ], + "required": ["auth_enabled", "has_users", "setup_required"], "title": "AuthStatusResponse", "type": "object" }, @@ -2061,9 +1916,7 @@ "title": "Attachment Ids" } }, - "required": [ - "message" - ], + "required": ["message"], "title": "SendRequest", "type": "object" }, @@ -2071,12 +1924,7 @@ "properties": { "status": { "description": "'ok', 'busy', 'queued', or 'queue_full'", - "examples": [ - "ok", - "busy", - "queued", - "queue_full" - ], + "examples": ["ok", "busy", "queued", "queue_full"], "title": "Status", "type": "string" }, @@ -2123,9 +1971,7 @@ "title": "Msg Id" } }, - "required": [ - "status" - ], + "required": ["status"], "title": "SendResponse", "type": "object" }, @@ -2138,9 +1984,7 @@ "type": "string" } }, - "required": [ - "msg_id" - ], + "required": ["msg_id"], "title": "DequeueRequest", "type": "object" }, @@ -2171,32 +2015,10 @@ "type": "boolean" } }, - "required": [ - "approved" - ], + "required": ["approved"], "title": "ApproveRequest", "type": "object" }, - "PlanFeedbackRequest": { - "properties": { - "feedback": { - "description": "Feedback text; empty string means approval", - "title": "Feedback", - "type": "string" - }, - "ws_id": { - "description": "Target workstream ID", - "title": "Ws Id", - "type": "string" - } - }, - "required": [ - "feedback", - "ws_id" - ], - "title": "PlanFeedbackRequest", - "type": "object" - }, "CommandRequest": { "properties": { "command": { @@ -2210,10 +2032,7 @@ "type": "string" } }, - "required": [ - "command", - "ws_id" - ], + "required": ["command", "ws_id"], "title": "CommandRequest", "type": "object" }, @@ -2238,9 +2057,7 @@ "type": "integer" } }, - "required": [ - "turns" - ], + "required": ["turns"], "title": "RewindRequest", "type": "object" }, @@ -2337,10 +2154,7 @@ }, "WorkstreamKind": { "description": "Classifier for which manager hosts a workstream.\n\nStrEnum so members are drop-in ``str`` replacements for the DB column,\nJSON payloads, and existing ``==`` comparisons against raw strings.\nNarrow internal annotations to this type; wide boundaries (HTTP body,\nDB row) stay ``str`` and parse via ``WorkstreamKind(raw)`` / ``from_raw``\nat the edge.", - "enum": [ - "interactive", - "coordinator" - ], + "enum": ["interactive", "coordinator"], "title": "WorkstreamKind", "type": "string" }, @@ -2377,10 +2191,7 @@ "type": "array" } }, - "required": [ - "ws_id", - "name" - ], + "required": ["ws_id", "name"], "title": "CreateWorkstreamResponse", "type": "object" }, @@ -2415,9 +2226,7 @@ "type": "array" } }, - "required": [ - "workstreams" - ], + "required": ["workstreams"], "title": "ListWorkstreamsResponse", "type": "object" }, @@ -2458,11 +2267,7 @@ "type": "string" } }, - "required": [ - "ws_id", - "name", - "state" - ], + "required": ["ws_id", "name", "state"], "title": "WorkstreamInfo", "type": "object" }, @@ -2508,12 +2313,7 @@ "description": "Inline approval payload \u2014 same shape as ``DashboardWorkstream.pending_approval_detail``. ``None`` when no approval is pending. Lets a reload paint the action row + judge verdicts immediately instead of relying on the SSE approve_request replay timing window." } }, - "required": [ - "ws_id", - "name", - "state", - "user_id" - ], + "required": ["ws_id", "name", "state", "user_id"], "title": "WorkstreamDetailResponse", "type": "object" }, @@ -2635,9 +2435,7 @@ "type": "array" } }, - "required": [ - "ws_id" - ], + "required": ["ws_id"], "title": "WorkstreamHistoryResponse", "type": "object" }, @@ -2654,10 +2452,7 @@ "$ref": "#/components/schemas/DashboardAggregate" } }, - "required": [ - "workstreams", - "aggregate" - ], + "required": ["workstreams", "aggregate"], "title": "DashboardResponse", "type": "object" }, @@ -2799,11 +2594,7 @@ "type": "array" } }, - "required": [ - "ws_id", - "name", - "state" - ], + "required": ["ws_id", "name", "state"], "title": "DashboardWorkstream", "type": "object" }, @@ -2851,9 +2642,7 @@ "type": "array" } }, - "required": [ - "workstreams" - ], + "required": ["workstreams"], "title": "ListSavedWorkstreamsResponse", "type": "object" }, @@ -2953,12 +2742,7 @@ "type": "number" } }, - "required": [ - "ws_id", - "created", - "updated", - "message_count" - ], + "required": ["ws_id", "created", "updated", "message_count"], "title": "SavedWorkstreamInfo", "type": "object" }, @@ -2987,10 +2771,7 @@ }, "kind": { "description": "'image' or 'text'", - "examples": [ - "image", - "text" - ], + "examples": ["image", "text"], "title": "Kind", "type": "string" } @@ -3016,9 +2797,7 @@ "type": "array" } }, - "required": [ - "attachments" - ], + "required": ["attachments"], "title": "ListAttachmentsResponse", "type": "object" }, @@ -3046,10 +2825,7 @@ }, "kind": { "description": "'image' or 'text'", - "examples": [ - "image", - "text" - ], + "examples": ["image", "text"], "title": "Kind", "type": "string" } @@ -3067,10 +2843,7 @@ "HealthResponse": { "properties": { "status": { - "examples": [ - "ok", - "degraded" - ], + "examples": ["ok", "degraded"], "title": "Status", "type": "string" }, @@ -3134,26 +2907,19 @@ "default": null } }, - "required": [ - "status" - ], + "required": ["status"], "title": "HealthResponse", "type": "object" }, "BackendStatus": { "properties": { "status": { - "examples": [ - "up", - "down" - ], + "examples": ["up", "down"], "title": "Status", "type": "string" } }, - "required": [ - "status" - ], + "required": ["status"], "title": "BackendStatus", "type": "object" }, @@ -3236,23 +3002,14 @@ "type": { "default": "project", "description": "Memory type", - "enum": [ - "user", - "project", - "feedback", - "reference" - ], + "enum": ["user", "project", "feedback", "reference"], "title": "Type", "type": "string" }, "scope": { "default": "global", "description": "Memory scope", - "enum": [ - "global", - "workstream", - "user" - ], + "enum": ["global", "workstream", "user"], "title": "Scope", "type": "string" }, @@ -3263,10 +3020,7 @@ "type": "string" } }, - "required": [ - "name", - "content" - ], + "required": ["name", "content"], "title": "SaveMemoryRequest", "type": "object" }, @@ -3286,21 +3040,12 @@ "type": "string" }, "type": { - "enum": [ - "user", - "project", - "feedback", - "reference" - ], + "enum": ["user", "project", "feedback", "reference"], "title": "Type", "type": "string" }, "scope": { - "enum": [ - "global", - "workstream", - "user" - ], + "enum": ["global", "workstream", "user"], "title": "Scope", "type": "string" }, @@ -3349,9 +3094,7 @@ "type": "integer" } }, - "required": [ - "memories" - ], + "required": ["memories"], "title": "ListMemoriesResponse", "type": "object" }, @@ -3365,25 +3108,14 @@ "type": { "default": "", "description": "Filter by memory type", - "enum": [ - "", - "user", - "project", - "feedback", - "reference" - ], + "enum": ["", "user", "project", "feedback", "reference"], "title": "Type", "type": "string" }, "scope": { "default": "", "description": "Filter by scope", - "enum": [ - "", - "global", - "workstream", - "user" - ], + "enum": ["", "global", "workstream", "user"], "title": "Scope", "type": "string" }, @@ -3402,9 +3134,7 @@ "type": "integer" } }, - "required": [ - "query" - ], + "required": ["query"], "title": "SearchMemoriesRequest", "type": "object" }, @@ -3466,9 +3196,7 @@ "type": "string" } }, - "required": [ - "name" - ], + "required": ["name"], "title": "SkillSummary", "type": "object" }, @@ -3482,9 +3210,7 @@ "type": "array" } }, - "required": [ - "skills" - ], + "required": ["skills"], "title": "ListSkillSummaryResponse", "type": "object" }, @@ -3503,11 +3229,7 @@ "type": "string" } }, - "required": [ - "alias", - "model", - "provider" - ], + "required": ["alias", "model", "provider"], "title": "AvailableModelInfo", "type": "object" }, diff --git a/sdk/typescript/src/events.ts b/sdk/typescript/src/events.ts index d344f2ee..63984977 100644 --- a/sdk/typescript/src/events.ts +++ b/sdk/typescript/src/events.ts @@ -109,16 +109,6 @@ export interface StatusEvent { turn_count?: number; } -export interface PlanReviewEvent { - type: "plan_review"; - content: string; -} - -export interface PlanResolvedEvent { - type: "plan_resolved"; - feedback: string; -} - export interface InfoEvent { type: "info"; message: string; @@ -192,8 +182,6 @@ export type ServerEvent = | ToolResultEvent | ToolOutputChunkEvent | StatusEvent - | PlanReviewEvent - | PlanResolvedEvent | InfoEvent | ErrorEvent | BusyErrorEvent @@ -314,14 +302,6 @@ export function isApprovalResolvedEvent( return e.type === "approval_resolved"; } -export function isPlanReviewEvent(e: ServerEvent): e is PlanReviewEvent { - return e.type === "plan_review"; -} - -export function isPlanResolvedEvent(e: ServerEvent): e is PlanResolvedEvent { - return e.type === "plan_resolved"; -} - export function isCancelledEvent(e: ServerEvent): e is CancelledEvent { return e.type === "cancelled"; } diff --git a/sdk/typescript/src/index.ts b/sdk/typescript/src/index.ts index 971ea9a7..4577e68c 100644 --- a/sdk/typescript/src/index.ts +++ b/sdk/typescript/src/index.ts @@ -42,7 +42,6 @@ export type { ToolResultEvent, ToolOutputChunkEvent, StatusEvent, - PlanReviewEvent, InfoEvent, ErrorEvent, BusyErrorEvent, @@ -71,7 +70,6 @@ export { isWsStateEvent, isApproveRequestEvent, isApprovalResolvedEvent, - isPlanReviewEvent, isCancelledEvent, } from "./events.js"; @@ -80,7 +78,6 @@ export type { SendRequest, SendResponse, ApproveRequest, - PlanFeedbackRequest, CommandRequest, CreateWorkstreamRequest, CreateWorkstreamResponse, diff --git a/sdk/typescript/src/server.ts b/sdk/typescript/src/server.ts index 1f6ef711..fd49e39c 100644 --- a/sdk/typescript/src/server.ts +++ b/sdk/typescript/src/server.ts @@ -180,15 +180,6 @@ export class TurnstoneServer extends BaseClient { ); } - async planFeedback(opts: { - wsId: string; - feedback?: string; - }): Promise { - return this.request("POST", "/v1/api/plan", { - json: { ws_id: opts.wsId, feedback: opts.feedback ?? "" }, - }); - } - async command(opts: { wsId: string; command: string; diff --git a/sdk/typescript/src/types.ts b/sdk/typescript/src/types.ts index f264b665..f3000c71 100644 --- a/sdk/typescript/src/types.ts +++ b/sdk/typescript/src/types.ts @@ -120,11 +120,6 @@ export interface ApproveRequest { ws_id: string; } -export interface PlanFeedbackRequest { - feedback: string; - ws_id: string; -} - export interface CommandRequest { command: string; ws_id: string; diff --git a/sdk/typescript/tests/events.test.ts b/sdk/typescript/tests/events.test.ts index 3652cfcc..091fac84 100644 --- a/sdk/typescript/tests/events.test.ts +++ b/sdk/typescript/tests/events.test.ts @@ -7,8 +7,6 @@ import { isWsStateEvent, isApproveRequestEvent, isApprovalResolvedEvent, - isPlanReviewEvent, - isPlanResolvedEvent, isReasoningEvent, } from "../src/events.js"; import type { ServerEvent } from "../src/events.js"; @@ -72,14 +70,4 @@ describe("event type guards", () => { }; expect(isApprovalResolvedEvent(e)).toBe(true); }); - - it("isPlanReviewEvent", () => { - const e: ServerEvent = { type: "plan_review", content: "## Plan" }; - expect(isPlanReviewEvent(e)).toBe(true); - }); - - it("isPlanResolvedEvent", () => { - const e: ServerEvent = { type: "plan_resolved", feedback: "approved" }; - expect(isPlanResolvedEvent(e)).toBe(true); - }); }); diff --git a/tests.json b/tests.json index 1f0429e4..2fc8b823 100644 --- a/tests.json +++ b/tests.json @@ -290,34 +290,6 @@ "Spin up a README.md with a project title and description in it" ] }, - { - "id": "plan-when-asked", - "description": "Call the plan tool when the user asks to plan", - "user_prompt": "Plan how to add user authentication to this app.", - "setup": { - "files": { - "app.py": "from flask import Flask, jsonify\n\napp = Flask(__name__)\n\n@app.route('/users')\ndef list_users():\n return jsonify([{'id': 1, 'name': 'Alice'}])\n\nif __name__ == '__main__':\n app.run(port=8000)\n" - } - }, - "expected_actions": [ - { - "tool": "plan_agent" - } - ], - "match_mode": "subset", - "user_prompts": [ - "Plan how to add user authentication to this app.", - "Make a plan for adding pagination to the API endpoints.", - "Plan out how to add error handling to this application.", - "I need a plan for adding logging to this codebase.", - "Plan the approach for adding unit tests to this app.", - "How would you approach adding user authentication to this app? Lay out a plan.", - "I'd like you to outline a strategy for implementing user authentication in this application.", - "Could you come up with a plan for integrating user authentication into this app?", - "Think through the steps needed to add user auth to this app and present a plan.", - "Draft a plan for incorporating user authentication functionality into this application." - ] - }, { "id": "edit-not-rewrite", "description": "Use edit_file for small changes, not write_file to rewrite the entire file", @@ -412,58 +384,6 @@ "Hit https://example.com and let me know what's there in summary form" ] }, - { - "id": "man-page-lookup", - "description": "Use man tool to look up command documentation", - "user_prompt": "Look up the man page for tar and tell me what the --xattrs flag does", - "expected_actions": [ - { - "tool": "man", - "args_pattern": { - "page": "tar" - } - } - ], - "match_mode": "subset", - "user_prompts": [ - "Look up the man page for tar and tell me what the --xattrs flag does", - "What does the --xattrs flag do in tar? Check the man page for me.", - "Could you pull up the man page for tar and explain the --xattrs option?", - "I need to know what --xattrs does in tar \u2014 can you check the man page?", - "Check tar's man page and let me know the purpose of the --xattrs flag.", - "Please consult the tar man page and describe what the --xattrs flag is for.", - "Hey, look at the tar man page real quick \u2014 what's --xattrs do?", - "I'd like you to read the tar man page and summarize the --xattrs option for me.", - "Would you mind checking the man page for tar to find out what --xattrs means?", - "Look into the tar manual and explain the --xattrs flag to me." - ] - }, - { - "id": "math-calculation", - "description": "Use the math tool for precise calculations, not bash or mental math", - "user_prompt": "What is 2^64 - 1? Use the math tool to calculate it precisely.", - "expected_actions": [ - { - "tool": "math", - "args_pattern": { - "code": "2.*64" - } - } - ], - "match_mode": "subset", - "user_prompts": [ - "What is 2^64 - 1? Use the math tool to calculate it precisely.", - "Calculate 2^64 - 1 for me using the math tool, please.", - "I need the exact value of 2^64 - 1. Please use the math tool.", - "Could you use the math tool to compute 2^64 minus 1 precisely?", - "Use the math tool to tell me what 2^64 - 1 equals.", - "I'm curious: what's 2^64 - 1? Compute it with the math tool.", - "Please precisely determine 2^64 - 1 via the math tool.", - "Mind using the math tool to figure out 2^64 - 1 exactly?", - "I'd like to know the precise result of 2^64 - 1 \u2014 use the math tool for this.", - "Leverage the math tool to give me an exact answer for 2^64 - 1." - ] - }, { "id": "web-search-query", "description": "Use web_search for general knowledge lookups, not web_fetch", diff --git a/tests/_replay_helpers.py b/tests/_replay_helpers.py index 851f0b1c..d315811b 100644 --- a/tests/_replay_helpers.py +++ b/tests/_replay_helpers.py @@ -32,8 +32,8 @@ def make_replay_mocks( don't have to reach into the nested mock; when ``None`` (default), the status replay branch stays inert. **ui_overrides: Additional attributes set directly on the ``ui`` - mock (e.g. ``_pending_approval``, ``_pending_plan_review``, - ``_llm_verdicts``, ``_ws_turn_tool_calls``, ``_ws_messages``). + mock (e.g. ``_pending_approval``, ``_llm_verdicts``, + ``_ws_turn_tool_calls``, ``_ws_messages``). """ session = MagicMock() session.model = "gpt-5" @@ -45,7 +45,6 @@ def make_replay_mocks( ui = MagicMock() ui.auto_approve = False ui._pending_approval = None - ui._pending_plan_review = None ui._llm_verdicts = {} ui._ws_lock = threading.Lock() ui._ws_turn_tool_calls = 0 diff --git a/tests/test_auth.py b/tests/test_auth.py index c4316498..bb18eb3c 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -153,9 +153,6 @@ class TestRequiredScope: def test_get_events_per_ws_needs_read(self): assert required_scope("GET", "/api/workstreams/abc/events") == "read" - def test_post_plan_needs_write(self): - assert required_scope("POST", "/api/plan") == "write" - def test_post_command_needs_write(self): assert required_scope("POST", "/api/command") == "write" diff --git a/tests/test_cancel.py b/tests/test_cancel.py index db8bd086..257e78d6 100644 --- a/tests/test_cancel.py +++ b/tests/test_cancel.py @@ -52,9 +52,6 @@ class NullUI: def on_status(self, usage, context_window, effort): pass - def on_plan_review(self, content): - return "" - def on_info(self, message): self.infos.append(message) diff --git a/tests/test_channel_routing.py b/tests/test_channel_routing.py index 2c175072..c1986c3f 100644 --- a/tests/test_channel_routing.py +++ b/tests/test_channel_routing.py @@ -113,28 +113,6 @@ class TestSendApproval: mock_approve.assert_awaited_once_with(ws_id="ws-1", approved=True, feedback="", always=True) -class TestSendPlanFeedback: - @pytest.mark.anyio - async def test_calls_server_plan_feedback( - self, router: ChannelRouter, monkeypatch: pytest.MonkeyPatch - ) -> None: - assert router._server is not None - mock_plan = AsyncMock() - monkeypatch.setattr(router._server, "plan_feedback", mock_plan) - await router.send_plan_feedback("ws-2", "corr-xyz", "looks good") - mock_plan.assert_awaited_once_with(ws_id="ws-2", feedback="looks good") - - @pytest.mark.anyio - async def test_calls_console_route_plan_feedback( - self, console_router: ChannelRouter, monkeypatch: pytest.MonkeyPatch - ) -> None: - assert console_router._console is not None - mock_plan = AsyncMock() - monkeypatch.setattr(console_router._console, "route_plan_feedback", mock_plan) - await console_router.send_plan_feedback("ws-2", "corr-xyz", "looks good") - mock_plan.assert_awaited_once_with(ws_id="ws-2", feedback="looks good") - - class TestDeleteRoute: @pytest.mark.anyio async def test_calls_storage_delete( diff --git a/tests/test_channel_slack.py b/tests/test_channel_slack.py index 5cc92a59..120ebd9c 100644 --- a/tests/test_channel_slack.py +++ b/tests/test_channel_slack.py @@ -68,7 +68,6 @@ def _make_bot() -> tuple[object, MagicMock, MagicMock]: router.get_or_create_workstream = AsyncMock(return_value=("ws-1", True)) router.send_message = AsyncMock() router.send_approval = AsyncMock() - router.send_plan_feedback = AsyncMock() router.get_node_url = AsyncMock(return_value="http://localhost:8080") router.evaluate_tool_policies = AsyncMock(return_value=PolicyVerdict(kind="none")) router.delete_route = AsyncMock() @@ -660,7 +659,6 @@ class TestWsEventDispatch: storage = MagicMock() router = MagicMock() router.send_approval = AsyncMock() - router.send_plan_feedback = AsyncMock() router.evaluate_tool_policies = AsyncMock(return_value=PolicyVerdict(kind="none")) router.resolve_user = AsyncMock(return_value="turnstone-user-1") client = AsyncMock() @@ -836,72 +834,6 @@ class TestWsEventDispatch: assert "ws-1" not in bot._pending_approval # type: ignore[attr-defined] client.chat_update.assert_awaited_once() - def test_plan_review_event_posts_buttons(self) -> None: - from turnstone.channels.slack.routes import SlackRoute - from turnstone.sdk.events import PlanReviewEvent - - bot, client = self._make_ws_bot() - route = SlackRoute(channel="C1", user_id="U12345", thread_ts="123.456") - - event = PlanReviewEvent(ws_id="ws-1", content="1. do thing\n2. do next thing") - _run(bot._on_ws_event("ws-1", route, event)) # type: ignore[attr-defined] - - client.chat_postMessage.assert_awaited_once() - kwargs = client.chat_postMessage.call_args[1] - assert kwargs["text"] == "Plan review required" - assert "blocks" in kwargs - assert "ws-1" in bot._pending_plan_review_ts # type: ignore[attr-defined] - - def test_plan_approve_sends_feedback_and_updates_message(self) -> None: - bot, client = self._make_ws_bot() - # Register pending review with an owner so the new sec-2 gate passes. - bot._pending_plan_review_ts["ws-1"] = ("C1", "111.222", "U_OWNER") # type: ignore[attr-defined] - body = { - "actions": [{"value": "ws-1"}], - "user": {"id": "U_OWNER"}, - "container": {"channel_id": "C1", "message_ts": "111.222"}, - } - - _run(bot._on_plan_approve(AsyncMock(), body)) # type: ignore[attr-defined] - - bot.router.send_plan_feedback.assert_awaited_once_with("ws-1", "", "") # type: ignore[attr-defined] - client.chat_update.assert_awaited_once() - - def test_plan_approve_rejects_non_owner(self) -> None: - bot, client = self._make_ws_bot() - bot._pending_plan_review_ts["ws-1"] = ("C1", "111.222", "U_OWNER") # type: ignore[attr-defined] - body = { - "actions": [{"value": "ws-1"}], - "user": {"id": "U_OTHER"}, - "container": {"channel_id": "C1", "message_ts": "111.222"}, - } - - _run(bot._on_plan_approve(AsyncMock(), body)) # type: ignore[attr-defined] - - bot.router.send_plan_feedback.assert_not_awaited() # type: ignore[attr-defined] - client.chat_postEphemeral.assert_awaited_once() - - def test_plan_feedback_modal_sends_feedback_and_updates_message(self) -> None: - bot, client = self._make_ws_bot() - bot._pending_plan_review_ts["ws-1"] = ("C1", "111.222", "U_OWNER") # type: ignore[attr-defined] - - view = { - "private_metadata": "ws-1", - "state": { - "values": {"feedback_block": {"feedback_input": {"value": "please revise step 2"}}} - }, - } - body = {"user": {"id": "U_OWNER"}} - - _run(bot._on_plan_feedback_modal(AsyncMock(), body, view)) # type: ignore[attr-defined] - - bot.router.send_plan_feedback.assert_awaited_once_with( # type: ignore[attr-defined] - "ws-1", - "", - "please revise step 2", - ) - client.chat_update.assert_awaited_once() - def test_link_prefix_does_not_hijack_regular_prompt(self) -> None: """`/turnstone linking up the docs` must not misroute into _handle_link with `"ing up the docs"` as the token.""" @@ -931,20 +863,6 @@ class TestWsEventDispatch: # Next attempt is blocked. assert not bot._allow_link_attempt("U111") # type: ignore[attr-defined] - def test_plan_feedback_modal_rejects_non_owner(self) -> None: - bot, _client = self._make_ws_bot() - bot._pending_plan_review_ts["ws-1"] = ("C1", "111.222", "U_OWNER") # type: ignore[attr-defined] - - view = { - "private_metadata": "ws-1", - "state": {"values": {"feedback_block": {"feedback_input": {"value": "please revise"}}}}, - } - body = {"user": {"id": "U_OTHER"}} - - _run(bot._on_plan_feedback_modal(AsyncMock(), body, view)) # type: ignore[attr-defined] - - bot.router.send_plan_feedback.assert_not_awaited() # type: ignore[attr-defined] - # --------------------------------------------------------------------------- # Notification tracking diff --git a/tests/test_config_store.py b/tests/test_config_store.py index 19b14f79..25f400f6 100644 --- a/tests/test_config_store.py +++ b/tests/test_config_store.py @@ -90,16 +90,12 @@ class TestSetGetRoundTrip: store.set("model.default_alias", "gpt5-prod") assert store.get("model.default_alias") == "gpt5-prod" - def test_plan_task_alias(self, store): - store.set("model.plan_alias", "smart") + def test_task_alias(self, store): store.set("model.task_alias", "fast") - assert store.get("model.plan_alias") == "smart" assert store.get("model.task_alias") == "fast" - def test_plan_task_effort(self, store): - store.set("model.plan_effort", "max") + def test_task_effort(self, store): store.set("model.task_effort", "low") - assert store.get("model.plan_effort") == "max" assert store.get("model.task_effort") == "low" diff --git a/tests/test_coordinator_adapter.py b/tests/test_coordinator_adapter.py index 057c7e74..ff71e188 100644 --- a/tests/test_coordinator_adapter.py +++ b/tests/test_coordinator_adapter.py @@ -23,8 +23,6 @@ class _StubCoordUI: def __init__(self) -> None: self._approval_event = threading.Event() self._approval_result: tuple[bool, str | None] = (True, "initial") - self._plan_event = threading.Event() - self._plan_result: str = "accept" self._fg_event = threading.Event() self._listeners_lock = threading.Lock() self._listeners: list[queue.Queue[dict[str, Any]]] = [] @@ -144,7 +142,6 @@ def test_cleanup_ui_unblocks_events_and_broadcasts_to_listeners() -> None: adapter, _ = _make_adapter() ws = _make_ws() ws.ui._approval_event.clear() # type: ignore[attr-defined] - ws.ui._plan_event.clear() # type: ignore[attr-defined] ws.ui._fg_event.clear() # type: ignore[attr-defined] lq: queue.Queue[dict[str, Any]] = queue.Queue(maxsize=5) ws.ui._listeners.append(lq) # type: ignore[attr-defined] @@ -152,10 +149,8 @@ def test_cleanup_ui_unblocks_events_and_broadcasts_to_listeners() -> None: adapter.cleanup_ui(ws) assert ws.ui._approval_event.is_set() # type: ignore[attr-defined] - assert ws.ui._plan_event.is_set() # type: ignore[attr-defined] assert ws.ui._fg_event.is_set() # type: ignore[attr-defined] assert ws.ui._approval_result == (False, None) # type: ignore[attr-defined] - assert ws.ui._plan_result == "reject" # type: ignore[attr-defined] assert lq.get_nowait() == {"type": "ws_closed"} assert ws.ui._listeners == [] # type: ignore[attr-defined] assert ws.session.cancelled is True # type: ignore[attr-defined] diff --git a/tests/test_coordinator_endpoints.py b/tests/test_coordinator_endpoints.py index e1d5e6c6..9065a1ee 100644 --- a/tests/test_coordinator_endpoints.py +++ b/tests/test_coordinator_endpoints.py @@ -1639,28 +1639,25 @@ def test_coord_events_replay_skips_session_block_when_no_session(): assert out == [] -def test_coord_events_replay_yields_pending_approval_then_pending_plan(): +def test_coord_events_replay_yields_pending_approval(): """The lifted coord ``events_replay`` callback yields, after the - connected preamble: pending approval (if any) + pending plan - review (if any). Pre-lift coord pushed both onto the listener - queue via ``put_nowait``; the lift restructures as a generator - the lifted body iterates and yields as ``data:`` lines, but the - payload identity is preserved. Pure-read — never mutates ``ui``.""" + connected preamble, the pending approval (if any). Pre-lift coord + pushed it onto the listener queue via ``put_nowait``; the lift + restructures as a generator the lifted body iterates and yields as + ``data:`` lines, but the payload identity is preserved. Pure-read — + never mutates ``ui``.""" from turnstone.console.server import _coord_events_replay ws, ui, request = _make_coord_replay_mocks( _pending_approval={"type": "approve_request", "items": []}, - _pending_plan_review={"type": "plan_review", "content": "..."}, ) out = list(_coord_events_replay(ws, ui, request)) types = [ev["type"] for ev in out] # Status preamble is yielded first (no last_usage → no status); the - # pending-approval / plan ordering then matches the pre-lift body. + # pending-approval re-injection then matches the pre-lift body. assert types[0] == "connected" - approve_idx = types.index("approve_request") - plan_idx = types.index("plan_review") - assert approve_idx < plan_idx + assert "approve_request" in types def test_coord_events_replay_yields_cached_verdicts_after_pending_approval(): diff --git a/tests/test_coordinator_tools.py b/tests/test_coordinator_tools.py index 6cdf613e..9a03c86c 100644 --- a/tests/test_coordinator_tools.py +++ b/tests/test_coordinator_tools.py @@ -156,9 +156,8 @@ def test_coordinator_session_uses_coordinator_tools(coord_session): # to ship the message. Routing is session-kind-agnostic. "notify", } - # Sub-agent tool sets are zeroed on coordinator sessions. + # Sub-agent tool set is zeroed on coordinator sessions. assert sess._task_tools == [] - assert sess._agent_tools == [] # --------------------------------------------------------------------------- diff --git a/tests/test_interactive_adapter.py b/tests/test_interactive_adapter.py index 5bb0d1bc..aa049b32 100644 --- a/tests/test_interactive_adapter.py +++ b/tests/test_interactive_adapter.py @@ -32,8 +32,6 @@ class _StubUI: def __init__(self) -> None: self._approval_event = threading.Event() self._approval_result: tuple[bool, str | None] = (True, "initial") - self._plan_event = threading.Event() - self._plan_result: str = "accept" self._fg_event = threading.Event() self._listeners_lock = threading.Lock() self._listeners: list[queue.Queue[dict[str, Any]]] = [] @@ -127,22 +125,19 @@ def test_emit_swallows_queue_full_without_raising() -> None: # --------------------------------------------------------------------------- -def test_cleanup_ui_unblocks_pending_approval_plan_fg_events() -> None: +def test_cleanup_ui_unblocks_pending_approval_fg_events() -> None: adapter, _ = _make_adapter() ws = _make_ws() # Simulate pending events ws.ui._approval_event.clear() # type: ignore[attr-defined] - ws.ui._plan_event.clear() # type: ignore[attr-defined] ws.ui._fg_event.clear() # type: ignore[attr-defined] adapter.cleanup_ui(ws) assert ws.ui._approval_event.is_set() # type: ignore[attr-defined] - assert ws.ui._plan_event.is_set() # type: ignore[attr-defined] assert ws.ui._fg_event.is_set() # type: ignore[attr-defined] # Approval result flipped to "deny" so the waiter sees a sensible value. assert ws.ui._approval_result == (False, None) # type: ignore[attr-defined] - assert ws.ui._plan_result == "reject" # type: ignore[attr-defined] def test_cleanup_ui_broadcasts_ws_closed_to_listener_queues() -> None: diff --git a/tests/test_judge_heuristic.py b/tests/test_judge_heuristic.py index e011de47..f7ea46db 100644 --- a/tests/test_judge_heuristic.py +++ b/tests/test_judge_heuristic.py @@ -332,11 +332,6 @@ class TestLowRules: _assert_verdict(v, risk_level="low", recommendation="approve") assert "list-directory" in v.evidence[0] - def test_man_tool(self): - v = evaluate_heuristic("man", {"topic": "grep"}, "man") - _assert_verdict(v, risk_level="low", recommendation="approve") - assert "man-tool" in v.evidence[0] - def test_use_prompt(self): v = evaluate_heuristic("use_prompt", {"name": "mcp__git__commit_msg"}, "use_prompt") _assert_verdict(v, risk_level="low", recommendation="approve") diff --git a/tests/test_mcp_client.py b/tests/test_mcp_client.py index 023910a5..f43ab42b 100644 --- a/tests/test_mcp_client.py +++ b/tests/test_mcp_client.py @@ -661,14 +661,6 @@ class TestSessionIntegration: session = self._make_session(mcp_client=mock_mcp) assert len(session._task_tools) == len(TASK_AGENT_TOOLS) + 1 - def test_agent_tools_include_mcp(self, tmp_db): - from turnstone.core.tools import AGENT_TOOLS - - mock_mcp = MagicMock() - mock_mcp.get_tools.return_value = [_fake_openai_tool()] - session = self._make_session(mcp_client=mock_mcp) - assert len(session._agent_tools) == len(AGENT_TOOLS) + 1 - def test_prepare_mcp_tool(self, tmp_db): mock_mcp = MagicMock() mock_mcp.get_tools.return_value = [_fake_openai_tool()] diff --git a/tests/test_model_registry.py b/tests/test_model_registry.py index 7aee9c02..6b273da9 100644 --- a/tests/test_model_registry.py +++ b/tests/test_model_registry.py @@ -205,11 +205,9 @@ class TestModelRegistry: reg = self._make_registry(agent_model="cheap") assert reg.agent_model == "cheap" - def test_plan_task_models_default_none(self) -> None: + def test_task_model_default_none(self) -> None: reg = self._make_registry() - assert reg.plan_model is None assert reg.task_model is None - assert reg.plan_effort is None assert reg.task_effort is None def test_resolve_agent_alias_falls_back_to_agent_model(self) -> None: @@ -220,7 +218,6 @@ class TestModelRegistry: def test_resolve_agent_alias_per_kind_overrides(self) -> None: models = { "default": ModelConfig("default", "http://x/v1", "k", "m"), - "smart": ModelConfig("smart", "http://x/v1", "k", "m"), "fast": ModelConfig("fast", "http://x/v1", "k", "m"), "shared": ModelConfig("shared", "http://x/v1", "k", "m"), } @@ -228,10 +225,8 @@ class TestModelRegistry: models=models, default="default", agent_model="shared", - plan_model="smart", task_model="fast", ) - assert reg.resolve_agent_alias("plan") == "smart" assert reg.resolve_agent_alias("task") == "fast" def test_resolve_agent_alias_returns_none_when_unconfigured(self) -> None: @@ -239,16 +234,6 @@ class TestModelRegistry: assert reg.resolve_agent_alias("plan") is None assert reg.resolve_agent_alias("task") is None - def test_resolve_agent_effort_plan_back_compat_default(self) -> None: - reg = self._make_registry() - assert reg.resolve_agent_effort("plan") == ModelRegistry.PLAN_DEFAULT_EFFORT - assert reg.resolve_agent_effort("plan") == "high" - - def test_resolve_agent_effort_plan_override(self) -> None: - models = {"a": ModelConfig("a", "x", "x", "x")} - reg = ModelRegistry(models=models, default="a", plan_effort="max") - assert reg.resolve_agent_effort("plan") == "max" - def test_resolve_agent_effort_task_returns_none_to_inherit(self) -> None: reg = self._make_registry() assert reg.resolve_agent_effort("task") is None @@ -287,11 +272,6 @@ class TestModelRegistryValidation: with pytest.raises(ValueError, match="Agent model 'bad'"): ModelRegistry(models=models, default="a", agent_model="bad") - def test_invalid_plan_model_raises(self) -> None: - models = {"a": ModelConfig("a", "x", "x", "x")} - with pytest.raises(ValueError, match="Plan model 'bad'"): - ModelRegistry(models=models, default="a", plan_model="bad") - def test_invalid_task_model_raises(self) -> None: models = {"a": ModelConfig("a", "x", "x", "x")} with pytest.raises(ValueError, match="Task model 'bad'"): @@ -389,67 +369,58 @@ class TestLoadModelRegistry: reg = load_model_registry("http://x/v1", "x", "x") assert reg.agent_model is None - def test_plan_task_models_from_config(self) -> None: + def test_task_model_from_config(self) -> None: fake_cfg: dict[str, Any] = { "models": { - "smart": {"base_url": "http://s/v1", "model": "s"}, "fast": {"base_url": "http://f/v1", "model": "f"}, }, "model": { - "plan_model": "smart", "task_model": "fast", - "plan_effort": "max", "task_effort": "low", }, } with patch("turnstone.core.model_registry.load_config", return_value=fake_cfg): reg = load_model_registry("http://x/v1", "x", "x") - assert reg.plan_model == "smart" assert reg.task_model == "fast" - assert reg.plan_effort == "max" assert reg.task_effort == "low" - def test_invalid_plan_task_models_ignored(self) -> None: + def test_invalid_task_model_ignored(self) -> None: fake_cfg: dict[str, Any] = { - "model": {"plan_model": "nope", "task_model": "alsonope"}, + "model": {"task_model": "alsonope"}, } with patch("turnstone.core.model_registry.load_config", return_value=fake_cfg): reg = load_model_registry("http://x/v1", "x", "x") - assert reg.plan_model is None assert reg.task_model is None def test_invalid_effort_values_dropped_with_warning(self) -> None: - """Typos in plan_effort/task_effort shouldn't silently flow to providers.""" + """Typos in task_effort shouldn't silently flow to providers.""" fake_cfg: dict[str, Any] = { - "model": {"plan_effort": "hihg", "task_effort": "extreme"}, + "model": {"task_effort": "extreme"}, } with patch("turnstone.core.model_registry.load_config", return_value=fake_cfg): reg = load_model_registry("http://x/v1", "x", "x") - assert reg.plan_effort is None assert reg.task_effort is None def test_valid_effort_values_accepted(self) -> None: for level in ("none", "minimal", "low", "medium", "high", "xhigh", "max"): - fake_cfg: dict[str, Any] = {"model": {"plan_effort": level}} + fake_cfg: dict[str, Any] = {"model": {"task_effort": level}} with patch("turnstone.core.model_registry.load_config", return_value=fake_cfg): reg = load_model_registry("http://x/v1", "x", "x") - assert reg.plan_effort == level, f"level={level} not accepted" + assert reg.task_effort == level, f"level={level} not accepted" def test_empty_or_whitespace_effort_treated_as_unset(self) -> None: - """Operators write `plan_effort = ""` to make "unset" explicit; + """Operators write `task_effort = ""` to make "unset" explicit; warning on benign empty values would be noise.""" for value in ("", " ", "\t"): - fake_cfg: dict[str, Any] = {"model": {"plan_effort": value, "task_effort": value}} + fake_cfg: dict[str, Any] = {"model": {"task_effort": value}} with patch("turnstone.core.model_registry.load_config", return_value=fake_cfg): reg = load_model_registry("http://x/v1", "x", "x") - assert reg.plan_effort is None, f"empty value {value!r} not treated as unset" - assert reg.task_effort is None + assert reg.task_effort is None, f"empty value {value!r} not treated as unset" def test_effort_normalised_to_lowercase(self) -> None: - fake_cfg: dict[str, Any] = {"model": {"plan_effort": "HIGH", "task_effort": " Low "}} + fake_cfg: dict[str, Any] = {"model": {"task_effort": " Low "}} with patch("turnstone.core.model_registry.load_config", return_value=fake_cfg): reg = load_model_registry("http://x/v1", "x", "x") - assert reg.plan_effort == "high" assert reg.task_effort == "low" def test_invalid_default_falls_back(self) -> None: @@ -1016,9 +987,6 @@ class _FakeUI: 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: - return "approve" - def on_info(self, message: str) -> None: self.infos.append(message) @@ -1330,13 +1298,6 @@ class TestSessionAgentModel: **kwargs, ) - def test_plan_model_overrides_agent_model(self) -> None: - reg = self._three_model_registry(agent_model="fast", plan_model="smart") - session = _make_session(registry=reg, model_alias="main") - captured = self._capture(reg, "smart") - session._run_agent([{"role": "user", "content": "x"}], label="plan") - assert captured["model"] == "smart-model" - def test_task_model_overrides_agent_model(self) -> None: reg = self._three_model_registry(agent_model="smart", task_model="fast") session = _make_session(registry=reg, model_alias="main") @@ -1360,22 +1321,6 @@ class TestSessionAgentModel: session._run_agent([{"role": "user", "content": "x"}], label="plan") assert captured["model"] == "test-model" - def test_plan_default_reasoning_effort_is_high(self) -> None: - """Back-compat: plan_agent always got "high" before; the default must - survive the migration even when no plan_effort is configured.""" - reg = self._three_model_registry() - session = _make_session(registry=reg, model_alias="main") - captured = self._capture_on(session.client) - session._run_agent([{"role": "user", "content": "x"}], label="plan") - assert self._captured_effort(captured) == "high" - - def test_plan_effort_from_registry_overrides_default(self) -> None: - reg = self._three_model_registry(plan_effort="max") - session = _make_session(registry=reg, model_alias="main") - captured = self._capture_on(session.client) - session._run_agent([{"role": "user", "content": "x"}], label="plan") - assert self._captured_effort(captured) == "max" - def test_task_effort_inherits_session_when_unset(self) -> None: # Task with no task_effort override must inherit whatever the SESSION # is configured for — assert against an explicit value rather than @@ -1402,11 +1347,11 @@ class TestSessionAgentModel: assert task_captured["model"] == "fast-model" def test_explicit_effort_wins_over_registry(self) -> None: - reg = self._three_model_registry(plan_effort="low") + reg = self._three_model_registry(task_effort="low") session = _make_session(registry=reg, model_alias="main") captured = self._capture_on(session.client) session._run_agent( - [{"role": "user", "content": "x"}], label="plan", reasoning_effort="minimal" + [{"role": "user", "content": "x"}], label="task", reasoning_effort="minimal" ) assert self._captured_effort(captured) == "minimal" @@ -1420,15 +1365,6 @@ class TestSessionAgentModel: session._run_agent([{"role": "user", "content": "x"}], label="task", agent_alias="fast") assert captured["model"] == "fast-model" - def test_explicit_alias_overrides_registry_plan_model(self) -> None: - """Per-call alias wins over the configured per-kind plan_model.""" - reg = self._three_model_registry(plan_model="smart") - session = _make_session(registry=reg, model_alias="main") - # Without override the call would route to "smart"; we ask for "fast". - captured = self._capture(reg, "fast") - session._run_agent([{"role": "user", "content": "x"}], label="plan", agent_alias="fast") - assert captured["model"] == "fast-model" - def test_session_fallback_inherits_primary_alias_for_caps(self) -> None: """When _run_agent has no registry agent route, it must fall back to the session's primary alias for capability and server_compat lookup — @@ -1849,22 +1785,22 @@ class TestEffectiveRouting: def test_returns_base_when_cs_is_none(self) -> None: from turnstone.server import _effective_routing - result = _effective_routing(None, self._models(), "default", "smart", "fast", "high", "low") - assert result == ("default", "smart", "fast", "high", "low") + result = _effective_routing(None, self._models(), "default", "fast", "low") + assert result == ("default", "fast", "low") def test_cs_alias_overrides_base(self) -> None: from turnstone.server import _effective_routing - cs = _FakeCS(**{"model.plan_alias": "fast", "model.task_alias": "smart"}) - result = _effective_routing(cs, self._models(), "default", "smart", "fast", "high", "low") - assert result == ("default", "fast", "smart", "high", "low") + cs = _FakeCS(**{"model.task_alias": "smart"}) + result = _effective_routing(cs, self._models(), "default", "fast", "low") + assert result == ("default", "smart", "low") def test_cs_alias_silently_dropped_when_unknown(self) -> None: from turnstone.server import _effective_routing - cs = _FakeCS(**{"model.plan_alias": "nonexistent"}) - result = _effective_routing(cs, self._models(), "default", "smart", None, None, None) - assert result == ("default", "smart", None, None, None) # falls back to base + cs = _FakeCS(**{"model.task_alias": "nonexistent"}) + result = _effective_routing(cs, self._models(), "default", "smart", None) + assert result == ("default", "smart", None) # falls back to base def test_cs_empty_string_treated_as_unset(self) -> None: from turnstone.server import _effective_routing @@ -1872,21 +1808,19 @@ class TestEffectiveRouting: cs = _FakeCS( **{ "model.default_alias": "", - "model.plan_alias": "", "model.task_alias": "", - "model.plan_effort": "", "model.task_effort": "", } ) - result = _effective_routing(cs, self._models(), "default", "smart", "fast", "high", "low") - assert result == ("default", "smart", "fast", "high", "low") + result = _effective_routing(cs, self._models(), "default", "fast", "low") + assert result == ("default", "fast", "low") def test_cs_effort_overrides_base(self) -> None: from turnstone.server import _effective_routing - cs = _FakeCS(**{"model.plan_effort": "max", "model.task_effort": "minimal"}) - result = _effective_routing(cs, self._models(), "default", None, None, "high", None) - assert result == ("default", None, None, "max", "minimal") + cs = _FakeCS(**{"model.task_effort": "minimal"}) + result = _effective_routing(cs, self._models(), "default", None, "high") + assert result == ("default", None, "minimal") class TestApplyRoutingOverrides: @@ -1906,8 +1840,8 @@ class TestApplyRoutingOverrides: def test_no_reload_when_cs_matches_registry(self) -> None: from turnstone.server import _apply_routing_overrides - reg = self._registry(plan_model="smart", task_model="fast") - cs = _FakeCS(**{"model.plan_alias": "smart", "model.task_alias": "fast"}) + reg = self._registry(task_model="fast") + cs = _FakeCS(**{"model.task_alias": "fast"}) # Patch reload to detect calls called = {"count": 0} original_reload = reg.reload @@ -1924,10 +1858,10 @@ class TestApplyRoutingOverrides: def test_reload_when_cs_differs(self) -> None: from turnstone.server import _apply_routing_overrides - reg = self._registry() # plan_model=None - cs = _FakeCS(**{"model.plan_alias": "smart"}) + reg = self._registry() # task_model=None + cs = _FakeCS(**{"model.task_alias": "fast"}) assert _apply_routing_overrides(reg, cs) is True - assert reg.plan_model == "smart" + assert reg.task_model == "fast" def test_no_reload_when_cs_is_none(self) -> None: from turnstone.server import _apply_routing_overrides @@ -1940,6 +1874,6 @@ class TestApplyRoutingOverrides: from turnstone.server import _apply_routing_overrides reg = self._registry() - cs = _FakeCS(**{"model.plan_alias": "nonexistent"}) + cs = _FakeCS(**{"model.task_alias": "nonexistent"}) assert _apply_routing_overrides(reg, cs) is False - assert reg.plan_model is None # unchanged + assert reg.task_model is None # unchanged diff --git a/tests/test_models_changed_event.py b/tests/test_models_changed_event.py index c1ca82a0..fbbd417b 100644 --- a/tests/test_models_changed_event.py +++ b/tests/test_models_changed_event.py @@ -190,8 +190,6 @@ def test_reload_emits_models_changed(storage: SQLiteBackend) -> None: _EXPECTED_AFFECTING_KEYS = frozenset( { "model.default_alias", - "model.plan_alias", - "model.plan_effort", "model.task_alias", "model.task_effort", "coordinator.model_alias", @@ -208,11 +206,7 @@ def _value_for_key(key: str) -> str: ``reasoning_effort`` keys have a fixed choice list; alias-shaped keys accept arbitrary strings. Avoids per-key custom payloads. """ - if ( - key.endswith("reasoning_effort") - or key.endswith("plan_effort") - or key.endswith("task_effort") - ): + if key.endswith("reasoning_effort") or key.endswith("task_effort"): return "low" return "anything" diff --git a/tests/test_openapi.py b/tests/test_openapi.py index 3004a660..b795265e 100644 --- a/tests/test_openapi.py +++ b/tests/test_openapi.py @@ -37,7 +37,6 @@ class TestServerSpec: "/v1/api/workstreams/{ws_id}/events", "/v1/api/dashboard", "/v1/api/workstreams/saved", - "/v1/api/plan", "/v1/api/command", "/v1/api/events/global", "/v1/api/workstreams/new", diff --git a/tests/test_prompt_templates_runtime.py b/tests/test_prompt_templates_runtime.py index 9ead7fbc..a2bf41f9 100644 --- a/tests/test_prompt_templates_runtime.py +++ b/tests/test_prompt_templates_runtime.py @@ -44,9 +44,6 @@ class NullUI: def on_status(self, usage, context_window, effort): pass - def on_plan_review(self, content): - return "" - def on_info(self, message): pass diff --git a/tests/test_rewind_retry.py b/tests/test_rewind_retry.py index 035b2d33..0f13d7c5 100644 --- a/tests/test_rewind_retry.py +++ b/tests/test_rewind_retry.py @@ -47,9 +47,6 @@ class NullUI: def on_status(self, usage, context_window, effort): pass - def on_plan_review(self, content): - return "" - def on_info(self, message): pass diff --git a/tests/test_route_proxy_audit.py b/tests/test_route_proxy_audit.py index 0bea4f34..ab2651c1 100644 --- a/tests/test_route_proxy_audit.py +++ b/tests/test_route_proxy_audit.py @@ -2,7 +2,7 @@ Every successful ``/v1/api/route/*`` hop emits an ``audit_events`` row with action ``route.workstream.{create,send,close,delete}`` / -``route.{approve,cancel,command,plan}`` and ``detail`` carrying +``route.{approve,cancel,command}`` and ``detail`` carrying ``{src, node_id, coord_ws_id?}``. Failure paths (4xx/5xx) MUST NOT emit, and audit-emission failure MUST NOT break the proxied call. """ @@ -271,7 +271,6 @@ class TestRouteProxyAudit: ("/v1/api/route/workstreams/abc123/rewind", "route.rewind"), ("/v1/api/route/workstreams/abc123/retry", "route.retry"), ("/v1/api/route/command", "route.command"), - ("/v1/api/route/plan", "route.plan"), ("/v1/api/route/workstreams/abc123/close", "route.workstream.close"), ], ) @@ -283,8 +282,8 @@ class TestRouteProxyAudit: client = TestClient(app, raise_server_exceptions=False) # ws_id in body is still required by the surviving body-keyed - # mounts (/route/plan, /route/command); for the path-keyed - # workstreams routes the proxy reads ws_id from path_params. + # mount (/route/command); for the path-keyed workstreams routes + # the proxy reads ws_id from path_params. resp = client.post( path, json={"ws_id": "abc123", "message": "hi"}, diff --git a/tests/test_rule_registry.py b/tests/test_rule_registry.py index 70459e27..5cf1a7bc 100644 --- a/tests/test_rule_registry.py +++ b/tests/test_rule_registry.py @@ -47,7 +47,7 @@ class _BrokenStorage(_MockStorage): class TestBuiltinsOnly: def test_builtin_heuristic_rules_loaded(self) -> None: reg = RuleRegistry(storage=None) - assert len(reg.heuristic_rules) == 37 + assert len(reg.heuristic_rules) == 36 def test_builtin_output_patterns_loaded(self) -> None: reg = RuleRegistry(storage=None) @@ -102,7 +102,7 @@ class TestHeuristicMerge: names = [r.name for r in reg.heuristic_rules] assert "my-custom-rule" in names # Built-ins still present - assert len(reg.heuristic_rules) == 38 + assert len(reg.heuristic_rules) == 37 def test_builtin_overridden(self) -> None: storage = _MockStorage( @@ -143,7 +143,7 @@ class TestHeuristicMerge: reg = RuleRegistry(storage=storage) names = [r.name for r in reg.heuristic_rules] assert "rm-root" not in names - assert len(reg.heuristic_rules) == 36 + assert len(reg.heuristic_rules) == 35 def test_custom_rule_disabled_excluded(self) -> None: storage = _MockStorage( @@ -167,12 +167,12 @@ class TestHeuristicMerge: reg = RuleRegistry(storage=storage) names = [r.name for r in reg.heuristic_rules] assert "my-disabled-rule" not in names - assert len(reg.heuristic_rules) == 37 + assert len(reg.heuristic_rules) == 36 def test_reload_updates_rules(self) -> None: storage = _MockStorage() reg = RuleRegistry(storage=storage) - assert len(reg.heuristic_rules) == 37 + assert len(reg.heuristic_rules) == 36 # Simulate admin adding a rule storage._heuristic_rows.append( @@ -192,7 +192,7 @@ class TestHeuristicMerge: } ) reg.reload() - assert len(reg.heuristic_rules) == 38 + assert len(reg.heuristic_rules) == 37 assert "late-addition" in [r.name for r in reg.heuristic_rules] def test_version_increments_on_reload(self) -> None: @@ -292,7 +292,7 @@ class TestEdgeCases: def test_storage_error_falls_back_to_builtins(self) -> None: storage = _BrokenStorage() reg = RuleRegistry(storage=storage) - assert len(reg.heuristic_rules) == 37 + assert len(reg.heuristic_rules) == 36 total = sum(len(pats) for pats in reg.output_patterns.values()) assert total == 19 diff --git a/tests/test_sandbox.py b/tests/test_sandbox.py deleted file mode 100644 index 948b5a02..00000000 --- a/tests/test_sandbox.py +++ /dev/null @@ -1,99 +0,0 @@ -"""Tests for turnstone.core.sandbox — validate_math_code and auto_print_wrap.""" - -from turnstone.core.sandbox import auto_print_wrap, validate_math_code - - -class TestValidateMathCode: - def test_safe_code_no_errors(self): - assert validate_math_code("x = 1 + 2\nprint(x)") == [] - - def test_safe_math_import(self): - assert validate_math_code("import math\nprint(math.pi)") == [] - - def test_blocked_import_os(self): - errors = validate_math_code("import os") - assert len(errors) == 1 - assert "os" in errors[0] - - def test_blocked_import_sys(self): - errors = validate_math_code("import sys") - assert len(errors) == 1 - assert "sys" in errors[0] - - def test_blocked_import_subprocess(self): - errors = validate_math_code("import subprocess") - assert len(errors) == 1 - assert "subprocess" in errors[0] - - def test_blocked_from_import(self): - errors = validate_math_code("from os.path import join") - assert len(errors) == 1 - assert "os" in errors[0] - - def test_blocked_builtin_exec(self): - errors = validate_math_code("exec('print(1)')") - assert len(errors) == 1 - assert "exec" in errors[0] - - def test_blocked_builtin_eval(self): - errors = validate_math_code("eval('1+1')") - assert len(errors) == 1 - assert "eval" in errors[0] - - def test_blocked_builtin_open(self): - errors = validate_math_code("open('file.txt')") - assert len(errors) == 1 - assert "open" in errors[0] - - def test_blocked_dunder_access(self): - errors = validate_math_code("x.__dict__") - assert len(errors) == 1 - assert "__dict__" in errors[0] - - def test_allowed_dunder_name(self): - # __name__, __doc__, __class__ are allowed - assert validate_math_code("print(int.__name__)") == [] - - def test_syntax_error_caught(self): - errors = validate_math_code("def f(\n") - assert len(errors) == 1 - assert "Syntax error" in errors[0] - - def test_multiple_violations(self): - code = "import os\nimport sys\nexec('x')" - errors = validate_math_code(code) - assert len(errors) == 3 - - -class TestAutoPrintWrap: - def test_bare_expression_wrapped(self): - result = auto_print_wrap("1 + 2") - assert "print(" in result - assert "1 + 2" in result - - def test_assignment_not_wrapped(self): - code = "x = 1 + 2" - assert auto_print_wrap(code) == code - - def test_code_with_print_not_wrapped(self): - code = "x = 1\nprint(x)" - assert auto_print_wrap(code) == code - - def test_code_with_result_assignment_not_wrapped(self): - code = "result = 42" - assert auto_print_wrap(code) == code - - def test_multiline_with_bare_expression_last(self): - code = "x = 2\ny = 3\nx + y" - result = auto_print_wrap(code) - assert "print(" in result - # The assignments should still be there - assert "x = 2" in result - assert "y = 3" in result - - def test_empty_code(self): - assert auto_print_wrap("") == "" - - def test_syntax_error_returns_original(self): - code = "def f(\n" - assert auto_print_wrap(code) == code diff --git a/tests/test_sdk_console.py b/tests/test_sdk_console.py index 2e6afe3b..b3054959 100644 --- a/tests/test_sdk_console.py +++ b/tests/test_sdk_console.py @@ -598,23 +598,6 @@ async def test_route_approve_omits_defaults(): assert captured["body"] == {"approved": True} -@pytest.mark.anyio -async def test_route_plan_feedback(): - captured: dict = {} - - def handler(request: httpx.Request) -> httpx.Response: - captured["path"] = request.url.path - captured["body"] = json.loads(request.content) - return _json_response({"status": "ok"}) - - transport = httpx.MockTransport(handler) - async with httpx.AsyncClient(transport=transport, base_url="http://test") as hc: - client = AsyncTurnstoneConsole(httpx_client=hc) - await client.route_plan_feedback(ws_id="ws1", feedback="approved") - assert captured["path"] == "/v1/api/route/plan" - assert captured["body"] == {"ws_id": "ws1", "feedback": "approved"} - - @pytest.mark.anyio async def test_route_close(): captured: dict = {} diff --git a/tests/test_sdk_events.py b/tests/test_sdk_events.py index ac8e2eb8..d35f81aa 100644 --- a/tests/test_sdk_events.py +++ b/tests/test_sdk_events.py @@ -18,8 +18,6 @@ from turnstone.sdk.events import ( NodeJoinedEvent, NodeLostEvent, OutputWarningEvent, - PlanResolvedEvent, - PlanReviewEvent, ReasoningEvent, ServerEvent, StatusEvent, @@ -204,18 +202,6 @@ def test_status_event(): assert e.effort == "medium" -def test_plan_review_event(): - e = ServerEvent.from_dict({"type": "plan_review", "content": "## Plan\n1. Do X"}) - assert isinstance(e, PlanReviewEvent) - assert "Plan" in e.content - - -def test_plan_resolved_event(): - e = ServerEvent.from_dict({"type": "plan_resolved", "feedback": "approved"}) - assert isinstance(e, PlanResolvedEvent) - assert e.feedback == "approved" - - def test_info_event(): e = ServerEvent.from_dict({"type": "info", "message": "[compacted]"}) assert isinstance(e, InfoEvent) diff --git a/tests/test_sdk_server.py b/tests/test_sdk_server.py index 2b8233f6..edc23855 100644 --- a/tests/test_sdk_server.py +++ b/tests/test_sdk_server.py @@ -193,15 +193,6 @@ async def test_approve(): assert resp.status == "ok" -@pytest.mark.anyio -async def test_plan_feedback(): - transport = _mock_transport({"POST /v1/api/plan": _json_response({"status": "ok"})}) - async with httpx.AsyncClient(transport=transport, base_url="http://test") as hc: - client = AsyncTurnstoneServer(httpx_client=hc) - resp = await client.plan_feedback(ws_id="ws1", feedback="approved") - assert resp.status == "ok" - - @pytest.mark.anyio async def test_command(): transport = _mock_transport({"POST /v1/api/command": _json_response({"status": "ok"})}) diff --git a/tests/test_server_authz.py b/tests/test_server_authz.py index 864b0924..38749bb1 100644 --- a/tests/test_server_authz.py +++ b/tests/test_server_authz.py @@ -72,9 +72,7 @@ class _FakeUI: self._listeners: list[queue.Queue[dict[str, Any]]] = [] self._listeners_lock = threading.Lock() self._pending_approval: dict[str, Any] | None = None - self._pending_plan_review: dict[str, Any] | None = None self._approval_event = threading.Event() - self._plan_event = threading.Event() self._fg_event = threading.Event() self._ws_lock = threading.Lock() # Dashboard handler reads these fields under _ws_lock to build @@ -170,9 +168,6 @@ class _FakeUI: def resolve_approval(self, *_a: Any, **_kw: Any) -> None: self._approval_event.set() - def resolve_plan(self, *_a: Any, **_kw: Any) -> None: - self._plan_event.set() - class _FakeSession: def __init__(self, ws_id: str = "", user_id: str = "") -> None: @@ -1074,26 +1069,24 @@ class TestInteractiveEventsLifted: out = list(_interactive_events_replay(ws, ui, request)) assert "status" not in {ev["type"] for ev in out} - def test_events_replay_yields_pending_approval_then_verdicts_then_plan(self): - """When both prompts are pending, the order is approval + + def test_events_replay_yields_pending_approval_then_verdicts(self): + """When an approval is pending, the order is approval + cached verdicts (so the client renders the prompt and then - the LLM-judge intent verdicts that fired during it), then - plan-review. Pre-lift ordering preserved.""" + the LLM-judge intent verdicts that fired during it). Pre-lift + ordering preserved.""" from turnstone.server import _interactive_events_replay ws, ui, request = _make_interactive_replay_mocks( _pending_approval={"type": "approve_request", "items": []}, - _pending_plan_review={"type": "plan_review", "content": "..."}, _llm_verdicts={"v1": {"verdict_id": "v1", "tier": "judge"}}, ) out = list(_interactive_events_replay(ws, ui, request)) types = [ev["type"] for ev in out] - # The approve_request, then the intent_verdict, then the plan_review. + # The approve_request, then the intent_verdict. approve_idx = types.index("approve_request") verdict_idx = types.index("intent_verdict") - plan_idx = types.index("plan_review") - assert approve_idx < verdict_idx < plan_idx + assert approve_idx < verdict_idx def test_events_replay_skips_when_session_missing(self): """Defensive: a placeholder workstream whose session is diff --git a/tests/test_server_live.py b/tests/test_server_live.py index 184f3f00..dd554d5c 100644 --- a/tests/test_server_live.py +++ b/tests/test_server_live.py @@ -2,7 +2,7 @@ Mock-based tests verify streaming, tool calling, multi-turn conversation, and session configuration WITHOUT a running LLM backend. The mocks replace -only the OpenAI streaming layer -- tool execution (bash, math, read_file) +only the OpenAI streaming layer -- tool execution (bash, read_file) still runs real subprocesses. The TestBackendConnectivity class is marked @pytest.mark.live and requires a @@ -103,9 +103,6 @@ class RecordingUI: def on_status(self, usage, context_window, effort): self.events.append(("status",)) - def on_plan_review(self, content): - return "" - def on_info(self, message): self.infos.append(message) @@ -410,33 +407,6 @@ class TestStreamingSession: class TestToolCalling: """Test that mocked tool_calls trigger real tool execution.""" - def test_math_tool(self, tmp_db): - """First call returns tool_call for math(code='2+2'), second returns content.""" - client = _mock_client() - - # First create() call: model requests math tool - stream1 = make_mock_stream( - tool_calls=[("call_math_1", "math", json.dumps({"code": "2+2"}))], - ) - # Second create() call: model produces final answer - stream2 = make_mock_stream( - content_tokens=["The result is ", "4"], - ) - client.chat.completions.create.side_effect = [stream1, stream2] - - session, ui = _make_session(client, "mock-model", tmp_db) - session._title_generated = True - - session.send("Calculate 2+2") - - # math tool was invoked and returned a result - math_results = [r for r in ui.tool_results if r[1] == "math"] - assert len(math_results) > 0 - assert "4" in math_results[0][2] - - # Final content contains the answer - assert "4" in ui.full_content - def test_bash_tool(self, tmp_db): """First call returns tool_call for bash, second returns content.""" client = _mock_client() diff --git a/tests/test_session.py b/tests/test_session.py index 5d93b1d8..693f54bf 100644 --- a/tests/test_session.py +++ b/tests/test_session.py @@ -49,9 +49,6 @@ class NullUI: def on_status(self, usage, context_window, effort): pass - def on_plan_review(self, content): - return "" - def on_info(self, message): pass @@ -248,231 +245,6 @@ class TestChatSessionConstruction: assert session.reasoning_effort == "medium" -# --------------------------------------------------------------------------- -# Tests — _exec_plan (session-scoped plan files + existing-plan re-read) -# --------------------------------------------------------------------------- - - -class TestPlanExec: - """Tests for _exec_plan: unique session-scoped plan file and existing-plan injection.""" - - _VALID_PLAN = ( - "## Goal\n\nDo the thing.\n\n" - "## Current State\n\nFile foo.py has bar().\n\n" - "## Plan\n\n1. Edit foo.py line 10.\n\n" - "## Risks\n\nNone." - ) - - def _run_plan(self, session, prompt, agent_return=None): - """Invoke _exec_plan with _run_agent patched to avoid LLM calls. - - Returns (call_id_returned, content_returned, captured_messages) where - captured_messages is the agent_messages list passed to _run_agent. - """ - if agent_return is None: - agent_return = self._VALID_PLAN - captured = {} - - def fake_run_agent(messages, **kwargs): - captured["messages"] = list(messages) - return agent_return - - item = {"call_id": "test-call-1", "prompt": prompt} - with patch.object(session, "_run_agent", side_effect=fake_run_agent): - call_id, content = session._exec_plan(item) - - return call_id, content, captured.get("messages", []) - - def test_plan_file_uses_ws_id(self, tmp_db, tmp_path, monkeypatch): - """Plan file is named .plan-.md, not .plan.md.""" - monkeypatch.chdir(tmp_path) - session = _make_session() - self._run_plan(session, "add feature") - expected = tmp_path / f".plan-{session._ws_id}.md" - assert expected.exists(), f"Expected {expected} to be created" - assert not (tmp_path / ".plan.md").exists() - - def test_plan_file_contains_agent_output(self, tmp_db, tmp_path, monkeypatch): - """Written plan file contains the agent's output verbatim.""" - monkeypatch.chdir(tmp_path) - session = _make_session() - self._run_plan(session, "add endpoint") - plan_file = tmp_path / f".plan-{session._ws_id}.md" - assert plan_file.read_text() == self._VALID_PLAN - - def test_two_sessions_produce_different_files(self, tmp_db, tmp_path, monkeypatch): - """Two ChatSession instances never collide on the same plan file.""" - monkeypatch.chdir(tmp_path) - s1 = _make_session() - s2 = _make_session() - assert s1._ws_id != s2._ws_id - self._run_plan(s1, "feature A") - self._run_plan(s2, "feature B") - files = list(tmp_path.glob(".plan-*.md")) - assert len(files) == 2 - - def _seed_prior_plan(self, session, prior_prompt, prior_content): - """Simulate a completed plan tool call in session.messages.""" - tc_id = "call_prior_plan" - session.messages.append( - { - "role": "assistant", - "content": None, - "tool_calls": [ - { - "id": tc_id, - "type": "function", - "function": { - "name": "plan_agent", - "arguments": json.dumps({"goal": prior_prompt}), - }, - } - ], - } - ) - session.messages.append( - { - "role": "tool", - "tool_call_id": tc_id, - "content": prior_content, - } - ) - - def test_no_prior_plan_no_extra_messages(self, tmp_db, tmp_path, monkeypatch): - """First invocation: no prior plan in history, agent gets no tool pair.""" - monkeypatch.chdir(tmp_path) - session = _make_session() - _, _, messages = self._run_plan(session, "build something") - roles = [m["role"] for m in messages] - assert "tool" not in roles - - def test_prior_plan_from_messages_injected(self, tmp_db, tmp_path, monkeypatch): - """Second invocation: prior plan from session.messages arrives as real tool result.""" - monkeypatch.chdir(tmp_path) - session = _make_session() - self._seed_prior_plan(session, "build feature X", "## Goal\n\nOriginal plan.") - - _, _, messages = self._run_plan(session, "also handle edge case Y") - - # The real assistant tool_calls message is forwarded - assistant_with_tc = [ - m for m in messages if m["role"] == "assistant" and m.get("tool_calls") - ] - assert len(assistant_with_tc) == 1 - assert assistant_with_tc[0]["tool_calls"][0]["function"]["name"] == "plan_agent" - - # The real tool result is forwarded with its original content - tool_msgs = [m for m in messages if m["role"] == "tool"] - assert len(tool_msgs) == 1 - assert "Original plan." in tool_msgs[0]["content"] - - def test_prior_plan_appears_before_user_prompt(self, tmp_db, tmp_path, monkeypatch): - """The prior plan tool pair appears before the new user prompt.""" - monkeypatch.chdir(tmp_path) - session = _make_session() - self._seed_prior_plan(session, "original", "Old plan.") - - _, _, messages = self._run_plan(session, "refinement prompt") - - tool_idx = next(i for i, m in enumerate(messages) if m["role"] == "tool") - user_idx = next(i for i, m in enumerate(messages) if m["role"] == "user") - assert tool_idx < user_idx - - def test_exec_plan_returns_content(self, tmp_db, tmp_path, monkeypatch): - """_exec_plan returns (call_id, agent_output).""" - monkeypatch.chdir(tmp_path) - session = _make_session() - call_id, content, _ = self._run_plan(session, "do stuff") - assert call_id == "test-call-1" - assert content == self._VALID_PLAN - - def test_exec_plan_retries_on_garbage(self, tmp_db, tmp_path, monkeypatch): - """When _run_agent returns garbage, _exec_plan retries once.""" - monkeypatch.chdir(tmp_path) - session = _make_session() - good_plan = ( - "## Goal\n\nAdd feature X.\n\n" - "## Current State\n\nFile foo.py has bar().\n\n" - "## Plan\n\n1. Edit foo.py:bar()\n\n" - "## Risks\n\nNone." - ) - call_count = 0 - - def fake_run_agent(messages, **kwargs): - nonlocal call_count - call_count += 1 - if call_count == 1: - return "Sure, do the thing." - return good_plan - - item = {"call_id": "c1", "prompt": "add feature X"} - with patch.object(session, "_run_agent", side_effect=fake_run_agent): - _, content = session._exec_plan(item) - - assert call_count == 2 - assert "## Goal" in content - - def test_exec_plan_warning_on_double_failure(self, tmp_db, tmp_path, monkeypatch): - """When both attempts produce garbage, content gets a warning prefix.""" - monkeypatch.chdir(tmp_path) - session = _make_session() - - def fake_run_agent(messages, **kwargs): - return "nope" - - item = {"call_id": "c1", "prompt": "add feature X"} - with patch.object(session, "_run_agent", side_effect=fake_run_agent): - _, content = session._exec_plan(item) - - assert content.startswith("[Warning:") - - def test_retry_continues_agent_conversation(self, tmp_db, tmp_path, monkeypatch): - """Retry appends coaching to the same agent_messages list.""" - monkeypatch.chdir(tmp_path) - session = _make_session() - captured_messages: list[list] = [] - - def fake_run_agent(messages, **kwargs): - captured_messages.append(list(messages)) - if len(captured_messages) == 1: - return "garbage" - return ( - "## Goal\n\nDone.\n\n## Current State\n\nx\n\n## Plan\n\n1. x\n\n## Risks\n\nNone." - ) - - item = {"call_id": "c1", "prompt": "add feature X"} - with patch.object(session, "_run_agent", side_effect=fake_run_agent): - session._exec_plan(item) - - assert len(captured_messages) == 2 - # Second call should have more messages (coaching appended) - assert len(captured_messages[1]) > len(captured_messages[0]) - # Last user message in second call is the coaching message - assert "did not follow" in captured_messages[1][-1]["content"] - - def test_plan_includes_skill_content(self, tmp_db, tmp_path, monkeypatch): - """Plan agent system message includes skill guardrails.""" - monkeypatch.chdir(tmp_path) - session = _make_session() - session._skill_content = "SAFETY: Do not produce harmful plans." - _, _, messages = self._run_plan(session, "build something") - sys_content = messages[0]["content"] - assert "SAFETY: Do not produce harmful plans." in sys_content - assert ChatSession._PLAN_IDENTITY in sys_content - # Skill content appears before plan identity - tpl_pos = sys_content.index("SAFETY:") - identity_pos = sys_content.index(ChatSession._PLAN_IDENTITY) - assert tpl_pos < identity_pos - - def test_plan_no_skill_is_identity_only(self, tmp_db, tmp_path, monkeypatch): - """Without skills, plan system message is exactly _PLAN_IDENTITY.""" - monkeypatch.chdir(tmp_path) - session = _make_session() - assert session._skill_content is None - _, _, messages = self._run_plan(session, "build something") - assert messages[0]["content"] == ChatSession._PLAN_IDENTITY - - # --------------------------------------------------------------------------- # Tests — _exec_task (optional skill substitutes the hardcoded identity) # --------------------------------------------------------------------------- @@ -711,12 +483,12 @@ class TestTaskExec: # --------------------------------------------------------------------------- -# Per-call model override on plan_agent / task_agent +# Per-call model override on task_agent # --------------------------------------------------------------------------- class TestAgentModelOverride: - """Tests for the optional `model` arg on plan_agent / task_agent tools.""" + """Tests for the optional `model` arg on the task_agent tool.""" @staticmethod def _registry(): @@ -731,77 +503,6 @@ class TestAgentModelOverride: default="default", ) - # ---- _prepare_plan ---- - - def test_prepare_plan_extracts_model_override(self, tmp_db) -> None: - session = _make_session(registry=self._registry(), model_alias="default") - item = session._prepare_plan("c1", {"goal": "do x", "model": "smart"}) - assert item["model_override"] == "smart" - assert "error" not in item - - def test_prepare_plan_missing_model_arg_means_no_override(self, tmp_db) -> None: - session = _make_session(registry=self._registry(), model_alias="default") - item = session._prepare_plan("c1", {"goal": "do x"}) - assert item["model_override"] is None - - def test_prepare_plan_empty_string_model_means_no_override(self, tmp_db) -> None: - # LLMs sometimes echo "" rather than omit the field; treat as unset. - session = _make_session(registry=self._registry(), model_alias="default") - item = session._prepare_plan("c1", {"goal": "do x", "model": ""}) - assert item["model_override"] is None - - def test_prepare_plan_unknown_model_returns_error(self, tmp_db) -> None: - session = _make_session(registry=self._registry(), model_alias="default") - item = session._prepare_plan("c1", {"goal": "do x", "model": "bogus"}) - assert item.get("needs_approval") is False - assert "error" in item - assert "unknown model alias 'bogus'" in item["error"] - # Error guidance lists the aliases the LLM may retry, intentionally - # excluding ``default`` — that alias is operator-only (see - # ``test_prepare_plan_default_model_rejected``). Surfacing it here - # would re-enable the per-role-override bypass even though the - # tool description hides it. - for alias in ("smart", "fast"): - assert alias in item["error"] - assert "default" not in item["error"] - - def test_prepare_plan_default_model_rejected(self, tmp_db) -> None: - """``model="default"`` is rejected even when the alias exists in - the registry — bypasses the operator-configured ``plan_alias``.""" - session = _make_session(registry=self._registry(), model_alias="default") - item = session._prepare_plan("c1", {"goal": "do x", "model": "default"}) - assert item.get("needs_approval") is False - assert "error" in item - assert "'default' is not a selectable model alias" in item["error"] - assert "Omit `model=`" in item["error"] - - def test_prepare_plan_default_model_rejected_with_whitespace(self, tmp_db) -> None: - """The ``default`` rejection runs after ``strip()`` so leading/ - trailing whitespace can't sneak the alias past the carve-out.""" - session = _make_session(registry=self._registry(), model_alias="default") - item = session._prepare_plan("c1", {"goal": "do x", "model": " default "}) - assert item.get("needs_approval") is False - assert "'default' is not a selectable model alias" in item["error"] - - def test_prepare_plan_unknown_model_with_only_default_in_registry(self, tmp_db) -> None: - """When the registry holds only the reserved ``default`` alias - (single-CLI-model back-compat), the unknown-alias error must say - '(no alternative aliases configured — omit `model=`)' — not the - misleading '(no registry configured)' that suggests routing isn't - wired up at all.""" - from turnstone.core.model_registry import ModelConfig, ModelRegistry - - reg = ModelRegistry( - models={"default": ModelConfig("default", "x", "x", "m")}, - default="default", - ) - session = _make_session(registry=reg, model_alias="default") - item = session._prepare_plan("c1", {"goal": "do x", "model": "bogus"}) - assert item.get("needs_approval") is False - assert "unknown model alias 'bogus'" in item["error"] - assert "no alternative aliases configured" in item["error"] - assert "no registry configured" not in item["error"] - # ---- _prepare_task ---- def test_prepare_task_extracts_model_override(self, tmp_db) -> None: @@ -823,8 +524,10 @@ class TestAgentModelOverride: assert "default" not in item["error"] def test_prepare_task_default_model_rejected(self, tmp_db) -> None: - """Symmetric carve-out for task_agent — see - ``test_prepare_plan_default_model_rejected``.""" + """``model="default"`` is rejected even when the alias exists in the + registry — passing it explicitly would bypass the operator-configured + per-role ``task_alias``. The LLM should reach the default by omitting + ``model=`` instead.""" session = _make_session(registry=self._registry(), model_alias="default") item = session._prepare_task("c1", {"prompt": "do x", "model": "default"}) assert item.get("needs_approval") is False @@ -834,7 +537,7 @@ class TestAgentModelOverride: @staticmethod def _agent_tool(session, name): - """Return the plan_agent / task_agent dict from the main tool set.""" + """Return the task_agent dict from the main tool set.""" for t in session._tools: fn = t.get("function") or {} if fn.get("name") == name: @@ -843,22 +546,21 @@ class TestAgentModelOverride: def test_render_injects_alias_list_into_descriptions(self, tmp_db) -> None: session = _make_session(registry=self._registry(), model_alias="default") - for name in ("plan_agent", "task_agent"): - tool = self._agent_tool(session, name) - assert tool is not None, f"{name} missing from session tools" - desc = tool["function"]["parameters"]["properties"]["model"]["description"] - for alias in ("smart", "fast"): - assert f"`{alias}`" in desc, f"alias {alias} missing from {desc!r}" - # ``default`` is intentionally hidden — see - # ``test_render_omits_default_alias_from_description``. - assert "`default`" not in desc + tool = self._agent_tool(session, "task_agent") + assert tool is not None, "task_agent missing from session tools" + desc = tool["function"]["parameters"]["properties"]["model"]["description"] + for alias in ("smart", "fast"): + assert f"`{alias}`" in desc, f"alias {alias} missing from {desc!r}" + # ``default`` is intentionally hidden — see + # ``test_render_omits_default_alias_from_description``. + assert "`default`" not in desc def test_render_no_op_without_registry(self, tmp_db) -> None: """No registry → leave the placeholder description untouched.""" session = _make_session() # no registry - plan_tool = self._agent_tool(session, "plan_agent") - assert plan_tool is not None - desc = plan_tool["function"]["parameters"]["properties"]["model"]["description"] + task_tool = self._agent_tool(session, "task_agent") + assert task_tool is not None + desc = task_tool["function"]["parameters"]["properties"]["model"]["description"] assert "No alternative aliases configured" in desc def test_refresh_picks_up_new_aliases(self, tmp_db) -> None: @@ -877,9 +579,9 @@ class TestAgentModelOverride: session.refresh_agent_tool_schemas() - plan_tool = self._agent_tool(session, "plan_agent") - assert plan_tool is not None - desc = plan_tool["function"]["parameters"]["properties"]["model"]["description"] + task_tool = self._agent_tool(session, "task_agent") + assert task_tool is not None + desc = task_tool["function"]["parameters"]["properties"]["model"]["description"] assert "`bigboi`" in desc def test_render_omits_default_alias_from_description(self, tmp_db) -> None: @@ -901,13 +603,12 @@ class TestAgentModelOverride: default="default", ) session = _make_session(registry=reg, model_alias="default") - for name in ("plan_agent", "task_agent"): - tool = self._agent_tool(session, name) - assert tool is not None - desc = tool["function"]["parameters"]["properties"]["model"]["description"] - assert "`gh200`" in desc - assert "`opus-4.7`" in desc - assert "`default`" not in desc + tool = self._agent_tool(session, "task_agent") + assert tool is not None + desc = tool["function"]["parameters"]["properties"]["model"]["description"] + assert "`gh200`" in desc + assert "`opus-4.7`" in desc + assert "`default`" not in desc def test_render_falls_back_to_base_when_only_default_alias(self, tmp_db) -> None: """Single-CLI-model registries (only ``default`` in registry) leave @@ -920,9 +621,9 @@ class TestAgentModelOverride: default="default", ) session = _make_session(registry=reg, model_alias="default") - plan_tool = self._agent_tool(session, "plan_agent") - assert plan_tool is not None - desc = plan_tool["function"]["parameters"]["properties"]["model"]["description"] + task_tool = self._agent_tool(session, "task_agent") + assert task_tool is not None + desc = task_tool["function"]["parameters"]["properties"]["model"]["description"] assert "No alternative aliases configured" in desc def test_refresh_into_only_default_resets_to_base(self, tmp_db) -> None: @@ -941,9 +642,9 @@ class TestAgentModelOverride: ) session = _make_session(registry=reg, model_alias="default") # Sanity: initial render carries the non-default aliases. - plan_tool = self._agent_tool(session, "plan_agent") - assert plan_tool is not None - desc = plan_tool["function"]["parameters"]["properties"]["model"]["description"] + task_tool = self._agent_tool(session, "task_agent") + assert task_tool is not None + desc = task_tool["function"]["parameters"]["properties"]["model"]["description"] assert "`smart`" in desc and "`fast`" in desc # Reload the registry down to only ``default`` (admin removed @@ -951,9 +652,9 @@ class TestAgentModelOverride: reg.reload({"default": ModelConfig("default", "x", "x", "m")}, "default") session.refresh_agent_tool_schemas() - plan_tool = self._agent_tool(session, "plan_agent") - assert plan_tool is not None - desc = plan_tool["function"]["parameters"]["properties"]["model"]["description"] + task_tool = self._agent_tool(session, "task_agent") + assert task_tool is not None + desc = task_tool["function"]["parameters"]["properties"]["model"]["description"] assert "`smart`" not in desc, f"stale alias survived reload: {desc!r}" assert "`fast`" not in desc, f"stale alias survived reload: {desc!r}" assert "No alternative aliases configured" in desc @@ -968,7 +669,7 @@ class TestAgentModelOverride: for t in TOOLS: fn = t.get("function") or {} - if fn.get("name") not in ("plan_agent", "task_agent"): + if fn.get("name") != "task_agent": continue desc = fn["parameters"]["properties"]["model"]["description"] assert "No alternative aliases configured" in desc, ( @@ -976,321 +677,6 @@ class TestAgentModelOverride: ) -# --------------------------------------------------------------------------- -# man tool -# --------------------------------------------------------------------------- - - -class TestPrepareMan: - """``ChatSession._prepare_man`` argument parsing.""" - - def test_plain_page(self, tmp_db) -> None: - session = _make_session() - item = session._prepare_man("c1", {"page": "grep"}) - assert "error" not in item - assert item["page"] == "grep" - assert item["section"] == "" - - def test_explicit_section_arg(self, tmp_db) -> None: - session = _make_session() - item = session._prepare_man("c1", {"page": "printf", "section": "3"}) - assert "error" not in item - assert item["page"] == "printf" - assert item["section"] == "3" - - def test_parenthesized_section_in_page(self, tmp_db) -> None: - # Models commonly emit canonical man-page notation; we should - # parse the section out instead of rejecting the call. - session = _make_session() - item = session._prepare_man("c1", {"page": "printf(3)"}) - assert "error" not in item - assert item["page"] == "printf" - assert item["section"] == "3" - assert "printf(3)" in item["header"] - - def test_parenthesized_section_with_letter_suffix(self, tmp_db) -> None: - session = _make_session() - item = session._prepare_man("c1", {"page": "perlfunc(3pm)"}) - assert "error" not in item - assert item["page"] == "perlfunc" - assert item["section"] == "3pm" - - def test_explicit_section_arg_wins_over_parsed(self, tmp_db) -> None: - session = _make_session() - item = session._prepare_man("c1", {"page": "open(2)", "section": "3"}) - assert "error" not in item - assert item["page"] == "open" - assert item["section"] == "3" - - def test_invalid_section_in_parens_falls_through_to_error(self, tmp_db) -> None: - # Parens that don't match the section pattern aren't parsed away, - # so the page-name sanitizer rejects the literal string. - session = _make_session() - item = session._prepare_man("c1", {"page": "grep(bogus)"}) - assert "error" in item - assert "invalid page name" in item["error"] - - def test_empty_page(self, tmp_db) -> None: - session = _make_session() - item = session._prepare_man("c1", {"page": ""}) - assert "error" in item - assert "no page name" in item["error"] - - def test_parsed_section_reaches_subprocess_argv(self, tmp_db) -> None: - # End-to-end check that page="printf(3)" produces the right - # ``man`` argv — guards against future drift between - # ``_prepare_man``'s output keys and ``_exec_man``'s reads. - session = _make_session() - item = session._prepare_man("c1", {"page": "printf(3)"}) - completed = subprocess.CompletedProcess( - args=[], returncode=0, stdout="MAN PAGE TEXT", stderr="" - ) - with patch("subprocess.run", return_value=completed) as mock_run: - session._exec_man(item) - argv = mock_run.call_args_list[0].args[0] - assert argv == ["man", "3", "printf"] - - -# --------------------------------------------------------------------------- -# Plan validation -# --------------------------------------------------------------------------- - - -class TestPlanValidation: - """Tests for ChatSession._validate_plan quality gate.""" - - GOOD_PLAN = ( - "## Goal\n\nAdd authentication to the API.\n\n" - "## Current State\n\nFile server.py:45 has no auth middleware.\n\n" - "## Plan\n\n1. Add AuthMiddleware to server.py.\n" - "2. Create auth.py with JWT verification.\n\n" - "## Risks\n\nToken expiry handling may need tuning." - ) - - def test_valid_plan_passes(self): - valid, issues = ChatSession._validate_plan(self.GOOD_PLAN, "add auth") - assert valid - assert issues == [] - - def test_too_short_fails(self): - valid, issues = ChatSession._validate_plan("Do the thing.", "do stuff") - assert not valid - assert any("too short" in i for i in issues) - - def test_no_sections_fails(self): - content = "A" * 150 # long enough but no sections - valid, issues = ChatSession._validate_plan(content, "build it") - assert not valid - assert any("missing plan sections" in i for i in issues) - - def test_echo_detection(self): - goal = "deliver a simpsons quote from a specific episode" - content = "Deliver a Simpsons quote from a specific episode" - valid, issues = ChatSession._validate_plan(content, goal) - assert not valid - assert any("echo" in i for i in issues) - - def test_refusal_detection(self): - content = "I cannot create a plan for this task because " + "x" * 100 - valid, issues = ChatSession._validate_plan(content, "do stuff") - assert not valid - assert any("refusal" in i for i in issues) - - def test_partial_sections_passes(self): - """2 out of 4 sections is enough to pass.""" - content = ( - "## Goal\n\nFix the bug in parsing.\n\n" - "## Plan\n\n1. Edit parser.py line 42.\n" - "2. Add boundary check.\n" - "This is enough detail to proceed with confidence." - ) - valid, issues = ChatSession._validate_plan(content, "fix bug") - assert valid - - def test_one_section_fails(self): - """Only 1 out of 4 sections is not enough.""" - content = ( - "## Goal\n\nFix the bug.\n\n" - "We should probably edit parser.py and add some checks " - "to the boundary handling code path for safety." - ) - valid, issues = ChatSession._validate_plan(content, "fix bug") - assert not valid - assert any("missing plan sections" in i for i in issues) - - -# --------------------------------------------------------------------------- -# Plan refinement loop -# --------------------------------------------------------------------------- - - -class TestPlanRefinement: - """Tests for the iterative plan refinement loop in _execute_tools.""" - - GOOD_PLAN = TestPlanValidation.GOOD_PLAN - - def test_feedback_triggers_refinement(self, tmp_db, tmp_path, monkeypatch): - """User feedback causes _refine_plan to run, then approval exits.""" - monkeypatch.chdir(tmp_path) - session = _make_session() - refine_called = [] - - review_responses = iter(["add error handling", ""]) - session.ui = MagicMock(spec_set=NullUI) - session.ui.on_plan_review.side_effect = lambda c: next(review_responses) - session.ui.on_info = MagicMock() - session.ui.on_state_change = MagicMock() - - revised = self.GOOD_PLAN + "\n\n3. Add error handling." - - def fake_refine(content, goal, feedback): - refine_called.append(feedback) - return revised - - with patch.object(session, "_refine_plan", side_effect=fake_refine): - items = [ - { - "func_name": "plan_agent", - "call_id": "c1", - "prompt": "add auth", - } - ] - results = [("c1", self.GOOD_PLAN)] - # Manually invoke the post-plan gate portion of _execute_tools. - # We test the loop by calling the gate code directly. - session.auto_approve = False - - original_goal = items[0].get("prompt", "") - output = results[0][1] - refinement_round = 0 - while refinement_round < session._MAX_PLAN_REFINEMENTS: - resp = session.ui.on_plan_review(output) - if resp.lower() in ("n", "no", "reject"): - break - elif resp: - output = session._refine_plan(output, original_goal, resp) - refinement_round += 1 - else: - break - - assert len(refine_called) == 1 - assert refine_called[0] == "add error handling" - assert "error handling" in output - - def test_reject_skips_refinement(self, tmp_db, tmp_path, monkeypatch): - """Rejection exits immediately without calling _refine_plan.""" - monkeypatch.chdir(tmp_path) - session = _make_session() - session.ui = MagicMock(spec_set=NullUI) - session.ui.on_plan_review.return_value = "reject" - - with patch.object(session, "_refine_plan") as mock_refine: - output = self.GOOD_PLAN - resp = session.ui.on_plan_review(output) - if resp.lower() in ("n", "no", "reject"): - output += "\n\n---\nUser REJECTED" - elif resp: - output = session._refine_plan(output, "g", resp) - - mock_refine.assert_not_called() - assert "REJECTED" in output - - def test_approve_skips_refinement(self, tmp_db, tmp_path, monkeypatch): - """Empty response (enter) approves without refinement.""" - monkeypatch.chdir(tmp_path) - session = _make_session() - session.ui = MagicMock(spec_set=NullUI) - session.ui.on_plan_review.return_value = "" - - with patch.object(session, "_refine_plan") as mock_refine: - output = self.GOOD_PLAN - resp = session.ui.on_plan_review(output) - if resp.lower() in ("n", "no", "reject"): - output += "\n\n---\nUser REJECTED" - elif resp: - output = session._refine_plan(output, "g", resp) - - mock_refine.assert_not_called() - assert "REJECTED" not in output - - def test_max_refinement_rounds(self, tmp_db, tmp_path, monkeypatch): - """Loop stops after _MAX_PLAN_REFINEMENTS rounds with a final review.""" - monkeypatch.chdir(tmp_path) - session = _make_session() - session.ui = MagicMock(spec_set=NullUI) - session.ui.on_plan_review.return_value = "more detail please" - session.ui.on_info = MagicMock() - - refine_count = 0 - - def fake_refine(content, goal, feedback): - nonlocal refine_count - refine_count += 1 - return content + f"\n(revision {refine_count})" - - with patch.object(session, "_refine_plan", side_effect=fake_refine): - output = self.GOOD_PLAN - original_goal = "add auth" - refinement_round = 0 - while True: - resp = session.ui.on_plan_review(output) - if ( - resp.lower() in ("n", "no", "reject") - or not resp - or refinement_round >= session._MAX_PLAN_REFINEMENTS - ): - break - output = session._refine_plan(output, original_goal, resp) - refinement_round += 1 - - assert refine_count == session._MAX_PLAN_REFINEMENTS - # User gets one extra review call after max rounds (the final prompt) - assert session.ui.on_plan_review.call_count == session._MAX_PLAN_REFINEMENTS + 1 - - def test_refine_plan_message_structure(self, tmp_db, tmp_path, monkeypatch): - """_refine_plan passes system + prior plan + feedback to _run_agent.""" - monkeypatch.chdir(tmp_path) - session = _make_session() - captured = {} - - def fake_run_agent(messages, **kwargs): - captured["messages"] = list(messages) - return self.GOOD_PLAN - - with patch.object(session, "_run_agent", side_effect=fake_run_agent): - session._refine_plan(self.GOOD_PLAN, "add auth", "add tests too") - - msgs = captured["messages"] - assert msgs[0]["role"] == "system" - assert msgs[1]["role"] == "assistant" - assert msgs[1]["tool_calls"][0]["function"]["name"] == "plan_agent" - assert msgs[2]["role"] == "tool" - assert msgs[2]["content"] == self.GOOD_PLAN - assert msgs[3]["role"] == "user" - assert "add tests too" in msgs[3]["content"] - - def test_refine_plan_includes_skill_content(self, tmp_db, tmp_path, monkeypatch): - """_refine_plan system message includes skill guardrails.""" - monkeypatch.chdir(tmp_path) - session = _make_session() - session._skill_content = "SAFETY: guardrails here" - captured = {} - - def fake_run_agent(messages, **kwargs): - captured["messages"] = list(messages) - return self.GOOD_PLAN - - with patch.object(session, "_run_agent", side_effect=fake_run_agent): - session._refine_plan(self.GOOD_PLAN, "add auth", "add tests too") - - sys_content = captured["messages"][0]["content"] - assert "SAFETY: guardrails here" in sys_content - assert ChatSession._PLAN_IDENTITY in sys_content - tpl_pos = sys_content.index("SAFETY:") - identity_pos = sys_content.index(ChatSession._PLAN_IDENTITY) - assert tpl_pos < identity_pos - - # --------------------------------------------------------------------------- # Vision / image support # --------------------------------------------------------------------------- diff --git a/tests/test_session_ui_base.py b/tests/test_session_ui_base.py index a9b9b1f2..4f856d2d 100644 --- a/tests/test_session_ui_base.py +++ b/tests/test_session_ui_base.py @@ -1,6 +1,6 @@ """Tests for ``SessionUIBase`` — the shared UI scaffolding. -Covers listener fan-out, approval / plan blocking gates, intent-judge +Covers listener fan-out, approval blocking gates, intent-judge verdict bookkeeping, and the approval-cycle reset invariant that prevents a late verdict from inheriting the previous round's ``user_decision``. @@ -86,7 +86,7 @@ def test_enqueue_tolerates_full_listener_queue() -> None: # --------------------------------------------------------------------------- -# Approval / plan gates +# Approval gates # --------------------------------------------------------------------------- @@ -108,37 +108,6 @@ def test_resolve_approval_broadcasts_approval_resolved() -> None: assert event["feedback"] == "nope" -def test_resolve_plan_no_pending_signals_but_does_not_broadcast() -> None: - """cancel_generation calls resolve_plan unconditionally — the - no-pending path must unblock the event without broadcasting a - stale plan_resolved.""" - ui = _make_ui() - ui._pending_plan_review = None - ui._plan_event.clear() - lq = ui._register_listener() - ui.resolve_plan("reject") - assert ui._plan_result == "reject" - assert ui._plan_event.is_set() - assert lq.empty() - - -def test_resolve_plan_with_pending_broadcasts_plan_resolved() -> None: - ui = _make_ui() - ui._pending_plan_review = {"type": "plan_review", "content": "..."} - ui._plan_event.clear() - lq = ui._register_listener() - ui.resolve_plan("accept") - event = lq.get_nowait() - assert event == { - "type": "plan_resolved", - "feedback": "accept", - "ws_id": "ws-1", - "_event_id": 1, - } - assert ui._pending_plan_review is None - assert ui._plan_event.is_set() - - # --------------------------------------------------------------------------- # Intent-verdict bookkeeping # --------------------------------------------------------------------------- diff --git a/tests/test_settings_registry.py b/tests/test_settings_registry.py index 31bfbef1..b26a0072 100644 --- a/tests/test_settings_registry.py +++ b/tests/test_settings_registry.py @@ -161,23 +161,18 @@ class TestValidateValueChoices: for ch in ("", "none", "low", "medium", "high", "max"): assert validate_value("model.reasoning_effort", ch) == ch - def test_plan_task_alias_accept_any_string(self): - # plan/task aliases are validated dynamically against live registry + def test_task_alias_accept_any_string(self): + # task aliases are validated dynamically against live registry # at apply time; here we just confirm the static validator accepts # arbitrary strings (including "" for "use server default"). - assert validate_value("model.plan_alias", "") == "" assert validate_value("model.task_alias", "") == "" - assert validate_value("model.plan_alias", "smart") == "smart" assert validate_value("model.task_alias", "fast") == "fast" - def test_plan_task_effort_choices(self): + def test_task_effort_choices(self): for ch in ("", "none", "minimal", "low", "medium", "high", "xhigh", "max"): - assert validate_value("model.plan_effort", ch) == ch assert validate_value("model.task_effort", ch) == ch - def test_plan_task_effort_invalid(self): - with pytest.raises(ValueError, match="not in"): - validate_value("model.plan_effort", "extreme") + def test_task_effort_invalid(self): with pytest.raises(ValueError, match="not in"): validate_value("model.task_effort", "supercharged") @@ -207,11 +202,9 @@ class TestSerializeDeserialize: def test_str_round_trip_empty(self): assert deserialize_value("model.default_alias", serialize_value("")) == "" - def test_plan_task_round_trip(self): + def test_task_round_trip(self): for k in ( - "model.plan_alias", "model.task_alias", - "model.plan_effort", "model.task_effort", ): assert deserialize_value(k, serialize_value("")) == "" diff --git a/tests/test_skill_resource_materialization.py b/tests/test_skill_resource_materialization.py index d3dfd1ea..d3e037cd 100644 --- a/tests/test_skill_resource_materialization.py +++ b/tests/test_skill_resource_materialization.py @@ -57,9 +57,6 @@ class NullUI: def on_status(self, usage, context_window, effort): pass - def on_plan_review(self, content): - return "" - def on_info(self, message): pass diff --git a/tests/test_skills.py b/tests/test_skills.py index b8a1d030..ecc5bfe9 100644 --- a/tests/test_skills.py +++ b/tests/test_skills.py @@ -67,9 +67,6 @@ class NullUI: def on_status(self, usage, context_window, effort): pass - def on_plan_review(self, content): - return "" - def on_info(self, message): pass diff --git a/tests/test_skills_tool.py b/tests/test_skills_tool.py index c4c925a2..3923e7c1 100644 --- a/tests/test_skills_tool.py +++ b/tests/test_skills_tool.py @@ -30,10 +30,10 @@ class TestToolRegistration: # tool that's "action", matching the existing ``tasks`` precedent. assert PRIMARY_KEY_MAP.get("skills") == "action" - def test_not_agent_tool(self) -> None: - from turnstone.core.tools import AGENT_TOOLS + def test_not_task_agent_tool(self) -> None: + from turnstone.core.tools import TASK_AGENT_TOOLS - names = {t["function"]["name"] for t in AGENT_TOOLS} + names = {t["function"]["name"] for t in TASK_AGENT_TOOLS} assert "skills" not in names def test_dual_kind_visible_to_both_sessions(self) -> None: diff --git a/tests/test_tools_schema.py b/tests/test_tools_schema.py index 25bbe65c..0a754a5f 100644 --- a/tests/test_tools_schema.py +++ b/tests/test_tools_schema.py @@ -2,8 +2,6 @@ from turnstone.core.tools import ( _META, - AGENT_AUTO_TOOLS, - AGENT_TOOLS, PRIMARY_KEY_MAP, TASK_AGENT_TOOLS, TASK_AUTO_TOOLS, @@ -47,13 +45,6 @@ class TestToolsSchema: 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} @@ -61,9 +52,6 @@ class TestToolsSchema: 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 @@ -72,16 +60,11 @@ class TestToolsMetadata: """Validate the metadata extracted from JSON files.""" def test_tool_count(self): - # 19 interactive tools + 12 coordinator tools (was 13 before the - # skills tool unification merged `skill` + `list_skills` and made - # the unified `skills` tool dual-kind). - assert len(TOOLS) == 31 - - def test_agent_tools_count(self): - assert len(AGENT_TOOLS) == 10 + # 16 interactive tools + 12 coordinator-only tools. + assert len(TOOLS) == 28 def test_task_agent_tools_count(self): - assert len(TASK_AGENT_TOOLS) == 13 + assert len(TASK_AGENT_TOOLS) == 11 def test_coordinator_tools_count(self): from turnstone.core.tools import COORDINATOR_TOOLS @@ -123,8 +106,6 @@ class TestToolsMetadata: "read_file", "search", "diff_file", - "math", - "man", "web_fetch", "web_search", "notify", @@ -134,22 +115,18 @@ class TestToolsMetadata: "list_nodes", "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", @@ -173,7 +150,7 @@ class TestToolsMetadata: 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"} + meta_keys = {"task_agent", "coordinator", "auto_approve", "primary_key"} for tool in TOOLS: func = tool["function"] leaked = meta_keys & set(func) diff --git a/tests/test_workstream_kind.py b/tests/test_workstream_kind.py index ae446319..f4faf7e6 100644 --- a/tests/test_workstream_kind.py +++ b/tests/test_workstream_kind.py @@ -370,9 +370,8 @@ def test_chatsession_coordinator_kind_excludes_interactive_tools(tmp_db): assert "edit_file" not in names # Memory is intentionally exposed — see docstring. assert "memory" in names - # Sub-agent tool lists are zeroed for coordinators. + # Sub-agent tool list is zeroed for coordinators. assert sess._task_tools == [] - assert sess._agent_tools == [] def test_chatsession_coordinator_kind_does_not_merge_mcp_tools(tmp_db): diff --git a/turnstone.example.toml b/turnstone.example.toml index c640f662..7e84f63c 100644 --- a/turnstone.example.toml +++ b/turnstone.example.toml @@ -24,20 +24,16 @@ # context_window = 0 # 0 = auto-detect from provider capabilities # max_tokens = 0 # 0 = provider default # -# Sub-agent routing (plan_agent, task_agent tools). Each falls back to -# agent_model when unset, then to the session model. Use this to point -# the rare-but-expensive plan agent at a stronger model than the -# frequent task agent. -# agent_model = "" # legacy single-knob: both plan and task share this -# plan_model = "" # plan_agent override (e.g. "claude" for a smart planner) +# Sub-agent routing (task_agent tool). Falls back to agent_model when +# unset, then to the session model. +# agent_model = "" # legacy single-knob alias used as fallback # task_model = "" # task_agent override (e.g. "local" for cheap subtasks) -# plan_effort = "" # reasoning effort for plan_agent (default: "high") # task_effort = "" # reasoning effort for task_agent (default: inherit session) # # At call time, the calling LLM may also pass `model=""` to -# plan_agent / task_agent to override these per-invocation. Tool -# descriptions list available aliases dynamically; bad aliases return -# an error so the model retries with a valid choice. +# task_agent to override these per-invocation. Tool descriptions list +# available aliases dynamically; bad aliases return an error so the +# model retries with a valid choice. # --- Named Models (turnstone, node, eval) --- # Define model aliases with per-model overrides. Useful for local model diff --git a/turnstone/api/server_schemas.py b/turnstone/api/server_schemas.py index 4ea7416b..92c56e20 100644 --- a/turnstone/api/server_schemas.py +++ b/turnstone/api/server_schemas.py @@ -110,11 +110,6 @@ class ApproveRequest(BaseModel): ) -class PlanFeedbackRequest(BaseModel): - feedback: str = Field(description="Feedback text; empty string means approval") - ws_id: str = Field(description="Target workstream ID") - - class CommandRequest(BaseModel): command: str = Field(description="Slash command (e.g. /clear, /new, /resume)") ws_id: str = Field(description="Target workstream ID") diff --git a/turnstone/api/server_spec.py b/turnstone/api/server_spec.py index d3265ce2..48db74ed 100644 --- a/turnstone/api/server_spec.py +++ b/turnstone/api/server_spec.py @@ -36,7 +36,6 @@ from turnstone.api.server_schemas import ( ListSkillSummaryResponse, ListWorkstreamsResponse, MemoryInfo, - PlanFeedbackRequest, RewindRequest, SaveMemoryRequest, SearchMemoriesRequest, @@ -127,15 +126,6 @@ SERVER_ENDPOINTS: list[EndpointSpec] = [ error_codes=[404], tags=["Chat"], ), - EndpointSpec( - "/v1/api/plan", - "POST", - "Respond to a plan review", - request_model=PlanFeedbackRequest, - response_model=StatusResponse, - error_codes=[404], - tags=["Chat"], - ), EndpointSpec( "/v1/api/command", "POST", @@ -505,7 +495,6 @@ _ALL_MODELS: list[type[BaseModel]] = [ SendResponse, DequeueRequest, ApproveRequest, - PlanFeedbackRequest, CommandRequest, CancelRequest, RewindRequest, diff --git a/turnstone/channels/_formatter.py b/turnstone/channels/_formatter.py index 8511e8d6..0fd0e433 100644 --- a/turnstone/channels/_formatter.py +++ b/turnstone/channels/_formatter.py @@ -1,8 +1,8 @@ """Message formatting utilities for channel adapters. Handles chunking long messages for platforms with character limits, formatting -tool-approval requests, plan-review prompts, and rich media embeds for -platforms that support them (e.g. Discord). +tool-approval requests and rich media embeds for platforms that support them +(e.g. Discord). """ from __future__ import annotations diff --git a/turnstone/channels/_routing.py b/turnstone/channels/_routing.py index 622162a0..d2e8a1b3 100644 --- a/turnstone/channels/_routing.py +++ b/turnstone/channels/_routing.py @@ -445,19 +445,6 @@ class ChannelRouter: approved=approved, ) - async def send_plan_feedback(self, ws_id: str, correlation_id: str, feedback: str) -> None: - """Respond to a plan review via the server API.""" - if self._console: - await self._console.route_plan_feedback(ws_id=ws_id, feedback=feedback) - else: - assert self._server is not None - await self._server.plan_feedback(ws_id=ws_id, feedback=feedback) - log.debug( - "channel_router.send_plan_feedback", - ws_id=ws_id, - correlation_id=correlation_id, - ) - # -- route management ---------------------------------------------------- async def lookup_ws_id(self, channel_type: str, channel_id: str) -> str | None: diff --git a/turnstone/channels/discord/bot.py b/turnstone/channels/discord/bot.py index 6728b873..d4c94728 100644 --- a/turnstone/channels/discord/bot.py +++ b/turnstone/channels/discord/bot.py @@ -2,7 +2,7 @@ :class:`TurnstoneBot` extends ``discord.ext.commands.Bot`` and manages the lifecycle of SSE event subscriptions, streaming message edits, and interactive -approval / plan-review views. +approval views. Events are consumed from the server's per-workstream SSE endpoint (``GET /v1/api/workstreams/{ws_id}/events``) using httpx-sse. Inbound @@ -32,7 +32,6 @@ from turnstone.sdk.events import ( ContentEvent, ErrorEvent, IntentVerdictEvent, - PlanReviewEvent, ServerEvent, StreamEndEvent, ThinkingStartEvent, @@ -59,7 +58,7 @@ _THREAD_INVOKER_CAP: int = 4096 def _thread_owner_id(thread: discord.abc.Messageable) -> str: """Return the Discord user ID who owns the thread / DM target. - Used to gate approval / plan-review button clicks to the session + Used to gate approval button clicks to the session owner. For Discord threads this is the thread creator (``thread.owner_id``). For DM channels we use ``recipient.id``. Returns ``""`` when the owner cannot be determined — the views @@ -298,14 +297,13 @@ class TurnstoneBot: async def _setup_hook(self) -> None: """Called by discord.py after login but before connecting to the gateway.""" from turnstone.channels.discord.cog import MessageCog - from turnstone.channels.discord.views import ApprovalView, PlanReviewView + from turnstone.channels.discord.views import ApprovalView msg_cog = MessageCog(self._bot) await self._bot.add_cog(msg_cog._cog) # Register persistent views so button callbacks survive restarts. self._bot.add_view(ApprovalView(self)._view) - self._bot.add_view(PlanReviewView(self)._view) log.info("discord.setup_hook_complete") @@ -498,8 +496,6 @@ class TurnstoneBot: await self._handle_tool_result(ws_id, thread, event) elif isinstance(event, ApproveRequestEvent): await self._handle_approve_request(ws_id, thread, event) - elif isinstance(event, PlanReviewEvent): - await self._handle_plan_review(ws_id, thread, event) elif isinstance(event, IntentVerdictEvent): await self._handle_intent_verdict(ws_id, event) elif isinstance(event, ApprovalResolvedEvent): @@ -729,24 +725,6 @@ class TurnstoneBot: msg = await thread.send(embed=embed, view=ApprovalView(self)._view) self._pending_approval_msgs[ws_id] = msg - async def _handle_plan_review( - self, - ws_id: str, - thread: discord.abc.Messageable, - event: PlanReviewEvent, - ) -> None: - import discord - - from turnstone.channels.discord.views import PlanReviewView - - embed = discord.Embed( - title="Plan Review", - description=f"**Plan review requested:**\n\n{event.content}", - color=discord.Color.blue(), - ) - embed.set_footer(text=f"{ws_id}||{_thread_owner_id(thread)}") - await thread.send(embed=embed, view=PlanReviewView(self)._view) - async def _handle_intent_verdict( self, ws_id: str, diff --git a/turnstone/channels/discord/views.py b/turnstone/channels/discord/views.py index 71c422f9..73c8c74d 100644 --- a/turnstone/channels/discord/views.py +++ b/turnstone/channels/discord/views.py @@ -1,4 +1,4 @@ -"""Persistent interactive views for Discord approval and plan review. +"""Persistent interactive views for Discord approval. These views use static ``custom_id`` values so they survive bot restarts. Correlation information (``ws_id`` and ``correlation_id``) is stored in the @@ -200,167 +200,3 @@ class ApprovalView: approved=approved, always=always, ) - - -# --------------------------------------------------------------------------- -# PlanReviewView -# --------------------------------------------------------------------------- - - -class PlanReviewView: - """Persistent view with Approve Plan / Request Changes buttons.""" - - def __init__(self, bot: TurnstoneBot) -> None: - import discord - - self.bot = bot - view_self = self - - class _FeedbackModal(discord.ui.Modal, title="Request Changes"): # type: ignore[call-arg] - feedback: discord.ui.TextInput[_FeedbackModal] = discord.ui.TextInput( - label="Feedback", - style=discord.TextStyle.paragraph, - placeholder="Describe the changes you'd like...", - required=True, - max_length=2000, - ) - - def __init__(modal_self, ws_id: str, correlation_id: str) -> None: # noqa: N805 - super().__init__() - modal_self.ws_id = ws_id - modal_self.correlation_id = correlation_id - - async def on_submit(modal_self, interaction: discord.Interaction) -> None: # noqa: N805 - await view_self._send_feedback( - interaction, - modal_self.ws_id, - modal_self.correlation_id, - str(modal_self.feedback), - ) - - self._modal_cls = _FeedbackModal - - class _View(discord.ui.View): - def __init__(inner_self) -> None: # noqa: N805 - super().__init__(timeout=None) - - @discord.ui.button( - label="Approve Plan", - style=discord.ButtonStyle.green, - custom_id="ts:plan_approve", - ) - async def approve_plan( - inner_self, # noqa: N805 - interaction: discord.Interaction, - button: discord.ui.Button[_View], - ) -> None: - await view_self._handle_approve(interaction) - - @discord.ui.button( - label="Request Changes", - style=discord.ButtonStyle.secondary, - custom_id="ts:plan_changes", - ) - async def request_changes( - inner_self, # noqa: N805 - interaction: discord.Interaction, - button: discord.ui.Button[_View], - ) -> None: - await view_self._handle_changes(interaction) - - self._view = _View() - - async def _handle_approve(self, interaction: discord.Interaction) -> None: - """Approve the plan (empty feedback = approved).""" - parsed = _parse_footer(interaction) - if parsed is None: - await interaction.response.send_message( - "Could not determine workstream context.", - ephemeral=True, - ) - return - - ws_id, correlation_id, owner_id = parsed - - if not owner_id or str(interaction.user.id) != owner_id: - await _deny_non_owner(interaction, "approve") - return - - user_id = await self.bot.router.resolve_user("discord", str(interaction.user.id)) - if user_id is None: - await interaction.response.send_message( - "Your Discord account is not linked. Use `/link` first.", - ephemeral=True, - ) - return - - # Defer before doing async work so Discord doesn't time out. - await interaction.response.defer(ephemeral=True) - - await self.bot.router.send_plan_feedback( - ws_id=ws_id, - correlation_id=correlation_id, - feedback="", - ) - - await _disable_buttons(interaction, "Approved") - await interaction.followup.send("Plan **approved**.", ephemeral=True) - log.info( - "discord.plan_approved", - ws_id=ws_id, - correlation_id=correlation_id, - ) - - async def _handle_changes(self, interaction: discord.Interaction) -> None: - """Open a modal for feedback text.""" - parsed = _parse_footer(interaction) - if parsed is None: - await interaction.response.send_message( - "Could not determine workstream context.", - ephemeral=True, - ) - return - - ws_id, correlation_id, owner_id = parsed - - if not owner_id or str(interaction.user.id) != owner_id: - await _deny_non_owner(interaction, "request changes on") - return - - modal = self._modal_cls(ws_id, correlation_id) - await interaction.response.send_modal(modal) - - async def _send_feedback( - self, - interaction: discord.Interaction, - ws_id: str, - correlation_id: str, - feedback: str, - ) -> None: - """Send plan feedback after the modal is submitted.""" - user_id = await self.bot.router.resolve_user("discord", str(interaction.user.id)) - if user_id is None: - await interaction.response.send_message( - "Your Discord account is not linked. Use `/link` first.", - ephemeral=True, - ) - return - - # Defer before doing async work so Discord doesn't time out. - await interaction.response.defer(ephemeral=True) - - await self.bot.router.send_plan_feedback( - ws_id=ws_id, - correlation_id=correlation_id, - feedback=feedback, - ) - - await interaction.followup.send( - "Feedback submitted. The plan will be revised.", - ephemeral=True, - ) - log.info( - "discord.plan_changes_requested", - ws_id=ws_id, - correlation_id=correlation_id, - ) diff --git a/turnstone/channels/slack/bot.py b/turnstone/channels/slack/bot.py index 9209b546..7c5a5868 100644 --- a/turnstone/channels/slack/bot.py +++ b/turnstone/channels/slack/bot.py @@ -47,7 +47,6 @@ from turnstone.sdk.events import ( ContentEvent, ErrorEvent, IntentVerdictEvent, - PlanReviewEvent, ServerEvent, StreamEndEvent, ThinkingStartEvent, @@ -277,9 +276,6 @@ class TurnstoneSlackBot: self._streaming: dict[str, StreamingMessage] = {} self._pending_approval: dict[str, PendingApproval] = {} - # (channel, message_ts, owner_user_id) per pending plan review, so - # _on_plan_* handlers can reject clicks from non-owners. - self._pending_plan_review_ts: dict[str, tuple[str, str, str]] = {} self._notify_ws_map: dict[str, tuple[str, SlackRoute]] = {} # Per-workstream override used to route the next streamed assistant # response back into a Slack notification reply thread instead of the @@ -319,9 +315,6 @@ class TurnstoneSlackBot: self._app.action("ts_approve")(_approve_cb) self._app.action("ts_deny")(_deny_cb) - self._app.action("ts_plan_approve")(self._on_plan_approve) - self._app.action("ts_plan_request_changes")(self._on_plan_request_changes) - self._app.view("ts_plan_feedback_modal")(self._on_plan_feedback_modal) async def start(self) -> None: from slack_bolt.adapter.socket_mode.async_handler import AsyncSocketModeHandler @@ -926,166 +919,6 @@ class TurnstoneSlackBot: except Exception: log.debug(f"slack.{event_key}_message_update_failed", exc_info=True) - async def _ensure_plan_review_owner( - self, - entry: tuple[str, str, str] | None, - actor_user_id: str, - channel: str, - verb: str, - ) -> bool: - """Return True when *actor_user_id* owns the pending plan review. - - The gateway forwards ``/plan`` calls with its service-scoped JWT, - and the server bypasses ownership checks on service scope — so - every plan-review interaction needs an adapter-side owner gate. - """ - if entry is None: - log.warning("slack.plan_review_missing_entry", actor_user_id=actor_user_id) - await self._client.chat_postEphemeral( - channel=channel, - user=actor_user_id, - text="This plan review can no longer be verified. Please retry from the active session.", - ) - return False - _channel, _ts, owner_user_id = entry - if not owner_user_id or actor_user_id != owner_user_id: - await self._client.chat_postEphemeral( - channel=channel, - user=actor_user_id, - text=f"Only the session owner can {verb} this plan.", - ) - return False - return True - - async def _on_plan_approve(self, ack: Any, body: dict[str, Any]) -> None: - await ack() - ws_id = body["actions"][0].get("value", "") - log.info("slack.plan_approve_clicked", ws_id=ws_id) - if not ws_id: - return - - actor_user_id = body.get("user", {}).get("id", "") - channel = body["container"]["channel_id"] - entry = self._pending_plan_review_ts.get(ws_id) - if not await self._ensure_plan_review_owner(entry, actor_user_id, channel, "approve"): - return - - self._streaming.pop(ws_id, None) - await self.router.send_plan_feedback(ws_id, "", "") - log.info("slack.plan_feedback_sent", ws_id=ws_id, feedback="") - - ts = body["container"]["message_ts"] - - self._pending_plan_review_ts.pop(ws_id, None) - - try: - await self._client.chat_update( - channel=channel, - ts=ts, - text="Plan approved", - blocks=[], - ) - except Exception: - log.debug("slack.plan_review_approve_update_failed", exc_info=True) - - async def _on_plan_request_changes(self, ack: Any, body: dict[str, Any], client: Any) -> None: - await ack() - ws_id = body["actions"][0].get("value", "") - if not ws_id: - return - - actor_user_id = body.get("user", {}).get("id", "") - channel = body["container"]["channel_id"] - entry = self._pending_plan_review_ts.get(ws_id) - if not await self._ensure_plan_review_owner( - entry, actor_user_id, channel, "request changes on" - ): - return - - trigger_id = body.get("trigger_id", "") - if not trigger_id: - return - - await client.views_open( - trigger_id=trigger_id, - view={ - "type": "modal", - "callback_id": "ts_plan_feedback_modal", - "private_metadata": ws_id, - "title": {"type": "plain_text", "text": "Plan feedback"}, - "submit": {"type": "plain_text", "text": "Send"}, - "close": {"type": "plain_text", "text": "Cancel"}, - "blocks": [ - { - "type": "input", - "block_id": "feedback_block", - "label": {"type": "plain_text", "text": "Requested changes"}, - "element": { - "type": "plain_text_input", - "action_id": "feedback_input", - "multiline": True, - }, - } - ], - }, - ) - - async def _on_plan_feedback_modal( - self, ack: Any, body: dict[str, Any], view: dict[str, Any] - ) -> None: - await ack() - - ws_id = view.get("private_metadata", "") - if not ws_id: - return - - actor_user_id = body.get("user", {}).get("id", "") - entry = self._pending_plan_review_ts.get(ws_id) - if entry is None: - # No pending review (stale modal) — silently drop. - return - _ignored_channel, _ignored_ts, owner_user_id = entry - if not owner_user_id or actor_user_id != owner_user_id: - # Modal submit can't emit ephemeral; just log and drop. - log.warning( - "slack.plan_feedback_non_owner", - ws_id=ws_id, - actor_user_id=actor_user_id, - owner_user_id=owner_user_id, - ) - return - - feedback = ( - view.get("state", {}) - .get("values", {}) - .get("feedback_block", {}) - .get("feedback_input", {}) - .get("value", "") - .strip() - ) - - if not feedback: - feedback = "Please revise the plan." - - log.info("slack.plan_feedback_modal_submitted", ws_id=ws_id, feedback=feedback) - - self._streaming.pop(ws_id, None) - await self.router.send_plan_feedback(ws_id, "", feedback) - log.info("slack.plan_feedback_sent", ws_id=ws_id, feedback=feedback) - - entry = self._pending_plan_review_ts.pop(ws_id, None) - if entry is not None: - channel, ts, _owner = entry - try: - await self._client.chat_update( - channel=channel, - ts=ts, - text="Plan changes requested", - blocks=[], - ) - except Exception: - log.debug("slack.plan_review_modal_update_failed", exc_info=True) - async def subscribe_ws(self, ws_id: str, channel_id: str) -> None: # Recover from a dead SSE task before the early-return check. If the # prior listener died with an unhandled exception, its ws_id is still @@ -1118,7 +951,6 @@ class TurnstoneSlackBot: self._subscribed_ws.discard(ws_id) self._streaming.pop(ws_id, None) self._pending_approval.pop(ws_id, None) - self._pending_plan_review_ts.pop(ws_id, None) self._clear_notification_tracking_for_ws(ws_id) async def unsubscribe_ws(self, ws_id: str) -> None: @@ -1191,8 +1023,6 @@ class TurnstoneSlackBot: await self._handle_approve_request(ws_id, route, event) elif isinstance(event, IntentVerdictEvent): await self._handle_intent_verdict(ws_id, event) - elif isinstance(event, PlanReviewEvent): - await self._handle_plan_review(ws_id, route, event) elif isinstance(event, ApprovalResolvedEvent): await self._handle_approval_resolved(ws_id, event) elif isinstance(event, StreamEndEvent): @@ -1307,53 +1137,6 @@ class TurnstoneSlackBot: except Exception: log.debug("slack.verdict_message_update_failed", ws_id=ws_id, exc_info=True) - async def _handle_plan_review( - self, ws_id: str, route: SlackRoute, event: PlanReviewEvent - ) -> None: - slack_channel = route.channel - thread_ts = route.thread_ts or "" - owner_user_id = route.user_id or "" - - log.info("slack.plan_review_received", ws_id=ws_id) - plan_preview = _sanitize_slack_preview(event.content, max_length=2000) - blocks = [ - { - "type": "section", - "text": { - "type": "mrkdwn", - "text": f"*Plan Review*\n```{plan_preview}```", - }, - }, - { - "type": "actions", - "elements": [ - { - "type": "button", - "text": {"type": "plain_text", "text": "Approve"}, - "style": "primary", - "action_id": "ts_plan_approve", - "value": ws_id, - }, - { - "type": "button", - "text": {"type": "plain_text", "text": "Request changes"}, - "style": "danger", - "action_id": "ts_plan_request_changes", - "value": ws_id, - }, - ], - }, - ] - - resp = await self._client.chat_postMessage( - channel=slack_channel, - thread_ts=thread_ts or None, - text="Plan review required", - blocks=cast("list[dict[str, Any]]", blocks), - ) - if resp.get("ok"): - self._pending_plan_review_ts[ws_id] = (slack_channel, resp["ts"], owner_user_id) - async def _handle_approval_resolved(self, ws_id: str, event: ApprovalResolvedEvent) -> None: entry = self._pending_approval.pop(ws_id, None) if entry is None: diff --git a/turnstone/cli.py b/turnstone/cli.py index 280172db..f81bedbe 100644 --- a/turnstone/cli.py +++ b/turnstone/cli.py @@ -294,25 +294,6 @@ class TerminalUI(SessionUI): sys.stdout.write(f"\n {DIM}[{' · '.join(parts)}]{RESET}\n") sys.stdout.flush() - def on_plan_review(self, content: str) -> str: - sys.stdout.write(f"\n{DIM}{'─' * 60}{RESET}\n") - for line in content.splitlines(): - sys.stdout.write(f" {line}\n") - sys.stdout.write(f"{DIM}{'─' * 60}{RESET}\n") - sys.stdout.flush() - try: - prompt_text = ( - f" \001{BOLD}\002Plan ready.\001{RESET}\002 " - f"\001{DIM}\002[enter to approve, feedback to amend, " - f"ctrl-c to reject]\001{RESET}\002 " - ) - resp = input(prompt_text).strip() - except EOFError: - resp = "" - except KeyboardInterrupt: - resp = "reject" - return resp - def on_info(self, message: str) -> None: print(message) @@ -506,16 +487,6 @@ class WorkstreamTerminalUI(TerminalUI): if self.is_foreground: super().on_tool_output_chunk(call_id, chunk) - def on_plan_review(self, content: str) -> str: - # Must wait until foregrounded to show plan review - if not self.is_foreground: - self._buffer( - "info", - f"{YELLOW}[Plan ready — switch to this workstream to review]{RESET}", - ) - self._fg_event.wait() - return super().on_plan_review(content) - def approve_tools(self, items: list[dict[str, Any]]) -> tuple[bool, str | None]: """Block until foregrounded if in background, then show approval prompt.""" if not self.is_foreground: diff --git a/turnstone/console/coordinator_ui.py b/turnstone/console/coordinator_ui.py index 11164b60..e79baa4e 100644 --- a/turnstone/console/coordinator_ui.py +++ b/turnstone/console/coordinator_ui.py @@ -5,9 +5,8 @@ Mirrors ``turnstone.server.WebUI`` but scoped to the console's needs: - Per-session SSE listener fan-out (inherited from :class:`SessionUIBase` — same ``threading.Lock`` + queue list pattern WebUI uses). -- ``threading.Event`` + ``_approval_result`` / ``_plan_result`` for - blocking the worker thread until a console endpoint delivers the - decision (inherited). +- ``threading.Event`` + ``_approval_result`` for blocking the worker + thread until a console endpoint delivers the decision (inherited). - Per-ws metric tracking + turn-content accumulator + activity bookkeeping (inherited from :class:`SessionUIBase` post the rich ``ws_state`` payload lift). Coord populates the same @@ -94,8 +93,8 @@ class ConsoleCoordinatorUI(SessionUIBase): # ------------------------------------------------------------------ # SessionUI protocol — approvals # - # ``approve_tools`` / ``resolve_approval`` / ``resolve_plan`` are - # inherited from :class:`SessionUIBase`. The shared body covers + # ``approve_tools`` / ``resolve_approval`` are inherited from + # :class:`SessionUIBase`. The shared body covers # tool-policy gating, per-tool auto-approve, blanket auto-approve, # heuristic-verdict persistence, and activity tagging the same way # interactive sessions get them. ``__budget_override__`` is @@ -103,19 +102,6 @@ class ConsoleCoordinatorUI(SessionUIBase): # no-op on coord (coord workstreams don't have token budgets). # ------------------------------------------------------------------ - def on_plan_review(self, content: str) -> str: - # Coordinator sessions don't fire plan_agent (AGENT_TOOLS is [] - # for coordinator kind) so this path shouldn't normally run. - # Implemented defensively for SessionUI protocol compatibility. - self._plan_event.clear() - self._pending_plan_review = {"type": "plan_review", "content": content} - self._enqueue(self._pending_plan_review) - if not self._plan_event.wait(timeout=self._APPROVAL_WAIT_TIMEOUT): - log.warning("coord_ui.plan_review_timeout ws=%s", self.ws_id) - self.resolve_plan("reject") - self._pending_plan_review = None - return self._plan_result - # ------------------------------------------------------------------ # SessionUI protocol — broadcast hook + state change + rename # ------------------------------------------------------------------ diff --git a/turnstone/console/server.py b/turnstone/console/server.py index bc88655d..51e42480 100644 --- a/turnstone/console/server.py +++ b/turnstone/console/server.py @@ -695,7 +695,6 @@ _ROUTE_PROXY_AUDIT_ACTIONS: dict[str, str] = { "rewind": "route.rewind", "retry": "route.retry", "command": "route.command", - "plan": "route.plan", "close": "route.workstream.close", } @@ -2289,8 +2288,8 @@ async def route_proxy(request: Request) -> Response: """Generic routing proxy for send/approve/cancel/command/close. Path-keyed shape: ``POST/DELETE /v1/api/route/workstreams/{ws_id}/`` - (or ``POST /v1/api/route/`` for the body-keyed plan/command - legacies still in scope). ``verb`` drives the audit action lookup; + (or ``POST /v1/api/route/`` for the body-keyed ``command`` + legacy still in scope). ``verb`` drives the audit action lookup; DELETE on ``/send`` is treated as dequeue for audit attribution. """ from turnstone.core.auth import require_any_permission @@ -2306,7 +2305,7 @@ async def route_proxy(request: Request) -> Response: # check (the upstream server gates again), but failing fast at the # proxy avoids a cluster round-trip on a forbidden request and keeps # the 403 attributed to the proxy in audit logs. Only the two verbs - # whose perms exist; other verbs (send/cancel/dequeue/command/plan) + # whose perms exist; other verbs (send/cancel/dequeue/command) # remain authenticated-only and pre-existing — leaving them alone # rather than expanding scope. ``admin.coordinator`` accepted as # an alternative on each so coord sessions driving interactive @@ -2353,8 +2352,8 @@ async def route_proxy(request: Request) -> Response: ) # Path-keyed shape (post-1.5) carries ws_id in the URL; the - # legacy plan/command routes still mount at body-keyed URLs and - # supply ws_id via the JSON body. Try path first, fall back to body. + # legacy command route still mounts at a body-keyed URL and + # supplies ws_id via the JSON body. Try path first, fall back to body. ws_id = request.path_params.get("ws_id", "") or str(body.get("ws_id") or "") if not ws_id: return _record_route( @@ -3260,7 +3259,6 @@ def _coord_events_replay( that fired since it surfaced. Without this replay a refresh loses the judge chip on the pending approval until the operator re-invokes the action. - 3. Pending plan-review (if any). Coord still skips conversation history — the dashboard fetches it via a separate ``GET /history`` endpoint and doesn't want a @@ -3287,9 +3285,6 @@ def _coord_events_replay( cached_verdicts = list(llm_verdicts.values()) for v in cached_verdicts: yield {"type": "intent_verdict", **v} - pending_plan = getattr(ui, "_pending_plan_review", None) - if pending_plan is not None: - yield pending_plan async def _coord_create_validate_request( @@ -8454,8 +8449,6 @@ def _emit_models_changed(request: Request) -> None: _MODEL_AFFECTING_SETTING_KEYS: frozenset[str] = frozenset( { "model.default_alias", - "model.plan_alias", - "model.plan_effort", "model.task_alias", "model.task_effort", "coordinator.model_alias", @@ -10280,7 +10273,7 @@ def _refresh_coord_registry(app_state: Any, storage: Any) -> None: 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 + # config issues — empty models, default/fallback/agent/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. @@ -10295,9 +10288,7 @@ def _refresh_coord_registry(app_state: Any, storage: Any) -> None: 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: @@ -12749,7 +12740,6 @@ def create_app( methods=["POST"], ), Route("/api/route/command", route_proxy, methods=["POST"]), - Route("/api/route/plan", route_proxy, methods=["POST"]), Route( "/api/route/workstreams/{ws_id}/close", route_proxy, diff --git a/turnstone/console/static/admin.js b/turnstone/console/static/admin.js index b22a27e0..f4f909ae 100644 --- a/turnstone/console/static/admin.js +++ b/turnstone/console/static/admin.js @@ -19,7 +19,6 @@ let _mobileSidebarOpen = false; // alias list, and whose empty-string option renders as "(server default)". const ALIAS_SETTING_KEYS = [ "model.default_alias", - "model.plan_alias", "model.task_alias", "channels.default_model_alias", "audio.stt_model_alias", @@ -30,7 +29,7 @@ const ALIAS_SETTING_KEYS = [ // opposed to "no value" — distinct from the literal "none" choice (e.g. // reasoning_effort="none" actually disables reasoning, very different // from leaving it unset). -const INHERIT_EMPTY_LABEL_KEYS = ["model.plan_effort", "model.task_effort"]; +const INHERIT_EMPTY_LABEL_KEYS = ["model.task_effort"]; // --------------------------------------------------------------------------- // View switching (called from app.js showHome/drillDown pattern) @@ -2979,8 +2978,6 @@ function loadSettings() { const roleKeys = { "coordinator.model_alias": 1, "coordinator.reasoning_effort": 1, - "model.plan_alias": 1, - "model.plan_effort": 1, "model.task_alias": 1, "model.task_effort": 1, "channels.default_model_alias": 1, @@ -5144,8 +5141,8 @@ let _modelCreateTrigger = null; // ``fallbackKind`` controls how the empty/blank option in the alias // dropdown is labelled. Coordinator and Judge fall back to a single // well-defined alias (model.default_alias / coordinator alias) so we -// surface that concrete model in the placeholder. Plan/Task agents -// cascade through ``[model].plan_model → [model].agent_model → +// surface that concrete model in the placeholder. The Task agent +// cascades through ``[model].task_model → [model].agent_model → // session model`` per turnstone/core/settings_registry.py — there's // no single "default" to advertise, so the blank reads "(inherit)" // to match the vocabulary of the reasoning-effort dropdowns. @@ -5172,14 +5169,6 @@ const MODEL_ROLES = [ aliasKey: "judge.output_guard_model", fallbackKind: "default", }, - { - label: "Plan agent", - description: - "plan_agent sub-agent — produces high-level plans before task dispatch.", - aliasKey: "model.plan_alias", - effortKey: "model.plan_effort", - fallbackKind: "inherit", - }, { label: "Task agent", description: diff --git a/turnstone/core/adapters/_ui_cleanup.py b/turnstone/core/adapters/_ui_cleanup.py index ca0c370c..f6fb6ddf 100644 --- a/turnstone/core/adapters/_ui_cleanup.py +++ b/turnstone/core/adapters/_ui_cleanup.py @@ -36,9 +36,6 @@ def cleanup_session_ui(ws: Workstream) -> None: if hasattr(ui, "_approval_event"): ui._approval_result = False, None # type: ignore[attr-defined] ui._approval_event.set() - if hasattr(ui, "_plan_event"): - ui._plan_result = "reject" # type: ignore[attr-defined] - ui._plan_event.set() if hasattr(ui, "_fg_event"): ui._fg_event.set() if hasattr(ui, "_listeners_lock"): diff --git a/turnstone/core/adapters/interactive_adapter.py b/turnstone/core/adapters/interactive_adapter.py index 45772697..b7f9b46a 100644 --- a/turnstone/core/adapters/interactive_adapter.py +++ b/turnstone/core/adapters/interactive_adapter.py @@ -6,9 +6,8 @@ surface is asymmetric on the interactive side and the asymmetry is load-bearing — see ``InteractiveAdapter`` docstring. Uses ``WebUI``'s per-UI listener set for ``cleanup_ui`` — the same -hooks (``_approval_event`` / ``_plan_event`` / ``_fg_event`` + the -``ws_closed`` broadcast) the old ``WorkstreamManager._cleanup_ui`` -touched. +hooks (``_approval_event`` / ``_fg_event`` + the ``ws_closed`` +broadcast) the old ``WorkstreamManager._cleanup_ui`` touched. """ from __future__ import annotations diff --git a/turnstone/core/auth.py b/turnstone/core/auth.py index b97a0f57..b64a2ded 100644 --- a/turnstone/core/auth.py +++ b/turnstone/core/auth.py @@ -304,7 +304,6 @@ PUBLIC_PREFIXES: tuple[str, ...] = ("/static/", "/shared/", "/acme/") WRITE_PATHS: frozenset[str] = frozenset( { - "/api/plan", "/api/command", "/api/workstreams/new", "/api/cluster/workstreams/new", diff --git a/turnstone/core/judge.py b/turnstone/core/judge.py index 90903b2b..64fb80c5 100644 --- a/turnstone/core/judge.py +++ b/turnstone/core/judge.py @@ -532,16 +532,6 @@ _LOW_RULES: list[_HeuristicRule] = [ intent_template="List directory: {arg_snippet}", reasoning_template="Listing directory contents is a read-only operation.", ), - _HeuristicRule( - name="man-tool", - risk_level="low", - confidence=0.85, - recommendation="approve", - tool_pattern="man", - arg_patterns=[], - intent_template="Manual page lookup: {arg_snippet}", - reasoning_template="Looking up a man page is a read-only operation.", - ), _HeuristicRule( name="use-prompt", risk_level="low", @@ -919,7 +909,7 @@ class IntentJudge: # Resolve judge model via ModelRegistry alias, otherwise self- # consistency on the session model. ``judge.model`` is alias-only # — same contract as ``coordinator.model_alias`` / - # ``model.plan_alias`` / ``model.task_alias``. A non-alias value + # ``model.task_alias``. A non-alias value # used to be accepted as a raw model id pinned onto the session # provider, but that path silently broke whenever the session # provider didn't speak that model id (e.g. coordinator on diff --git a/turnstone/core/model_registry.py b/turnstone/core/model_registry.py index 49387de6..778442af 100644 --- a/turnstone/core/model_registry.py +++ b/turnstone/core/model_registry.py @@ -73,7 +73,6 @@ def _validate_registry_args( default: str, fallback: list[str] | None, agent_model: str | None, - plan_model: str | None, task_model: str | None, ) -> None: """Validate ModelRegistry construction / reload arguments. @@ -97,8 +96,6 @@ def _validate_registry_args( raise ValueError(f"Fallback model '{alias}' not found in registry") if agent_model and agent_model not in models: raise ValueError(f"Agent model '{agent_model}' not found in registry") - if plan_model and plan_model not in models: - raise ValueError(f"Plan model '{plan_model}' not found in registry") if task_model and task_model not in models: raise ValueError(f"Task model '{task_model}' not found in registry") @@ -110,14 +107,10 @@ class ModelRegistry: models: Mapping of alias → ModelConfig. default: Alias of the default model. fallback: Ordered list of aliases to try when the primary model fails. - agent_model: Optional alias for plan/task sub-agents (single-knob - fallback used when ``plan_model``/``task_model`` are unset). - plan_model: Optional alias for the plan_agent sub-agent. Overrides - ``agent_model`` for plan calls; falls back to it when unset. + agent_model: Optional alias for the task_agent sub-agent (single-knob + fallback used when ``task_model`` is unset). task_model: Optional alias for the task_agent sub-agent. Overrides ``agent_model`` for task calls; falls back to it when unset. - plan_effort: Reasoning effort for plan_agent. ``None`` means use the - built-in default of ``"high"`` (preserves prior behaviour). task_effort: Reasoning effort for task_agent. ``None`` means inherit the parent session's reasoning effort. """ @@ -128,20 +121,16 @@ class ModelRegistry: default: str, fallback: list[str] | None = None, agent_model: str | None = None, - plan_model: str | None = None, task_model: str | None = None, - plan_effort: str | None = None, task_effort: str | None = None, ) -> None: - _validate_registry_args(models, default, fallback, agent_model, plan_model, task_model) + _validate_registry_args(models, default, fallback, agent_model, task_model) self._models = dict(models) self.default = default self.fallback = list(fallback) if fallback else [] self.agent_model = agent_model - self.plan_model = plan_model self.task_model = task_model - self.plan_effort = plan_effort self.task_effort = task_effort self._clients: dict[str, Any] = {} self._providers: dict[str, LLMProvider] = {} @@ -199,33 +188,24 @@ class ModelRegistry: def resolve_agent_alias(self, kind: str) -> str | None: """Return the configured alias for a sub-agent ``kind``. - Per-kind overrides (``plan_model``/``task_model``) win over the - legacy single-knob ``agent_model``. Returns ``None`` when nothing - is configured (caller should fall back to the session model). + The per-kind override (``task_model``) wins over the legacy + single-knob ``agent_model``. Returns ``None`` when nothing is + configured (caller should fall back to the session model). - Recognised kinds: ``"plan"``, ``"task"``. Any other value (e.g. - ``"agent"``, eval/utility paths) returns the legacy ``agent_model`` - as-is — preserves prior behaviour for non-plan/task callers. + Recognised kind: ``"task"``. Any other value (e.g. ``"agent"``, + eval/utility paths) returns the legacy ``agent_model`` as-is — + preserves prior behaviour for non-task callers. """ - if kind == "plan": - return self.plan_model or self.agent_model if kind == "task": return self.task_model or self.agent_model return self.agent_model - # Built-in default effort for plan_agent — preserves the value the three - # plan call sites used to pass explicitly before the split. - PLAN_DEFAULT_EFFORT = "high" - def resolve_agent_effort(self, kind: str) -> str | None: """Return the reasoning effort for a sub-agent ``kind``. - Plan defaults to :attr:`PLAN_DEFAULT_EFFORT` (back-compat with the - previously hardcoded ``"high"``). Task returns ``None`` to indicate - the caller should fall through to the session default. + Task returns ``None`` to indicate the caller should fall through + to the session default. """ - if kind == "plan": - return self.plan_effort or self.PLAN_DEFAULT_EFFORT if kind == "task": return self.task_effort return None @@ -248,9 +228,7 @@ class ModelRegistry: default: str, fallback: list[str] | None = None, agent_model: str | None = None, - plan_model: str | None = None, task_model: str | None = None, - plan_effort: str | None = None, task_effort: str | None = None, ) -> None: """Hot-reload all model configs. Thread-safe; clears cached clients. @@ -258,16 +236,14 @@ class ModelRegistry: Validates arguments before mutating state so a bad reload does not leave the registry in an inconsistent state. """ - _validate_registry_args(models, default, fallback, agent_model, plan_model, task_model) + _validate_registry_args(models, default, fallback, agent_model, task_model) with self._client_lock: old_models = self._models self._models = dict(models) self.default = default self.fallback = list(fallback) if fallback else [] self.agent_model = agent_model - self.plan_model = plan_model self.task_model = task_model - self.plan_effort = plan_effort self.task_effort = task_effort # Selective teardown — close + drop only clients whose # connection target actually changed (alias removed, or @@ -372,10 +348,9 @@ def load_model_registry( 3. CLI ``--base-url`` / ``--api-key`` / ``--model`` always create a ``"default"`` entry. 4. ``[model].default``, ``[model].fallback``, ``[model].agent_model``, - ``[model].plan_model``, ``[model].task_model``, - ``[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. + ``[model].task_model``, ``[model].task_effort`` control routing. + ``task_model`` overrides ``agent_model`` for the task sub-agent; + it falls back to ``agent_model`` 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. @@ -519,9 +494,9 @@ def load_model_registry( # 3. Back-compat shim: synthesize a "default" alias from CLI/auto-detected # ``--base-url`` + ``--model`` only when no DB or config.toml models exist. # Auto-creating "default" alongside DB models leaks a non-routing alias - # into the public list — the LLM picks it in plan_agent / task_agent + # into the public list — the LLM picks it in task_agent # ``model=`` and silently bypasses the operator's per-role - # plan_alias / task_alias overrides (the "default" alias points at + # task_alias override (the "default" alias points at # whatever LLM_BASE_URL was at boot, not at the configured default). if not configs and model: configs["default"] = ModelConfig( @@ -573,24 +548,19 @@ def load_model_registry( else: log.warning("Fallback alias '%s' not found in models, ignoring", alias) - # Agent model (legacy single-knob shared between plan_agent and task_agent) + # Agent model (legacy single-knob fallback for the task_agent sub-agent) agent_model = model_section.get("agent_model") if agent_model and agent_model not in configs: log.warning("Configured agent_model '%s' not found, ignoring", agent_model) agent_model = None - # Per-kind sub-agent models — override agent_model for each role - plan_model = model_section.get("plan_model") - if plan_model and plan_model not in configs: - log.warning("Configured plan_model '%s' not found, ignoring", plan_model) - plan_model = None + # Per-kind sub-agent model — overrides agent_model for the task role task_model = model_section.get("task_model") if task_model and task_model not in configs: log.warning("Configured task_model '%s' not found, ignoring", task_model) task_model = None - # Per-kind reasoning effort. None means: plan defaults to "high" (back- - # compat with the previous hardcoded value); task inherits the session. + # Per-kind reasoning effort. None means task inherits the session. # Typos in config.toml shouldn't silently flow to the provider — log and # drop unknown values, mirroring the model-not-found warning above. valid_efforts = {"none", "minimal", "low", "medium", "high", "xhigh", "max"} @@ -599,7 +569,7 @@ def load_model_registry( if value is None: return None # Treat empty / whitespace as unset. Operators commonly write - # `plan_effort = ""` to make "leave it default" explicit; warning + # `task_effort = ""` to make "leave it default" explicit; warning # on that benign case would just be noise. coerced = str(value).strip().lower() if not coerced: @@ -615,7 +585,6 @@ def load_model_registry( return None return coerced - plan_effort = _validate_effort(model_section.get("plan_effort"), "plan_effort") task_effort = _validate_effort(model_section.get("task_effort"), "task_effort") return ModelRegistry( @@ -623,9 +592,7 @@ def load_model_registry( default=default_alias, fallback=fallback, agent_model=agent_model, - plan_model=plan_model, task_model=task_model, - plan_effort=plan_effort, task_effort=task_effort, ) diff --git a/turnstone/core/sandbox.py b/turnstone/core/sandbox.py deleted file mode 100644 index d03bda06..00000000 --- a/turnstone/core/sandbox.py +++ /dev/null @@ -1,345 +0,0 @@ -"""Sandboxed Python executor for the math tool.""" - -from __future__ import annotations - -import ast -import multiprocessing -import re -import traceback -from typing import Any - -_MATH_BLOCKED_BUILTINS = { - "open", - "exec", - "eval", - "compile", - "input", - "breakpoint", - "memoryview", - "globals", - "locals", - "vars", - # Reflection primitives — bypass AST dunder checks via runtime strings - "getattr", - "setattr", - "delattr", - # Type system — can reconstruct arbitrary classes - "type", - # Import — the replaced _safe_import is in the namespace, but block the - # name so direct __import__ calls are caught by the AST validator - "__import__", -} - -_MATH_BLOCKED_MODULES = { - "os", - "sys", - "subprocess", - "shutil", - "pathlib", - "socket", - "http", - "urllib", - "requests", - "pickle", - "marshal", - "shelve", - "dbm", - "sqlite3", - "ctypes", - "multiprocessing", - "threading", - "asyncio", - "concurrent", - "signal", - "pty", - "tty", - "termios", - "fcntl", - "resource", - "syslog", - "tempfile", - "io", - "builtins", - "__builtin__", - "importlib", -} - - -class _ASTValidator(ast.NodeVisitor): - """Validates AST for dangerous constructs.""" - - def __init__(self) -> None: - self.errors: list[str] = [] - - def visit_Import(self, node: ast.Import) -> None: - for alias in node.names: - if alias.name.split(".")[0] in _MATH_BLOCKED_MODULES: - self.errors.append(f"Import of '{alias.name}' is not allowed") - self.generic_visit(node) - - def visit_ImportFrom(self, node: ast.ImportFrom) -> None: - if node.module and node.module.split(".")[0] in _MATH_BLOCKED_MODULES: - self.errors.append(f"Import from '{node.module}' is not allowed") - self.generic_visit(node) - - def visit_Call(self, node: ast.Call) -> None: - if isinstance(node.func, ast.Name) and node.func.id in _MATH_BLOCKED_BUILTINS: - self.errors.append(f"Call to '{node.func.id}' is not allowed") - self.generic_visit(node) - - def visit_Attribute(self, node: ast.Attribute) -> None: - if ( - node.attr.startswith("__") - and node.attr.endswith("__") - and node.attr not in {"__name__", "__doc__", "__class__"} - ): - self.errors.append(f"Access to '{node.attr}' is not allowed") - # Block operator.attrgetter/itemgetter which act as getattr bypasses - if node.attr in ("attrgetter", "itemgetter"): - self.errors.append(f"Access to '{node.attr}' is not allowed") - self.generic_visit(node) - - -def validate_math_code(code: str) -> list[str]: - """Validate code for dangerous constructs. Returns list of errors.""" - try: - tree = ast.parse(code) - except SyntaxError as e: - lines = code.split("\n") - msg = f"Syntax error on line {e.lineno}: {e.msg}" - if e.lineno and e.lineno <= len(lines): - msg += f"\n {e.lineno}: {lines[e.lineno - 1]}" - if e.offset: - msg += f"\n {' ' * (e.offset - 1)}^" - return [msg] - except (ValueError, UnicodeError) as e: - return [f"Code contains invalid characters: {e}"] - v = _ASTValidator() - v.visit(tree) - return v.errors - - -def _math_exec_in_process(code: str, result_queue: multiprocessing.Queue[tuple[str, str]]) -> None: - """Execute code in a subprocess, put (status, output) in queue.""" - import contextlib - import signal as _signal - import sys as _sys - from io import StringIO - - _signal.signal(_signal.SIGTERM, _signal.SIG_DFL) - _signal.signal(_signal.SIGINT, _signal.SIG_DFL) - _sys.set_int_max_str_digits(100_000) - - try: - captured = StringIO() - _sys.stdout = captured - - def _safe_import(name: str, *args: Any, **kwargs: Any) -> Any: - if name.split(".")[0] in _MATH_BLOCKED_MODULES: - raise ImportError(f"Import of '{name}' is blocked") - mod = original_import(name, *args, **kwargs) - # Strip __builtins__ from every imported module so - # module.__builtins__['__import__'] can't bypass _safe_import - # (covers operator.attrgetter('__builtins__') and similar). - if hasattr(mod, "__builtins__"): - with contextlib.suppress(AttributeError, TypeError): - mod.__builtins__ = {} # type: ignore[attr-defined] - return mod - - original_import = ( - __builtins__["__import__"] - if isinstance(__builtins__, dict) - else __builtins__.__import__ - ) - safe_builtins = ( - {k: v for k, v in __builtins__.items() if k not in _MATH_BLOCKED_BUILTINS} - if isinstance(__builtins__, dict) - else { - k: getattr(__builtins__, k) - for k in dir(__builtins__) - if k not in _MATH_BLOCKED_BUILTINS and not k.startswith("_") - } - ) - safe_builtins["__import__"] = _safe_import - - # Pre-import safe modules - import collections - import decimal - import fractions - import functools - import itertools - import math - import operator - import random - import re - import string - - ns: dict[str, Any] = { - "__builtins__": safe_builtins, - "math": math, - "fractions": fractions, - "Fraction": fractions.Fraction, - "itertools": itertools, - "functools": functools, - "operator": operator, - "collections": collections, - "decimal": decimal, - "Decimal": decimal.Decimal, - "random": random, - "re": re, - "string": string, - } - - try: - import sympy - - ns["sympy"] = sympy - for name in ( - "symbols", - "Symbol", - "solve", - "simplify", - "expand", - "factor", - "Eq", - "sqrt", - "Rational", - "pi", - "E", - "I", - "oo", - "sin", - "cos", - "tan", - "exp", - "log", - "factorial", - "binomial", - "gcd", - "lcm", - "prime", - "isprime", - "factorint", - "divisors", - "totient", - "mod_inverse", - "Matrix", - "integrate", - "diff", - "limit", - "series", - "Sum", - "Product", - "floor", - "ceiling", - "Abs", - ): - ns[name] = getattr(sympy, name) - except ImportError: - pass # optional dependency - - try: - import numpy as _np - - ns["np"] = ns["numpy"] = _np - except ImportError: - pass # optional dependency - - try: - import scipy # type: ignore[import-untyped] - import scipy.integrate # type: ignore[import-untyped] - import scipy.linalg # type: ignore[import-untyped] - import scipy.optimize # type: ignore[import-untyped] - import scipy.special # type: ignore[import-untyped] - - ns["scipy"] = scipy - ns["special"] = scipy.special - ns["optimize"] = scipy.optimize - ns["comb"] = scipy.special.comb - ns["perm"] = scipy.special.perm - ns["gamma"] = scipy.special.gamma - ns["beta"] = scipy.special.beta - except ImportError: - pass # optional dependency - - # Strip __builtins__ from all pre-imported modules so - # module.__builtins__['__import__'] can't bypass _safe_import. - for v in list(ns.values()): - if hasattr(v, "__builtins__"): - with contextlib.suppress(AttributeError, TypeError): - v.__builtins__ = {} - - exec(code, ns) # noqa: S102 - - _sys.stdout = _sys.__stdout__ - printed = captured.getvalue() - result_var = ns.get("result") - if result_var is not None: - out = f"{printed.rstrip()}\nresult = {result_var}" if printed else str(result_var) - elif printed: - out = printed.rstrip() - else: - out = "No output. Add print() to see results." - result_queue.put(("success", out)) - - except Exception as e: - _sys.stdout = _sys.__stdout__ - result_queue.put(("error", f"{type(e).__name__}: {e}\n{traceback.format_exc()}")) - - -def auto_print_wrap(code: str) -> str: - """If code has no print/result and the last statement is an expression, wrap it in print().""" - # Skip if code already has print() or assigns to 'result' - if "print(" in code or re.search(r"\bresult\s*=", code): - return code - try: - tree = ast.parse(code) - except SyntaxError: - return code - if not tree.body: - return code - last = tree.body[-1] - if isinstance(last, ast.Expr): - # Get the source of the last expression and wrap in print() - lines = code.split("\n") - last_line_start = last.lineno - 1 # 0-based - last_line_end = last.end_lineno # 1-based, exclusive after slicing - expr_lines = lines[last_line_start:last_line_end] - expr_text = "\n".join(expr_lines) - prefix = lines[:last_line_start] - wrapped = prefix + [f"print({expr_text})"] - return "\n".join(wrapped) - return code - - -def execute_math_sandboxed(code: str, timeout: float = 30.0) -> tuple[str, bool]: - """Execute Python code in a sandboxed subprocess. Returns (output, is_error).""" - code = auto_print_wrap(code) - errors = validate_math_code(code) - if errors: - return "Validation errors:\n" + "\n".join(f"- {e}" for e in errors), True - - result_queue: multiprocessing.Queue[tuple[str, str]] = multiprocessing.Queue() - proc = multiprocessing.Process(target=_math_exec_in_process, args=(code, result_queue)) - proc.start() - proc.join(timeout=timeout) - - if proc.is_alive(): - proc.terminate() - proc.join(timeout=1.0) - if proc.is_alive(): - proc.kill() - proc.join() - result_queue.close() - result_queue.join_thread() - return f"Execution timed out after {timeout}s", True - - if result_queue.empty(): - result_queue.close() - result_queue.join_thread() - return "Execution failed with no output", True - - status, output = result_queue.get() - result_queue.close() - result_queue.join_thread() - return output, status == "error" diff --git a/turnstone/core/session.py b/turnstone/core/session.py index 2318862e..1b4daed2 100644 --- a/turnstone/core/session.py +++ b/turnstone/core/session.py @@ -98,7 +98,6 @@ from turnstone.core.nudge_queue import TOOL_DRAIN, USER_DRAIN, NudgeQueue from turnstone.core.providers import create_provider from turnstone.core.ratelimit import TokenBucket from turnstone.core.safety import is_command_blocked, sanitize_command -from turnstone.core.sandbox import execute_math_sandboxed from turnstone.core.skill_field_validation import SKILL_RUNTIME_CONFIG_FIELDS from turnstone.core.skill_parser import MAX_SKILL_DESCRIPTION_LEN from turnstone.core.storage._registry import get_storage @@ -106,8 +105,6 @@ from turnstone.core.storage._utils import normalize_search_terms 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, - AGENT_TOOLS, BUILTIN_TOOL_NAMES, COORDINATOR_TOOLS, INTERACTIVE_TOOLS, @@ -783,7 +780,6 @@ class SessionUI(Protocol): ) -> 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: ... def on_info(self, message: str) -> None: ... def on_error(self, message: str) -> None: ... def on_user_reminder( @@ -1132,12 +1128,10 @@ class ChatSession: if kind == WorkstreamKind.COORDINATOR: self._tools = list(COORDINATOR_TOOLS) self._task_tools = [] - self._agent_tools = [] elif mcp_client: mcp_tools = mcp_client.get_tools(user_id=self._mcp_user_id) self._tools = merge_mcp_tools(INTERACTIVE_TOOLS, mcp_tools) self._task_tools = merge_mcp_tools(TASK_AGENT_TOOLS, mcp_tools) - self._agent_tools = merge_mcp_tools(AGENT_TOOLS, mcp_tools) # Register for tool-change notifications from MCP servers. # ``user_id`` is the listener identity component — pool-only # changes for OTHER users must not fire this callback. @@ -1156,9 +1150,8 @@ class ChatSession: else: self._tools = INTERACTIVE_TOOLS self._task_tools = TASK_AGENT_TOOLS - self._agent_tools = AGENT_TOOLS - # Inject the live alias list into plan_agent / task_agent tool - # descriptions so the calling LLM sees its `model` parameter options. + # Inject the live alias list into the task_agent tool + # description so the calling LLM sees its `model` parameter options. # Replaces affected tool dicts with deep copies — module-level # constants are not mutated. self._render_agent_tool_descriptions() @@ -1844,13 +1837,12 @@ class ChatSession: mcp_tools = self._mcp_client.get_tools(user_id=self._mcp_user_id) self._tools = merge_mcp_tools(INTERACTIVE_TOOLS, mcp_tools) self._task_tools = merge_mcp_tools(TASK_AGENT_TOOLS, mcp_tools) - self._agent_tools = merge_mcp_tools(AGENT_TOOLS, mcp_tools) self._render_agent_tool_descriptions() self._rebuild_tool_search() def _render_agent_tool_descriptions(self) -> None: """Inject the live alias list into the ``model`` parameter description - on plan_agent / task_agent tools. + on the task_agent tool. Lets the calling LLM see which aliases are valid right now. Called on session init and on registry reload (via @@ -1860,19 +1852,18 @@ class ChatSession: Replaces affected tool dicts with deep copies so the module-level tool-list constants stay untouched across sessions. - plan_agent and task_agent live in ``self._tools`` (the main session's - tool set) — not in ``self._agent_tools`` / ``self._task_tools``, - which are what *sub-agents* see (sub-agents don't get delegation - tools to avoid infinite recursion). + task_agent lives in ``self._tools`` (the main session's tool set) — + not in ``self._task_tools``, which is what *sub-agents* see + (sub-agents don't get delegation tools to avoid infinite recursion). """ if self._registry is None: return # Hide ``default`` from the alias list — the LLM reads the English # word and picks it explicitly, which routes to whichever model # carries that alias rather than the operator-configured per-role - # default (plan_alias / task_alias). Omitting ``model=`` already - # selects the per-role default; offering the literal name as an - # alternative invites the bypass. + # default (task_alias). Omitting ``model=`` already selects the + # per-role default; offering the literal name as an alternative + # invites the bypass. aliases = sorted(a for a in self._registry.list_aliases() if a != "default") aliases_str = ", ".join(f"`{a}`" for a in aliases) @@ -1880,10 +1871,9 @@ class ChatSession: for tool in self._tools: fn = tool.get("function") or {} name = fn.get("name", "") - if name not in ("plan_agent", "task_agent"): + if name != "task_agent": new_tools.append(tool) continue - kind = "plan model" if name == "plan_agent" else "task model" new_tool = copy.deepcopy(tool) props = new_tool.get("function", {}).get("parameters", {}).get("properties", {}) if "model" in props: @@ -1893,13 +1883,13 @@ class ChatSession: # render, not return early and leave them in place. if aliases: props["model"]["description"] = ( - f"Optional model alias to run this {name} on. " - f"Omit to use the operator-configured {kind}. " + "Optional model alias to run this task_agent on. " + "Omit to use the operator-configured task model. " f"Available aliases: {aliases_str}." ) else: props["model"]["description"] = ( - f"Optional model alias to run this {name} on. " + "Optional model alias to run this task_agent on. " "Omit to use the current session model. " "(No alternative aliases configured in this session.)" ) @@ -1907,8 +1897,8 @@ class ChatSession: self._tools = new_tools def refresh_agent_tool_schemas(self) -> None: - """Public entry point: re-render plan_agent / task_agent tool - descriptions to reflect the current ModelRegistry state, and + """Public entry point: re-render the task_agent tool + description to reflect the current ModelRegistry state, and rebuild the BM25 tool-search index so its text matches. Called by the server after a registry reload (sync-to-nodes / @@ -3156,7 +3146,7 @@ class ChatSession: """Persist token usage for a non-streaming auxiliary completion. Title generation, compaction, web-fetch summarisation, and - plan/task sub-agents all run via ``create_completion`` and bypass + task sub-agents all run via ``create_completion`` and bypass the streaming ``on_status`` accounting path; without this their spend never reaches the usage dashboard. Delegates to the UI's ``on_aux_usage`` hook (which owns the storage write + any node @@ -5147,8 +5137,6 @@ class ChatSession: "prompt": (it.get("prompt") or "")[:200], "skill": skill_dict.get("name", "") if isinstance(skill_dict, dict) else "", } - elif name == "plan_agent": - it["func_args"] = {"goal": (it.get("prompt") or "")[:200]} # Coordinator tool args — only the ``needs_approval=True`` set # reaches this point (read-only inspect / list_* / wait # tools are filtered above), so this matches the auditable @@ -5993,78 +5981,6 @@ class ChatSession: with concurrent.futures.ThreadPoolExecutor(max_workers=4) as pool: results = list(pool.map(run_one, items)) - # Post-plan gate: iterative review loop. When the user gives - # feedback the plan agent re-runs and the revised plan is shown - # again, up to _MAX_PLAN_REFINEMENTS rounds. - for i, item in enumerate(items): - if item.get("func_name") != "plan_agent" or item.get("error") or item.get("denied"): - continue - - cid, output = results[i] - 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: - original_goal = item.get("prompt", "") - - refinement_round = 0 - while True: - self._emit_state("attention") - resp = self.ui.on_plan_review(output) - self._emit_state("running") - - if resp.lower() in ("n", "no", "reject"): - output += ( - "\n\n---\nUser REJECTED this plan. Do not " - "proceed with implementation. Ask the user " - "what they want instead." - ) - break - elif not resp: - break # empty response = approve - elif refinement_round >= self._MAX_PLAN_REFINEMENTS: - self.ui.on_info("[plan] max refinement rounds reached") - break - else: - # Re-run plan agent with user feedback. - # Strip any internal warning prefix so the - # agent sees the raw plan content. - raw = output - _warn = "[Warning: plan may be incomplete or poorly structured]\n\n" - if raw.startswith(_warn): - raw = raw[len(_warn) :] - try: - output = self._refine_plan( - raw, - original_goal, - resp, - ) - refinement_round += 1 - except (KeyboardInterrupt, GenerationCancelled): - output += "\n\n---\n(plan refinement interrupted)" - break - except Exception as e: - self.ui.on_info(f"[plan refinement error] {e}") - output += f"\n\n---\nUser feedback: {resp}" - break - # Loop continues → show revised plan to user - - # Write final version to disk (overwrites initial write) - try: - with open(plan_path, "w") as f: - f.write(output) - except OSError: - 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. - output += f"\n\n---\nPlan saved to `{plan_path}`" - results[i] = (cid, output) - return results, user_feedback @staticmethod @@ -6258,13 +6174,10 @@ class ChatSession: "diff_file": self._prepare_diff, "write_file": self._prepare_write_file, "edit_file": self._prepare_edit_file, - "math": self._prepare_math, - "man": self._prepare_man, "web_fetch": self._prepare_web_fetch, "web_search": self._prepare_web_search, "tool_search": self._prepare_tool_search, "task_agent": self._prepare_task, - "plan_agent": self._prepare_plan, "memory": self._prepare_memory, "recall": self._prepare_recall, "notify": self._prepare_notify, @@ -6820,79 +6733,6 @@ class ChatSession: "replace_all": replace_all, } - def _prepare_math(self, call_id: str, args: dict[str, Any]) -> dict[str, Any]: - code = args.get("code", "") - if isinstance(code, list): - code = "\n".join(code) - if not code: - return { - "call_id": call_id, - "func_name": "math", - "header": "\u2717 math: empty code", - "preview": "", - "needs_approval": False, - "error": "Error: no code provided", - } - # Show code preview - preview = f"{DIM}{textwrap.indent(code, ' ')}{RESET}" - return { - "call_id": call_id, - "func_name": "math", - "header": f"\u2699 math: ({len(code)} chars)", - "preview": preview, - "needs_approval": True, - "approval_label": "math", - "execute": self._exec_math, - "code": code, - } - - def _prepare_man(self, call_id: str, args: dict[str, Any]) -> dict[str, Any]: - """Prepare a man/info page lookup.""" - page = (args.get("page") or "").strip() - if not page: - return { - "call_id": call_id, - "func_name": "man", - "header": "\u2717 man: empty page", - "preview": "", - "needs_approval": False, - "error": "Error: no page name provided", - } - section = (args.get("section") or "").strip() - # Accept the canonical "name(section)" notation that the model often - # emits (e.g. printf(3), open(2), perlfunc(3pm)) \u2014 the parens are - # otherwise rejected by the page-name sanitizer below. An explicit - # ``section`` arg, if provided, takes precedence over the parsed one. - m = re.match(r"^([a-zA-Z0-9._-]+)\(([1-9][a-z]*)\)$", page) - if m: - page = m.group(1) - if not section: - section = m.group(2) - # Sanitize: only allow alphanumeric, dash, underscore, dot - if not re.match(r"^[a-zA-Z0-9._-]+$", page): - return { - "call_id": call_id, - "func_name": "man", - "header": "\u2717 man: invalid page name", - "preview": f" {page}", - "needs_approval": False, - "error": f"Error: invalid page name {page!r}", - } - if section and not re.match(r"^[1-9][a-z]*$", section): - section = "" - label = f"{page}({section})" if section else page - preview = f" {DIM}{label}{RESET}" - return { - "call_id": call_id, - "func_name": "man", - "header": f"\u2699 man: {label}", - "preview": preview, - "needs_approval": False, - "execute": self._exec_man, - "page": page, - "section": section, - } - def _prepare_web_fetch(self, call_id: str, args: dict[str, Any]) -> dict[str, Any]: url = args.get("url", "").strip() question = args.get("question", "").strip() @@ -7040,7 +6880,7 @@ class ChatSession: def _validate_agent_model_override( self, call_id: str, func_name: str, args: dict[str, Any] ) -> tuple[str | None, dict[str, Any] | None]: - """Pull and validate the optional `model` arg for plan/task agents. + """Pull and validate the optional `model` arg for the task agent. Returns (alias, error_item). When the caller passed a `model` and it isn't in the registry, returns an error_item shaped like the @@ -7056,7 +6896,7 @@ class ChatSession: # ``default`` is operator-only — the alias either back-compat-shims # a single-CLI-model registry or aliases a hand-named DB row, and # in both cases an LLM that explicitly routes here bypasses the - # operator-configured ``plan_alias`` / ``task_alias`` per-role + # operator-configured ``task_alias`` per-role # default. Symmetric with the description filter at # ``_render_agent_tool_descriptions`` — closes the loophole where # an LLM that learned the alias name out-of-band (training data, @@ -7184,34 +7024,6 @@ class ChatSession: "skill": skill_data, } - def _prepare_plan(self, call_id: str, args: dict[str, Any]) -> dict[str, Any]: - """Prepare a planning agent for approval.""" - goal = args.get("goal", "").strip() - if not goal: - return { - "call_id": call_id, - "func_name": "plan_agent", - "header": "\u2717 plan_agent: empty goal", - "preview": "", - "needs_approval": False, - "error": "Error: empty goal", - } - model_override, err = self._validate_agent_model_override(call_id, "plan_agent", args) - if err is not None: - return err - preview_text = goal[:300] + ("..." if len(goal) > 300 else "") - return { - "call_id": call_id, - "func_name": "plan_agent", - "header": "\u2699 plan_agent (planning agent)", - "preview": f" {preview_text}", - "needs_approval": True, - "approval_label": "plan_agent", - "execute": self._exec_plan, - "prompt": goal, - "model_override": model_override, - } - def _resolve_scope_id(self, scope: str) -> str: """Map a scope name to its scope_id. @@ -11069,12 +10881,14 @@ class ChatSession: Args: agent_messages: Pre-built message list (system + developer + user). - label: Display prefix for progress lines ("agent" or "plan"). - tools: Tool definitions to send to the API. Defaults to AGENT_TOOLS (read-only). - auto_tools: Set of tool names the agent may execute. Defaults to AGENT_AUTO_TOOLS. + label: Display prefix for progress lines (e.g. "task"). + tools: Tool definitions to send to the API. Defaults to the + session's task tool set. + auto_tools: Set of tool names the agent may execute. Defaults to + TASK_AUTO_TOOLS. reasoning_effort: Override reasoning effort for this agent. agent_alias: Per-call model alias override (the LLM passed - ``model=""`` to plan_agent/task_agent). Wins over + ``model=""`` to task_agent). Wins over the registry's per-kind resolution when set. Caller is expected to have validated the alias against the registry; an unknown alias here raises ``ValueError``. @@ -11083,13 +10897,13 @@ class ChatSession: Final content string from the agent. """ if tools is None: - tools = self._agent_tools + tools = self._task_tools if auto_tools is None: - auto_tools = AGENT_AUTO_TOOLS + auto_tools = TASK_AUTO_TOOLS max_tool_turns = self.agent_max_turns - # Resolve agent model: explicit per-call override wins, then per-kind - # registry override (plan_model/task_model), then the legacy single- + # Resolve agent model: explicit per-call override wins, then the + # per-kind registry override (task_model), then the legacy single- # knob agent_model, then the session's primary model. if agent_alias is not None: if self._registry is None or not self._registry.has_alias(agent_alias): @@ -11109,17 +10923,10 @@ class ChatSession: agent_alias = self._model_alias # Per-kind reasoning effort. Explicit caller arg wins; otherwise - # delegate to the registry which knows the per-kind default (plan - # gets the back-compat "high", task returns None to inherit the - # session). When no registry exists, apply the plan back-compat - # default directly so single-process callers keep prior behaviour. - if reasoning_effort is None: - if self._registry: - reasoning_effort = self._registry.resolve_agent_effort(label) - elif label == "plan": - from turnstone.core.model_registry import ModelRegistry - - reasoning_effort = ModelRegistry.PLAN_DEFAULT_EFFORT + # delegate to the registry which knows the per-kind default (task + # returns None to inherit the session). + if reasoning_effort is None and self._registry: + reasoning_effort = self._registry.resolve_agent_effort(label) # Gate web_search: remove when no backend exists for the agent model agent_caps = self._resolve_capabilities(agent_provider, agent_model, agent_alias) @@ -11141,7 +10948,7 @@ class ChatSession: # NOT wired here. Agent assistant messages are built from # ``CompletionResult.content + tool_calls`` only (no # ``_provider_content`` carried), so the helper would no-op - # every turn anyway. Plan/task agents are excluded from the + # every turn anyway. Task agents are excluded from the # persistence/replay contract — their conversation history # is in-memory and rebuilt per ``_run_agent`` invocation. last_err: Exception | None = None @@ -11162,7 +10969,7 @@ class ChatSession: ), ) # Sub-agent turns bypass on_status — record per-turn so - # plan/task spend is visible in the dashboard, attributed + # task-agent spend is visible in the dashboard, attributed # to the agent's own model. self._record_aux_usage(agent_result, model=agent_model) return agent_result @@ -11229,7 +11036,7 @@ class ChatSession: tool_name = tc_dict["function"]["name"].strip() # Guard 1: block recursive agent calls. - if tool_name in ("task_agent", "plan_agent"): + if tool_name == "task_agent": output = "Error: agents cannot spawn further agents" # Guard 2: tool not in this agent's API tool list. elif tool_name not in tool_names: @@ -11309,7 +11116,7 @@ class ChatSession: "# Task Agent\n\n" "You are an autonomous task agent with full tool access. " "You can use bash, read_file, write_file, edit_file, search, " - "math, web_fetch, and web_search." + "web_fetch, and web_search." ) # Operating guidance always applies — these are sub-agent semantics # (one-shot, tool-use over narration, no follow-up questions) that a @@ -11383,224 +11190,6 @@ class ChatSession: self.ui.on_info(f"[task error] {e}") return call_id, f"Task error: {e}" - _PLAN_IDENTITY = ( - "You are a planning agent. Explore the codebase with read_file and search, " - "then write a plan with these sections: " - "## Goal (1-2 sentences), " - "## Current State (files/line numbers found), " - "## Plan (numbered steps naming exact files and functions), " - "## Risks (edge cases and unknowns). " - "Never guess at structure — verify first. Be specific: name files, line numbers, " - "and functions in every step." - ) - - def _plan_system_content(self) -> str: - """Plan agent system message: skill guardrails + plan identity.""" - if not self._skill_content: - return self._PLAN_IDENTITY - tpl = self._skill_content - if len(tpl) > _MAX_SKILL_CONTENT: - log.warning("skill_content.truncated", length=len(tpl), agent="plan") - tpl = tpl[:_MAX_SKILL_CONTENT] - return tpl + "\n\n" + self._PLAN_IDENTITY - - _MIN_PLAN_LENGTH = 100 - _PLAN_REQUIRED_SECTIONS = ("## goal", "## current state", "## plan", "## risks") - _MIN_PLAN_SECTIONS = 2 - _MAX_PLAN_REFINEMENTS = 5 - - @staticmethod - def _validate_plan(content: str, goal: str) -> tuple[bool, list[str]]: - """Check if plan output meets minimum quality bar. - - Returns ``(valid, issues)`` where *issues* is a list of - human-readable problem descriptions (empty when valid). - """ - issues: list[str] = [] - stripped = content.strip() - stripped_lower = stripped.lower() - - # 1. Minimum length - if len(stripped) < ChatSession._MIN_PLAN_LENGTH: - issues.append( - f"too short ({len(stripped)} chars, minimum {ChatSession._MIN_PLAN_LENGTH})" - ) - - # 2. Section structure - found_sections = sum( - 1 for section in ChatSession._PLAN_REQUIRED_SECTIONS if section in stripped_lower - ) - if found_sections < ChatSession._MIN_PLAN_SECTIONS: - issues.append( - f"missing plan sections (found {found_sections}/" - f"{len(ChatSession._PLAN_REQUIRED_SECTIONS)}, " - f"need at least {ChatSession._MIN_PLAN_SECTIONS})" - ) - - # 3. Echo detection: plan is basically just the goal repeated - goal_stripped = goal.strip().lower() - if ( - goal_stripped - and len(stripped) < len(goal_stripped) * 2 - and goal_stripped in stripped_lower - ): - issues.append("plan appears to echo the goal without elaboration") - - # 4. Refusal detection - refusal_starts = ( - "i cannot", - "i'm sorry", - "i am sorry", - "error:", - "i can't", - ) - if any(stripped_lower.startswith(r) for r in refusal_starts): - issues.append("plan appears to be a refusal or error") - - return (len(issues) == 0, issues) - - def _exec_plan(self, item: dict[str, Any]) -> tuple[str, str]: - """Run a planning agent and write the result to .plan-.md.""" - call_id, prompt = item["call_id"], item["prompt"] - plan_path = f".plan-{self._ws_id}.md" - - # If plan was called before in this session, the previous assistant - # tool_call + tool result are already in self.messages — pass them - # directly to the inner agent so it refines rather than restarts. - prior_plan_msgs: list[dict[str, Any]] = [] - for i, msg in enumerate(self.messages): - if msg.get("role") == "assistant" and msg.get("tool_calls"): - for tc in msg["tool_calls"]: - if tc.get("function", {}).get("name") == "plan_agent": - tc_id = tc["id"] - for j in range(i + 1, len(self.messages)): - if ( - self.messages[j].get("role") == "tool" - and self.messages[j].get("tool_call_id") == tc_id - ): - prior_plan_msgs = [msg, self.messages[j]] - break - - # Plan agent gets template guardrails + its own identity — no tool - # patterns, MCP resources, or general conversation history (only - # prior plan tool_call/result pairs are forwarded for refinement). - agent_messages: list[dict[str, Any]] = [ - {"role": "system", "content": self._plan_system_content()}, - ] - agent_messages.extend(prior_plan_msgs) - agent_messages.append({"role": "user", "content": prompt}) - - plan_alias = item.get("model_override") - try: - content = self._run_agent( - agent_messages, - label="plan", - agent_alias=plan_alias, - ) - except (KeyboardInterrupt, GenerationCancelled): - return call_id, "(plan interrupted by user)" - except Exception as e: - self.ui.on_info(f"[plan error] {e}") - return call_id, f"Plan error: {e}" - - # Validate plan quality — retry once with coaching on failure - valid, issues = self._validate_plan(content, prompt) - if not valid: - self.ui.on_info(f"[plan] quality issues: {', '.join(issues)}") - preview = content[:200] + ("..." if len(content) > 200 else "") - coaching = ( - "Your previous response did not follow the required plan " - "format. A valid plan should include at least two of " - "these markdown sections:\n" - "## Goal (1-2 sentences)\n" - "## Current State (files/line numbers found)\n" - "## Plan (numbered steps with file names and functions)\n" - "## Risks (edge cases and unknowns)\n\n" - f'Your previous response was: "{preview}"\n\n' - "Please try again. Explore the codebase first, then write " - "the plan." - ) - agent_messages.append({"role": "user", "content": coaching}) - try: - content = self._run_agent( - agent_messages, - label="plan", - agent_alias=plan_alias, - ) - except (KeyboardInterrupt, GenerationCancelled): - return call_id, "(plan interrupted by user)" - except Exception as e: - self.ui.on_info(f"[plan retry error] {e}") - return call_id, f"Plan error: {e}" - - valid2, issues2 = self._validate_plan(content, prompt) - if not valid2: - self.ui.on_info(f"[plan] still has issues after retry: {', '.join(issues2)}") - content = "[Warning: plan may be incomplete or poorly structured]\n\n" + content - - # Write to file separately — always return content even if write fails - try: - with open(plan_path, "w") as f: - f.write(content) - self.ui.on_info(f"Plan written to {plan_path}") - except OSError as e: - self.ui.on_info(f"[plan] could not write {plan_path}: {e}") - - return call_id, content - - def _refine_plan( - self, - original_content: str, - original_goal: str, - feedback: str, - ) -> str: - """Re-run the plan agent incorporating user feedback.""" - tc_id = f"plan_refine_{uuid.uuid4().hex[:8]}" - agent_messages: list[dict[str, Any]] = [ - {"role": "system", "content": self._plan_system_content()}, - { - "role": "assistant", - "content": "", - "tool_calls": [ - { - "id": tc_id, - "type": "function", - "function": { - "name": "plan_agent", - "arguments": json.dumps({"goal": original_goal}), - }, - } - ], - }, - { - "role": "tool", - "tool_call_id": tc_id, - "content": original_content, - }, - { - "role": "user", - "content": ( - "The user reviewed this plan and provided feedback:\n\n" - f"{feedback}\n\n" - "Please revise the plan accordingly. Keep the same " - "format (## Goal, ## Current State, ## Plan, ## Risks) " - "and address the feedback." - ), - }, - ] - - self.ui.on_info("[plan] revising based on feedback...") - content = self._run_agent( - agent_messages, - label="plan", - ) - - valid, issues = self._validate_plan(content, original_goal) - if not valid: - self.ui.on_info(f"[plan] revised plan has issues: {', '.join(issues)}") - - return content - def _audit_memory_event( self, action: str, @@ -12428,74 +12017,6 @@ class ChatSession: 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]: - """Execute Python code in sandboxed subprocess.""" - call_id, code = item["call_id"], item["code"] - output, is_error = execute_math_sandboxed(code, timeout=self.tool_timeout) - output = self._truncate_output(output) - - result_msg = f"Error:\n{output}" if is_error else output if output else "(no output)" - 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]: - """Look up a man or info page.""" - self._check_cancelled() - call_id = item["call_id"] - page = item["page"] - section = item.get("section", "") - - # Try man first, fall back to info - cmd = ["man"] - if section: - cmd.append(section) - cmd.append(page) - - text = "" - try: - from turnstone.core.env import scrubbed_env - - result = subprocess.run( - cmd, - capture_output=True, - text=True, - timeout=10, - env=scrubbed_env(extra={"MANWIDTH": "80", "MAN_KEEP_FORMATTING": "0"}), - ) - if result.returncode == 0 and result.stdout.strip(): - # Strip formatting: backspace overstrikes and ANSI escapes - text = re.sub(r".\x08", "", result.stdout) - text = re.sub(r"\x1b\[[0-9;]*m", "", text) - else: - # Fall back to info - result = subprocess.run( - ["info", page], - capture_output=True, - text=True, - timeout=10, - env=scrubbed_env(), - ) - if result.returncode == 0 and result.stdout.strip(): - text = result.stdout - else: - msg = f"No man or info page found for '{page}'" - self._report_tool_result(call_id, "man", msg) - return call_id, msg - except FileNotFoundError: - msg = "Error: man command not available" - 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._report_tool_result(call_id, "man", msg, is_error=True) - return call_id, msg - - text = self._truncate_output(text) - - self._report_tool_result(call_id, "man", f"{len(text)} chars") - - return call_id, text - def _exec_web_fetch(self, item: dict[str, Any]) -> tuple[str, str]: """Fetch a URL, then summarize/extract using an API call.""" self._check_cancelled() diff --git a/turnstone/core/session_manager.py b/turnstone/core/session_manager.py index 042c047a..14c77a62 100644 --- a/turnstone/core/session_manager.py +++ b/turnstone/core/session_manager.py @@ -53,7 +53,7 @@ class SessionKindAdapter(Protocol): - **Session construction**: what UI class wraps the workstream, what ``ChatSession`` factory signature applies. - - **UI cleanup**: unblocking pending approval / plan / foreground + - **UI cleanup**: unblocking pending approval / foreground events when a workstream closes. Lifecycle event fan-out (``ws_created`` / ``ws_state`` / @@ -906,7 +906,7 @@ class SessionManager: self._state_subscribers.remove(callback) def cancel(self, ws_id: str) -> bool: - """Cancel in-flight generation and unblock any pending approval / plan. + """Cancel in-flight generation and unblock any pending approval. Does NOT unload the workstream — use ``close`` for that. The session stays live and can receive further messages. Returns @@ -920,13 +920,9 @@ class SessionManager: ws.session.cancel() except Exception: log.debug("session_mgr.cancel_failed ws=%s", ws_id[:8], exc_info=True) - if ws.ui is not None: - if hasattr(ws.ui, "resolve_approval"): - with contextlib.suppress(Exception): - ws.ui.resolve_approval(False, "cancelled") - if hasattr(ws.ui, "resolve_plan"): - with contextlib.suppress(Exception): - ws.ui.resolve_plan("reject") + if ws.ui is not None and hasattr(ws.ui, "resolve_approval"): + with contextlib.suppress(Exception): + ws.ui.resolve_approval(False, "cancelled") return True def close_idle(self, max_age_seconds: float) -> list[str]: diff --git a/turnstone/core/session_routes.py b/turnstone/core/session_routes.py index 068c9cf4..d6a8398a 100644 --- a/turnstone/core/session_routes.py +++ b/turnstone/core/session_routes.py @@ -153,15 +153,14 @@ class EventsReplay(Protocol): live event loop starts. Each yielded dict gets JSON-serialised and sent as a single ``data:`` line to the client. - Interactive yields five things on connect: ``connected`` (model + + Interactive yields four things on connect: ``connected`` (model + skip_permissions), ``status`` (token usage + context %, only when ``session._last_usage`` exists), ``history`` (replayed conversation), - ``pending_approval`` + cached intent verdicts, and - ``pending_plan_review``. Coord yields just two: ``pending_approval`` - and ``pending_plan_review`` (the rest aren't needed because coord's - dashboard fetches history via a separate ``/history`` endpoint - and doesn't render the per-tab status bar). Kinds that don't - need any pre-replay wire ``None`` and the live loop starts + and ``pending_approval`` + cached intent verdicts. Coord yields + just one: ``pending_approval`` (the rest aren't needed because + coord's dashboard fetches history via a separate ``/history`` + endpoint and doesn't render the per-tab status bar). Kinds that + don't need any pre-replay wire ``None`` and the live loop starts immediately. """ @@ -389,9 +388,8 @@ class SessionEndpointConfig: # SSE replay payload the lifted ``events`` body yields after # registering the per-UI listener queue but before the live # event loop. Interactive replays connected + status + history - # + pending_approval (with cached intent verdicts) + - # pending_plan_review. Coord replays just pending_approval + - # pending_plan_review (its dashboard fetches history via a + # + pending_approval (with cached intent verdicts). Coord replays + # just pending_approval (its dashboard fetches history via a # separate ``/history`` endpoint and doesn't render the per-tab # status bar). Kinds that don't need pre-replay wire ``None``. events_replay: EventsReplay | None = None @@ -978,12 +976,12 @@ def make_cancel_handler( """Lifted body for ``POST {prefix}/{ws_id}/cancel``. Cancels in-flight generation on a workstream. Sets the cooperative - cancel flag on the session, unblocks any pending approval / plan - waits, and (when the request body asks for it) force-abandons a - stuck worker thread so the UI recovers immediately. + cancel flag on the session, unblocks any pending approval, and + (when the request body asks for it) force-abandons a stuck worker + thread so the UI recovers immediately. Both kinds share the cancel sequence (``session.cancel`` → - ``ui.resolve_approval(False)`` → ``ui.resolve_plan("reject")``). + ``ui.resolve_approval(False)``). Per-kind divergence captured via the cfg + ``audit_emit``: - ``cancel_forensics`` (cfg) — when set, the lifted body calls @@ -1020,14 +1018,7 @@ def make_cancel_handler( ``coord_mgr.cancel`` which silently no-op'd on a placeholder; the lifted body 400s for parity with interactive's existing "No session" branch. - - **Interactive ``resolve_plan`` now runs on every cancel** (was - gated on ``was_running``). Lifts coord's always-resolve - behaviour onto interactive — a stuck plan-pending state from - a crashed worker thread can now be cleared via ``cancel``, - matching coord's pre-lift recovery path. ``resolve_plan`` has - its own internal ``_pending_plan_review is None`` guard, so - the call is genuinely no-op when nothing is blocked. - ``resolve_approval`` is **gated on ``ui._pending_approval is not None``** + - ``resolve_approval`` is **gated on ``ui._pending_approval is not None``** because :meth:`SessionUIBase.resolve_approval` is *not* idempotent — it always broadcasts ``approval_resolved`` and overwrites ``_approval_result``. Without the gate, every idle @@ -1091,18 +1082,16 @@ def make_cancel_handler( dropped = {} # Always set the cooperative cancel flag — cheap, no harm if - # nothing's running. resolve_approval / resolve_plan are - # gated by their respective ``_pending_*`` slots: pre-lift - # coord called them unconditionally via ``mgr.cancel`` (which - # is recovery-friendly: a stuck approval-pending state from a + # nothing's running. resolve_approval is gated on its + # ``_pending_approval`` slot: pre-lift coord called it + # unconditionally via ``mgr.cancel`` (which is + # recovery-friendly: a stuck approval-pending state from a # crashed worker can still be cleared), but ``resolve_approval`` # is NOT idempotent — calling it with no pending approval # broadcasts a stale ``approval_resolved`` SSE event and # overwrites ``_approval_result``. Gating on the pending slot # preserves the recovery semantics for the actual stuck case - # while skipping the broadcast on idle cancels. ``resolve_plan`` - # has its own internal no-pending guard, so the call is - # already safe to make unconditionally. + # while skipping the broadcast on idle cancels. try: session.cancel() except Exception: @@ -1116,15 +1105,6 @@ def make_cancel_handler( ws_id[:8], exc_info=True, ) - if hasattr(ui, "resolve_plan"): - try: - ui.resolve_plan("reject") - except Exception: - log.debug( - "ws.cancel.resolve_plan_failed ws=%s", - ws_id[:8], - exc_info=True, - ) # The remaining steps only matter when a worker is actually # running: force-recovery has nothing to recover otherwise, diff --git a/turnstone/core/session_ui_base.py b/turnstone/core/session_ui_base.py index 9d0172c8..f5aaf7e2 100644 --- a/turnstone/core/session_ui_base.py +++ b/turnstone/core/session_ui_base.py @@ -4,8 +4,8 @@ Both :class:`turnstone.server.WebUI` (interactive node UI) and :class:`turnstone.console.coordinator_ui.ConsoleCoordinatorUI` wrap a :class:`~turnstone.core.session.ChatSession` and fan events out over SSE to one or more connected browser tabs. They also block the worker -thread on two pending-input gates (tool approval, plan review) that -HTTP handlers resolve. +thread on a pending-input gate (tool approval) that HTTP handlers +resolve. That skeleton — plus per-workstream metrics tracking, intent-verdict bookkeeping, output-warning persistence, and the canonical @@ -195,14 +195,14 @@ class AutoApproveReason: class SessionUIBase: - """SSE listener fan-out + approval/plan event machinery. + """SSE listener fan-out + approval event machinery. Thread-safety: the ChatSession worker thread calls the ``on_*`` - methods (and the approval/plan blocking helpers that live on + methods (and the approval blocking helpers that live on subclasses); HTTP handlers drive ``_register_listener`` / - ``_unregister_listener`` / ``resolve_approval`` / ``resolve_plan`` - from the event loop. All shared state is guarded by - ``_listeners_lock`` or ``threading.Event`` primitives. + ``_unregister_listener`` / ``resolve_approval`` from the event + loop. All shared state is guarded by ``_listeners_lock`` or + ``threading.Event`` primitives. """ def __init__(self, ws_id: str = "", user_id: str = "") -> None: @@ -249,9 +249,6 @@ class SessionUIBase: # Pending approval shape — re-sent on SSE reconnect so a user # switching tabs still sees the prompt. self._pending_approval: dict[str, Any] | None = None - self._plan_event = threading.Event() - self._plan_result: str = "" - self._pending_plan_review: dict[str, Any] | None = None self.auto_approve = False self.auto_approve_tools: set[str] = set() # Smart Approvals (``judge.smart_approvals``): when enabled, a @@ -658,7 +655,7 @@ class SessionUIBase: return latest_id > cursor # ------------------------------------------------------------------ - # Approval / plan blocking gates + # Approval blocking gates # ------------------------------------------------------------------ def _reset_approval_cycle(self) -> None: @@ -756,26 +753,6 @@ class SessionUIBase: except Exception: log.debug("Failed to update verdict user_decision", exc_info=True) - def resolve_plan(self, feedback: str) -> None: - """Unblock a pending plan review with the caller's verdict. - - ``cancel_generation`` calls this unconditionally to unblock - any wait, so the path has to be safe when no plan is pending - (just signal and skip the broadcast). - """ - self._plan_result = feedback - if self._pending_plan_review is None: - self._plan_event.set() - return - # Clear pending BEFORE broadcasting so a client reconnecting - # in the window between enqueue and clear cannot receive both - # the replayed plan_review (SSE re-injection at the connect - # handler) AND the live plan_resolved. Mirrors the - # approval_resolved pattern above. - self._pending_plan_review = None - self._enqueue({"type": "plan_resolved", "feedback": feedback}) - self._plan_event.set() - def approve_tools(self, items: list[dict[str, Any]]) -> tuple[bool, str | None]: """Two-phase approval gate for a batch of tool calls. @@ -1308,10 +1285,10 @@ class SessionUIBase: _LLM_VERDICT_CACHE_MAX = 50 # Hard cap on how long a worker thread blocks waiting for an - # approval / plan-review decision. Subclasses' ``approve_tools`` and - # ``on_plan_review`` reference this rather than the literal so a - # future ``settings.approval_timeout_seconds`` knob can swap it in - # one place. + # approval decision. Subclasses' ``approve_tools`` references this + # rather than the literal so a future + # ``settings.approval_timeout_seconds`` knob can swap it in one + # place. _APPROVAL_WAIT_TIMEOUT = 3600 def on_intent_verdict(self, verdict: dict[str, Any]) -> None: @@ -2313,7 +2290,7 @@ class SessionUIBase: """Persist a ``usage_event`` for an auxiliary (non-main-loop) LLM call. Title generation, conversation compaction, web-fetch - summarisation, and plan/task sub-agents all run through the + summarisation, and task sub-agents all run through the provider's non-streaming ``create_completion`` and never reach :meth:`on_status` — so without this their tokens are invisible to the governance usage dashboard, undercounting real consumption diff --git a/turnstone/core/settings_registry.py b/turnstone/core/settings_registry.py index dddd5711..ecb703bd 100644 --- a/turnstone/core/settings_registry.py +++ b/turnstone/core/settings_registry.py @@ -83,17 +83,6 @@ def _build_registry() -> dict[str, SettingDef]: "Higher effort improves quality on complex tasks but is slower and uses more " "tokens. Per-model overrides can be set in the Models tab.", ), - SettingDef( - "model.plan_alias", - "str", - "", - "Model alias for plan_agent (empty = inherit from config / session)", - "model", - help="Which model the plan_agent sub-agent uses. When empty, falls back to " - "[model].plan_model in config.toml, then [model].agent_model, then the session " - "model. Plan_agent runs rarely but benefits from a stronger model for " - "high-quality plans \u2014 point this at your strongest reasoner.", - ), SettingDef( "model.task_alias", "str", @@ -103,20 +92,7 @@ def _build_registry() -> dict[str, SettingDef]: help="Which model the task_agent sub-agent uses. When empty, falls back to " "[model].task_model in config.toml, then [model].agent_model, then the session " "model. Task_agent fires frequently for autonomous subtasks \u2014 point this " - "at a cheaper/faster model than your plan_agent.", - ), - SettingDef( - "model.plan_effort", - "str", - "", - "Reasoning effort for plan_agent (empty = inherit from config; default \u2018high\u2019)", - "model", - choices=["", "none", "minimal", "low", "medium", "high", "xhigh", "max"], - help="Reasoning effort for plan_agent specifically. When empty, falls back to " - "[model].plan_effort in config.toml, then to the built-in default \u2018high\u2019. " - "Use \u2018xhigh\u2019 or \u2018max\u2019 with models that support deeper reasoning " - "for higher-quality plans. (Empty here means \u201cinherit\u201d \u2014 use " - "\u2018none\u2019 to actually disable reasoning.)", + "at a cheaper/faster model than your conversation model.", ), SettingDef( "model.task_effort", @@ -195,12 +171,12 @@ def _build_registry() -> dict[str, SettingDef]: "tools.agent_max_turns", "int", -1, - "Max turns for plan/task agents (-1 = unlimited)", + "Max turns for the task agent (-1 = unlimited)", "tools", min_value=-1, max_value=200, - help="Limits how many back-and-forth steps a sub-agent can take when executing a plan " - "or task. Prevents runaway agents from consuming excessive tokens.", + help="Limits how many back-and-forth steps the task sub-agent can take when executing " + "a task. Prevents runaway agents from consuming excessive tokens.", ), SettingDef( "tools.skip_permissions", diff --git a/turnstone/core/tools.py b/turnstone/core/tools.py index 89364501..7c6d12b8 100644 --- a/turnstone/core/tools.py +++ b/turnstone/core/tools.py @@ -9,7 +9,6 @@ from typing import Any _TOOLS_DIR = Path(__file__).resolve().parent.parent / "tools" _META_KEYS = { - "agent", "task_agent", "coordinator", "interactive", @@ -38,7 +37,7 @@ def _load_tools() -> tuple[list[dict[str, Any]], dict[str, Any]]: Returns (tool_defs, metadata) where: - tool_defs: list of OpenAI function-calling dicts - - metadata: dict mapping tool_name -> {agent, task_agent, coordinator, + - metadata: dict mapping tool_name -> {task_agent, coordinator, interactive, auto_approve, primary_key, kind_variants} """ tools = [] @@ -89,7 +88,6 @@ def _apply_kind_variant(tool: dict[str, Any], kind: str, meta: dict[str, Any]) - TOOLS, _META = _load_tools() -AGENT_TOOLS = [t for t in TOOLS if _META[t["function"]["name"]].get("agent")] TASK_AGENT_TOOLS = [t for t in TOOLS if _META[t["function"]["name"]].get("task_agent")] # COORDINATOR_TOOLS apply the ``coordinator`` kind variant (if any) so a # coord session sees the coord-tailored description + parameter schema. @@ -121,7 +119,6 @@ INTERACTIVE_TOOLS = [ ) ] INTERACTIVE_TOOL_NAMES = frozenset(t["function"]["name"] for t in INTERACTIVE_TOOLS) -AGENT_AUTO_TOOLS = {n for n, m in _META.items() if m.get("auto_approve")} TASK_AUTO_TOOLS = {n for n, m in _META.items() if m.get("auto_approve")} PRIMARY_KEY_MAP = {n: m["primary_key"] for n, m in _META.items() if "primary_key" in m} BUILTIN_TOOL_NAMES = frozenset(_META) diff --git a/turnstone/eval.py b/turnstone/eval.py index fbedfeb0..6a71a3a9 100644 --- a/turnstone/eval.py +++ b/turnstone/eval.py @@ -136,9 +136,6 @@ class NullUI: def on_status(self, usage: dict[str, Any], context_window: int, effort: str) -> None: pass - def on_plan_review(self, content: str) -> str: - return "" - def on_info(self, message: str) -> None: pass @@ -1208,10 +1205,9 @@ against expected tool call sequences. Your job: identify SEMANTIC PATTERNS across failures and successes — \ not just what failed, but WHY, and what the failures have in common. -You have access to `math` (Python with numpy/scipy/collections) and \ -`bash` tools. Use them to compute statistics, build confusion matrices, \ -analyze tool co-occurrence, or quantify patterns — don't just eyeball \ -the data. +You have access to a `bash` tool. Use it to compute statistics, build \ +confusion matrices, analyze tool co-occurrence, or quantify patterns — \ +don't just eyeball the data. ## Analysis Framework @@ -1278,8 +1274,8 @@ right and wrong. Your job: edit the prompt so more tests pass. Style — the prompt teaches through patterns and examples, not rules: -- Good: "Plan a refactor → plan_agent:\\n plan_agent(goal='...')" -- Bad: "You MUST call plan_agent. NEVER skip it. ALWAYS use it." +- Good: "Delegate a subtask → task_agent:\\n task_agent(prompt='...')" +- Bad: "You MUST call task_agent. NEVER skip it. ALWAYS use it." - If the current prompt contains imperative rules (MUST, NEVER, \ ALWAYS, ABSOLUTE RULE, etc.), replace them with a pattern that \ demonstrates the right behavior. Rules are noise — examples teach. @@ -1727,24 +1723,6 @@ def _build_failure_analysis( _ANALYST_TOOLS = [ - { - "type": "function", - "function": { - "name": "math", - "description": ( - "Execute Python code for analysis. Available: numpy, scipy, " - "collections, itertools, math, json, re. Use print() for output. " - "Example: print(numpy.mean([0.8, 0.6, 1.0]))" - ), - "parameters": { - "type": "object", - "properties": { - "code": {"type": "string", "description": "Python code to execute."}, - }, - "required": ["code"], - }, - }, - }, { "type": "function", "function": { @@ -1769,12 +1747,7 @@ def _exec_analyst_tool(name: str, arguments: str) -> str: except json.JSONDecodeError: return f"Invalid JSON arguments: {arguments[:200]}" - if name == "math": - from turnstone.core.sandbox import execute_math_sandboxed - - output, is_error = execute_math_sandboxed(args.get("code", ""), timeout=15.0) - return output[:4000] - elif name == "bash": + if name == "bash": import subprocess try: @@ -1802,7 +1775,7 @@ def _run_analyst( ) -> str: """Run the analyst agent to produce a semantic failure analysis. - Multi-turn agent with math and bash tools for computing statistics. + Multi-turn agent with a bash tool for computing statistics. Phase 1 of the two-phase optimization: analyst diagnoses patterns, then optimizer uses the diagnosis to modify the prompt. """ @@ -1878,7 +1851,7 @@ def _run_analyst( user_content += ( "\n\nAnalyze the semantic patterns across these results. " "Focus on WHY failures happen and what passing cases have in common. " - "Use the math or bash tools if you need to compute statistics, " + "Use the bash tool if you need to compute statistics, " "build confusion matrices, or analyze distributions." ) diff --git a/turnstone/prompts/tools.md b/turnstone/prompts/tools.md index 8990cbf6..7a62db34 100644 --- a/turnstone/prompts/tools.md +++ b/turnstone/prompts/tools.md @@ -19,11 +19,6 @@ Find something across files → search: Find and modify → search then read_file then edit_file: search(query='MAX_RETRIES') → read_file(path='found.py') → edit_file(path='found.py') -Plan, design, or architect something → explore codebase then plan_agent: - bash(command='ls') → read_file(path='app.py') → plan_agent(goal='add caching to the application') - plan_agent(goal='refactor database layer from monolith to service') - plan_agent(goal='restructure auth module') - Run a command, git, or tests → bash: bash(command='git log -5') bash(command='pytest') @@ -34,6 +29,3 @@ Retrieve a URL → web_fetch: Search the web for information → web_search: web_search(query='current population of Tokyo') -Look up command flags or documentation → man: - man(page='tar') - man(page='grep') \ No newline at end of file diff --git a/turnstone/sdk/__init__.py b/turnstone/sdk/__init__.py index 8d1d82e8..8c4782c1 100644 --- a/turnstone/sdk/__init__.py +++ b/turnstone/sdk/__init__.py @@ -30,7 +30,6 @@ from turnstone.sdk.events import ( InfoEvent, NodeJoinedEvent, NodeLostEvent, - PlanReviewEvent, ReasoningEvent, ServerEvent, StatusEvent, @@ -73,7 +72,6 @@ __all__ = [ "ToolResultEvent", "ToolOutputChunkEvent", "StatusEvent", - "PlanReviewEvent", "InfoEvent", "ErrorEvent", "BusyErrorEvent", diff --git a/turnstone/sdk/console.py b/turnstone/sdk/console.py index 07bcf0f7..cb58980c 100644 --- a/turnstone/sdk/console.py +++ b/turnstone/sdk/console.py @@ -426,12 +426,6 @@ class AsyncTurnstoneConsole(_BaseClient): "POST", f"/v1/api/route/workstreams/{ws_id}/approve", json_body=body ) - async def route_plan_feedback(self, *, ws_id: str, feedback: str) -> dict[str, Any]: - """Send plan feedback via the routing proxy.""" - return await self._request( - "POST", "/v1/api/route/plan", json_body={"ws_id": ws_id, "feedback": feedback} - ) - async def route_close(self, ws_id: str) -> dict[str, Any]: """Close a workstream via the routing proxy.""" return await self._request("POST", f"/v1/api/route/workstreams/{ws_id}/close", json_body={}) @@ -1360,9 +1354,6 @@ class TurnstoneConsole: ) ) - def route_plan_feedback(self, *, ws_id: str, feedback: str) -> dict[str, Any]: - return self._runner.run(self._async.route_plan_feedback(ws_id=ws_id, feedback=feedback)) - def route_close(self, ws_id: str) -> dict[str, Any]: return self._runner.run(self._async.route_close(ws_id)) diff --git a/turnstone/sdk/events.py b/turnstone/sdk/events.py index 2ccdb4d1..2b309e3b 100644 --- a/turnstone/sdk/events.py +++ b/turnstone/sdk/events.py @@ -193,18 +193,6 @@ class StatusEvent(ServerEvent): turn_count: int = 0 -@dataclass -class PlanReviewEvent(ServerEvent): - type: str = "plan_review" - content: str = "" - - -@dataclass -class PlanResolvedEvent(ServerEvent): - type: str = "plan_resolved" - feedback: str = "" - - @dataclass class InfoEvent(ServerEvent): type: str = "info" @@ -452,8 +440,6 @@ _SERVER_REGISTRY: dict[str, type[ServerEvent]] = { ToolResultEvent, ToolOutputChunkEvent, StatusEvent, - PlanReviewEvent, - PlanResolvedEvent, InfoEvent, ErrorEvent, BusyErrorEvent, diff --git a/turnstone/sdk/server.py b/turnstone/sdk/server.py index 17056677..216b6c73 100644 --- a/turnstone/sdk/server.py +++ b/turnstone/sdk/server.py @@ -283,14 +283,6 @@ class AsyncTurnstoneServer(_BaseClient): response_model=StatusResponse, ) - async def plan_feedback(self, *, ws_id: str, feedback: str = "") -> StatusResponse: - return await self._request( - "POST", - "/v1/api/plan", - json_body={"ws_id": ws_id, "feedback": feedback}, - response_model=StatusResponse, - ) - async def command(self, *, ws_id: str, command: str) -> StatusResponse: return await self._request( "POST", @@ -689,9 +681,6 @@ class TurnstoneServer: self._async.approve(ws_id=ws_id, approved=approved, feedback=feedback, always=always) ) - def plan_feedback(self, *, ws_id: str, feedback: str = "") -> StatusResponse: - return self._runner.run(self._async.plan_feedback(ws_id=ws_id, feedback=feedback)) - def command(self, *, ws_id: str, command: str) -> StatusResponse: return self._runner.run(self._async.command(ws_id=ws_id, command=command)) diff --git a/turnstone/server.py b/turnstone/server.py index 8c5621f1..27599e1d 100644 --- a/turnstone/server.py +++ b/turnstone/server.py @@ -371,16 +371,6 @@ class WebUI(SessionUIBase): ) super().on_aux_usage(usage) - def on_plan_review(self, content: str) -> str: - self._plan_event.clear() - self._pending_plan_review = {"type": "plan_review", "content": content} - self._enqueue(self._pending_plan_review) - if not self._plan_event.wait(timeout=self._APPROVAL_WAIT_TIMEOUT): - log.warning("Plan review timed out for ws_id=%s", self.ws_id) - self._plan_result = "" - self._pending_plan_review = None - return self._plan_result - def on_error(self, message: str) -> None: """Layer node-only Prometheus error counter on top of the shared body.""" _metrics.record_error() @@ -417,10 +407,9 @@ class WebUI(SessionUIBase): # ``on_output_warning`` inherited from :class:`SessionUIBase`. - # ``resolve_approval`` / ``resolve_plan`` inherited from - # :class:`SessionUIBase`. Intent-verdict decision propagation lives - # in the base now — both interactive and coord share the same - # bookkeeping. + # ``resolve_approval`` inherited from :class:`SessionUIBase`. + # Intent-verdict decision propagation lives in the base now — both + # interactive and coord share the same bookkeeping. # --------------------------------------------------------------------------- @@ -727,9 +716,8 @@ def _interactive_events_replay( Yields a ``connected`` event (model + skip_permissions), a ``status`` event with the workstream's last token usage + context % - (when a turn has completed), the pending approval prompt + cached - intent verdicts (if a prompt is pending), and the pending - plan-review (if a review is pending). The lifted + (when a turn has completed), and the pending approval prompt + cached + intent verdicts (if a prompt is pending). The lifted ``make_events_handler`` body delegates that yield sequence to this callback so the kind-specific shape stays in this module. @@ -762,11 +750,6 @@ def _interactive_events_replay( for v in cached_verdicts: yield {"type": "intent_verdict", **v} - # Pending plan-review re-injection. - pending_plan = getattr(ui, "_pending_plan_review", None) - if pending_plan is not None: - yield pending_plan - def _interactive_open_post_load(request: Request, ws: Workstream) -> None: """Post-load hook for the lifted interactive ``open`` body. @@ -1465,26 +1448,6 @@ async def metrics_endpoint(request: Request) -> Response: return Response(content, media_type="text/plain; version=0.0.4; charset=utf-8") -async def plan_feedback(request: Request) -> JSONResponse: - """POST /v1/api/plan — respond to a plan review.""" - from turnstone.core.web_helpers import read_json_or_400 - - body = await read_json_or_400(request) - if isinstance(body, JSONResponse): - return body - feedback = body.get("feedback", "") - ws_id = body.get("ws_id") - mgr = request.app.state.workstreams - _owner, err = _require_ws_access(request, str(ws_id or ""), mgr=mgr) - if err: - return err - ws, ui = _get_ws(mgr, ws_id) - if not ws or not ui: - return JSONResponse({"error": "Unknown workstream"}, status_code=404) - ui.resolve_plan(feedback) - return JSONResponse({"status": "ok"}) - - def _capture_cancel_forensics(session: Any, ui: Any, *, was_running: bool) -> dict[str, Any]: """Snapshot in-flight session state for the cancel response. @@ -2822,7 +2785,7 @@ def config_reload(request: Request) -> JSONResponse: return JSONResponse({"status": "noop"}) cs.reload() # Apply routing overrides to the live registry — admin settings updates - # fan out via this endpoint and would otherwise not affect plan/task + # fan out via this endpoint and would otherwise not affect task # routing until a model-reload or restart. registry = getattr(request.app.state, "registry", None) if registry is not None: @@ -3006,13 +2969,11 @@ def _effective_routing( cs: Any, base_models: dict[str, Any], base_default: str, - base_plan_model: str | None, base_task_model: str | None, - base_plan_effort: str | None, base_task_effort: str | None, -) -> tuple[str, str | None, str | None, str | None, str | None]: - """Compute (default, plan_model, task_model, plan_effort, task_effort) - after layering ConfigStore overrides on top of the supplied base values. +) -> tuple[str, str | None, str | None]: + """Compute (default, task_model, task_effort) after layering ConfigStore + overrides on top of the supplied base values. Aliases require existence in *base_models* (silently dropped otherwise); effort values were validated against SettingDef choices on write, so a @@ -3021,32 +2982,24 @@ def _effective_routing( Returns the base values unchanged when *cs* is None. """ eff_default = base_default - eff_plan_model = base_plan_model eff_task_model = base_task_model - eff_plan_effort = base_plan_effort eff_task_effort = base_task_effort if cs is not None: cs_default = cs.get("model.default_alias") if cs_default and cs_default in base_models: eff_default = cs_default - cs_plan_alias = cs.get("model.plan_alias") - if cs_plan_alias and cs_plan_alias in base_models: - eff_plan_model = cs_plan_alias cs_task_alias = cs.get("model.task_alias") if cs_task_alias and cs_task_alias in base_models: eff_task_model = cs_task_alias - cs_plan_effort = cs.get("model.plan_effort") - if cs_plan_effort: - eff_plan_effort = cs_plan_effort cs_task_effort = cs.get("model.task_effort") if cs_task_effort: eff_task_effort = cs_task_effort - return eff_default, eff_plan_model, eff_task_model, eff_plan_effort, eff_task_effort + return eff_default, eff_task_model, eff_task_effort def _broadcast_agent_tool_schema_refresh(app_state: Any) -> None: - """Tell every active session on this node to re-render its plan_agent / - task_agent tool descriptions. Best-effort: a session that lacks the + """Tell every active session on this node to re-render its task_agent + tool description. Best-effort: a session that lacks the method (older code path or test stub) is skipped silently. Called after a registry reload that may have added/removed model @@ -3084,27 +3037,21 @@ def _apply_routing_overrides(registry: Any, cs: Any) -> bool: cs, registry.models, registry.default, - registry.plan_model, registry.task_model, - registry.plan_effort, registry.task_effort, ) if ( eff[0] != registry.default - or eff[1] != registry.plan_model - or eff[2] != registry.task_model - or eff[3] != registry.plan_effort - or eff[4] != registry.task_effort + or eff[1] != registry.task_model + or eff[2] != registry.task_effort ): registry.reload( registry.models, eff[0], registry.fallback, registry.agent_model, - plan_model=eff[1], - task_model=eff[2], - plan_effort=eff[3], - task_effort=eff[4], + task_model=eff[1], + task_effort=eff[2], ) return True return False @@ -3132,16 +3079,12 @@ def internal_model_reload(request: Request) -> JSONResponse: cs = getattr(request.app.state, "config_store", None) if cs is not None: cs.reload() # Ensure latest settings from DB - eff_default, eff_plan_model, eff_task_model, eff_plan_effort, eff_task_effort = ( - _effective_routing( - cs, - new_registry.models, - new_registry.default, - new_registry.plan_model, - new_registry.task_model, - new_registry.plan_effort, - new_registry.task_effort, - ) + eff_default, eff_task_model, eff_task_effort = _effective_routing( + cs, + new_registry.models, + new_registry.default, + new_registry.task_model, + new_registry.task_effort, ) if eff_default != new_registry.default: log.info( @@ -3157,9 +3100,7 @@ def internal_model_reload(request: Request) -> JSONResponse: and new_registry.fallback == registry.fallback and new_registry.agent_model == registry.agent_model and eff_default == registry.default - and eff_plan_model == registry.plan_model and eff_task_model == registry.task_model - and eff_plan_effort == registry.plan_effort and eff_task_effort == registry.task_effort ) if unchanged: @@ -3172,9 +3113,7 @@ def internal_model_reload(request: Request) -> JSONResponse: eff_default, new_registry.fallback, new_registry.agent_model, - plan_model=eff_plan_model, task_model=eff_task_model, - plan_effort=eff_plan_effort, task_effort=eff_task_effort, ) except ValueError as exc: @@ -3189,8 +3128,8 @@ def internal_model_reload(request: Request) -> JSONResponse: cfg = registry.get_config(alias) health_reg.get_tracker(provider=cfg.provider, base_url=cfg.base_url) - # Push the new alias list into active sessions so plan_agent/task_agent - # `model` parameter descriptions reflect the current registry. + # Push the new alias list into active sessions so the task_agent + # `model` parameter description reflects the current registry. _broadcast_agent_tool_schema_refresh(request.app.state) # Refresh the per-node ``models`` metadata entry the coord reads on @@ -4002,7 +3941,6 @@ def create_app( *v1_routes, Route("/api/skills", list_skills_summary), Route("/api/models", list_available_models), - Route("/api/plan", plan_feedback, methods=["POST"]), Route("/api/command", command, methods=["POST"]), Route("/api/watches", list_watches), Route("/api/watches/{watch_id}/cancel", cancel_watch, methods=["POST"]), diff --git a/turnstone/tools/diff_file.json b/turnstone/tools/diff_file.json index 250eba9b..cb7adba6 100644 --- a/turnstone/tools/diff_file.json +++ b/turnstone/tools/diff_file.json @@ -23,7 +23,6 @@ }, "required": ["path_a"] }, - "agent": true, "task_agent": true, "auto_approve": true, "primary_key": "path_a" diff --git a/turnstone/tools/man.json b/turnstone/tools/man.json deleted file mode 100644 index c1f873d8..00000000 --- a/turnstone/tools/man.json +++ /dev/null @@ -1,22 +0,0 @@ -{ - "name": "man", - "description": "Read a man page. Questions about command flags, options, or usage are tool-use tasks — use man, not memory. Use this to look up, check, or explain a command's flags, options, or usage — e.g. 'What does --xattrs do in tar?' → man(page='tar'). Use this instead of bash('man ...') or web_search for command documentation.", - "parameters": { - "type": "object", - "properties": { - "page": { - "type": "string", - "description": "The man page name (e.g. 'grep', 'socket', 'printf')." - }, - "section": { - "type": "string", - "description": "Manual section (e.g. '1' commands, '2' syscalls, '3' library). Optional." - } - }, - "required": ["page"] - }, - "agent": true, - "task_agent": true, - "auto_approve": true, - "primary_key": "page" -} diff --git a/turnstone/tools/math.json b/turnstone/tools/math.json deleted file mode 100644 index 933ee672..00000000 --- a/turnstone/tools/math.json +++ /dev/null @@ -1,18 +0,0 @@ -{ - "name": "math", - "description": "Execute Python code for math/computation. Use this for any arithmetic, algebra, or numerical task — e.g. math(code='print(2**64 - 1)'). Code must use print() to produce output. Available: sympy, numpy, scipy (with scipy.special, scipy.optimize, scipy.integrate, scipy.linalg), math, fractions, itertools, functools, collections, decimal, operator, random, re, string. Common sympy names (symbols, solve, simplify, expand, factor, sqrt, Rational, Matrix, integrate, diff, etc.) are pre-imported. Example: x = symbols('x'); print(solve(x**2 - 4, x))", - "parameters": { - "type": "object", - "properties": { - "code": { - "type": "string", - "description": "Python code to execute. Must use print() for output." - } - }, - "required": ["code"] - }, - "agent": true, - "task_agent": true, - "auto_approve": true, - "primary_key": "code" -} diff --git a/turnstone/tools/notify.json b/turnstone/tools/notify.json index 407a7762..aba2e0b6 100644 --- a/turnstone/tools/notify.json +++ b/turnstone/tools/notify.json @@ -28,7 +28,6 @@ }, "required": ["message"] }, - "agent": true, "task_agent": true, "coordinator": true, "interactive": true, diff --git a/turnstone/tools/plan_agent.json b/turnstone/tools/plan_agent.json deleted file mode 100644 index a144b02b..00000000 --- a/turnstone/tools/plan_agent.json +++ /dev/null @@ -1,19 +0,0 @@ -{ - "name": "plan_agent", - "description": "Delegate planning to a sub-agent. The agent autonomously explores the codebase, gathers context, and writes a step-by-step plan — just pass the goal directly. Use when asked to plan, design, think through, or strategize. Not for direct code changes like 'add a docstring' or 'fix a bug' — use read_file+edit_file for those. The plan agent has read-only tools (read_file, search, web_fetch, web_search, man) but cannot run bash, save memories, set watches, or delegate further.", - "parameters": { - "type": "object", - "properties": { - "goal": { - "type": "string", - "description": "The goal and scope of the plan, including any constraints." - }, - "model": { - "type": "string", - "description": "Optional model alias to run this plan agent on. Omit to use the current session model. (No alternative aliases configured in this session.)" - } - }, - "required": ["goal"] - }, - "primary_key": "goal" -} diff --git a/turnstone/tools/read_file.json b/turnstone/tools/read_file.json index 7bf03f56..26508ae5 100644 --- a/turnstone/tools/read_file.json +++ b/turnstone/tools/read_file.json @@ -19,7 +19,6 @@ }, "required": ["path"] }, - "agent": true, "task_agent": true, "auto_approve": true, "primary_key": "path" diff --git a/turnstone/tools/read_resource.json b/turnstone/tools/read_resource.json index 5c5a1f95..e23fcd24 100644 --- a/turnstone/tools/read_resource.json +++ b/turnstone/tools/read_resource.json @@ -11,7 +11,6 @@ }, "required": ["uri"] }, - "agent": true, "task_agent": true, "auto_approve": false, "primary_key": "uri" diff --git a/turnstone/tools/search.json b/turnstone/tools/search.json index 2f3a6c2d..1009d686 100644 --- a/turnstone/tools/search.json +++ b/turnstone/tools/search.json @@ -15,7 +15,6 @@ }, "required": ["query"] }, - "agent": true, "task_agent": true, "auto_approve": true, "primary_key": "query" diff --git a/turnstone/tools/task_agent.json b/turnstone/tools/task_agent.json index dc0aa43a..7ad28c39 100644 --- a/turnstone/tools/task_agent.json +++ b/turnstone/tools/task_agent.json @@ -1,6 +1,6 @@ { "name": "task_agent", - "description": "Delegate a general-purpose task to an autonomous sub-agent. The agent can read, write, edit, search, and run commands but does NOT have access to memory, recall, watch, skill, plan_agent, or task_agent — it cannot save memories, search conversation history, set up watches, switch skills mid-task, or delegate further. All context must be in the prompt. Provide a clear, self-contained task description. Optionally pass `skill=` to run the sub-agent under a specific persona (the skill is fixed at invocation and cannot be changed mid-task); use `skill(action='search', query='...')` to find an appropriate name first. An empty `skill` value is acceptable — the sub-agent runs as a competent general-purpose task helper.", + "description": "Delegate a general-purpose task to an autonomous sub-agent. The agent can read, write, edit, search, and run commands but does NOT have access to memory, recall, watch, skill, or task_agent — it cannot save memories, search conversation history, set up watches, switch skills mid-task, or delegate further. All context must be in the prompt. Provide a clear, self-contained task description. Optionally pass `skill=` to run the sub-agent under a specific persona (the skill is fixed at invocation and cannot be changed mid-task); use `skill(action='search', query='...')` to find an appropriate name first. An empty `skill` value is acceptable — the sub-agent runs as a competent general-purpose task helper.", "parameters": { "type": "object", "properties": { diff --git a/turnstone/tools/use_prompt.json b/turnstone/tools/use_prompt.json index 062a1cc7..d250ee7d 100644 --- a/turnstone/tools/use_prompt.json +++ b/turnstone/tools/use_prompt.json @@ -16,7 +16,6 @@ }, "required": ["name"] }, - "agent": true, "task_agent": true, "auto_approve": false, "primary_key": "name" diff --git a/turnstone/tools/web_fetch.json b/turnstone/tools/web_fetch.json index 3a87b7b1..586950a1 100644 --- a/turnstone/tools/web_fetch.json +++ b/turnstone/tools/web_fetch.json @@ -15,7 +15,6 @@ }, "required": ["url", "question"] }, - "agent": true, "task_agent": true, "auto_approve": true, "primary_key": "url" diff --git a/turnstone/tools/web_search.json b/turnstone/tools/web_search.json index 8bb82534..5eaf8b7c 100644 --- a/turnstone/tools/web_search.json +++ b/turnstone/tools/web_search.json @@ -20,7 +20,6 @@ }, "required": ["query"] }, - "agent": true, "task_agent": true, "auto_approve": true, "primary_key": "query" diff --git a/turnstone/ui/static/app.js b/turnstone/ui/static/app.js index 1f8572a9..6036bc16 100644 --- a/turnstone/ui/static/app.js +++ b/turnstone/ui/static/app.js @@ -1226,18 +1226,6 @@ class Pane { this.updateStatus(evt); break; - case "plan_review": - showPlanDialog(evt.content); - break; - - case "plan_resolved": - // Plan was resolved on another client (or by server-initiated cancel). - // Only act if our modal is for this pane's workstream. - if (_planWsId === this.wsId) { - dismissPlanDialog(evt.feedback); - } - break; - case "info": this.addInfoMessage(evt.message); break; @@ -6522,148 +6510,6 @@ document.addEventListener("keydown", function (e) { btn.click(); }); -// =========================================================================== -// 13. Plan review dialog -// =========================================================================== - -let _planContent = ""; -let _planPaneId = null; -let _planWsId = null; - -function showPlanDialog(content) { - _planContent = content; - _planPaneId = focusedPaneId; - const paneNow = panes[_planPaneId]; - _planWsId = paneNow ? paneNow.wsId : currentWsId; - document.getElementById("plan-content").textContent = content; - const feedbackEl = document.getElementById("plan-feedback"); - feedbackEl.value = ""; - _updatePlanRejectBtn(); - - // Disable focused pane input - const pane = panes[_planPaneId]; - if (pane) { - pane.inputEl.disabled = true; - pane.sendBtn.disabled = true; - } - - document.getElementById("plan-overlay").classList.add("active"); - setTimeout(function () { - feedbackEl.focus(); - }, 50); -} - -function _updatePlanRejectBtn() { - const btn = document.getElementById("btn-plan-reject"); - const hasFeedback = - document.getElementById("plan-feedback").value.trim().length > 0; - btn.replaceChildren(makeKeyLabel("Esc", hasFeedback ? "Amend" : "Reject")); - btn.style.background = hasFeedback ? "var(--accent)" : ""; - btn.style.color = hasFeedback ? "var(--on-color)" : ""; - btn.onclick = function () { - resolvePlan(hasFeedback ? "" : "reject"); - }; -} - -function resolvePlan(defaultFeedback) { - let feedback = document.getElementById("plan-feedback").value.trim(); - if (!feedback && defaultFeedback) feedback = defaultFeedback; - // Removing 'active' synchronously is what lets dismissPlanDialog's - // early-return guard treat the server's echoed plan_resolved as a no-op. - document.getElementById("plan-overlay").classList.remove("active"); - - const pane = panes[_planPaneId]; - if (pane) { - pane.inputEl.disabled = false; - pane.sendBtn.disabled = false; - pane.inputEl.focus(); - } - - // Critical: fire the API call first — this unblocks the server. - // Use the ws_id captured when the dialog opened, not the current pane - // (user may have switched tabs while the dialog was open). - const wsId = _planWsId || (pane ? pane.wsId : currentWsId); - _planWsId = null; - authFetch("/v1/api/plan", { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ feedback: feedback, ws_id: wsId }), - }).catch(function (err) { - if (pane) pane.addErrorMessage("Connection error: " + err.message); - }); - - // Render plan inline in the chat (best-effort). `action` is hoisted so - // the catch handler can still build a fallback message if the inline - // render throws after the action label was computed. - let action; - try { - const isReject = feedback === "reject"; - const isAmend = feedback && !isReject; - action = isReject ? "rejected" : isAmend ? "amending" : "approved"; - _addInlinePlan(_planContent, action, feedback); - } catch (err) { - console.error("Failed to render inline plan:", err); - if (pane) pane.addInfoMessage("Plan " + action); - } - - if (pane) { - pane.setBusy(true); - pane.addThinkingIndicator(); - } -} - -function dismissPlanDialog(feedback) { - // Sync-dismiss: another client (or the server) already resolved the plan. - // Do NOT call /v1/api/plan — the server has already moved on. The early - // return also handles self-receipt: the client that called resolvePlan() - // already removed the active class, so this is a no-op for that client. - const overlay = document.getElementById("plan-overlay"); - if (!overlay.classList.contains("active")) return; - overlay.classList.remove("active"); - - const pane = panes[_planPaneId]; - if (pane) { - pane.inputEl.disabled = false; - pane.sendBtn.disabled = false; - // Restore keyboard context — but skip on touch so we don't surprise the - // mobile user with a soft-keyboard pop after a remote approval. - if (!matchMedia("(pointer: coarse)").matches) pane.inputEl.focus(); - } - - const fb = feedback || ""; - const isReject = fb === "reject"; - const isAmend = fb && !isReject; - const action = isReject ? "rejected" : isAmend ? "amending" : "approved"; - - // Race fallback: if plan_resolved arrives before plan_review (e.g. SSE - // reconnect ordering), _planContent is empty and _addInlinePlan early- - // returns silently. Surface a one-line info message so the user sees - // what happened. - if (_planContent) { - try { - _addInlinePlan(_planContent, action, fb, "remote"); - } catch (err) { - console.error("Failed to render inline plan:", err); - if (pane) pane.addInfoMessage("Plan " + action + " on another device"); - } - } else if (pane) { - pane.addInfoMessage("Plan " + action + " on another device"); - } - - // SR announcement (visible toast styling deferred — #toast already has - // aria-live="polite" in markup, this just gives screen-reader parity). - _announce("Plan " + action + " on another device"); - - if (pane) { - pane.setBusy(true); - pane.addThinkingIndicator(); - } - - _planContent = ""; - _planPaneId = null; - _planWsId = null; -} - function _announce(text) { const el = document.getElementById("toast"); if (!el) return; @@ -6675,66 +6521,10 @@ function _announce(text) { }, 50); } -function _addInlinePlan(content, action, feedback, origin) { - if (!content) return; - const pane = panes[_planPaneId]; - if (!pane) return; - - const wrapper = document.createElement("div"); - wrapper.className = "plan-inline"; - - const header = document.createElement("div"); - header.className = "plan-inline-header"; - let label = - action === "rejected" - ? "Plan rejected" - : action === "amending" - ? "Plan \u2014 amending" - : "Plan approved"; - // Disambiguate remote dismissal — otherwise the desktop user sees "Plan - // approved" with no attribution and may wonder if the agent self-approved. - if (origin === "remote") label += " (synced)"; - const labelEl = document.createElement("span"); - labelEl.className = "plan-inline-label plan-" + action; - labelEl.textContent = label; - header.appendChild(labelEl); - wrapper.appendChild(header); - - const body = document.createElement("div"); - body.className = "plan-inline-body"; - try { - setMarkdown(body, content); - } catch (e) { - body.textContent = content; - } - if (content.split("\n").length > 12) { - makeCollapsible(body); - body.setAttribute( - "aria-label", - "Plan content (collapsed). Activate to expand.", - ); - } - wrapper.appendChild(body); - - if (feedback && action === "amending") { - const fb = document.createElement("div"); - fb.className = "plan-inline-feedback"; - fb.textContent = "Feedback: " + feedback; - wrapper.appendChild(fb); - } - - pane.messagesEl.appendChild(wrapper); - pane.scrollToBottom(); -} - // =========================================================================== // 14. Keyboard shortcuts // =========================================================================== -document - .getElementById("plan-feedback") - .addEventListener("input", _updatePlanRejectBtn); - // Dashboard composer wiring — Enter (no shift) submits, input refreshes the // button label, paperclip + drag-drop + paste-image stage files, options // toggle expands the dropdown panel. @@ -7467,32 +7257,6 @@ document.addEventListener("keydown", function (e) { } return; } - - // Plan dialog - if (document.getElementById("plan-overlay").classList.contains("active")) { - if (e.key === "Enter" && !e.shiftKey) { - e.preventDefault(); - resolvePlan(""); - } else if (e.key === "Escape") { - e.preventDefault(); - const hasFb = - document.getElementById("plan-feedback").value.trim().length > 0; - resolvePlan(hasFb ? "" : "reject"); - } else if (e.key === "Tab") { - const focusable = document.querySelectorAll( - "#plan-dialog input, #plan-dialog button", - ); - const first = focusable[0], - last = focusable[focusable.length - 1]; - if (e.shiftKey && document.activeElement === first) { - e.preventDefault(); - last.focus(); - } else if (!e.shiftKey && document.activeElement === last) { - e.preventDefault(); - first.focus(); - } - } - } }); // =========================================================================== diff --git a/turnstone/ui/static/index.html b/turnstone/ui/static/index.html index f81d7e47..71f2b9f6 100644 --- a/turnstone/ui/static/index.html +++ b/turnstone/ui/static/index.html @@ -503,33 +503,6 @@ - -
- -
-