diff --git a/CHANGELOG.md b/CHANGELOG.md index e55382ecc..3bb1b36ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ - Browser: add tests for snapshot labels/efficient query params and labeled image responses. - Doctor: avoid re-adding WhatsApp config when only legacy ack reactions are set. (#927, fixes #900) — thanks @grp06. - Agents: scrub tuple `items` schemas for Gemini tool calls. (#926, fixes #746) — thanks @grp06. +- Agents: stabilize sub-agent announce status from runtime outcomes and normalize Result/Notes. (#835) — thanks @roshanasingh4. - Embedded runner: suppress raw API error payloads from replies. (#924) — thanks @grp06. - Auth: normalize Claude Code CLI profile mode to oauth and auto-migrate config. (#855) — thanks @sebslight. - Daemon: clear persisted launchd disabled state before bootstrap (fixes `daemon install` after uninstall). (#849) — thanks @ndraiman. diff --git a/docs/concepts/session-tool.md b/docs/concepts/session-tool.md index 9c93b0619..f7bd4511c 100644 --- a/docs/concepts/session-tool.md +++ b/docs/concepts/session-tool.md @@ -145,6 +145,7 @@ Behavior: - Always non-blocking: returns `{ status: "accepted", runId, childSessionKey }` immediately. - After completion, Clawdbot runs a sub-agent **announce step** and posts the result to the requester chat channel. - Reply exactly `ANNOUNCE_SKIP` during the announce step to stay silent. +- Announce replies are normalized to `Status`/`Result`/`Notes`; `Status` comes from runtime outcome (not model text). - Sub-agent sessions are auto-archived after `agents.defaults.subagents.archiveAfterMinutes` (default: 60). - Announce replies include a stats line (runtime, tokens, sessionKey/sessionId, transcript path, and optional cost). diff --git a/docs/tools/subagents.md b/docs/tools/subagents.md index 4647bcf37..d80bbed7a 100644 --- a/docs/tools/subagents.md +++ b/docs/tools/subagents.md @@ -49,8 +49,13 @@ Sub-agents report back via an announce step: - The announce step runs inside the sub-agent session (not the requester session). - If the sub-agent replies exactly `ANNOUNCE_SKIP`, nothing is posted. - Otherwise the announce reply is posted to the requester chat channel via the gateway `send` method. +- Announce messages are normalized to a stable template: + - `Status:` derived from the run outcome (`success`, `error`, `timeout`, or `unknown`). + - `Result:` the summary content from the announce step (or `(not available)` if missing). + - `Notes:` error details and other useful context. +- `Status` is not inferred from model output; it comes from runtime outcome signals. -Announce payloads include a stats line at the end: +Announce payloads include a stats line at the end (even when wrapped): - Runtime (e.g., `runtime 5m12s`) - Token usage (input/output/total) - Estimated cost when model pricing is configured (`models.providers.*.models[].cost`) diff --git a/src/agents/subagent-announce.format.test.ts b/src/agents/subagent-announce.format.test.ts new file mode 100644 index 000000000..b8adc0ca8 --- /dev/null +++ b/src/agents/subagent-announce.format.test.ts @@ -0,0 +1,103 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const sendSpy = vi.fn(async () => ({})); + +vi.mock("../gateway/call.js", () => ({ + callGateway: vi.fn(async (req: unknown) => { + const typed = req as { method?: string; params?: { message?: string } }; + if (typed.method === "send") { + return await sendSpy(typed); + } + if (typed.method === "agent.wait") { + return { status: "error", startedAt: 10, endedAt: 20, error: "boom" }; + } + if (typed.method === "sessions.patch") return {}; + if (typed.method === "sessions.delete") return {}; + return {}; + }), +})); + +vi.mock("./tools/agent-step.js", () => ({ + runAgentStep: vi.fn(async () => "did some stuff"), + readLatestAssistantReply: vi.fn(async () => "raw subagent reply"), +})); + +vi.mock("./tools/sessions-announce-target.js", () => ({ + resolveAnnounceTarget: vi.fn(async () => ({ + provider: "telegram", + to: "+15550001111", + accountId: "default", + })), +})); + +vi.mock("./tools/sessions-send-helpers.js", () => ({ + isAnnounceSkip: () => false, +})); + +vi.mock("../config/sessions.js", () => ({ + loadSessionStore: vi.fn(async () => ({ entries: {} })), + resolveAgentIdFromSessionKey: () => "main", + resolveStorePath: () => "/tmp/sessions.json", +})); + +vi.mock("../config/config.js", () => ({ + loadConfig: () => ({ + session: { mainKey: "agent:main:main" }, + }), +})); + +describe("subagent announce formatting", () => { + beforeEach(() => { + sendSpy.mockClear(); + }); + + it("wraps unstructured announce into Status/Result/Notes", async () => { + const { runSubagentAnnounceFlow } = await import("./subagent-announce.js"); + await runSubagentAnnounceFlow({ + childSessionKey: "agent:main:subagent:test", + childRunId: "run-123", + requesterSessionKey: "agent:main:main", + requesterDisplayKey: "main", + task: "do thing", + timeoutMs: 1000, + cleanup: "keep", + waitForCompletion: true, + startedAt: 10, + endedAt: 20, + }); + + expect(sendSpy).toHaveBeenCalled(); + const msg = sendSpy.mock.calls[0]?.[0]?.params?.message as string; + expect(msg).toContain("Status:"); + expect(msg).toContain("Status: error"); + expect(msg).toContain("Result:"); + expect(msg).toContain("Notes:"); + expect(msg).toContain("boom"); + }); + + it("keeps runtime status even when announce reply is structured", async () => { + const agentStep = await import("./tools/agent-step.js"); + vi.mocked(agentStep.runAgentStep).mockResolvedValueOnce( + "- **Status:** success\n\n- **Result:** did some stuff\n\n- **Notes:** all good", + ); + + const { runSubagentAnnounceFlow } = await import("./subagent-announce.js"); + await runSubagentAnnounceFlow({ + childSessionKey: "agent:main:subagent:test", + childRunId: "run-456", + requesterSessionKey: "agent:main:main", + requesterDisplayKey: "main", + task: "do thing", + timeoutMs: 1000, + cleanup: "keep", + waitForCompletion: true, + startedAt: 10, + endedAt: 20, + }); + + const msg = sendSpy.mock.calls[0]?.[0]?.params?.message as string; + expect(msg).toContain("Status: error"); + expect(msg).toContain("Result:"); + expect(msg).toContain("Notes:"); + }); +}); diff --git a/src/agents/subagent-announce.ts b/src/agents/subagent-announce.ts index 93a011b7c..80197ecc8 100644 --- a/src/agents/subagent-announce.ts +++ b/src/agents/subagent-announce.ts @@ -183,6 +183,85 @@ export function buildSubagentSystemPrompt(params: { return lines.join("\n"); } +export type SubagentRunOutcome = { + status: "ok" | "error" | "timeout" | "unknown"; + error?: string; +}; + +const ANNOUNCE_SECTION_RE = /^\s*[-*]?\s*(?:\*\*)?(status|result|notes)(?:\*\*)?\s*:\s*(.*)$/i; + +function parseAnnounceSections(announce: string) { + const sections = { + status: [] as string[], + result: [] as string[], + notes: [] as string[], + }; + let current: keyof typeof sections | null = null; + let sawSection = false; + + for (const line of announce.split(/\r?\n/)) { + const match = line.match(ANNOUNCE_SECTION_RE); + if (match) { + const key = match[1]?.toLowerCase() as keyof typeof sections; + current = key; + sawSection = true; + const rest = match[2]?.trim(); + if (rest) sections[key].push(rest); + continue; + } + if (current) sections[current].push(line); + } + + const normalize = (lines: string[]) => { + const joined = lines.join("\n").trim(); + return joined.length > 0 ? joined : undefined; + }; + + return { + sawSection, + status: normalize(sections.status), + result: normalize(sections.result), + notes: normalize(sections.notes), + }; +} + +function normalizeAnnounceBody(params: { + outcome: SubagentRunOutcome; + announceReply: string; + statsLine?: string; +}) { + const announce = params.announceReply.trim(); + const statsLine = params.statsLine?.trim(); + + const statusLabel = + params.outcome.status === "ok" + ? "success" + : params.outcome.status === "timeout" + ? "timeout" + : params.outcome.status === "unknown" + ? "unknown" + : "error"; + + const parsed = parseAnnounceSections(announce); + const resultText = parsed.result ?? (announce || "(not available)"); + const notesParts: string[] = []; + if (parsed.notes) notesParts.push(parsed.notes); + if (params.outcome.error) notesParts.push(`- Error: ${params.outcome.error}`); + const notesBlock = notesParts.length ? notesParts.join("\n") : "- (none)"; + + const message = [ + `Status: ${statusLabel}`, + "", + "Result:", + resultText, + "", + "Notes:", + notesBlock, + ].join("\n"); + + return statsLine ? `${message}\n\n${statsLine}` : message; +} + function buildSubagentAnnouncePrompt(params: { requesterSessionKey?: string; requesterChannel?: string; @@ -202,6 +281,10 @@ function buildSubagentAnnouncePrompt(params: { "", "**You MUST announce your result.** The requester is waiting for your response.", "Provide a brief, useful summary of what you accomplished.", + "Reply with Result and Notes only (no Status line; status is added by the system).", + "Format:", + "Result: ", + "Notes: ", 'Only reply "ANNOUNCE_SKIP" if the task completely failed with no useful output.', "Your reply will be posted to the requester chat.", ].filter(Boolean); @@ -222,10 +305,12 @@ export async function runSubagentAnnounceFlow(params: { startedAt?: number; endedAt?: number; label?: string; + outcome?: SubagentRunOutcome; }): Promise { let didAnnounce = false; try { let reply = params.roundOneReply; + let outcome: SubagentRunOutcome | undefined = params.outcome; if (!reply && params.waitForCompletion !== false) { const waitMs = Math.min(params.timeoutMs, 60_000); const wait = (await callGateway({ @@ -235,8 +320,30 @@ export async function runSubagentAnnounceFlow(params: { timeoutMs: waitMs, }, timeoutMs: waitMs + 2000, - })) as { status?: string }; - if (wait?.status !== "ok") return false; + })) as { + status?: string; + error?: string; + startedAt?: number; + endedAt?: number; + }; + if (wait?.status === "timeout") { + outcome = { status: "timeout" }; + } else if (wait?.status === "error") { + outcome = { status: "error", error: wait.error }; + } else if (wait?.status === "ok") { + outcome = { status: "ok" }; + } + if (typeof wait?.startedAt === "number" && !params.startedAt) { + params.startedAt = wait.startedAt; + } + if (typeof wait?.endedAt === "number" && !params.endedAt) { + params.endedAt = wait.endedAt; + } + if (wait?.status === "timeout") { + // No lifecycle end seen before timeout. Still attempt an announce so + // requesters are not left hanging. + if (!outcome) outcome = { status: "timeout" }; + } reply = await readLatestAssistantReply({ sessionKey: params.childSessionKey, }); @@ -248,6 +355,8 @@ export async function runSubagentAnnounceFlow(params: { }); } + if (!outcome) outcome = { status: "unknown" }; + const announceTarget = await resolveAnnounceTarget({ sessionKey: params.requesterSessionKey, displayKey: params.requesterDisplayKey, @@ -278,7 +387,11 @@ export async function runSubagentAnnounceFlow(params: { startedAt: params.startedAt, endedAt: params.endedAt, }); - const message = statsLine ? `${announceReply.trim()}\n\n${statsLine}` : announceReply.trim(); + const message = normalizeAnnounceBody({ + outcome, + announceReply, + statsLine, + }); await callGateway({ method: "send", diff --git a/src/agents/subagent-registry.ts b/src/agents/subagent-registry.ts index 5604ce590..b020bd5a4 100644 --- a/src/agents/subagent-registry.ts +++ b/src/agents/subagent-registry.ts @@ -1,7 +1,7 @@ import { loadConfig } from "../config/config.js"; import { callGateway } from "../gateway/call.js"; import { onAgentEvent } from "../infra/agent-events.js"; -import { runSubagentAnnounceFlow } from "./subagent-announce.js"; +import { runSubagentAnnounceFlow, type SubagentRunOutcome } from "./subagent-announce.js"; import { loadSubagentRegistryFromDisk, saveSubagentRegistryToDisk, @@ -20,6 +20,7 @@ export type SubagentRunRecord = { createdAt: number; startedAt?: number; endedAt?: number; + outcome?: SubagentRunOutcome; archiveAtMs?: number; announceCompletedAt?: number; announceHandled: boolean; @@ -62,6 +63,7 @@ function resumeSubagentRun(runId: string) { startedAt: entry.startedAt, endedAt: entry.endedAt, label: entry.label, + outcome: entry.outcome, }); void announce.then((didAnnounce) => { finalizeSubagentAnnounce(runId, entry.cleanup, didAnnounce); @@ -176,6 +178,12 @@ function ensureListener() { const endedAt = typeof evt.data?.endedAt === "number" ? (evt.data.endedAt as number) : Date.now(); entry.endedAt = endedAt; + if (phase === "error") { + const error = typeof evt.data?.error === "string" ? (evt.data.error as string) : undefined; + entry.outcome = { status: "error", error }; + } else { + entry.outcome = { status: "ok" }; + } persistSubagentRuns(); if (!beginSubagentAnnounce(evt.runId)) { @@ -194,6 +202,7 @@ function ensureListener() { startedAt: entry.startedAt, endedAt: entry.endedAt, label: entry.label, + outcome: entry.outcome, }); void announce.then((didAnnounce) => { finalizeSubagentAnnounce(evt.runId, entry.cleanup, didAnnounce); @@ -272,7 +281,7 @@ async function waitForSubagentCompletion(runId: string, waitTimeoutMs: number) { timeoutMs, }, timeoutMs: timeoutMs + 10_000, - })) as { status?: string; startedAt?: number; endedAt?: number }; + })) as { status?: string; startedAt?: number; endedAt?: number; error?: string }; if (wait?.status !== "ok" && wait?.status !== "error") return; const entry = subagentRuns.get(runId); if (!entry) return; @@ -289,6 +298,11 @@ async function waitForSubagentCompletion(runId: string, waitTimeoutMs: number) { entry.endedAt = Date.now(); mutated = true; } + entry.outcome = + wait.status === "error" + ? { status: "error", error: wait.error } + : { status: "ok" }; + mutated = true; if (mutated) persistSubagentRuns(); if (!beginSubagentAnnounce(runId)) return; const announce = runSubagentAnnounceFlow({ @@ -304,6 +318,7 @@ async function waitForSubagentCompletion(runId: string, waitTimeoutMs: number) { startedAt: entry.startedAt, endedAt: entry.endedAt, label: entry.label, + outcome: entry.outcome, }); void announce.then((didAnnounce) => { finalizeSubagentAnnounce(runId, entry.cleanup, didAnnounce);