From 062a8b3992724ff2e7d088b86c97da9ed6d5e51b Mon Sep 17 00:00:00 2001 From: omartelo Date: Thu, 8 Oct 2026 22:15:51 -0300 Subject: [PATCH 1/2] fix(relay): a worker reports only once the work it handed off is back The mod answers a worker's errand from the first main-loop Stop whose background_tasks is empty. An errand the worker sent to another session (a subagent card, or a send that handed back a ticket) is not one of Claude Code's tasks, so the turn that handed it off reported "I handed it off" as the result and the worker was parked under the session it was waiting on. WorkerAnswered now ignores a worker that still awaits an outcome: an errand it sent is open, or a result for it has not been read by a turn of its own. The outcome reaching its prompt starts the turn that answers. A worker in that state is not parked, and one whose own worker is closed hears the stop through its inbox instead of waiting on silence. --- CHANGELOG.md | 7 ++ docs/ceilings.md | 7 +- docs/cli.md | 9 +- docs/hooks/mod-answer.md | 24 ++++- internal/relay/inbox.go | 3 + internal/relay/observe.go | 3 + internal/relay/relay.go | 3 +- internal/relay/subagent.go | 9 +- internal/relay/workeranswer.go | 82 +++++++++++++++- internal/relay/workeranswer_test.go | 145 ++++++++++++++++++++++++++++ 10 files changed, 278 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ffe485c7..f29cecbb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **A subagent card reports back only once the work it handed off is back.** A + Claude Code worker that handed part of its task to another session used to + report "handed it off" as its result the moment its turn ended, and closed + before the other session answered. Its report now waits for that answer, and + for a result that reached it mid-turn, and the worker stays open until then. + If the session it handed work to is closed first, the worker is told so + instead of waiting forever. - **Dragging a footer item in Settings keeps its shape.** The chip under the pointer stays on one line at its own size instead of wrapping into a taller, narrower block. diff --git a/docs/ceilings.md b/docs/ceilings.md index 8483a8b3..febfd272 100644 --- a/docs/ceilings.md +++ b/docs/ceilings.md @@ -502,7 +502,10 @@ work when nobody knows it and that the call site never shows. The mechanism and `[lich]` note through the asker's mod. The worker's own mod answers for it (docs/hooks/mod-answer.md), so it is handed the task with no ticket and its last message is its report; a worker whose mod had not polled when the task went in, or runs a lich-plugin older than 0.17.0, is handed the ticket instead, and one whose final turn - ends blank, aborted, in an API error or a refusal leaves its errand open with nothing reported. Closing a worker + ends blank, aborted, in an API error or a refusal leaves its errand open with nothing reported. A worker's report + waits for the errands it sent itself (`relay.awaitsOutcomeLocked`), since Claude Code does not list them as + background work; one of those that ages out on `ticketTTL` reaches no prompt, and the report then waits for the + next turn something else starts in the worker. Closing a worker ends its errand without a word to the asker (`relay.SessionClosed`, wired before the PTY is killed so the SessionEnd its CLI reports finds nothing to call unanswered). Both reach the asker the way Claude Code's own background agent's completion does, one `● lich session "…" finished` line on screen and the note whole in the model's context @@ -511,7 +514,7 @@ work when nobody knows it and that the call site never shows. The mechanism and a prompt from the plugin. Without a mod polling there the note is typed short and the report waits for `wait_for_answer`, because typing a long report into a TUI is what the mod route exists to avoid. A worker in the asker's checkout is parked once it answered and the turn it answered in ended (`relay.SetWorkerFinished`, - `spawn.CloseFinishedWorker`), so typing into its card after its report means resuming it from the history; one + `spawn.CloseFinishedWorker`) with nothing it sent still out, so typing into its card after its report means resuming it from the history; one on its own worktree, pinned, stopped with Esc, holding another errand, or whose asker is gone stays open. Esc in the asker leaves its workers running, as it leaves Claude Code's own background agents (measured on 2.1.289); the mod's TaskStop stops one, Claude Code's `ctrl+x ctrl+k` does not, and the asker's status line counts them diff --git a/docs/cli.md b/docs/cli.md index 6a8db962..b934b7f1 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -452,13 +452,18 @@ It answers to "auth-fix" and to "auth-fix-9f8e". Its agent may still be starting `close_session`, the caller's TaskStop or the card's own close: the caller hears nothing and nothing waits in its inbox, since whoever closed it stopped the work. A caller holding the line on the ticket hears `stopped` - (exit 3). + (exit 3), and so does a caller that is itself a worker somebody waits on, + through its inbox: its own report waits for this outcome. + - **reports only after the work it handed off is back**: while an errand it + sent is open, or a result for it is unread, its mod's report is ignored, + and the turn that outcome starts is the one that answers. - **closes once it is done** when it runs in the caller's checkout: after it answers the errand and the turn it answered in ends, lich parks it the way `lich close` does, like a native subagent that returned its result. A worker on a worktree of its own stays open, since that checkout holds its work, and so does one the user pinned, one whose caller is gone, one with another - errand still open at it, and one whose turn was stopped with Esc. + errand still open at it, one with an errand of its own still out, and one + whose turn was stopped with Esc. - tells the caller, in one short note per block, when it is **waiting on a permission** prompt in its card. - spawns with `LICH_SUBAGENT_CARDS=off` on every start, a resume included, so diff --git a/docs/hooks/mod-answer.md b/docs/hooks/mod-answer.md index 78595068..b8fb6305 100644 --- a/docs/hooks/mod-answer.md +++ b/docs/hooks/mod-answer.md @@ -41,6 +41,14 @@ otherwise: an ordinary session, a worker whose errand was already answered `reply_to_session` itself), and one holding two subagent errands, where nothing says which one the text belongs to. +It also ignores the report of a worker still awaiting an outcome of its own: +an errand it sent to another session (a subagent card of its own, or a +`send_to_session` that handed back a ticket) is still open, or a result for it +has not been read by a turn of its yet. None of those is in Claude Code's +`background_tasks`, so the turn that handed the work off ends with the list +empty and reports "I handed it off". The outcome reaches the worker's prompt +and starts the turn whose report answers the errand. + Both sides test against the payloads in [`fixtures/mod-answer.jsonl`](fixtures/mod-answer.jsonl). @@ -75,9 +83,15 @@ Claude Code 2.1.289: wired with `terminal.SetWorkerAnswer`). - **Answer** (`relay.WorkerAnswered`): finds the one open subagent errand at the session, delivered or lapsed, and answers it through `Reply`, so the caller - gets the report the way it gets one sent with `reply_to_session`. A worker in - its caller's checkout is closed once the answer is in and its turn has - ended, whichever of the two lands first. + gets the report the way it gets one sent with `reply_to_session`, unless the + worker still awaits an outcome of its own (`relay.awaitsOutcomeLocked`). A + result counts as read once a turn of the worker's starts after the worker was + told about it (`inboxEntry.seen`). A worker in its caller's checkout is + closed once the answer is in, its turn has ended and nothing it sent is still + out, whichever lands last. +- **A worker's own worker closed** (`relay.SessionClosed`): the stop is filed in + the inbox of a caller that is itself a worker somebody waits on, so it + resumes and answers instead of waiting on an errand that ended in silence. - **The task** (`relay.handOff`): a subagent errand handed to a worker whose mod polls from a plugin release that posts here (`agentplugin.ModAnswerRelease`) is composed without the ticket and the reply instructions, and is marked as @@ -102,3 +116,7 @@ Claude Code 2.1.289: instructions; its mod still reports, and whichever answer lands first wins. - **A mod older than 0.17.0 does not post here.** Its worker is handed the ticket, as before. +- **An errand the worker sent that ages out holds its answer.** An ordinary + errand with nobody holding the line is dropped after an hour without + activity (`ticketTTL`) and nothing reaches the worker's prompt, so its report + waits for the next turn something else starts there. diff --git a/internal/relay/inbox.go b/internal/relay/inbox.go index 656a6be0..5c3006a5 100644 --- a/internal/relay/inbox.go +++ b/internal/relay/inbox.go @@ -57,6 +57,9 @@ type inboxEntry struct { // per entry: re-nudging on every turn end would start a turn per nudge, // forever, for a sender that chose not to collect. nudged bool + // seen is whether a turn of the sender's started after it was nudged, so + // the sender has read it or chose not to collect it (awaitsOutcomeLocked). + seen bool } // Collect drains every outcome waiting for fromID, oldest first. With nothing diff --git a/internal/relay/observe.go b/internal/relay/observe.go index 957f1919..527fb295 100644 --- a/internal/relay/observe.go +++ b/internal/relay/observe.go @@ -30,6 +30,9 @@ func (s *Service) Observe(sessionID, state string) { finished := s.finishedWorkerLocked(sessionID, state) resumed := s.takeResumeLocked(sessionID, state) s.recordState(sessionID, state) + if state == stateBusy { + s.seeNewsLocked(sessionID) + } ended, notice := s.endedErrands(sessionID, state) // A waiter still holding the line carries the news out through its own // select; an errand nobody is attending is stashed for the sender instead, diff --git a/internal/relay/relay.go b/internal/relay/relay.go index 0f858791..5a6d0292 100644 --- a/internal/relay/relay.go +++ b/internal/relay/relay.go @@ -119,7 +119,8 @@ const ( StatusUndelivered = "undelivered" // StatusStopped means a subagent worker was closed before it answered: its // caller stopped it (SessionClosed). Only a caller holding the line hears - // it; nothing is filed or announced. + // it, or a caller that is itself a worker somebody waits on, whose inbox it + // is filed in; otherwise nothing is filed or announced. StatusStopped = "stopped" ) diff --git a/internal/relay/subagent.go b/internal/relay/subagent.go index c5dad8d8..1b680d2c 100644 --- a/internal/relay/subagent.go +++ b/internal/relay/subagent.go @@ -154,9 +154,10 @@ func (s *Service) noteWorkerReportedLocked(t *ticket) { } // finishedWorkerLocked reads one state report for a worker that reported this -// turn: a done finishes it unless another errand is still open at it, checked -// before endedErrands takes this turn's errands off the table, since one of -// those may still be answered late. An interrupt or the session ending clears +// turn: a done finishes it unless another errand is still open at it, or one +// it sent has not come back (a worker that answered by its ticket while its own +// worker runs), checked before endedErrands takes this turn's errands off the +// table, since one of those may still be answered late. An interrupt or the session ending clears // the mark without finishing it: whoever stopped that turn is still at its // card. Called under s.mu. func (s *Service) finishedWorkerLocked(sessionID, state string) bool { @@ -172,7 +173,7 @@ func (s *Service) finishedWorkerLocked(sessionID, state string) bool { return false } delete(s.reportedWorkers, sessionID) - return !s.errandOpenAtLocked(sessionID) && s.workerFinished != nil + return !s.errandOpenAtLocked(sessionID) && !s.awaitsOutcomeLocked(sessionID) && s.workerFinished != nil } func (s *Service) finishWorker(workerID string) { diff --git a/internal/relay/workeranswer.go b/internal/relay/workeranswer.go index 19087678..13e246dc 100644 --- a/internal/relay/workeranswer.go +++ b/internal/relay/workeranswer.go @@ -25,13 +25,24 @@ func (s *Service) taskMessage(id string, t *ticket, kind string) string { // from a session whose subagents the user kept native, and nothing in the text // says which of two errands it answers. An errand answered already keeps its // first answer. +// +// A worker still awaiting an outcome of its own (awaitsOutcomeLocked) is +// ignored too. The mod reports a turn that left nothing in Claude Code's +// background list, and an errand the worker sent to another session is not in +// that list, so the text is "I handed it off" rather than the result. The +// outcome reaching the worker starts the turn whose report is the answer. func (s *Service) WorkerAnswered(workerID, text string) { s.mu.Lock() id, ok := s.subagentErrandLocked(workerID) + awaiting := s.awaitsOutcomeLocked(workerID) s.mu.Unlock() if !ok { return } + if awaiting { + slog.Debug("relay: a worker reported while an errand of its own is unread", "session", workerID) + return + } if err := s.Reply(workerID, id, text); err != nil { slog.Debug("relay: a worker's answer was not taken", "session", workerID, "err", err) return @@ -59,6 +70,63 @@ func (s *Service) subagentErrandLocked(workerID string) (string, bool) { return found[0], true } +// owesSubagentAnswerLocked is whether a subagent errand still waits on +// sessionID's answer: someone opened it as their subagent and has not heard +// back. Called under s.mu. +func (s *Service) owesSubagentAnswerLocked(sessionID string) bool { + for _, t := range s.tickets { + if t.targetID == sessionID && t.subagent { + return true + } + } + for _, t := range s.lapsed { + if t.targetID == sessionID && t.subagent { + return true + } + } + return false +} + +// awaitsOutcomeLocked is whether an outcome is still on its way to sessionID's +// prompt: an errand it sent is open, or a result for it has not been read by a +// turn of its own yet (inboxEntry.seen). A private errand never reaches that +// prompt, so it is not counted: nothing would ever resume the session for it. +// Called under s.mu. +func (s *Service) awaitsOutcomeLocked(sessionID string) bool { + for _, t := range s.tickets { + if t.fromID == sessionID && !t.private { + return true + } + } + for _, e := range s.ready { + if e.fromID == sessionID && !e.seen { + return true + } + } + for _, e := range s.held { + if e.fromID == sessionID && !e.private && !e.seen { + return true + } + } + return false +} + +// seeNewsLocked marks every result sessionID was already told about as read: a +// turn starting after the notice is the one the notice started, or one that +// came after it. Called under s.mu on a busy report. +func (s *Service) seeNewsLocked(sessionID string) { + for _, e := range s.ready { + if e.fromID == sessionID && e.nudged { + e.seen = true + } + } + for _, e := range s.held { + if e.fromID == sessionID && e.nudged { + e.seen = true + } + } +} + // finishIfTurnEnded finishes a worker whose answer landed after the turn it // answered in had already ended: the mod reports from Stop, and the done report // of the same Stop races it. One landing mid-turn is finished by that done @@ -66,7 +134,7 @@ func (s *Service) subagentErrandLocked(workerID string) (string, bool) { func (s *Service) finishIfTurnEnded(workerID string) { s.mu.Lock() finish := s.reportedWorkers[workerID] && s.state[workerID] != stateBusy && - !s.errandOpenAtLocked(workerID) && s.workerFinished != nil + !s.errandOpenAtLocked(workerID) && !s.awaitsOutcomeLocked(workerID) && s.workerFinished != nil if finish { delete(s.reportedWorkers, workerID) } @@ -92,11 +160,15 @@ func (s *Service) errandOpenAtLocked(sessionID string) bool { // (TaskStop runs `lich close`), or the user doing the same from its card, and // neither has anything left to hear. A caller still holding the line hears // stopped; the ticket stays waitable as stopped, and nothing reaches the inbox. +// The exception is a caller that is itself a worker somebody waits on +// (owesSubagentAnswerLocked): its answer waits for this outcome +// (WorkerAnswered), so the stop is filed in its inbox, where it resumes it. // The terminal calls this before the process dies (terminal.SetSessionClosed), // so the SessionEnd its CLI reports on the way out finds nothing to call // unanswered. Errands of any other kind end as they always did. func (s *Service) SessionClosed(sessionID string) { var stopped []*ticket + var told []string s.mu.Lock() delete(s.reportedWorkers, sessionID) for id, t := range s.tickets { @@ -104,11 +176,17 @@ func (s *Service) SessionClosed(sessionID string) { continue } delete(s.tickets, id) - s.lapseLocked(id, t, StatusStopped) + if t.attended == 0 && s.owesSubagentAnswerLocked(t.fromID) { + s.stashLocked(id, t, StatusStopped, "") + told = append(told, t.fromID) + } else { + s.lapseLocked(id, t, StatusStopped) + } t.stopped = true close(t.stalled) stopped = append(stopped, t) } s.mu.Unlock() s.clearAll(stopped) + s.announceInboxAll(told) } diff --git a/internal/relay/workeranswer_test.go b/internal/relay/workeranswer_test.go index 9d46d876..77fcafba 100644 --- a/internal/relay/workeranswer_test.go +++ b/internal/relay/workeranswer_test.go @@ -238,3 +238,148 @@ func TestAWorkerWhoseAnswerLandsMidTurnIsFinishedWhenItEnds(t *testing.T) { svc.Observe("s2", stateDone) expectFinished(t, finished, "s2") } + +// expectOpen fails unless the errand is still open: no answer reached it. +func expectOpen(t *testing.T, svc *Service, id string) { + t.Helper() + svc.mu.Lock() + defer svc.mu.Unlock() + if svc.tickets[id] == nil { + t.Fatalf("errand %s was closed", id) + } +} + +func expectAnswer(t *testing.T, svc *Service, id, want string) { + t.Helper() + got, err := svc.Wait(context.Background(), id, 1) + if err != nil || got.Status != StatusAnswered || got.Answer != want { + t.Fatalf("Wait(%s) = %+v, %v, want answered %q", id, got, err, want) + } +} + +// A worker that hands a task to a session of its own ends its turn with +// nothing in Claude Code's background list, since that session is not one of +// Claude Code's tasks. Its mod reports that turn, and the report is only "I +// handed it off": the errand must wait for the turn that has the result. +func TestAWorkersAnswerWaitsForTheErrandsItSent(t *testing.T) { + term := newFakeTerminal("s1", "s2", "s3") + svc := newRelay(workspace(), term, nil) + plantAnsweredByMod(svc, "t1", "s1", "s2", "docs") + plantAnsweredByMod(svc, "t2", "s2", "s3", "api") + + svc.Observe("s2", stateBusy) + svc.WorkerAnswered("s2", "handed the api part to a worker") + svc.Observe("s2", stateDone) + expectOpen(t, svc, "t1") + + svc.WorkerAnswered("s3", "the api is done") + if !awaitWritten(term, "s2", "api") { + t.Fatal("the worker was never told its own worker finished") + } + svc.Observe("s2", stateBusy) + svc.WorkerAnswered("s2", "docs and api are done") + expectAnswer(t, svc, "t1", "docs and api are done") +} + +// An ordinary errand the worker sent holds its answer the same way: its +// outcome reaches the worker's prompt and starts the turn that answers. +func TestAWorkersAnswerWaitsForAnOrdinaryErrandItSent(t *testing.T) { + svc := newRelay(workspace(), newFakeTerminal("s1", "s2", "s3"), nil) + plantAnsweredByMod(svc, "t1", "s1", "s2", "docs") + plant(svc, "t2", "s2", "s3", "api") + + svc.WorkerAnswered("s2", "asked api, waiting") + expectOpen(t, svc, "t1") +} + +// A private errand reaches no prompt, so nothing would ever resume the worker +// for it: it does not hold the answer. +func TestAPrivateErrandDoesNotHoldAWorkersAnswer(t *testing.T) { + svc := newRelay(workspace(), newFakeTerminal("s1", "s2", "s3"), nil) + plantAnsweredByMod(svc, "t1", "s1", "s2", "docs") + plant(svc, "t2", "s2", "s3", "api") + svc.mu.Lock() + svc.tickets["t2"].private = true + svc.mu.Unlock() + + svc.WorkerAnswered("s2", "the report") + expectAnswer(t, svc, "t1", "the report") +} + +// A result that lands while the worker is mid-turn is handed to it only when +// that turn ends, so the turn's own report was written without it. +func TestAWorkersAnswerWaitsForAResultItHasNotRead(t *testing.T) { + term := newFakeTerminal("s1", "s2", "s3") + svc := newRelay(workspace(), term, nil) + plantAnsweredByMod(svc, "t1", "s1", "s2", "docs") + plantAnsweredByMod(svc, "t2", "s2", "s3", "api") + + svc.Observe("s2", stateBusy) + svc.WorkerAnswered("s3", "the api is done") + svc.Observe("s2", stateDone) + svc.WorkerAnswered("s2", "still waiting on api") + expectOpen(t, svc, "t1") + + if !awaitWritten(term, "s2", "api") { + t.Fatal("the worker was never told its own worker finished") + } + svc.Observe("s2", stateBusy) + svc.WorkerAnswered("s2", "docs and api are done") + expectAnswer(t, svc, "t1", "docs and api are done") +} + +// A result the worker was told about and chose not to collect does not hold +// its answer forever: the turn after the notice is the one that read it. +func TestAResultTheWorkerWasToldAboutDoesNotHoldItsAnswer(t *testing.T) { + term := newFakeTerminal("s1", "s2", "s3") + svc := newRelay(workspace(), term, nil) + plantAnsweredByMod(svc, "t1", "s1", "s2", "docs") + plant(svc, "t2", "s2", "s3", "api") + + if err := svc.Reply("s3", "t2", "the api answer"); err != nil { + t.Fatalf("Reply: %v", err) + } + if !awaitWritten(term, "s2", "api") { + t.Fatal("the worker was never told the result is waiting") + } + svc.Observe("s2", stateBusy) + svc.WorkerAnswered("s2", "the report") + expectAnswer(t, svc, "t1", "the report") +} + +// A worker whose own worker is closed under it would wait forever on an errand +// that ended in silence. Someone is waiting on it, so it hears the errand +// stopped, and that is what resumes it. +func TestAWorkerHearsItsOwnWorkerStopped(t *testing.T) { + svc := newRelay(workspace(), newFakeTerminal("s1", "s2", "s3"), nil) + plantAnsweredByMod(svc, "t1", "s1", "s2", "docs") + plantAnsweredByMod(svc, "t2", "s2", "s3", "api") + + svc.SessionClosed("s3") + + collected, err := svc.CollectNow("s2") + if err != nil { + t.Fatalf("CollectNow: %v", err) + } + if len(collected.Results) != 1 || collected.Results[0].Status != StatusStopped { + t.Fatalf("collect = %+v, want the errand stopped", collected) + } + svc.Observe("s2", stateBusy) + svc.WorkerAnswered("s2", "api was stopped; docs are done") + expectAnswer(t, svc, "t1", "api was stopped; docs are done") +} + +// A worker that answered by its ticket while its own worker still runs is not +// closed under that worker: the report it is owed would reach no one. +func TestAWorkerWithAnErrandOfItsOwnOpenIsNotFinished(t *testing.T) { + svc, finished := finishing(t) + plantSubagent(svc, "t1", "s1", "s2", "docs") + plantSubagent(svc, "t2", "s2", "s3", "api") + + svc.Observe("s2", stateBusy) + if err := svc.Reply("s2", "t1", "the report"); err != nil { + t.Fatalf("Reply: %v", err) + } + svc.Observe("s2", stateDone) + expectNoneFinished(t, finished) +} From 16ddef769f46f5e7f48895db08a5d3b8e701e7c4 Mon Sep 17 00:00:00 2001 From: omartelo Date: Thu, 8 Oct 2026 22:26:18 -0300 Subject: [PATCH 2/2] feat: a subagent card can open subagent cards of its own, one level down A worker used to spawn with LICH_SUBAGENT_CARDS=off, which kept its own subagents native and was also how the plugin's worker-answer mod told a worker. Cards now nest two levels (maxSubagentDepth): - The sessions row's subagent column holds the depth (spawn: caller's depth + 1), carried through resume and fork. reopen read it as a bool, which would fail on a depth of 2. - Every Claude Code session gets LICH_SUBAGENT_DEPTH; LICH_SUBAGENT_CARDS=off now means only "subagents stay native": the setting is off, the card is at the limit, or the installed plugin predates 0.19.0, whose mod tells a worker by the depth. - The briefing has a third route: a card at the limit is told its Agent tool stays inside it and to fan out with that, not with new sessions. Tests changed because the contract changed: the store's subagent mark is now a depth (SessionSubagentDepth), SpawnBriefing and providerArgs take a SubagentRoute instead of a bool, and the terminal's env helper adds the depth. The plugin ceiling is already 0.20.0 (#687): nesting rides the same 0.19 contract as compacting. --- CHANGELOG.md | 6 ++ README.md | 2 +- README.zh-CN.md | 2 +- docs/ceilings.md | 15 +++- docs/cli.md | 9 +- docs/hooks/README.md | 25 ++++-- docs/hooks/mod-answer.md | 6 +- internal/agentplugin/claude.go | 19 ++++ internal/agentplugin/claude_test.go | 17 ++++ internal/cli/wire_test.go | 6 +- internal/relay/compose.go | 55 +++++++++--- internal/relay/relay_test.go | 22 ++++- internal/spawn/spawn.go | 32 ++++--- internal/spawn/spawn_test.go | 14 +-- internal/spawn/subagent_test.go | 24 +++-- internal/store/migrate.go | 8 +- internal/store/mutations.go | 51 ++++++----- internal/store/subagent_test.go | 29 +++--- internal/terminal/command.go | 9 +- internal/terminal/command_kiro_test.go | 3 +- internal/terminal/start.go | 6 +- internal/terminal/subagent_cards.go | 64 +++++++++---- internal/terminal/subagent_cards_test.go | 90 +++++++++++++++---- internal/terminal/subagent_cards_unix_test.go | 40 +++++---- internal/terminal/terminal.go | 2 +- internal/terminal/terminal_test.go | 36 ++++---- 26 files changed, 422 insertions(+), 170 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f29cecbb..8d4d6c1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `/compact` or an automatic compaction shows "Compacting…" under the session's name with a spinner, instead of the finished turn's check or a spinner that names nothing. Needs the next lich-plugin release. +- **A subagent card can open subagent cards of its own.** A Claude Code worker + that splits its task now opens each part as a card too, filed beside it, and + reports back once those parts are done. One level of that is allowed: a card + opened by a card keeps its own subagents inside Claude Code, and is told so + when it starts. Needs lich-plugin 0.19.0 or later; with an older plugin a + worker's subagents stay inside Claude Code as before. ### Changed diff --git a/README.md b/README.md index 0aece988..d801a388 100644 --- a/README.md +++ b/README.md @@ -122,7 +122,7 @@ reads what each CLI writes down and no two of them write down the same things. | Open a session with ultracode on | yes | no | no | no | no | no | no | no | | Drive a running session from `lich control` or an agent: prompt, stop, model, effort, slash command | yes | no | no | no | no | no | no | no | | A message from another session arrives without being typed into the terminal | yes | no | no | no | no | no | no | no | -| A subagent the agent starts runs as a lich session with its own card, in the asking session's checkout, hands its full report back and closes once done | yes | no | no | no | no | no | no | no | +| A subagent the agent starts runs as a lich session with its own card, in the asking session's checkout, hands its full report back and closes once done; its own subagents get cards too, one level down | yes | no | no | no | no | no | no | no | | Ask a running session a side question without stopping it (`lich ask`) | yes | no | no | no | no | no | no | no | | Context, cost and plan usage as the CLI measured them itself | yes | no | no | no | no | no | no | no | | The terminal's status line shows who the session owes an answer, who it is waiting on, and answers ready to collect | yes | no | no | no | no | no | no | no | diff --git a/README.zh-CN.md b/README.zh-CN.md index 10e4d155..24b75566 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -109,7 +109,7 @@ | 以 ultracode 模式打开会话 | 有 | 无 | 无 | 无 | 无 | 无 | 无 | 无 | | 用 `lich control` 或智能体操控运行中的会话：发提示、停止、换模型、推理强度、斜杠命令 | 有 | 无 | 无 | 无 | 无 | 无 | 无 | 无 | | 其他会话发来的消息直接送达，不经终端键入 | 有 | 无 | 无 | 无 | 无 | 无 | 无 | 无 | -| 智能体启动的子智能体作为 lich 会话运行，有自己的卡片，在发起会话的检出目录中工作，交回完整报告后自动关闭 | 有 | 无 | 无 | 无 | 无 | 无 | 无 | 无 | +| 智能体启动的子智能体作为 lich 会话运行，有自己的卡片，在发起会话的检出目录中工作，交回完整报告后自动关闭；它自己启动的子智能体也有卡片，再往下一层为止 | 有 | 无 | 无 | 无 | 无 | 无 | 无 | 无 | | 向运行中的会话问一个旁支问题，不打断它（`lich ask`） | 有 | 无 | 无 | 无 | 无 | 无 | 无 | 无 | | 上下文、花费和套餐用量取 CLI 自己量出的数 | 有 | 无 | 无 | 无 | 无 | 无 | 无 | 无 | | 终端状态栏显示本会话欠谁一个回复、在等谁，以及待收取的回复 | 有 | 无 | 无 | 无 | 无 | 无 | 无 | 无 | diff --git a/docs/ceilings.md b/docs/ceilings.md index febfd272..8f38f7af 100644 --- a/docs/ceilings.md +++ b/docs/ceilings.md @@ -518,11 +518,18 @@ work when nobody knows it and that the call site never shows. The mechanism and on its own worktree, pinned, stopped with Esc, holding another errand, or whose asker is gone stays open. Esc in the asker leaves its workers running, as it leaves Claude Code's own background agents (measured on 2.1.289); the mod's TaskStop stops one, Claude Code's `ctrl+x ctrl+k` does not, and the asker's status line counts them - from `lich sessions --json` every 5 seconds. The - worker spawns with `LICH_SUBAGENT_CARDS=off`, so its own subagents stay native. The other seven + from `lich sessions --json` every 5 seconds. Cards + nest two levels deep (`maxSubagentDepth`, internal/terminal): a worker turns its own subagents into cards, + and a worker it opened spawns with `LICH_SUBAGENT_CARDS=off`, so that one's stay native, and is briefed to fan + out with them rather than with new lich sessions; nothing but that briefing stops it opening plain sessions + with `lich open`. A worker needs lich-plugin 0.19.0 to nest, since an older mod tells a worker only by + `LICH_SUBAGENT_CARDS=off`; under one every worker keeps its subagents native. A nested card's report climbs + back through each worker above it, and each answers only once its own outcomes are in, so a card stopped deep + in the tree reaches its asker as a stop, not as silence. Closing a worker leaves the cards it opened running, + as it leaves its own. The other seven providers have no mod system, so nothing can take a subagent call from them, and their subagents stay inside their CLI. On Claude - Code too, a typed agent (Explore, Plan, a plugin's), a workflow step, another plugin's spawn, a remote one and a - worker's own subagents stay native, and the worker is reached with `send_to_session` or `lich send`, never with + Code too, a typed agent (Explore, Plan, a plugin's), a workflow step, another plugin's spawn, a remote one and the + subagents of a worker two levels down stay native, and the worker is reached with `send_to_session` or `lich send`, never with Claude Code's `SendMessage`. Settings › Providers › Claude Code turns it off with "Subagents as lich sessions" (`store.SubagentCards`), which reaches the mod as `LICH_SUBAGENT_CARDS=off` at spawn, so a session already running keeps the value it started with. diff --git a/docs/cli.md b/docs/cli.md index b934b7f1..d0b0520b 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -466,8 +466,13 @@ It answers to "auth-fix" and to "auth-fix-9f8e". Its agent may still be starting whose turn was stopped with Esc. - tells the caller, in one short note per block, when it is **waiting on a permission** prompt in its card. - - spawns with `LICH_SUBAGENT_CARDS=off` on every start, a resume included, so - its own subagents stay inside its CLI: a card does not open cards. + - runs **one level below the caller**, recorded on its row and handed to the + plugin as `LICH_SUBAGENT_DEPTH` on every start, a resume included. A + Claude Code worker one level down still turns its own general-purpose + subagents into cards (lich-plugin 0.19.0 or later); one two levels down, a + card a card opened, spawns with `LICH_SUBAGENT_CARDS=off` and keeps them + inside its CLI, and its briefing says so. Under an older plugin every + worker keeps them native. `--json` keeps its shape: the session and its `delivery`. - `--folder` files the new session under that sidebar folder, written before the window hears of the session, so the card arrives in the folder's block instead diff --git a/docs/hooks/README.md b/docs/hooks/README.md index 71fc893d..68f2a317 100644 --- a/docs/hooks/README.md +++ b/docs/hooks/README.md @@ -74,15 +74,22 @@ the agent in the PTY calls to reach the sessions beside it. That surface has its own contract in [cli.md](../cli.md). `LICH_SUBAGENT_CARDS=off` is the fifth, and the only one set from a setting: a -Claude Code session gets it when the user turned "Subagents as lich sessions" -off, and the plugin's agent-cards mod then leaves every subagent native. A -session opened with `lich open --subagent` gets it too, whatever the setting, -on every spawn of it including a resume, so a worker's own subagents never open -cards of their own. It is absent otherwise, which the mod reads as on. The mod -also reads it as "this session may be a worker" and reports its turns' -answers ([mod-answer.md](mod-answer.md)); lich ignores the report from a -session with no subagent errand open, which is every session the setting -turned it on for. +Claude Code session gets it when its subagents stay native, which is when the +user turned "Subagents as lich sessions" off, and in a session opened with +`lich open --subagent` that runs as deep as lich nests cards (two levels: a card +a card opened) or whose installed plugin is older than 0.19.0, since such a mod +tells a worker only by this variable. The plugin's agent-cards mod then leaves +every subagent native. It is absent otherwise, which the mod reads as on. + +`LICH_SUBAGENT_DEPTH` is the sixth, set on every Claude Code session lich +starts: `0` for one nobody opened as a subagent, `n` for a worker opened `n` +`--subagent` levels down, on every spawn of it including a resume and a fork. +The mod reads a depth above 0 as "this session is a worker" and reports its +turns' answers ([mod-answer.md](mod-answer.md)); lich ignores the report from a +session with no subagent errand open. A plugin from 0.19.0 that finds it absent +is under an older lich, and falls back to reading `LICH_SUBAGENT_CARDS=off` as +both "leave subagents native" and "this session may be a worker", the way every +release before it did. ### The plugin release diff --git a/docs/hooks/mod-answer.md b/docs/hooks/mod-answer.md index b8fb6305..994f888e 100644 --- a/docs/hooks/mod-answer.md +++ b/docs/hooks/mod-answer.md @@ -59,8 +59,10 @@ Both sides test against the payloads in | `classic.Stop` on the main loop, then `turn.complete` | none (no mod system) | none (no mod system) | none (no mod system) | none (no mod system) | none (no mod system) | none (no mod system) | none (no mod system) | The mod posts once per main-loop turn, and only from a worker: a session -spawned with `LICH_SUBAGENT_CARDS=off` (README, Shared transport), which every -`--subagent` session is. A turn is reported when all of these hold, measured on +whose `LICH_SUBAGENT_DEPTH` is above 0 (README, Shared transport), which every +`--subagent` session is. A mod from 0.19.0 under a lich that sets no depth, and +every mod before 0.19.0, takes `LICH_SUBAGENT_CARDS=off` for that instead, which +lich then sets on every worker. A turn is reported when all of these hold, measured on Claude Code 2.1.289: - `classic.Stop` fired without an `agent_id` (the main loop, not a subagent or diff --git a/internal/agentplugin/claude.go b/internal/agentplugin/claude.go index 776fa3d2..a84307a8 100644 --- a/internal/agentplugin/claude.go +++ b/internal/agentplugin/claude.go @@ -57,6 +57,25 @@ func runsSubagentCards(version string) bool { return !semver.Less(version, SubagentCardsRelease) } +// NestedSubagentCardsRelease is the first lich-plugin release whose mod tells a +// worker by LICH_SUBAGENT_DEPTH rather than by LICH_SUBAGENT_CARDS=off +// (hooks/worker-answer.js), so a worker can open cards of its own and still +// answer its errand. An older one reads a worker without that variable as a +// session that answers nothing. +const NestedSubagentCardsRelease = "0.19.0" + +// ClaudeNestsSubagentCards reports whether the installed Claude Code plugin is a +// release a worker can open cards from. It reads Claude Code's plugin state on +// each call. +func ClaudeNestsSubagentCards() bool { + version, ok := claudeInstalledVersion() + return ok && nestsSubagentCards(version) +} + +func nestsSubagentCards(version string) bool { + return !semver.Less(version, NestedSubagentCardsRelease) +} + // claudeInstalledVersion reads the plugin's installed version from Claude // Code's plugin state, or ("", false) when absent or unreadable. func claudeInstalledVersion() (string, bool) { diff --git a/internal/agentplugin/claude_test.go b/internal/agentplugin/claude_test.go index eac61d66..931a9de2 100644 --- a/internal/agentplugin/claude_test.go +++ b/internal/agentplugin/claude_test.go @@ -2,6 +2,23 @@ package agentplugin import "testing" +// A worker opens cards only under a release that tells it a worker by its +// depth, since an older one would stop answering for it. +func TestNestsSubagentCardsFromTheFirstNestingRelease(t *testing.T) { + cases := map[string]bool{ + "0.18.4": false, + "0.19.0-rc.1": false, + "0.19.0": true, + "0.19.2": true, + "not-a-semver": false, + } + for version, want := range cases { + if got := nestsSubagentCards(version); got != want { + t.Errorf("nestsSubagentCards(%q) = %v, want %v", version, got, want) + } + } +} + // The briefing points Claude Code at its own Agent tool only once the // installed plugin is a release whose mod turns subagents into lich cards. func TestRunsSubagentCardsFromTheFirstCardRelease(t *testing.T) { diff --git a/internal/cli/wire_test.go b/internal/cli/wire_test.go index 7fc8afa6..6c12e4b6 100644 --- a/internal/cli/wire_test.go +++ b/internal/cli/wire_test.go @@ -245,13 +245,17 @@ func (s *spawnStore) AddSessionFrom(_, _, _, _, _ string, _ int, _, _ string) er return nil } -func (s *spawnStore) SetSessionSubagent(string) error { +func (s *spawnStore) SetSessionSubagentDepth(string, int) error { s.mu.Lock() defer s.mu.Unlock() s.subagent = true return nil } +func (s *spawnStore) SessionSubagentDepth(string) int { + return 0 +} + func (s *spawnStore) SetSessionModel(_, model string) error { s.mu.Lock() defer s.mu.Unlock() diff --git a/internal/relay/compose.go b/internal/relay/compose.go index 11f5e697..85b4a9a7 100644 --- a/internal/relay/compose.go +++ b/internal/relay/compose.go @@ -40,33 +40,60 @@ import ( // CommandLineToArgvW, cmd.exe does not read that as an escape, and the < and > // left outside quotes by it are redirection. // -// agentCards is whether this is a Claude Code session whose lich-plugin mod -// runs a general-purpose subagent as one of those cards. There the agent's own -// Agent tool is the better route (in the background, its report back on its -// own, stopped with TaskStop), so the briefing sends it there and keeps lich's -// own route for what a subagent cannot be. -func SpawnBriefing(hasTools, agentCards bool) string { - route := "Open one with `lich open --worktree BRANCH --prompt 'the task'`, which opens the " + +// route is where this spawn's own subagents run (SubagentRoute), which decides +// what the agent is sent to for fanning work out. +func SpawnBriefing(hasTools bool, route SubagentRoute) string { + command := "Open one with `lich open --worktree BRANCH --prompt 'the task'`, which opens the " + "session and hands it the task in one command." if hasTools { - route = "The lich tools in your list open one and hand it the task." + command = "The lich tools in your list open one and hand it the task." } intro := "You are running inside lich, which runs coding-agent sessions side by side and can " + "open more of them beside this one — each a card the user watches and can take over " + "mid-task, in its own git worktree when the work needs its own checkout. " - if agentCards { + switch route { + case RouteCards: return intro + "Here your own Agent tool opens them: a general-purpose subagent runs as " + "one of those cards, in this checkout unless you ask for isolation 'worktree', " + "in the background, and its report comes back to you on its own. Fan work out with " + "it. Open a session yourself only for what a subagent cannot be: another agent " + - "kind, a branch the user named, or work that must outlive this session. " + route + "kind, a branch the user named, or work that must outlive this session. " + command + case RouteNative: + return intro + "This session is itself a subagent card another session opened, and here " + + "your own Agent tool runs a subagent inside this session rather than as a card. Fan " + + "work out with it rather than with new lich sessions: your final message is the " + + "report the session that opened you is waiting for." + default: + return intro + "When work is " + + "to be fanned out — several tasks at once, one per branch or checkout — those sessions " + + "are what to open, not the subagents your own harness runs: a subagent has no checkout, " + + "no card, and nothing the user can steer or resume. " + command } - return intro + "When work is " + - "to be fanned out — several tasks at once, one per branch or checkout — those sessions " + - "are what to open, not the subagents your own harness runs: a subagent has no checkout, " + - "no card, and nothing the user can steer or resume. " + route } +// SubagentRoute is where a spawned session's own subagents run, as far as its +// briefing is concerned. +type SubagentRoute int + +const ( + // RouteSessions is every session whose subagents stay inside its harness and + // that may open lich sessions to fan out. + RouteSessions SubagentRoute = iota + // RouteCards is a Claude Code session whose lich-plugin mod runs a + // general-purpose subagent as one of those cards. There the agent's own + // Agent tool is the better route (in the background, its report back on its + // own, stopped with TaskStop), so the briefing sends it there and keeps + // lich's own route for what a subagent cannot be. + RouteCards + // RouteNative is a Claude Code card whose own subagents stay native, because + // it runs as deep as lich nests cards. lich's server instructions still say + // a subagent becomes a card in a session with the plugin, so the briefing + // says it does not here, and keeps the fan-out under this card rather than + // sending it to new sessions, which would nest cards past the limit by + // another door. + RouteNative +) + // compose is the message typed at the target's prompt. It names where the // request came from so the receiving agent knows this did not come from the // person in front of it, and carries the exact command that sends an answer diff --git a/internal/relay/relay_test.go b/internal/relay/relay_test.go index c352aea2..16a982d3 100644 --- a/internal/relay/relay_test.go +++ b/internal/relay/relay_test.go @@ -2851,7 +2851,7 @@ func TestTheNudgeNamesTheToolOnlyWhereItExists(t *testing.T) { // has. Naming tools that are not in the list would leave that session believing // it has no way to open anything. func TestSpawnBriefingNamesTheRouteThisSpawnGave(t *testing.T) { - withTools, withCommand := SpawnBriefing(true, false), SpawnBriefing(false, false) + withTools, withCommand := SpawnBriefing(true, RouteSessions), SpawnBriefing(false, RouteSessions) if strings.Contains(withTools, "lich open") { t.Errorf("a session with the tools is sent to the command line:\n%s", withTools) @@ -2882,7 +2882,7 @@ func TestSpawnBriefingNamesTheRouteThisSpawnGave(t *testing.T) { // report back on its own), so the briefing points at it instead of drawing a // line against it, and keeps open_session for what a subagent cannot be. func TestSpawnBriefingSendsCardsToTheAgentTool(t *testing.T) { - cards := SpawnBriefing(true, true) + cards := SpawnBriefing(true, RouteCards) for _, want := range []string{"Agent tool", "card", "isolation", "tools in your list"} { if !strings.Contains(cards, want) { t.Errorf("the card briefing is missing %q:\n%s", want, cards) @@ -2898,6 +2898,24 @@ func TestSpawnBriefingSendsCardsToTheAgentTool(t *testing.T) { } } +// A card as deep as lich nests them is told its Agent tool stays inside it, and +// is not sent to open sessions instead, which would nest cards by another door. +func TestSpawnBriefingKeepsANativeCardsFanOutUnderIt(t *testing.T) { + for _, hasTools := range []bool{true, false} { + native := SpawnBriefing(hasTools, RouteNative) + for _, want := range []string{"Agent tool", "inside this session", "rather than with new lich sessions"} { + if !strings.Contains(native, want) { + t.Errorf("the native briefing is missing %q:\n%s", want, native) + } + } + for _, unwanted := range []string{"lich open", "tools in your list", `"`} { + if strings.Contains(native, unwanted) { + t.Errorf("the native briefing carries %q:\n%s", unwanted, native) + } + } + } +} + // Stopping a turn is not answering the errand it was running. lich raises // "interrupted" for a turn the user ended at the PTY (internal/terminal, // Service.noteInterrupt), and the errand has to survive it: the target is still diff --git a/internal/spawn/spawn.go b/internal/spawn/spawn.go index 26a21730..01990df2 100644 --- a/internal/spawn/spawn.go +++ b/internal/spawn/spawn.go @@ -89,7 +89,8 @@ type Sessions interface { SetSessionModel(sessionID, model string) error SetSessionEffort(sessionID, effort string) error SetSessionUltracode(sessionID string) error - SetSessionSubagent(sessionID string) error + SetSessionSubagentDepth(sessionID string, depth int) error + SessionSubagentDepth(sessionID string) int SetRunEntrypoint(sessionID, entrypoint string) error RenameSession(sessionID, label string) error SetSessionFolder(sessionID, folder string) error @@ -233,8 +234,9 @@ func (s *Service) Open( // - it is filed with the caller, so workers sit under whoever asked for them: // in the caller's folder, or one named after the caller's label that the // caller is filed under too (subagentFolder); -// - it is marked on its row (SetSessionSubagent), so every spawn of it keeps -// its own subagents native: a card does not open cards. +// - its row records how deep it runs, one level below the caller +// (SetSessionSubagentDepth), so every spawn of it knows whether its own +// subagents may still become cards (internal/terminal, subagentCardsOn). // // The project is always the caller's. A caller that is not a session lich holds // is refused before anything is opened: there is no checkout to share and no @@ -249,7 +251,7 @@ func (s *Service) OpenSubagent( } // request is one Open or OpenSubagent call. subagent selects OpenSubagent's -// placement, folder and row mark. +// placement, folder and row depth. type request struct { fromID, projectName, kind, worktree, base, model, effort, folder string ultracode, subagent bool @@ -345,7 +347,7 @@ func (s *Service) openSession(req request) (Session, error) { // default and the failure is reported once it is running. callerErr := s.fileCaller(target.ID, callerToFile, folder) overrideErr := s.recordOverrides(id, label, model, effort, req.ultracode) - markErr := s.markSubagent(id, label, req.subagent) + markErr := s.markSubagent(id, label, req) if err := s.term.Start(id, target.ID, cwd, kind, "", opened.Name, false, at.setup, startCols, startRows); err != nil { return Session{}, fmt.Errorf("session %q was created but its terminal did not start: %w", label, err) } @@ -403,18 +405,20 @@ func (s *Service) fileCaller(projectID, callerID, folder string) error { return nil } -// markSubagent records a subagent's mark on its row before its terminal starts, -// which reads it back (internal/terminal, subagentCardsOn). A write that fails -// is reported once the session is running, for the reason recordOverrides -// gives. -func (s *Service) markSubagent(id, label string, subagent bool) error { - if !subagent { +// markSubagent records a subagent's depth on its row before its terminal +// starts, which reads it back (internal/terminal, subagentCardsOn): one level +// below the session that asked for it. A write that fails is reported once the +// session is running, for the reason recordOverrides gives. +func (s *Service) markSubagent(id, label string, req request) error { + if !req.subagent { return nil } - if err := s.sessions.SetSessionSubagent(id); err != nil { + depth := s.sessions.SessionSubagentDepth(req.fromID) + 1 + if err := s.sessions.SetSessionSubagentDepth(id, depth); err != nil { return fmt.Errorf( - "session %q is open, but it could not be marked as a subagent, so its own "+ - "subagents may open cards: %w", + "session %q is open, but it could not be marked as a subagent, so it runs as "+ + "a session nobody opened: its own subagents may open cards, and it does not "+ + "report back by itself: %w", label, err, ) } diff --git a/internal/spawn/spawn_test.go b/internal/spawn/spawn_test.go index ff8edf8b..613b48a5 100644 --- a/internal/spawn/spawn_test.go +++ b/internal/spawn/spawn_test.go @@ -45,8 +45,8 @@ type fakeSessions struct { // ultracodes records each session row ultracode was turned on for. ultracodes map[string]bool ultracodeErr error - // subagents records each session row marked as a subagent. - subagents map[string]bool + // subagents records how deep each session row marked as a subagent runs. + subagents map[string]int subagentErr error // entrypoints records the run command written on each session row, keyed by // session id. Only a Run card gets one from this service, so a row in here is @@ -257,17 +257,21 @@ func (f *fakeSessions) SetSessionUltracode(sessionID string) error { return nil } -func (f *fakeSessions) SetSessionSubagent(sessionID string) error { +func (f *fakeSessions) SetSessionSubagentDepth(sessionID string, depth int) error { if f.subagentErr != nil { return f.subagentErr } if f.subagents == nil { - f.subagents = map[string]bool{} + f.subagents = map[string]int{} } - f.subagents[sessionID] = true + f.subagents[sessionID] = depth return nil } +func (f *fakeSessions) SessionSubagentDepth(sessionID string) int { + return f.subagents[sessionID] +} + func (f *fakeSessions) SetRunEntrypoint(sessionID, entrypoint string) error { if f.entrypointErr != nil { return f.entrypointErr diff --git a/internal/spawn/subagent_test.go b/internal/spawn/subagent_test.go index 54064bbb..a17404d3 100644 --- a/internal/spawn/subagent_test.go +++ b/internal/spawn/subagent_test.go @@ -136,8 +136,8 @@ func TestOpenSubagentPassesTheOverridesThrough(t *testing.T) { } } -// The mark is on the row before the terminal starts, which reads it to keep the -// worker's own subagents native. +// The depth is on the row before the terminal starts, which reads it to decide +// whether the worker's own subagents may still become cards. func TestOpenSubagentMarksTheRow(t *testing.T) { svc, sessions, _, _, _ := newService(t) @@ -145,14 +145,28 @@ func TestOpenSubagentMarksTheRow(t *testing.T) { if err != nil { t.Fatalf("OpenSubagent: %v", err) } - if !sessions.subagents[opened.ID] { - t.Errorf("subagents = %v, want the worker marked", sessions.subagents) + if sessions.subagents[opened.ID] != 1 { + t.Errorf("subagents = %v, want the worker one level deep", sessions.subagents) } - if plain, _ := svc.Open("s1", "", "", "", "", "", "", "", false); sessions.subagents[plain.ID] { + if plain, _ := svc.Open("s1", "", "", "", "", "", "", "", false); sessions.subagents[plain.ID] != 0 { t.Error("a plain Open was marked as a subagent") } } +// A worker a worker opened runs one level below the one that asked for it. +func TestOpenSubagentFromAWorkerGoesOneLevelDeeper(t *testing.T) { + svc, sessions, _, _, _ := newService(t) + sessions.subagents = map[string]int{"s1": 1} + + opened, err := svc.OpenSubagent("s1", "", "", "", "", "", false) + if err != nil { + t.Fatalf("OpenSubagent: %v", err) + } + if got := sessions.subagents[opened.ID]; got != 2 { + t.Errorf("the worker's worker is %d deep, want 2", got) + } +} + func TestOpenSubagentStartsEvenWhenTheMarkCannotBeWritten(t *testing.T) { svc, sessions, _, term, _ := newService(t) sessions.subagentErr = errors.New("disk full") diff --git a/internal/store/migrate.go b/internal/store/migrate.go index 84fa7512..6c76f865 100644 --- a/internal/store/migrate.go +++ b/internal/store/migrate.go @@ -59,9 +59,11 @@ func addSessionUltracode(tx *sql.Tx) error { } // addSessionSubagent is version 5: whether a session was opened as another -// session's subagent (SetSessionSubagent). A flag for the same reason as -// ultracode: every spawn of the worker, a resume included, must keep its own -// subagents native, or a card would open cards of its own. +// session's subagent, on the row for the same reason as ultracode: every spawn +// of the worker, a resume included, must know it. Written as a 0/1 flag then; +// the same column now holds how deep the worker runs +// (SetSessionSubagentDepth), and a 1 written by an older lich reads as one +// level down, which is what it was. func addSessionSubagent(tx *sql.Tx) error { _, err := tx.Exec(`ALTER TABLE sessions ADD COLUMN subagent INTEGER NOT NULL DEFAULT 0`) return err diff --git a/internal/store/mutations.go b/internal/store/mutations.go index 9cca23e8..5b1cc842 100644 --- a/internal/store/mutations.go +++ b/internal/store/mutations.go @@ -303,7 +303,7 @@ func (s *Service) ReopenSession(sessionID, newSessionID string) (*Session, error // reopen is the resume behind both doors: it finds one parked session with the // given WHERE clause and re-adds it to the workspace under a fresh id // (newSessionID), carrying over the old label, kind, path, provider session id, -// label_auto flag, model, effort, ultracode, subagent mark, entrypoint, sandbox, pin, origin and scheduled prompt. +// label_auto flag, model, effort, ultracode, subagent depth, entrypoint, sandbox, pin, origin and scheduled prompt. // The fresh id is deliberate: it makes the frontend treat the card as // never-spawned, so its resume prompt fires and the provider conversation // continues instead of starting cold. @@ -315,7 +315,7 @@ func (s *Service) reopen(newSessionID, where string, args ...any) (*Session, err // cycle — reinserting without it would reset to 1 and let the ai-title // stomp the chosen name, breaking SetSessionTitle's contract. // - // model, effort, ultracode, the subagent mark and entrypoint ride along for the same reason, one rung lower: + // model, effort, ultracode, the subagent depth and entrypoint ride along for the same reason, one rung lower: // they are what the session was born with, and a resumed card that the // user restarts as a new conversation starts on them again; a reinsert // that dropped the entrypoint would put the terminal back on a bare shell @@ -340,7 +340,8 @@ func (s *Service) reopen(newSessionID, where string, args ...any) (*Session, err // session, and the row is what says where it belongs. var labelAuto int var projectID, model, effort, entrypoint, sandbox, folder, color string - var run, ultracode, subagent bool + var run, ultracode bool + var subagent int var forkOffset float64 row := tx.QueryRow( `SELECT id, project_id, label, kind, path, provider_session_id, label_auto, @@ -728,38 +729,44 @@ func (s *Service) SessionUltracode(sessionID string) bool { return on } -// SetSessionSubagent records that a session was opened as another session's -// subagent (internal/spawn, OpenSubagent), so every spawn of it, a resume -// included, keeps its own subagents native. -func (s *Service) SetSessionSubagent(sessionID string) error { +// SetSessionSubagentDepth records that a session was opened as another +// session's subagent, depth levels below a session nobody opened as one +// (internal/spawn, OpenSubagent), so every spawn of it, a resume included, +// knows how deep it runs. The subagent column holds the depth; 0 is a session +// nobody opened as a subagent, which is every row written before nesting. +func (s *Service) SetSessionSubagentDepth(sessionID string, depth int) error { if _, err := s.db.Exec( - `UPDATE sessions SET subagent = 1 WHERE id = ?`, sessionID, + `UPDATE sessions SET subagent = ? WHERE id = ?`, depth, sessionID, ); err != nil { - return fmt.Errorf("mark %q as a subagent: %w", sessionID, err) + return fmt.Errorf("mark %q as a subagent %d deep: %w", sessionID, depth, err) } return nil } -// SessionSubagent reports whether a session was opened as a subagent. A read -// failure answers false, which leaves the user's setting in charge. -func (s *Service) SessionSubagent(sessionID string) bool { - var on bool +// SessionSubagentDepth is how many subagent levels below a session nobody +// opened as one this session runs, 0 for such a session. A read failure answers +// 0, which leaves the user's setting in charge. +func (s *Service) SessionSubagentDepth(sessionID string) int { + var depth int if err := s.db.QueryRow( `SELECT subagent FROM sessions WHERE id = ?`, sessionID, - ).Scan(&on); err != nil { - return false + ).Scan(&depth); err != nil { + return 0 } - return on + return depth } -// InheritSubagent marks a fork a subagent when the session that ran the -// conversation it branches, forkedFrom, was one. The fork's row is a fresh -// insert, so without this a copy of a worker would open cards of its own. +// InheritSubagent gives a fork the subagent depth of the session that ran the +// conversation it branches, forkedFrom, when that one was a subagent. The +// fork's row is a fresh insert, so without this a copy of a worker would open +// cards a worker that deep does not. func (s *Service) InheritSubagent(sessionID, forkedFrom string) error { if _, err := s.db.Exec( - `UPDATE sessions SET subagent = 1 WHERE id = ? AND EXISTS - (SELECT 1 FROM sessions WHERE provider_session_id = ? AND subagent = 1)`, - sessionID, forkedFrom, + `UPDATE sessions SET subagent = + (SELECT MAX(subagent) FROM sessions WHERE provider_session_id = ?) + WHERE id = ? AND EXISTS + (SELECT 1 FROM sessions WHERE provider_session_id = ? AND subagent > 0)`, + forkedFrom, sessionID, forkedFrom, ); err != nil { return fmt.Errorf("inherit the subagent mark on %q: %w", sessionID, err) } diff --git a/internal/store/subagent_test.go b/internal/store/subagent_test.go index 67916c78..a783c787 100644 --- a/internal/store/subagent_test.go +++ b/internal/store/subagent_test.go @@ -2,24 +2,27 @@ package store import "testing" -func TestSessionSubagentRoundTripsAndSurvivesAResume(t *testing.T) { +func TestSessionSubagentDepthRoundTripsAndSurvivesAResume(t *testing.T) { svc := newTestStore(t) _ = svc.AddProject("p1", "alpha", "/tmp/alpha") _ = svc.AddSession("p1", "base", "Session 1", "claude", "", 2, "") _ = svc.AddSession("p1", "wt1", "worker", "claude", "/wt/foo", 3, "") - if err := svc.SetSessionSubagent("wt1"); err != nil { - t.Fatalf("SetSessionSubagent: %v", err) + if err := svc.SetSessionSubagentDepth("wt1", 2); err != nil { + t.Fatalf("SetSessionSubagentDepth: %v", err) } - if !svc.SessionSubagent("wt1") || svc.SessionSubagent("base") || svc.SessionSubagent("ghost") { + if got := svc.SessionSubagentDepth("wt1"); got != 2 { + t.Fatalf("SessionSubagentDepth(wt1) = %d, want 2", got) + } + if svc.SessionSubagentDepth("base") != 0 || svc.SessionSubagentDepth("ghost") != 0 { t.Fatal("want only the marked session read as a subagent") } _ = svc.CloseSession("p1", "wt1", "base") if _, err := svc.ReopenWorktreeSession("p1", "/wt/foo", "wt2"); err != nil { t.Fatalf("ReopenWorktreeSession: %v", err) } - if !svc.SessionSubagent("wt2") { - t.Error("the resumed worker lost its subagent mark") + if got := svc.SessionSubagentDepth("wt2"); got != 2 { + t.Errorf("the resumed worker is %d deep, want 2", got) } } @@ -41,9 +44,9 @@ func TestSessionBranchReadsTheSessionsCheckout(t *testing.T) { } } -// A fork of a subagent worker's conversation is marked a subagent too, found by -// the provider conversation it branches, so the copy keeps its own subagents -// native like the worker it was copied from. +// A fork of a subagent worker's conversation is a subagent as deep as the +// worker, found by the provider conversation it branches, so the copy opens +// cards exactly as the worker it was copied from would. func TestInheritSubagentFollowsTheForkedConversation(t *testing.T) { svc := newTestStore(t) _ = svc.AddProject("p1", "alpha", "/tmp/alpha") @@ -53,18 +56,18 @@ func TestInheritSubagentFollowsTheForkedConversation(t *testing.T) { _ = svc.AddSession("p1", "fork2", "Session 4", "claude", "", 5, "") _ = svc.SetProviderSession("worker", "conv-worker") _ = svc.SetProviderSession("plain", "conv-plain") - _ = svc.SetSessionSubagent("worker") + _ = svc.SetSessionSubagentDepth("worker", 2) if err := svc.InheritSubagent("fork1", "conv-worker"); err != nil { t.Fatalf("InheritSubagent: %v", err) } - if !svc.SessionSubagent("fork1") { - t.Error("fork of a worker's conversation is not a subagent, want it marked") + if got := svc.SessionSubagentDepth("fork1"); got != 2 { + t.Errorf("fork of a worker's conversation is %d deep, want the worker's 2", got) } if err := svc.InheritSubagent("fork2", "conv-plain"); err != nil { t.Fatalf("InheritSubagent: %v", err) } - if svc.SessionSubagent("fork2") { + if svc.SessionSubagentDepth("fork2") != 0 { t.Error("fork of a plain conversation is marked a subagent, want it left alone") } } diff --git a/internal/terminal/command.go b/internal/terminal/command.go index ce255a9a..cd212c22 100644 --- a/internal/terminal/command.go +++ b/internal/terminal/command.go @@ -309,7 +309,8 @@ func flagValue(value string) (string, bool) { // come last. Kiro's subcommand opens the session rather than a conversation, so // it comes before every flag. func providerArgs( - kind, name, resume, model, effort, lichBin, agent string, fork, skipPermissions, ultracode, agentCards bool, + kind, name, resume, model, effort, lichBin, agent string, fork, skipPermissions, ultracode bool, + route relay.SubagentRoute, ) []string { mcp := mcpArgs(kind, lichBin) args := append([]string{}, subcommandArgs(kind)...) @@ -320,7 +321,7 @@ func providerArgs( args = append(args, modelArgs(kind, model, resume)...) args = append(args, effortArgs(kind, effort, resume)...) args = append(args, ultracodeArgs(kind, ultracode)...) - args = append(args, briefingArgs(kind, agentCards)...) + args = append(args, briefingArgs(kind, route)...) if kind == providers.Codex { return append(mcp, args...) } @@ -414,12 +415,12 @@ func ultracodeArgs(kind string, on bool) []string { // briefing is worded for what this spawn actually registered: a provider handed // lich's MCP server is pointed at the tools, everyone else at the command line // (relay.SpawnBriefing). -func briefingArgs(kind string, agentCards bool) []string { +func briefingArgs(kind string, route relay.SubagentRoute) []string { flag, ok := briefingFlags[kind] if !ok { return nil } - return []string{flag, relay.SpawnBriefing(providers.AcceptsMCPServer(kind), agentCards)} + return []string{flag, relay.SpawnBriefing(providers.AcceptsMCPServer(kind), route)} } // skipPermissionArgs returns the flag that drops a provider's permission diff --git a/internal/terminal/command_kiro_test.go b/internal/terminal/command_kiro_test.go index c2be7ce1..57c1f57d 100644 --- a/internal/terminal/command_kiro_test.go +++ b/internal/terminal/command_kiro_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/omartelo/lich/internal/providers" + "github.com/omartelo/lich/internal/relay" ) // The Kiro CLI side of a spawn: which agent it is told to run, and the shape of @@ -134,7 +135,7 @@ func TestKiroSpawnsTheChatSubcommandBeforeEveryFlag(t *testing.T) { for _, tt := range tests { got := providerArgs( providers.Kiro, "", tt.resume, tt.model, tt.effort, "/usr/bin/lich", tt.agent, - false, tt.skipPermissions, false, false, + false, tt.skipPermissions, false, relay.RouteSessions, ) if !slices.Equal(got, tt.want) { t.Errorf("%s: args = %v, want %v", tt.name, got, tt.want) diff --git a/internal/terminal/start.go b/internal/terminal/start.go index 0056e5e4..e54680c8 100644 --- a/internal/terminal/start.go +++ b/internal/terminal/start.go @@ -134,6 +134,8 @@ func (s *Service) spawnSession( } skipPermissions := s.store.SkipPermissions(kind, projectID, cwd) ultracode := s.store.Ultracode(kind) || s.store.SessionUltracode(id) + depth := s.store.SessionSubagentDepth(id) + cardsOn := s.subagentCardsOn(kind, depth) // The model and effort are read from the row rather than passed in, so every // spawn that starts a conversation of its own gets them: the window's first // view and a restart. A resume leaves them to the provider (modelArgs). @@ -141,10 +143,10 @@ func (s *Service) spawnSession( bin: resolveCommand(kind, s.store.ProviderBin(kind, projectID), userShell()), args: providerArgs( kind, name, resume, s.store.SessionModel(id), s.store.SessionEffort(id), mcpBin, - kiroPluginAgent(kind), fork, skipPermissions, ultracode, s.briefsAgentCards(id, kind), + kiroPluginAgent(kind), fork, skipPermissions, ultracode, subagentRoute(kind, depth, cardsOn), ), dir: cwd, - env: subagentCardsEnv(s.sessionEnv(id, projectID, cwd), kind, s.subagentCardsOn(id, kind)), + env: subagentEnv(s.sessionEnv(id, projectID, cwd), kind, depth, cardsOn), cols: cols, rows: rows, } diff --git a/internal/terminal/subagent_cards.go b/internal/terminal/subagent_cards.go index f7705655..8d7cba3f 100644 --- a/internal/terminal/subagent_cards.go +++ b/internal/terminal/subagent_cards.go @@ -1,32 +1,62 @@ package terminal import ( + "strconv" + "github.com/omartelo/lich/internal/agentplugin" "github.com/omartelo/lich/internal/providers" + "github.com/omartelo/lich/internal/relay" ) -// subagentCardsEnv tells a Claude Code session's lich-plugin mod to leave its -// subagents native (the mod's agent-cards.js reads LICH_SUBAGENT_CARDS). The -// variable is absent while the setting is on, which is also what a session -// spawned by an older lich sees, so the mod's default stays the cards. -func subagentCardsEnv(env []string, kind string, on bool) []string { - if kind != providers.Claude || on { +// maxSubagentDepth is how many levels of subagent cards lich nests: a session +// opens cards, and so does a card it opened; a card one level further keeps its +// own subagents native. Every level multiplies the cards on screen and the +// sessions a report climbs back through, and two let a worker split its own +// task once more, which is the case nesting was asked for. +const maxSubagentDepth = 2 + +// subagentEnv tells a Claude Code session's lich-plugin mod how deep it runs as +// a subagent (LICH_SUBAGENT_DEPTH, 0 for a session nobody opened as one), which +// the mod's worker-answer.js reads to tell a worker, and to leave its subagents +// native (LICH_SUBAGENT_CARDS=off, read by agent-cards.js) when they do not +// become cards. The second is absent while they do, which is also what a +// session spawned by an older lich sees, so the mod's default stays the cards. +func subagentEnv(env []string, kind string, depth int, cardsOn bool) []string { + if kind != providers.Claude { + return env + } + env = append(env, "LICH_SUBAGENT_DEPTH="+strconv.Itoa(depth)) + if cardsOn { return env } return append(env, "LICH_SUBAGENT_CARDS=off") } -// subagentCardsOn is whether a session's subagents become cards: the user's -// setting, except in a session that is itself a subagent's card -// (store.SessionSubagent), whose own subagents stay native so a worker sharing -// its caller's checkout never opens cards of its own. -// briefsAgentCards is whether this spawn's briefing sends the agent to its own -// Agent tool: a Claude Code session whose subagents become cards, with a plugin -// release whose mod does that installed. -func (s *Service) briefsAgentCards(id, kind string) bool { - return kind == providers.Claude && s.subagentCardsOn(id, kind) && agentplugin.ClaudeRunsSubagentCards() +// subagentRoute is where this spawn's briefing sends the agent to fan work out +// (relay.SubagentRoute): its own Agent tool, as cards, where a plugin release +// whose mod does that is installed; the same tool kept native in a card that +// does not open cards; and lich sessions everywhere else. +func subagentRoute(kind string, depth int, cardsOn bool) relay.SubagentRoute { + if kind != providers.Claude || !agentplugin.ClaudeRunsSubagentCards() { + return relay.RouteSessions + } + if cardsOn { + return relay.RouteCards + } + if depth > 0 { + return relay.RouteNative + } + return relay.RouteSessions } -func (s *Service) subagentCardsOn(id, kind string) bool { - return s.store.SubagentCards(kind) && !s.store.SessionSubagent(id) +// subagentCardsOn is whether a session at depth turns its subagents into cards: +// the user's setting, a session nobody opened as a subagent, or a card above +// maxSubagentDepth whose plugin tells a worker by its depth. Under an older +// plugin a worker's own subagents stay native, since that mod would stop +// answering for a worker that opens cards. +func (s *Service) subagentCardsOn(kind string, depth int) bool { + if !s.store.SubagentCards(kind) { + return false + } + return depth == 0 || (depth < maxSubagentDepth && agentplugin.ClaudeNestsSubagentCards()) } diff --git a/internal/terminal/subagent_cards_test.go b/internal/terminal/subagent_cards_test.go index 027f2f41..f0534902 100644 --- a/internal/terminal/subagent_cards_test.go +++ b/internal/terminal/subagent_cards_test.go @@ -1,43 +1,103 @@ package terminal import ( + "os" + "path/filepath" "slices" "strings" "testing" "github.com/omartelo/lich/internal/providers" + "github.com/omartelo/lich/internal/relay" ) -// The lich-plugin mod reads LICH_SUBAGENT_CARDS to leave a Claude Code -// session's subagents native; lich sets it only when the user turned the -// setting off, so a session spawned with it on carries no variable at all. -func TestSubagentCardsEnv(t *testing.T) { +// withClaudePlugin points Claude Code's config at a directory whose plugin +// state says version is installed, "" for none. +func withClaudePlugin(t *testing.T, version string) { + t.Helper() + dir := t.TempDir() + t.Setenv("CLAUDE_CONFIG_DIR", dir) + if version == "" { + return + } + state := `{"plugins":{"lich@lich-plugin":[{"scope":"user","version":"` + version + `"}]}}` + if err := os.MkdirAll(filepath.Join(dir, "plugins"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "plugins", "installed_plugins.json"), []byte(state), 0o644); err != nil { + t.Fatal(err) + } +} + +// The lich-plugin mod reads LICH_SUBAGENT_DEPTH on every Claude Code session to +// tell a worker, and LICH_SUBAGENT_CARDS=off only where its subagents stay +// native, so a session whose subagents become cards carries no off at all. +func TestSubagentEnv(t *testing.T) { base := []string{"A=1"} cases := []struct { - name string - kind string - on bool - want []string + name string + kind string + depth int + cardsOn bool + want []string }{ - {"claude with cards on", providers.Claude, true, []string{"A=1"}}, - {"claude with cards off", providers.Claude, false, []string{"A=1", "LICH_SUBAGENT_CARDS=off"}}, - {"another provider with the setting off", providers.Codex, false, []string{"A=1"}}, + {"claude with cards on", providers.Claude, 0, true, []string{"A=1", "LICH_SUBAGENT_DEPTH=0"}}, + {"claude with cards off", providers.Claude, 0, false, + []string{"A=1", "LICH_SUBAGENT_DEPTH=0", "LICH_SUBAGENT_CARDS=off"}}, + {"a worker that opens cards", providers.Claude, 1, true, []string{"A=1", "LICH_SUBAGENT_DEPTH=1"}}, + {"a worker kept native", providers.Claude, 2, false, + []string{"A=1", "LICH_SUBAGENT_DEPTH=2", "LICH_SUBAGENT_CARDS=off"}}, + {"another provider", providers.Codex, 1, false, []string{"A=1"}}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { - got := subagentCardsEnv(slices.Clone(base), c.kind, c.on) + got := subagentEnv(slices.Clone(base), c.kind, c.depth, c.cardsOn) if !slices.Equal(got, c.want) { - t.Errorf("subagentCardsEnv = %v, want %v", got, c.want) + t.Errorf("subagentEnv = %v, want %v", got, c.want) + } + }) + } +} + +// A spawn is briefed for where its subagents actually run: as cards, kept +// native in a card, or inside a harness lich has no mod in. +func TestSubagentRoute(t *testing.T) { + withClaudePlugin(t, "0.19.0") + cases := []struct { + name string + kind string + depth int + cardsOn bool + want relay.SubagentRoute + }{ + {"a session whose subagents become cards", providers.Claude, 0, true, relay.RouteCards}, + {"a worker whose subagents become cards", providers.Claude, 1, true, relay.RouteCards}, + {"a worker kept native", providers.Claude, 2, false, relay.RouteNative}, + {"a session the user kept native", providers.Claude, 0, false, relay.RouteSessions}, + {"another provider's worker", providers.Codex, 1, false, relay.RouteSessions}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if got := subagentRoute(c.kind, c.depth, c.cardsOn); got != c.want { + t.Errorf("subagentRoute = %v, want %v", got, c.want) } }) } } +// Without a plugin whose mod opens cards, nothing is told about the Agent tool. +func TestSubagentRouteWithoutTheCardPlugin(t *testing.T) { + withClaudePlugin(t, "0.14.0") + if got := subagentRoute(providers.Claude, 1, false); got != relay.RouteSessions { + t.Errorf("subagentRoute = %v, want the sessions route", got) + } +} + // A Claude Code spawn whose subagents become lich cards is briefed to fan out // through its own Agent tool; every other spawn keeps the line against it. func TestBriefingFollowsSubagentCards(t *testing.T) { - cards := providerArgs(providers.Claude, "", "", "", "", "/usr/bin/lich", "", false, false, false, true) - plain := providerArgs(providers.Claude, "", "", "", "", "/usr/bin/lich", "", false, false, false, false) + cards := providerArgs(providers.Claude, "", "", "", "", "/usr/bin/lich", "", false, false, false, relay.RouteCards) + plain := providerArgs(providers.Claude, "", "", "", "", "/usr/bin/lich", "", false, false, false, relay.RouteSessions) briefing := func(args []string) string { at := slices.Index(args, "--append-system-prompt") if at < 0 || at+1 >= len(args) { diff --git a/internal/terminal/subagent_cards_unix_test.go b/internal/terminal/subagent_cards_unix_test.go index 0f940b8d..5d0142cf 100644 --- a/internal/terminal/subagent_cards_unix_test.go +++ b/internal/terminal/subagent_cards_unix_test.go @@ -11,35 +11,43 @@ import ( // Unix-only with the stubBins it reads, which live in the PTY suite. -// A session opened as a subagent keeps its own subagents native whatever the -// setting says: a card does not open cards. -func TestASubagentSessionNeverOpensCards(t *testing.T) { +// A worker opens cards one level down, and only under a plugin that tells a +// worker by its depth; a card at maxSubagentDepth keeps its own subagents +// native whatever the setting says. +func TestSubagentCardsOnFollowsTheDepth(t *testing.T) { cases := []struct { name string bins stubBins + plugin string wantOn bool }{ - {"setting on", stubBins{}, true}, - {"setting off", stubBins{subagentCardsOff: true}, false}, - {"opened as a subagent", stubBins{subagentSession: true}, false}, + {"setting on", stubBins{}, "0.19.0", true}, + {"setting off", stubBins{subagentCardsOff: true}, "0.19.0", false}, + {"a worker", stubBins{subagentDepth: 1}, "0.19.0", true}, + {"a worker under an older plugin", stubBins{subagentDepth: 1}, "0.18.4", false}, + {"a worker with the setting off", stubBins{subagentDepth: 1, subagentCardsOff: true}, "0.19.0", false}, + {"a worker's worker", stubBins{subagentDepth: 2}, "0.19.0", false}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { + withClaudePlugin(t, c.plugin) svc := New(c.bins, nil, events.New()) - if got := svc.subagentCardsOn("s1", providers.Claude); got != c.wantOn { + depth := c.bins.SessionSubagentDepth("s1") + if got := svc.subagentCardsOn(providers.Claude, depth); got != c.wantOn { t.Errorf("subagentCardsOn = %v, want %v", got, c.wantOn) } }) } } -// A fork of a worker's conversation carries the subagent mark onto its own row -// before it spawns, so the copy never opens cards either. -func TestForkOfASubagentKeepsItsSubagentsNative(t *testing.T) { +// A fork of a worker's conversation carries the worker's depth onto its own +// row before it spawns, so the copy opens cards exactly as the worker would. +func TestForkOfASubagentKeepsItsDepth(t *testing.T) { + withClaudePlugin(t, "0.19.0") store := stubBins{ bin: stayAliveBin(t), - subagentParents: map[string]bool{"conv-worker": true}, - inheritedSubagent: map[string]bool{}, + subagentParents: map[string]int{"conv-worker": maxSubagentDepth}, + inheritedSubagent: map[string]int{}, } svc := New(store, nil, events.New()) t.Cleanup(func() { _ = svc.Close("s2") }) @@ -47,10 +55,10 @@ func TestForkOfASubagentKeepsItsSubagentsNative(t *testing.T) { if err := svc.Start("s2", "p1", t.TempDir(), providers.Claude, "conv-worker", "", true, false, 80, 24); err != nil { t.Fatalf("Start = %v, want nil", err) } - if !store.inheritedSubagent["s2"] { - t.Error("the fork's row was not marked a subagent") + if got := store.inheritedSubagent["s2"]; got != maxSubagentDepth { + t.Errorf("the fork's row is %d deep, want the worker's %d", got, maxSubagentDepth) } - if svc.subagentCardsOn("s2", providers.Claude) { - t.Error("the fork of a worker opens cards, want its subagents native") + if svc.subagentCardsOn(providers.Claude, store.SessionSubagentDepth("s2")) { + t.Error("the fork of a worker at the limit opens cards, want its subagents native") } } diff --git a/internal/terminal/terminal.go b/internal/terminal/terminal.go index f0b764e7..184a07d4 100644 --- a/internal/terminal/terminal.go +++ b/internal/terminal/terminal.go @@ -231,7 +231,7 @@ type Store interface { SessionModel(sessionID string) string SessionEffort(sessionID string) string SessionUltracode(sessionID string) bool - SessionSubagent(sessionID string) bool + SessionSubagentDepth(sessionID string) int InheritUltracode(sessionID, forkedFrom string) error InheritSubagent(sessionID, forkedFrom string) error SessionEntrypoint(sessionID string) string diff --git a/internal/terminal/terminal_test.go b/internal/terminal/terminal_test.go index beee9f37..bf5b6513 100644 --- a/internal/terminal/terminal_test.go +++ b/internal/terminal/terminal_test.go @@ -20,6 +20,7 @@ import ( "github.com/omartelo/lich/internal/events" "github.com/omartelo/lich/internal/providers" + "github.com/omartelo/lich/internal/relay" "github.com/omartelo/lich/internal/shquote" "github.com/omartelo/lich/internal/store" ) @@ -50,9 +51,9 @@ type stubBins struct { skipPerms bool ultracode bool subagentCardsOff bool - subagentSession bool - subagentParents map[string]bool - inheritedSubagent map[string]bool + subagentDepth int + subagentParents map[string]int + inheritedSubagent map[string]int // sessionUltracode is the per-session flag a session opened with ultracode // carries on its row, apart from the provider-wide ultracode above. sessionUltracode bool @@ -128,12 +129,15 @@ func (s stubBins) SessionModel(_ string) string { return s.model } func (s stubBins) SessionEffort(_ string) string { return s.effort } func (s stubBins) Ultracode(_ string) bool { return s.ultracode } func (s stubBins) SubagentCards(_ string) bool { return !s.subagentCardsOff } -func (s stubBins) SessionSubagent(id string) bool { - return s.subagentSession || s.inheritedSubagent[id] +func (s stubBins) SessionSubagentDepth(id string) int { + if depth, ok := s.inheritedSubagent[id]; ok { + return depth + } + return s.subagentDepth } func (s stubBins) InheritSubagent(sessionID, forkedFrom string) error { - if s.subagentParents[forkedFrom] { - s.inheritedSubagent[sessionID] = true + if depth := s.subagentParents[forkedFrom]; depth > 0 { + s.inheritedSubagent[sessionID] = depth } return nil } @@ -1315,7 +1319,7 @@ func spawnPins(t *testing.T, got []string, want ...string) { func TestProviderArgsRegistersTheMCPServer(t *testing.T) { const bin = "/usr/bin/lich" - claude := providerArgs(providers.Claude, "", "", "", "", bin, "", false, false, false, false) + claude := providerArgs(providers.Claude, "", "", "", "", bin, "", false, false, false, relay.RouteSessions) at := slices.Index(claude, "--mcp-config") if at < 0 || at+1 >= len(claude) { t.Fatalf("claude args = %v", claude) @@ -1340,7 +1344,7 @@ func TestProviderArgsRegistersTheMCPServer(t *testing.T) { t.Errorf("a secret reached the argv, which /proc exposes: %q", claude[at+1]) } - codex := providerArgs(providers.Codex, "", "", "", "", bin, "", false, false, false, false) + codex := providerArgs(providers.Codex, "", "", "", "", bin, "", false, false, false, relay.RouteSessions) want := []string{ "-c", `mcp_servers.lich.command="/usr/bin/lich"`, "-c", `mcp_servers.lich.args=["mcp"]`, @@ -1355,7 +1359,7 @@ func TestProviderArgsRegistersTheMCPServer(t *testing.T) { // follows it, and Codex reads resume as a subcommand that every global option // must precede. func TestProviderArgsOrdersEachProvidersConstraint(t *testing.T) { - claude := providerArgs(providers.Claude, "lich-4f2a", "conv-1", "", "", "/usr/bin/lich", "", false, true, false, false) + claude := providerArgs(providers.Claude, "lich-4f2a", "conv-1", "", "", "/usr/bin/lich", "", false, true, false, relay.RouteSessions) if claude[len(claude)-2] != "--mcp-config" { t.Errorf("--mcp-config is not last, so it eats what follows: %v", claude) } @@ -1367,7 +1371,7 @@ func TestProviderArgsOrdersEachProvidersConstraint(t *testing.T) { // A resuming session is not named (nameArgs), so --name is pinned on the // spawn that carries it: a session being born. - born := providerArgs(providers.Claude, "lich-4f2a", "", "", "", "/usr/bin/lich", "", false, true, false, false) + born := providerArgs(providers.Claude, "lich-4f2a", "", "", "", "/usr/bin/lich", "", false, true, false, relay.RouteSessions) if born[len(born)-2] != "--mcp-config" { t.Errorf("--mcp-config is not last, so it eats what follows: %v", born) } @@ -1375,7 +1379,7 @@ func TestProviderArgsOrdersEachProvidersConstraint(t *testing.T) { t.Errorf("claude args lost --name: %v", born) } - codex := providerArgs(providers.Codex, "", "conv-1", "", "", "/usr/bin/lich", "", false, false, false, false) + codex := providerArgs(providers.Codex, "", "conv-1", "", "", "/usr/bin/lich", "", false, false, false, relay.RouteSessions) resume := slices.Index(codex, "resume") if resume < 0 { t.Fatalf("codex args lost the resume subcommand: %v", codex) @@ -1414,7 +1418,7 @@ func TestModelAndEffortAreBirthValues(t *testing.T) { t.Errorf("%s at birth = %v, want %v", tc.kind, born, tc.born) } for _, fork := range []bool{false, true} { - args := providerArgs(tc.kind, "", "conv-1", tc.model, tc.effort, "", "", fork, false, false, false) + args := providerArgs(tc.kind, "", "conv-1", tc.model, tc.effort, "", "", fork, false, false, relay.RouteSessions) for _, flag := range tc.born { if slices.Contains(args, flag) { t.Errorf("%s resume (fork=%v) = %v, carries birth value %q", tc.kind, fork, args, flag) @@ -1446,7 +1450,7 @@ func TestProviderArgsWithoutARegistration(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - args := providerArgs(tt.kind, "", "", "", "", tt.bin, "", false, false, false, false) + args := providerArgs(tt.kind, "", "", "", "", tt.bin, "", false, false, false, relay.RouteSessions) if tt.bare && len(args) != 0 { t.Errorf("args = %v, want none", args) } @@ -1471,7 +1475,7 @@ func TestTheBriefingGoesToTheProvidersThatTakeOne(t *testing.T) { } for kind, want := range briefed { t.Run(kind, func(t *testing.T) { - args := providerArgs(kind, "", "", "", "", "/usr/bin/lich", "", false, false, false, false) + args := providerArgs(kind, "", "", "", "", "/usr/bin/lich", "", false, false, false, relay.RouteSessions) flag := slices.Index(args, "--append-system-prompt") if flag < 0 || flag+1 >= len(args) { t.Fatalf("args = %v, want a briefing", args) @@ -1484,7 +1488,7 @@ func TestTheBriefingGoesToTheProvidersThatTakeOne(t *testing.T) { for _, kind := range []string{providers.Codex, providers.OpenCode, providers.Crush, KindShell} { t.Run(kind+" takes none", func(t *testing.T) { - args := providerArgs(kind, "", "", "", "", "/usr/bin/lich", "", false, false, false, false) + args := providerArgs(kind, "", "", "", "", "/usr/bin/lich", "", false, false, false, relay.RouteSessions) if slices.Contains(args, "--append-system-prompt") { t.Errorf("args = %v, want no briefing: %s has no flag that appends one", args, kind) }