From d3744b002232a0caddd2fd33e83f32c02ace04e3 Mon Sep 17 00:00:00 2001 From: Evan Date: Sat, 10 Oct 2026 00:32:19 +0800 Subject: [PATCH 1/2] feat: centralize V3 tool schemas and policy in registry --- docs/extensions.md | 34 ++ docs/index.md | 1 + .../plans/2026-10-09-v3-issue-232.md | 39 +++ docs/tool-inventory.md | 102 +++--- openkyrozen/agent/delegation.py | 3 +- openkyrozen/agent/executor.py | 2 +- openkyrozen/agent/prompts.py | 16 +- openkyrozen/agent/protocol.py | 2 +- openkyrozen/agent/response_recovery.py | 2 +- openkyrozen/agent/runtime.py | 36 +- openkyrozen/agent/types.py | 24 +- openkyrozen/app/state.py | 71 +--- openkyrozen/interfaces/mcp/server.py | 130 +------- openkyrozen/learning/regression.py | 15 +- openkyrozen/security/permission_gate.py | 18 +- openkyrozen/security/permissions.py | 7 +- openkyrozen/security/tool_policy.py | 97 ++---- openkyrozen/tools/__init__.py | 2 + openkyrozen/tools/adapters.py | 21 +- openkyrozen/tools/catalog.py | 137 ++++++++ openkyrozen/tools/dynamic.py | 14 +- openkyrozen/tools/manifest.py | 314 ++++++++++++++++-- openkyrozen/tools/ports.py | 5 +- openkyrozen/workspace/binding.py | 3 - scripts/generate_tool_inventory.py | 28 +- tests/test_tool_registry.py | 189 +++++++++++ 26 files changed, 904 insertions(+), 408 deletions(-) create mode 100644 docs/superpowers/plans/2026-10-09-v3-issue-232.md create mode 100644 openkyrozen/tools/catalog.py create mode 100644 tests/test_tool_registry.py diff --git a/docs/extensions.md b/docs/extensions.md index 5a6d61c..54a7717 100644 --- a/docs/extensions.md +++ b/docs/extensions.md @@ -29,3 +29,37 @@ Self-learning produces bounded candidate artifacts and records their evidence, v ## Troubleshooting If `/skills` does not show an installed package, check the selected skill root, manifest, activation state, and compatibility diagnostics. If plugin loading fails, inspect the sanitized startup diagnostic and import path; do not solve an import problem by adding a credentials file or private environment to a public report. Restore a prior skill through its documented rollback path; disable or remove untrusted extensions only after recording any state they own. + +## Tool catalog contracts + +`ToolRegistry` owns canonical tool identity, schema, capability, risk, side-effect +class, parallel safety, visibility, aliases and version/source provenance. +`ToolSpec` binds that metadata to a workspace/runtime executor; `ToolManifest` +remains a compatibility projection. MCP `inputSchema` and provider-neutral +`parameters` are generated from the same defensive schema copy. Tool descriptions +in prompts, discovery and generated inventory come from registered specs. + +`AVAILABLE_TOOLS` is a registry-backed compatibility mapping. Adding a callable +registers a complete spec, replacing an executor retains its metadata, and +removal removes its aliases and descriptors. Unknown legacy callables default to +the restrictive `dynamic` capability, unknown side effects and no parallel +safety. Explicit `register_spec` rejects duplicate names and alias collisions; +replacement requires `replace=True`. Schemas and alias metadata are immutable, +and snapshots preserve complete specs rather than only callables. + +Dynamic source registration retains its existing opt-in, capability, AST checks, +confirmation and audit gates. Registry metadata does not grant capabilities or +bypass approvals. Side effects are separate from capabilities: graph refresh can +update derived state under its existing read capability and remains nonparallel. +Browser and orchestration tools remain nonparallel. Only the pure calculator is +currently declared parallel-safe; this catalog does not schedule parallel calls. + +Existing string executors and MCP compatibility argument conversion remain in +use. The native typed invocation migration belongs to V3 issue #233. Tool +inventory is generated with `scripts/generate_tool_inventory.py --write` and +checked by `make docs-check`. + +Registration validates the supported object-schema vocabulary, including nested +properties/items/combinators and scalar constraints. Unsupported keywords are +rejected rather than forwarded unchecked; this registry is not a general JSON +Schema evaluation engine. Existing legacy argument conversion remains unchanged. diff --git a/docs/index.md b/docs/index.md index af2c455..e71551a 100644 --- a/docs/index.md +++ b/docs/index.md @@ -29,6 +29,7 @@ This index covers the current product guides, developer references, and dated en - [V3 architecture ownership plan](superpowers/plans/2026-10-07-v3-issue-227.md) records the scoped implementation and verification for issue #227. - [V3 provider contract plan](superpowers/plans/2026-10-08-v3-issue-228.md) records the structured response and compatibility migration for issue #228. - [V3 provider error plan](superpowers/plans/2026-10-09-v3-issue-231.md) records typed failures, cancellation and bounded retry for issue #231. +- [V3 authoritative tool catalog plan](superpowers/plans/2026-10-09-v3-issue-232.md) records registry ownership and schema/policy migration for issue #232. - Machine-readable audit receipts and benchmark inputs remain beside their corresponding reports under `docs/` and `docs/benchmarks/`. The repository's [English overview](../README.md) links here. Technical guides are maintained in English; README translations provide localized entry points. diff --git a/docs/superpowers/plans/2026-10-09-v3-issue-232.md b/docs/superpowers/plans/2026-10-09-v3-issue-232.md new file mode 100644 index 0000000..f1f95a0 --- /dev/null +++ b/docs/superpowers/plans/2026-10-09-v3-issue-232.md @@ -0,0 +1,39 @@ +# V3 authoritative tool catalog implementation plan + +> **For agentic workers:** Use superpowers:executing-plans and test-driven-development. Deliver only issue #232; no merge. + +**Goal:** Complete R011, R012, R013, R022 and R023 with the existing ToolRegistry as the authoritative catalog for all 48 runtime tools. +**Architecture:** ToolSpec stores immutable schema/policy/identity metadata and workspace-bound executors. Registry-backed compatibility mappings preserve string invocation and aliases; MCP/provider descriptors, prompts, discovery and docs derive from specs. Security remains the permission authority. +**Tech stack:** Python 3.12/3.13, standard library, existing SDK/test environments; no new dependency or paid calls. +**Spec:** User-approved plan and https://github.com/EvanProgramming/OpenKyrozen/issues/232. + +## Task 1: Catalog contracts +- [x] Write failing specs/registration/runtime-manifest regressions. +- [x] Extend tools/manifest.py with ToolSpec, ToolManifest projection, validated atomic registration, defensive schema copies and registry-backed mutable compatibility mappings. +- [x] Declare all built-in metadata once in a tool-owned catalog, including canonical identities, aliases, capabilities, side effects, conservative parallel safety and current MCP/string conversions. +- [x] Verify invalid metadata/schema/aliases, duplicate rejection, replacement/removal and snapshot preservation. + +## Task 2: Runtime and security consumers +- [x] Bind adapter, memory, discovery and orchestration executors per workspace/runtime; preserve child orchestration exclusions and workspace/session isolation. +- [x] Make compatibility AVAILABLE_TOOLS writes register metadata; route dynamic definitions and learning restore through registry. +- [x] Derive capability helpers and aliases from catalog; retain config/token/mode/approval bounds and argument-sensitive checks. +- [x] Build prompts/discovery from spec descriptions and visibility, without adding deferred-discovery behavior assigned to #234. + +## Task 3: Schemas and documentation +- [x] Move current MCP schema/legacy argument conversion metadata into catalog; preserve existing canonical objects, strings and argument aliases. Typed invocation belongs to #233. +- [x] Generate MCP and provider-neutral descriptors from the same schema; do not serialize provider SDK requests (#229). +- [x] Generate tool inventory from registered specs, retain HTTP route inventory and documentation drift checks. + +## Task 4: Verify and deliver +- [x] Run focused 3.12/3.13 tests, full suite, make check/lint/docs-check and offline agent/subagent acceptance. +- [x] Obtain independent review; reproduce/fix material findings. +- [ ] GPG-sign task-scoped commits, verify locally/GitHub, create/attach one PR, wait for final-head CI, leave unmerged. + +## Review focus +- No schema/policy bypass via compatibility dictionary mutation or dynamic registration. +- No shared bound executor across workspaces/users; child callers cannot gain orchestration tools. +- Schema copies/snapshots cannot mutate authoritative metadata. +- Dynamic tools retain restrictive capability and existing source-validation/approval/audit gates. +- Capabilities do not imply side-effect freedom or parallel safety; browser, graph refresh and orchestration remain nonparallel. + +Final local verification: 612 full Python3.12 tests,73 focused Python3.13 integration tests; make check/lint/docs-check/agent/subagent acceptance PASS. Independent review findings reproduced and fixed; final review clear. Unsupported schema vocabulary is rejected explicitly; no paid calls. diff --git a/docs/tool-inventory.md b/docs/tool-inventory.md index 565fc70..5bb656a 100644 --- a/docs/tool-inventory.md +++ b/docs/tool-inventory.md @@ -2,7 +2,7 @@ # OpenKyrozen runtime inventory This file is generated from the live runtime registry (`AgentRuntime.AVAILABLE_TOOLS`), -the FastAPI route table (`server.app.routes`), and the MCP schema builder. +ToolSpec metadata and the FastAPI route table (`server.app.routes`). Run `make docs-check` after changing a tool, endpoint, or MCP contract. ## Runtime tools @@ -12,56 +12,56 @@ Run `make docs-check` after changing a tool, endpoint, or MCP contract. - The CLI Action contract is a plain string in the `args` field. - Web and MCP exposure is filtered by the configured capability profile. -| Tool | Capability | Plain-string Action contract | MCP object input schema | -|---|---|---|---| -| `analyze_remote_repo` | `network` | Clone a remote git repository, read its files, and return a summary. | `url`: string; required: `url` | -| `browser_click` | `browser` | Click a selector. Args format: session_id\|CSS selector. | `session_id`: string, `selector`: string; required: `session_id`, `selector` | -| `browser_close` | `browser` | Close an isolated browser session. Args: browser session ID. | `session_id`: string; required: `session_id` | -| `browser_open` | `browser` | Open a URL in an isolated headless browser profile. Args: URL. | `url`: string; required: `url` | -| `browser_snapshot` | `browser` | Read the current page text. Args: browser session ID. | `session_id`: string; required: `session_id` | -| `browser_type` | `browser` | Fill a selector. Args format: session_id\|CSS selector\|text. | `session_id`: string, `selector`: string, `text`: string; required: `session_id`, `selector`, `text` | -| `calculate` | `read` | Check a bounded arithmetic expression, e.g. sum([Decimal('1.00'), Decimal('2.00')]); no workspace code runs. | `args`: string | -| `cancel_subagent` | `read` | Cancel a scoped sub-agent and prevent subsequent tool execution; args JSON contains run_id. | `args`: string | -| `check_stored_data` | `read` | Return a categorized summary of stored memories. | `args`: string | -| `discover_tools` | `read` | Discover permitted tools; args is empty for the catalog or comma-separated exact tool names for full descriptions. | `args`: string | -| `edit_file` | `write` | Replace one exact snippet only when the file still matches its read hash. | `args`: string | -| `execute_terminal_command` | `shell` | Execute a terminal command. This is an alias for run_cmd. | `command`: string; required: `command` | -| `find_files` | `read` | Find files matching a pattern. Args format: "pattern" or "pattern\|directory". | `pattern`: string, `directory`: string; required: `pattern` | -| `git_add` | `git` | Stage files for commit. Args format: "file1 file2" or "." (stage all). | `args`: string | -| `git_branch` | `git` | List or manage git branches. Args format: "" (list all), "branch_name" (create), | `args`: string | -| `git_checkout` | `git` | Switch branches or restore files. Args format: "branch_name" (switch), | `args`: string | -| `git_clone` | `git` | Clone a git repository. Args format: "url" or "url\|destination". | `url`: string, `destination`: string; required: `url` | -| `git_commit` | `git` | Commit staged changes. Args format: '"commit message"' (quotes recommended). | `args`: string | -| `git_diff` | `read` | Show git diff (unstaged, staged, or between commits). | `args`: string | -| `git_log` | `read` | Show git commit log. Args format: "-5" or "--since='2 days ago' --author='Name'" | `args`: string | -| `git_pull` | `git` | Pull changes from remote. Args format: "" (pull current branch), | `args`: string | -| `git_push` | `git` | Push commits to remote. Args format: "" (push current branch), | `args`: string | -| `git_remote` | `git` | Manage remote repositories. Args format: "" (list remotes), | `args`: string | -| `git_reset` | `destructive` | Reset current HEAD to a specified state. | `args`: string | -| `git_show` | `read` | Show details of a git object (commit, tag, etc). Args format: "HEAD" or commit hash. | `args`: string | -| `git_stash` | `git` | Stash or unstash working directory changes. | `args`: string | -| `git_status` | `read` | Show git status of a repository. Args format: "path" (default "."). | `args`: string | -| `github_cli` | `git` | Run GitHub CLI arguments directly without a shell. Agent mode and approval are required. | `args`: string | -| `github_read` | `network` | Run an allowlisted read-only GitHub CLI command. | `args`: string | -| `github_status` | `network` | Inspect GitHub CLI installation and authentication without exposing tokens. | `args`: string | -| `graph_explain` | `read` | Explain one graph node. Args: node label. | `args`: string | -| `graph_path` | `read` | Trace the shortest graph path. Args: left\|right. | `args`: string | -| `graph_query` | `read` | Query the private Graphify index. Args: a project question. | `args`: string | -| `graph_refresh` | `read` | Refresh the private local code graph. Pass --full for a clean rebuild. | `args`: string | -| `graph_status` | `read` | Return the private Graphify index state and a bounded mini graph. | `args`: string | -| `list_dir` | `read` | List contents of a directory. Args format: "path" (default "."). | `path`: string | -| `list_subagents` | `read` | List agents or inspect one run_id from this chat/project; args is JSON. | `args`: string | -| `list_tree` | `read` | Recursively list the directory tree of the given path. Args format: "path" (default "."). | `path`: string | -| `read_file` | `read` | Read content from a file. Args format: "path". | `path`: string; required: `path` | -| `read_webpage` | `network` | Fetch the content of a web page and return its plain‑text body. | `url`: string; required: `url` | -| `run_cmd` | `shell` | Execute a shell command. Args: the full command string. | `command`: string; required: `command` | -| `search_files` | `read` | Search literal text with bounded, relative file-and-line results. | `args`: string | -| `search_memory` | `read` | Search stored memories for facts relevant to the query. Args: "query" | `args`: string | -| `search_web` | `network` | Search the internet for real-time information. | `query`: string; required: `query` | -| `send_subagent` | `read` | Reuse a finished sub-agent; args JSON contains run_id and a complete assignment. | `args`: string | -| `spawn_agents` | `read` | Automatically start parallel specialist assignments; args is a JSON assignment batch. | `args`: string | -| `wait_subagents` | `read` | Wait up to 60 seconds for delegated work and its mandatory reviews; args is JSON. | `args`: string | -| `write_file` | `write` | Write content to a file. Args format: "path\|content". | `path`: string, `content`: string; required: `path`, `content` | +| Tool | Capability | Side effects | Parallel safe | Plain-string Action contract | MCP object input schema | +|---|---|---|---|---|---| +| `analyze_remote_repo` | `network` | `external` | false | Clone a remote git repository, read its files, and return a summary. | `url`: string; required: `url` | +| `browser_click` | `browser` | `stateful` | false | Click a selector. Args format: session_id\|CSS selector. | `selector`: string, `session_id`: string; required: `session_id`, `selector` | +| `browser_close` | `browser` | `stateful` | false | Close an isolated browser session. Args: browser session ID. | `session_id`: string; required: `session_id` | +| `browser_open` | `browser` | `stateful` | false | Open a URL in an isolated headless browser profile. Args: URL. | `url`: string; required: `url` | +| `browser_snapshot` | `browser` | `stateful` | false | Read the current page text. Args: browser session ID. | `session_id`: string; required: `session_id` | +| `browser_type` | `browser` | `stateful` | false | Fill a selector. Args format: session_id\|CSS selector\|text. | `selector`: string, `session_id`: string, `text`: string; required: `session_id`, `selector`, `text` | +| `calculate` | `read` | `none` | true | Check a bounded arithmetic expression, e.g. sum([Decimal('1.00'), Decimal('2.00')]); no workspace code runs. | `args`: string | +| `cancel_subagent` | `read` | `orchestration` | false | Cancel a scoped sub-agent and prevent subsequent tool execution; args JSON contains run_id. | `args`: string | +| `check_stored_data` | `read` | `read` | false | Return a categorized summary of stored memories. | `args`: string | +| `discover_tools` | `read` | `read` | false | Discover permitted tools; args is empty for the catalog or comma-separated exact tool names for full descriptions. | `args`: string | +| `edit_file` | `write` | `mutation` | false | Replace one exact snippet only when the file still matches its read hash. | `args`: string | +| `execute_terminal_command` | `shell` | `unknown` | false | Execute a terminal command. This is an alias for run_cmd. | `command`: string; required: `command` | +| `find_files` | `read` | `read` | false | Find files matching a pattern. Args format: "pattern" or "pattern\|directory". | `directory`: string, `pattern`: string; required: `pattern` | +| `git_add` | `git` | `mutation` | false | Stage files for commit. Args format: "file1 file2" or "." (stage all). | `args`: string | +| `git_branch` | `git` | `mutation` | false | List or manage git branches. Args format: "" (list all), "branch_name" (create), | `args`: string | +| `git_checkout` | `git` | `mutation` | false | Switch branches or restore files. Args format: "branch_name" (switch), | `args`: string | +| `git_clone` | `git` | `mutation` | false | Clone a git repository. Args format: "url" or "url\|destination". | `destination`: string, `url`: string; required: `url` | +| `git_commit` | `git` | `mutation` | false | Commit staged changes. Args format: '"commit message"' (quotes recommended). | `args`: string | +| `git_diff` | `read` | `read` | false | Show git diff (unstaged, staged, or between commits). | `args`: string | +| `git_log` | `read` | `read` | false | Show git commit log. Args format: "-5" or "--since='2 days ago' --author='Name'" | `args`: string | +| `git_pull` | `git` | `mutation` | false | Pull changes from remote. Args format: "" (pull current branch), | `args`: string | +| `git_push` | `git` | `mutation` | false | Push commits to remote. Args format: "" (push current branch), | `args`: string | +| `git_remote` | `git` | `mutation` | false | Manage remote repositories. Args format: "" (list remotes), | `args`: string | +| `git_reset` | `destructive` | `mutation` | false | Reset current HEAD to a specified state. | `args`: string | +| `git_show` | `read` | `read` | false | Show details of a git object (commit, tag, etc). Args format: "HEAD" or commit hash. | `args`: string | +| `git_stash` | `git` | `mutation` | false | Stash or unstash working directory changes. | `args`: string | +| `git_status` | `read` | `read` | false | Show git status of a repository. Args format: "path" (default "."). | `args`: string | +| `github_cli` | `git` | `mutation` | false | Run GitHub CLI arguments directly without a shell. Agent mode and approval are required. | `args`: string | +| `github_read` | `network` | `external` | false | Run an allowlisted read-only GitHub CLI command. | `args`: string | +| `github_status` | `network` | `external` | false | Inspect GitHub CLI installation and authentication without exposing tokens. | `args`: string | +| `graph_explain` | `read` | `read` | false | Explain one graph node. Args: node label. | `args`: string | +| `graph_path` | `read` | `read` | false | Trace the shortest graph path. Args: left\|right. | `args`: string | +| `graph_query` | `read` | `read` | false | Query the private Graphify index. Args: a project question. | `args`: string | +| `graph_refresh` | `read` | `internal_write` | false | Refresh the private local code graph. Pass --full for a clean rebuild. | `args`: string | +| `graph_status` | `read` | `read` | false | Return the private Graphify index state and a bounded mini graph. | `args`: string | +| `list_dir` | `read` | `read` | false | List contents of a directory. Args format: "path" (default "."). | `path`: string | +| `list_subagents` | `read` | `orchestration` | false | List agents or inspect one run_id from this chat/project; args is JSON. | `args`: string | +| `list_tree` | `read` | `read` | false | Recursively list the directory tree of the given path. Args format: "path" (default "."). | `path`: string | +| `read_file` | `read` | `read` | false | Read content from a file. Args format: "path". | `path`: string; required: `path` | +| `read_webpage` | `network` | `external` | false | Fetch the content of a web page and return its plain‑text body. | `url`: string; required: `url` | +| `run_cmd` | `shell` | `unknown` | false | Execute a shell command. Args: the full command string. | `command`: string; required: `command` | +| `search_files` | `read` | `read` | false | Search literal text with bounded, relative file-and-line results. | `args`: string | +| `search_memory` | `read` | `read` | false | Search stored memories for facts relevant to the query. Args: "query" | `args`: string | +| `search_web` | `network` | `external` | false | Search the internet for real-time information. | `query`: string; required: `query` | +| `send_subagent` | `read` | `orchestration` | false | Reuse a finished sub-agent; args JSON contains run_id and a complete assignment. | `args`: string | +| `spawn_agents` | `read` | `orchestration` | false | Automatically start parallel specialist assignments; args is a JSON assignment batch. | `args`: string | +| `wait_subagents` | `read` | `orchestration` | false | Wait up to 60 seconds for delegated work and its mandatory reviews; args is JSON. | `args`: string | +| `write_file` | `write` | `mutation` | false | Write content to a file. Args format: "path\|content". | `content`: string, `path`: string; required: `path`, `content` | ## HTTP and MCP endpoints diff --git a/openkyrozen/agent/delegation.py b/openkyrozen/agent/delegation.py index 5f2764a..85b7c42 100644 --- a/openkyrozen/agent/delegation.py +++ b/openkyrozen/agent/delegation.py @@ -15,7 +15,8 @@ NAMES = ("Evan", "Morning", "Joseph", "Addison", "Jerry", "Leo", "Obert", "Jason", "Justin", "Dewey", "Old Fool", "Lianto") -TOOLS = ("spawn_agents", "send_subagent", "list_subagents", "wait_subagents", "cancel_subagent") +from openkyrozen.tools.catalog import builtin_names +TOOLS = builtin_names("orchestration") TERMINAL = {"succeeded", "unverified", "failed", "blocked", "cancelled", "interrupted"} REPORT_LISTS = ("findings", "evidence", "artifacts", "checks", "uncertainties", "suggested_followups") GUIDANCE = '''Delegate automatically when independent work can run in parallel or a specialist can diff --git a/openkyrozen/agent/executor.py b/openkyrozen/agent/executor.py index a4d9951..fa0b654 100644 --- a/openkyrozen/agent/executor.py +++ b/openkyrozen/agent/executor.py @@ -140,7 +140,7 @@ def finish(result: str, success: bool = False, authorized: bool = False, result = f"Error: unknown tool '{action}'" self._notify_tool_execute(action, args, result) return finish(result, failure="unknown_tool") - required_capability = tool_capability(action) + required_capability = tool_capability(action,self.AVAILABLE_TOOLS) try: if required_capability not in effective_capabilities(load_agent_config(self._get_workspace_root())): result = ( diff --git a/openkyrozen/agent/prompts.py b/openkyrozen/agent/prompts.py index bb29cc0..b46c3d0 100644 --- a/openkyrozen/agent/prompts.py +++ b/openkyrozen/agent/prompts.py @@ -29,9 +29,11 @@ def _compose_skills(self, task_description: str) -> str | None: def _build_tools_list(self, capabilities: frozenset[str] | None = None) -> str: lines = [] for name, fn in self.AVAILABLE_TOOLS.items(): - if capabilities is not None and tool_capability(name) not in capabilities: + if self.tool_registry.get_spec(name).visibility != 'public': continue - doc = getattr(fn, "__doc__", None) or "" + if capabilities is not None and tool_capability(name,self.AVAILABLE_TOOLS) not in capabilities: + continue + doc = self.tool_registry.get_spec(name).description desc = doc.strip().replace("\n", " ").strip() lines.append(f"- {name}: {desc}") return "\n".join(lines) @@ -49,7 +51,7 @@ def _permitted_tool_names(self, agent_config: dict[str, Any] | None = None) -> s active = getattr(self, "_execution_capability_token", None) capabilities = configured & active.capabilities if active else configured capabilities = mode_capabilities(capabilities, self._active_interaction_mode.get()) - return {name for name in self.AVAILABLE_TOOLS if tool_capability(name) in capabilities} + return {name for name in self.AVAILABLE_TOOLS if self.tool_registry.get_spec(name).visibility == 'public' and tool_capability(name,self.AVAILABLE_TOOLS) in capabilities} def _discover_tools(self, args: str) -> str: @@ -59,13 +61,13 @@ def _discover_tools(self, args: str) -> str: if not names: groups: dict[str, list[str]] = {} for name in sorted(permitted): - groups.setdefault(tool_capability(name), []).append(name) + groups.setdefault(tool_capability(name,self.AVAILABLE_TOOLS), []).append(name) return "\n".join(f"{capability}: {', '.join(group)}" for capability, group in sorted(groups.items())) if any(name not in permitted for name in names): return "Error: unknown or unavailable tool requested" self.execution_context.discovered_tools |= frozenset(names) return "\n".join( - f"- {name}: {(getattr(self.AVAILABLE_TOOLS[name], '__doc__', None) or '').strip()}" + f"- {name}: {self.tool_registry.get_spec(name).description}" for name in names ) @@ -77,7 +79,7 @@ def _agent_prompt_tools_list(self, agent_config: dict[str, Any]) -> str: if self._prompt_profile() == "compact": detailed = permitted & ({"list_dir", "find_files", "search_files", "read_file", "write_file", "edit_file", "run_cmd", "discover_tools"} | set(self.execution_context.discovered_tools)) - lines = [f"- {name}: {(getattr(fn, '__doc__', None) or '').strip()}" + lines = [f"- {name}: {self.tool_registry.get_spec(name).description}" for name, fn in self.AVAILABLE_TOOLS.items() if name in detailed] if self._prompt_profile() == "compact": if "search_files" in detailed: @@ -86,7 +88,7 @@ def _agent_prompt_tools_list(self, agent_config: dict[str, Any]) -> str: lines.append('edit_file JSON: {"path":"file","old_text":"exact text","new_text":"replacement","expected_sha256":"hash from structured read_file"}') groups: dict[str, list[str]] = {} for name in sorted(permitted - detailed): - groups.setdefault(tool_capability(name), []).append(name) + groups.setdefault(tool_capability(name,self.AVAILABLE_TOOLS), []).append(name) if groups: lines.append("Other permitted tools (call discover_tools with comma-separated names for descriptions):") lines.extend(f"{capability}: {', '.join(group)}" for capability, group in sorted(groups.items())) diff --git a/openkyrozen/agent/protocol.py b/openkyrozen/agent/protocol.py index d2f667b..32e8d08 100644 --- a/openkyrozen/agent/protocol.py +++ b/openkyrozen/agent/protocol.py @@ -470,7 +470,7 @@ def _parse_model_response(self, text: str) -> dict[str, Any]: ) disallowed_plan_action = ( self._active_interaction_mode.get() == "plan" - and any(tool_capability(self._operation_action(call.get("action", ""))) not in {"read", "network"} + and any(tool_capability(self._operation_action(call.get("action", "")),self.AVAILABLE_TOOLS) not in {"read", "network"} for call in tool_calls) ) if disallowed_plan_action: diff --git a/openkyrozen/agent/response_recovery.py b/openkyrozen/agent/response_recovery.py index c4694aa..dcbc47e 100644 --- a/openkyrozen/agent/response_recovery.py +++ b/openkyrozen/agent/response_recovery.py @@ -127,7 +127,7 @@ def valid(candidate: dict[str, Any]) -> bool: return inspection_complete calls = candidate.get("tool_calls") or [] return bool(calls) and all( - tool_capability(self._operation_action(call.get("action", ""))) in {"read", "network"} + tool_capability(self._operation_action(call.get("action", "")),self.AVAILABLE_TOOLS) in {"read", "network"} for call in calls ) diff --git a/openkyrozen/agent/runtime.py b/openkyrozen/agent/runtime.py index a5a927a..2d4238e 100644 --- a/openkyrozen/agent/runtime.py +++ b/openkyrozen/agent/runtime.py @@ -2,7 +2,6 @@ from __future__ import annotations import copy -from collections import ChainMap from contextlib import contextmanager from contextvars import ContextVar from .models import AgentSession, WorkspaceState @@ -342,16 +341,45 @@ def chat(self, session, message, *, clear_tasks=False, profile=None, memory_cont self._stream_event_callback.reset(events) self._approval_callback.reset(approval) + @property + def tool_registry(self): + from openkyrozen.tools.catalog import builtin_names + registry = self.current_session.workspace.adapters.tool_registry + for owner in ('runtime','orchestration'): + for name in builtin_names(owner): + if name not in registry.specs: + registry.register(name,getattr(self,('_' if owner=='runtime' else '')+name)) + return registry + @property def AVAILABLE_TOOLS(self): + from openkyrozen.tools.manifest import ToolMapping from .delegation import TOOLS - orchestration = {} if self.execution_context.child_run_id else {name: getattr(self, name) for name in TOOLS} - return ChainMap(self.current_session.workspace.adapters.AVAILABLE_TOOLS, orchestration) + return ToolMapping(self.tool_registry, TOOLS if self.execution_context.child_run_id else ()) @AVAILABLE_TOOLS.setter def AVAILABLE_TOOLS(self, value): self.current_session.workspace.adapters.AVAILABLE_TOOLS = value + @property + def TOOL_ALIASES(self): + return self.tool_registry.aliases + + + @property + def _ACTION_MARKER_NAMES(self): + return tuple(sorted(set(self.AVAILABLE_TOOLS)|set(self.TOOL_ALIASES),key=lambda name:(-len(name),name))) + + @property + def _ACTION_MARKER_RE(self): + names = self._ACTION_MARKER_NAMES + cached = getattr(self,'_action_marker_cache',None) + if cached is None or cached[0] != names: + import re + cached = (names,re.compile(r"(?i)(?(?:"+'|'.join(map(re.escape,names))+r"))\s*:")) + self._action_marker_cache = cached + return cached[1] + def run_command(self, args): return self.current_session.workspace.adapters.run_command(args) @@ -582,4 +610,4 @@ def _ponytail_level(self, value): def DeepSeekDSMLFilter(self): from .types import DeepSeekDSMLFilter - return DeepSeekDSMLFilter(self._is_valid_action) + return DeepSeekDSMLFilter(self._is_valid_action,self._ACTION_MARKER_NAMES) diff --git a/openkyrozen/agent/types.py b/openkyrozen/agent/types.py index 0554c2d..bf58699 100644 --- a/openkyrozen/agent/types.py +++ b/openkyrozen/agent/types.py @@ -6,8 +6,9 @@ from typing import Any -TOOL_ALIASES = {'bash': 'run_cmd', 'shell': 'run_cmd', 'sh': 'run_cmd', 'browse_summary': 'read_webpage', 'run_terminal_command': 'execute_terminal_command', 'run_terminal': 'execute_terminal_command', 'terminal': 'execute_terminal_command', 'run_command': 'run_cmd', 'cmd': 'run_cmd', 'exec': 'run_cmd', 'execute': 'run_cmd', 'list_tree': 'list_tree', 'tree': 'list_tree', 'check_memory': 'check_stored_data', 'run_shell_command': 'run_cmd', 'run_shell': 'run_cmd', 'shell_command': 'run_cmd', 'execute_shell': 'run_cmd', 'shell_cmd': 'run_cmd', 'bash_cmd': 'run_cmd', 'command': 'run_cmd', 'run': 'run_cmd', 'run_shell': 'run_cmd', 'write': 'write_file', 'status': 'git_status', 'diff': 'git_diff', 'log': 'git_log', 'branch': 'git_branch', 'add': 'git_add', 'commit': 'git_commit', 'push': 'git_push', 'pull': 'git_pull', 'checkout': 'git_checkout', 'stash': 'git_stash', 'clone': 'git_clone', 'reset': 'git_reset', 'show': 'git_show', 'remote': 'git_remote'} -_ACTION_MARKER_NAMES = ('execute_terminal_command', 'run_terminal_command', 'analyze_remote_repo', 'run_shell_command', 'check_stored_data', 'browser_snapshot', 'browse_summary', 'browser_close', 'graph_refresh', 'graph_explain', 'shell_command', 'github_status', 'browser_click', 'search_memory', 'execute_shell', 'read_webpage', 'graph_status', 'browser_open', 'run_terminal', 'git_checkout', 'browser_type', 'check_memory', 'run_command', 'graph_query', 'github_read', 'graph_path', 'git_status', 'search_web', 'find_files', 'github_cli', 'git_remote', 'git_branch', 'git_commit', 'write_file', 'git_reset', 'read_file', 'list_tree', 'git_stash', 'shell_cmd', 'run_shell', 'git_clone', 'list_dir', 'git_pull', 'git_diff', 'terminal', 'checkout', 'bash_cmd', 'git_push', 'git_show', 'git_log', 'execute', 'run_cmd', 'git_add', 'command', 'status', 'commit', 'remote', 'branch', 'write', 'shell', 'reset', 'stash', 'clone', 'show', 'pull', 'push', 'diff', 'bash', 'exec', 'tree', 'cmd', 'add', 'run', 'log', 'sh') +from openkyrozen.tools.catalog import builtin_aliases, builtin_names +TOOL_ALIASES = builtin_aliases() +_ACTION_MARKER_NAMES = tuple(sorted(set(builtin_names()) | set(TOOL_ALIASES), key=len, reverse=True)) _ACTION_MARKER_RE = re.compile( r"(?i)(?(?:" + "|".join(map(re.escape, _ACTION_MARKER_NAMES)) + r"))\s*:" ) @@ -149,8 +150,11 @@ class DeepSeekDSMLFilter: for length in range(1, len(item) + 1) ) - def __init__(self, is_valid_action=None) -> None: + def __init__(self, is_valid_action=None, action_names=None) -> None: self._is_valid_action = is_valid_action or _is_valid_action + if action_names is not None: + self._ACTION_MARKER_RE = re.compile(r"(?i)(?"+'|'.join(map(re.escape,action_names))+r")\s*:") + self._CONTROL_PREFIXES = self._CONTROL_PREFIXES + tuple(name[:length].lower() for name in action_names for length in range(1,len(name)+1)) self._buffer = "" self._control_buffer = "" @@ -170,8 +174,7 @@ def _partial_suffix_length(cls, value: str) -> int: def _close_for(cls, match: re.Match[str]) -> str: return f"" - @classmethod - def _control_partial_suffix_length(cls, value: str) -> int: + def _control_partial_suffix_length(self, value: str) -> int: start = value.rfind("<") if start >= 0: suffix = value[start:] @@ -185,11 +188,11 @@ def _control_partial_suffix_length(cls, value: str) -> int: name_match = re.match(r"[A-Za-z_][A-Za-z0-9_]*", remainder) if name_match: name = name_match.group(0).lower() - if any(kind.startswith(name) for kind in cls._GENERIC_KINDS): + if any(kind.startswith(name) for kind in self._GENERIC_KINDS): return len(suffix) lowered = value.lower() - for length in range(min(len(value), max(map(len, cls._CONTROL_PREFIXES))), 0, -1): - if lowered[-length:] in cls._CONTROL_PREFIXES: + for length in range(min(len(value), max(map(len, self._CONTROL_PREFIXES))), 0, -1): + if lowered[-length:] in self._CONTROL_PREFIXES: start = len(value) - length if start and value[start - 1].isalnum(): continue @@ -203,9 +206,8 @@ def _find_control_marker(cls, value: str, *, final: bool) -> re.Match[str] | Non return match return None - @classmethod - def _find_action_marker(cls, value: str, *, final: bool) -> re.Match[str] | None: - for match in cls._ACTION_MARKER_RE.finditer(value): + def _find_action_marker(self, value: str, *, final: bool) -> re.Match[str] | None: + for match in self._ACTION_MARKER_RE.finditer(value): if _action_marker_is_protocol(value, match, final=final): return match return None diff --git a/openkyrozen/app/state.py b/openkyrozen/app/state.py index 6904ba1..3a74af8 100644 --- a/openkyrozen/app/state.py +++ b/openkyrozen/app/state.py @@ -74,48 +74,6 @@ def initialise(self): except importlib.metadata.PackageNotFoundError: self.__version__ = self.RELEASE_VERSION - self.TOOL_ALIASES: dict[str, str] = { - "bash": "run_cmd", - "shell": "run_cmd", - "sh": "run_cmd", - "browse_summary": "read_webpage", - "run_terminal_command": "execute_terminal_command", - "run_terminal": "execute_terminal_command", - "terminal": "execute_terminal_command", - "run_command": "run_cmd", - "cmd": "run_cmd", - "exec": "run_cmd", - "execute": "run_cmd", - "list_tree": "list_tree", - "tree": "list_tree", - "check_memory": "check_stored_data", - "run_shell_command": "run_cmd", - "run_shell": "run_cmd", - "shell_command": "run_cmd", - "execute_shell": "run_cmd", - "shell_cmd": "run_cmd", - "bash_cmd": "run_cmd", - "command": "run_cmd", - "run": "run_cmd", - "run_shell": "run_cmd", - "write": "write_file", - # Git aliases - "status": "git_status", - "diff": "git_diff", - "log": "git_log", - "branch": "git_branch", - "add": "git_add", - "commit": "git_commit", - "push": "git_push", - "pull": "git_pull", - "checkout": "git_checkout", - "stash": "git_stash", - "clone": "git_clone", - "reset": "git_reset", - "show": "git_show", - "remote": "git_remote", - } - self._UNSUPPORTED_ACTION_PROTOCOL_RE = re.compile( r"(?is)<\s*ssai_action\b[^>]*>(?:[\s\S]*?|[\s\S]*\Z)" r"|<\s*details\b[^>]*>\s*<\s*summary\b[^>]*>\s*Action\s*:[\s\S]*?(?:|\Z)" @@ -177,11 +135,6 @@ def initialise(self): else self._EXECUTION_SURFACE == "cli" or self._surface_capabilities == "full" ) - self._APPROVAL_REQUIRED_TOOLS = frozenset({ - "git_push", "git_pull", "git_checkout", "git_stash", "git_reset", "git_remote", - "github_cli", "define_tool", - }) - self._provider_config: ProviderConfig | None = None self.llm_provider: LLMProvider | None = None @@ -275,17 +228,8 @@ def initialise(self): }, ] - self._BUILTIN_TOOL_NAMES = { - "write_file","read_file","calculate","run_cmd","search_web","find_files","list_dir", - "git_clone","git_status","execute_terminal_command","analyze_remote_repo", - "list_tree","read_webpage","check_stored_data","search_memory", - "git_diff","git_log","git_branch","git_add","git_commit", - "git_push","git_pull","git_checkout","git_stash","git_reset", - "git_show","git_remote", - "browser_open","browser_snapshot","browser_click","browser_type","browser_close", - "graph_status","graph_query","graph_explain","graph_path","graph_refresh", - "github_status","github_read","github_cli", - } + from openkyrozen.tools.catalog import builtin_names + self._BUILTIN_TOOL_NAMES = set(builtin_names()) self._saved_user_tools: dict[str, Any] = {} @@ -430,11 +374,6 @@ def initialise(self): re.IGNORECASE, ) - self.AVAILABLE_TOOLS["check_stored_data"] = self._check_stored_data - - self.AVAILABLE_TOOLS["search_memory"] = self._search_memory - self.AVAILABLE_TOOLS["discover_tools"] = self._discover_tools - self.TOOLS_LIST = self._build_tools_list() @@ -455,12 +394,6 @@ def initialise(self): r"act\s+as\s+if", ] - self._ACTION_MARKER_NAMES = tuple(sorted(set(self.AVAILABLE_TOOLS) | set(self.TOOL_ALIASES), key=len, reverse=True)) - - self._ACTION_MARKER_RE = re.compile( - r"(?i)(?(?:" + "|".join(map(re.escape, self._ACTION_MARKER_NAMES)) + r"))\s*:" - ) - self._ACTION_SENTENCE_ENDS = (".", "!", "?", "…", "。", "！", "？", ")", "]", "}", "`", '"', "'") self._ACTION_PROSE_START_RE = re.compile( diff --git a/openkyrozen/interfaces/mcp/server.py b/openkyrozen/interfaces/mcp/server.py index f2980d0..452588f 100644 --- a/openkyrozen/interfaces/mcp/server.py +++ b/openkyrozen/interfaces/mcp/server.py @@ -29,136 +29,16 @@ def _mcp_error(self, request_id: Any, code: int, message: str, data: Any = None) def _mcp_tool_schema(self, name: str, fn: Any) -> dict[str, Any]: - service = self - """Return the object schema for a tool's existing string contract.""" - schemas: dict[str, dict[str, Any]] = { - "read_file": { - "properties": {"path": {"type": "string"}}, "required": ["path"], - }, - "write_file": { - "properties": { - "path": {"type": "string"}, - "content": {"type": "string"}, - }, "required": ["path", "content"], - }, - "list_dir": {"properties": {"path": {"type": "string"}}}, - "list_tree": {"properties": {"path": {"type": "string"}}}, - "find_files": { - "properties": { - "pattern": {"type": "string"}, - "directory": {"type": "string"}, - }, "required": ["pattern"], - }, - "run_cmd": {"properties": {"command": {"type": "string"}}, "required": ["command"]}, - "execute_terminal_command": { - "properties": {"command": {"type": "string"}}, "required": ["command"], - }, - "search_web": {"properties": {"query": {"type": "string"}}, "required": ["query"]}, - "read_webpage": {"properties": {"url": {"type": "string"}}, "required": ["url"]}, - "git_clone": { - "properties": { - "url": {"type": "string"}, - "destination": {"type": "string"}, - }, "required": ["url"], - }, - "analyze_remote_repo": {"properties": {"url": {"type": "string"}}, "required": ["url"]}, - "browser_open": {"properties": {"url": {"type": "string"}}, "required": ["url"]}, - "browser_snapshot": {"properties": {"session_id": {"type": "string"}}, "required": ["session_id"]}, - "browser_close": {"properties": {"session_id": {"type": "string"}}, "required": ["session_id"]}, - "browser_click": { - "properties": { - "session_id": {"type": "string"}, - "selector": {"type": "string"}, - }, "required": ["session_id", "selector"], - }, - "browser_type": { - "properties": { - "session_id": {"type": "string"}, - "selector": {"type": "string"}, - "text": {"type": "string"}, - }, "required": ["session_id", "selector", "text"], - }, - } - schema = copy.deepcopy(schemas.get(name, { - "properties": {"args": {"type": "string"}}, - })) - schema["type"] = "object" - schema["additionalProperties"] = False - if name not in schemas: - schema["description"] = "Plain-string arguments can be supplied as the `args` property." - return schema + return self._agent.tool_registry.get_spec(name).input_schema def _mcp_tool_descriptors(self, allowed: set[str]) -> list[dict[str, Any]]: - service = self - return [ - { - "name": name, - "description": (getattr(fn, "__doc__", "") or "").strip().split("\n")[0], - "inputSchema": service._mcp_tool_schema(name, fn), - } - for name, fn in service._agent.AVAILABLE_TOOLS.items() - if name in allowed - ] + return self._agent.tool_registry.mcp_descriptors(names=allowed) def _mcp_string_arguments(self, tool_name: str, arguments: Any) -> str: - service = self - """Map MCP object arguments to the tool's documented pipe/string format.""" - if arguments is None: - return "" - if isinstance(arguments, str): - return arguments - if not isinstance(arguments, dict): - raise ValueError("arguments must be an object or plain string") - if not arguments: - return "" - if set(arguments) == {"args"}: - if not isinstance(arguments["args"], str): - raise ValueError("arguments.args must be a string") - return arguments["args"] - - def value(*names: str, required: bool = True) -> str: - present = next((name for name in names if name in arguments), None) - if present is None: - if required: - raise ValueError(f"missing required argument: {names[0]}") - return "" - if not isinstance(arguments[present], str): - raise ValueError(f"argument '{present}' must be a string") - return arguments[present] - - if tool_name == "read_file": - return value("path", "file_path") - if tool_name == "write_file": - return f"{value('path', 'file_path')}|{value('content', 'text')}" - if tool_name in {"list_dir", "list_tree"}: - return value("path", required=False) - if tool_name == "find_files": - pattern = value("pattern") - directory = value("directory", required=False) - return f"{pattern}|{directory}" if directory else pattern - if tool_name in {"run_cmd", "execute_terminal_command"}: - return value("command", "cmd") - if tool_name == "search_web": - return value("query") - if tool_name in {"read_webpage", "browser_open", "analyze_remote_repo"}: - return value("url") - if tool_name == "git_clone": - url = value("url") - destination = value("destination", required=False) - return f"{url}|{destination}" if destination else url - if tool_name in {"browser_snapshot", "browser_close"}: - return value("session_id", "sessionId") - if tool_name == "browser_click": - return f"{value('session_id', 'sessionId')}|{value('selector')}" - if tool_name == "browser_type": - return f"{value('session_id', 'sessionId')}|{value('selector')}|{value('text')}" - - # Every legacy tool has a plain-string contract. Requiring the explicit - # `args` wrapper keeps ambiguous object shapes from silently changing the - # command sent to a tool. - raise ValueError(f"tool '{tool_name}' accepts object arguments only as {{'args': ''}}") + """Translate compatibility inputs using the catalog's declared string adapter.""" + return self._agent.tool_registry.get_spec(tool_name).legacy_arguments(arguments) async def mcp_endpoint(self, request: Request): @@ -219,7 +99,7 @@ async def mcp_endpoint(self, request: Request): tool_name, tool_args, f"Error: Tool '{tool_name}' is not authorized", ) return service._mcp_error(request_id, -32001, ( - f"Tool requires '{tool_capability(tool_name)}' capability; " + f"Tool requires '{tool_capability(tool_name,service._agent.AVAILABLE_TOOLS)}' capability; " "set KYROZEN_MCP_CAPABILITIES or use the full profile to enable it" )) try: diff --git a/openkyrozen/learning/regression.py b/openkyrozen/learning/regression.py index fac2487..f36b419 100644 --- a/openkyrozen/learning/regression.py +++ b/openkyrozen/learning/regression.py @@ -3,16 +3,17 @@ def _take_tool_snapshot(self) -> None: - self._saved_user_tools = {k: v for k, v in self.AVAILABLE_TOOLS.items() if k not in self._BUILTIN_TOOL_NAMES} + names = {k for k in self.AVAILABLE_TOOLS if k not in self._BUILTIN_TOOL_NAMES} + self._saved_user_tools = self.tool_registry.snapshot(names) def _restore_tool_snapshot(self) -> None: - # Remove current user‑defined tools - keys_to_remove = [k for k in self.AVAILABLE_TOOLS if k not in self._BUILTIN_TOOL_NAMES] - for k in keys_to_remove: - del self.AVAILABLE_TOOLS[k] - # Restore from snapshot - self.AVAILABLE_TOOLS.update(self._saved_user_tools) + for name in tuple(self.AVAILABLE_TOOLS): + if name not in self._BUILTIN_TOOL_NAMES: + self.tool_registry.unregister(name) + for spec in self._saved_user_tools.values(): + self.tool_registry.register_spec(spec) + self.TOOLS_LIST = self._build_tools_list() def _run_regression_tests(self) -> bool: diff --git a/openkyrozen/security/permission_gate.py b/openkyrozen/security/permission_gate.py index c982cca..e47bff0 100644 --- a/openkyrozen/security/permission_gate.py +++ b/openkyrozen/security/permission_gate.py @@ -5,14 +5,10 @@ import re import shlex -from openkyrozen.security.tool_policy import tool_capability +from openkyrozen.security.tool_policy import tool_capability, tool_risk _PRIVATE = re.compile(r"(?i)(?:^|[\s/\\.\"'=|])(?:\.env(?:[./\\\s|\"']|$)|\.ssh|\.aws|\.config|keychain|credentials?|secrets?|private[_-]?key|id_rsa|id_ed25519)") -_HIGH_IMPACT = frozenset({ - "git_push", "git_pull", "git_reset", "git_checkout", "git_stash", "git_remote", - "git_commit", "git_add", "git_clone", "github_cli", "define_tool", -}) _SAFE_COMMANDS = frozenset({ "pwd", "ls", "find", "rg", "grep", "cat", "head", "tail", "wc", "file", "git status", "git diff", "git log", "git show", "git branch --show-current", @@ -99,13 +95,13 @@ def _safe_shell_command(args: str) -> bool: return True -def risk_category(action: str, args: object) -> str | None: +def risk_category(action: str, args: object, tools=None) -> str | None: """Return a bounded risk label requiring Jev or local approval.""" action = str(action or "").strip() text = _arguments(args) if _PRIVATE.search(text) or re.search(r"(?i)\b(api[_-]?key|password|credential|secret|private key|token)\b", text): return "private_data_access" - if action in _HIGH_IMPACT: + if action == 'define_tool' or tool_risk(action,tools) == 'high': return "external_or_irreversible_change" if action in {"search_memory", "check_stored_data", "browser_snapshot"}: return "private_data_access" @@ -117,13 +113,13 @@ def risk_category(action: str, args: object) -> str | None: return None if _safe_shell_command(text) else "opaque_or_high_impact_command" if action == "write_file": return None - if tool_capability(action) in {"shell", "git", "destructive", "dynamic"}: + if tool_capability(action, tools) in {"shell", "git", "destructive", "dynamic"}: return "external_or_irreversible_change" return None -def requires_ask_approval(action: str, args: object) -> bool: +def requires_ask_approval(action: str, args: object, tools=None) -> bool: action = str(action or "").strip() - if risk_category(action, args): + if risk_category(action, args, tools): return True - return tool_capability(action) in {"write", "shell", "git", "browser", "destructive", "dynamic"} + return tool_capability(action, tools) in {"write", "shell", "git", "browser", "destructive", "dynamic"} diff --git a/openkyrozen/security/permissions.py b/openkyrozen/security/permissions.py index f8e43ff..3da4b42 100644 --- a/openkyrozen/security/permissions.py +++ b/openkyrozen/security/permissions.py @@ -39,7 +39,8 @@ def _record_tool_approval(self, action: str, decision: str, args: str = "") -> N def _confirm_tool_action(self, action: str, args: str = "", *, force: bool = False) -> bool: """Confirm high-impact local CLI actions while keeping normal tools frictionless.""" - if (not force and action not in self._APPROVAL_REQUIRED_TOOLS) or self._EXECUTION_SURFACE not in {"cli", "tui"}: + from openkyrozen.security.tool_policy import tool_confirmation_required + if (not force and not tool_confirmation_required(action,self._EXECUTION_SURFACE,self.AVAILABLE_TOOLS)) or self._EXECUTION_SURFACE not in {"cli", "tui"}: return True mode = os.environ.get("KYROZEN_APPROVAL_MODE", "dangerous").strip().lower() if not force and mode in {"never", "none", "off"}: @@ -128,7 +129,7 @@ def _authorize_tool_action(self, action: str, args: object, *, approve=None) -> from openkyrozen.security.permission_gate import requires_ask_approval, risk_category mode = self.permission_mode() - risk = risk_category(action, args) + risk = risk_category(action, args, self.AVAILABLE_TOOLS) if mode == "full": if risk: self._record_permission_decision(action, risk, "unprotected_full") @@ -137,7 +138,7 @@ def _authorize_tool_action(self, action: str, args: object, *, approve=None) -> return True if mode == "full_jev" and risk and self._jev_permission_check(action, risk): return True - if mode == "ask" and not requires_ask_approval(action, args): + if mode == "ask" and not requires_ask_approval(action, args, self.AVAILABLE_TOOLS): return True callback = approve or (lambda current, details: self._confirm_tool_action(current, details, force=True)) self._emit_stream_event({"event": "permission_check", "state": "approval_required", diff --git a/openkyrozen/security/tool_policy.py b/openkyrozen/security/tool_policy.py index 21c8e3c..f921fbf 100644 --- a/openkyrozen/security/tool_policy.py +++ b/openkyrozen/security/tool_policy.py @@ -2,53 +2,6 @@ from typing import Any -_TOOL_CAPABILITIES: dict[str, str] = { - "spawn_agents": "read", "send_subagent": "read", "list_subagents": "read", - "wait_subagents": "read", "cancel_subagent": "read", - "check_stored_data": "read", - "search_memory": "read", - "read_file": "read", - "search_files": "read", - "calculate": "read", - "discover_tools": "read", - "list_dir": "read", - "list_tree": "read", - "find_files": "read", - "git_status": "read", - "git_diff": "read", - "git_log": "read", - "git_show": "read", - "write_file": "write", - "edit_file": "write", - "run_cmd": "shell", - "execute_terminal_command": "shell", - "search_web": "network", - "read_webpage": "network", - "git_clone": "git", - "git_branch": "git", - "analyze_remote_repo": "network", - "browser_open": "browser", - "browser_snapshot": "browser", - "browser_click": "browser", - "browser_type": "browser", - "browser_close": "browser", - "git_add": "git", - "git_commit": "git", - "git_push": "git", - "git_pull": "git", - "git_checkout": "git", - "git_stash": "git", - "git_reset": "destructive", - "git_remote": "git", - "graph_status": "read", - "graph_query": "read", - "graph_explain": "read", - "graph_path": "read", - "graph_refresh": "read", - "github_status": "network", - "github_read": "network", - "github_cli": "git", -} _CAPABILITY_PROFILES: dict[str, frozenset[str]] = { @@ -75,20 +28,38 @@ def allowed_tool_names(tools: dict[str, Any] | None = None, capabilities: str | Unknown tools are treated as dynamic tools, so a newly registered tool cannot silently bypass a restricted Web/MCP profile. """ - available = tools if tools is not None else _TOOL_CAPABILITIES + from openkyrozen.tools.catalog import builtin_names + available = tools if tools is not None else builtin_names() granted = resolve_capabilities(capabilities) - allowed: set[str] = set() - for name in available: - capability = _TOOL_CAPABILITIES.get(name) - if capability is None: - if "dynamic" in granted: - allowed.add(name) - continue - if capability in granted: - allowed.add(name) - return allowed - - -def tool_capability(name: str) -> str: - """Return the capability required by a tool, treating unknown tools as dynamic.""" - return _TOOL_CAPABILITIES.get(name, "dynamic") + return {name for name in available if tool_capability(name,tools) in granted} + + +def tool_capability(name: str, tools=None) -> str: + """Read active registry policy; unknown compatibility tools remain dynamic.""" + registry = getattr(tools, 'registry', None) + if registry is not None and name in registry.specs: + return registry.get_spec(name).capability + from openkyrozen.tools.catalog import builtin_metadata + metadata = builtin_metadata(name) + return metadata['capability'] if metadata else 'dynamic' + + +def tool_policy(name, tools=None): + registry = getattr(tools, 'registry', None) + if registry is not None and name in registry.specs: + return registry.get_spec(name) + from openkyrozen.tools.catalog import builtin_metadata + return builtin_metadata(name) + + +def tool_risk(name, tools=None): + policy = tool_policy(name,tools) + return policy.risk if hasattr(policy,'risk') else policy['risk'] if policy else 'normal' + + +def tool_confirmation_required(name, surface, tools=None): + if name == 'define_tool': + return True + policy = tool_policy(name,tools) + surfaces = policy.confirmation_surfaces if hasattr(policy,'confirmation_surfaces') else policy.get('confirmation_surfaces',()) if policy else () + return tool_risk(name,tools) == 'high' and surface in surfaces diff --git a/openkyrozen/tools/__init__.py b/openkyrozen/tools/__init__.py index 8d30628..d17b318 100644 --- a/openkyrozen/tools/__init__.py +++ b/openkyrozen/tools/__init__.py @@ -2,3 +2,5 @@ from .models import CommandResult from .adapters import ToolAdapters from openkyrozen.security.tool_policy import allowed_tool_names, resolve_capabilities, tool_capability + +from .manifest import ToolSpec, ToolManifest, ToolRegistry diff --git a/openkyrozen/tools/adapters.py b/openkyrozen/tools/adapters.py index 1673da0..1f0355b 100644 --- a/openkyrozen/tools/adapters.py +++ b/openkyrozen/tools/adapters.py @@ -22,8 +22,25 @@ def __init__(self, root=None): self._BROWSER = BrowserManager(root=str(self._WORKSPACE_ROOT)) self._PROJECT_GRAPH = None self._GITHUB_CLI = None - self.AVAILABLE_TOOLS = {'write_file': self.write_file, 'edit_file': self.edit_file, 'read_file': self.read_file, 'search_files': self.search_files, 'run_cmd': self.run_cmd, 'search_web': self.search_web, 'find_files': self.find_files, 'list_dir': self.list_dir, 'git_clone': self.git_clone, 'git_status': self.git_status, 'git_diff': self.git_diff, 'git_log': self.git_log, 'git_branch': self.git_branch, 'git_add': self.git_add, 'git_commit': self.git_commit, 'git_push': self.git_push, 'git_pull': self.git_pull, 'git_checkout': self.git_checkout, 'git_stash': self.git_stash, 'git_reset': self.git_reset, 'git_show': self.git_show, 'git_remote': self.git_remote, 'execute_terminal_command': self.execute_terminal_command, 'analyze_remote_repo': self.analyze_remote_repo, 'list_tree': self.list_tree, 'read_webpage': self.read_webpage, 'browser_open': self.browser_open, 'browser_snapshot': self.browser_snapshot, 'browser_click': self.browser_click, 'browser_type': self.browser_type, 'browser_close': self.browser_close, 'graph_status': self.graph_status, 'graph_query': self.graph_query, 'graph_explain': self.graph_explain, 'graph_path': self.graph_path, 'graph_refresh': self.graph_refresh, 'github_status': self.github_status, 'github_read': self.github_read, 'github_cli': self.github_cli} - self.AVAILABLE_TOOLS["calculate"] = self.calculate + from .manifest import ToolRegistry + from .catalog import builtin_names + self.tool_registry = ToolRegistry({name:getattr(self,name) for name in builtin_names('adapter')}) + + @property + def AVAILABLE_TOOLS(self): + return self.tool_registry.tools + + @AVAILABLE_TOOLS.setter + def AVAILABLE_TOOLS(self,value): + from .manifest import ToolRegistry + previous = self.tool_registry.snapshot() if hasattr(self,'tool_registry') else {} + registry = ToolRegistry() + for name,function in value.items(): + if name in previous: + registry.register_spec(previous[name].replace(executor=function)) + else: + registry.register(name,function) + self.tool_registry = registry from .calculation import calculate diff --git a/openkyrozen/tools/catalog.py b/openkyrozen/tools/catalog.py new file mode 100644 index 0000000..016f1ad --- /dev/null +++ b/openkyrozen/tools/catalog.py @@ -0,0 +1,137 @@ +"""Single declaration of canonical tool identity, schema and policy metadata.""" +from __future__ import annotations +import copy + +# name, capability, side effects, owner, risk, parallel safety, aliases, conversion fields, argument aliases, confirmation surfaces +_ROWS = ( + ('spawn_agents', 'read', 'orchestration', 'orchestration', 'normal', False, (), (), (), ()), + ('send_subagent', 'read', 'orchestration', 'orchestration', 'normal', False, (), (), (), ()), + ('list_subagents', 'read', 'orchestration', 'orchestration', 'normal', False, (), (), (), ()), + ('wait_subagents', 'read', 'orchestration', 'orchestration', 'normal', False, (), (), (), ()), + ('cancel_subagent', 'read', 'orchestration', 'orchestration', 'normal', False, (), (), (), ()), + ('check_stored_data', 'read', 'read', 'runtime', 'normal', False, ('check_memory',), (), (), ()), + ('search_memory', 'read', 'read', 'runtime', 'normal', False, (), (), (), ()), + ('read_file', 'read', 'read', 'adapter', 'normal', False, (), ('path',), (('path', 'file_path'),), ()), + ('search_files', 'read', 'read', 'adapter', 'normal', False, (), (), (), ()), + ('calculate', 'read', 'none', 'adapter', 'normal', True, (), (), (), ()), + ('discover_tools', 'read', 'read', 'runtime', 'normal', False, (), (), (), ()), + ('list_dir', 'read', 'read', 'adapter', 'normal', False, (), ('path',), (('path', 'file_path'),), ()), + ('list_tree', 'read', 'read', 'adapter', 'normal', False, ('tree',), ('path',), (('path', 'file_path'),), ()), + ('find_files', 'read', 'read', 'adapter', 'normal', False, (), ('pattern', 'directory'), (), ()), + ('git_status', 'read', 'read', 'adapter', 'normal', False, ('status',), (), (), ()), + ('git_diff', 'read', 'read', 'adapter', 'normal', False, ('diff',), (), (), ()), + ('git_log', 'read', 'read', 'adapter', 'normal', False, ('log',), (), (), ()), + ('git_show', 'read', 'read', 'adapter', 'normal', False, ('show',), (), (), ()), + ('write_file', 'write', 'mutation', 'adapter', 'normal', False, ('write',), ('path', 'content'), (('path', 'file_path'), ('content', 'text')), ()), + ('edit_file', 'write', 'mutation', 'adapter', 'normal', False, (), (), (), ()), + ('run_cmd', 'shell', 'unknown', 'adapter', 'normal', False, ('bash', 'shell', 'sh', 'run_command', 'cmd', 'exec', 'execute', 'run_shell_command', 'run_shell', 'shell_command', 'execute_shell', 'shell_cmd', 'bash_cmd', 'command', 'run'), ('command',), (('command', 'cmd'),), ()), + ('execute_terminal_command', 'shell', 'unknown', 'adapter', 'normal', False, ('run_terminal_command', 'run_terminal', 'terminal'), ('command',), (('command', 'cmd'),), ()), + ('search_web', 'network', 'external', 'adapter', 'normal', False, (), ('query',), (), ()), + ('read_webpage', 'network', 'external', 'adapter', 'normal', False, ('browse_summary',), ('url',), (), ()), + ('git_clone', 'git', 'mutation', 'adapter', 'high', False, ('clone',), ('url', 'destination'), (), ('tui',)), + ('git_branch', 'git', 'mutation', 'adapter', 'normal', False, ('branch',), (), (), ()), + ('analyze_remote_repo', 'network', 'external', 'adapter', 'normal', False, (), ('url',), (), ()), + ('browser_open', 'browser', 'stateful', 'adapter', 'normal', False, (), ('url',), (), ()), + ('browser_snapshot', 'browser', 'stateful', 'adapter', 'normal', False, (), ('session_id',), (('session_id', 'sessionId'),), ()), + ('browser_click', 'browser', 'stateful', 'adapter', 'normal', False, (), ('session_id', 'selector'), (('session_id', 'sessionId'),), ()), + ('browser_type', 'browser', 'stateful', 'adapter', 'normal', False, (), ('session_id', 'selector', 'text'), (('session_id', 'sessionId'),), ()), + ('browser_close', 'browser', 'stateful', 'adapter', 'normal', False, (), ('session_id',), (('session_id', 'sessionId'),), ()), + ('git_add', 'git', 'mutation', 'adapter', 'high', False, ('add',), (), (), ('tui',)), + ('git_commit', 'git', 'mutation', 'adapter', 'high', False, ('commit',), (), (), ('tui',)), + ('git_push', 'git', 'mutation', 'adapter', 'high', False, ('push',), (), (), ('cli', 'tui')), + ('git_pull', 'git', 'mutation', 'adapter', 'high', False, ('pull',), (), (), ('cli', 'tui')), + ('git_checkout', 'git', 'mutation', 'adapter', 'high', False, ('checkout',), (), (), ('cli', 'tui')), + ('git_stash', 'git', 'mutation', 'adapter', 'high', False, ('stash',), (), (), ('cli', 'tui')), + ('git_reset', 'destructive', 'mutation', 'adapter', 'high', False, ('reset',), (), (), ('cli', 'tui')), + ('git_remote', 'git', 'mutation', 'adapter', 'high', False, ('remote',), (), (), ('cli', 'tui')), + ('graph_status', 'read', 'read', 'adapter', 'normal', False, (), (), (), ()), + ('graph_query', 'read', 'read', 'adapter', 'normal', False, (), (), (), ()), + ('graph_explain', 'read', 'read', 'adapter', 'normal', False, (), (), (), ()), + ('graph_path', 'read', 'read', 'adapter', 'normal', False, (), (), (), ()), + ('graph_refresh', 'read', 'internal_write', 'adapter', 'normal', False, (), (), (), ()), + ('github_status', 'network', 'external', 'adapter', 'normal', False, (), (), (), ()), + ('github_read', 'network', 'external', 'adapter', 'normal', False, (), (), (), ()), + ('github_cli', 'git', 'mutation', 'adapter', 'high', False, (), (), (), ('cli', 'tui')), +) + +_SCHEMAS = {'read_file': {'properties': {'path': {'type': 'string'}}, + 'required': ['path'], + 'type': 'object', + 'additionalProperties': False}, + 'list_dir': {'properties': {'path': {'type': 'string'}}, 'type': 'object', 'additionalProperties': False}, + 'list_tree': {'properties': {'path': {'type': 'string'}}, 'type': 'object', 'additionalProperties': False}, + 'find_files': {'properties': {'pattern': {'type': 'string'}, 'directory': {'type': 'string'}}, + 'required': ['pattern'], + 'type': 'object', + 'additionalProperties': False}, + 'write_file': {'properties': {'path': {'type': 'string'}, 'content': {'type': 'string'}}, + 'required': ['path', 'content'], + 'type': 'object', + 'additionalProperties': False}, + 'run_cmd': {'properties': {'command': {'type': 'string'}}, + 'required': ['command'], + 'type': 'object', + 'additionalProperties': False}, + 'execute_terminal_command': {'properties': {'command': {'type': 'string'}}, + 'required': ['command'], + 'type': 'object', + 'additionalProperties': False}, + 'search_web': {'properties': {'query': {'type': 'string'}}, + 'required': ['query'], + 'type': 'object', + 'additionalProperties': False}, + 'read_webpage': {'properties': {'url': {'type': 'string'}}, + 'required': ['url'], + 'type': 'object', + 'additionalProperties': False}, + 'git_clone': {'properties': {'url': {'type': 'string'}, 'destination': {'type': 'string'}}, + 'required': ['url'], + 'type': 'object', + 'additionalProperties': False}, + 'analyze_remote_repo': {'properties': {'url': {'type': 'string'}}, + 'required': ['url'], + 'type': 'object', + 'additionalProperties': False}, + 'browser_open': {'properties': {'url': {'type': 'string'}}, + 'required': ['url'], + 'type': 'object', + 'additionalProperties': False}, + 'browser_snapshot': {'properties': {'session_id': {'type': 'string'}}, + 'required': ['session_id'], + 'type': 'object', + 'additionalProperties': False}, + 'browser_click': {'properties': {'session_id': {'type': 'string'}, 'selector': {'type': 'string'}}, + 'required': ['session_id', 'selector'], + 'type': 'object', + 'additionalProperties': False}, + 'browser_type': {'properties': {'session_id': {'type': 'string'}, + 'selector': {'type': 'string'}, + 'text': {'type': 'string'}}, + 'required': ['session_id', 'selector', 'text'], + 'type': 'object', + 'additionalProperties': False}, + 'browser_close': {'properties': {'session_id': {'type': 'string'}}, + 'required': ['session_id'], + 'type': 'object', + 'additionalProperties': False}} + + +def builtin_names(owner=None): + return tuple(row[0] for row in _ROWS if owner is None or row[3] == owner) + + +def builtin_metadata(name): + for row in _ROWS: + if row[0] != name: + continue + _, capability, effects, _, risk, parallel, aliases, legacy_fields, argument_aliases, surfaces = row + schema = _SCHEMAS.get(name, {'type':'object','properties':{'args':{'type':'string'}},'additionalProperties':False, + 'description':'Plain-string arguments can be supplied as the `args` property.'}) + return dict(capability=capability,side_effects=effects,risk=risk,parallel_safe=parallel,aliases=aliases, + input_schema=copy.deepcopy(schema),legacy_fields=legacy_fields,legacy_argument_aliases=argument_aliases, + confirmation_surfaces=surfaces) + return None + + +def builtin_aliases(): + return {alias:row[0] for row in _ROWS for alias in row[6]} diff --git a/openkyrozen/tools/dynamic.py b/openkyrozen/tools/dynamic.py index 41298f6..feffde5 100644 --- a/openkyrozen/tools/dynamic.py +++ b/openkyrozen/tools/dynamic.py @@ -1,5 +1,7 @@ from __future__ import annotations +import hashlib +import json import re from typing import Any from openkyrozen.app.config import AgentConfigError, effective_capabilities, load_agent_config @@ -10,6 +12,11 @@ def _record_dynamic_tool_event(self, event_type: str, name: str, *, reason: str description: str = "") -> None: """Record a bounded dynamic-tool decision without persisting source code.""" payload = {"name": str(name)[:80] or ""} + if event_type == 'tool.registered' and name in self.tool_registry.specs: + spec = self.tool_registry.get_spec(name) + payload.update(capability=spec.capability,risk=spec.risk,version=spec.version,source=spec.source, + visibility=spec.visibility,parallel_safe=spec.parallel_safe,side_effects=spec.side_effects, + schema_sha256=hashlib.sha256(json.dumps(spec.input_schema,sort_keys=True).encode()).hexdigest()) if reason: payload["reason"] = str(reason)[:300] if description: @@ -47,7 +54,7 @@ def _register_tool(self, name: str, code: str, description: str = "") -> bool: return self._reject_dynamic_tool(name, "tool name must be a Python identifier") # Don't overwrite built-in or already registered tools. - if name in self.AVAILABLE_TOOLS: + if name in self.tool_registry.specs or name in self.tool_registry.aliases: return self._reject_dynamic_tool(name, "tool name is already registered") valid, reason = validate_tool_source(code, name) @@ -72,7 +79,10 @@ def _register_tool(self, name: str, code: str, description: str = "") -> bool: return self._reject_dynamic_tool(name, "dynamic-tool approval was denied") # Register - self.AVAILABLE_TOOLS[name] = fn + try: + self.tool_registry.register(name,fn,capability='dynamic',source='dynamic') + except (TypeError,ValueError) as exc: + return self._reject_dynamic_tool(name,f'registry rejected registration: {type(exc).__name__}') # Rebuild tools list for system prompt self.TOOLS_LIST = self._build_tools_list() diff --git a/openkyrozen/tools/manifest.py b/openkyrozen/tools/manifest.py index 2854866..b44aae0 100644 --- a/openkyrozen/tools/manifest.py +++ b/openkyrozen/tools/manifest.py @@ -1,13 +1,83 @@ -"""Versioned tool catalog used by runtime and sub-agents.""" - +"""Authoritative tool contracts bound to one workspace/runtime.""" from __future__ import annotations -from dataclasses import asdict, dataclass +import json +import re +from collections.abc import MutableMapping +from dataclasses import dataclass, fields +from types import MappingProxyType from typing import Any, Callable from openkyrozen.security.capabilities import CapabilityToken +def _json_value(value): + if isinstance(value, dict): + if any(not isinstance(key, str) for key in value): + raise ValueError('Schema object keys must be strings') + for item in value.values(): + _json_value(item) + elif isinstance(value, list): + for item in value: + _json_value(item) + elif value is not None and type(value) not in {str, bool, int, float}: + raise ValueError('Schema values must be JSON values') + + +def _schema_shape(schema): + if not isinstance(schema, dict): + raise ValueError('Schema must be an object') + supported = {'type','properties','required','additionalProperties','items','enum','anyOf','oneOf','allOf', + 'description','title','default','examples','minLength','maxLength','minItems','maxItems', + 'minProperties','maxProperties','minimum','maximum','exclusiveMinimum','exclusiveMaximum', + 'multipleOf','pattern','format','uniqueItems'} + if set(schema) - supported: + raise ValueError('Unsupported schema keyword') + for key in ('description','title','pattern','format'): + if key in schema and not isinstance(schema[key],str): + raise ValueError(f'{key} must be a string') + if 'pattern' in schema: + try: + re.compile(schema['pattern']) + except re.error as exc: + raise ValueError('Invalid schema pattern') from exc + for key in ('minLength','maxLength','minItems','maxItems','minProperties','maxProperties'): + if key in schema and (type(schema[key]) is not int or schema[key]<0): + raise ValueError(f'{key} must be a nonnegative integer') + for key in ('minimum','maximum','exclusiveMinimum','exclusiveMaximum','multipleOf'): + if key in schema and (type(schema[key]) not in {int,float} or (key=='multipleOf' and schema[key]<=0)): + raise ValueError(f'Invalid {key} constraint') + if 'uniqueItems' in schema and type(schema['uniqueItems']) is not bool: + raise ValueError('uniqueItems must be a boolean') + if 'additionalProperties' in schema and type(schema['additionalProperties']) is not bool: + raise ValueError('additionalProperties must be a boolean') + kinds = {'string','integer','number','boolean','object','array','null'} + kind = schema.get('type') + if kind is not None and (not isinstance(kind,str) or kind not in kinds): + raise ValueError('Invalid schema type') + if 'examples' in schema and not isinstance(schema['examples'],list): + raise ValueError('Schema examples must be an array') + if 'enum' in schema and (not isinstance(schema['enum'],list) or not schema['enum']): + raise ValueError('Schema enum must be a nonempty array') + properties = schema.get('properties',{}) + if not isinstance(properties,dict): + raise ValueError('Schema properties must be an object') + required = schema.get('required',[]) + if not isinstance(required,list) or any(not isinstance(k,str) or k not in properties for k in required) or len(set(required))!=len(required): + raise ValueError('Invalid required tool properties') + for item in properties.values(): + _schema_shape(item) + if 'items' in schema: + _schema_shape(schema['items']) + for keyword in ('anyOf','oneOf','allOf'): + if keyword in schema: + if not isinstance(schema[keyword],list) or not schema[keyword]: + raise ValueError(f'{keyword} must be a nonempty schema array') + for item in schema[keyword]: + _schema_shape(item) + + + @dataclass(frozen=True) class ToolManifest: name: str @@ -17,30 +87,216 @@ class ToolManifest: source: str = "builtin" +@dataclass(frozen=True, init=False) +class ToolSpec: + name: str + description: str + capability: str + side_effects: str + parallel_safe: bool + executor: Callable[..., Any] + visibility: str + aliases: tuple[str, ...] + risk: str + version: str + source: str + legacy_fields: tuple[str, ...] + legacy_argument_aliases: tuple[tuple[str, str], ...] + confirmation_surfaces: tuple[str, ...] + _schema_json: str + + def __init__(self, name, executor, *, description, input_schema, capability, + side_effects, parallel_safe=False, visibility="public", aliases=(), + risk="normal", version="1.0.0", source="builtin", legacy_fields=(), + legacy_argument_aliases=(), confirmation_surfaces=('cli','tui')): + for label, value in (("name",name),("description",description),("version",version),("source",source)): + if not isinstance(value,str) or not value.strip(): + raise ValueError(f"ToolSpec {label} must be a nonempty string") + if not name.isidentifier() or not callable(executor): + raise ValueError("ToolSpec requires an identifier and callable executor") + if capability not in {"read","write","shell","network","git","browser","destructive","dynamic"}: + raise ValueError("Unknown tool capability") + if side_effects not in {"none","read","mutation","external","stateful","internal_write","orchestration","unknown"}: + raise ValueError("Unknown tool side-effect class") + if type(parallel_safe) is not bool or visibility not in {"public","hidden"} or risk not in {"normal","high"}: + raise ValueError("Invalid tool policy metadata") + if parallel_safe and side_effects not in {"none","read"}: + raise ValueError("Stateful tools cannot declare parallel safety") + try: + _json_value(input_schema) + _schema_shape(input_schema) + schema_json=json.dumps(input_schema,allow_nan=False,sort_keys=True) + schema=json.loads(schema_json) + except (TypeError,ValueError,RecursionError) as exc: + raise ValueError("Tool schema must be JSON-safe") from exc + if not isinstance(schema,dict) or schema.get('type')!='object' or not isinstance(schema.get('properties'),dict): + raise ValueError("Tool schema must describe an object with properties") + required=schema.get('required',[]) + if not isinstance(required,list) or any(not isinstance(k,str) or k not in schema['properties'] for k in required) or len(set(required))!=len(required): + raise ValueError("Invalid required tool properties") + if schema.get('additionalProperties') is not False: + raise ValueError("Tool schema must reject additional properties") + for key,value in schema['properties'].items(): + if not isinstance(value,dict) or value.get('type') not in {'string','integer','number','boolean','object','array','null'}: + raise ValueError("Invalid tool property schema") + if isinstance(aliases,str) or isinstance(legacy_fields,str) or isinstance(legacy_argument_aliases,str): + raise ValueError('Alias and conversion metadata must be collections') + alias_values=tuple(aliases) + if any(not isinstance(a,str) or not a.isidentifier() or a==name for a in alias_values) or len(set(alias_values))!=len(alias_values): + raise ValueError("Invalid tool aliases") + legacy_values=tuple(legacy_fields) + argument_aliases=tuple(tuple(pair) for pair in legacy_argument_aliases) + if any(field not in schema['properties'] for field in legacy_values): + raise ValueError("Legacy conversion fields must match schema") + if len(set(argument_aliases)) != len(argument_aliases) or any(len(pair)!=2 or pair[0] not in legacy_values or not isinstance(pair[1],str) or pair[1] in legacy_values for pair in argument_aliases): + raise ValueError("Invalid legacy argument aliases") + confirmation_surfaces=tuple(confirmation_surfaces) + if any(s not in {'cli','tui'} for s in confirmation_surfaces): + raise ValueError('Invalid confirmation surface') + values=locals() + for field in fields(self): + value = schema_json if field.name=='_schema_json' else alias_values if field.name=='aliases' else legacy_values if field.name=='legacy_fields' else argument_aliases if field.name=='legacy_argument_aliases' else values[field.name] + object.__setattr__(self,field.name,value) + + @property + def input_schema(self): + return json.loads(self._schema_json) + + @property + def manifest(self): + return ToolManifest(self.name,self.capability,self.risk,self.version,self.source) + + def replace(self, **changes): + values={field.name:getattr(self,field.name) for field in fields(self) if field.name!='_schema_json'} + return ToolSpec(**(values|{'input_schema':self.input_schema}|changes)) + + def legacy_arguments(self, arguments): + if arguments is None: + return '' + if isinstance(arguments,str): + return arguments + if not isinstance(arguments,dict): + raise ValueError('arguments must be an object or plain string') + if not arguments: + return '' + if set(arguments)=={'args'}: + if not isinstance(arguments['args'],str): + raise ValueError('arguments.args must be a string') + return arguments['args'] + if not self.legacy_fields: + raise ValueError(f"tool '{self.name}' accepts object arguments only as {{'args': ''}}") + schema=self.input_schema + values=[] + for field in self.legacy_fields: + names=(field,)+tuple(alias for canonical,alias in self.legacy_argument_aliases if canonical==field) + present=next((name for name in names if name in arguments),None) + if present is None: + if field in schema.get('required',[]): + raise ValueError(f'missing required argument: {field}') + values.append('') + else: + if not isinstance(arguments[present],str): + raise ValueError(f"argument '{present}' must be a string") + values.append(arguments[present]) + if self.name in {'find_files','git_clone'} and not values[-1]: + values.pop() + return '|'.join(values) + + +class ToolMapping(MutableMapping): + def __init__(self, registry, excluded=()): + self.registry=registry + self.excluded=frozenset(excluded) + def __getitem__(self,name): + if name in self.excluded: + raise KeyError(name) + return self.registry.get_spec(name).executor + def __setitem__(self,name,function): + if name in self.excluded: + raise PermissionError('Tool is excluded from this view') + self.registry.register(name,function) + def __delitem__(self,name): + if name in self.excluded: + raise KeyError(name) + self.registry.unregister(name) + def __iter__(self): + return iter(tuple(name for name in self.registry._specs if name not in self.excluded)) + def __len__(self): + return sum(1 for _ in self) + + class ToolRegistry: - def __init__(self, tools: dict[str, Callable[..., Any]] | None = None): - self.tools = tools if tools is not None else {} - self.manifests: dict[str, ToolManifest] = {} - - def register(self, name: str, function: Callable[..., Any], *, capability: str = "dynamic", - risk: str = "normal", version: str = "1.0.0", source: str = "builtin") -> None: - self.tools[name] = function - self.manifests[name] = ToolManifest(name, capability, risk, version, source) - - def catalog(self, token: CapabilityToken | None = None) -> list[dict[str, Any]]: - result = [] - for name, function in sorted(self.tools.items()): - manifest = self.manifests.get(name, ToolManifest(name, "dynamic")) - if token is not None and not token.allows(manifest.capability): - continue - result.append(asdict(manifest) | {"description": (function.__doc__ or "").strip().split("\n")[0]}) - return result - - def invoke(self, name: str, args: Any, token: CapabilityToken) -> Any: - function = self.tools.get(name) - if function is None: - raise KeyError(f"Unknown tool: {name}") - manifest = self.manifests.get(name, ToolManifest(name, "dynamic")) - if not token.allows(manifest.capability): - raise PermissionError(f"Capability '{manifest.capability}' is not granted by token") - return function(args) + def __init__(self, tools=None): + self._specs={} + self.tools=ToolMapping(self) + for name,executor in (tools or {}).items(): + self.register(name,executor) + + @property + def specs(self): + return MappingProxyType(self._specs) + + @property + def manifests(self): + return MappingProxyType({name:spec.manifest for name,spec in self._specs.items()}) + + @property + def aliases(self): + return {alias:name for name,spec in self._specs.items() for alias in spec.aliases} + + def get_spec(self,name): + return self._specs[name] + + def register_spec(self,spec,*,replace=False): + if not isinstance(spec,ToolSpec): + raise TypeError('Registration requires ToolSpec') + if spec.name in self._specs and not replace: + raise ValueError(f'Tool already registered: {spec.name}') + occupied=set(self._specs)|set(self.aliases) + previous=self._specs.get(spec.name) + if previous: + occupied-=set(previous.aliases)|{previous.name} + if spec.name in occupied or any(alias in occupied for alias in spec.aliases): + raise ValueError('Tool name or alias collision') + self._specs[spec.name]=spec + + def register(self,name,function,*,capability=None,risk=None,version=None,source=None): + from .catalog import builtin_metadata + old=self._specs.get(name) + if old: + updates={'executor':function} + updates.update({k:v for k,v in {'capability':capability,'risk':risk,'version':version,'source':source}.items() if v is not None}) + spec=old.replace(**updates) + else: + metadata=builtin_metadata(name) or {'capability':'dynamic','side_effects':'unknown','parallel_safe':False, + 'input_schema':{'type':'object','properties':{'args':{'type':'string'}},'additionalProperties':False},'source':'legacy'} + metadata.update({k:v for k,v in {'capability':capability,'risk':risk,'version':version,'source':source}.items() if v is not None}) + spec=ToolSpec(name,function,description=(function.__doc__ or '').strip() or f'Invoke {name}.',**metadata) + self.register_spec(spec,replace=old is not None) + + def unregister(self,name): + del self._specs[name] + + def snapshot(self,names=None): + return {name:spec for name,spec in self._specs.items() if names is None or name in names} + + def catalog(self,token=None,*,names=None): + return [dict(name=s.name,capability=s.capability,risk=s.risk,version=s.version,source=s.source, + description=s.description,side_effects=s.side_effects,parallel_safe=s.parallel_safe,visibility=s.visibility) + for s in self._visible(token,names)] + + def _visible(self,token,names): + return (s for name,s in sorted(self._specs.items()) if s.visibility=='public' and (names is None or name in names) + and (token is None or token.allows(s.capability))) + + def mcp_descriptors(self,token=None,*,names=None): + return [{'name':s.name,'description':s.description.splitlines()[0],'inputSchema':s.input_schema} for s in self._visible(token,names)] + + def provider_schemas(self,token=None,*,names=None): + return [{'name':s.name,'description':s.description.splitlines()[0],'parameters':s.input_schema} for s in self._visible(token,names)] + + def invoke(self,name,args,token): + spec=self.get_spec(name) + if not token.allows(spec.capability): + raise PermissionError(f"Capability '{spec.capability}' is not granted by token") + return spec.executor(args) diff --git a/openkyrozen/tools/ports.py b/openkyrozen/tools/ports.py index 5f6ed8a..e7fffd5 100644 --- a/openkyrozen/tools/ports.py +++ b/openkyrozen/tools/ports.py @@ -1,9 +1,12 @@ """Workspace-bound tool operations required by the executor.""" from typing import Protocol, Callable +from collections.abc import MutableMapping +from .manifest import ToolRegistry from .models import CommandResult class ToolPort(Protocol): - AVAILABLE_TOOLS: dict[str, Callable[[str], str]] + tool_registry: ToolRegistry + AVAILABLE_TOOLS: MutableMapping[str, Callable[[str], str]] def run_command(self, args: str) -> CommandResult: ... def set_workspace_root(self, root: str) -> None: ... def set_project_graph(self, graph) -> None: ... diff --git a/openkyrozen/workspace/binding.py b/openkyrozen/workspace/binding.py index 38996c9..ab362c5 100644 --- a/openkyrozen/workspace/binding.py +++ b/openkyrozen/workspace/binding.py @@ -49,9 +49,6 @@ def _set_workspace_root(self, path: str) -> None: if workspace is None: adapters = self.workspace_factory() adapters.set_workspace_root(root) - adapters.AVAILABLE_TOOLS["check_stored_data"] = self._check_stored_data - adapters.AVAILABLE_TOOLS["search_memory"] = self._search_memory - adapters.AVAILABLE_TOOLS["discover_tools"] = self._discover_tools workspace = WorkspaceState(root, adapters) self._workspaces[root] = workspace if any(session is self.current_session for session in self._sessions.values()): diff --git a/scripts/generate_tool_inventory.py b/scripts/generate_tool_inventory.py index e7c7959..4a1e5dd 100755 --- a/scripts/generate_tool_inventory.py +++ b/scripts/generate_tool_inventory.py @@ -4,7 +4,6 @@ from __future__ import annotations import argparse -import inspect import re import sys from pathlib import Path @@ -21,12 +20,14 @@ def _load_runtime() -> tuple[dict[str, Any], Any, Any]: from openkyrozen.memory.retrieval import _check_stored_data, _search_memory from openkyrozen.interfaces.web.service import WebService tools = ToolAdapters().AVAILABLE_TOOLS - tools.update(check_stored_data=_check_stored_data, search_memory=_search_memory) from openkyrozen.agent import delegation_runtime - from openkyrozen.agent.delegation import TOOLS - tools.update({name: getattr(delegation_runtime, name) for name in TOOLS}) from openkyrozen.agent.prompts import _discover_tools - tools["discover_tools"] = _discover_tools + from openkyrozen.tools.catalog import builtin_names + runtime_functions = {'check_stored_data':_check_stored_data, 'search_memory':_search_memory, 'discover_tools':_discover_tools} + for name in builtin_names('runtime'): + tools[name] = runtime_functions[name] + for name in builtin_names('orchestration'): + tools[name] = getattr(delegation_runtime,name) return tools, WebService(), tool_capability @@ -41,17 +42,12 @@ def runtime_routes(server: Any) -> list[tuple[str, str]]: return sorted(rows, key=lambda row: (row[1], row[0])) -def _first_doc_line(function: Any) -> str: - doc = inspect.getdoc(function) or "No description provided." - return doc.splitlines()[0].strip() - - def _escape_table(value: str) -> str: return re.sub(r"\s+", " ", value.replace("|", "\\|")) def _mcp_contract(server: Any, name: str, function: Any) -> str: - schema = server._mcp_tool_schema(name, function) + schema = function.input_schema properties = schema.get("properties", {}) if not properties: return "none" @@ -71,7 +67,7 @@ def render_inventory() -> str: "# OpenKyrozen runtime inventory", "", "This file is generated from the live runtime registry (`AgentRuntime.AVAILABLE_TOOLS`),", - "the FastAPI route table (`server.app.routes`), and the MCP schema builder.", + "ToolSpec metadata and the FastAPI route table (`server.app.routes`).", "Run `make docs-check` after changing a tool, endpoint, or MCP contract.", "", "## Runtime tools", @@ -81,13 +77,13 @@ def render_inventory() -> str: "- The CLI Action contract is a plain string in the `args` field.", "- Web and MCP exposure is filtered by the configured capability profile.", "", - "| Tool | Capability | Plain-string Action contract | MCP object input schema |", - "|---|---|---|---|", + "| Tool | Capability | Side effects | Parallel safe | Plain-string Action contract | MCP object input schema |", + "|---|---|---|---|---|---|", ] for name in sorted(tools): - function = tools[name] + function = tools.registry.get_spec(name) lines.append( - f"| `{name}` | `{tool_capability(name)}` | {_escape_table(_first_doc_line(function))} " + f"| `{name}` | `{function.capability}` | `{function.side_effects}` | {str(function.parallel_safe).lower()} | {_escape_table(function.description.splitlines()[0])} " f"| {_escape_table(_mcp_contract(server, name, function))} |" ) diff --git a/tests/test_tool_registry.py b/tests/test_tool_registry.py new file mode 100644 index 0000000..2cf9c28 --- /dev/null +++ b/tests/test_tool_registry.py @@ -0,0 +1,189 @@ +import unittest +from openkyrozen.tools.manifest import ToolRegistry +from openkyrozen.tools import ToolAdapters + +class RegistryRegressions(unittest.TestCase): + def test_every_adapter_has_complete_spec(self): + adapters = ToolAdapters() + self.assertEqual(len(adapters.AVAILABLE_TOOLS),40) + for name, executor in adapters.AVAILABLE_TOOLS.items(): + spec = adapters.tool_registry.get_spec(name) + self.assertIs(spec.executor,executor) + self.assertTrue(spec.description) + self.assertEqual(spec.input_schema['type'],'object') + self.assertTrue(spec.capability) + self.assertTrue(spec.side_effects) + self.assertIsInstance(spec.parallel_safe,bool) + + def test_compatibility_registration_has_restrictive_metadata(self): + adapters = ToolAdapters() + adapters.AVAILABLE_TOOLS['new_helper'] = lambda args: args + spec = adapters.tool_registry.get_spec('new_helper') + self.assertEqual(spec.capability,'dynamic') + self.assertFalse(spec.parallel_safe) + self.assertEqual(spec.input_schema['properties'],{'args':{'type':'string'}}) + + def test_mcp_provider_schemas_cannot_drift_or_mutate_catalog(self): + adapters = ToolAdapters() + registry = adapters.tool_registry + mcp = {s['name']:s for s in registry.mcp_descriptors()} + provider = {s['name']:s for s in registry.provider_schemas()} + self.assertEqual(mcp['write_file']['inputSchema'],provider['write_file']['parameters']) + mcp['write_file']['inputSchema']['properties']['path']['type']='number' + self.assertEqual(registry.get_spec('write_file').input_schema['properties']['path']['type'],'string') + self.assertNotIn('bash',provider) + +class RegistryContractTests(unittest.TestCase): + def make(self,name='custom',**changes): + from openkyrozen.tools import ToolSpec + values=dict(description='Custom helper.',input_schema={'type':'object','properties':{'args':{'type':'string'}},'additionalProperties':False}, + capability='read',side_effects='read',parallel_safe=False,aliases=('helper_alias',),source='test',version='2.0.0') + values.update(changes) + return ToolSpec(name,lambda args:args,**values) + + def test_collision_and_invalid_registration_are_atomic(self): + registry=ToolRegistry() + spec=self.make() + registry.register_spec(spec) + for conflicting in (spec,self.make('helper_alias',aliases=()),self.make('other')): + with self.assertRaises(ValueError): + registry.register_spec(conflicting) + self.assertEqual(dict(registry.specs),{'custom':spec}) + for change in ({'capability':'admin'},{'parallel_safe':True,'side_effects':'stateful'}, + {'input_schema':{'type':'object','properties':{},'required':['missing'],'additionalProperties':False}}, + {'input_schema':{'type':'object','properties':{},'additionalProperties':False,'default':float('nan')}}): + with self.assertRaises(ValueError): + self.make(**change) + + def test_snapshot_restore_preserves_complete_metadata(self): + registry=ToolRegistry() + spec=self.make(visibility='hidden',risk='high') + registry.register_spec(spec) + snapshot=registry.snapshot() + registry.tools['custom']=lambda args:'changed' + self.assertEqual(registry.get_spec('custom').manifest,spec.manifest) + del registry.tools['custom'] + registry.register_spec(snapshot['custom']) + self.assertIs(registry.get_spec('custom'),spec) + self.assertEqual(registry.mcp_descriptors(),[]) + + def test_specs_are_immutable_including_schema_and_aliases(self): + from dataclasses import FrozenInstanceError + schema={'type':'object','properties':{'args':{'type':'string'}},'additionalProperties':False} + spec=self.make(input_schema=schema) + schema['properties']['args']['type']='number' + self.assertEqual(spec.input_schema['properties']['args']['type'],'string') + with self.assertRaises(FrozenInstanceError): + spec.capability='dynamic' + + def test_schema_conversion_examples_and_legacy_aliases(self): + registry=ToolAdapters().tool_registry + cases={'read_file':({'file_path':'a'},'a'),'write_file':({'path':'a','content':'x|y'},'a|x|y'), + 'find_files':({'pattern':'*.py'},'*.py'),'run_cmd':({'cmd':'echo hi'},'echo hi'), + 'browser_type':({'sessionId':'s','selector':'x','text':'y'},'s|x|y')} + for name,(arguments,expected) in cases.items(): + self.assertEqual(registry.get_spec(name).legacy_arguments(arguments),expected) + with self.assertRaises(ValueError): + registry.get_spec('read_file').legacy_arguments({'path':1}) + +class RuntimeCatalogTests(unittest.TestCase): + def test_all48_bound_tools_and_workspace_isolation(self): + import tempfile + from pathlib import Path + from unittest.mock import patch + from openkyrozen.app.bootstrap import build_application,build_memory + with tempfile.TemporaryDirectory() as home,patch.dict('os.environ',{'KYROZEN_DISABLE_VECTOR_INDEX':'1'}): + app=build_application(memory=build_memory(Path(home)/'state.sqlite3')) + try: + runtime=app.runtime + self.assertEqual(len(runtime.AVAILABLE_TOOLS),48) + self.assertEqual(set(runtime.AVAILABLE_TOOLS),set(runtime.tool_registry.specs)) + first=Path(home)/'first';second=Path(home)/'second';first.mkdir();second.mkdir() + runtime._set_workspace_root(first) + original=runtime.tool_registry + runtime.AVAILABLE_TOOLS['private_helper']=lambda args:'first' + runtime._set_workspace_root(second) + self.assertNotIn('private_helper',runtime.AVAILABLE_TOOLS) + self.assertIsNot(runtime.tool_registry,original) + runtime._set_workspace_root(first) + self.assertEqual(runtime.AVAILABLE_TOOLS['private_helper'](''),'first') + runtime.execution_context.child_run_id='child' + self.assertNotIn('spawn_agents',runtime.AVAILABLE_TOOLS) + finally: + app.close() + +class SchemaValidationRegressions(unittest.TestCase): + def test_schema_rejects_non_json_keys_and_bad_nested_keywords(self): + helper=RegistryContractTests() + for schema in ({'type':'object','properties':{1:{'type':'string'}},'additionalProperties':False}, + {'type':'object','properties':{'args':{'type':'string','enum':[]}},'additionalProperties':False}, + {'type':'object','properties':{'args':{'type':'array','items':{'type':'invalid'}}},'additionalProperties':False}): + with self.subTest(schema=schema),self.assertRaises(ValueError): + helper.make(input_schema=schema) + + def test_alias_collections_and_legacy_fields_have_explicit_types(self): + helper=RegistryContractTests() + for change in ({'aliases':'xyz'},{'legacy_fields':'args'},{'legacy_argument_aliases':(('args','args'),)}, + {'legacy_argument_aliases':(('args','x'),('args','x'))}): + with self.subTest(change=change),self.assertRaises(ValueError): + helper.make(legacy_fields=('args',),**change) if 'legacy_fields' not in change else helper.make(**change) + +class PolicyMetadataRegressions(unittest.TestCase): + def test_custom_risk_metadata_reaches_permission_gate(self): + from openkyrozen.security.permission_gate import risk_category + registry=ToolRegistry() + registry.register_spec(RegistryContractTests().make(risk='high')) + self.assertEqual(risk_category('custom','',registry.tools),'external_or_irreversible_change') + +class ViewAuthorizationRegressions(unittest.TestCase): + def test_filtered_mapping_cannot_overwrite_hidden_executor(self): + from openkyrozen.tools.manifest import ToolMapping + registry=ToolRegistry({'spawn_agents':lambda args:'original'}) + child=ToolMapping(registry,excluded=('spawn_agents',)) + with self.assertRaises(PermissionError): + child['spawn_agents']=lambda args:'overwritten' + self.assertEqual(registry.tools['spawn_agents'](''),'original') + +class ReviewRegressions(unittest.TestCase): + def test_invalid_constraints_and_unsupported_keywords_rejected(self): + helper=RegistryContractTests() + for prop in ({'type':'string','minLength':-1},{'type':'string','pattern':42}, + {'type':'string','unexpected':'value'},{'type':'string','minLength':True},{'type':'string','examples':42}): + with self.subTest(prop=prop),self.assertRaises(ValueError): + helper.make(input_schema={'type':'object','properties':{'args':prop},'additionalProperties':False}) + + def test_alias_collision_is_rejected_before_dynamic_approval(self): + from unittest.mock import Mock,patch + from types import SimpleNamespace as NS + from openkyrozen.tools.dynamic import _register_tool + from openkyrozen.security.capabilities import issue_capability_token + registry=ToolAdapters().tool_registry + rejected=Mock(return_value=False) + approved=Mock(return_value=True) + runtime=NS(ALLOW_DYNAMIC_TOOLS=True,tool_registry=registry,AVAILABLE_TOOLS=registry.tools, + _get_workspace_root=lambda:'.',_execution_capability_token=issue_capability_token('test',{'dynamic'}), + _reject_dynamic_tool=rejected,_confirm_tool_action=approved) + with patch('openkyrozen.tools.dynamic.effective_capabilities',return_value={'dynamic'}),patch('openkyrozen.tools.dynamic.load_agent_config'): + self.assertFalse(_register_tool(runtime,'bash','def bash(args):\n return args')) + rejected.assert_called_once() + approved.assert_not_called() + +class LiveInventoryRegressions(unittest.TestCase): + def test_registered_names_and_aliases_immediately_parse(self): + import tempfile + from pathlib import Path + from unittest.mock import patch + from openkyrozen.app.bootstrap import build_application,build_memory + with tempfile.TemporaryDirectory() as home,patch.dict('os.environ',{'KYROZEN_DISABLE_VECTOR_INDEX':'1'}): + app=build_application(memory=build_memory(Path(home)/'state.sqlite3')) + try: + runtime=app.runtime + spec=RegistryContractTests().make() + runtime.tool_registry.register_spec(spec) + for name in ('custom','helper_alias'): + calls=runtime._collect_tool_calls(name+': hello') + self.assertEqual(calls,[{'action':'custom','args':'hello'}]) + filter=runtime.DeepSeekDSMLFilter() + self.assertNotIn('helper_alias:',filter.feed('helper_alias: hello',final=True)) + finally: + app.close() From 0807549c00301635fd9a644de99bee4e837b5299 Mon Sep 17 00:00:00 2001 From: Evan Date: Sat, 10 Oct 2026 08:36:56 +0800 Subject: [PATCH 2/2] fix: preserve registry identity and removal semantics --- .../plans/2026-10-09-v3-issue-232.md | 4 +- openkyrozen/agent/protocol.py | 6 +-- openkyrozen/agent/runtime.py | 10 +++-- openkyrozen/tools/adapters.py | 1 + openkyrozen/tools/manifest.py | 20 ++++++--- tests/test_tool_registry.py | 44 +++++++++++++++++++ 6 files changed, 70 insertions(+), 15 deletions(-) diff --git a/docs/superpowers/plans/2026-10-09-v3-issue-232.md b/docs/superpowers/plans/2026-10-09-v3-issue-232.md index f1f95a0..128b7b8 100644 --- a/docs/superpowers/plans/2026-10-09-v3-issue-232.md +++ b/docs/superpowers/plans/2026-10-09-v3-issue-232.md @@ -36,4 +36,6 @@ - Dynamic tools retain restrictive capability and existing source-validation/approval/audit gates. - Capabilities do not imply side-effect freedom or parallel safety; browser, graph refresh and orchestration remain nonparallel. -Final local verification: 612 full Python3.12 tests,73 focused Python3.13 integration tests; make check/lint/docs-check/agent/subagent acceptance PASS. Independent review findings reproduced and fixed; final review clear. Unsupported schema vocabulary is rejected explicitly; no paid calls. +Final local verification: 615 full Python3.12 tests,76 focused Python3.13 integration tests; make check/lint/docs-check/agent/subagent acceptance PASS. Independent review findings reproduced and fixed; final review clear. Unsupported schema vocabulary is rejected explicitly; no paid calls. + +PR #273 follow-up: reproduced and resolved four automated-review findings. Runtime-tool deletion survives lookup/replacement; mixed-case canonical names and aliases work through plain/XML/DSML calls; valid combinator schemas register while invalid explicit types reject; legacy pipe conversion requires string fields. Final independent re-review reports no material defects. Final local suite and offline acceptance PASS. diff --git a/openkyrozen/agent/protocol.py b/openkyrozen/agent/protocol.py index 32e8d08..d95b48e 100644 --- a/openkyrozen/agent/protocol.py +++ b/openkyrozen/agent/protocol.py @@ -129,7 +129,7 @@ def _collect_unwrapped_tool_calls(self, text: str) -> tuple[list[dict], bool]: if next_marker is not None and next_marker.start() < line_end: malformed = True continue - raw_action = match.group("name").lower() + raw_action = self.tool_registry.resolve_legacy_name(match.group("name")) calls.append({"action": self.TOOL_ALIASES.get(raw_action, raw_action), "args": args}) return calls, malformed @@ -189,7 +189,7 @@ def _add(data: dict) -> None: ) for invoke in invoke_pattern.finditer(text): parameters = list(parameter_pattern.finditer(invoke.group("body"))) - raw_action = invoke.group("name").strip().lower() + raw_action = self.tool_registry.resolve_legacy_name(invoke.group("name").strip()) if (len(parameters) == 1 and parameters[0].group("name").strip().lower() == "args" and self._is_valid_action(raw_action)): args = html.unescape(parameters[0].group("value").strip()) @@ -206,7 +206,7 @@ def _add(data: dict) -> None: if len(xml_actions) == 1 and not re.search( r"<\s*action\s*>[\s\S]*<\s*action\s*>", text, re.IGNORECASE): xml_action = xml_actions[0] - raw_action = xml_action.group("name").strip().lower() + raw_action = self.tool_registry.resolve_legacy_name(xml_action.group("name").strip()) args = html.unescape(xml_action.group("args").strip()) if args and self._is_valid_action(raw_action): _add({"action": self.TOOL_ALIASES.get(raw_action, raw_action), "args": args}) diff --git a/openkyrozen/agent/runtime.py b/openkyrozen/agent/runtime.py index 2d4238e..2dc483a 100644 --- a/openkyrozen/agent/runtime.py +++ b/openkyrozen/agent/runtime.py @@ -345,10 +345,12 @@ def chat(self, session, message, *, clear_tasks=False, profile=None, memory_cont def tool_registry(self): from openkyrozen.tools.catalog import builtin_names registry = self.current_session.workspace.adapters.tool_registry - for owner in ('runtime','orchestration'): - for name in builtin_names(owner): - if name not in registry.specs: - registry.register(name,getattr(self,('_' if owner=='runtime' else '')+name)) + if not registry.runtime_bindings_initialized: + for owner in ('runtime','orchestration'): + for name in builtin_names(owner): + if name not in registry.specs: + registry.register(name,getattr(self,('_' if owner=='runtime' else '')+name)) + registry.runtime_bindings_initialized=True return registry @property diff --git a/openkyrozen/tools/adapters.py b/openkyrozen/tools/adapters.py index 1f0355b..a652395 100644 --- a/openkyrozen/tools/adapters.py +++ b/openkyrozen/tools/adapters.py @@ -35,6 +35,7 @@ def AVAILABLE_TOOLS(self,value): from .manifest import ToolRegistry previous = self.tool_registry.snapshot() if hasattr(self,'tool_registry') else {} registry = ToolRegistry() + registry.runtime_bindings_initialized = getattr(getattr(self,'tool_registry',None),'runtime_bindings_initialized',False) for name,function in value.items(): if name in previous: registry.register_spec(previous[name].replace(executor=function)) diff --git a/openkyrozen/tools/manifest.py b/openkyrozen/tools/manifest.py index b44aae0..dcbabef 100644 --- a/openkyrozen/tools/manifest.py +++ b/openkyrozen/tools/manifest.py @@ -53,7 +53,7 @@ def _schema_shape(schema): raise ValueError('additionalProperties must be a boolean') kinds = {'string','integer','number','boolean','object','array','null'} kind = schema.get('type') - if kind is not None and (not isinstance(kind,str) or kind not in kinds): + if 'type' in schema and (not isinstance(kind,str) or kind not in kinds): raise ValueError('Invalid schema type') if 'examples' in schema and not isinstance(schema['examples'],list): raise ValueError('Schema examples must be an array') @@ -137,16 +137,16 @@ def __init__(self, name, executor, *, description, input_schema, capability, if schema.get('additionalProperties') is not False: raise ValueError("Tool schema must reject additional properties") for key,value in schema['properties'].items(): - if not isinstance(value,dict) or value.get('type') not in {'string','integer','number','boolean','object','array','null'}: + if not isinstance(value,dict) or (value.get('type') not in {'string','integer','number','boolean','object','array','null'} and not any(key in value for key in ('anyOf','oneOf','allOf'))): raise ValueError("Invalid tool property schema") if isinstance(aliases,str) or isinstance(legacy_fields,str) or isinstance(legacy_argument_aliases,str): raise ValueError('Alias and conversion metadata must be collections') alias_values=tuple(aliases) - if any(not isinstance(a,str) or not a.isidentifier() or a==name for a in alias_values) or len(set(alias_values))!=len(alias_values): + if any(not isinstance(a,str) or not a.isidentifier() or a.casefold()==name.casefold() for a in alias_values) or len({a.casefold() for a in alias_values})!=len(alias_values): raise ValueError("Invalid tool aliases") legacy_values=tuple(legacy_fields) argument_aliases=tuple(tuple(pair) for pair in legacy_argument_aliases) - if any(field not in schema['properties'] for field in legacy_values): + if any(field not in schema['properties'] or schema['properties'][field].get('type') != 'string' for field in legacy_values): raise ValueError("Legacy conversion fields must match schema") if len(set(argument_aliases)) != len(argument_aliases) or any(len(pair)!=2 or pair[0] not in legacy_values or not isinstance(pair[1],str) or pair[1] in legacy_values for pair in argument_aliases): raise ValueError("Invalid legacy argument aliases") @@ -228,6 +228,7 @@ def __len__(self): class ToolRegistry: def __init__(self, tools=None): self._specs={} + self.runtime_bindings_initialized=False self.tools=ToolMapping(self) for name,executor in (tools or {}).items(): self.register(name,executor) @@ -244,6 +245,11 @@ def manifests(self): def aliases(self): return {alias:name for name,spec in self._specs.items() for alias in spec.aliases} + def resolve_legacy_name(self,name): + identities={key.casefold():key for key in self._specs} + identities.update({alias.casefold():canonical for alias,canonical in self.aliases.items()}) + return identities.get(name.casefold(),name) + def get_spec(self,name): return self._specs[name] @@ -252,11 +258,11 @@ def register_spec(self,spec,*,replace=False): raise TypeError('Registration requires ToolSpec') if spec.name in self._specs and not replace: raise ValueError(f'Tool already registered: {spec.name}') - occupied=set(self._specs)|set(self.aliases) + occupied={value.casefold() for value in (set(self._specs)|set(self.aliases))} previous=self._specs.get(spec.name) if previous: - occupied-=set(previous.aliases)|{previous.name} - if spec.name in occupied or any(alias in occupied for alias in spec.aliases): + occupied-={value.casefold() for value in (*previous.aliases,previous.name)} + if spec.name.casefold() in occupied or any(alias.casefold() in occupied for alias in spec.aliases): raise ValueError('Tool name or alias collision') self._specs[spec.name]=spec diff --git a/tests/test_tool_registry.py b/tests/test_tool_registry.py index 2cf9c28..f0bed7d 100644 --- a/tests/test_tool_registry.py +++ b/tests/test_tool_registry.py @@ -187,3 +187,47 @@ def test_registered_names_and_aliases_immediately_parse(self): self.assertNotIn('helper_alias:',filter.feed('helper_alias: hello',final=True)) finally: app.close() + +class GitHubReviewRegressions(unittest.TestCase): + def test_combinator_properties_register_but_nonstring_legacy_fields_do_not(self): + helper=RegistryContractTests() + schema={'type':'object','properties':{'args':{'oneOf':[{'type':'string'},{'type':'number'}]}},'additionalProperties':False} + self.assertEqual(helper.make(input_schema=schema).input_schema,schema) + schema['properties']['args']['type']=None + with self.assertRaises(ValueError): + helper.make(input_schema=schema) + for kind in ('integer','number','boolean','object','array'): + schema={'type':'object','properties':{'args':{'type':kind}},'additionalProperties':False} + with self.subTest(kind=kind),self.assertRaises(ValueError): + helper.make(input_schema=schema,legacy_fields=('args',)) + + def test_case_insensitive_identity_collisions_rejected(self): + registry=ToolRegistry() + registry.register_spec(RegistryContractTests().make('MyTool',aliases=('MyAlias',))) + for name in ('mytool','MYALIAS'): + with self.assertRaises(ValueError): + registry.register_spec(RegistryContractTests().make(name,aliases=())) + + def test_runtime_removal_and_case_preserving_parser(self): + import tempfile + from pathlib import Path + from unittest.mock import patch + from openkyrozen.app.bootstrap import build_application,build_memory + with tempfile.TemporaryDirectory() as home,patch.dict('os.environ',{'KYROZEN_DISABLE_VECTOR_INDEX':'1'}): + app=build_application(memory=build_memory(Path(home)/'state.sqlite3')) + try: + runtime=app.runtime + runtime.tool_registry.register_spec(RegistryContractTests().make('MyTool',aliases=('MyAlias',))) + for spelling in ('MyTool','mytool','MYALIAS'): + for text in (spelling+': hello', '\n'+spelling+'\nhello\n', + '<｜DSML｜invoke name="'+spelling+'"><｜DSML｜parameter name="args">hello'): + with self.subTest(text=text): + self.assertEqual(runtime._collect_tool_calls(text),[{'action':'MyTool','args':'hello'}]) + for name in ('spawn_agents','discover_tools'): + del runtime.AVAILABLE_TOOLS[name] + self.assertNotIn(name,runtime.AVAILABLE_TOOLS) + self.assertNotIn(name,runtime.tool_registry.specs) + runtime.AVAILABLE_TOOLS=dict(runtime.AVAILABLE_TOOLS) + self.assertNotIn('spawn_agents',runtime.AVAILABLE_TOOLS) + finally: + app.close()