diff --git a/docs/spec/worktree-monitor.md b/docs/spec/worktree-monitor.md index bc3063f0..279c5255 100644 --- a/docs/spec/worktree-monitor.md +++ b/docs/spec/worktree-monitor.md @@ -80,14 +80,15 @@ Machine-level state persists in `~/.treemon/config.json` (or `$TREEMON_CONFIG_DI #### Copilot CLI Status Detection -Copilot CLI writes streaming events to `events.jsonl`. Recognized events: `user.message`, `assistant.turn_start`, `assistant.turn_end`, `assistant.message`, `skill.invoked`, `subagent.started`, `subagent.completed`. Status, the current skill, and the last user/assistant messages are all derived from a single **forward fold** of the file (oldest→newest, `foldSessionEvents` in `CopilotDetector.fs`) through an **append-aware per-session cache**, so a `skill.invoked` or `user.message` megabytes back is still seen regardless of session size. +Copilot CLI writes streaming events to `events.jsonl`. Recognized events: `user.message`, `assistant.turn_start`, `assistant.turn_end`, `assistant.message`, `skill.invoked`, `subagent.started`, `subagent.completed`, `system.notification`. Status, the current skill, and the last user/assistant messages are all derived from a single **forward fold** of the file (oldest→newest, `foldSessionEvents` in `CopilotDetector.fs`) through an **append-aware per-session cache**, so a `skill.invoked` or `user.message` megabytes back is still seen regardless of session size. **Why forward + incremental.** A `/skill` invocation is written once, at session start — a `skill.invoked` event plus a `user.message` with `source: "skill-"` and `` content; there is no later plain `/skill` line. Sub-agents (Task tool) emit only `subagent.*` + tool/assistant events, never `user.message`. On real sessions the skill invocation and the sole `user.message` sit 2–12 MB back while sessions run 2–207 MB, so a fixed ~1 MB backward tail scan saw neither and reported a generic `Working` with no user line. Folding the whole stream forward fixes this; the incremental cache keeps it O(new bytes) per refresh instead of re-reading the file each cycle. -**Fold state** (`SessionScanCache`): `CurrentSkill`, `LastUserMessage` (genuine prompts only), `LastAssistantMessage`, `RawStatus`, `LastAssistantWasAskUser`, `SubagentDepth`. Key rules: +**Fold state** (`SessionScanCache`): `CurrentSkill`, `LastUserMessage` (genuine prompts only), `LastAssistantMessage`, `RawStatus`, `LastAssistantWasAskUser`, `SubagentDepth`, `RunningBackgroundAgents`. Key rules: - **Current skill persists** from its `skill.invoked` until a genuine new top-level `user.message` supersedes it. An `ask_user` reply mid-skill resumes the same skill; a `` injection is transparent (not a boundary). - **`` injections are never recorded** as `LastUserMessage` — but, like any `user.message`, they still set `RawStatus = Working`. `skill.invoked` alone does not change `RawStatus` (the following `assistant.message` does). +- **Background agents keep a finished turn Working.** A `task` tool call with `mode: "background"` (e.g. the `review` skill launching a wave of `review-runner`s) is a fire-and-forget agent: the session launches it, then ends its turn (`turn_end`) to wait for it, freezing the file mtime. Each launch adds its agent id to `RunningBackgroundAgents`; the matching `system.notification` of kind `agent_idle` (structured `data.kind.agentId`) removes it. `turn_end` yields `Done` only when that set is empty — while any launched agent is still running the turn stays `Working` (red dot), so a review parked on its background reviewers no longer decays to a blue `Done` dot (and still counts in the Overview band's Reviewing group). Launches are gated on `SubagentDepth = 0` (a nested sub-agent's launch is not the top-level session's), mirroring skill gating. - **Sub-agent depth gating:** `subagent.started`/`subagent.completed` bracket a sub-agent's events; a `skill.invoked` inside a bracket is the sub-agent's and is ignored (gated on `SubagentDepth = 0`) so the top-level skill the user invoked (e.g. `bd-execute`) is what surfaces. A sub-agent's `skill.invoked` bubbles into the parent file with no depth marker of its own, so the enclosing bracket is the only signal — see the depth-gating comment on `foldForwardEvent`. **Incremental cache.** A `ConcurrentDictionary` (`{ State; Length }`) holds the fold state plus bytes consumed. Each refresh reads only bytes `[Length, fileLength)` (`FileUtils.readByteRangeLines`, a chunked int64 read), folds the complete newline-terminated lines onto the cached state, and advances `Length` past the last `\n` (a partial trailing line is left for next time). A shrunken file (rotation) or a cache miss triggers a full rescan; `getRefreshData` prunes entries whose file is missing or older than the 2 h Idle cutoff, throttled to once per 5 minutes. `CodingToolStatus` assembles a worktree's Copilot surface from **one** `getRefreshData` call (`Status`, `Mtime`, `CurrentSkill`, `LastUserMessage`, `LastMessage`) instead of four separate ~1 MB scans. A path-based `getStatusFromEventsFile` (full `File.ReadLines` forward scan) is retained for the status unit tests. diff --git a/src/Server/CopilotDetector.fs b/src/Server/CopilotDetector.fs index 17030a39..218094ee 100644 --- a/src/Server/CopilotDetector.fs +++ b/src/Server/CopilotDetector.fs @@ -121,16 +121,16 @@ type private SkillEvent = /// A plain user.message: a genuine new request UNLESS it turns out to be an ask_user reply. | UserRequest -let private tryReadSkillArgument (req: JsonElement) = - let skillFromObject (obj: JsonElement) = - match obj.TryGetProperty("skill") with - | true, s when s.ValueKind = JsonValueKind.String -> - let name = s.GetString() - if String.IsNullOrWhiteSpace(name) then None else Some name - | _ -> None +let private toolRequestName (req: JsonElement) : string option = + match req.TryGetProperty("name") with + | true, n when n.ValueKind = JsonValueKind.String -> Some(n.GetString()) + | _ -> None - // Real events.jsonl encodes arguments as a nested object ({"skill":"..."}); the session-store - // schema names it arguments_json as a JSON string. Handle both so the source doesn't matter. +/// Apply an extractor to a tool call's arguments, whether they are a nested object (real +/// events.jsonl encodes `arguments` as `{"skill":"..."}`) or a JSON string (the session-store schema +/// names it `arguments_json`). The extractor must return a value, not a `JsonElement`: a parsed +/// `arguments_json` document is disposed on return, so any element read from it would dangle. +let private withToolArguments (extract: JsonElement -> 'a option) (req: JsonElement) : 'a option = let argsElement = match req.TryGetProperty("arguments"), req.TryGetProperty("arguments_json") with | (true, a), _ -> Some a @@ -140,14 +140,49 @@ let private tryReadSkillArgument (req: JsonElement) = argsElement |> Option.bind (fun args -> match args.ValueKind with - | JsonValueKind.Object -> skillFromObject args + | JsonValueKind.Object -> extract args | JsonValueKind.String -> try use argsDoc = JsonDocument.Parse(args.GetString()) - skillFromObject argsDoc.RootElement + extract argsDoc.RootElement with _ -> None | _ -> None) +let private tryReadSkillArgument (req: JsonElement) = + req + |> withToolArguments (fun obj -> + match obj.TryGetProperty("skill") with + | true, s when s.ValueKind = JsonValueKind.String -> + let name = s.GetString() + if String.IsNullOrWhiteSpace(name) then None else Some name + | _ -> None) + +/// The agent id of a `task` tool call launched with `mode: "background"` (a fire-and-forget agent the +/// session keeps working alongside and is later woken about via a `system.notification`), or None for +/// any other tool call. A SYNC task agent blocks the turn, so only background launches leave work +/// outstanding once the turn ends — the signal that separates a "waiting on background agents" +/// (Working) session from a genuinely finished (Done) one. +let private backgroundAgentName (req: JsonElement) : string option = + if toolRequestName req <> Some "task" then + None + else + req + |> withToolArguments (fun args -> + let isBackground = + match args.TryGetProperty("mode") with + | true, m when m.ValueKind = JsonValueKind.String -> + String.Equals(m.GetString(), "background", StringComparison.OrdinalIgnoreCase) + | _ -> false + + if not isBackground then + None + else + match args.TryGetProperty("name") with + | true, n when n.ValueKind = JsonValueKind.String -> + let s = n.GetString() + if String.IsNullOrWhiteSpace s then None else Some s + | _ -> None) + /// A user.message that is a skill's own context injection (written immediately after skill.invoked) /// rather than a genuine new request. Copilot CLI tags these with BOTH a `source` of `skill-` /// and a `` content preamble. Both markers are required (focused-review F4): the @@ -176,10 +211,7 @@ let private assistantMessageEvent (data: JsonElement) : SkillEvent = | true, reqs when reqs.ValueKind = JsonValueKind.Array -> // A single assistant.message can carry several tool calls; a `skill` call wins (the skill is // starting), otherwise an `ask_user` call marks the request the user will reply to. - let toolNamed (name: string) (req: JsonElement) = - match req.TryGetProperty("name") with - | true, n when n.ValueKind = JsonValueKind.String -> n.GetString() = name - | _ -> false + let toolNamed (name: string) (req: JsonElement) = toolRequestName req = Some name let skill = reqs.EnumerateArray() @@ -201,6 +233,35 @@ let private skillInvokedName (data: JsonElement) : string option = if String.IsNullOrWhiteSpace(name) then None else Some name | _ -> None +/// The background-agent ids launched by one assistant.message — its `task` tool calls with +/// `mode: "background"`. Empty when the message launches no background agents. +let private backgroundTaskLaunches (data: JsonElement) : string list = + match data.TryGetProperty("toolRequests") with + | true, reqs when reqs.ValueKind = JsonValueKind.Array -> + reqs.EnumerateArray() |> Seq.choose backgroundAgentName |> Seq.toList + | _ -> [] + +/// The agent id a `system.notification` reports as having gone idle (its background run finished), +/// read from the structured `data.kind` (`{ type = "agent_idle"; agentId = … }`) rather than the +/// prose content. None for any other notification kind. +let private agentIdleId (data: JsonElement) : string option = + match data.TryGetProperty("kind") with + | true, kind when kind.ValueKind = JsonValueKind.Object -> + let isIdle = + match kind.TryGetProperty("type") with + | true, t when t.ValueKind = JsonValueKind.String -> t.GetString() = "agent_idle" + | _ -> false + + if not isIdle then + None + else + match kind.TryGetProperty("agentId") with + | true, a when a.ValueKind = JsonValueKind.String -> + let s = a.GetString() + if String.IsNullOrWhiteSpace s then None else Some s + | _ -> None + | _ -> None + // --- Forward fold (oldest→newest) ------------------------------------------------------------- // events.jsonl is append-only, so the same skill/message/status determination the backward scans do // can be expressed as a FORWARD fold that carries state and is fed each new line as the file grows — @@ -229,7 +290,11 @@ type SessionScanCache = /// Was the most recent assistant.message an `ask_user` request? Distinguishes a user reply /// (skill resumes) from a genuine new request (skill's run is over). LastAssistantWasAskUser: bool - SubagentDepth: int } + SubagentDepth: int + /// Background agents (`task`, `mode: "background"`) launched but not yet reported idle. While + /// this is non-empty a turn that ends is still Working — the session is waiting on background + /// work, not the user — so it keeps its red dot instead of decaying to a blue Done dot. + RunningBackgroundAgents: Set } let internal emptySessionScan = { CurrentSkill = None @@ -237,7 +302,8 @@ let internal emptySessionScan = LastAssistantMessage = None RawStatus = Idle LastAssistantWasAskUser = false - SubagentDepth = 0 } + SubagentDepth = 0 + RunningBackgroundAgents = Set.empty } /// One event's bearing on the forward fold. Reuses the same classification as the backward /// `classifySkillEvent`, and adds the events that scan ignored but the fold needs: sub-agent brackets, @@ -257,8 +323,16 @@ type private ForwardEvent = /// marks the agent as Working like any user.message (matches the backward status scan). | InjectionUserMessage /// An assistant.message: whether it requested `ask_user`, the skill it started (a `skill` tool-call, - /// depth-0 only), and its text content when non-empty. - | AssistantMessage of isAskUser: bool * skill: string option * content: (string * DateTimeOffset) option + /// depth-0 only), its text content when non-empty, and any background agents it launched (`task` + /// calls with `mode: "background"`, depth-0 only). + | AssistantMessage of + isAskUser: bool * + skill: string option * + content: (string * DateTimeOffset) option * + backgroundLaunches: string list + /// A `system.notification` reporting a launched background agent has gone idle (its run finished), + /// carrying that agent's id — the signal that clears it from the outstanding set. + | AgentIdle of string | TurnStarted | TurnEnded @@ -293,13 +367,16 @@ let private classifyForwardEvent (line: string) : ForwardEvent option = | "assistant.turn_end" -> Some TurnEnded | "skill.invoked" -> data () |> Option.bind skillInvokedName |> Option.map SkillInvoked | "assistant.message" -> - let content = data () |> Option.bind (fun d -> readMessageContent d (readMessageTimestamp root)) - let kind = data () |> Option.map assistantMessageEvent |> Option.defaultValue AssistantWork + let d = data () + let content = d |> Option.bind (fun d -> readMessageContent d (readMessageTimestamp root)) + let kind = d |> Option.map assistantMessageEvent |> Option.defaultValue AssistantWork + let launches = d |> Option.map backgroundTaskLaunches |> Option.defaultValue [] match kind with - | SkillSignal name -> Some(AssistantMessage(false, Some name, content)) - | AssistantAskUser -> Some(AssistantMessage(true, None, content)) + | SkillSignal name -> Some(AssistantMessage(false, Some name, content, launches)) + | AssistantAskUser -> Some(AssistantMessage(true, None, content, launches)) | AssistantWork - | UserRequest -> Some(AssistantMessage(false, None, content)) + | UserRequest -> Some(AssistantMessage(false, None, content, launches)) + | "system.notification" -> data () |> Option.bind agentIdleId |> Option.map AgentIdle | "user.message" -> match data () with | Some d when isSkillContextMessage d -> Some InjectionUserMessage @@ -343,17 +420,33 @@ let private foldForwardEvent (state: SessionScanCache) (ev: ForwardEvent) : Sess LastUserMessage = (match content with Some c -> Some c | None -> state.LastUserMessage) LastAssistantWasAskUser = false RawStatus = Working } - | AssistantMessage(isAskUser, skill, content) -> + | AssistantMessage(isAskUser, skill, content, backgroundLaunches) -> let withSkill = match skill with | Some name when state.SubagentDepth = 0 -> { state with CurrentSkill = Some name } | _ -> state + // Background launches inside a sub-agent bracket belong to the sub-agent, not the top-level + // session, so only depth-0 launches count toward the outstanding set (mirrors skill gating). + let running = + if state.SubagentDepth = 0 then + backgroundLaunches |> List.fold (fun acc name -> Set.add name acc) withSkill.RunningBackgroundAgents + else + withSkill.RunningBackgroundAgents { withSkill with LastAssistantMessage = (match content with Some c -> Some c | None -> withSkill.LastAssistantMessage) LastAssistantWasAskUser = isAskUser + RunningBackgroundAgents = running RawStatus = (if isAskUser then WaitingForUser else Working) } + | AgentIdle agentId -> + // A launched background agent went idle (its run finished); drop it from the outstanding set. + // A no-op when the id was never tracked (e.g. launched before the scan window). + { state with RunningBackgroundAgents = Set.remove agentId state.RunningBackgroundAgents } | TurnStarted -> { state with RawStatus = Working } - | TurnEnded -> { state with RawStatus = Done } + | TurnEnded -> + // A finished turn is Done — UNLESS background agents are still running. The session launched + // them and ended its turn to wait for their idle notifications, so it is genuinely Working + // (red dot), not a blue Done dot, until they all report back (or the file goes stale). + { state with RawStatus = (if state.RunningBackgroundAgents.IsEmpty then Done else Working) } /// Fold event lines (oldest→newest) onto an existing state. Unknown/irrelevant lines are skipped. /// Pure and append-friendly: folding a later batch onto the state from an earlier batch yields the diff --git a/src/Tests/CopilotDetectorTests.fs b/src/Tests/CopilotDetectorTests.fs index 1401ed2c..3851fac3 100644 --- a/src/Tests/CopilotDetectorTests.fs +++ b/src/Tests/CopilotDetectorTests.fs @@ -296,6 +296,16 @@ let private makeSubagentStarted (toolCallId: string) (timestamp: string) = let private makeSubagentCompleted (toolCallId: string) (timestamp: string) = $"""{{"type":"subagent.completed","data":{{"toolCallId":"{toolCallId}"}},"timestamp":"{timestamp}"}}""" +let private makeBackgroundLaunch (name: string) (timestamp: string) = + // An assistant.message launching a background agent: a `task` tool call with mode "background". + // The main session keeps working alongside it and is woken by an agent_idle notification later. + $"""{{"type":"assistant.message","data":{{"content":"launching {name}","toolRequests":[{{"toolCallId":"tc-{name}","name":"task","arguments":{{"name":"{name}","agent_type":"review-runner","mode":"background","description":"Review rule {name}"}},"type":"function"}}]}},"timestamp":"{timestamp}"}}""" + +let private makeAgentIdle (agentId: string) (timestamp: string) = + // A system.notification reporting a background agent has finished its run and is now idle; the + // structured data.kind ({ type = "agent_idle"; agentId = … }) drives the outstanding-set removal. + $"""{{"type":"system.notification","data":{{"content":"Agent \"{agentId}\" is now idle","kind":{{"type":"agent_idle","agentId":"{agentId}","agentType":"review-runner"}}}},"timestamp":"{timestamp}"}}""" + /// The exact event streams of the CurrentSkillTests scenarios, paired with the skill each should /// report. Used to prove the forward fold is equivalent to the backward scanSkill (and returns the /// same known-good values) on every no-sub-agent scenario. @@ -543,6 +553,113 @@ type ForwardFoldTests() = Assert.That(scan.LastAssistantMessage, Is.EqualTo(None)) Assert.That(scan.RawStatus, Is.EqualTo(Idle)) Assert.That(scan.SubagentDepth, Is.EqualTo(0)) + Assert.That(scan.RunningBackgroundAgents, Is.EqualTo(Set.empty)) + + +/// A turn that ends while background agents (`task`, `mode: "background"`) are still running is the +/// review-skill scenario that regressed: the session launches N reviewers, ends its turn to wait for +/// them, and — with the file mtime frozen at that turn_end — must keep reading Working (red dot), not +/// decay to Done (blue). Only once every launched agent has reported idle does turn_end mean Done. +[] +[] +[] +type BackgroundAgentStatusTests() = + + [] + member _.``A turn ending with an outstanding background agent is Working``() = + let events = + [ makeUserEvent "/review the changes" "2026-03-01T10:00:00Z" + makeSkillInvoked "review" "2026-03-01T10:00:01Z" + makeBackgroundLaunch "incomplete-changes" "2026-03-01T10:00:02Z" + makeAssistantEvent "Waiting for `incomplete-changes`." "2026-03-01T10:00:03Z" + makeTurnEnd "2026-03-01T10:00:04Z" ] + + let scan = scanSessionEvents events + Assert.That(scan.RawStatus, Is.EqualTo(Working)) + Assert.That(scan.RunningBackgroundAgents, Is.EqualTo(Set.ofList [ "incomplete-changes" ])) + + [] + member _.``A turn ending after every launched agent reported idle is Done``() = + let events = + [ makeUserEvent "/review the changes" "2026-03-01T10:00:00Z" + makeSkillInvoked "review" "2026-03-01T10:00:01Z" + makeBackgroundLaunch "immutability" "2026-03-01T10:00:02Z" + makeBackgroundLaunch "no-loops" "2026-03-01T10:00:03Z" + makeAssistantEvent "Waiting for the reviewers." "2026-03-01T10:00:04Z" + makeTurnEnd "2026-03-01T10:00:05Z" + makeAgentIdle "immutability" "2026-03-01T10:00:30Z" + makeAgentIdle "no-loops" "2026-03-01T10:00:40Z" + makeAssistantEvent "All reviewers done; summarizing." "2026-03-01T10:00:41Z" + makeTurnEnd "2026-03-01T10:00:42Z" ] + + let scan = scanSessionEvents events + Assert.That(scan.RawStatus, Is.EqualTo(Done)) + Assert.That(scan.RunningBackgroundAgents, Is.EqualTo(Set.empty)) + + [] + member _.``A turn ending with some reviewers still running is Working``() = + // Two launched, only one idle: the session is genuinely waiting on the other, so Working. + let events = + [ makeSkillInvoked "review" "2026-03-01T10:00:01Z" + makeBackgroundLaunch "incomplete-changes" "2026-03-01T10:00:02Z" + makeBackgroundLaunch "no-null" "2026-03-01T10:00:03Z" + makeAssistantEvent "reviewers launched" "2026-03-01T10:00:04Z" + makeTurnEnd "2026-03-01T10:00:05Z" + makeAgentIdle "no-null" "2026-03-01T10:00:30Z" + makeAssistantEvent "Still waiting for `incomplete-changes`." "2026-03-01T10:00:31Z" + makeTurnEnd "2026-03-01T10:00:32Z" ] + + let scan = scanSessionEvents events + Assert.That(scan.RawStatus, Is.EqualTo(Working)) + Assert.That(scan.RunningBackgroundAgents, Is.EqualTo(Set.ofList [ "incomplete-changes" ])) + + [] + member _.``A waiting-on-background turn stays Working past the Done grace window``() = + // The regression itself: past the 15 s grace a normal finished turn reads Done, but a turn left + // waiting on a background agent must still read Working (red dot) through getStatusFromEventsFile. + let content = + [ makeSkillInvoked "review" "2026-03-01T10:00:01Z" + makeBackgroundLaunch "incomplete-changes" "2026-03-01T10:00:02Z" + makeAssistantEvent "Waiting for `incomplete-changes`." "2026-03-01T10:00:03Z" + makeTurnEnd "2026-03-01T10:00:04Z" ] + |> String.concat Environment.NewLine + + withTempEventsFile content (fun path -> + let pastGrace = DateTimeOffset.UtcNow.AddSeconds(20.0) + let status = getStatusFromEventsFile path pastGrace + Assert.That(status, Is.EqualTo(Working))) + + [] + member _.``A background launch inside a sub-agent bracket does not count as outstanding``() = + // A launch nested in a subagent.started/…completed bracket belongs to the sub-agent, not the + // top-level session, so it must not keep the top-level turn Working (mirrors skill depth gating). + let events = + [ makeSkillInvoked "review" "2026-03-01T10:00:01Z" + makeSubagentStarted "toolu_x" "2026-03-01T10:00:02Z" + makeBackgroundLaunch "nested-agent" "2026-03-01T10:00:03Z" + makeSubagentCompleted "toolu_x" "2026-03-01T10:00:04Z" + makeAssistantEvent "sub-agent done" "2026-03-01T10:00:05Z" + makeTurnEnd "2026-03-01T10:00:06Z" ] + + let scan = scanSessionEvents events + Assert.That(scan.RawStatus, Is.EqualTo(Done)) + Assert.That(scan.RunningBackgroundAgents, Is.EqualTo(Set.empty)) + + [] + member _.``Folding appended background events equals folding the whole stream``() = + let events = + [ makeSkillInvoked "review" "2026-03-01T10:00:01Z" + makeBackgroundLaunch "immutability" "2026-03-01T10:00:02Z" + makeBackgroundLaunch "no-loops" "2026-03-01T10:00:03Z" + makeTurnEnd "2026-03-01T10:00:04Z" + makeAgentIdle "immutability" "2026-03-01T10:00:30Z" + makeAgentIdle "no-loops" "2026-03-01T10:00:40Z" + makeTurnEnd "2026-03-01T10:00:41Z" ] + + let whole = scanSessionEvents events + let firstBatch, secondBatch = List.splitAt 3 events + let incremental = foldSessionEvents (scanSessionEvents firstBatch) secondBatch + Assert.That(incremental, Is.EqualTo(whole)) /// Join event lines the way a real events.jsonl is written: every line, including the last, is