From 42e3dbec006c0e499e4e70d807717cafcabaf298 Mon Sep 17 00:00:00 2001 From: Ayla Croft Date: Mon, 21 Sep 2026 13:19:54 -0400 Subject: [PATCH 1/8] docs(s041): what the slice joins, the G1 plan, and the slice opens Signed-off-by: Ayla Croft --- ROADMAP.md | 2 +- slices/041-skill-self-management/NOTES.md | 85 +++++++++++++++++++++++ 2 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 slices/041-skill-self-management/NOTES.md diff --git a/ROADMAP.md b/ROADMAP.md index ebb7395..7e04ed9 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -50,7 +50,7 @@ standards register names the rows that ask for them. | 033 | Project context: AGENTS.md | 3 Memory | S | 030, 022 | approved | | 034 | Export, import, restore | 3 Memory | S | 030, 031 | approved | | 040 | Skills registry + agentskills.io format + progressive disclosure | 4 Skills | M | 020 | approved | -| 041 | Skill self-management with staged approval + scanner | 4 Skills | M | 040, 021 | planned | +| 041 | Skill self-management with staged approval + scanner | 4 Skills | M | 040, 021 | in_progress | | 050 | Scheduler: Oban cron agent tasks with delivery targets | 5 Automation | M | 012 | planned | | 059 | MCP capability gap against beam_mcp, and the server seam probe | 6 MCP | S/M | 020 | planned | | 060 | MCP client: Trinity's thin driver (2026-07-28 preferred, 2025-11-25 compat, MRTR, Tasks) | 6 MCP | L | 059, 021 | planned | diff --git a/slices/041-skill-self-management/NOTES.md b/slices/041-skill-self-management/NOTES.md new file mode 100644 index 0000000..44385dc --- /dev/null +++ b/slices/041-skill-self-management/NOTES.md @@ -0,0 +1,85 @@ +# Slice 041: NOTES + +## Read before code, 2026-09-21 + +The pieces this slice joins are all built: 021's `Trinity.Permissions` (a request is a row and a broadcast, +a decision writes the grant or rule it implies), 024's receipts (`Trinity.Receipts.append/2` on a chain scope), +040's registry (the filesystem canonical, the `skills` rows an index, the user root +`/skills`), the effects census pattern (024 AC1: `git ls-files` grepped, a planted bypass in +`test/support` that must be found). docs/07's Skills section: agent-authored skills land in +`pending_approval` with a diff and rationale; the scanner's list; hub skills default to disabled (hub install +is out of this slice). + +No new dependency: the unified diff is a small line-based LCS of Trinity's own (`Trinity.Skills.Diff`), since no +Hex package for it is in VERSIONS.md and the need is one function. + +## G1 plan, 2026-09-21 + +Tree at `0f46141` on `main` (040 approved); branch `slice/041-skill-self-management`; ROADMAP row 041 to +`in_progress` in this commit. Each line names its test. + +1. Migration `skill_changes` (docs/05): `skill_name`, `action` (`create | patch | write_file | remove_file | + delete`), `source` (the target root: `user`), `change_dir`, `diff` (text), `rationale`, `destructive` + (a whole-file replace or a delete), `digest` (SHA-256 over the staged tree's paths and bytes), `status` + (`pending | approved | rejected | applied | failed`), `severity` (`none | low | medium | high`), `findings` + (the scanner's), `proposed_by` (session id), `approval_id`, `decided_by`, `decided_at`, `comment`, + `receipt_hash`, `applied_version`. +2. `Trinity.Skills.Staging` (the proposer): every action writes the whole target tree as it would be after + the change into `/pending/skills///` (a copy of the current skill directory + with the change applied; `delete` an empty tree with a marker), never anywhere else; `Trinity.Skills.Diff` + renders a unified diff per changed text file, and a non-text or oversized file is "replaced, N bytes" + (SLICE risk); the scanner runs on the pending tree; the row is written. Tests AC1 (a `create` is a row and + staged files, and `Trinity.Skills.list/0` does not show it: the pending directory is outside the roots) and + the proposer census half of AC8 (the only `File` writes under `lib/trinity/skills/` outside promotion are + `Staging`'s, and all of them target the pending directory; a planted write in `test/support` is flagged). +3. `Trinity.Skills.Promotion` (the one apply path): `swap/3` takes the change, an approval id and the + deciding party; verifies the approval row (decided, not denied, tool `skill_apply`, its arguments naming + this change's id and digest), recomputes the pending tree's digest, archives the current directory to + `/skills/.history//v/`, moves the pending tree into place (a rename, atomic per + directory), rescans the registry, writes an `effect` receipt on the `skills` chain scope carrying the change's + digest and the approval id, and marks the row `applied` with the version. Tests AC2 (approve a create: files + in the user root, listed, version 1), AC3 (a patch: the diff; approve: version 2 and `.history/…/v1`; reject: + nothing changed), AC8 (the census: exactly one caller of `Promotion.swap/3` in the tree, plus the planted + one in `test/support`; `swap/3` refuses a missing, pending, denied or mismatched approval by name). +4. The approval: `Trinity.Skills.Manager.approve(change, opts)` requests a `skill_apply` approval through + `Trinity.Permissions.request_approval/4` (risk `:write`, the change id and digest as its arguments, so the + fingerprint binds them and gateways see it on `approvals:all`), decides it (`:once`, `by:` the deciding + party), and calls `Promotion.swap/3`; `reject/2` marks the row and removes the staged files. + Auto-approval: `persona.settings["skills"]["auto_approve"]` (`"off"` by default, `"low"` to allow) applies + a change whose severity is `none` or `low` at once with `decided_by: "auto"`; `high` (and `medium`) never. + Tests AC4 (a proposal with `curl … | sh` and an API-key-looking string is `high`; auto-approval enabled and + refused) and AC5 (the registry and the gate restarted between staging and approval; the row and the files + survive; approval applies). +5. `Trinity.Skills.Scanner`: every text file of a pending tree against the heuristics (shell pipes to a + shell and destructive commands: high; credential shapes: high; instructions to ignore or disable: high; + plain shell commands, network calls and external URLs: medium; base64 blobs: medium); a file skipped + (binary, over 256 KB, not UTF-8) is a low finding naming it and why. Findings carry file, line, rule, + severity, the matched text truncated. Tests per rule and for the exclusion. +6. The `skill_manage` tool (`Trinity.Skills.Tools.Manage`, risk `:write`, effect `:artifact`; actions and + arguments validated by its schema) stages and answers with the change id, the severity and "awaiting + approval on /skills". The `learn` tool (`Trinity.Skills.Tools.Learn`, risk `:write`, effect `:artifact`): + a source (a file under the session's roots through `Trinity.Tools.FS.Read`, a URL through + `Trinity.Tools.Web.Fetch`, or pasted text) distilled by the session's model (`generate_object/3`) into a + SKILL.md and a `references/` file, then staged as a `create`. Tests through the runner with the fake + provider (the object scripted); the manual GIF and the sample skill for AC6 on the real model. +7. `/skills` gains the pending changes: the list with severity, a diff view, the findings, approve and reject + with a comment; the learn form. LiveView tests; screenshots for AC7. +8. docs/07 (the section as built), docs/05, docs/01. Manual queue: AC6 (the learned skill's SKILL.md and + reference in `proof/`, under 200 lines), AC7 (the three screenshots). + +Manual verification queue (two items, for the owner at G4): +- **AC6**: `/learn` on a local markdown file: the staged skill's `SKILL.md` and `references/` file in + `proof/learned/`, the line count named. +- **AC7**: `proof/ac7-*.png`: the pending list, the diff, the findings. + +Decisions stated before code: (a) the proposer is `Trinity.Skills.Staging` and it writes under +`/pending/skills` only; the apply path is `Trinity.Skills.Promotion.swap/3` and nothing else moves +files into a root (AC8, the census); (b) `skill_manage` and `learn` are `:write` tools like `fs_write`: under +the default policy the staging call itself asks (an approval to propose), and the staged change asks again +to apply; an "always allow" on `skill_manage` makes proposing free while the promotion stays gated, which +is the point of staging; (c) the promotion's receipt is written on a `skills` chain scope, not a session's, +because a change may be approved from a page with no session and outlives the session that proposed it; +(d) "medium" severity is neither auto-approved nor blocked from a human's approval; only `high` is named on +the card as blocking auto-approval; (e) cross-scope promotion (project to global) is not built: the target +root is always the user root, and a project skill's edit stages against the user root under the same name +(the platform alignment's second approval waits for a slice that has two targets). From 0c16db3d403a007123c008d9b558575adc971ede Mon Sep 17 00:00:00 2001 From: Ayla Croft Date: Mon, 21 Sep 2026 13:40:33 -0400 Subject: [PATCH 2/8] feat(s041): staged skill changes: the proposer, the one promotion path, the scanner, the manager, skill_manage and learn, the pending changes on /skills Every change is staged as the whole target tree under the pending root with a unified diff, the scanner's findings and a skill_changes row; the only apply path is Promotion.swap, which requires an allowed skill_apply approval naming the change's id and digest, archives the previous version under .history, renames the pending tree into place and writes an effect receipt on the skills chain scope; the manager requests and decides the approval through the permission gate (an approval may now have no session: migration and changeset) and applies the persona's auto-approval to none/low only; the census finds the one caller of swap and the two filesystem writers, and the plant; the compaction test's crossing message is sized from the tool surface. Signed-off-by: Ayla Croft --- config/config.exs | 5 +- config/test.exs | 8 +- lib/trinity.ex | 5 + lib/trinity/permissions.ex | 1 + lib/trinity/permissions/approval.ex | 4 +- lib/trinity/skills.ex | 30 +- lib/trinity/skills/change.ex | 86 +++++ lib/trinity/skills/diff.ex | 79 +++++ lib/trinity/skills/learn.ex | 170 ++++++++++ lib/trinity/skills/manager.ex | 66 ++++ lib/trinity/skills/promotion.ex | 162 ++++++++++ lib/trinity/skills/scanner.ex | 103 ++++++ lib/trinity/skills/staging.ex | 303 ++++++++++++++++++ lib/trinity/skills/tools/learn.ex | 53 +++ lib/trinity/skills/tools/manage.ex | 93 ++++++ lib/trinity/tools.ex | 2 +- lib/trinity_web/live/skills_live.ex | 173 +++++++++- .../20260921160000_create_skill_changes.exs | 36 +++ ...0921161000_approvals_without_a_session.exs | 63 ++++ test/support/skills/bypass.ex | 21 ++ test/trinity/memory/compaction_test.exs | 16 +- test/trinity/skills/census_test.exs | 74 +++++ test/trinity/skills/manage_tools_test.exs | 212 ++++++++++++ test/trinity/skills/scanner_test.exs | 107 +++++++ test/trinity/skills/staging_test.exs | 269 ++++++++++++++++ test/trinity/skills/tools_test.exs | 2 +- test/trinity/tools/registry_test.exs | 4 + test/trinity_web/live/skill_changes_test.exs | 107 +++++++ 28 files changed, 2237 insertions(+), 17 deletions(-) create mode 100644 lib/trinity/skills/change.ex create mode 100644 lib/trinity/skills/diff.ex create mode 100644 lib/trinity/skills/learn.ex create mode 100644 lib/trinity/skills/manager.ex create mode 100644 lib/trinity/skills/promotion.ex create mode 100644 lib/trinity/skills/scanner.ex create mode 100644 lib/trinity/skills/staging.ex create mode 100644 lib/trinity/skills/tools/learn.ex create mode 100644 lib/trinity/skills/tools/manage.ex create mode 100644 priv/repo/migrations/20260921160000_create_skill_changes.exs create mode 100644 priv/repo/migrations/20260921161000_approvals_without_a_session.exs create mode 100644 test/support/skills/bypass.ex create mode 100644 test/trinity/skills/census_test.exs create mode 100644 test/trinity/skills/manage_tools_test.exs create mode 100644 test/trinity/skills/scanner_test.exs create mode 100644 test/trinity/skills/staging_test.exs create mode 100644 test/trinity_web/live/skill_changes_test.exs diff --git a/config/config.exs b/config/config.exs index 0caf96f..efec080 100644 --- a/config/config.exs +++ b/config/config.exs @@ -79,6 +79,9 @@ config :trinity, :tools, Trinity.Skills.Tools.List, Trinity.Skills.Tools.View, Trinity.Skills.Tools.File, + # Slice 041: proposals and learning, staged for approval. + Trinity.Skills.Tools.Manage, + Trinity.Skills.Tools.Learn, # Slice 030: the always-on memory tiers. Trinity.Tools.Memory, Trinity.Tools.Shell.Run @@ -90,7 +93,7 @@ config :trinity, :tools, # Slice 031: search over past conversations. memory: ["session_search", "recall", "memory"], # Slice 040: the skill tools. - skills: ["skills_list", "skill_view", "skill_file"] + skills: ["skills_list", "skill_view", "skill_file", "skill_manage", "learn"] } # Slice 040: the skill roots. `user_dir:` and `bundled_dir:` default to the data directory's diff --git a/config/test.exs b/config/test.exs index 0e3aeef..da59415 100644 --- a/config/test.exs +++ b/config/test.exs @@ -33,6 +33,9 @@ config :trinity, :tools, Trinity.Skills.Tools.List, Trinity.Skills.Tools.View, Trinity.Skills.Tools.File, + # Slice 041: proposals and learning, staged for approval. + Trinity.Skills.Tools.Manage, + Trinity.Skills.Tools.Learn, # Slice 030: the always-on memory tiers. Trinity.Tools.Memory, Trinity.Tools.Shell.Run @@ -45,7 +48,7 @@ config :trinity, :tools, # Slice 031: search over past conversations. memory: ["session_search", "recall", "memory"], # Slice 040: the skill tools. - skills: ["skills_list", "skill_view", "skill_file"] + skills: ["skills_list", "skill_view", "skill_file", "skill_manage", "learn"] }, timeout_ms: 2_000 @@ -183,5 +186,8 @@ config :trinity, :memory, embedder: :fake, observer: false config :trinity, :skills, bundled_dir: Path.expand("../test/support/fixtures/skills/bundled", __DIR__), user_dir: Path.expand("../test/support/fixtures/skills/user", __DIR__), + # Slice 041: staged changes go under a scratch pending root; a test that promotes points + # user_dir at a temporary directory first. + pending_dir: Path.join(System.tmp_dir!(), "trinity-test-pending-skills"), watch: true, index_tokens: 338 diff --git a/lib/trinity.ex b/lib/trinity.ex index 4068561..63fff8d 100644 --- a/lib/trinity.ex +++ b/lib/trinity.ex @@ -55,6 +55,11 @@ defmodule Trinity do Skills.Skill, Skills.Registry, Skills.Sources, + Skills.Change, + Skills.Staging, + Skills.Promotion, + Skills.Manager, + Skills.Learn, CorePolicy, Receipts, Receipts.Receipt, diff --git a/lib/trinity/permissions.ex b/lib/trinity/permissions.ex index a72d96a..5d3a3dc 100644 --- a/lib/trinity/permissions.ex +++ b/lib/trinity/permissions.ex @@ -157,6 +157,7 @@ defmodule Trinity.Permissions do @doc "The PubSub topic of a session's approvals, or of all of them." @spec topic(String.t() | :all) :: String.t() def topic(:all), do: "approvals:all" + def topic(nil), do: "approvals:none" def topic(session_id), do: "approvals:" <> session_id @doc "Subscribes the caller to `{:approval, :requested | :decided, %Approval{}}` for a session or all." diff --git a/lib/trinity/permissions/approval.ex b/lib/trinity/permissions/approval.ex index 6210f70..38bf517 100644 --- a/lib/trinity/permissions/approval.ex +++ b/lib/trinity/permissions/approval.ex @@ -45,7 +45,9 @@ defmodule Trinity.Permissions.Approval do def request_changeset(approval, attrs) do approval |> cast(attrs, [:session_id, :tool, :args, :risk, :fingerprint, :expires_at]) - |> validate_required([:session_id, :tool, :risk, :fingerprint, :expires_at]) + # Slice 041: a request may have no session (a staged skill change approved from the + # page); its topic is `approvals:none` and `approvals:all`, its scope `session:none`. + |> validate_required([:tool, :risk, :fingerprint, :expires_at]) |> foreign_key_constraint(:session_id) end diff --git a/lib/trinity/skills.ex b/lib/trinity/skills.ex index 8291996..6c6b73f 100644 --- a/lib/trinity/skills.ex +++ b/lib/trinity/skills.ex @@ -15,8 +15,34 @@ defmodule Trinity.Skills do never depends on Skills. """ use Boundary, - deps: [Trinity, Trinity.Tools, Trinity.Memory], - exports: [Skill, Parser, Sources, Registry, Row, Index, Tools.List, Tools.View, Tools.File] + deps: [ + Trinity, + Trinity.Tools, + Trinity.Memory, + Trinity.Permissions, + Trinity.Receipts, + Trinity.LLM + ], + exports: [ + Skill, + Parser, + Sources, + Registry, + Row, + Index, + Tools.List, + Tools.View, + Tools.File, + Tools.Manage, + Tools.Learn, + Change, + Staging, + Promotion, + Manager, + Scanner, + Diff, + Learn + ] alias Trinity.Skills.Registry diff --git a/lib/trinity/skills/change.ex b/lib/trinity/skills/change.ex new file mode 100644 index 0000000..4b3a0b1 --- /dev/null +++ b/lib/trinity/skills/change.ex @@ -0,0 +1,86 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.Skills.Change do + @moduledoc """ + One row of `skill_changes` (slice 041, docs/05): a staged change to a skill, proposed by the + agent (or the learn flow), with its diff, rationale, the scanner's findings and severity, + and, once decided, the approval that promoted it and the receipt the promotion wrote. + """ + use Ecto.Schema + import Ecto.Changeset + + @primary_key {:id, Trinity.UUID, autogenerate: true} + + @type t :: %__MODULE__{} + + @actions ~w(create patch write_file remove_file delete) + @statuses ~w(pending approved rejected applied failed) + @severities ~w(none low medium high) + + schema "skill_changes" do + field :skill_name, :string + field :action, :string + field :source, :string, default: "user" + field :change_dir, :string + field :diff, :string, default: "" + field :rationale, :string, default: "" + field :destructive, :boolean, default: false + field :digest, :string + field :status, :string, default: "pending" + field :severity, :string, default: "none" + field :findings, :map, default: %{} + field :proposed_by, Trinity.UUID + field :approval_id, Trinity.UUID + field :decided_by, :string + field :decided_at, :utc_datetime_usec + field :comment, :string + field :receipt_hash, :string + field :applied_version, :integer + timestamps(type: :utc_datetime_usec) + end + + @doc "The actions, statuses and severities." + @spec actions() :: [String.t()] + def actions, do: @actions + @spec statuses() :: [String.t()] + def statuses, do: @statuses + @spec severities() :: [String.t()] + def severities, do: @severities + + @doc false + def changeset(change, attrs) do + change + |> cast(attrs, [ + :skill_name, + :action, + :source, + :change_dir, + :diff, + :rationale, + :destructive, + :digest, + :status, + :severity, + :findings, + :proposed_by, + :approval_id, + :decided_by, + :decided_at, + :comment, + :receipt_hash, + :applied_version + ]) + |> validate_required([ + :skill_name, + :action, + :source, + :change_dir, + :digest, + :status, + :severity + ]) + |> validate_inclusion(:action, @actions) + |> validate_inclusion(:status, @statuses) + |> validate_inclusion(:severity, @severities) + end +end diff --git a/lib/trinity/skills/diff.ex b/lib/trinity/skills/diff.ex new file mode 100644 index 0000000..0a1f2e6 --- /dev/null +++ b/lib/trinity/skills/diff.ex @@ -0,0 +1,79 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.Skills.Diff do + @moduledoc """ + A unified diff of two texts (slice 041), line based, from a longest-common-subsequence over + the lines: enough for a SKILL.md and its references, which is all a change carries. A file + that is not text (not valid UTF-8, or over the size limit) is not diffed: the caller says + "replaced, N bytes" instead (SLICE.md's risk line). + """ + + @max_bytes 262_144 + + @doc "The size over which a file is described, not diffed." + @spec max_bytes() :: pos_integer() + def max_bytes, do: @max_bytes + + @doc "True when both sides can be diffed as text." + @spec text?(binary()) :: boolean() + def text?(bin), do: is_binary(bin) and byte_size(bin) <= @max_bytes and String.valid?(bin) + + @doc "A unified diff with the two paths in its header; `\"\"` when the texts are equal." + @spec unified(String.t(), String.t(), String.t(), String.t()) :: String.t() + def unified(a, b, path_a \\ "a", path_b \\ "b") + def unified(same, same, _pa, _pb), do: "" + + def unified(a, b, path_a, path_b) do + la = String.split(a, "\n") + lb = String.split(b, "\n") + + body = + la + |> lcs_diff(lb) + |> Enum.map_join("\n", fn + {:eq, l} -> " " <> l + {:del, l} -> "-" <> l + {:add, l} -> "+" <> l + end) + + "--- #{path_a}\n+++ #{path_b}\n@@ -1,#{length(la)} +1,#{length(lb)} @@\n" <> body + end + + @doc "The edit script: `{:eq | :del | :add, line}` in order." + @spec lcs_diff([String.t()], [String.t()]) :: [{:eq | :del | :add, String.t()}] + def lcs_diff(a, b) do + ta = List.to_tuple(a) + tb = List.to_tuple(b) + n = tuple_size(ta) + m = tuple_size(tb) + # lengths[i][j] = LCS length of a[i..] and b[j..], filled from the end. + table = + for i <- (n - 1)..0//-1, j <- (m - 1)..0//-1, reduce: %{} do + acc -> Map.put(acc, {i, j}, lcs_at(ta, tb, i, j, acc)) + end + + walk(ta, tb, 0, 0, n, m, table, []) + end + + defp lcs_at(ta, tb, i, j, acc) do + if elem(ta, i) == elem(tb, j), + do: 1 + Map.get(acc, {i + 1, j + 1}, 0), + else: max(Map.get(acc, {i + 1, j}, 0), Map.get(acc, {i, j + 1}, 0)) + end + + defp walk(ta, tb, i, j, n, m, table, acc) do + cond do + i < n and j < m and elem(ta, i) == elem(tb, j) -> + walk(ta, tb, i + 1, j + 1, n, m, table, [{:eq, elem(ta, i)} | acc]) + + i < n and (j >= m or Map.get(table, {i + 1, j}, 0) >= Map.get(table, {i, j + 1}, 0)) -> + walk(ta, tb, i + 1, j, n, m, table, [{:del, elem(ta, i)} | acc]) + + j < m -> + walk(ta, tb, i, j + 1, n, m, table, [{:add, elem(tb, j)} | acc]) + + true -> + Enum.reverse(acc) + end + end +end diff --git a/lib/trinity/skills/learn.ex b/lib/trinity/skills/learn.ex new file mode 100644 index 0000000..1409e54 --- /dev/null +++ b/lib/trinity/skills/learn.ex @@ -0,0 +1,170 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.Skills.Learn do + @moduledoc """ + A document into a knowledge skill (slice 041, `/learn`): the source (a file under the + session's roots, a URL, or pasted text) is distilled by the model given into a lean + SKILL.md (the frontmatter and a body under 200 lines) and one `references/` file (the + detail the body points at), then staged as a `create` like any other change: nothing lands + without an approval. The source's text is data; it is sent to the model the operator chose + for the session and to nothing else. + """ + + alias Trinity.LLM + alias Trinity.LLM.Request + alias Trinity.Skills.Staging + alias Trinity.Tools.{Context, FS} + + Module.register_attribute(__MODULE__, :sobelow_skip, persist: true) + + @max_source_bytes 200_000 + @max_body_lines 200 + + @schema %{ + "type" => "object", + "properties" => %{ + "name" => %{"type" => "string"}, + "description" => %{"type" => "string"}, + "category" => %{"type" => "string"}, + "body" => %{"type" => "string"}, + "reference_name" => %{"type" => "string"}, + "reference" => %{"type" => "string"} + }, + "required" => ["name", "description", "body", "reference"] + } + + @doc "The JSON Schema the model answers with." + @spec schema() :: map() + def schema, do: @schema + + @doc """ + Reads the source: `%{"file" => path}` under the session's roots, `%{"url" => url}` through + the web fetch tool, or `%{"text" => text}`; at most #{@max_source_bytes} bytes either way. + """ + @spec read_source(map(), Context.t()) :: {:ok, String.t(), String.t()} | {:error, term()} + # sobelow_skip reason: Traversal.FileModule: the path is resolved by `Trinity.Tools.FS` + # against the session's working directory and read only when inside the configured roots. + @sobelow_skip ["Traversal.FileModule"] + def read_source(%{"file" => path}, %Context{cwd: cwd}) when is_binary(path) do + case FS.resolve(path, cwd) do + {:ok, real, :inside} -> + with {:ok, bytes} <- File.read(real), + do: {:ok, String.slice(bytes, 0, @max_source_bytes), "file:" <> real} + + {:ok, _real, :outside} -> + {:error, {:outside_roots, path}} + end + end + + def read_source(%{"url" => url}, ctx) when is_binary(url) do + case Trinity.Tools.Web.Fetch.execute(%{"url" => url}, ctx) do + {:ok, %{content: text}} -> {:ok, String.slice(text, 0, @max_source_bytes), "url:" <> url} + {:error, reason} -> {:error, reason} + end + end + + def read_source(%{"text" => text}, _ctx) when is_binary(text), + do: {:ok, String.slice(text, 0, @max_source_bytes), "text"} + + def read_source(_, _), do: {:error, {:args, "learn needs file, url or text"}} + + @doc "Asks the model for the skill; the answer cleaned to a name the parser accepts and a body under the line cap." + @spec distil(String.t(), String.t(), keyword()) :: {:ok, map()} | {:error, term()} + def distil(text, source_ref, opts \\ []) do + request = + Request.new!(%{ + model: Keyword.get(opts, :model), + system: + "You turn a document into a skill for a personal agent, in the agentskills.io shape. " <> + "Answer with: name (lowercase words joined by hyphens, at most 64 characters, describing the task the skill covers); " <> + "description (one or two sentences: what the skill does and when to use it, with the words a person would use); " <> + "category (one lowercase word); body (Markdown: the procedure or the knowledge, step by step, under #{@max_body_lines} lines, " <> + "pointing at the reference file for detail); reference_name (a file name like overview.md); " <> + "reference (Markdown: the detail worth keeping from the document, condensed). Keep only what a future task needs. Invent nothing.", + messages: [%{role: "user", content: "Source: #{source_ref}\n\n" <> text}] + }) + + case LLM.generate_object(request, @schema, session_id: Keyword.get(opts, :session_id)) do + {:ok, %{"name" => _, "body" => _} = obj} -> {:ok, clean(obj)} + {:ok, other} -> {:error, {:no_skill, other}} + {:error, reason} -> {:error, reason} + end + end + + @doc "Reads, distils and stages; `opts`: `model:`, `session_id:`." + @spec learn(map(), Context.t(), keyword()) :: + {:ok, Trinity.Skills.Change.t()} | {:error, term()} + def learn(source, %Context{} = ctx, opts \\ []) do + with {:ok, text, ref} <- read_source(source, ctx), + {:ok, skill} <- distil(text, ref, opts) do + md = + "---\nname: #{skill.name}\ndescription: #{yaml_string(skill.description)}\nmetadata:\n category: #{skill.category}\n learned_from: #{yaml_string(ref)}\n---\n\n" <> + skill.body <> "\n" + + Staging.propose( + "create", + skill.name, + %{ + "skill_md" => md, + "files" => %{("references/" <> skill.reference_name) => skill.reference <> "\n"} + }, + rationale: "learned from #{ref}", + proposed_by: ctx.session_id + ) + end + end + + @doc "The page's entry: a source for a project root and persona, without a session (the change's `proposed_by` is nil)." + @spec learn_for(map(), String.t() | nil, map() | nil) :: + {:ok, Trinity.Skills.Change.t()} | {:error, term()} + def learn_for(source, project_root, persona) do + ctx = %Context{cwd: project_root, persona: persona} + learn(source, ctx, model: persona && Map.get(persona, :model)) + end + + defp clean(obj) do + name = + obj["name"] + |> to_string() + |> String.downcase() + |> String.replace(~r/[^a-z0-9]+/, "-") + |> String.trim("-") + |> String.slice(0, 64) + |> String.trim("-") + + body = + obj["body"] + |> to_string() + |> String.split("\n") + |> Enum.take(@max_body_lines) + |> Enum.join("\n") + + ref_name = + (obj["reference_name"] || "overview.md") + |> to_string() + |> String.downcase() + |> String.replace(~r/[^a-z0-9.\-]+/, "-") + |> then(&if(String.ends_with?(&1, ".md"), do: &1, else: &1 <> ".md")) + + %{ + name: if(name == "", do: "learned-skill", else: name), + description: + obj["description"] + |> to_string() + |> String.replace(~r/\s+/, " ") + |> String.slice(0, 1_000), + category: + (obj["category"] || "knowledge") + |> to_string() + |> String.downcase() + |> String.replace(~r/[^a-z0-9]+/, "-"), + body: body, + reference_name: ref_name, + reference: to_string(obj["reference"]) + } + end + + # A double-quoted YAML scalar for a description that may hold a colon or a quote. + defp yaml_string(s), + do: "\"" <> String.replace(String.replace(s, "\\", "\\\\"), "\"", "\\\"") <> "\"" +end diff --git a/lib/trinity/skills/manager.ex b/lib/trinity/skills/manager.ex new file mode 100644 index 0000000..2bc08d8 --- /dev/null +++ b/lib/trinity/skills/manager.ex @@ -0,0 +1,66 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.Skills.Manager do + @moduledoc """ + Decisions on staged changes (slice 041). `approve/2` asks the permission gate for a + `skill_apply` approval naming the change's id and digest (a row and a broadcast on + `approvals:all`, so a gateway sees it too), decides it as the party given, and promotes + through `Trinity.Skills.Promotion.swap/3`; `reject/2` marks the row and removes the staged + files. `auto/2` is the persona's auto-approval: `settings["skills"]["auto_approve"]` is + `"off"` by default, `"low"` applies a change whose severity is `none` or `low` at once as + `"auto"`; `medium` and `high` are never auto-approved, whatever a rule says (AC4). + """ + + alias Trinity.Permissions + alias Trinity.Repo + alias Trinity.Skills.{Change, Promotion, Staging} + + @doc "Approves and promotes a pending change; `opts`: `by:` (default `\"ui\"`), `comment:`, `session_id:`." + @spec approve(Change.t(), keyword()) :: {:ok, Change.t()} | {:error, term()} + def approve(%Change{status: "pending"} = change, opts \\ []) do + by = Keyword.get(opts, :by, "ui") + # The proposing session when there was one; a change approved from the page with none + # requests on `approvals:none` (and `approvals:all`, where a gateway listens). + session_id = Keyword.get(opts, :session_id) || change.proposed_by + args = %{"change_id" => change.id, "digest" => change.digest, "skill" => change.skill_name} + + with {:ok, approval} <- + Permissions.request_approval(session_id, Promotion.tool(), args, risk: :write), + {:ok, _} <- Permissions.decide_request(approval.id, :once, by: by) do + Promotion.swap(change, approval.id, by, Keyword.get(opts, :comment)) + end + end + + @doc "Rejects a pending change: the row says who and why, the staged files go." + @spec reject(Change.t(), keyword()) :: {:ok, Change.t()} | {:error, term()} + def reject(%Change{status: "pending"} = change, opts \\ []) do + :ok = Staging.discard(change) + + change + |> Change.changeset(%{ + status: "rejected", + decided_by: Keyword.get(opts, :by, "ui"), + decided_at: DateTime.utc_now(), + comment: Keyword.get(opts, :comment) + }) + |> Repo.update() + end + + @doc "The persona's auto-approval, applied to a fresh change when it qualifies; `{:ok, change}` either way." + @spec auto(Change.t(), map() | nil) :: {:ok, Change.t()} + def auto(%Change{status: "pending", severity: severity} = change, persona) do + if auto_approve?(persona) and severity in ["none", "low"] do + case approve(change, by: "auto") do + {:ok, applied} -> {:ok, applied} + {:error, _} -> {:ok, change} + end + else + {:ok, change} + end + end + + @doc "True when the persona's settings allow auto-approval of low-severity changes." + @spec auto_approve?(map() | nil) :: boolean() + def auto_approve?(%{settings: %{"skills" => %{"auto_approve" => "low"}}}), do: true + def auto_approve?(_), do: false +end diff --git a/lib/trinity/skills/promotion.ex b/lib/trinity/skills/promotion.ex new file mode 100644 index 0000000..20a9ca6 --- /dev/null +++ b/lib/trinity/skills/promotion.ex @@ -0,0 +1,162 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.Skills.Promotion do + @moduledoc """ + The one path that moves a staged change into a skill root (slice 041, AC8: the census over + the tree finds exactly one caller of `swap/3`, `Trinity.Skills.Manager`). `swap/3` takes + the change, an approval id and the deciding party, and refuses by name unless the approval + row exists, was allowed, was for `skill_apply`, and names this change's id and digest in + its arguments (the fingerprint 021 bound); then it recomputes the pending tree's digest + against the row's, archives the skill as it is under `/.history//v/`, + renames the pending tree into place (atomic per directory), rescans the registry, writes an + `effect` receipt on the `skills` chain scope carrying the change's digest and the approval + id, and marks the row `applied` with the version now in the registry. A step that fails + after the rename leaves the row `failed` with the reason; the files are in place and the + receipt says what happened, or the alarm does. + """ + + alias Trinity.Permissions.Approval + alias Trinity.Repo + alias Trinity.Skills.{Change, Registry, Sources, Staging} + + Module.register_attribute(__MODULE__, :sobelow_skip, persist: true) + + @tool "skill_apply" + @scope "skills" + + @doc "The tool name the approval carries." + @spec tool() :: String.t() + def tool, do: @tool + + @doc "The receipt chain scope promotions write on." + @spec scope() :: String.t() + def scope, do: @scope + + @doc "Promotes a pending change under an approval; every refusal is named. `comment` is the decider's, kept on the row." + @spec swap(Change.t(), String.t() | nil, String.t(), String.t() | nil) :: + {:ok, Change.t()} | {:error, term()} + def swap(change, approval_id, by, comment \\ nil) + + def swap(%Change{} = change, approval_id, by, comment) + when is_binary(approval_id) and is_binary(by) do + with {:ok, approval} <- approval(change, approval_id), + :ok <- pending?(change), + :ok <- digest_holds?(change) do + promote(change, approval, by, comment) + end + end + + def swap(%Change{}, nil, _by, _comment), do: {:error, :approval_required} + + defp approval(change, id) do + case Trinity.Permissions.get_approval(id) do + nil -> + {:error, {:no_such_approval, id}} + + %Approval{tool: @tool, status: "allowed", args: args} = a -> + if args["change_id"] == change.id and args["digest"] == change.digest, + do: {:ok, a}, + else: {:error, {:approval_for_another_change, id}} + + %Approval{tool: @tool, status: status} -> + {:error, {:approval_not_allowed, status}} + + %Approval{tool: other} -> + {:error, {:approval_for_another_tool, other}} + end + end + + defp pending?(%Change{status: "pending"}), do: :ok + defp pending?(%Change{status: s}), do: {:error, {:not_pending, s}} + + defp digest_holds?(%Change{change_dir: dir, digest: digest}) do + cond do + not File.dir?(dir) -> {:error, :staged_files_missing} + Staging.digest(dir) != digest -> {:error, :staged_files_changed} + true -> :ok + end + end + + # sobelow_skip reason: Traversal.FileModule: the paths are the target root joined with the + # change's skill name (validated at staging) and the change's own pending directory. + @sobelow_skip ["Traversal.FileModule"] + defp promote(%Change{} = change, %Approval{} = approval, by, comment) do + root = Path.expand(Sources.user_dir()) + target = Path.join(root, change.skill_name) + File.mkdir_p!(root) + previous = Registry.get(change.skill_name) + version_before = if previous && previous.source == "user", do: previous.version, else: 0 + archive!(target, root, change.skill_name, version_before) + + if change.action == "delete" do + File.rm_rf!(change.change_dir) + else + File.rename!(change.change_dir, target) + end + + Registry.rescan() + now = Registry.get(change.skill_name) + + version = + if change.action == "delete", do: nil, else: now && now.source == "user" && now.version + + receipt = + Trinity.Receipts.append(@scope, %{ + kind: "effect", + subject: %{ + "skill" => change.skill_name, + "action" => change.action, + "change_id" => change.id, + "digest" => change.digest, + "approval_id" => approval.id, + "decided_by" => by, + "version" => version + }, + subject_ref: "skill:#{change.skill_name}@#{change.digest}", + meta: %{"phase" => "done", "destructive" => change.destructive} + }) + + case receipt do + {:ok, r} -> + change + |> Change.changeset(%{ + status: "applied", + approval_id: approval.id, + decided_by: by, + decided_at: DateTime.utc_now(), + receipt_hash: r.receipt_hash, + applied_version: version, + comment: comment + }) + |> Repo.update() + + {:error, reason} -> + {:ok, _} = + change + |> Change.changeset(%{ + status: "failed", + approval_id: approval.id, + decided_by: by, + comment: "receipt: #{inspect(reason)}" + }) + |> Repo.update() + + {:error, {:promoted_but_not_receipted, reason}} + end + end + + # The skill as it is, if it exists, to `.history//v/`. + # sobelow_skip reason: Traversal.FileModule: `target` and the history path are the root + # joined with the validated name and a version number. + @sobelow_skip ["Traversal.FileModule"] + defp archive!(target, root, name, version) do + if File.dir?(target) do + history = Path.join([root, ".history", name, "v#{version}"]) + File.rm_rf!(history) + File.mkdir_p!(Path.dirname(history)) + File.rename!(target, history) + end + + :ok + end +end diff --git a/lib/trinity/skills/scanner.ex b/lib/trinity/skills/scanner.ex new file mode 100644 index 0000000..731b8ab --- /dev/null +++ b/lib/trinity/skills/scanner.ex @@ -0,0 +1,103 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.Skills.Scanner do + @moduledoc """ + Heuristics over a staged skill's files (slice 041, docs/07): shell pipes into a shell and + destructive commands, credential shapes and instructions to ignore or disable safety are + `high`; plain shell commands, network calls, external URLs and base64 blobs are `medium`. + A file the scanner cannot read as text (binary, over the size limit, not UTF-8) is a `low` + finding naming the file and why: nothing is dropped in silence. The scan runs on the + pending directory, before promotion; `high` blocks auto-approval whatever a rule says. + """ + + Module.register_attribute(__MODULE__, :sobelow_skip, persist: true) + + @max_bytes 262_144 + + @type finding :: %{ + file: String.t(), + line: non_neg_integer(), + rule: String.t(), + severity: String.t(), + match: String.t() + } + + @rules [ + {"shell_pipe", "high", ~r/\b(curl|wget)\b[^\n|]*\|\s*(sudo\s+)?(sh|bash|zsh|python\d?)\b/}, + {"destructive_command", "high", + ~r/\b(rm\s+-rf\s+[\/~]|mkfs\.|dd\s+if=|:\(\)\s*\{\s*:\|:&\s*\};:)/}, + {"credential", "high", + ~r/(AKIA[0-9A-Z]{16}|sk-[A-Za-z0-9]{20,}|ghp_[A-Za-z0-9]{30,}|xox[baprs]-[A-Za-z0-9-]{10,}|-----BEGIN [A-Z ]*PRIVATE KEY-----|\b(api[_-]?key|secret|token|password)\b\s*[:=]\s*["']?[A-Za-z0-9_\-\/+=]{16,})/i}, + {"instruction_override", "high", + ~r/\b(ignore|disregard|forget)\b[^\n]{0,40}\b(previous|prior|above|earlier|all)\b[^\n]{0,40}\b(instructions?|rules?|prompts?)\b|\b(disable|turn off|bypass)\b[^\n]{0,30}\b(safety|permission|approval|guard|sandbox)/i}, + {"shell_command", "medium", + ~r/(^|\n)\s*(\$\s+)?(sudo|chmod|chown|curl|wget|ssh|scp|nc|netcat|powershell|bash\s+-c)\b/}, + {"network_call", "medium", + ~r/\b(fetch|requests\.(get|post)|http\.(get|post)|urllib|socket\.connect|Net::HTTP)\b/}, + {"external_url", "medium", ~r/https?:\/\/(?!localhost|127\.0\.0\.1)[^\s)>"']+/}, + {"base64_blob", "medium", ~r/[A-Za-z0-9+\/]{200,}={0,2}/} + ] + + @doc "The rules: `{name, severity}`." + @spec rules() :: [{String.t(), String.t()}] + def rules, do: Enum.map(@rules, fn {n, s, _} -> {n, s} end) + + @doc "Scans every file under `dir`; findings sorted by severity then file and line." + @spec scan_dir(Path.t()) :: [finding()] + # sobelow_skip reason: Traversal.FileModule: the walk reads what Path.wildcard found under the + # pending directory the staging built; nothing from a request. + @sobelow_skip ["Traversal.FileModule"] + def scan_dir(dir) do + dir = Path.expand(dir) + + dir + |> Path.join("**") + |> Path.wildcard(match_dot: true) + |> Enum.filter(&File.regular?/1) + |> Enum.sort() + |> Enum.flat_map(fn path -> scan_file(Path.relative_to(path, dir), File.read!(path)) end) + |> Enum.sort_by(&{rank(&1.severity), &1.file, &1.line}) + end + + @doc "Scans one file's bytes; a file that is not readable text is one `low` finding." + @spec scan_file(String.t(), binary()) :: [finding()] + def scan_file(file, bytes) do + cond do + byte_size(bytes) > @max_bytes -> + [excluded(file, "over #{@max_bytes} bytes (#{byte_size(bytes)}), not scanned")] + + not String.valid?(bytes) -> + [excluded(file, "not UTF-8 text, not scanned")] + + true -> + for {rule, severity, re} <- @rules, + [{start, len}] <- Regex.scan(re, bytes, return: :index) |> Enum.map(&[hd(&1)]), + do: %{ + file: file, + line: line_of(bytes, start), + rule: rule, + severity: severity, + match: bytes |> binary_part(start, min(len, 120)) |> String.slice(0, 120) + } + end + end + + @doc "The highest severity among findings (`none` for none)." + @spec severity([finding()]) :: String.t() + def severity([]), do: "none" + def severity(findings), do: findings |> Enum.map(& &1.severity) |> Enum.min_by(&rank/1) + + @doc "The order: high first." + @spec rank(String.t()) :: non_neg_integer() + def rank("high"), do: 0 + def rank("medium"), do: 1 + def rank("low"), do: 2 + def rank(_), do: 3 + + defp excluded(file, why), + do: %{file: file, line: 0, rule: "excluded", severity: "low", match: why} + + defp line_of(bytes, start) do + bytes |> binary_part(0, start) |> String.split("\n") |> length() + end +end diff --git a/lib/trinity/skills/staging.ex b/lib/trinity/skills/staging.ex new file mode 100644 index 0000000..412633d --- /dev/null +++ b/lib/trinity/skills/staging.ex @@ -0,0 +1,303 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.Skills.Staging do + @moduledoc """ + The proposer (slice 041): every change to a skill is staged as the whole target tree as it + would be after the change, under `/pending/skills///`, with a + unified diff against the skill as it is, the scanner's findings and a `skill_changes` row. + Nothing here writes anywhere but the pending directory (the census, AC8, holds it to that), + and the pending directory is outside every root the registry scans, so a staged skill + never loads until `Trinity.Skills.Promotion` moves it, and only with an approval. + + Actions: `create` (a new skill: `SKILL.md` and optional `files`), `patch` (a unified diff or + a whole-body replace of `SKILL.md`, the replace tagged destructive), `write_file` (one file + under the skill, added or replaced), `remove_file`, `delete` (the whole skill: destructive). + """ + + import Ecto.Query + + alias Trinity.Repo + alias Trinity.Skills.{Change, Diff, Parser, Scanner, Sources} + + Module.register_attribute(__MODULE__, :sobelow_skip, persist: true) + + @delete_marker ".trinity-delete" + + @doc "The pending root in force (`config :trinity, :skills, pending_dir:`; the data directory's `pending/skills`)." + @spec pending_dir() :: Path.t() + def pending_dir do + Application.get_env(:trinity, :skills, [])[:pending_dir] || + Path.join(Trinity.Paths.data_dir(), "pending/skills") + end + + @doc "The marker file a staged delete carries." + @spec delete_marker() :: String.t() + def delete_marker, do: @delete_marker + + @doc """ + Stages a change. `args` by action: `create` needs `"skill_md"` and may carry `"files"` + (a map of relative path to content); `patch` needs `"diff"` (a unified diff of `SKILL.md`) + or `"skill_md"` (a replace); `write_file` needs `"path"` and `"content"`; `remove_file` + needs `"path"`; `delete` needs nothing. `opts`: `rationale:`, `proposed_by:` (a session id). + """ + @spec propose(String.t(), String.t(), map(), keyword()) :: {:ok, Change.t()} | {:error, term()} + def propose(action, name, args, opts \\ []) + when action in ~w(create patch write_file remove_file delete) do + with :ok <- valid_name(name), + :ok <- not_already_proposed(action, name), + {:ok, current} <- current_tree(name, action), + {:ok, tree, destructive?} <- next_tree(action, current, args), + :ok <- valid_result(action, name, tree) do + id = Trinity.UUID.generate() + dir = Path.join([pending_dir(), name, id]) + write_tree!(dir, tree) + findings = Scanner.scan_dir(dir) + + attrs = %{ + skill_name: name, + action: action, + source: "user", + change_dir: dir, + diff: render_diff(current, tree, name), + rationale: to_string(Keyword.get(opts, :rationale, "")), + destructive: destructive?, + digest: digest(dir), + status: "pending", + severity: Scanner.severity(findings), + findings: %{ + "findings" => Enum.map(findings, &Map.new(&1, fn {k, v} -> {Atom.to_string(k), v} end)) + }, + proposed_by: Keyword.get(opts, :proposed_by) + } + + %Change{id: id} |> Change.changeset(attrs) |> Repo.insert() + end + end + + @doc "Pending changes, oldest first; `status:` for another status; `name:` for one skill." + @spec list(keyword()) :: [Change.t()] + def list(opts \\ []) do + status = Keyword.get(opts, :status, "pending") + + from(c in Change, where: c.status == ^status, order_by: c.inserted_at) + |> then(fn q -> if n = opts[:name], do: where(q, [c], c.skill_name == ^n), else: q end) + |> Repo.all() + end + + @doc "A change by id." + @spec get(String.t()) :: Change.t() | nil + def get(id), do: Repo.get(Change, id) + + @doc "The SHA-256 over a tree: every regular file's relative path and bytes, sorted; the promotion recomputes it." + @spec digest(Path.t()) :: String.t() + # sobelow_skip reason: Traversal.FileModule: the walk reads under the pending directory the + # staging built (or a skill root the promotion checks); the paths are Path.wildcard's. + @sobelow_skip ["Traversal.FileModule"] + def digest(dir) do + dir = Path.expand(dir) + + dir + |> Path.join("**") + |> Path.wildcard(match_dot: true) + |> Enum.filter(&File.regular?/1) + |> Enum.sort() + |> Enum.reduce(:crypto.hash_init(:sha256), fn path, acc -> + rel = Path.relative_to(path, dir) + + acc + |> :crypto.hash_update(rel <> "\0") + |> :crypto.hash_update(File.read!(path)) + |> :crypto.hash_update("\0") + end) + |> :crypto.hash_final() + |> Base.encode16(case: :lower) + end + + @doc "The staged tree of a change as a map of relative path to bytes (the delete marker included)." + @spec read_tree(Path.t()) :: %{String.t() => binary()} + # sobelow_skip reason: Traversal.FileModule: reads under a change directory the staging built. + @sobelow_skip ["Traversal.FileModule"] + def read_tree(dir) do + dir = Path.expand(dir) + + dir + |> Path.join("**") + |> Path.wildcard(match_dot: true) + |> Enum.filter(&File.regular?/1) + |> Map.new(fn p -> {Path.relative_to(p, dir), File.read!(p)} end) + end + + @doc "Removes a change's staged files (a rejection, or after a promotion)." + @spec discard(Change.t()) :: :ok + # sobelow_skip reason: Traversal.FileModule: the directory removed is the row's `change_dir`, + # which this module wrote under the pending root, and it is checked to be under it first. + @sobelow_skip ["Traversal.FileModule"] + def discard(%Change{change_dir: dir}) do + root = Path.expand(pending_dir()) + if String.starts_with?(Path.expand(dir), root <> "/"), do: File.rm_rf!(dir) + :ok + end + + ## The next tree + + # One pending create per name: a second proposal of the same new skill waits for the first. + defp not_already_proposed("create", name) do + if Enum.any?(list(name: name), &(&1.action == "create")), + do: {:error, {:pending, name}}, + else: :ok + end + + defp not_already_proposed(_action, _name), do: :ok + + defp valid_name(name) do + if Regex.match?(~r/^[a-z0-9]+(-[a-z0-9]+)*$/, name) and String.length(name) <= 64, + do: :ok, + else: {:error, {:name, "lowercase a-z, 0-9 and single hyphens, at most 64"}} + end + + # The skill as it is in the user root (the target), as a tree; `create` needs it absent. + defp current_tree(name, action) do + dir = Path.join(Sources.user_dir(), name) + + case {File.dir?(dir), action} do + {true, "create"} -> {:error, {:exists, name}} + {false, "create"} -> {:ok, %{}} + {false, _} -> {:error, {:no_such_skill, name}} + {true, _} -> {:ok, read_tree(dir)} + end + end + + defp next_tree("create", _current, %{"skill_md" => md} = args) when is_binary(md) do + files = + for {p, c} <- Map.get(args, "files", %{}), is_binary(p), is_binary(c), into: %{}, do: {p, c} + + with :ok <- valid_paths(Map.keys(files)), do: {:ok, Map.put(files, "SKILL.md", md), false} + end + + defp next_tree("create", _, _), do: {:error, {:args, "create needs skill_md"}} + + defp next_tree("patch", current, %{"skill_md" => md}) when is_binary(md), + do: {:ok, Map.put(current, "SKILL.md", md), true} + + defp next_tree("patch", current, %{"diff" => diff}) when is_binary(diff) do + with {:ok, patched} <- apply_diff(Map.get(current, "SKILL.md", ""), diff), + do: {:ok, Map.put(current, "SKILL.md", patched), false} + end + + defp next_tree("patch", _, _), do: {:error, {:args, "patch needs diff or skill_md"}} + + defp next_tree("write_file", current, %{"path" => path, "content" => content}) + when is_binary(path) and is_binary(content) do + with :ok <- valid_paths([path]), + do: {:ok, Map.put(current, path, content), Map.has_key?(current, path)} + end + + defp next_tree("write_file", _, _), do: {:error, {:args, "write_file needs path and content"}} + + defp next_tree("remove_file", current, %{"path" => path}) when is_binary(path) do + cond do + path == "SKILL.md" -> {:error, {:args, "SKILL.md cannot be removed; delete the skill"}} + not Map.has_key?(current, path) -> {:error, {:no_such_file, path}} + true -> {:ok, Map.delete(current, path), false} + end + end + + defp next_tree("remove_file", _, _), do: {:error, {:args, "remove_file needs path"}} + defp next_tree("delete", _current, _), do: {:ok, %{@delete_marker => "delete\n"}, true} + + defp valid_paths(paths) do + bad = + Enum.find(paths, fn p -> + p == "" or String.starts_with?(p, "/") or String.contains?(p, "..") or + String.contains?(p, "\\") or String.starts_with?(p, ".trinity") + end) + + if bad, do: {:error, {:path, bad}}, else: :ok + end + + # The result must be a skill the parser accepts (a delete excepted), so a bad proposal is + # refused here rather than at promotion. + defp valid_result("delete", _name, _tree), do: :ok + + defp valid_result(_action, name, tree) do + case Parser.parse(Map.get(tree, "SKILL.md", ""), name) do + {:ok, _} -> :ok + {:error, reason} -> {:error, {:invalid_skill, reason}} + end + end + + # A unified diff as `Trinity.Skills.Diff` writes it (or any with the same line marks), applied + # to the current text; refused when a context or removed line is not where it says. + defp apply_diff(current, diff) do + lines = String.split(current, "\n") + + ops = + diff + |> String.split("\n") + |> Enum.reject( + &(String.starts_with?(&1, "---") or String.starts_with?(&1, "+++") or + String.starts_with?(&1, "@@")) + ) + |> Enum.map(fn + " " <> l -> {:eq, l} + "-" <> l -> {:del, l} + "+" <> l -> {:add, l} + "" -> {:eq, ""} + other -> {:bad, other} + end) + + case Enum.find(ops, &match?({:bad, _}, &1)) do + {:bad, l} -> {:error, {:diff, "unreadable line: #{inspect(l)}"}} + nil -> replay(ops, lines, []) + end + end + + defp replay([], rest, acc), do: {:ok, Enum.join(Enum.reverse(acc) ++ rest, "\n")} + defp replay([{:add, l} | ops], rest, acc), do: replay(ops, rest, [l | acc]) + defp replay([{:eq, l} | ops], [l | rest], acc), do: replay(ops, rest, [l | acc]) + defp replay([{:del, l} | ops], [l | rest], acc), do: replay(ops, rest, acc) + + defp replay([{op, l} | _], _rest, _acc), + do: {:error, {:diff, "#{op} of #{inspect(l)} does not match the skill as it is"}} + + ## Rendering and writing + + defp render_diff(current, tree, name) do + paths = (Map.keys(current) ++ Map.keys(tree)) |> Enum.uniq() |> Enum.sort() + + Enum.map_join(paths, "\n", fn p -> + a = Map.get(current, p, "") + b = Map.get(tree, p, "") + + cond do + a == b -> + "" + + p == @delete_marker -> + "--- #{name}/\n+++ (deleted)\n" + + not Diff.text?(a) or not Diff.text?(b) -> + "--- #{name}/#{p}\n+++ #{name}/#{p}\n(replaced, #{byte_size(b)} bytes; not text or too large to diff)\n" + + true -> + Diff.unified(a, b, "#{name}/#{p}", "#{name}/#{p}") + end + end) + |> String.trim() + end + + # sobelow_skip reason: Traversal.FileModule: the writes land under `dir`, a fresh directory + # under the pending root with a generated id; the relative paths were checked by valid_paths/1. + @sobelow_skip ["Traversal.FileModule"] + defp write_tree!(dir, tree) do + File.mkdir_p!(dir) + + for {rel, content} <- tree do + path = Path.join(dir, rel) + File.mkdir_p!(Path.dirname(path)) + File.write!(path, content) + end + + :ok + end +end diff --git a/lib/trinity/skills/tools/learn.ex b/lib/trinity/skills/tools/learn.ex new file mode 100644 index 0000000..2e1ff1a --- /dev/null +++ b/lib/trinity/skills/tools/learn.ex @@ -0,0 +1,53 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.Skills.Tools.Learn do + @moduledoc """ + `learn` (slice 041): a document (a file under the session's roots, a URL, or text) into a + staged knowledge skill through `Trinity.Skills.Learn`, with the session's own model. Risk + `:write`, effect `:artifact` like `skill_manage`; the staged change is decided on `/skills`. + """ + @behaviour Trinity.Tools.Tool + + alias Trinity.Skills.{Learn, Manager} + alias Trinity.Tools.{Context, Result} + + @impl true + def name, do: "learn" + + @impl true + def description, + do: + "Distils a document into a knowledge skill (a SKILL.md and a reference file) and stages it for the person's approval. Give one of file (a path in the project), url, or text." + + @impl true + def schema, + do: %{ + "type" => "object", + "properties" => %{ + "file" => %{"type" => "string", "description" => "A file path under the project"}, + "url" => %{"type" => "string", "description" => "A web page"}, + "text" => %{"type" => "string", "description" => "The document itself"} + }, + "additionalProperties" => false + } + + @impl true + def risk, do: :write + + @impl true + def effect, do: :artifact + + @impl true + def execute(args, %Context{session_id: sid, persona: persona} = ctx) do + model = persona && Map.get(persona, :model) + + with {:ok, change} <- Learn.learn(args, ctx, model: model, session_id: sid), + {:ok, change} <- Manager.auto(change, persona) do + {:ok, + Result.text( + "Staged the learned skill #{change.skill_name} (change #{change.id}, severity #{change.severity}, status #{change.status}). The person decides on the skills page.", + %{"change_id" => change.id, "skill" => change.skill_name, "status" => change.status} + )} + end + end +end diff --git a/lib/trinity/skills/tools/manage.ex b/lib/trinity/skills/tools/manage.ex new file mode 100644 index 0000000..9c4a078 --- /dev/null +++ b/lib/trinity/skills/tools/manage.ex @@ -0,0 +1,93 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.Skills.Tools.Manage do + @moduledoc """ + `skill_manage` (slice 041): the agent proposes a skill or a change to one. Every action is + staged by `Trinity.Skills.Staging` (never applied), answers with the change id, the + scanner's severity and where the owner decides (`/skills`), and then the persona's + auto-approval is tried (`Trinity.Skills.Manager.auto/2`: off by default; `high` never). + Risk `:write`, effect `:artifact`, as `fs_write` is: the call is receipted through the + membrane, and under the default policy it asks; an "always allow" on it makes proposing + free while the promotion stays gated, which is the point of staging. + """ + @behaviour Trinity.Tools.Tool + + alias Trinity.Skills.{Manager, Staging} + alias Trinity.Tools.{Context, Result} + + @impl true + def name, do: "skill_manage" + + @impl true + def description, + do: + "Proposes a new skill or a change to one. Nothing is applied: the proposal is staged with a diff and scanned, and the person approves it on the skills page. Actions: create (skill_md, optional files), patch (a unified diff of SKILL.md, or skill_md to replace it), write_file (path, content), remove_file (path), delete. Give a rationale." + + @impl true + def schema, + do: %{ + "type" => "object", + "properties" => %{ + "action" => %{ + "type" => "string", + "enum" => ["create", "patch", "write_file", "remove_file", "delete"] + }, + "name" => %{ + "type" => "string", + "description" => "The skill's name: lowercase, digits, hyphens; the directory name" + }, + "rationale" => %{ + "type" => "string", + "description" => "Why this change, for the person deciding" + }, + "skill_md" => %{ + "type" => "string", + "description" => "create or patch: the whole SKILL.md (frontmatter and body)" + }, + "diff" => %{"type" => "string", "description" => "patch: a unified diff of SKILL.md"}, + "files" => %{ + "type" => "object", + "additionalProperties" => %{"type" => "string"}, + "description" => "create: relative path to content, for references/" + }, + "path" => %{ + "type" => "string", + "description" => "write_file or remove_file: a path relative to the skill" + }, + "content" => %{"type" => "string", "description" => "write_file: the file's content"} + }, + "required" => ["action", "name", "rationale"], + "additionalProperties" => false + } + + @impl true + def risk, do: :write + + @impl true + def effect, do: :artifact + + @impl true + def execute(%{"action" => action, "name" => name} = args, %Context{ + session_id: sid, + persona: persona + }) do + with {:ok, change} <- + Staging.propose(action, name, args, rationale: args["rationale"], proposed_by: sid), + {:ok, change} <- Manager.auto(change, persona) do + {:ok, + Result.text(answer(change), %{ + "change_id" => change.id, + "severity" => change.severity, + "status" => change.status + })} + end + end + + defp answer(%{status: "applied"} = c), + do: + "Applied: #{c.action} of #{c.skill_name} was auto-approved (severity #{c.severity}); it is live at version #{c.applied_version || "none"}." + + defp answer(c), + do: + "Staged: #{c.action} of #{c.skill_name} (change #{c.id}, severity #{c.severity}#{if c.destructive, do: ", destructive", else: ""}). It is not applied: the person decides on the skills page. Do not retry." +end diff --git a/lib/trinity/tools.ex b/lib/trinity/tools.ex index 26de425..5892a57 100644 --- a/lib/trinity/tools.ex +++ b/lib/trinity/tools.ex @@ -15,7 +15,7 @@ defmodule Trinity.Tools do deps: [Trinity, Trinity.Permissions, Trinity.Memory], # Slice 040 exports Untrusted: the skill tools (Trinity.Skills.Tools.*) wrap their results # the way session_search does, and they live in the Skills boundary. - exports: [Tool, Context, Result, Registry, Runner, Schema, Catalog, Untrusted] + exports: [Tool, Context, Result, Registry, Runner, Schema, Catalog, Untrusted, FS, Web.Fetch] alias Trinity.Sessions.Message alias Trinity.Tools.Registry diff --git a/lib/trinity_web/live/skills_live.ex b/lib/trinity_web/live/skills_live.ex index 36bb52a..89856e6 100644 --- a/lib/trinity_web/live/skills_live.ex +++ b/lib/trinity_web/live/skills_live.ex @@ -11,14 +11,16 @@ defmodule TrinityWeb.SkillsLive do use TrinityWeb, :live_view alias Trinity.Skills - alias Trinity.Skills.Registry + alias Trinity.Skills.{Manager, Registry, Staging} @impl true def mount(params, _session, socket) do project = params["project"] {:ok, - socket |> assign(page_title: gettext("Skills"), project: project, viewing: nil) |> load()} + socket + |> assign(page_title: gettext("Skills"), project: project, viewing: nil, showing: nil) + |> load()} end defp load(%{assigns: %{project: project}} = socket) do @@ -29,12 +31,76 @@ defmodule TrinityWeb.SkillsLive do skills: skills, active: active, errors: Registry.errors(), + pending: Staging.list(), + recent: Staging.list(status: "applied") |> Enum.take(-5), viewing: socket.assigns.viewing && Enum.find(skills, &(&1.name == socket.assigns.viewing.name)) ) end + # Slice 041: the staged changes. @impl true + def handle_event("show_change", %{"id" => id}, socket) do + {:noreply, assign(socket, showing: Enum.find(socket.assigns.pending, &(&1.id == id)))} + end + + def handle_event("hide_change", _params, socket), do: {:noreply, assign(socket, showing: nil)} + + def handle_event("approve_change", %{"change_id" => id} = params, socket) do + with %{} = change <- Staging.get(id), + {:ok, _} <- Manager.approve(change, by: "ui", comment: blank_to_nil(params["comment"])) do + {:noreply, + socket + |> assign(showing: nil) + |> load() + |> put_flash(:info, gettext("Applied %{name}.", name: change.skill_name))} + else + nil -> + {:noreply, put_flash(socket, :error, gettext("No such change."))} + + {:error, reason} -> + {:noreply, + socket |> load() |> put_flash(:error, gettext("Not applied: %{r}", r: inspect(reason)))} + end + end + + def handle_event("reject_change", %{"id" => id} = params, socket) do + with %{} = change <- Staging.get(id), + {:ok, _} <- Manager.reject(change, by: "ui", comment: blank_to_nil(params["comment"])) do + {:noreply, + socket |> assign(showing: nil) |> load() |> put_flash(:info, gettext("Rejected."))} + else + _ -> {:noreply, socket |> load() |> put_flash(:error, gettext("Not rejected."))} + end + end + + # The learn form: a URL or a file under the project's roots, distilled by the default + # persona's model, staged like any change. + def handle_event("learn", %{"source" => source}, socket) do + source = String.trim(source) + persona = Trinity.Personas.default() + + args = + if String.match?(source, ~r/^https?:\/\//), + do: %{"url" => source}, + else: %{"file" => source} + + case Skills.Learn.learn_for(args, socket.assigns.project, persona) do + {:ok, change} -> + {:noreply, + socket + |> load() + |> put_flash( + :info, + gettext("Staged the learned skill %{name} for your approval.", name: change.skill_name) + )} + + {:error, reason} -> + {:noreply, + put_flash(socket, :error, gettext("Nothing learned: %{r}", r: inspect(reason)))} + end + end + def handle_event("view", %{"name" => name}, socket) do {:noreply, assign(socket, viewing: Enum.find(socket.assigns.skills, &(&1.name == name)))} end @@ -65,6 +131,9 @@ defmodule TrinityWeb.SkillsLive do def handle_event("cancel", _params, socket), do: {:noreply, socket} + defp blank_to_nil(nil), do: nil + defp blank_to_nil(s), do: if(String.trim(s) == "", do: nil, else: String.trim(s)) + @impl true def render(assigns) do ~H""" @@ -87,6 +156,99 @@ defmodule TrinityWeb.SkillsLive do +
+

+ {gettext("Pending changes")} + {length(@pending)} +

+

{gettext("Nothing waiting for a decision.")}

+
    +
  • + {c.severity} + {c.action} + + {c.rationale} + {gettext( + "destructive" + )} + {Calendar.strftime( + c.inserted_at, + "%Y-%m-%d %H:%M" + )} +
    • +
+ +
+
+

{@showing.action} {@showing.skill_name}

+ {@showing.severity} + {@showing.id} + + +
+

{@showing.rationale}

+

+ {gettext("High severity: never auto-approved. Read the findings before approving.")} +

+

{gettext("Findings")}

+

{gettext("None.")}

+
    +
  • + [{f["severity"]}] {f["rule"]} {f["file"]}:{f["line"]} {f["match"]} +
    • +
+

{gettext("Diff")}

+ 
{@showing.diff}
+
+ + + + +
+
+ +
+ + +
+ +
    +
  • + {gettext("applied")} {c.action} {c.skill_name} v{c.applied_version || "-"} {gettext( + "by" + )} {c.decided_by} +
    • +
+
+

{gettext("No skills found under any root.")}