Merge pull request #1711 from XiaomiMiMo/fix/actor-notification-wording

fix(inbox): sub-session notification wording reflects reported task status, not just actor lifecycle

Author: wqymi

packages/opencode/src/cli/cmd/tui/routes/session/index.tsx

@@ -1533,14 +1533,15 @@ function UserMessage(props: {
             if (s === "completed") return { icon: "✓", fg: theme.success, label: "completed" }
             if (s === "failed") return { icon: "✗", fg: theme.error, label: "failed" }
             if (s === "stalled") return { icon: "⏳", fg: theme.warning, label: "stalled" }
+            if (s === "ended") return { icon: "⊙", fg: theme.textMuted, label: "ended" }
             return { icon: "⊜", fg: theme.textMuted, label: "cancelled" }
           })
           return (
             <box id={props.message.id} marginTop={props.index === 0 ? 0 : 1} paddingLeft={2} flexDirection="row" gap={1}>
               <text fg={theme.textMuted}>
                 <span style={{ bg: theme.backgroundElement, fg: style().fg, bold: true }}>
                   {" "}
-                  {style().icon} actor {style().label}{" "}
+                  {style().icon} sub-session {style().label}{" "}
                 </span>
                 <span style={{ fg: theme.text }}> {note().description}</span>
                 <Show when={note().summary}>

packages/opencode/src/inbox/render.ts

@@ -29,11 +29,30 @@ export function renderActorNotification(event: {
   // For a stalled notification: how long (ms) since the child's last turn advanced.
   stalledForMs?: number
 }): string {
-  const header = `Background actor "${event.description}" (actor_id: ${event.actorID})`
+  const header = `Background sub-session "${event.description}" (actor_id: ${event.actorID})`
   if (event.status === "completed") {
-    const statusLine = `Status: ${event.reportedStatus ?? "unknown"}`
+    // event.status is the sub-session *process lifecycle* — it ended cleanly.
+    // event.reportedStatus is the *task* outcome the sub-session self-reported
+    // via a `**Status**: ...` header. These are independent: a process can exit
+    // cleanly while the task failed/blocked. Word the top line by the task
+    // outcome so we never imply a success the sub-session didn't claim.
+    const reported = event.reportedStatus?.toLowerCase()
     const summaryLine = event.reportedSummary ? `\nSummary: ${event.reportedSummary}` : ""
-    return `<actor-notification>\n${header} completed.\n${statusLine}${summaryLine}\nResult: ${event.result ?? "(no output)"}\n</actor-notification>`
+    const resultLine = `\nResult: ${event.result ?? "(no output)"}`
+    // success/partial (or absent → treat as a plain completion) keep the
+    // affirmative "completed" verb.
+    if (!reported || reported === "success" || reported === "partial") {
+      const statusLine = reported ? `\nStatus: ${reported}` : ""
+      return `<actor-notification>\n${header} completed.${statusLine}${summaryLine}${resultLine}\n</actor-notification>`
+    }
+    // failed/blocked → the sub-session ran to the end but the task did not
+    // succeed. State the outcome; never say "completed".
+    if (reported === "failed" || reported === "blocked") {
+      return `<actor-notification>\n${header} finished (status: ${reported}).${summaryLine}${resultLine}\n</actor-notification>`
+    }
+    // Any other reported value = unknown/unrecognized → neutral verb, and omit
+    // the misleading "Status: unknown" line entirely.
+    return `<actor-notification>\n${header} ended (status not reported).${summaryLine}${resultLine}\n</actor-notification>`
   }
   if (event.status === "failed") {
     return `<actor-notification>\n${header} failed.\nError: ${event.error ?? "unknown"}\n</actor-notification>`
@@ -49,8 +68,10 @@ export function renderActorNotification(event: {
 export type ParsedActorNotification = {
   // "stalled" is reserved for a future watchdog-emitted notification;
   // renderActorNotification never produces it today (only completed/failed/
-  // cancelled). The parse + card styling exist ahead of that producer.
-  status: "completed" | "failed" | "cancelled" | "stalled"
+  // cancelled lifecycle). The parse + card styling exist ahead of that producer.
+  // "ended" is the completed-lifecycle case where the sub-session's task
+  // outcome was not reported — neutral, neither success nor failure.
+  status: "completed" | "failed" | "cancelled" | "stalled" | "ended"
   description: string
   summary?: string
 }
@@ -61,12 +82,28 @@ export type ParsedActorNotification = {
 // Returns null for any text that isn't an actor notification.
 export function parseActorNotification(text: string): ParsedActorNotification | null {
   if (!text.trimStart().startsWith("<actor-notification>")) return null
-  const header = text.match(/Background actor "(.*?)" \(actor_id: [^)]*\)\s+(completed|failed|was cancelled|stalled)\b/)
+  // The verb reflects the *task* outcome, not just the process lifecycle:
+  //   completed                         → task succeeded / plain completion
+  //   finished (status: failed|blocked) → process ended cleanly, task not ok
+  //   ended (status not reported)       → process ended cleanly, outcome unknown
+  //   failed                            → the process itself failed
+  //   was cancelled / stalled           → cancelled / watchdog
+  const header = text.match(
+    /Background (?:sub-session|actor) "(.*?)" \(actor_id: [^)]*\)\s+(completed|finished|ended|failed|was cancelled|stalled)\b/,
+  )
   if (!header) return null
   const description = header[1]
   const verb = header[2]
   const status: ParsedActorNotification["status"] =
-    verb === "completed" ? "completed" : verb === "failed" ? "failed" : verb === "stalled" ? "stalled" : "cancelled"
+    verb === "completed"
+      ? "completed"
+      : verb === "finished" || verb === "failed"
+        ? "failed"
+        : verb === "ended"
+          ? "ended"
+          : verb === "stalled"
+            ? "stalled"
+            : "cancelled"
   // Prefer the most human-relevant one-liner: Summary > Result > Error.
   // renderActorNotification always emits the Summary line before the Result
   // line, so restrict the Summary match to the region before the first

packages/opencode/src/tool/actor.ts

@@ -767,7 +767,7 @@ export const ActorTool = Tool.define(
             metadata: { sessionId: spawnResult.sessionID, actorId: spawnResult.actorID, model },
             output:
               (taskNotice ? taskNotice + "\n" : "") +
-              `Background actor started. actor_id: ${spawnResult.actorID}\nThe result will be delivered as a notification when complete.`,
+              `Background sub-session started. actor_id: ${spawnResult.actorID}\nThe result will be delivered as a notification when complete.`,
           }
         }
 

packages/opencode/test/inbox/parse-actor-notification.test.ts

@@ -62,6 +62,92 @@ describe("parseActorNotification", () => {
     })
   })
 
+  test("completed lifecycle but reportedStatus=failed reads as failed, never completed", () => {
+    const text = renderActorNotification({
+      actorID: "general-4",
+      description: "Fix the flaky test",
+      status: "completed",
+      reportedStatus: "failed",
+      reportedSummary: "could not reproduce",
+      result: "full body",
+    })
+    // Top line states the outcome and must never imply success.
+    expect(text).toContain("finished (status: failed)")
+    expect(text).not.toContain("completed")
+    expect(parseActorNotification(text)).toEqual({
+      status: "failed",
+      description: "Fix the flaky test",
+      summary: "could not reproduce",
+    })
+  })
+
+  test("completed lifecycle but reportedStatus=blocked reads as failed", () => {
+    const text = renderActorNotification({
+      actorID: "general-5",
+      description: "Wire up the API",
+      status: "completed",
+      reportedStatus: "blocked",
+      result: "waiting on credentials",
+    })
+    expect(text).toContain("finished (status: blocked)")
+    expect(parseActorNotification(text)).toEqual({
+      status: "failed",
+      description: "Wire up the API",
+      summary: "waiting on credentials",
+    })
+  })
+
+  test("completed lifecycle with reportedStatus=unknown reads as neutral ended", () => {
+    const text = renderActorNotification({
+      actorID: "general-6",
+      description: "Do the thing",
+      status: "completed",
+      reportedStatus: "unknown", // sub-session ran but did not report a task outcome
+      result: "some output",
+    })
+    // Must NOT imply success, and must NOT emit the misleading "Status: unknown".
+    expect(text).toContain("ended (status not reported)")
+    expect(text).not.toContain("Status: unknown")
+    expect(parseActorNotification(text)).toEqual({
+      status: "ended",
+      description: "Do the thing",
+      summary: "some output",
+    })
+  })
+
+  test("completed lifecycle with no reportedStatus at all reads as a plain completion", () => {
+    const text = renderActorNotification({
+      actorID: "general-8",
+      description: "Plain job",
+      status: "completed",
+      result: "done",
+    })
+    expect(text).toContain("completed")
+    expect(text).not.toContain("Status:")
+    expect(parseActorNotification(text)).toEqual({
+      status: "completed",
+      description: "Plain job",
+      summary: "done",
+    })
+  })
+
+  test("completed lifecycle with reportedStatus=partial still reads as completed", () => {
+    const text = renderActorNotification({
+      actorID: "general-7",
+      description: "Partial job",
+      status: "completed",
+      reportedStatus: "partial",
+      reportedSummary: "did half",
+      result: "body",
+    })
+    expect(text).toContain("completed")
+    expect(parseActorNotification(text)).toEqual({
+      status: "completed",
+      description: "Partial job",
+      summary: "did half",
+    })
+  })
+
   test("parses a cancelled notification (no summary)", () => {
     const text = renderActorNotification({
       actorID: "peer-3",
@@ -76,7 +162,7 @@ describe("parseActorNotification", () => {
 
   test("parses a stalled notification (watchdog variant)", () => {
     const text =
-      '<actor-notification>\nBackground actor "Wedged agent" (actor_id: general-7) stalled.\nSummary: no output for 10m\n</actor-notification>'
+      '<actor-notification>\nBackground sub-session "Wedged agent" (actor_id: general-7) stalled.\nSummary: no output for 10m\n</actor-notification>'
     expect(parseActorNotification(text)).toEqual({
       status: "stalled",
       description: "Wedged agent",
@@ -93,4 +179,14 @@ describe("parseActorNotification", () => {
   test("returns null when the wrapper is present but the header is malformed", () => {
     expect(parseActorNotification("<actor-notification>\ngarbage\n</actor-notification>")).toBeNull()
   })
+
+  test("backward compat: parses legacy 'Background actor' format as a card", () => {
+    const text =
+      '<actor-notification>\nBackground actor "Legacy task" (actor_id: explore-1) completed.\nResult: done\n</actor-notification>'
+    expect(parseActorNotification(text)).toEqual({
+      status: "completed",
+      description: "Legacy task",
+      summary: "done",
+    })
+  })
 })

packages/opencode/test/tool/actor.test.ts

@@ -746,7 +746,7 @@ describe("Actor tool task_id degradation", () => {
 
         expect(result.output).toContain("not-a-task")
         expect(result.output.toLowerCase()).toContain("ad-hoc")
-        expect(result.output).toContain("Background actor started")
+        expect(result.output).toContain("Background sub-session started")
       }),
     ),
   )