Keep review agent Working (red dot) while background agents run

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-<name>"` and `<skill-context>` 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 `<skill-context>` injection is transparent (not a boundary).
 - **`<skill-context>` 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<eventsPath, SessionScanEntry>` (`{ 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.

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-<name>`
 /// and a `<skill-context …>` 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,15 +290,20 @@ 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<string> }
 
 let internal emptySessionScan =
     { CurrentSkill = None
       LastUserMessage = None
       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