feat(s041): complete slice 041 (skill self-management with approval)

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 | done |
 | 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 |

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

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

coverage.tsv

@@ -17,3 +17,4 @@ slice_id	percent	sha	date
 034	78.83	3a5785f	2026-09-21
 032	78.99	193800e	2026-09-21
 040	79.86	dc9dc6b	2026-09-21
+041	80.55	ae200d7	2026-09-21

docs/01-architecture.md

@@ -88,7 +88,7 @@ without anything failing.
 | `Trinity.Authority` | Behaviour; `Local` implementation (the one caller of `execute/2` for effectful tools); selection at boot; `Staged` | Receipts, Repo |
 | `Trinity.Receipts` | Local chain (one supervised writer per scope, ADR-0013), the signer seam (Ed25519, P-384, ML-DSA-87), key custody and the registry, checkpoints, the verifier, the alarm | Repo (`Repo.Receipts`) |
 | `Trinity.Memory` | Always-on tiers with their budget and consolidator (030), search (031), semantic store and retrieval (032), compaction (023) | LLM (summaries/embeddings), Repo |
-| `Trinity.Skills` | SKILL.md parsing, registry, loader, manager, scanner (as built at 040: parser, sources, registry, index, the three tools) | Repo, Permissions, **Effects**, **Receipts**, Sandbox (as built at 040: Tools, for conditional activation and the tool behaviour, and Memory, for the token estimator; Tools never depends on Skills) |
+| `Trinity.Skills` | SKILL.md parsing, registry, loader, manager, scanner (as built at 040: parser, sources, registry, index, the three tools; at 041: staging, promotion, manager, scanner, diff, learn, `skill_manage` and `learn`) | Repo, Permissions, **Effects**, **Receipts**, Sandbox (as built at 041: Tools, Memory, Permissions, Receipts and LLM; Effects is not a dependency: the promotion is not a tool call, it writes its own effect receipt; Tools never depends on Skills) |
 | `Trinity.Scheduler` | Oban workers for agent tasks, delivery | Sessions, Gateways, **Repo** |
 | `Trinity.MCP` | Client manager, tool bridge, server | Tools, **Effects**, **Permissions**, Memory |
 | `Trinity.Gateways` | Adapter behaviour, router, allowlists, pairing | Sessions, **Permissions**, PubSub |

docs/05-data-model.md

@@ -129,6 +129,16 @@ on the next scan.
 ### skill_changes (Slice 041)
 Staged proposals by the agent: `skill_id`, `diff`, `rationale`, `status`, `decided_by`, `decided_at`.
 
+As built at slice 041: `skill_name` (not an id: a create names a skill that has no row yet), `action`
+(`create | patch | write_file | remove_file | delete`), `source` (the target root, `user`), `change_dir` (the
+staged tree under the pending root), `diff` (unified, per changed text file; a non-text or oversized file is
+"replaced, N bytes"), `rationale`, `destructive` (a whole-file replace or a delete), `digest` (SHA-256 over the
+staged tree's paths and bytes; the promotion recomputes it), `status` (`pending | approved | rejected | applied
+| failed`), `severity` (`none | low | medium | high`), `findings` (the scanner's, with file, line, rule,
+severity and the matched text), `proposed_by` (session id, nullable), `approval_id`, `decided_by`, `decided_at`,
+`comment`, `receipt_hash` (the promotion's effect receipt) and `applied_version`. `approvals.session_id` is
+nullable since this slice (a change approved from the page has no session).
+
 ### tool_permissions (Slice 021, as built)
 | column | type | notes |
 |---|---|---|

docs/07-security-model.md

@@ -114,6 +114,24 @@ is not public (loopback, private, link-local) to `:ask`.
 - Scanner flags: shell commands, network calls, credential-looking strings, instructions to disable safety, external URLs.
 - Hub-installed skills are scanned and default to `disabled` until the human enables.
 
+As built at slice 041: the agent's `skill_manage` (and the `learn` flow) never writes a skill root. Every
+change is staged by `Trinity.Skills.Staging` as the whole target tree under `<data dir>/pending/skills/<name>/<change id>/`,
+outside every root the registry scans, with a unified diff, the scanner's findings (`Trinity.Skills.Scanner`:
+shell pipes into a shell, destructive commands, credential shapes and instructions to ignore or disable safety
+are `high`; plain shell commands, network calls, external URLs and base64 blobs `medium`; a file it cannot read
+as text is a `low` finding naming it) and a `skill_changes` row. The one path that moves a staged tree into a
+root is `Trinity.Skills.Promotion.swap/4`, and it requires an allowed `skill_apply` approval whose arguments
+name the change's id and digest (021's fingerprint binds them), recomputes the staged tree's digest, archives
+the previous version under `.history/`, renames the tree into place and writes an `effect` receipt on the
+`skills` chain scope carrying the digest and the approval id. The census (`test/trinity/skills/census_test.exs`)
+holds the tree to one caller of `swap` and two filesystem writers under `lib/trinity/skills/`, with a plant.
+Auto-approval is the persona's (`settings.skills.auto_approve`, off by default, `"low"` applies `none` and
+`low`); `medium` and `high` are never auto-approved, whatever a rule says. Proposing is itself a `:write` tool
+call under the default policy (an approval to propose); "always allow" on `skill_manage` makes proposing free
+while the promotion stays gated. An approval may have no session (a change approved from the page): its topic
+is `approvals:none` and `approvals:all`. Hub installation is not built; a skill dropped by hand into the user
+root loads as any other and is not scanned (a follow-up in the slice's NOTES).
+
 ## Sandbox (Slice 110)
 
 - Luerl with reduction limits, no `os`/`io`/`require`, no filesystem; explicit host functions only.

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,

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."

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
 

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
 

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

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