diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 42dd2b72cc..f216de0e9e 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,7 @@ ## [Unreleased] - Fixed interactive steering stalling during post-prompt unwind when the terminal transcript message is a bash or Python execution; the auto-continue gate now treats all three terminal roles consistently and regression coverage pins delivery for assistant, bash, and Python tails (#4739). +- Restored live subagent progress in the `subagent await` panel through a bounded public DTO. #4748 dropped the `progress` field believing it exposed model-generated deltas and tool output to the parent model; the renderer needs live status, tool names, output counts, fast mode, and retry state, but raw progress must not enter tool-result, ACP, or telemetry envelopes. The await signature now tracks only those approved fields and ignores countdown-only churn, so the panel updates without exposing model deltas, tool arguments, arbitrary output, or nested task payloads. - Custom-provider `apiKeyEnv` credentials sourced from the trusted agent `.env` now follow rotations on the next request instead of remaining pinned to the process-start value. Long-running sessions previously kept sending a revoked OAuth access token after an external account synchronizer atomically replaced the agent `.env`; the registry now re-reads only that trusted regular file and updates or removes the config credential override, failing closed for a symlink or read error after agent ownership is established. Presence in the agent file makes it authoritative over an older value inherited from the launching shell, while names absent from the agent file keep their existing environment precedence. Explicit `apiKey` values remain authoritative when both forms are configured, and caller-project `.env` files remain excluded from credential resolution. - Coordinator event journal rows can now be pushed to one opt-in webhook (#4706). External orchestrators that cannot stay attached to `gjc_coordinator_watch_events` long-poll (a 300s `await_turn` timeout is not session death) had no push of **existing** journal rows; they can now set `GJC_COORDINATOR_MCP_EVENT_WEBHOOK_URL` to receive each row as an authenticated POST whose body is the exact native `watch_events` record — same `seq`, same stable `id`, at-least-once so sinks dedupe on `id`. The feature is env-only and default-off (no MCP tool can set or read it), destinations are allowlisted (`https:` anywhere, `http:` loopback only, no redirects), the bearer token comes from a secret file path rather than env, an optional session-id scope restricts delivery to authorized sessions, and delivery runs through a durable per-row outbox off the journal append path with bounded attempts, exponential backoff, and a bounded request timeout — a dead sink never delays or rewrites terminal turn/session persistence. The five `GJC_COORDINATOR_MCP_EVENT_WEBHOOK_*` variables resolve through the trusted credential environment (`$credentialEnv`, the same provenance as the crash-relay DSN) rather than raw `process.env`, so a checkout's `.env` cannot select the egress destination or the token file. `watch_events` long-poll is unchanged and remains the source of truth; `gjc coordinator doctor` reports the resolved webhook state. - Runtime skill discovery now scans `skills.customDirectories`. Session startup already loaded those directories through `loadSkills`, but `discoverRuntimeSkills` and `findRuntimeSkillByName` searched only the canonical project and user roots, so a configured custom skill was invocable by exact name yet absent from every `skill_discovery` search -- usable only by someone who already knew it existed. Both discovery entry points now scan the configured directories at user level (so project-scoped queries exclude them), deduplicated and tilde-expanded the same way `loadSkills` does. Naming a directory is explicit consent, so custom directories are not gated on `skills.trustUserSkills` -- matching the startup rule -- while the `skills.enabled` master switch still suppresses them. diff --git a/packages/coding-agent/src/tools/subagent-render.ts b/packages/coding-agent/src/tools/subagent-render.ts index c6d6cd6893..f0e2f5bd23 100644 --- a/packages/coding-agent/src/tools/subagent-render.ts +++ b/packages/coding-agent/src/tools/subagent-render.ts @@ -11,13 +11,7 @@ import type { Component } from "@gajae-code/tui"; import { Text } from "@gajae-code/tui"; import type { RenderResultOptions } from "../extensibility/custom-tools/types"; import type { Theme } from "../modes/theme/theme"; -import { - collectProviderDegradationGroups, - hasActiveProviderRetryInProgress, - providerProgressAgeLabel, - providerRetryPhaseLabel, -} from "../task/provider-retry-status"; -import { renderSubagentLiveProgress } from "../task/render"; +import { providerRetryPhaseLabel } from "../task/provider-retry-status"; import { Ellipsis, Hasher, renderStatusLine } from "../tui"; import { formatDuration, @@ -27,7 +21,12 @@ import { type ToolUIStatus, truncateToWidth, } from "./render-utils"; -import { type SubagentSnapshot, type SubagentToolDetails, subagentAwaitRenderedStateSignature } from "./subagent"; +import { + type SubagentLiveProgress, + type SubagentSnapshot, + type SubagentToolDetails, + subagentAwaitRenderedStateSignature, +} from "./subagent"; export { subagentAwaitRenderedStateSignature } from "./subagent"; @@ -89,6 +88,30 @@ function snapshotHasActiveRetry(snapshot: SubagentSnapshot): boolean { return hasActiveProviderRetryInProgress(snapshot.progress); } +function hasActiveProviderRetryInProgress(progress: SubagentLiveProgress): boolean { + return progress.status === "running" && progress.retryState !== undefined; +} + +function providerProgressAgeLabel(progress: SubagentLiveProgress["retryState"], nowMs: number): string { + if (progress?.lastProviderProgressAtMs === undefined) return "no provider events yet"; + const ageSeconds = Math.max(0, Math.floor((nowMs - progress.lastProviderProgressAtMs) / 1000)); + return `last provider progress ${ageSeconds}s ago`; +} + +function collectProviderDegradationGroups( + progress: readonly SubagentLiveProgress[], +): Array<{ provider: string; count: number }> { + const counts = new Map(); + for (const item of progress) { + if (item.status !== "running" || !item.retryState) continue; + const provider = item.retryState.provider ?? "provider"; + counts.set(provider, (counts.get(provider) ?? 0) + 1); + } + return Array.from(counts, ([provider, count]) => ({ provider, count })) + .filter(group => group.count > 1) + .sort((a, b) => b.count - a.count || a.provider.localeCompare(b.provider)); +} + function boundSubagentBodyLines(lines: string[], width: number): string[] { return lines.map(line => (line.length > 0 ? truncateToWidth(replaceTabs(line), width, Ellipsis.Omit) : "")); } @@ -166,6 +189,72 @@ function renderSubagentStatusLine(snapshot: SubagentSnapshot, theme: Theme, spin return `${icon} ${id} ${status} ${duration}`; } +function renderSubagentLiveProgress( + progress: SubagentLiveProgress, + expanded: boolean, + theme: Theme, + spinnerFrame?: number, + staticTime = false, +): string[] { + const lines: string[] = []; + const prefix = theme.fg("dim", theme.tree.last); + const iconColor = progress.status === "failed" || progress.status === "aborted" ? "error" : "accent"; + const icon = formatStatusIcon( + progress.status === "completed" ? "success" : progress.status === "failed" ? "error" : "info", + theme, + progress.status === "running" ? spinnerFrame : undefined, + ); + let statusLine = `${prefix} ${theme.fg(iconColor, icon)} ${theme.fg("accent", progress.id)}`; + if (progress.fastMode && theme.icon.fast) statusLine += ` ${theme.icon.fast}`; + if (progress.retryState && progress.status === "running") { + statusLine += ` ${theme.fg("warning", "provider degraded")}`; + } + lines.push(statusLine); + + const continuePrefix = " "; + if (progress.status === "running") { + const tool = progress.currentTool ?? progress.recentTool; + if (tool) lines.push(`${continuePrefix}${theme.tree.hook} ${theme.fg("muted", tool)}`); + } + if (progress.recentOutputSummary) { + const count = progress.recentOutputSummary.lineCount; + lines.push( + `${continuePrefix}${theme.tree.hook} ${theme.fg("dim", `recent output available (${count} ${count === 1 ? "line" : "lines"})`)}`, + ); + } + if (progress.retryState && progress.status === "running") { + const retry = progress.retryState; + const attemptLabel = retry.unbounded + ? `attempt ${retry.attempt}, unbounded` + : `attempt ${retry.attempt} of ${retry.maxAttempts}, bounded`; + const progressAge = staticTime ? "" : ` · ${providerProgressAgeLabel(retry, Date.now())}`; + let waitLabel = ""; + if (!staticTime) { + const remainingMs = Math.max(0, retry.startedAtMs + retry.delayMs - Date.now()); + waitLabel = remainingMs > 0 ? ` in ${formatDuration(remainingMs)}` : " now"; + } + lines.push( + `${continuePrefix}${theme.tree.hook} ${theme.fg("warning", `${providerRetryPhaseLabel(retry.kind)} · retrying ${attemptLabel}${waitLabel}${progressAge}`)}`, + ); + } + if (progress.retryFailure && progress.status !== "running") { + const attempts = progress.retryFailure.attempt; + lines.push( + `${continuePrefix}${theme.tree.hook} ${theme.fg("error", `auto-retry gave up after ${attempts} attempt${attempts === 1 ? "" : "s"}`)}`, + ); + } + if ( + expanded && + progress.status === "running" && + !progress.currentTool && + !progress.recentTool && + !progress.recentOutputSummary + ) { + lines.push(`${continuePrefix}${theme.fg("dim", "running, no approved activity summary yet")}`); + } + return lines; +} + // Heavy per-subagent body. The cache path uses staticTime=true; the bounded dynamic // path opts into wall-clock displays when an active retry exists in the nested tree. function renderSubagentSnapshotBody( @@ -220,7 +309,7 @@ function renderSubagentSnapshotBody( lines.push(` ${pl}`); } } else if (snapshot.liveProgressAvailable && (snapshot.status === "running" || snapshot.status === "queued")) { - lines.push(` ${theme.fg("dim", "running, no activity yet")}`); + lines.push(` ${theme.fg("dim", "running, no approved activity summary yet")}`); } const preview = snapshot.errorText?.trim() || snapshot.resultText?.trim(); diff --git a/packages/coding-agent/src/tools/subagent.ts b/packages/coding-agent/src/tools/subagent.ts index 8421d2a912..cee5186fd6 100644 --- a/packages/coding-agent/src/tools/subagent.ts +++ b/packages/coding-agent/src/tools/subagent.ts @@ -4,7 +4,7 @@ import { formatDuration, logger, prompt } from "@gajae-code/utils"; import * as z from "zod/v4"; import { type AsyncJob, AsyncJobManager, jobElapsedMs, type SubagentRecord } from "../async"; import subagentDescription from "../prompts/tools/subagent.md" with { type: "text" }; -import type { AgentProgress, AgentSource, TaskToolDetails } from "../task/types"; +import type { AgentProgress, AgentSource } from "../task/types"; import { Ellipsis, truncateToWidth } from "../tui"; import type { ToolSession } from "./index"; import { replaceTabs } from "./render-utils"; @@ -92,8 +92,8 @@ export interface SubagentSnapshot { steerMessage?: string; steerState?: "queued" | "resume_queued" | "resume_started"; steerPauseRequested?: boolean; - /** Live streaming progress for the awaited subagent (await panel only; UI detail). */ - progress?: AgentProgress; + /** Bounded live progress approved for the await panel and public tool details. */ + progress?: SubagentLiveProgress; /** True when a live in-session progress producer exists for this subagent. */ liveProgressAvailable?: boolean; /** Model the subagent actually runs on (after any auth fallback). */ @@ -106,6 +106,63 @@ export interface SubagentSnapshot { fastMode?: boolean; } +/** + * Public await-panel progress. This is deliberately not `AgentProgress`: raw + * progress contains model deltas, tool arguments, arbitrary output, and nested + * task payloads that must never enter tool-result, ACP, or telemetry envelopes. + */ +export interface SubagentLiveProgress { + id: string; + status: AgentProgress["status"]; + currentTool?: string; + recentTool?: string; + recentOutputSummary?: { lineCount: number }; + fastMode?: boolean; + retryState?: { + attempt: number; + maxAttempts: number; + unbounded?: boolean; + kind: NonNullable["kind"]; + provider?: string; + lastProviderProgressAtMs?: number; + delayMs: number; + startedAtMs: number; + }; + retryFailure?: { attempt: number }; +} + +function toSubagentLiveProgress(progress: AgentProgress): SubagentLiveProgress { + return { + id: progress.id, + status: progress.status, + ...(progress.currentTool ? { currentTool: progress.currentTool } : {}), + ...(progress.currentTool === undefined && progress.recentTools[0] + ? { recentTool: progress.recentTools[0].tool } + : {}), + ...(progress.recentOutput.length > 0 + ? { recentOutputSummary: { lineCount: Math.min(progress.recentOutput.length, 6) } } + : {}), + ...(progress.fastMode ? { fastMode: true } : {}), + ...(progress.retryState + ? { + retryState: { + attempt: progress.retryState.attempt, + maxAttempts: progress.retryState.maxAttempts, + ...(progress.retryState.unbounded ? { unbounded: true } : {}), + kind: progress.retryState.kind, + ...(progress.retryState.provider ? { provider: progress.retryState.provider } : {}), + ...(progress.retryState.lastProviderProgressAtMs !== undefined + ? { lastProviderProgressAtMs: progress.retryState.lastProviderProgressAtMs } + : {}), + delayMs: progress.retryState.delayMs, + startedAtMs: progress.retryState.startedAtMs, + }, + } + : {}), + ...(progress.retryFailure ? { retryFailure: { attempt: progress.retryFailure.attempt } } : {}), + }; +} + export type SubagentAwaitOutcome = "completed" | "timed_out" | "interrupted"; const AWAIT_INTERRUPTED_GUIDANCE = @@ -767,11 +824,11 @@ export class SubagentTool implements AgentTool ({ tool: tool.tool, args: tool.args })), - recentOutput: progress.recentOutput, - toolCount: progress.toolCount, - tokens: progress.tokens, - contextTokens: progress.contextTokens ?? null, - contextWindow: progress.contextWindow ?? null, - cost: progress.cost, - modelOverride: progress.modelOverride ?? null, - modelSubstitutionWarning: progress.modelSubstitutionWarning ?? null, - // The nested task panel renders this, so it must reach the signature or a - // fast-mode-only change would render differently while comparing byte-identical, - // suppressing the very update that introduces the glyph. + recentTool: progress.recentTool ?? null, + recentOutputSummary: progress.recentOutputSummary ?? null, fastMode: progress.fastMode ?? false, - // durationMs intentionally excluded (time-derived). - extractedToolData: progress.extractedToolData - ? canonicalizeExtractedToolDataForSignature(progress.extractedToolData) - : null, + retryFailure: progress.retryFailure ?? null, retryState: progress.retryState ? { attempt: progress.retryState.attempt, @@ -1101,52 +1137,7 @@ function canonicalizeProgressForSignature(progress: AgentProgress): unknown { kind: progress.retryState.kind, provider: progress.retryState.provider ?? null, delayMs: progress.retryState.delayMs, - errorMessage: progress.retryState.errorMessage, - // startedAtMs intentionally excluded (drives countdown only). } : null, - retryFailure: progress.retryFailure ?? null, - inflightTaskDetails: progress.inflightTaskDetails - ? canonicalizeTaskDetailsForSignature(progress.inflightTaskDetails) - : null, - }; -} - -/** - * Nested `task` data (`extractedToolData.task` and `inflightTaskDetails`) is the - * one place the await signature reaches into a live, ticking structure: nested - * `AgentProgress` carries the same time-derived fields excluded above, and - * `TaskToolDetails` adds `totalDurationMs` / per-result `durationMs`. Signing it - * wholesale would defeat idle gating whenever an awaited subagent is itself inside - * a live `task` call, so these helpers canonicalize the rendered, non-time subset - * recursively (mutually recursive with `canonicalizeProgressForSignature`). - */ -function canonicalizeExtractedToolDataForSignature(data: Record): Record { - const out: Record = {}; - for (const key of Object.keys(data)) { - // Only the `task` key holds time-ticking `TaskToolDetails`; other handler - // data (yield/report_finding/generic) is stable and passes through as-is. - out[key] = key === "task" ? (data[key] as TaskToolDetails[]).map(canonicalizeTaskDetailsForSignature) : data[key]; - } - return out; -} - -function canonicalizeTaskDetailsForSignature(details: TaskToolDetails): unknown { - // `extractedToolData` is an untyped boundary (`Record`), so - // guard each field instead of trusting the `TaskToolDetails` cast. - return { - // totalDurationMs intentionally excluded (time-derived). - results: Array.isArray(details.results) ? details.results.map(canonicalizeTaskResultForSignature) : null, - progress: Array.isArray(details.progress) ? details.progress.map(canonicalizeProgressForSignature) : null, - async: details.async - ? { state: details.async.state, jobId: details.async.jobId, type: details.async.type } - : null, }; } - -function canonicalizeTaskResultForSignature(result: TaskToolDetails["results"][number]): unknown { - // Completed results do not tick, but drop `durationMs` so the only time-derived - // field in the receipt can never reintroduce idle churn. - const { durationMs: _durationMs, ...rest } = result; - return rest; -} diff --git a/packages/coding-agent/test/tools/subagent-live-progress.test.ts b/packages/coding-agent/test/tools/subagent-live-progress.test.ts index a7727653dd..57ee2de54e 100644 --- a/packages/coding-agent/test/tools/subagent-live-progress.test.ts +++ b/packages/coding-agent/test/tools/subagent-live-progress.test.ts @@ -1,6 +1,11 @@ import { afterEach, describe, expect, it, vi } from "bun:test"; +import { GenAIAttr, resolveTelemetry, startChatSpan } from "@gajae-code/agent-core/telemetry"; +import { createMockModel } from "@gajae-code/ai/providers/mock"; +import { BasicTracerProvider, InMemorySpanExporter, SimpleSpanProcessor } from "@opentelemetry/sdk-trace-base"; import { AsyncJobManager, type SubagentRecord } from "../../src/async"; import { Settings } from "../../src/config/settings"; +import { mapAgentSessionEventToAcpSessionUpdates } from "../../src/modes/acp/acp-event-mapper"; +import { getThemeByName, setThemeInstance } from "../../src/modes/theme/theme"; import type { AgentProgress } from "../../src/task/types"; import type { ToolSession } from "../../src/tools"; import { SubagentTool } from "../../src/tools/implementations"; @@ -9,6 +14,7 @@ import { type SubagentToolDetails, subagentAwaitRenderedStateSignature, } from "../../src/tools/subagent"; +import { subagentToolRenderer } from "../../src/tools/subagent-render"; function createSession(agentId = "0-Main"): ToolSession { return { @@ -90,7 +96,7 @@ describe("subagent await live progress", () => { expect(snap?.status).toBe("running"); expect(snap?.liveProgressAvailable).toBe(true); expect(snap?.progress?.currentTool).toBe("read"); - expect(snap?.progress?.recentOutput).toContain("scanning files"); + expect(snap?.progress?.recentOutputSummary).toEqual({ lineCount: 1 }); manager.cancelSubagent("0-Live", { ownerId: "0-Main" }); await manager.dispose({ timeoutMs: 100 }); @@ -414,12 +420,17 @@ describe("subagentAwaitRenderedStateSignature", () => { it("is value-based: equal values from independent clones produce identical signatures", () => { const a = makeSnapshot({ id: "0-A", - progress: makeProgress({ id: "0-A", currentTool: "read", recentOutput: ["x"] }), + progress: { + id: "0-A", + status: "running", + currentTool: "read", + recentOutputSummary: { lineCount: 1 }, + }, }); const b = makeSnapshot({ id: "0-A", // structuredClone yields a different object reference with equal values. - progress: structuredClone(makeProgress({ id: "0-A", currentTool: "read", recentOutput: ["x"] })), + progress: structuredClone(a.progress), }); expect(subagentAwaitRenderedStateSignature([a])).toBe(subagentAwaitRenderedStateSignature([b])); }); @@ -428,52 +439,46 @@ describe("subagentAwaitRenderedStateSignature", () => { const early = makeSnapshot({ id: "0-A", durationMs: 1_000, - progress: makeProgress({ + progress: { id: "0-A", - durationMs: 1_000, + status: "running", currentTool: "read", - currentToolStartMs: 1_000, retryState: { attempt: 1, maxAttempts: 3, kind: "provider_error", delayMs: 5_000, - errorMessage: "429", startedAtMs: 1_000, }, - }), + }, }); const later = makeSnapshot({ id: "0-A", durationMs: 999_999, - progress: makeProgress({ + progress: { id: "0-A", - durationMs: 999_999, + status: "running", currentTool: "read", - currentToolStartMs: 2_000, retryState: { attempt: 1, maxAttempts: 3, kind: "provider_error", delayMs: 5_000, - errorMessage: "429", startedAtMs: 2_000, }, - }), + }, }); expect(subagentAwaitRenderedStateSignature([later])).toBe(subagentAwaitRenderedStateSignature([early])); }); it("changes when any rendered field changes", () => { - const baseProgress = makeProgress({ + const baseProgress = { id: "0-A", status: "running", currentTool: "read", - recentOutput: ["x"], - toolCount: 1, - tokens: 10, - cost: 0.1, - }); + recentOutputSummary: { lineCount: 1 }, + fastMode: false, + } as const; const base = makeSnapshot({ id: "0-A", status: "running", progress: baseProgress }); const baseSig = subagentAwaitRenderedStateSignature([base]); @@ -492,23 +497,10 @@ describe("subagentAwaitRenderedStateSignature", () => { s => ({ ...s, description: "new description" }), s => ({ ...s, assignment: "new assignment" }), s => ({ ...s, progress: { ...baseProgress, currentTool: "bash" } }), - s => ({ ...s, progress: { ...baseProgress, currentToolArgs: "ls -la" } }), - s => ({ ...s, progress: { ...baseProgress, lastIntent: "thinking" } }), - s => ({ ...s, progress: { ...baseProgress, recentOutput: ["y"] } }), - s => ({ ...s, progress: { ...baseProgress, recentTools: [{ tool: "read", args: "f", endMs: 0 }] } }), - s => ({ ...s, progress: { ...baseProgress, toolCount: 2 } }), - s => ({ ...s, progress: { ...baseProgress, tokens: 20 } }), - s => ({ ...s, progress: { ...baseProgress, contextTokens: 100 } }), - s => ({ ...s, progress: { ...baseProgress, contextWindow: 200_000 } }), - s => ({ ...s, progress: { ...baseProgress, cost: 0.2 } }), + s => ({ ...s, progress: { ...baseProgress, recentTool: "read", currentTool: undefined } }), + s => ({ ...s, progress: { ...baseProgress, recentOutputSummary: { lineCount: 2 } } }), + s => ({ ...s, progress: { ...baseProgress, fastMode: true } }), s => ({ ...s, progress: { ...baseProgress, status: "completed" } }), - s => ({ - ...s, - progress: { - ...baseProgress, - modelSubstitutionWarning: { requested: "a", effective: "b", reason: "auth_unavailable" }, - }, - }), s => ({ ...s, progress: { @@ -518,20 +510,11 @@ describe("subagentAwaitRenderedStateSignature", () => { maxAttempts: 3, kind: "provider_error", delayMs: 1_000, - errorMessage: "429", startedAtMs: 0, }, }, }), - s => ({ ...s, progress: { ...baseProgress, retryFailure: { attempt: 3, errorMessage: "gave up" } } }), - s => ({ ...s, progress: { ...baseProgress, extractedToolData: { task: [{ id: "n1", status: "running" }] } } }), - s => ({ - ...s, - progress: { - ...baseProgress, - inflightTaskDetails: { id: "t1" } as unknown as NonNullable, - }, - }), + s => ({ ...s, progress: { ...baseProgress, retryFailure: { attempt: 3 } } }), ]; for (const mutate of mutations) { @@ -539,86 +522,32 @@ describe("subagentAwaitRenderedStateSignature", () => { } }); - it("recentOutput tail changes are reflected (producer never drops real output progress)", () => { - const a = makeSnapshot({ id: "0-A", progress: makeProgress({ id: "0-A", recentOutput: ["a", "b"] }) }); - const b = makeSnapshot({ id: "0-A", progress: makeProgress({ id: "0-A", recentOutput: ["a", "b", "c"] }) }); - expect(subagentAwaitRenderedStateSignature([a])).not.toBe(subagentAwaitRenderedStateSignature([b])); - }); - - it("ignores nested task time churn but reflects nested status changes", () => { - const makeInflight = ( - durationMs: number, - nestedStatus: AgentProgress["status"], - ): NonNullable => - ({ - projectAgentsDir: null, - results: [], - totalDurationMs: durationMs, - progress: [makeProgress({ id: "child", status: nestedStatus, durationMs, currentToolStartMs: durationMs })], - }) as unknown as NonNullable; - - const early = makeSnapshot({ + it("recent-output summary count changes are reflected without exposing output", () => { + const a = makeSnapshot({ id: "0-A", - progress: makeProgress({ id: "0-A", inflightTaskDetails: makeInflight(1_000, "running") }), + progress: { id: "0-A", status: "running", recentOutputSummary: { lineCount: 2 } }, }); - const later = makeSnapshot({ + const b = makeSnapshot({ id: "0-A", - progress: makeProgress({ id: "0-A", inflightTaskDetails: makeInflight(999_999, "running") }), + progress: { id: "0-A", status: "running", recentOutputSummary: { lineCount: 3 } }, }); - const statusChanged = makeSnapshot({ + expect(subagentAwaitRenderedStateSignature([a])).not.toBe(subagentAwaitRenderedStateSignature([b])); + }); + + it("does not include nested task payloads in the signature", () => { + const safe = makeSnapshot({ id: "0-A", progress: { id: "0-A", status: "running", currentTool: "task" } }); + const withRawNested = makeSnapshot({ id: "0-A", - progress: makeProgress({ id: "0-A", inflightTaskDetails: makeInflight(1_000, "completed") }), + progress: { id: "0-A", status: "running", currentTool: "task" }, }); - - // Nested task time churn (totalDurationMs + nested progress duration/elapsed) is ignored. - expect(subagentAwaitRenderedStateSignature([later])).toBe(subagentAwaitRenderedStateSignature([early])); - // A real nested status change still emits. - expect(subagentAwaitRenderedStateSignature([statusChanged])).not.toBe( - subagentAwaitRenderedStateSignature([early]), - ); + expect(subagentAwaitRenderedStateSignature([withRawNested])).toBe(subagentAwaitRenderedStateSignature([safe])); }); - it("changes when only fastMode flips, at every rendered progress route", () => { - // The nested task panel renders `AgentProgress.fastMode`, and the producer's - // await gating plus the body cache both key off this signature. If the shared - // progress canonicalizer omits fastMode, the one update that introduces the - // glyph compares byte-identical and is suppressed, leaving a stale body. - const nestedTask = (fastMode: boolean) => - [{ id: "n1", progress: [makeProgress({ id: "n1", fastMode })] }] as unknown as NonNullable< - AgentProgress["extractedToolData"] - >["task"]; - const inflight = (fastMode: boolean) => - ({ id: "t1", progress: [makeProgress({ id: "t1", fastMode })] }) as unknown as NonNullable< - AgentProgress["inflightTaskDetails"] - >; - - const routes: Array<[string, (fastMode: boolean) => SubagentSnapshot]> = [ - ["root progress", fastMode => makeSnapshot({ id: "0-A", progress: makeProgress({ id: "0-A", fastMode }) })], - [ - "inflightTaskDetails.progress", - fastMode => - makeSnapshot({ - id: "0-A", - progress: makeProgress({ id: "0-A", inflightTaskDetails: inflight(fastMode) }), - }), - ], - [ - "extractedToolData.task[].progress", - fastMode => - makeSnapshot({ - id: "0-A", - progress: makeProgress({ id: "0-A", extractedToolData: { task: nestedTask(fastMode) } }), - }), - ], - ]; - - for (const [route, build] of routes) { - const slow = subagentAwaitRenderedStateSignature([build(false)]); - const fast = subagentAwaitRenderedStateSignature([build(true)]); - expect(fast, `fastMode must reach the signature via ${route}`).not.toBe(slow); - // Same value twice stays stable, so the difference is fastMode and not churn. - expect(subagentAwaitRenderedStateSignature([build(true)])).toBe(fast); - } + it("changes when only approved fastMode flips", () => { + const slow = makeSnapshot({ id: "0-A", progress: { id: "0-A", status: "running", fastMode: false } }); + const fast = makeSnapshot({ id: "0-A", progress: { id: "0-A", status: "running", fastMode: true } }); + expect(subagentAwaitRenderedStateSignature([fast])).not.toBe(subagentAwaitRenderedStateSignature([slow])); + expect(subagentAwaitRenderedStateSignature([fast])).toBe(subagentAwaitRenderedStateSignature([fast])); }); }); @@ -679,7 +608,7 @@ describe("subagent await emit gating", () => { await manager.dispose({ timeoutMs: 100 }); }); - it("emits exactly once when only a nested task's fastMode flips, and not for an unchanged poll", async () => { + it("emits exactly once when only approved fastMode flips, and not for an unchanged poll", async () => { vi.useFakeTimers(); const manager = createManager(); const tool = new SubagentTool(createSession()); @@ -699,15 +628,7 @@ describe("subagent await emit gating", () => { ); manager.registerSubagentRecord(runningRecord("0-Nested", jobId)); - const nested = (fastMode: boolean) => - makeProgress({ - id: "0-Nested", - currentTool: "task", - inflightTaskDetails: { - id: "t1", - progress: [makeProgress({ id: "n1", fastMode })], - } as unknown as NonNullable, - }); + const nested = (fastMode: boolean) => makeProgress({ id: "0-Nested", currentTool: "read", fastMode }); manager.recordSubagentProgress("0-Nested", nested(false)); const ac = new AbortController(); @@ -721,17 +642,17 @@ describe("subagent await emit gating", () => { await Promise.resolve(); expect(spy).toHaveBeenCalledTimes(1); - // Re-recording the identical nested progress is not a rendered-state change. + // Re-recording the identical approved progress is not a rendered-state change. manager.recordSubagentProgress("0-Nested", nested(false)); vi.advanceTimersByTime(500); expect(spy).toHaveBeenCalledTimes(1); - // Flipping only the nested fastMode changes what renders, so it must emit once. + // Flipping only the approved fastMode changes what renders, so it must emit once. manager.recordSubagentProgress("0-Nested", nested(true)); vi.advanceTimersByTime(500); expect(spy).toHaveBeenCalledTimes(2); const emitted = spy.mock.calls.at(-1)?.[0] as { details?: SubagentToolDetails } | undefined; - expect(emitted?.details?.subagents?.[0]?.progress?.inflightTaskDetails?.progress?.[0]?.fastMode).toBe(true); + expect(emitted?.details?.subagents?.[0]?.progress?.fastMode).toBe(true); // And the new value is then stable: another identical poll stays quiet. manager.recordSubagentProgress("0-Nested", nested(true)); @@ -743,4 +664,197 @@ describe("subagent await emit gating", () => { await exec; await manager.dispose({ timeoutMs: 100 }); }); + + it("emits retry start and recovery but suppresses countdown-only churn", async () => { + vi.useFakeTimers(); + const manager = createManager(); + const tool = new SubagentTool(createSession()); + const control = Promise.withResolvers(); + const jobId = manager.register( + "task", + "retrying subagent", + async () => { + await control.promise; + return "done"; + }, + { + id: "job-retry", + ownerId: "0-Main", + metadata: { subagent: { id: "0-Retry", agent: "executor", agentSource: "bundled" } }, + }, + ); + manager.registerSubagentRecord(runningRecord("0-Retry", jobId)); + const retry = (attempt: number, startedAtMs: number, lastProviderProgressAtMs?: number) => + makeProgress({ + id: "0-Retry", + status: "running", + retryState: { + attempt, + maxAttempts: 3, + kind: "provider_error", + provider: "anthropic", + delayMs: 5_000, + errorMessage: "provider unavailable", + startedAtMs, + ...(lastProviderProgressAtMs === undefined ? {} : { lastProviderProgressAtMs }), + }, + }); + + manager.recordSubagentProgress("0-Retry", retry(1, 0)); + const ac = new AbortController(); + const spy = vi.fn(); + const pending = tool.execute( + "await-retry", + { action: "await", ids: ["0-Retry"], timeout_ms: 3_600_000 }, + ac.signal, + spy, + ); + await Promise.resolve(); + expect(spy).toHaveBeenCalledTimes(1); + + // Only retry timing changed; the approved signature must stay stable. + manager.recordSubagentProgress("0-Retry", retry(1, 1_000, 900)); + vi.advanceTimersByTime(500); + expect(spy).toHaveBeenCalledTimes(1); + + // A new attempt is a real rendered-state transition. + manager.recordSubagentProgress("0-Retry", retry(2, 1_000, 900)); + vi.advanceTimersByTime(500); + expect(spy).toHaveBeenCalledTimes(2); + + // Recovery clears retry state and emits once. + manager.recordSubagentProgress("0-Retry", makeProgress({ id: "0-Retry", currentTool: "read" })); + vi.advanceTimersByTime(500); + expect(spy).toHaveBeenCalledTimes(3); + + ac.abort(); + control.resolve(); + await pending; + await manager.dispose({ timeoutMs: 100 }); + }); +}); + +describe("subagent await progress visibility boundary", () => { + afterEach(() => { + AsyncJobManager.resetForTests(); + }); + + it("carries live progress in details for the renderer and never in model-visible content", async () => { + const manager = createManager(); + const tool = new SubagentTool(createSession()); + const jobId = manager.register( + "task", + "visibility subagent", + async () => { + await Bun.sleep(150); + return "done"; + }, + { + id: "job-vis", + ownerId: "0-Main", + metadata: { subagent: { id: "0-Vis", agent: "executor", agentSource: "bundled" } }, + }, + ); + manager.registerSubagentRecord(runningRecord("0-Vis", jobId)); + manager.recordSubagentProgress( + "0-Vis", + makeProgress({ id: "0-Vis", currentTool: "read", recentOutput: ["secret-marker-text"] }), + ); + + const result = await tool.execute("await", { action: "await", ids: ["0-Vis"], timeout_ms: 5 }); + + const snap = result.details?.subagents.find(s => s.id === "0-Vis"); + expect(snap?.progress?.currentTool).toBe("read"); + expect(snap?.progress?.recentOutputSummary).toEqual({ lineCount: 1 }); + + const modelText = result.content.map(part => ("text" in part ? part.text : "")).join("\n"); + expect(modelText).not.toContain("secret-marker-text"); + expect(modelText).toContain("0-Vis"); + + // Tool-result serialization is a public boundary: the approved DTO carries + // only a count, never the raw marker or any other recent output text. + const serializedToolResult = JSON.stringify({ + role: "toolResult", + toolName: "subagent", + content: result.content, + details: result.details, + }); + expect(serializedToolResult).not.toContain("secret-marker-text"); + + const acpNotifications = mapAgentSessionEventToAcpSessionUpdates( + { + type: "tool_execution_end", + toolCallId: "call-vis", + toolName: "subagent", + result, + isError: false, + }, + "session-vis", + ); + expect(JSON.stringify(acpNotifications)).not.toContain("secret-marker-text"); + + const exporter = new InMemorySpanExporter(); + const tracerProvider = new BasicTracerProvider({ spanProcessors: [new SimpleSpanProcessor(exporter)] }); + const telemetry = resolveTelemetry( + { tracer: tracerProvider.getTracer("subagent-live-progress-test"), captureMessageContent: true }, + "session-vis", + ); + const mockModel = createMockModel({ id: "mock-model", provider: "mock-provider", responses: [] }).model; + const telemetrySpan = startChatSpan(telemetry, mockModel, { + stepNumber: 0, + request: { + messages: [ + { + role: "toolResult", + toolCallId: "call-vis", + toolName: "subagent", + content: result.content, + details: result.details, + isError: false, + timestamp: Date.now(), + }, + ], + }, + }); + telemetrySpan?.end(); + await tracerProvider.forceFlush(); + const telemetryInput = exporter.getFinishedSpans()[0]?.attributes[GenAIAttr.InputMessages]; + expect(telemetryInput).toBeDefined(); + expect(String(telemetryInput)).not.toContain("secret-marker-text"); + await tracerProvider.shutdown(); + + const theme = (await getThemeByName("red-claw"))!; + setThemeInstance(theme); + const rendered = Bun.stripANSI( + subagentToolRenderer + .renderResult(result, { expanded: true, isPartial: true, spinnerFrame: 0 }, theme) + .render(160) + .join("\n"), + ); + expect(rendered).toContain("read"); + expect(rendered).toContain("recent output available (1 line)"); + expect(rendered).not.toContain("secret-marker-text"); + + const staleResult = { + ...result, + details: { + ...result.details, + subagents: result.details!.subagents.map(snapshot => ({ + ...snapshot, + liveProgressAvailable: false, + })), + }, + }; + const staleRendered = Bun.stripANSI( + subagentToolRenderer + .renderResult(staleResult, { expanded: true, isPartial: true, spinnerFrame: 0 }, theme) + .render(160) + .join("\n"), + ); + expect(staleRendered).not.toContain("recent output available"); + expect(staleRendered).not.toContain("read"); + + manager.cancelSubagent("0-Vis", { ownerId: "0-Main" }); + await manager.dispose({ timeoutMs: 100 }); + }); }); diff --git a/packages/coding-agent/test/tools/subagent-render.test.ts b/packages/coding-agent/test/tools/subagent-render.test.ts index 2e8a9c550b..ccca2b6c14 100644 --- a/packages/coding-agent/test/tools/subagent-render.test.ts +++ b/packages/coding-agent/test/tools/subagent-render.test.ts @@ -2,7 +2,7 @@ import { beforeAll, beforeEach, describe, expect, it } from "bun:test"; import type { Theme } from "../../src/modes/theme/theme"; import { getThemeByName, setThemeInstance } from "../../src/modes/theme/theme"; import type { AgentProgress } from "../../src/task/types"; -import type { SubagentSnapshot, SubagentToolDetails } from "../../src/tools/subagent"; +import type { SubagentLiveProgress, SubagentSnapshot, SubagentToolDetails } from "../../src/tools/subagent"; import { subagentAwaitRenderedStateSignature, subagentBodyCacheTestHooks, @@ -17,20 +17,36 @@ beforeAll(async () => { setThemeInstance(theme); }); -function progress(overrides: Partial & Pick): AgentProgress { +function progress(overrides: Partial & Pick): SubagentLiveProgress { + const retryState = overrides.retryState; return { - index: 0, - agent: "executor", - agentSource: "bundled", - status: "running", - task: "assignment", - recentTools: [], - recentOutput: [], - toolCount: 0, - tokens: 0, - cost: 0, - durationMs: 0, - ...overrides, + id: overrides.id, + status: overrides.status ?? "running", + ...(overrides.currentTool ? { currentTool: overrides.currentTool } : {}), + ...(overrides.currentTool === undefined && overrides.recentTools?.[0] + ? { recentTool: overrides.recentTools[0].tool } + : {}), + ...(overrides.recentOutput && overrides.recentOutput.length > 0 + ? { recentOutputSummary: { lineCount: Math.min(overrides.recentOutput.length, 6) } } + : {}), + ...(overrides.fastMode ? { fastMode: true } : {}), + ...(retryState + ? { + retryState: { + attempt: retryState.attempt, + maxAttempts: retryState.maxAttempts, + ...(retryState.unbounded ? { unbounded: true } : {}), + kind: retryState.kind, + ...(retryState.provider ? { provider: retryState.provider } : {}), + ...(retryState.lastProviderProgressAtMs !== undefined + ? { lastProviderProgressAtMs: retryState.lastProviderProgressAtMs } + : {}), + delayMs: retryState.delayMs, + startedAtMs: retryState.startedAtMs, + }, + } + : {}), + ...(overrides.retryFailure ? { retryFailure: { attempt: overrides.retryFailure.attempt } } : {}), }; } @@ -67,7 +83,7 @@ describe("subagentToolRenderer", () => { ], }); expect(out).toContain("read"); - expect(out).toContain("scanning the repo"); + expect(out).toContain("recent output available (1 line)"); }); it("renders the fast glyph on the model line only when fast mode is enabled", () => { const out = render({ @@ -101,7 +117,7 @@ describe("subagentToolRenderer", () => { expect(out).not.toContain(`Model: anthropic/claude-sonnet-4-5 ${theme.icon.fast}`); }); - it("expands live recent output, tool args, and the full task section when expanded=true and collapses them back (AC1/AC2)", () => { + it("renders only the approved current-tool and recent-output summary", () => { const details: SubagentToolDetails = { subagents: [ snapshot({ @@ -110,11 +126,6 @@ describe("subagentToolRenderer", () => { progress: progress({ id: "0-Toggle", currentTool: "bash", - currentToolArgs: "bun test --watch", - // First line is wider than the 40-col collapsed header preview, - // so the second line can only surface via the expand-gated - // Task section (renderTaskSection). - task: "Refactor the authentication module across services\nMigrate sessions to JWT with rotating refresh tokens", recentOutput: ["compiling workspace", "running unit tests"], }), }), @@ -123,17 +134,15 @@ describe("subagentToolRenderer", () => { const expanded = render(details, true); expect(expanded).toContain("bash"); - expect(expanded).toContain("bun test --watch"); - expect(expanded).toContain("compiling workspace"); - expect(expanded).toContain("running unit tests"); - expect(expanded).toContain("Migrate sessions to JWT with rotating refresh tokens"); + expect(expanded).toContain("recent output available (2 lines)"); + expect(expanded).not.toContain("compiling workspace"); + expect(expanded).not.toContain("running unit tests"); const collapsed = render(details, false); expect(collapsed).toContain("bash"); - // Truncated task title stays visible in the collapsed header line. - expect(collapsed).toContain("Refactor the authentication"); - // The expand-gated Task section and recent output must not leak. - expect(collapsed).not.toContain("Migrate sessions to JWT"); + expect(collapsed).toContain("recent output available (2 lines)"); + // Raw tool output and arguments never cross the approved DTO boundary. + expect(collapsed).not.toContain("bun test --watch"); expect(collapsed).not.toContain("compiling workspace"); expect(collapsed).not.toContain("running unit tests"); }); @@ -152,7 +161,7 @@ describe("subagentToolRenderer", () => { expect(out).toContain("0-Stale"); expect(out).not.toContain("edit"); expect(out).not.toContain("stale output line"); - expect(out).not.toContain("running, no activity yet"); + expect(out).not.toContain("running, no approved activity summary yet"); }); it("shows the ctrl+s observe hint under the header while any subagent is running, in both expand states (AC3)", () => { @@ -213,7 +222,7 @@ describe("subagentToolRenderer", () => { const out = render({ subagents: [snapshot({ id: "0-Pending", status: "running", liveProgressAvailable: true })], }); - expect(out).toContain("running, no activity yet"); + expect(out).toContain("running, no approved activity summary yet"); }); it("renders static status without a no-activity claim when no live producer", () => { @@ -221,7 +230,7 @@ describe("subagentToolRenderer", () => { subagents: [snapshot({ id: "0-Static", status: "running", liveProgressAvailable: false })], }); expect(out).toContain("0-Static"); - expect(out).not.toContain("running, no activity yet"); + expect(out).not.toContain("running, no approved activity summary yet"); }); it("stacks multiple awaited subagents", () => { @@ -443,51 +452,27 @@ describe("subagent await renderer body cache (PR2)", () => { const nestedRetry = ( provider = "anthropic", - errorMessage = "Anthropic stream stalled while waiting for the next event", + _errorMessage = "Anthropic stream stalled while waiting for the next event", ): SubagentToolDetails => ({ - subagents: [ + subagents: ["0-Nested.0-Child", "0-Nested.1-Child"].map(id => snapshot({ - id: "0-Nested", + id, liveProgressAvailable: true, progress: progress({ - id: "0-Nested", - currentTool: "task", - inflightTaskDetails: { - projectAgentsDir: null, - results: [], - totalDurationMs: 0, - progress: [ - progress({ - id: "0-Nested.0-Child", - retryState: { - attempt: 2, - maxAttempts: 4, - kind: "idle_stream_stall", - provider, - lastProviderProgressAtMs: 0, - delayMs: 60_000, - errorMessage, - startedAtMs: 0, - }, - }), - progress({ - id: "0-Nested.1-Child", - retryState: { - attempt: 2, - maxAttempts: 4, - kind: "idle_stream_stall", - provider, - lastProviderProgressAtMs: 0, - delayMs: 60_000, - errorMessage, - startedAtMs: 0, - }, - }), - ], + id, + retryState: { + attempt: 2, + maxAttempts: 4, + kind: "idle_stream_stall", + provider, + lastProviderProgressAtMs: 0, + delayMs: 60_000, + errorMessage: _errorMessage, + startedAtMs: 0, }, }), }), - ], + ), }); it("refreshes nested retry age and countdown on await-body updates", () => { @@ -518,7 +503,7 @@ describe("subagent await renderer body cache (PR2)", () => { expect(lines.join("\n")).not.toContain("\t"); }); - it("keeps nested retry groups isolated by snapshot and bypasses only their dynamic cache entries", () => { + it("keeps retry groups isolated by snapshot and bypasses only their dynamic cache entries", () => { const firstNested = nestedRetry(); const secondNested = nestedRetry(); secondNested.subagents[0] = snapshot({ @@ -526,8 +511,16 @@ describe("subagent await renderer body cache (PR2)", () => { liveProgressAvailable: true, progress: progress({ id: "0-OtherNested", - currentTool: "task", - inflightTaskDetails: firstNested.subagents[0]?.progress?.inflightTaskDetails, + retryState: { + attempt: 2, + maxAttempts: 4, + kind: "idle_stream_stall", + provider: "anthropic", + lastProviderProgressAtMs: 0, + delayMs: 60_000, + errorMessage: "provider unavailable", + startedAtMs: 0, + }, }), }); const combined: SubagentToolDetails = { subagents: [...firstNested.subagents, ...secondNested.subagents] }; @@ -541,13 +534,13 @@ describe("subagent await renderer body cache (PR2)", () => { const first = renderWith(combined).join("\n"); Date.now = () => 35_000; const second = renderWith(combined).join("\n"); - const notice = "provider degraded: 2 subagents retrying on anthropic"; - expect(first.split(notice).length - 1).toBe(2); + const notice = "provider degraded: 4 subagents retrying on anthropic"; + expect(first.split(notice).length - 1).toBe(1); expect(second).toContain("last provider progress 35s ago"); - expect(subagentBodyCacheTestHooks.bodyRenders).toBe(5); + expect(subagentBodyCacheTestHooks.bodyRenders).toBe(9); expect(subagentBodyCacheTestHooks.size).toBe(1); renderWith(healthy); - expect(subagentBodyCacheTestHooks.bodyRenders).toBe(5); + expect(subagentBodyCacheTestHooks.bodyRenders).toBe(9); expect(subagentBodyCacheTestHooks.size).toBe(1); } finally { Date.now = originalNow; @@ -664,23 +657,13 @@ describe("subagent await renderer body cache (PR2)", () => { expect(subagentBodyCacheTestHooks.size).toBeLessThanOrEqual(128); }); - it("invalidates the cached body when only a nested task's fastMode flips", () => { - // The body cache is keyed by subagentAwaitRenderedStateSignature, so a nested - // fastMode change that the signature ignored would serve a stale body and the - // glyph would never appear. + it("invalidates the cached body when only approved fastMode flips", () => { const nested = (fastMode: boolean): SubagentToolDetails => ({ subagents: [ snapshot({ id: "0-Nested", liveProgressAvailable: true, - progress: progress({ - id: "0-Nested", - currentTool: "task", - inflightTaskDetails: { - id: "t1", - progress: [progress({ id: "n1", currentTool: "read", fastMode })], - } as unknown as NonNullable, - }), + progress: progress({ id: "0-Nested", currentTool: "read", fastMode }), }), ], }); diff --git a/packages/natives/native/index.d.ts b/packages/natives/native/index.d.ts index 4f6b7d2ebb..2b8e113b0c 100644 --- a/packages/natives/native/index.d.ts +++ b/packages/natives/native/index.d.ts @@ -51,6 +51,7 @@ export declare class ComputerController { keypress(expectedEpoch: number | undefined | null, keys: Array): void wait(expectedEpoch: number | undefined | null, ms: number): void } + /** * Long-lived macOS appearance observer. *