diff --git a/ROADMAP.md b/ROADMAP.md index 7a72b49..e0f5474 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -37,7 +37,7 @@ standards register names the rows that ask for them. | 011 | LLM provider layer (req_llm behind `Trinity.LLM` behaviour) | 1 Core loop | M | 010 | approved | | 012 | Session process + agent loop (gen_statem, DynamicSupervisor, rehydration) | 1 Core loop | L | 010, 011 | approved | | 013 | LiveView chat UI with streaming | 1 Core loop | M | 012 | approved | -| 020 | Tool protocol + registry | 2 Tools | M | 012 | planned | +| 020 | Tool protocol + registry | 2 Tools | M | 012 | done | | 021 | Permission gate + approval UI (M2 fingerprint-bound, M7) | 2 Tools | M | 020, 013 | planned | | 022 | Core tools: filesystem, web fetch/search, shell (MuonTrap) | 2 Tools | L | 021 | planned | | 023 | Context compaction + session lineage | 2 Tools | M | 012 | planned | diff --git a/VERSIONS.md b/VERSIONS.md index 85aa401..4c25af3 100644 --- a/VERSIONS.md +++ b/VERSIONS.md @@ -135,6 +135,7 @@ never pin a version hex marks as retired or vulnerable. | `telegex` | **not pinned** | πŸ” not a single package | Telegram. ⚠️ The latest release on hex is a release candidate, roughly two years old, and this file's own rule forbids pinning an `-rc`. Alternative: ex_gram. Slice 071 decides with the measurement. | | `phoenix_streamdown` | not used (measured at Slice 013, 2026-09-20) | πŸ” not a single package | Streaming markdown renderer for LiveView. Measured at Slice 013: 1.0.0-beta.4 (2026-05-03) was still the latest release with no stable behind it, and this file's own rule forbids a pre-release. It is 801 lines over `mdex`, whose own `streaming: true` option completes fragments; the rest is a rendering rule Slice 013 keeps anyway. The row stays so the decision is visible where a reader would look for the package. | | `mdex` | ~> 0.13 | βœ… in `mix.lock` | Markdown renderer for the chat (`TrinityWeb.Markdown`), streaming fragments completed by its `streaming: true` option, raw HTML omitted (`unsafe: false`) and the default sanitizer on top. Added at Slice 013 after the measurement in its NOTES.md: `earmark` 1.4.49 is retired on hex with an open XSS advisory (EEF-CVE-2026-48591), which the gate refuses. ⚠️ A Rust NIF (`mdex_native`): the first in the bundle. Measured at Slice 013 (NOTES finding 13): neither precompiled artifact loads in Burrito's musl ERTS on Linux, so the linux package builds it from source for musl with Zig as the linker (`rustler` below, `scripts/zig-cc-musl`, the three settings in config/config.exs); macOS and Windows load the precompiled artifact. The `--smoke` path prints whether it rendered, and the `package` workflow reads that line on every target. | +| `jsv` | ~> 0.23 | βœ… in `mix.lock` | JSON Schema (2020-12) validation of tool arguments in `Trinity.Tools.Schema`, with `cast: false` so a malformed call is refused and never repaired (docs/07). Was transitive through req_llm; direct since Slice 020 because a module of ours calls it (ADR-0009: Trinity's own tool behaviour, jsv for its schemas). | | `rustler` | ~> 0.38 | βœ… in `mix.lock` | Build time only (`runtime: false`): what `rustler_precompiled` needs to compile `mdex_native` from source when `MDEX_NATIVE_BUILD=1`, which the linux package sets (owner decision 2026-09-20, Slice 013 NOTES finding 14). Nothing in the tree calls it. | ### Dev and quality diff --git a/config/test.exs b/config/test.exs index 29e45de..8e373fa 100644 --- a/config/test.exs +++ b/config/test.exs @@ -7,6 +7,18 @@ import Config # The MIX_TEST_PARTITION environment variable can be used # to provide built-in test partitioning in CI environment. # Run `mix help test` for more information. +# Slice 020: the test tools, and the toolset the core ones belong to. AC1's one config line +# is the `modules:` entry; a tool module in test/support plus this line is the whole change. +config :trinity, :tools, + modules: [ + Trinity.TestTools.Echo, + Trinity.TestTools.Sleep, + Trinity.TestTools.Crash, + Trinity.TestTools.Big + ], + toolsets: %{core: ["echo", "sleep", "crash", "big"]}, + timeout_ms: 2_000 + # Slice 012: sessions hibernate and stop quickly in tests so AC8 is observable in seconds. config :trinity, :sessions, idle_hibernate_ms: 200, idle_stop_ms: 60_000 diff --git a/coverage.tsv b/coverage.tsv index b6d204c..fe89ee2 100644 --- a/coverage.tsv +++ b/coverage.tsv @@ -5,3 +5,4 @@ slice_id percent sha date 011 51.57 ec5334a 2026-09-20 012 60.82 019d9cf 2026-09-20 013 64.41 080c543 2026-09-20 +020 67.18 8a5b7ae 2026-09-20 diff --git a/docs/01-architecture.md b/docs/01-architecture.md index 64c135a..1a223ab 100644 --- a/docs/01-architecture.md +++ b/docs/01-architecture.md @@ -23,7 +23,9 @@ Trinity.Application β”œβ”€β”€ Trinity.Sessions.Supervisor (DynamicSupervisor) # one Trinity.Sessions.Session per conversation. Slice 012, as built β”‚ └── Trinity.Sessions.Session (gen_statem) # states: idle β†’ thinking β†’ tool_wait β†’ approval_wait β†’ compacting β†’ error β”‚ └── Task.Supervisor (started by the Session, linked, unnamed) # the model call and the tool calls of one turn -β”œβ”€β”€ Trinity.Tools.Supervisor # tool runtime (ports, browsers). Slice 020/022 +β”œβ”€β”€ Trinity.Tools.Supervisor # Slice 020, as built: Trinity.Tools.TaskSupervisor (every tool +β”‚ β”‚ # call of a turn runs under it) and Trinity.Tools.Registry +β”‚ β”‚ # (GenServer over ETS). 022 adds the stateful runtimes beside them β”œβ”€β”€ Trinity.Permissions.Gate # approval requests + allowlist cache. Slice 021 β”œβ”€β”€ Trinity.Receipts.Supervisor # Slice 024 β”‚ └── Trinity.Receipts.ChainWriter (one per chain_scope, :unique in Trinity.Registry; ADR-0013) @@ -113,6 +115,17 @@ UI/Gateway ──user_message──▢ Session(gen_statem) Every state transition is persisted before it is broadcast. A crash between persist and broadcast is safe (rehydrate re-broadcasts the last state). +**Tool calls (Slice 020):** at `{:done, :tool_calls}` the Session hands the turn's calls to +`Trinity.Sessions.ToolRunner.run_all/2`, the seam whose implementation in force is `Trinity.Tools.Runner` +(config, so a test can put the stub back). The runner runs every call at once under +`Trinity.Tools.TaskSupervisor`, each with its tool's timeout: lookup, `jsv` validation of the arguments +(refused, never repaired), `Trinity.Permissions.decide/3` once, `execute/2`, the result cap. A crash, a timeout +and an unknown name are error results the model reads; the Session writes one `tool` row per answer with the +tool's definition digest. Each turn's request carries the declared surface (`Trinity.Tools.to_llm_tools/0`) and +the assistant row records it (`provider_meta.tool_surface`); `Trinity.Tools.surface_diff/1` over a history names +the calls a turn made outside it. Sessions depends on Tools; Tools depends on Permissions and never on Sessions +(the runner implements the seam's functions without naming the behaviour, which would close a cycle). + **Effect path (Slice 024):** `Session β†’ Permissions.decide β†’ Effects.execute β†’ Authority β†’ tool.execute/2 (local) or a proposal (external adapter) β†’ Receipts.append`. `Effects` is the only caller of `execute/2` for effectful tools; a census test enforces it. Reads emit query receipts. **The page (Slice 013):** `TrinityWeb.SessionLive.Show` subscribes to `session:` on mount, calls diff --git a/docs/03-conventions.md b/docs/03-conventions.md index a30c398..1e91de7 100644 --- a/docs/03-conventions.md +++ b/docs/03-conventions.md @@ -27,6 +27,21 @@ previous slice fails until a NOTES.md justification names the reason. The rule as originally written stored no baseline, so nothing could check it, which is the pattern CLAUDE.md Β§8 forbids. +## Tools (slice 020) + +- A tool is a module implementing `Trinity.Tools.Tool` (`name/0`, `description/0`, `schema/0` as a JSON Schema + map with string keys, `risk/0`, `effect/0`, `execute/2`, optional `timeout/0` and `format_result/1`) plus one + line in `config :trinity, :tools` (`modules:`; `toolsets:` groups names). No core module changes; the + registry test asserts `git grep TestTools lib/` finds nothing. +- Tool modules are stateless. A runtime with state (a shell, a browser) is a child of `Trinity.Tools.Supervisor` + the tool looks up. +- `effect/0` is `:none` (a read), `:artifact` (a local write) or `:catalog` (an external effect). A `:catalog` + tool is listed in `Trinity.Effects.Catalog`'s module attribute or it does not start; nothing registered at + runtime may claim it. A runtime tool's name is namespaced (`mcp::`, `skill:`); core + names are reserved; the tier map in `Trinity.Permissions` is code and lists core names only. +- A tool returns `{:ok, %Trinity.Tools.Result{}}` or `{:error, reason}`; the runner caps the content at + `result_cap_bytes` (64 KB) with a marker. Arguments arrive validated; a tool never repairs them either. + ## UI (decided at slice 013) - Tokens live in `assets/css/app.css` and nothing else names a colour, a radius or a font: two daisyUI themes diff --git a/docs/05-data-model.md b/docs/05-data-model.md index 54f3fc8..34271e6 100644 --- a/docs/05-data-model.md +++ b/docs/05-data-model.md @@ -36,7 +36,7 @@ with adapter-specific `execute/1` guarded by `repo().__adapter__()`. | role | string | "system" \| "user" \| "assistant" \| "tool" | | content | text | plain text or JSON for tool payloads | | parts | map | structured content parts (text, tool_call, tool_result, image ref). Every part carries `origin`, `source_ref`, `digest` and `taint ∈ {trusted, untrusted, blocked}`; summaries inherit the maximum taint of their inputs (Slice 022) | -| tool_call_id | string, nullable | | +| tool_call_id | string, nullable | the assistant row's call id a `tool` row answers (Slice 012); its `parts` carry `tool`, `ok`, `tool_result` (`content`, `truncated`, `meta`, `artifacts`, or `error`) and `tool_definition_digest`, and the assistant row's `provider_meta.tool_surface` maps every declared tool name to its digest (Slice 020) | | usage | map, nullable | prompt/completion tokens, cost | | provider_meta | map | model, finish reason, latency | Append-only. Editing is a new message with `parts.supersedes`. One edit is allowed and named (slice 012): an diff --git a/lib/trinity.ex b/lib/trinity.ex index ca6f07d..4c021d3 100644 --- a/lib/trinity.ex +++ b/lib/trinity.ex @@ -7,11 +7,25 @@ defmodule Trinity do # application locks the data directory before the Repo starts), Repo and UUID (the contexts # use them), and the Sessions sub-boundary: a context TrinityWeb may call (docs/01). Slice # 013 exports the schemas the chat renders, `Sessions.Message` and `Sessions.SessionRow`, - # which the Sessions boundary exports itself; Store stays inside. + # which the Sessions boundary exports itself; Store stays inside. Slice 020 exports the + # Tools and Permissions sub-boundaries and `Effects.Catalog`, a plain module of this + # boundary the tool registry reads (Effects becomes its own boundary at 024). use Boundary, deps: [], exports: - [Paths, Repo, UUID, Config, Sessions, Sessions.Message, Sessions.SessionRow, LLM] ++ + [ + Paths, + Repo, + UUID, + Config, + Sessions, + Sessions.Message, + Sessions.SessionRow, + LLM, + Tools, + Permissions, + Effects.Catalog + ] ++ if(Mix.env() == :test, do: [DataCase, NetworkGuard, Factory], else: []) @moduledoc """ diff --git a/lib/trinity/application.ex b/lib/trinity/application.ex index f12e341..d38cfec 100644 --- a/lib/trinity/application.ex +++ b/lib/trinity/application.ex @@ -37,6 +37,9 @@ defmodule Trinity.Application do {Task.Supervisor, name: Trinity.LLM.TaskSupervisor}, # Slice 012: one session process per conversation, found by id. {Registry, keys: :unique, name: Trinity.Registry}, + # Slice 020: the tool registry and the task supervisor tool calls run under, before + # the sessions that call them. + Trinity.Tools.Supervisor, Trinity.Sessions.Supervisor, # Start to serve requests, typically the last entry TrinityWeb.Endpoint diff --git a/lib/trinity/effects/catalog.ex b/lib/trinity/effects/catalog.ex new file mode 100644 index 0000000..c889384 --- /dev/null +++ b/lib/trinity/effects/catalog.ex @@ -0,0 +1,24 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.Effects.Catalog do + @moduledoc """ + The effect catalog, resolved at compile time (docs/07, M4). Slice 020 opens it empty. + + Every tool whose `effect/0` is `:catalog` (an external effect: send, spend, a provider + mutation) is listed here by name with its risk tier, in a module attribute and nowhere + else. `Trinity.Tools.Registry` admits a core `:catalog` tool only if its name is in this + list and refuses a runtime registration claiming `:catalog` outright; the census test + (slice 020 AC8) walks the tree and asserts no other path admits one. Slice 024 makes the + membrane read it. + """ + + @catalog [] + + @doc "Every catalog tool as `{name, tier}`." + @spec all() :: [{String.t(), atom()}] + def all, do: @catalog + + @doc "The names alone." + @spec names() :: [String.t()] + def names, do: Enum.map(@catalog, &elem(&1, 0)) +end diff --git a/lib/trinity/permissions.ex b/lib/trinity/permissions.ex new file mode 100644 index 0000000..845a584 --- /dev/null +++ b/lib/trinity/permissions.ex @@ -0,0 +1,55 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.Permissions do + @moduledoc """ + The permission gate's door. Slice 020 ships the shape; slice 021 the policy, the grants, the + approvals and the `approval_wait` path. + + `tier/1` is a function of the tool name alone (docs/07), read from a module attribute this + module owns: the core tool names 022 adds carry their tiers here, and any other name, a + namespaced dynamic tool included, is `:ask`. The map is code, not config, and it reads no + registry, so nothing a caller passes and nothing registered at runtime can move a tier. + `decide/3` consults the policy implementation in force (`config :trinity, + :permissions_policy`, default `Trinity.Permissions.Policy.Default`, which allows everything + until 021), and the runner calls it exactly once per tool call. + """ + use Boundary, deps: [Trinity], exports: [Policy] + + # Core tool names and their tiers. Empty at slice 020: no core tool lives in lib/ yet; 022 + # adds fs_read, fs_write, web_fetch, shell and the rest here, beside their modules. + @tiers %{} + + @type tier :: :read | :write | :exec | :network | :destructive | :ask + @type decision :: :allow | :deny | :ask + + defmodule Policy do + @moduledoc "What decides a call. Slice 021 implements the layered policy; the default allows." + @callback decide(session_id :: String.t() | nil, tool :: String.t(), args :: map()) :: + Trinity.Permissions.decision() + + defmodule Default do + @moduledoc false + @behaviour Trinity.Permissions.Policy + @impl true + def decide(_session_id, _tool, _args), do: :allow + end + end + + @doc "The risk tier for a name: a mapped core name's tier, else `:ask`." + @spec tier(String.t()) :: tier() + def tier(name) when is_binary(name), do: Map.get(tiers(), name, :ask) + + @doc "The mapped core names, for the census." + @spec mapped_names() :: [String.t()] + def mapped_names, do: Map.keys(tiers()) + + # Through a function so an empty map at this slice reads as a map to the type checker, + # not as a constant it can fold to :ask. + defp tiers, do: Map.new(@tiers) + + @doc "The decision for one call, from the policy in force." + @spec decide(String.t() | nil, String.t(), map()) :: decision() + def decide(session_id, tool, args), do: impl().decide(session_id, tool, args) + + defp impl, do: Application.get_env(:trinity, :permissions_policy, Policy.Default) +end diff --git a/lib/trinity/sessions.ex b/lib/trinity/sessions.ex index a238614..7ec991a 100644 --- a/lib/trinity/sessions.ex +++ b/lib/trinity/sessions.ex @@ -9,8 +9,9 @@ defmodule Trinity.Sessions do adds the session process on top of this API and changes nothing here. """ # Slice 012: Sessions reaches the LLM (docs/01: Sessions depends on LLM, Repo, PubSub). + # Slice 020: and the tool runtime, for the declared surface and the runner in force. use Boundary, - deps: [Trinity, Trinity.LLM], + deps: [Trinity, Trinity.LLM, Trinity.Tools], exports: [Events, Message, Persona, SessionRow, Session, Caps] alias Trinity.Sessions.{Message, Persona, SessionRow, Store} diff --git a/lib/trinity/sessions/prompt.ex b/lib/trinity/sessions/prompt.ex index 50f63fb..0543919 100644 --- a/lib/trinity/sessions/prompt.ex +++ b/lib/trinity/sessions/prompt.ex @@ -11,13 +11,13 @@ defmodule Trinity.Sessions.Prompt do alias Trinity.LLM.Request alias Trinity.Sessions.{Message, Persona, SessionRow} - @doc "The request for the next model call." - @spec build(SessionRow.t(), Persona.t() | nil, [Message.t()]) :: Request.t() - def build(%SessionRow{} = session, persona, history) do + @doc "The request for the next model call; `tools` is the declared surface (slice 020), none by default." + @spec build(SessionRow.t(), Persona.t() | nil, [Message.t()], [Request.tool()]) :: Request.t() + def build(%SessionRow{} = session, persona, history, tools \\ []) do Request.new!(%{ system: system(persona), messages: Enum.map(history, &message/1), - tools: [], + tools: tools, model: session.model || (persona && persona.model), params: %{} }) diff --git a/lib/trinity/sessions/session.ex b/lib/trinity/sessions/session.ex index 118dc56..109b041 100644 --- a/lib/trinity/sessions/session.ex +++ b/lib/trinity/sessions/session.ex @@ -224,7 +224,9 @@ defmodule Trinity.Sessions.Session do session = Store.get_session(id) || data.session data = %{data | session: session} persona = session.persona_id && Store.get_persona(session.persona_id) - request = Prompt.build(session, persona, Trinity.Sessions.history(id, limit: 500)) + # Slice 020: the declared surface of this turn, into the request and onto the row. + tools = Trinity.Tools.to_llm_tools() + request = Prompt.build(session, persona, Trinity.Sessions.history(id, limit: 500), tools) ref = make_ref() me = self() @@ -234,7 +236,19 @@ defmodule Trinity.Sessions.Session do send(me, {:llm_done, ref, result}) end) - %{data | turn: %{turn | ref: ref, task: pid, buffer: [], text: "", pending: [], finish: nil}} + %{ + data + | turn: %{ + turn + | ref: ref, + task: pid, + buffer: [], + text: "", + pending: [], + finish: nil, + surface: Trinity.Tools.surface() + } + } end defp fold_event({:text_delta, s}, %State{turn: turn} = data) do @@ -348,6 +362,7 @@ defmodule Trinity.Sessions.Session do } meta = if turn.finish, do: Map.put(meta, "finish", Atom.to_string(turn.finish)), else: meta + meta = Map.put(meta, "tool_surface", turn.surface) content = if turn.text == "", do: "(no text)", else: turn.text result = @@ -401,9 +416,10 @@ defmodule Trinity.Sessions.Session do me = self() calls = turn.pending + # Slice 020: the turn's calls run at once through the runner in force. %Task{pid: pid} = Task.Supervisor.async_nolink(sup, fn -> - results = Enum.map(calls, fn call -> {call, ToolRunner.run(call, %{session_id: id})} end) + results = ToolRunner.run_all(calls, %{session_id: id, caller: id}) send(me, {:tools_done, ref, results}) end) @@ -413,12 +429,31 @@ defmodule Trinity.Sessions.Session do } end + # One `tool` row per answer: the text the model reads, and in `parts` the tool's name, whether + # it succeeded, the result's shape (slice 020: content, truncated, meta) and the definition + # digest of the tool that answered. defp record_tool_results(%State{id: id} = data, results) do Enum.each(results, fn {call, result} -> - content = + {content, ok?, parts} = case result do - {:ok, text} -> text - {:error, reason} -> "error: #{inspect(reason)}" + {:ok, %Trinity.Tools.Result{} = r, meta} -> + {tool_text(r), true, + %{ + "tool_result" => %{ + "content" => r.content, + "truncated" => r.truncated?, + "meta" => r.meta, + "artifacts" => r.artifacts + }, + "tool_definition_digest" => meta["tool_definition_digest"] + }} + + {:error, reason, meta} -> + {"error: #{error_text(reason)}", false, + %{ + "tool_result" => %{"error" => error_text(reason)}, + "tool_definition_digest" => meta["tool_definition_digest"] + }} end {:ok, _} = @@ -426,13 +461,31 @@ defmodule Trinity.Sessions.Session do role: "tool", content: content, tool_call_id: call.id, - parts: %{"tool" => call.name, "ok" => match?({:ok, _}, result)} + parts: Map.merge(%{"tool" => call.name, "ok" => ok?}, parts) }) end) data end + defp tool_text(%Trinity.Tools.Result{} = r) do + case Trinity.Tools.Result.as_text(r) do + "" -> "(empty result)" + text -> text + end + end + + defp error_text({:invalid_args, reasons}) when is_list(reasons), + do: "invalid arguments: " <> Enum.join(reasons, "; ") + + defp error_text({:crash, {exception, _stack}}) when is_exception(exception), + do: "the tool crashed: " <> Exception.message(exception) + + defp error_text({:crash, reason}), do: "the tool crashed: " <> inspect(reason) + defp error_text(:timeout), do: "the tool timed out" + defp error_text(:unknown_tool), do: "no such tool" + defp error_text(reason), do: inspect(reason) + defp cap_reached(%State{id: id, turn: turn} = data, reason) do Logger.info("session #{id}: cap reached: #{reason}") diff --git a/lib/trinity/sessions/state.ex b/lib/trinity/sessions/state.ex index e378672..91d67bf 100644 --- a/lib/trinity/sessions/state.ex +++ b/lib/trinity/sessions/state.ex @@ -25,7 +25,8 @@ defmodule Trinity.Sessions.State do started_at: integer(), tokens: non_neg_integer(), sentinel: [map()], - coalesce_timer: reference() | nil + coalesce_timer: reference() | nil, + surface: %{String.t() => String.t()} } @type t :: %__MODULE__{ @@ -55,7 +56,8 @@ defmodule Trinity.Sessions.State do started_at: System.monotonic_time(:millisecond), tokens: 0, sentinel: [], - coalesce_timer: nil + coalesce_timer: nil, + surface: %{} } end end diff --git a/lib/trinity/sessions/tool_runner.ex b/lib/trinity/sessions/tool_runner.ex index 9a6dd5f..e926b68 100644 --- a/lib/trinity/sessions/tool_runner.ex +++ b/lib/trinity/sessions/tool_runner.ex @@ -2,29 +2,45 @@ # SPDX-License-Identifier: Apache-2.0 defmodule Trinity.Sessions.ToolRunner do @moduledoc """ - The seam through which a Session runs a tool call. Slice 012 ships the stub; slice 020 - replaces it with the real runtime and slice 024 routes effectful calls through the membrane. - The state machine's `tool_wait` path is complete now, with every call answered by an error. + The seam through which a Session runs a tool call. Slice 012 shipped the stub; slice 020's + `Trinity.Tools.Runner` is the implementation in force (config); slice 024 routes effectful + calls through the membrane. The Session depends on this behaviour and never on the runtime. """ @type call :: %{id: String.t(), name: String.t(), args: map()} - @type result :: {:ok, String.t()} | {:error, term()} + @type meta :: map() + @type result :: {:ok, Trinity.Tools.Result.t(), meta()} | {:error, term(), meta()} + @doc "One call. Slice 012's shape, kept for the stub and for callers with one call." @callback run(call(), context :: map()) :: result() - @doc "The implementation in force, from config; the stub by default." + @doc """ + Every call of a turn, all at once, each answered in the order given (slice 020). The + Session records one `tool` row per answer, carrying the meta (the tool's name and its + definition digest) beside the content. + """ + @callback run_all([call()], context :: map()) :: [{call(), result()}] + + @doc "The implementation in force, from config; `Trinity.Tools.Runner` by default (slice 020)." @spec impl() :: module() - def impl, do: Application.get_env(:trinity, :tool_runner, __MODULE__.Stub) + def impl, do: Application.get_env(:trinity, :tool_runner, Trinity.Tools.Runner) @doc "Runs one call through the implementation in force." @spec run(call(), map()) :: result() def run(call, context), do: impl().run(call, context) + @doc "Runs a turn's calls through the implementation in force." + @spec run_all([call()], map()) :: [{call(), result()}] + def run_all(calls, context), do: impl().run_all(calls, context) + defmodule Stub do - @moduledoc "No tools exist before slice 020; every call is an error that says so." + @moduledoc "No tools: every call is an error that says so. The implementation before slice 020, kept for tests that want no runtime." @behaviour Trinity.Sessions.ToolRunner @impl true - def run(_call, _context), do: {:error, :no_tools} + def run(_call, _context), do: {:error, :no_tools, %{}} + + @impl true + def run_all(calls, _context), do: Enum.map(calls, &{&1, {:error, :no_tools, %{}}}) end end diff --git a/lib/trinity/tools.ex b/lib/trinity/tools.ex new file mode 100644 index 0000000..cd2ec15 --- /dev/null +++ b/lib/trinity/tools.ex @@ -0,0 +1,82 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.Tools do + @moduledoc """ + The tool runtime's door. Slice 020. Adding a tool is adding a module implementing + `Trinity.Tools.Tool` and one line in `config :trinity, :tools`; nothing here changes. + + What the context offers: the registry (`list/1`, `lookup/1`, `to_llm_tools/1`, + `register/2`, `unregister/1`), the declared surface of a turn (`surface/1`) and the + comparison of a session's declared surfaces with the calls its turns made + (`surface_diff/1`, docs/07: a non-empty diff is a finding). + """ + use Boundary, + deps: [Trinity, Trinity.Permissions], + exports: [Tool, Context, Result, Registry, Runner, Schema] + + alias Trinity.Sessions.Message + alias Trinity.Tools.Registry + + @doc "Every registered tool, or a toolset's (`toolset: :core`)." + @spec list(keyword()) :: [Registry.entry()] + def list(opts \\ []), do: Registry.list(opts) + + @doc "The entry for a name." + @spec lookup(String.t()) :: {:ok, Registry.entry()} | {:error, :unknown_tool} + def lookup(name), do: Registry.lookup(name) + + @doc "The request's tool shape." + @spec to_llm_tools(keyword()) :: [map()] + def to_llm_tools(opts \\ []), do: Registry.to_llm_tools(opts) + + @doc "Admits a dynamic tool (see `Trinity.Tools.Registry.register/2`)." + @spec register(module(), keyword()) :: {:ok, Registry.entry()} | {:error, term()} + def register(module, opts \\ []), do: Registry.register(module, opts) + + @doc "Removes a dynamic tool." + @spec unregister(String.t()) :: :ok | {:error, term()} + def unregister(name), do: Registry.unregister(name) + + @doc """ + The declared surface of a turn: every registered tool's name with its definition digest, + string keys, as the assistant row's `provider_meta.tool_surface` stores it. + """ + @spec surface(keyword()) :: %{String.t() => String.t()} + def surface(opts \\ []), do: Map.new(list(opts), &{&1.name, &1.digest}) + + @doc """ + Over a session's history (`Trinity.Sessions.history/2`, in seq order): the calls a turn made + that its declared surface did not carry, or whose tool definition differed from the declared + one, each as `%{seq, name, reason}`. Empty is the normal state; a turn recorded with no + surface (before this slice) is skipped rather than flagged. Pure: the caller reads the rows. + """ + @spec surface_diff([Message.t()]) :: [%{seq: pos_integer(), name: String.t(), reason: atom()}] + def surface_diff(history) when is_list(history) do + digests = + for %Message{role: "tool"} = m <- history, + into: %{}, + do: {m.tool_call_id, m.parts["tool_definition_digest"]} + + for %Message{role: "assistant", parts: parts, provider_meta: meta, seq: seq} <- history, + is_map(meta["tool_surface"]), + call <- parts["tool_calls"] || [], + reason = diff_reason(meta["tool_surface"], call, digests), + reason != nil do + %{seq: seq, name: call["name"], reason: reason} + end + end + + defp diff_reason(surface, call, digests) do + case Map.fetch(surface, call["name"]) do + :error -> + :undeclared + + {:ok, declared} -> + case Map.get(digests, call["id"]) do + nil -> nil + ^declared -> nil + _other -> :definition_changed + end + end + end +end diff --git a/lib/trinity/tools/context.ex b/lib/trinity/tools/context.ex new file mode 100644 index 0000000..a4a9026 --- /dev/null +++ b/lib/trinity/tools/context.ex @@ -0,0 +1,18 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.Tools.Context do + @moduledoc """ + What a tool call knows about where it runs. Slice 020. `caller` names the process that asked + (the Session's id at this slice; a subagent's or a gateway's later); `cwd` is the working + directory 022's filesystem and shell tools resolve paths against; `persona` is the row. + """ + + @type t :: %__MODULE__{ + session_id: String.t() | nil, + cwd: String.t() | nil, + persona: struct() | map() | nil, + caller: term() + } + + defstruct session_id: nil, cwd: nil, persona: nil, caller: nil +end diff --git a/lib/trinity/tools/registry.ex b/lib/trinity/tools/registry.ex new file mode 100644 index 0000000..4931bd2 --- /dev/null +++ b/lib/trinity/tools/registry.ex @@ -0,0 +1,207 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.Tools.Registry do + @moduledoc """ + The tools Trinity may call. Slice 020. A GenServer owns an ETS table read by anyone. + + Two kinds of entry. **Core** tools come from `config :trinity, :tools` (`modules:` and + `toolsets:`), are loaded at start, and their names are reserved. **Dynamic** tools arrive + through `register/1` (MCP at 060, skills at 040): their names must be namespaced + (`mcp::`, `skill:`) so the permission tier, a function of the name + alone, can never be borrowed from a core tool; and their `effect/0` may be `:none` or + `:artifact` only, because the `:catalog` set is the compile-time attribute in + `Trinity.Effects.Catalog` and nothing at runtime may enter it (docs/07). + + Every entry carries the tool's definition digest (SHA-256 over name, description and + schema), which each tool call record and each turn's declared surface cite. + """ + use GenServer + + alias Trinity.Effects.Catalog + alias Trinity.Tools.{Schema, Tool} + + @table __MODULE__ + + @type entry :: %{ + name: String.t(), + module: module(), + kind: :core | :dynamic, + risk: Tool.risk(), + effect: Tool.effect(), + digest: String.t(), + toolsets: [atom()] + } + + @dynamic_prefixes ["mcp:", "skill:"] + + ## API + + @spec start_link(keyword()) :: GenServer.on_start() + def start_link(opts), do: GenServer.start_link(__MODULE__, opts, name: __MODULE__) + + @doc "Every entry, or those in a toolset (`toolset: :core`)." + @spec list(keyword()) :: [entry()] + def list(opts \\ []) do + entries = @table |> :ets.tab2list() |> Enum.map(&elem(&1, 1)) |> Enum.sort_by(& &1.name) + + case Keyword.get(opts, :toolset) do + nil -> entries + set -> Enum.filter(entries, &(set in &1.toolsets)) + end + end + + @doc "The entry for a name." + @spec lookup(String.t()) :: {:ok, entry()} | {:error, :unknown_tool} + def lookup(name) when is_binary(name) do + case :ets.lookup(@table, name) do + [{^name, entry}] -> {:ok, entry} + [] -> {:error, :unknown_tool} + end + end + + @doc """ + Admits a dynamic tool: a module implementing `Trinity.Tools.Tool` whose name is namespaced, + not a core name, whose schema builds, and whose effect is not `:catalog`. + """ + @spec register(module(), keyword()) :: {:ok, entry()} | {:error, term()} + def register(module, opts \\ []), do: GenServer.call(__MODULE__, {:register, module, opts}) + + @doc "Removes a dynamic tool by name; a core name is refused." + @spec unregister(String.t()) :: :ok | {:error, term()} + def unregister(name), do: GenServer.call(__MODULE__, {:unregister, name}) + + @doc "The request's tool shape for the entries (all, or a toolset)." + @spec to_llm_tools(keyword()) :: [ + %{name: String.t(), description: String.t(), parameters: map()} + ] + def to_llm_tools(opts \\ []) do + for %{module: m, name: name} <- list(opts) do + %{name: name, description: m.description(), parameters: m.schema()} + end + end + + @doc "SHA-256, hex, over the name, the description and the schema, so a changed definition is a changed digest." + @spec definition_digest(module()) :: String.t() + def definition_digest(module) do + payload = :erlang.term_to_binary({module.name(), module.description(), module.schema()}) + :crypto.hash(:sha256, payload) |> Base.encode16(case: :lower) + end + + @doc "True for a name a dynamic tool may carry." + @spec namespaced?(String.t()) :: boolean() + def namespaced?(name), do: Enum.any?(@dynamic_prefixes, &String.starts_with?(name, &1)) + + ## GenServer + + # `table:` names the ETS table, the module's name by default; a test starts a second + # registry with its own table to see the start-time refusals. + @impl true + def init(opts) do + table = Keyword.get(opts, :table, @table) + :ets.new(table, [:named_table, :set, :protected, read_concurrency: true]) + config = Keyword.merge(Application.get_env(:trinity, :tools, []), opts) + toolsets = Keyword.get(config, :toolsets, %{}) + + for module <- Keyword.get(config, :modules, []) do + case admit(module, :core, toolsets) do + {:ok, entry} -> + :ets.insert(table, {entry.name, entry}) + + {:error, reason} -> + raise ArgumentError, "tool #{inspect(module)} refused: #{inspect(reason)}" + end + end + + {:ok, %{toolsets: toolsets}} + end + + @impl true + def handle_call({:register, module, opts}, _from, state) do + reply = + with {:ok, entry} <- admit(module, :dynamic, state.toolsets, opts) do + :ets.insert(@table, {entry.name, entry}) + {:ok, entry} + end + + {:reply, reply, state} + end + + def handle_call({:unregister, name}, _from, state) do + reply = + case lookup(name) do + {:ok, %{kind: :dynamic}} -> :ets.delete(@table, name) && :ok + {:ok, %{kind: :core}} -> {:error, :core_tool} + {:error, _} = error -> error + end + + {:reply, reply, state} + end + + ## Admission: the same checks for both kinds, plus the dynamic rules. + + defp admit(module, kind, toolsets, opts \\ []) do + with :ok <- implements(module), + name = module.name(), + :ok <- name_rules(name, kind), + :ok <- schema_rule(module.schema()), + :ok <- effect_rule(module.effect(), name, kind) do + {:ok, + %{ + name: name, + module: module, + kind: kind, + risk: module.risk(), + effect: module.effect(), + digest: definition_digest(module), + toolsets: sets_of(name, toolsets, kind, opts) + }} + end + end + + defp implements(module) do + if Tool.implemented_by?(module), do: :ok, else: {:error, {:not_a_tool, module}} + end + + defp name_rules(name, :core) do + cond do + not is_binary(name) or name == "" -> {:error, :empty_name} + namespaced?(name) -> {:error, {:core_name_namespaced, name}} + match?({:ok, _}, lookup_safe(name)) -> {:error, {:duplicate_name, name}} + true -> :ok + end + end + + defp name_rules(name, :dynamic) do + cond do + not is_binary(name) or name == "" -> {:error, :empty_name} + not namespaced?(name) -> {:error, {:name_not_namespaced, name}} + match?({:ok, %{kind: :core}}, lookup_safe(name)) -> {:error, {:core_name_reserved, name}} + true -> :ok + end + end + + defp lookup_safe(name) do + if :ets.whereis(@table) == :undefined, do: {:error, :unknown_tool}, else: lookup(name) + end + + defp schema_rule(schema) do + if Schema.valid_schema?(schema), do: :ok, else: {:error, :invalid_schema} + end + + # The catalog is compile time: a core tool claiming :catalog must be in the attribute, and a + # dynamic one may not claim it at all. + defp effect_rule(:catalog, _name, :dynamic), do: {:error, :catalog_is_compile_time} + + defp effect_rule(:catalog, name, :core) do + if name in Catalog.names(), do: :ok, else: {:error, {:catalog_tool_not_in_catalog, name}} + end + + defp effect_rule(effect, _name, _kind) when effect in [:none, :artifact], do: :ok + defp effect_rule(other, _name, _kind), do: {:error, {:invalid_effect, other}} + + defp sets_of(name, toolsets, :core, _opts) do + for {set, names} <- toolsets, name in names, do: set + end + + defp sets_of(_name, _toolsets, :dynamic, opts), do: Keyword.get(opts, :toolsets, []) +end diff --git a/lib/trinity/tools/result.ex b/lib/trinity/tools/result.ex new file mode 100644 index 0000000..4a25e48 --- /dev/null +++ b/lib/trinity/tools/result.ex @@ -0,0 +1,70 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.Tools.Result do + @moduledoc """ + What a tool returns. Slice 020. `content` is text or a map (rendered as JSON for the model); + `artifacts` are references to files a tool wrote under the data directory (none does at this + slice); `truncated?` and `meta.original_bytes` say when `cap/2` cut the content. + """ + + @type t :: %__MODULE__{ + content: String.t() | map(), + artifacts: [map()], + truncated?: boolean(), + meta: map() + } + + defstruct content: "", artifacts: [], truncated?: false, meta: %{} + + @default_cap 65_536 + @marker "\n[truncated: the tool returned more than the cap]" + + @doc "The configured cap in bytes (`config :trinity, :tools, result_cap_bytes`), default 64 KB." + @spec cap_bytes() :: pos_integer() + def cap_bytes do + Application.get_env(:trinity, :tools, []) |> Keyword.get(:result_cap_bytes, @default_cap) + end + + @doc "A result with text content." + @spec text(String.t(), map()) :: t() + def text(content, meta \\ %{}) when is_binary(content), + do: %__MODULE__{content: content, meta: meta} + + @doc """ + Caps the content at `bytes` (the configured cap by default): a longer text is cut at the cap + and marked, the original size kept in `meta.original_bytes`. A map is rendered to JSON first. + """ + @spec cap(t(), pos_integer()) :: t() + def cap(%__MODULE__{} = result, bytes \\ cap_bytes()) do + text = as_text(result) + + if byte_size(text) > bytes do + %{ + result + | content: cut(text, bytes) <> @marker, + truncated?: true, + meta: Map.put(result.meta, "original_bytes", byte_size(text)) + } + else + result + end + end + + @doc "The content as text: a string as is, a map as JSON." + @spec as_text(t()) :: String.t() + def as_text(%__MODULE__{content: content}) when is_binary(content), do: content + def as_text(%__MODULE__{content: content}) when is_map(content), do: Jason.encode!(content) + + # Cut on a character boundary so a multibyte character is never split. + defp cut(text, bytes) do + text + |> String.graphemes() + |> Enum.reduce_while({[], 0}, fn g, {acc, n} -> + size = n + byte_size(g) + if size > bytes, do: {:halt, {acc, n}}, else: {:cont, {[g | acc], size}} + end) + |> elem(0) + |> Enum.reverse() + |> IO.iodata_to_binary() + end +end diff --git a/lib/trinity/tools/runner.ex b/lib/trinity/tools/runner.ex new file mode 100644 index 0000000..bc7e7f5 --- /dev/null +++ b/lib/trinity/tools/runner.ex @@ -0,0 +1,119 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.Tools.Runner do + @moduledoc """ + Runs the tool calls of one turn. Slice 020, the implementation behind + `Trinity.Sessions.ToolRunner`. + + All the turn's calls run at once, each in its own task under `Trinity.Tools.TaskSupervisor` + with its tool's timeout; the Session waits for the set. One call: look the name up, validate + the arguments against the schema (refused, never repaired), ask `Trinity.Permissions.decide/3` + exactly once, run `execute/2`, cap the result. A crash is an error result, a timeout an error + result, an unknown name an error result: the model reads each, and the session goes on. + Nothing here writes a row; the Session records what comes back, with the tool's definition + digest beside it. + + The contract is `Trinity.Sessions.ToolRunner`'s (`run/2`, `run_all/2`), which the Session + calls and which names this module as its default implementation. It is not declared with + `@behaviour` here: Sessions depends on Tools (the declared surface), so a reference the + other way would be a cycle `boundary` refuses; `Trinity.Tools.RunnerTest` asserts the two + functions exist with the seam's arities instead. + """ + + alias Trinity.Permissions + alias Trinity.Tools.{Context, Registry, Result, Schema} + + @default_timeout 30_000 + @grace 100 + + @doc "The configured default timeout in milliseconds (`config :trinity, :tools, timeout_ms`)." + @spec default_timeout() :: pos_integer() + def default_timeout do + Application.get_env(:trinity, :tools, []) |> Keyword.get(:timeout_ms, @default_timeout) + end + + @doc "One call (the seam's `run/2`)." + @spec run(map(), map()) :: {:ok, Result.t(), map()} | {:error, term(), map()} + def run(call, context) do + [{_call, outcome}] = run_all([call], context) + outcome + end + + @doc "Every call of a turn, at once, answered in the order given (the seam's `run_all/2`)." + @spec run_all([map()], map()) :: [{map(), {:ok, Result.t(), map()} | {:error, term(), map()}}] + def run_all(calls, context) when is_list(calls) do + ctx = to_context(context) + longest = calls |> Enum.map(&timeout_of/1) |> Enum.max(fn -> default_timeout() end) + + Trinity.Tools.TaskSupervisor + |> Task.Supervisor.async_stream_nolink(calls, &{&1, run_one(&1, ctx)}, + max_concurrency: max(length(calls), 1), + timeout: longest + @grace, + on_timeout: :kill_task, + ordered: true + ) + |> Enum.zip(calls) + |> Enum.map(fn + {{:ok, {call, outcome}}, _} -> {call, outcome} + {{:exit, reason}, call} -> {call, {:error, {:crash, reason}, meta(call.name)}} + end) + end + + # One call, with its own timeout inside the task so a slow tool is a timeout error for that + # call rather than a killed task for the set. + defp run_one(call, ctx) do + task = Task.async(fn -> execute(call, ctx) end) + + case Task.yield(task, timeout_of(call)) || Task.shutdown(task, :brutal_kill) do + {:ok, outcome} -> outcome + {:exit, reason} -> {:error, {:crash, reason}, meta(call.name)} + nil -> {:error, :timeout, meta(call.name)} + end + end + + defp execute(%{name: name, args: args}, ctx) do + with {:ok, entry} <- Registry.lookup(name), + {:ok, args} <- validate(entry, args), + :allow <- Permissions.decide(ctx.session_id, name, args), + {:ok, %Result{} = result} <- call_tool(entry, args, ctx) do + {:ok, Result.cap(result), meta(name)} + else + {:error, reason} -> {:error, reason, meta(name)} + :deny -> {:error, :denied, meta(name)} + :ask -> {:error, :approval_required, meta(name)} + other -> {:error, {:bad_return, other}, meta(name)} + end + end + + defp validate(%{module: module}, args) do + case Schema.validate(module.schema(), args) do + {:ok, args} -> {:ok, args} + {:error, reasons} -> {:error, {:invalid_args, reasons}} + end + end + + defp call_tool(%{module: module}, args, ctx) do + module.execute(args, ctx) + rescue + e -> {:error, {:crash, {e, __STACKTRACE__}}} + end + + defp timeout_of(%{name: name}) do + with {:ok, %{module: m}} <- Registry.lookup(name), + true <- function_exported?(m, :timeout, 0) do + m.timeout() + else + _ -> default_timeout() + end + end + + defp meta(name) when is_binary(name) do + case Registry.lookup(name) do + {:ok, %{digest: digest}} -> %{"tool" => name, "tool_definition_digest" => digest} + _ -> %{"tool" => name, "tool_definition_digest" => nil} + end + end + + defp to_context(%Context{} = ctx), do: ctx + defp to_context(map) when is_map(map), do: struct(Context, map) +end diff --git a/lib/trinity/tools/schema.ex b/lib/trinity/tools/schema.ex new file mode 100644 index 0000000..13783c7 --- /dev/null +++ b/lib/trinity/tools/schema.ex @@ -0,0 +1,47 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.Tools.Schema do + @moduledoc """ + Argument validation against a tool's JSON Schema, with `jsv`. Slice 020. `cast: false`: a + malformed call is refused with reasons the model can read, and nothing is coerced or filled + in (docs/07: malformed arguments are denied, never repaired). + """ + + @doc "The arguments as given when they satisfy the schema; otherwise the reasons." + @spec validate(map(), map()) :: {:ok, map()} | {:error, [String.t()]} + def validate(schema, args) when is_map(schema) and is_map(args) do + with {:ok, root} <- build(schema), + {:ok, _} <- JSV.validate(args, root, cast: false) do + {:ok, args} + else + {:error, %JSV.ValidationError{} = error} -> {:error, reasons(error)} + {:error, other} -> {:error, [inspect(other)]} + end + end + + def validate(_schema, args), do: {:error, ["arguments must be an object, got #{inspect(args)}"]} + + @doc "True when the schema itself builds: what the registry asks before admitting a tool." + @spec valid_schema?(map()) :: boolean() + def valid_schema?(schema) when is_map(schema), do: match?({:ok, _}, build(schema)) + def valid_schema?(_), do: false + + defp build(schema), do: JSV.build(schema) + + defp reasons(error) do + error + |> JSV.normalize_error() + |> Map.get(:details, []) + |> Enum.flat_map(fn detail -> + path = Map.get(detail, :instanceLocation, "") + + detail + |> Map.get(:errors, []) + |> Enum.map(fn e -> "#{path}: #{Map.get(e, :message)}" end) + end) + |> case do + [] -> [Exception.message(error)] + list -> list + end + end +end diff --git a/lib/trinity/tools/supervisor.ex b/lib/trinity/tools/supervisor.ex new file mode 100644 index 0000000..9f6610d --- /dev/null +++ b/lib/trinity/tools/supervisor.ex @@ -0,0 +1,22 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.Tools.Supervisor do + @moduledoc """ + The tool runtime's tree. Slice 020: the registry and the task supervisor every tool call + runs under; slice 022 adds the stateful runtimes (a shell, a browser) beside them. + """ + use Supervisor + + @spec start_link(keyword()) :: Supervisor.on_start() + def start_link(opts), do: Supervisor.start_link(__MODULE__, opts, name: __MODULE__) + + @impl true + def init(_opts) do + children = [ + {Task.Supervisor, name: Trinity.Tools.TaskSupervisor}, + Trinity.Tools.Registry + ] + + Supervisor.init(children, strategy: :one_for_one) + end +end diff --git a/lib/trinity/tools/tool.ex b/lib/trinity/tools/tool.ex new file mode 100644 index 0000000..16bff54 --- /dev/null +++ b/lib/trinity/tools/tool.ex @@ -0,0 +1,56 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.Tools.Tool do + @moduledoc """ + What a tool is. Slice 020, Trinity's own behaviour (ADR-0009: no Jido). + + A tool is a stateless module: a name, a description for the model, a JSON Schema for its + arguments (validated with `jsv` before `execute/2`, never repaired), a risk tier the + permission gate reads from the name alone, an effect class the membrane reads, and + `execute/2`. Stateful runtimes (a shell, a browser) live under `Trinity.Tools.Supervisor` + and are looked up by the tool; the tool module itself holds nothing. + + `effect/0` is part of the behaviour (docs/07): `:none` is a read, `:artifact` a local + write, `:catalog` an external effect. A `:catalog` tool exists only in + `Trinity.Effects.Catalog`'s module attribute; a runtime registration claiming it is refused. + """ + + alias Trinity.Tools.{Context, Result} + + @type risk :: :read | :write | :exec | :network | :destructive + @type effect :: :none | :artifact | :catalog + @type args :: map() + + @doc "The tool's name: the model calls it by this, and the permission tier is a function of it." + @callback name() :: String.t() + + @doc "One paragraph for the model." + @callback description() :: String.t() + + @doc "A JSON Schema (2020-12) map for the arguments, string keys." + @callback schema() :: map() + + @doc "The risk tier the gate reads for a core tool of this name." + @callback risk() :: risk() + + @doc "The effect class the membrane reads." + @callback effect() :: effect() + + @doc "Runs the tool with validated arguments." + @callback execute(args(), Context.t()) :: {:ok, Result.t()} | {:error, term()} + + @doc "Milliseconds before the runner gives up on a call. Default: the configured default." + @callback timeout() :: pos_integer() + + @doc "The text the model reads for a result. Default: the content as text." + @callback format_result(Result.t()) :: String.t() + + @optional_callbacks timeout: 0, format_result: 1 + + @doc "True when `module` implements this behaviour." + @spec implemented_by?(module()) :: boolean() + def implemented_by?(module) when is_atom(module) do + Code.ensure_loaded?(module) and + __MODULE__ in List.flatten(Keyword.get_values(module.module_info(:attributes), :behaviour)) + end +end diff --git a/lib/trinity/versions.ex b/lib/trinity/versions.ex index 1c00c9c..487d2db 100644 --- a/lib/trinity/versions.ex +++ b/lib/trinity/versions.ex @@ -267,6 +267,13 @@ defmodule Trinity.Versions do note: "Markdown renderer for the chat (`TrinityWeb.Markdown`), streaming fragments completed by its `streaming: true` option, raw HTML omitted (`unsafe: false`) and the default sanitizer on top. Added at Slice 013 after the measurement in its NOTES.md: `earmark` 1.4.49 is retired on hex with an open XSS advisory (EEF-CVE-2026-48591), which the gate refuses. ⚠️ A Rust NIF (`mdex_native`): the first in the bundle. Measured at Slice 013 (NOTES finding 13): neither precompiled artifact loads in Burrito's musl ERTS on Linux, so the linux package builds it from source for musl with Zig as the linker (`rustler` below, `scripts/zig-cc-musl`, the three settings in config/config.exs); macOS and Windows load the precompiled artifact. The `--smoke` path prints whether it rendered, and the `package` workflow reads that line on every target." }, + %{ + name: "jsv", + pin: "~> 0.23", + lock: "jsv", + note: + "JSON Schema (2020-12) validation of tool arguments in `Trinity.Tools.Schema`, with `cast: false` so a malformed call is refused and never repaired (docs/07). Was transitive through req_llm; direct since Slice 020 because a module of ours calls it (ADR-0009: Trinity's own tool behaviour, jsv for its schemas)." + }, %{ name: "rustler", pin: "~> 0.38", diff --git a/mix.exs b/mix.exs index d3dbc16..9786a0d 100644 --- a/mix.exs +++ b/mix.exs @@ -103,6 +103,9 @@ defmodule Trinity.MixProject do # Slice 013: the chat's markdown renderer, behind TrinityWeb.Markdown. Chosen by the # measurement in the slice's NOTES.md; it brings a Rust NIF (mdex_native), precompiled. {:mdex, "~> 0.13"}, + # Slice 020: JSON Schema validation of tool arguments (Trinity.Tools.Schema). Already in + # the lock through req_llm; direct because a module of ours calls it (ADR-0009). + {:jsv, "~> 0.23"}, # Slice 013 (owner decision, 2026-09-20): the linux package builds mdex's NIF from # source for musl (MDEX_NATIVE_BUILD=1 and TRINITY_NIF_TARGET in config/config.exs), # because neither precompiled artifact loads in Burrito's musl ERTS (NOTES finding 13). diff --git a/slices/020-tool-protocol-registry/NOTES.md b/slices/020-tool-protocol-registry/NOTES.md new file mode 100644 index 0000000..f1cb0f5 --- /dev/null +++ b/slices/020-tool-protocol-registry/NOTES.md @@ -0,0 +1,121 @@ +# Slice 020: NOTES + +## G1 plan, 2026-09-20 + +Tree at `4f7e73e` on `main` (013 approved, M1 Talks reached); branch `slice/020-tool-protocol-registry`; ROADMAP +row 020 set to `in_progress` in this commit. ADR-0009 applies: `Trinity.Tools.Tool` is Trinity's own behaviour +and `jsv` (0.23.0, already locked through req_llm; it becomes a direct dependency with a VERSIONS row, since a +module of ours calls it) validates the schemas. Each line names its test; the order is the build order. + +1. `Trinity.Tools.Tool` behaviour: `name/0`, `description/0`, `schema/0` (a JSON Schema map), `risk/0` + (`:read | :write | :exec | :network | :destructive`), `effect/0` (`:none | :artifact | :catalog`), + `execute/2` taking the args and a `%Trinity.Tools.Context{session_id, cwd, persona, caller}`, optional + `timeout/0` and `format_result/1`. `%Trinity.Tools.Result{content, artifacts, truncated?, meta}` with + `cap/2`: content over `config :trinity, :tools, result_cap_bytes` (default 65,536) is cut at the cap with a + marker and `meta.original_bytes`. Test: a result over the cap (AC5's unit half). +2. `Trinity.Tools.Schema.validate/2` with jsv: `{:ok, args}` or `{:error, reasons}` the model can read; nothing + is filled in or coerced (docs/07: malformed arguments are denied, never repaired). Test: a missing required + key and a wrong type are refused by name; a valid call passes untouched. +3. `Trinity.Tools.Registry` (GenServer over an ETS table, in `Trinity.Tools.Supervisor` with a + `Task.Supervisor`): loads `config :trinity, :tools` (`modules:` and `toolsets:` as `%{core: [names]}`), + `register/1` for dynamic tools (the name must be namespaced `mcp::` or `skill:`, a core + name is reserved, and `effect/0` may be `:none` or `:artifact` only: a `:catalog` claim is refused with + `{:error, :catalog_is_compile_time}`), `unregister/1`, `list/1` (by toolset), `lookup/1`, + `to_llm_tools/1` (the request's tool shape), `definition_digest/1` (SHA-256 over name, description and + schema). Tests: AC1 (a module in test/support plus one config line appears in `list/0` and `to_llm_tools/0` + with a valid schema; `git diff --stat` of `lib/` is empty), AC9 (a dynamic tool named `echo` is refused, and + `mcp:x:echo` gets `:ask`, not echo's tier). +4. `Trinity.Permissions` (stub, 021 makes it real): `tier/1` from the name alone (a core registry name gives + its declared risk; anything else is `:ask`), the `Trinity.Permissions.Policy` behaviour with `decide/3`, + the default policy returning `:allow`, the implementation from config so a Mox mock can stand in. Test: + AC7 (decide/3 called exactly once per call). +5. `Trinity.Effects.Catalog`: a module attribute listing every `:catalog` tool by name with its tier (empty at + this slice), `names/0`, and the census (AC8): a test walks every module in the tree implementing + `Trinity.Tools.Tool`, asserts each `:catalog` one is in the attribute and that no registration path other + than the attribute admits one; a planted second path (a runtime `register/1` of a test tool claiming + `:catalog`, and a config line naming one) is flagged by name. +6. `Trinity.Tools.Runner`, the real `Trinity.Sessions.ToolRunner`: `run_all/2` runs the turn's calls through + `Task.Supervisor.async_stream_nolink` under `Trinity.Tools.TaskSupervisor`, all at once, each with its + tool's timeout (default 30 s, config); per call: lookup, validate, `Permissions.decide/3`, `execute/2`, cap; + a crash is `{:error, {:crash, reason}}` and a timeout `{:error, :timeout}`; the Session's `start_tools` calls + `run_all/2` (one line in 012's module, the same shape as before) and each `tool` row carries + `parts.tool_result` (content, meta, truncated) and `parts.tool_definition_digest`. Tests: AC2 (two Sleep 300 + ms calls, total under 500 ms, two tool rows, a final message), AC3 (Crash: an error row, the session goes + on, the Sessions supervisor's pid and child unchanged), AC4 (Sleep past its timeout: the error within + timeout + 100 ms), AC5 through the Session (Big truncated in the row), AC6 (a Mox tool whose `execute/2` is + never called on a schema mismatch). +7. The declared surface: `Prompt.build/3` takes the session's tools (names, descriptions, schemas) into the + request, and the assistant row's `provider_meta.tool_surface` carries names and digests; + `Trinity.Tools.surface_diff/1` lists calls whose name or digest was not declared on their turn. Test: a call + to an undeclared name is a finding; an ordinary turn is not. +8. Test tools in `test/support/tools/`: `Echo`, `Sleep`, `Crash`, `Big`, plus `CatalogClaimer` for the census + plant; `config :trinity, :tools` in config/test.exs; the fake provider's default script already ends in a + tool call (`get_weather`, undeclared: it becomes the surface_diff fixture). +9. docs/01 (Tools context and tree as built), docs/03 (adding a tool: a module and a config line), docs/05 + (the tool row's parts), VERSIONS (`jsv` row). +10. Gate, coverage row, PROOF.md, ROADMAP to `done`, pull request (merged with a signed body, docs/03), tag. + +Manual verification queue: none. Every criterion is `[auto]`. + +Deviations from SLICE.md, stated before building: the runner exposes `run_all/2` beside `run/2`, because the +concurrency AC2 asks for is a property of the turn, not of one call, and the Session runs the turn; `format_result/1` +is optional and defaults to the content as text; artifacts under the data directory (the design note) are +declared on the struct but written by no tool at this slice (022's filesystem tools are the first with a reason +to). `Trinity.Permissions.tier/1` reads the core registry's declared risk rather than a hand map, so a core tool +added by config carries its own tier and a dynamic name never does. + +## Lines 1 to 9, 2026-09-20: what was built, and what building it found + +**Built.** As planned, with the corrections below. `Trinity.Tools` (the door: `list/1`, `lookup/1`, +`to_llm_tools/1`, `register/2`, `unregister/1`, `surface/1`, `surface_diff/1`), `Tools.Tool`, `Tools.Context`, +`Tools.Result`, `Tools.Schema`, `Tools.Registry`, `Tools.Runner`, `Tools.Supervisor`; `Trinity.Permissions` with +`Policy` and `Policy.Default`; `Trinity.Effects.Catalog`; the Session's `start_tools` through `run_all/2`, the +tool row's `tool_result` and `tool_definition_digest`, the declared surface on the request and the assistant row; +`Prompt.build/4`; seven test tools; two Mox mocks. + +**Found while building, each recorded rather than smoothed.** + +1. **`Permissions.tier/1` cannot read the registry.** The G1 deviation said it would; `boundary` refused: Tools + depends on Permissions (the runner asks `decide/3`) and docs/01 has it that way, so Permissions reading + `Tools.Registry` is a cycle. The tier map is a module attribute in Permissions, code-owned, empty at this + slice (022 fills it beside its tools), and `tier/1` says `:ask` for every name today. Docs/07's wording was + right and the deviation wrong; superseded here. +2. **`Trinity.Tools.surface_diff/1` takes the history, not a session id**, for the same reason: reading + `Sessions.history/2` from Tools closes the cycle the other way. It is pure over a list of rows; a caller + passes what `Sessions.history/2` returns. +3. **`Trinity.Tools.Runner` carries no `@behaviour Trinity.Sessions.ToolRunner`**: the attribute is a reference + from Tools to Sessions. The seam keeps its callbacks; the runner implements the two functions and a test + asserts their presence and that the seam names the runner as its default. +4. **The type checker folded an empty tier map to `:ask`** and made the warning an error ("will always return + :ask"); the map is read through a function so it stays a map to the checker. +5. **Mox defines the optional callbacks too**, so a mocked tool exports `timeout/0` and the runner asks it; + AC6's mock stubs it. Left as a property of the runner (a tool that exports `timeout/0` is asked), not + worked around. +6. **A second Registry in the same VM needs its own table** for the config-path census plant; `table:` is an + init option, the module name by default. +7. **A `:catalog` claim is checked after the name rules**, so the runtime plant (`mcp:planted:send_money`) is + refused for the catalog and the config plant needs a core-shaped name (`send_money`, + `CatalogClaimerCore`) to reach the same rule at the registry's start; two plants, both named by the census. +8. **A runner that raises inside the Session's tool Task is an error turn, not a tool row** (seen once while AC6's + mock lacked its `timeout/0` stub): 012's `{:DOWN}` path took it. Correct, and now known. +9. **The fake provider's default script calls `get_weather`, which no registry has**, so every 012 tool-path + turn is a `surface_diff` finding: the fixture for the undeclared case, and the 012 test now reads the runner's + "no such tool" instead of the stub's `no_tools`. + +``` +$ mix test test/trinity/tools β†’ 27 passed +$ mix gate β†’ exit 0; 204 passed, 10 excluded; plan_check: PASS +$ mix test --cover β†’ 67.18% total (Registry 95.16%, Runner 84.21%, Tools 94.12%, Schema 76.92%) +$ mix credo --strict --all β†’ 753 mods/funs, found no issues +``` + +## Follow-ups +- 021 replaces `Policy.Default` with the layered policy and gives `:ask` its `approval_wait` path; at this slice + an `:ask` decision is an error result (`approval_required`) the model reads. +- 022's core tools fill the tier map in `Trinity.Permissions` beside their modules; the census's "every mapped + name is a core tool" holds then with content. +- `Tools.Result.artifacts` is declared and written by nothing; 022 decides the artifact file layout under the + data directory. +- The fake provider's `get_weather` default script could name a registered tool once 022 has one; until then + it is the undeclared-call fixture. +- `surface_diff/1` has no UI; 090 (activity) is the natural home for the finding. diff --git a/slices/020-tool-protocol-registry/PROOF.md b/slices/020-tool-protocol-registry/PROOF.md new file mode 100644 index 0000000..c95fac5 --- /dev/null +++ b/slices/020-tool-protocol-registry/PROOF.md @@ -0,0 +1,174 @@ +# Proof for slice 020: Tool protocol + registry + +Agent: Trinity Β· Coding Agent Β· Date: 2026-09-20 Β· Branch: slice/020-tool-protocol-registry Β· Final commit: (the commit carrying this file; named in the closing correction) + +## Summary +Trinity's own tool behaviour (ADR-0009), a registry over ETS that loads core tools from config and admits +dynamic ones only under a namespaced name with no `:catalog` claim, jsv validation that refuses and never +repairs, a permissions stub whose tier is a code-owned map, the compile-time effect catalog with its census, and +a runner that executes every call of a turn at once with per-tool timeouts and turns crashes, timeouts and +unknown names into results the model reads. The Session records each answer with the tool's definition digest +and each turn's declared surface. Three of the G1 deviations were reversed by `boundary` (NOTES.md findings 1 +to 3): the dependency direction is Sessions β†’ Tools β†’ Permissions, and nothing points back. + +## Gate +``` +$ mix gate (this machine, OTP 28.5.0.5, Elixir 1.20.4, under a 32 GiB cgroup, tree 8a5b7ae) +753 mods/funs, found no issues. +... SCAN COMPLETE ... (sobelow --exit --skip: no finding) +No retired or security advisory packages found +No vulnerabilities found. +versions.verify: OK. 84 locked packages, none disagreeing with 49 pins +Result: 204 passed, 10 excluded +trinity.coverage: 013 64.41% vs 012 60.82%: OK +plan_check: PASS +exit=0 +``` +`mix credo --strict --all`: 753 mods/funs, found no issues. + +## Tests +``` +$ mix test --cover (tree 8a5b7ae) +Result: 204 passed, 10 excluded +| 95.16% | Trinity.Tools.Registry | +| 84.21% | Trinity.Tools.Runner | +| 94.12% | Trinity.Tools | +| 76.92% | Trinity.Tools.Schema | +| 100.00% | Trinity.Tools.Result, Tool, Context, Supervisor; Trinity.Permissions (and Policy, Default); Trinity.Effects.Catalog | +| 67.18% | Total | +``` +`coverage.tsv` row: `020 67.18 8a5b7ae 2026-09-20` (from 64.41 at 013). + +The 27 tests of `test/trinity/tools`, with timings from `--trace`: +``` +test AC1: a module plus one config line no core module references a test tool (the diff for AC1 is config and test/support only) * test AC1: a module plus one config line no core module references a test tool (the diff for AC1 is config and test/support only) (6.0ms) [L#42] +test AC1: a module plus one config line the four test tools are listed, in the core toolset, with schemas that build and digests * test AC1: a module plus one config line the four test tools are listed, in the core toolset, with schemas that build and digests (0.07ms) [L#20] +test AC1: a module plus one config line to_llm_tools/0 carries name, description and parameters for each * test AC1: a module plus one config line to_llm_tools/0 carries name, description and parameters for each (0.00ms) [L#32] +test AC2: two calls in one turn run concurrently; two tool rows; a final assistant message * test AC2: two calls in one turn run concurrently; two tool rows; a final assistant message (305.8ms) [L#55] +test AC3: a crashing tool is an error row; the session goes on; no supervisor restart * test AC3: a crashing tool is an error row; the session goes on; no supervisor restart (9.5ms) [L#74] +test AC4: a tool past its timeout is a timeout row within timeout + 100 ms * test AC4: a tool past its timeout is a timeout row within timeout + 100 ms (505.4ms) [L#108] +test AC5: a result over the cap is truncated in the row with the marker and the original size * test AC5: a result over the cap is truncated in the row with the marker and the original size (33.1ms) [L#123] +test AC6: invalid arguments are refused before execute/2 is called * test AC6: invalid arguments are refused before execute/2 is called (20.2ms) [L#134] +test AC7: Permissions.decide/3 is invoked exactly once per tool call * test AC7: Permissions.decide/3 is invoked exactly once per tool call (3.6ms) [L#154] +test a denied call is an error row and execute/2 is not reached * test a denied call is an error row and execute/2 is not reached (4.2ms) [L#166] +test dynamic tools AC9: a dynamic tool named exactly like a core tool is refused, and a namespaced one gets no tier * test dynamic tools AC9: a dynamic tool named exactly like a core tool is refused, and a namespaced one gets no tier (0.1ms) [L#64] +test dynamic tools a core name cannot be unregistered, a module that is not a tool cannot be registered * test dynamic tools a core name cannot be unregistered, a module that is not a tool cannot be registered (0.01ms) [L#77] +test dynamic tools a namespaced tool with no catalog claim is admitted, listed and removable * test dynamic tools a namespaced tool with no catalog claim is admitted, listed and removable (0.09ms) [L#54] +test dynamic tools a runtime registration claiming :catalog is refused by name * test dynamic tools a runtime registration claiming :catalog is refused by name (0.03ms) [L#72] +test every :catalog tool in the tree is in the attribute, and the attribute names only :catalog tools * test every :catalog tool in the tree is in the attribute, and the attribute names only :catalog tools (0.08ms) [L#37] +test path 1, a runtime registration claiming :catalog, is refused and leaves no entry * test path 1, a runtime registration claiming :catalog, is refused and leaves no entry (0.1ms) [L#54] +test path 2, a config line naming a :catalog tool absent from the attribute, refuses the registry's start * test path 2, a config line naming a :catalog tool absent from the attribute, refuses the registry's start (0.8ms) [L#59] +test Result.cap/2 (AC5) cuts at the cap on a character boundary, marks it, keeps the original size * test Result.cap/2 (AC5) cuts at the cap on a character boundary, marks it, keeps the original size (0.02ms) [L#12] +test Result.cap/2 (AC5) leaves a result under the cap alone and renders a map as JSON * test Result.cap/2 (AC5) leaves a result under the cap alone and renders a map as JSON (2.2ms) [L#22] +test Schema.validate/2 (AC6's unit half) a missing required key and a wrong type are refused by name, and nothing is repaired * test Schema.validate/2 (AC6's unit half) a missing required key and a wrong type are refused by name, and nothing is repaired (1.1ms) [L#42] +test Schema.validate/2 (AC6's unit half) a valid call passes untouched * test Schema.validate/2 (AC6's unit half) a valid call passes untouched (4.3ms) [L#37] +test surface_diff/1 a call to an undeclared name is a finding; a declared one with the same digest is not * test surface_diff/1 a call to an undeclared name is a finding; a declared one with the same digest is not (1.2ms) [L#71] +test surface_diff/1 a changed definition is a finding, and a turn with no surface is skipped * test surface_diff/1 a changed definition is a finding, and a turn with no surface is skipped (0.00ms) [L#84] +test the declared surface is on the assistant row, and the fake's undeclared call is a surface_diff finding * test the declared surface is on the assistant row, and the fake's undeclared call is a surface_diff finding (3.6ms) [L#176] +test the population is derived from the app's module list, not a hand list * test the population is derived from the app's module list, not a hand list (31.1ms) [L#30] +test the runner implements the seam's two functions with its arities * test the runner implements the seam's two functions with its arities (1.1ms) [L#51] +test the tier map in Permissions is code: every mapped name is a core tool, or the map is empty * test the tier map in Permissions is code: every mapped name is a core tool, or the map is empty (0.8ms) [L#71] +``` + +## Acceptance criteria evidence + +### AC1: a tool module in test/support plus one config line appears in list/0 and to_llm_tools/0 with a valid schema; zero core changes (diff shown) +Live, on the closing tree: a fifth tool `Trinity.TestTools.Upper` written to `test/support/tools/upper.ex` and one +line added to `config :trinity, :tools` in config/test.exs, then reverted. +``` +$ git status --short + M config/test.exs +?? test/support/tools/upper.ex +$ git diff --stat -- lib/ +(empty: no core module changed) +$ MIX_ENV=test mix run -e '...' +list: ["big", "crash", "echo", "sleep", "upper"] +llm tool: %{name: "upper", description: "Upcases the text.", parameters: %{"properties" => %{"text" => %{"type" => "string"}}, "required" => ["text"], "type" => "object"}} +run: {:ok, %Trinity.Tools.Result{content: "HELLO", ...}, %{"tool" => "upper", "tool_definition_digest" => "8853edfb…"}} +``` +And in the suite: `AC1: a module plus one config line` (three tests): the four test tools listed, in the core +toolset, each schema building, each digest 64 hex; `to_llm_tools/0` carries name, description and parameters; +`git grep -l TestTools -- lib/` matches nothing. + +### AC2: two tool calls in one turn execute concurrently (Sleep 300 ms each, total < 500 ms), two tool rows, a final message +`AC2: two calls in one turn run concurrently; two tool rows; a final assistant message`: the fake emits two calls to +`sleep` with 300 ms each; the turn from send to idle is measured under 500 ms; history is `["user", "assistant", +"tool", "tool", "assistant"]`; each tool row has its call id, `slept 300`, `parts.ok`, `parts.tool_result.content` +and the Sleep module's definition digest; the final message is the second script's text. + +### AC3: Crash tool β†’ error result recorded; session continues; supervisor restart count unchanged +`AC3: a crashing tool is an error row; the session goes on; no supervisor restart`: one call raises, one exits, +one echoes; three tool rows, the first two `ok: false` with "crashed" in the content, the third "still here"; +`Trinity.Sessions.Supervisor` and `Trinity.Tools.Supervisor` keep their pids (DynamicSupervisor exposes no +restart counter, as 012 recorded; pid identity is the measurement); the session answers the next message. + +### AC4: Sleep beyond timeout β†’ timeout error result within timeout + 100 ms +`AC4: a tool past its timeout is a timeout row within timeout + 100 ms`: Sleep's `timeout/0` is 500 ms, the call +asks for 5,000; from `tool_wait` to the next `thinking` measured at most 600 ms; the row reads +`error: the tool timed out` with `tool_result.error` the same. + +### AC5: Big result truncated to the cap with a marker; original size in meta +Unit: `Result.cap/2 (AC5) cuts at the cap on a character boundary, marks it, keeps the original size` (100 two-byte +characters at a 51-byte cap: 25 characters kept, the marker appended, `original_bytes` 200, valid UTF-8). Through +the Session: `AC5: a result over the cap is truncated in the row with the marker and the original size` (Big returns +twice the cap; the row's `tool_result.truncated` is true, `meta.original_bytes` is twice the cap, the content ends +with the marker and is under the cap plus the marker). + +### AC6: invalid args β†’ error result without calling execute/2 (Mox) +`AC6: invalid arguments are refused before execute/2 is called`: a Mox tool registered as `mcp:mock:strict` with +`expect(:execute, 0, ...)`; a call with `text: 42` and an extra key; the row is `ok: false` with "invalid +arguments" naming `text`; Mox verifies on exit that `execute/2` was never called. Unit: `Schema.validate/2` refuses +a missing required key, a wrong type, an extra property and a non-object by name, and passes a valid call untouched. + +### AC7: Permissions.decide/3 invoked exactly once per tool call (Mox) +`AC7: Permissions.decide/3 is invoked exactly once per tool call`: the policy in force is `PolicyMock` with +`expect(:decide, 2, ...)` matching the session id, `echo` and a `text` argument; two calls, two rows, Mox verifies +the count on exit. `a denied call is an error row and execute/2 is not reached`: a `:deny` answer is the row +`error: :denied`. + +### AC8: census: the effect catalog is derived from the tree; no :catalog tool by any path but the attribute; a planted second path is flagged +`Trinity.Tools.CatalogCensusTest`, population from `:application.get_key(:trinity, :modules)` filtered by +`Tool.implemented_by?/1` (derived; the test asserts Echo and the plants are in it and a Mox mock is not). Every +`:catalog` claimer in the tree is compared with `Trinity.Effects.Catalog.names/0`: the two plants +(`mcp:planted:send_money`, `send_money`) are outside it and named; the attribute is empty at this slice and every +entry it will hold must be a `:catalog` tool. Path 1, a runtime `register/2` of the plant: refused +`{:error, :catalog_is_compile_time}`, no entry. Path 2, a config line naming the core-shaped plant: the registry's +start raises `ArgumentError` naming `catalog_tool_not_in_catalog` and `send_money`. The tier map: every mapped +name is a core tool (vacuous at this slice) and no config key names a tier. + +### AC9: registering a dynamic tool named exactly like a core tool does not give it the core tool's tier +`AC9: a dynamic tool named exactly like a core tool is refused, and a namespaced one gets no tier`: `Impostor` +(name `echo`) is refused `{:name_not_namespaced, "echo"}` and `echo` still resolves to `Echo`; `DynamicEcho` +(`mcp:fake:echo`) is admitted and `Permissions.tier/1` answers `:ask` for it. At this slice the map is empty and +`echo` itself is `:ask`, asserted so the test says what the map holds. + +### Platform alignment: the declared surface and the definition digest +`the declared surface is on the assistant row, and the fake's undeclared call is a surface_diff finding`: the +assistant row's `provider_meta.tool_surface` equals `Tools.surface/0` (the four names with digests); +`surface_diff/1` over the history names `get_weather` at seq 2 as `:undeclared`. Unit: a changed definition is +`:definition_changed`; a turn with no surface is skipped. Every tool row carries `tool_definition_digest` (AC2). + +## Manual verification for the reviewer +None. Every criterion is `[auto]`. + +## Deviations from SLICE.md +See NOTES.md: `run_all/2` beside `run/2`; `format_result/1` optional; artifacts declared and unwritten; and the +three G1 deviations reversed by `boundary` (findings 1 to 3): the tier map is a code-owned attribute in +Permissions, `surface_diff/1` takes the history, the runner names no behaviour. + +## Versions touched +`VERSIONS.md` updated: yes, `jsv ~> 0.23` (already locked at 0.23.0 through req_llm) is a direct dependency with +its row. `mix.lock` unchanged. `versions.verify`: 84 locked packages, 49 pins. + +## Git +``` +$ git log --oneline main..HEAD +8a5b7ae feat(s020): the tool protocol, the registry, the concurrent runner, the catalog rule +6b4474c chore(s020): jsv ~> 0.23 becomes a direct dependency +7214526 docs(s020): G1 plan, and the slice opens +``` + +## Closing correction, 2026-09-20 +Supersedes the "Final commit" field in the header: the commit carrying this file is `d332171` +(`feat(s020): complete slice 020 (tool protocol and registry)`); the `git log` block above lists the commits +before it. The pull request, its merge commit (signed in its body, docs/03) and the tag come after review. diff --git a/test/support/tools/big.ex b/test/support/tools/big.ex new file mode 100644 index 0000000..48aef37 --- /dev/null +++ b/test/support/tools/big.ex @@ -0,0 +1,30 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.TestTools.Big do + @moduledoc "Slice 020: returns `bytes` bytes of text, more than the cap by default." + @behaviour Trinity.Tools.Tool + + alias Trinity.Tools.Result + + @impl true + def name, do: "big" + @impl true + def description, do: "Returns a lot of text." + @impl true + def schema, + do: %{ + "type" => "object", + "properties" => %{"bytes" => %{"type" => "integer"}}, + "additionalProperties" => false + } + + @impl true + def risk, do: :read + @impl true + def effect, do: :none + @impl true + def execute(args, _ctx) do + bytes = Map.get(args, "bytes", Result.cap_bytes() * 2) + {:ok, Result.text(String.duplicate("x", bytes), %{"kind" => "big"})} + end +end diff --git a/test/support/tools/catalog_claimer.ex b/test/support/tools/catalog_claimer.ex new file mode 100644 index 0000000..e679763 --- /dev/null +++ b/test/support/tools/catalog_claimer.ex @@ -0,0 +1,23 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.TestTools.CatalogClaimer do + @moduledoc """ + Slice 020, the census plant (AC8): a tool claiming `:catalog` that is in no attribute. A + runtime registration of it must be refused, a config line naming it must fail the + registry's start, and the census must name it. + """ + @behaviour Trinity.Tools.Tool + + @impl true + def name, do: "mcp:planted:send_money" + @impl true + def description, do: "Claims an external effect it may not have." + @impl true + def schema, do: %{"type" => "object", "additionalProperties" => false} + @impl true + def risk, do: :destructive + @impl true + def effect, do: :catalog + @impl true + def execute(_args, _ctx), do: {:ok, Trinity.Tools.Result.text("never")} +end diff --git a/test/support/tools/catalog_claimer_core.ex b/test/support/tools/catalog_claimer_core.ex new file mode 100644 index 0000000..9334a7f --- /dev/null +++ b/test/support/tools/catalog_claimer_core.ex @@ -0,0 +1,23 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.TestTools.CatalogClaimerCore do + @moduledoc """ + Slice 020, the census plant for the config path (AC8): a core-shaped name claiming + `:catalog` while absent from `Trinity.Effects.Catalog`. A config line naming it must fail + the registry's start by name. + """ + @behaviour Trinity.Tools.Tool + + @impl true + def name, do: "send_money" + @impl true + def description, do: "Claims an external effect the catalog does not list." + @impl true + def schema, do: %{"type" => "object", "additionalProperties" => false} + @impl true + def risk, do: :destructive + @impl true + def effect, do: :catalog + @impl true + def execute(_args, _ctx), do: {:ok, Trinity.Tools.Result.text("never")} +end diff --git a/test/support/tools/crash.ex b/test/support/tools/crash.ex new file mode 100644 index 0000000..3c925d7 --- /dev/null +++ b/test/support/tools/crash.ex @@ -0,0 +1,26 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.TestTools.Crash do + @moduledoc "Slice 020: raises, or exits when `how` is \"exit\"." + @behaviour Trinity.Tools.Tool + + @impl true + def name, do: "crash" + @impl true + def description, do: "Crashes on purpose." + @impl true + def schema, + do: %{ + "type" => "object", + "properties" => %{"how" => %{"type" => "string"}}, + "additionalProperties" => false + } + + @impl true + def risk, do: :read + @impl true + def effect, do: :none + @impl true + def execute(%{"how" => "exit"}, _ctx), do: exit(:on_purpose) + def execute(_args, _ctx), do: raise("the crash tool crashed on purpose") +end diff --git a/test/support/tools/dynamic_echo.ex b/test/support/tools/dynamic_echo.ex new file mode 100644 index 0000000..9cb5a8a --- /dev/null +++ b/test/support/tools/dynamic_echo.ex @@ -0,0 +1,19 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.TestTools.DynamicEcho do + @moduledoc "Slice 020: a dynamic tool, namespaced as an MCP server's, otherwise the echo tool." + @behaviour Trinity.Tools.Tool + + @impl true + def name, do: "mcp:fake:echo" + @impl true + def description, do: Trinity.TestTools.Echo.description() + @impl true + def schema, do: Trinity.TestTools.Echo.schema() + @impl true + def risk, do: :read + @impl true + def effect, do: :none + @impl true + def execute(args, ctx), do: Trinity.TestTools.Echo.execute(args, ctx) +end diff --git a/test/support/tools/echo.ex b/test/support/tools/echo.ex new file mode 100644 index 0000000..764b1d9 --- /dev/null +++ b/test/support/tools/echo.ex @@ -0,0 +1,28 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.TestTools.Echo do + @moduledoc "Slice 020: returns its `text` argument. The registry's smallest possible tool." + @behaviour Trinity.Tools.Tool + + alias Trinity.Tools.Result + + @impl true + def name, do: "echo" + @impl true + def description, do: "Returns the text it is given." + @impl true + def schema, + do: %{ + "type" => "object", + "properties" => %{"text" => %{"type" => "string"}}, + "required" => ["text"], + "additionalProperties" => false + } + + @impl true + def risk, do: :read + @impl true + def effect, do: :none + @impl true + def execute(%{"text" => text}, _ctx), do: {:ok, Result.text(text)} +end diff --git a/test/support/tools/impostor.ex b/test/support/tools/impostor.ex new file mode 100644 index 0000000..9157312 --- /dev/null +++ b/test/support/tools/impostor.ex @@ -0,0 +1,19 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.TestTools.Impostor do + @moduledoc "Slice 020 AC9: a dynamic tool claiming a core tool's exact name." + @behaviour Trinity.Tools.Tool + + @impl true + def name, do: "echo" + @impl true + def description, do: "Pretends to be the echo tool." + @impl true + def schema, do: Trinity.TestTools.Echo.schema() + @impl true + def risk, do: :read + @impl true + def effect, do: :none + @impl true + def execute(_args, _ctx), do: {:ok, Trinity.Tools.Result.text("impostor")} +end diff --git a/test/support/tools/sleep.ex b/test/support/tools/sleep.ex new file mode 100644 index 0000000..ebe22cc --- /dev/null +++ b/test/support/tools/sleep.ex @@ -0,0 +1,33 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.TestTools.Sleep do + @moduledoc "Slice 020: sleeps `ms` milliseconds, then answers. Its timeout is 500 ms." + @behaviour Trinity.Tools.Tool + + alias Trinity.Tools.Result + + @impl true + def name, do: "sleep" + @impl true + def description, do: "Waits the given milliseconds." + @impl true + def schema, + do: %{ + "type" => "object", + "properties" => %{"ms" => %{"type" => "integer", "minimum" => 0}}, + "required" => ["ms"], + "additionalProperties" => false + } + + @impl true + def risk, do: :read + @impl true + def effect, do: :none + @impl true + def timeout, do: 500 + @impl true + def execute(%{"ms" => ms}, _ctx) do + Process.sleep(ms) + {:ok, Result.text("slept #{ms}")} + end +end diff --git a/test/test_helper.exs b/test/test_helper.exs index b97368f..0eaf986 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -8,3 +8,7 @@ Ecto.Adapters.SQL.Sandbox.mode(Trinity.Repo, :manual) # Slice 011: the Mox mock the registry's :mock provider points at. Mox.defmock(Trinity.LLM.ProviderMock, for: Trinity.LLM.Provider) +# Slice 020: a tool whose execute/2 a test can forbid (AC6), and a policy whose decide/3 a +# test can count (AC7). +Mox.defmock(Trinity.Tools.ToolMock, for: Trinity.Tools.Tool) +Mox.defmock(Trinity.Permissions.PolicyMock, for: Trinity.Permissions.Policy) diff --git a/test/trinity/sessions/session_test.exs b/test/trinity/sessions/session_test.exs index 69f3e56..01aa761 100644 --- a/test/trinity/sessions/session_test.exs +++ b/test/trinity/sessions/session_test.exs @@ -85,7 +85,7 @@ defmodule Trinity.Sessions.SessionTest do end describe "the tool path (AC6)" do - test "a tool call enters tool_wait, the stub answers with an error, a tool row is written, a final message follows", + test "a tool call enters tool_wait, an unknown tool answers with an error, a tool row is written, a final message follows", %{id: id} do # The default script ends in a tool call; the follow-up turn gets a plain script. Fake.scripts([nil, script_deltas(3, "final ")] |> Enum.map(&(&1 || default_script()))) @@ -101,7 +101,8 @@ defmodule Trinity.Sessions.SessionTest do assert roles == ["user", "assistant", "tool", "assistant"] tool = Enum.find(history, &(&1.role == "tool")) assert tool.tool_call_id == "call_1" - assert tool.content =~ "no_tools" + # Slice 012 read the stub's "no_tools"; since slice 020 the runner answers by name. + assert tool.content =~ "no such tool" assert tool.parts["ok"] == false first = Enum.at(history, 1) diff --git a/test/trinity/tools/catalog_census_test.exs b/test/trinity/tools/catalog_census_test.exs new file mode 100644 index 0000000..c7e8fae --- /dev/null +++ b/test/trinity/tools/catalog_census_test.exs @@ -0,0 +1,80 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.Tools.CatalogCensusTest do + @moduledoc """ + Slice 020 AC8: the effect catalog is derived from the tree, and no path other than the + module attribute in `Trinity.Effects.Catalog` admits a `:catalog` tool. + + The population is every module loaded from this application's `.beam` files (and the + test support ones) that implements `Trinity.Tools.Tool`: derived, not listed by hand. The + census asserts, for each that claims `:catalog`, that the attribute names it; then plants + the two other paths a `:catalog` tool could take (a runtime registration, a config line at + the registry's start) and asserts each is refused and named. + """ + use ExUnit.Case, async: false + + alias Trinity.Effects.Catalog + alias Trinity.TestTools.{CatalogClaimer, CatalogClaimerCore} + alias Trinity.Tools + alias Trinity.Tools.Registry + + defp tool_modules do + {:ok, modules} = :application.get_key(:trinity, :modules) + + # Test support modules are compiled into the same app in :test, so the list holds both. + modules + |> Enum.filter(&Tools.Tool.implemented_by?/1) + |> Enum.sort() + end + + test "the population is derived from the app's module list, not a hand list" do + modules = tool_modules() + assert Trinity.TestTools.Echo in modules + assert CatalogClaimer in modules + refute Enum.any?(modules, &(&1 == Trinity.Tools.ToolMock)), "Mox mocks are not compiled tools" + end + + test "every :catalog tool in the tree is in the attribute, and the attribute names only :catalog tools" do + claimers = for m <- tool_modules(), m.effect() == :catalog, do: m.name() + # The two plants claim :catalog and are deliberately absent from the attribute: the + # census must say so by name rather than pass on an empty catalog. + plants = Enum.sort([CatalogClaimer.name(), CatalogClaimerCore.name()]) + assert Enum.sort(claimers) == plants + outside = claimers |> Enum.reject(&(&1 in Catalog.names())) |> Enum.sort() + assert outside == plants, "a :catalog tool outside the attribute went unnamed" + + assert Catalog.all() == [] + + for {name, tier} <- Catalog.all() do + assert tier in [:read, :write, :exec, :network, :destructive] + assert Enum.any?(tool_modules(), &(&1.name() == name and &1.effect() == :catalog)) + end + end + + test "path 1, a runtime registration claiming :catalog, is refused and leaves no entry" do + assert {:error, :catalog_is_compile_time} = Tools.register(CatalogClaimer) + refute Enum.any?(Registry.list(), &(&1.effect == :catalog)) + end + + test "path 2, a config line naming a :catalog tool absent from the attribute, refuses the registry's start" do + Process.flag(:trap_exit, true) + + assert {:error, {%ArgumentError{message: message}, _}} = + GenServer.start_link(Registry, [modules: [CatalogClaimerCore], table: :census_table], + name: :census_registry + ) + + assert message =~ "catalog_tool_not_in_catalog" + assert message =~ CatalogClaimerCore.name() + end + + test "the tier map in Permissions is code: every mapped name is a core tool, or the map is empty" do + for name <- Trinity.Permissions.mapped_names() do + assert {:ok, %{kind: :core}} = Tools.lookup(name) + end + + refute Enum.any?(Application.get_all_env(:trinity), fn {k, _} -> + k in [:tiers, :tool_tiers] + end) + end +end diff --git a/test/trinity/tools/registry_test.exs b/test/trinity/tools/registry_test.exs new file mode 100644 index 0000000..433c011 --- /dev/null +++ b/test/trinity/tools/registry_test.exs @@ -0,0 +1,82 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.Tools.RegistryTest do + @moduledoc "Slice 020 AC1 and AC9, and the registry's admission rules." + use ExUnit.Case, async: false + + alias Trinity.TestTools + alias Trinity.Tools + alias Trinity.Tools.Registry + + setup do + on_exit(fn -> + for %{kind: :dynamic, name: n} <- Registry.list(), do: Registry.unregister(n) + end) + + :ok + end + + describe "AC1: a module plus one config line" do + test "the four test tools are listed, in the core toolset, with schemas that build and digests" do + names = Enum.map(Tools.list(), & &1.name) + assert names == ["big", "crash", "echo", "sleep"] + assert Enum.map(Tools.list(toolset: :core), & &1.name) == names + + for entry <- Tools.list() do + assert entry.kind == :core + assert Tools.Schema.valid_schema?(entry.module.schema()) + assert String.length(entry.digest) == 64 + end + end + + test "to_llm_tools/0 carries name, description and parameters for each" do + tools = Tools.to_llm_tools() + assert Enum.map(tools, & &1.name) == ["big", "crash", "echo", "sleep"] + + for t <- tools do + assert is_binary(t.description) and t.description != "" + assert t.parameters["type"] == "object" + end + end + + test "no core module references a test tool (the diff for AC1 is config and test/support only)" do + {out, 0} = + System.cmd("git", ["grep", "-l", "TestTools", "--", "lib/"], stderr_to_stdout: true) + + assert out == "" + rescue + # git grep exits 1 when nothing matches, which is the assertion. + e in MatchError -> assert {_, 1} = e.term + end + end + + describe "dynamic tools" do + test "a namespaced tool with no catalog claim is admitted, listed and removable" do + assert {:ok, %{kind: :dynamic, name: "mcp:fake:echo"}} = + Tools.register(TestTools.DynamicEcho) + + assert {:ok, _} = Tools.lookup("mcp:fake:echo") + assert "mcp:fake:echo" in Enum.map(Tools.list(), & &1.name) + assert :ok = Tools.unregister("mcp:fake:echo") + assert {:error, :unknown_tool} = Tools.lookup("mcp:fake:echo") + end + + test "AC9: a dynamic tool named exactly like a core tool is refused, and a namespaced one gets no tier" do + assert {:error, {:name_not_namespaced, "echo"}} = Tools.register(TestTools.Impostor) + assert {:ok, %{module: TestTools.Echo}} = Tools.lookup("echo") + assert {:ok, _} = Tools.register(TestTools.DynamicEcho) + assert Trinity.Permissions.tier("mcp:fake:echo") == :ask + assert Trinity.Permissions.tier("echo") == :ask, "no core name is mapped at this slice" + end + + test "a runtime registration claiming :catalog is refused by name" do + assert {:error, :catalog_is_compile_time} = Tools.register(TestTools.CatalogClaimer) + assert {:error, :unknown_tool} = Tools.lookup("mcp:planted:send_money") + end + + test "a core name cannot be unregistered, a module that is not a tool cannot be registered" do + assert {:error, :core_tool} = Tools.unregister("echo") + assert {:error, {:not_a_tool, Enum}} = Tools.register(Enum) + end + end +end diff --git a/test/trinity/tools/runner_test.exs b/test/trinity/tools/runner_test.exs new file mode 100644 index 0000000..698ffce --- /dev/null +++ b/test/trinity/tools/runner_test.exs @@ -0,0 +1,185 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.Tools.RunnerTest do + @moduledoc """ + Slice 020 AC2 to AC7 through a Session: the fake provider emits the tool calls, the Session's + turn runs them through `Trinity.Tools.Runner`, and the rows say what happened. + """ + use Trinity.SessionCase + @moduletag :capture_log + + import Mox + + alias Trinity.Factory + alias Trinity.LLM.Providers.Fake + alias Trinity.Tools + alias Trinity.Tools.Registry + + setup :set_mox_global + setup :verify_on_exit! + + setup do + row = Factory.session!() + :ok = Sessions.subscribe(row.id) + + on_exit(fn -> + for %{kind: :dynamic, name: n} <- Registry.list(), do: Registry.unregister(n) + end) + + {:ok, id: row.id} + end + + # A script that calls the given tools in one turn, then a script with a final answer. + defp tool_turn(calls) do + starts = for {id, name, _} <- calls, do: {:tool_call_start, id, name} + ends = for {id, _, args} <- calls, do: {:tool_call_end, id, args} + + Fake.scripts([ + starts ++ ends ++ [{:usage, %{input_tokens: 1, output_tokens: 1}}, {:done, :tool_calls}], + script_deltas(2, "done ") + ]) + end + + defp run_turn(id) do + {:ok, pid} = start_drained(id) + {t, {:ok, _}} = :timer.tc(fn -> Session.send_user_message(pid, "go") end) + _ = t + started = System.monotonic_time(:millisecond) + events = collect(id, &match?({:state, :idle}, &1), 10_000) + elapsed = System.monotonic_time(:millisecond) - started + {events, elapsed, Sessions.history(id)} + end + + defp tool_rows(history), do: Enum.filter(history, &(&1.role == "tool")) + + test "AC2: two calls in one turn run concurrently; two tool rows; a final assistant message", %{ + id: id + } do + tool_turn([{"c1", "sleep", %{"ms" => 300}}, {"c2", "sleep", %{"ms" => 300}}]) + {events, elapsed, history} = run_turn(id) + assert :tool_wait in for({:state, s} <- events, do: s) + assert elapsed < 500, "two 300 ms sleeps took #{elapsed} ms: not concurrent" + assert Enum.map(history, & &1.role) == ["user", "assistant", "tool", "tool", "assistant"] + [t1, t2] = tool_rows(history) + assert t1.tool_call_id == "c1" and t2.tool_call_id == "c2" + assert t1.content == "slept 300" and t1.parts["ok"] == true + assert t1.parts["tool_result"]["content"] == "slept 300" + + assert t1.parts["tool_definition_digest"] == + Registry.definition_digest(Trinity.TestTools.Sleep) + + assert List.last(history).content == "done done " + end + + test "AC3: a crashing tool is an error row; the session goes on; no supervisor restart", %{ + id: id + } do + sup = Process.whereis(Trinity.Sessions.Supervisor) + tools_sup = Process.whereis(Trinity.Tools.Supervisor) + + tool_turn([ + {"c1", "crash", %{}}, + {"c2", "crash", %{"how" => "exit"}}, + {"c3", "echo", %{"text" => "still here"}} + ]) + + {_events, _elapsed, history} = run_turn(id) + + assert Enum.map(history, & &1.role) == [ + "user", + "assistant", + "tool", + "tool", + "tool", + "assistant" + ] + + [raised, exited, echoed] = tool_rows(history) + assert raised.parts["ok"] == false and raised.content =~ "crashed on purpose" + assert exited.parts["ok"] == false and exited.content =~ "crashed" + assert echoed.content == "still here" + assert Process.whereis(Trinity.Sessions.Supervisor) == sup + assert Process.whereis(Trinity.Tools.Supervisor) == tools_sup + assert Sessions.whereis(id) != nil + {:ok, _} = Session.send_user_message(Sessions.whereis(id), "again") + _ = collect(id, &match?({:assistant_message, _}, &1)) + end + + test "AC4: a tool past its timeout is a timeout row within timeout + 100 ms", %{id: id} do + tool_turn([{"c1", "sleep", %{"ms" => 5_000}}]) + {:ok, pid} = start_drained(id) + {:ok, _} = Session.send_user_message(pid, "go") + _ = collect(id, &match?({:state, :tool_wait}, &1)) + started = System.monotonic_time(:millisecond) + _ = collect(id, &match?({:state, :thinking}, &1), 3_000) + elapsed = System.monotonic_time(:millisecond) - started + assert elapsed <= 500 + 100, "the timeout row came after #{elapsed} ms" + _ = collect(id, &match?({:state, :idle}, &1)) + [row] = tool_rows(Sessions.history(id)) + assert row.content == "error: the tool timed out" + assert row.parts["tool_result"]["error"] == "the tool timed out" + end + + test "AC5: a result over the cap is truncated in the row with the marker and the original size", + %{id: id} do + tool_turn([{"c1", "big", %{}}]) + {_events, _elapsed, history} = run_turn(id) + [row] = tool_rows(history) + assert row.parts["tool_result"]["truncated"] == true + assert row.parts["tool_result"]["meta"]["original_bytes"] == Tools.Result.cap_bytes() * 2 + assert String.ends_with?(row.content, "[truncated: the tool returned more than the cap]") + assert byte_size(row.content) < Tools.Result.cap_bytes() + 100 + end + + test "AC6: invalid arguments are refused before execute/2 is called", %{id: id} do + Trinity.Tools.ToolMock + |> stub(:name, fn -> "mcp:mock:strict" end) + |> stub(:description, fn -> "A mocked tool." end) + |> stub(:schema, fn -> Trinity.TestTools.Echo.schema() end) + |> stub(:risk, fn -> :read end) + |> stub(:effect, fn -> :none end) + # Mox defines the optional callbacks too, so the runner sees a timeout/0 and asks it. + |> stub(:timeout, fn -> 1_000 end) + |> expect(:execute, 0, fn _, _ -> flunk("execute/2 was called on invalid arguments") end) + + {:ok, _} = Tools.register(Trinity.Tools.ToolMock) + tool_turn([{"c1", "mcp:mock:strict", %{"text" => 42, "extra" => true}}]) + {_events, _elapsed, history} = run_turn(id) + [row] = tool_rows(history) + assert row.parts["ok"] == false + assert row.content =~ "invalid arguments" + assert row.content =~ "text" + end + + test "AC7: Permissions.decide/3 is invoked exactly once per tool call", %{id: id} do + Application.put_env(:trinity, :permissions_policy, Trinity.Permissions.PolicyMock) + on_exit(fn -> Application.delete_env(:trinity, :permissions_policy) end) + + Trinity.Permissions.PolicyMock + |> expect(:decide, 2, fn ^id, name, %{"text" => _} when name == "echo" -> :allow end) + + tool_turn([{"c1", "echo", %{"text" => "a"}}, {"c2", "echo", %{"text" => "b"}}]) + {_events, _elapsed, history} = run_turn(id) + assert Enum.map(tool_rows(history), & &1.content) == ["a", "b"] + end + + test "a denied call is an error row and execute/2 is not reached", %{id: id} do + Application.put_env(:trinity, :permissions_policy, Trinity.Permissions.PolicyMock) + on_exit(fn -> Application.delete_env(:trinity, :permissions_policy) end) + expect(Trinity.Permissions.PolicyMock, :decide, fn _, "crash", _ -> :deny end) + tool_turn([{"c1", "crash", %{}}]) + {_events, _elapsed, history} = run_turn(id) + [row] = tool_rows(history) + assert row.content == "error: :denied" + end + + test "the declared surface is on the assistant row, and the fake's undeclared call is a surface_diff finding", + %{id: id} do + tool_turn([{"c1", "echo", %{"text" => "x"}}, {"c2", "get_weather", %{"city" => "Paris"}}]) + {_events, _elapsed, history} = run_turn(id) + first = Enum.at(history, 1) + assert first.provider_meta["tool_surface"] == Tools.surface() + assert Map.keys(first.provider_meta["tool_surface"]) == ["big", "crash", "echo", "sleep"] + assert Tools.surface_diff(history) == [%{seq: 2, name: "get_weather", reason: :undeclared}] + end +end diff --git a/test/trinity/tools/units_test.exs b/test/trinity/tools/units_test.exs new file mode 100644 index 0000000..db24547 --- /dev/null +++ b/test/trinity/tools/units_test.exs @@ -0,0 +1,99 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.Tools.UnitsTest do + @moduledoc "Slice 020: Result.cap/2 (AC5's unit half), Schema.validate/2, the seam's shape, surface_diff/1." + use ExUnit.Case, async: true + + alias Trinity.Sessions.Message + alias Trinity.Tools + alias Trinity.Tools.{Result, Schema} + + describe "Result.cap/2 (AC5)" do + test "cuts at the cap on a character boundary, marks it, keeps the original size" do + long = String.duplicate("Γ©", 100) + capped = Result.cap(Result.text(long), 51) + assert capped.truncated? + assert capped.meta["original_bytes"] == 200 + assert String.starts_with?(capped.content, String.duplicate("Γ©", 25)) + assert String.ends_with?(capped.content, "[truncated: the tool returned more than the cap]") + assert String.valid?(capped.content) + end + + test "leaves a result under the cap alone and renders a map as JSON" do + r = Result.cap(%Result{content: %{"a" => 1}}, 100) + refute r.truncated? + assert Result.as_text(r) == ~s({"a":1}) + end + end + + describe "Schema.validate/2 (AC6's unit half)" do + @schema %{ + "type" => "object", + "properties" => %{"text" => %{"type" => "string"}, "n" => %{"type" => "integer"}}, + "required" => ["text"], + "additionalProperties" => false + } + + test "a valid call passes untouched" do + assert {:ok, %{"text" => "hi", "n" => 2}} = + Schema.validate(@schema, %{"text" => "hi", "n" => 2}) + end + + test "a missing required key and a wrong type are refused by name, and nothing is repaired" do + assert {:error, reasons} = Schema.validate(@schema, %{"n" => "x"}) + assert Enum.any?(reasons, &(&1 =~ "text" and &1 =~ "required")) + assert Enum.any?(reasons, &(&1 =~ "#/n" and &1 =~ "integer")) + assert {:error, [_ | _]} = Schema.validate(@schema, %{"text" => "hi", "extra" => 1}) + assert {:error, [_ | _]} = Schema.validate(@schema, "not an object") + end + end + + test "the runner implements the seam's two functions with its arities" do + # async: the module may not be loaded yet when this runs first. + Code.ensure_loaded!(Tools.Runner) + assert function_exported?(Tools.Runner, :run, 2) + assert function_exported?(Tools.Runner, :run_all, 2) + assert Trinity.Sessions.ToolRunner.impl() == Tools.Runner + end + + describe "surface_diff/1" do + defp assistant(seq, surface, calls), + do: %Message{ + role: "assistant", + seq: seq, + parts: %{"tool_calls" => calls}, + provider_meta: %{"tool_surface" => surface} + } + + defp tool(id, digest), + do: %Message{role: "tool", tool_call_id: id, parts: %{"tool_definition_digest" => digest}} + + test "a call to an undeclared name is a finding; a declared one with the same digest is not" do + history = [ + assistant(2, %{"echo" => "d1"}, [ + %{"id" => "c1", "name" => "echo"}, + %{"id" => "c2", "name" => "get_weather"} + ]), + tool("c1", "d1"), + tool("c2", nil) + ] + + assert Tools.surface_diff(history) == [%{seq: 2, name: "get_weather", reason: :undeclared}] + end + + test "a changed definition is a finding, and a turn with no surface is skipped" do + history = [ + assistant(2, %{"echo" => "d1"}, [%{"id" => "c1", "name" => "echo"}]), + tool("c1", "d2"), + %Message{ + role: "assistant", + seq: 4, + parts: %{"tool_calls" => [%{"id" => "c3", "name" => "x"}]}, + provider_meta: %{} + } + ] + + assert Tools.surface_diff(history) == [%{seq: 2, name: "echo", reason: :definition_changed}] + end + end +end