Repository navigation
Slice 080: subagents and delegation - #84
Merged
Merged
Conversation
…an-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 <aylacroft@proton.me>
…rees, 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 <aylacroft@proton.me>
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 <aylacroft@proton.me>
… 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 <aylacroft@proton.me>
The architecture table's Subagents row said "delegation, result collection" and depended on Sessions and Tools. As built it depends on Sessions alone, and the row now carries what a reader needs in order not to have to read the module: the awaiting shape reused from slice 050, the budgets, the restart default of zero and why, the depth-first cancellation, running? asked of the process rather than of SessionRow.status, and short_id taking the tail of a UUIDv7 rather than its shared head. coverage.tsv gains the row for this slice at 80.31%, level with the previous one. The slice adds library code, a tool, a panel and 25 tests in proportion, so the number not moving is the expected result rather than a lucky one. ROADMAP row 080 to done. Signed-off-by: Ayla Croft <aylacroft@proton.me>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A parent hands a brief to a child session and gets its answer back without the child's working entering the parent's context.
delegateis the tool the model calls,Trinity.Subagentsis the context beneath it, and the session view shows the tree with a stop control.Two things here deserve more attention than the feature does.
AC4 is answered differently from the way the slice wrote it
The criterion said a killed child restarts once by default. It does not. The
configurablehalf is honoured; the default is inverted, and the reasoning is researched rather than preferred.Durable-execution guidance states the rule as match the retry semantics to the side effect: at-least-once for idempotent work, at-most-once with retries disabled for anything with an external side effect. An agent turn is an arbitrary sequence of tool calls.
And in this tree it is unsound rather than merely wasteful, which I measured: a
:oncedecision allows a fingerprint one execution and would refuse a replay, but:sessionand:alwaysgrants cover a replay silently, andlib/trinity/effects/carries no idempotency key and no retry concept at all. A restart could re-run an effect a person approved once, leaving only a second set of receipts nobody was watching for.Vetoing this is a one-line default change and one flipped assertion.
AC7 found two defects no test could
The panel rendered correctly and reported two things falsely. Both screenshots are kept in the slice's
proof/, and the difference between them is the argument for this criterion being manual.active, each with a stop button offering to stop work that was over. The label readSessionRow.status, which is the session lifecycle and staysactiveforever after a subagent finishes. It promised "which are running" and delivered "which are not archived".Neither was visible to a test, because the tests asserted that rows appeared, not that the rows were true.
The first fix was then wrong in the other direction and the tests caught that one:
Sessions.state/1answers with a view map rather than a bare atom, so!= :idlewas true for every child.The rest
Isolation is the criterion, not a detail. "The parent got a result" passes with no isolation whatever, so the test asserts the parent's stored history contains neither the brief nor the answer, then asserts the child's contains both.
No timing assertions. The concurrency cap is counted; the budget test uses a zero budget. A timing assertion on a loaded runner tests the runner, and R26 is already one of those too many.
delegateis:readwith no effect, and the moduledoc says why that is not a contradiction: 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, so delegation cannot launder a:destructivecall through a:readone.The description spends its words on the mistake a model makes quietly: the child cannot see the parent's conversation, so "summarise that" delegates a question with no referent.
Evidence
mix gateexit 0; 639 passed, 18 excluded; coverage 80.31%. 25 tests in the slice. Full record in PROOF; the manual queue is complete.