diff --git a/src/browser/features/Tools/TaskToolCall.test.tsx b/src/browser/features/Tools/TaskToolCall.test.tsx index 54ed59e5c0d..6a3c317197c 100644 --- a/src/browser/features/Tools/TaskToolCall.test.tsx +++ b/src/browser/features/Tools/TaskToolCall.test.tsx @@ -331,6 +331,247 @@ describe("TaskToolCall", () => { ); }); + test("keeps the later pages when the agent rereads an earlier page", () => { + workspaceContextMock = { workspaceMetadata: new Map() }; + const agentTaskArgs = { + subagent_type: "explore", + prompt: "Look around.", + title: "Explore task", + run_in_background: true, + }; + const spawnResult = { status: "running", taskId: "task-reread", note: "Started." }; + const report = "Page zero. Page one. Page two."; + const awaitPage = (offset: number) => { + const page = pageToolText(report, offset, 10); + return createToolMessage({ + toolName: "task_await", + args: { task_ids: ["task-reread"], ...(offset > 0 ? { offset_chars: offset } : {}) }, + result: { + results: [ + { + status: "completed", + taskId: "task-reread", + reportMarkdown: page.text, + ...(page.startCharOffset != null ? { startCharOffset: page.startCharOffset } : {}), + ...(page.nextCharOffset != null ? { nextCharOffset: page.nextCharOffset } : {}), + }, + ], + }, + }); + }; + const linking = computeTaskReportLinking([ + createToolMessage({ toolName: "task", args: agentTaskArgs, result: spawnResult }), + awaitPage(0), + awaitPage(10), + awaitPage(20), + // The agent reads the first page again after the last one. + awaitPage(0), + ]); + expect(linking.reportByTaskId.get("task-reread")?.reportMarkdown).toBe(report); + expect(linking.suppressReportInAwaitTaskIds.has("task-reread")).toBe(true); + + const AgentTaskToolCall = getToolComponent("task", agentTaskArgs, undefined); + const view = render( + + + + ); + fireEvent.click(view.getByText("task")); + expect(view.container.textContent).toContain(report); + }); + + describe("paged foreground reports", () => { + const report = "Page zero. Page one. Page two."; + const foregroundArgs = { + subagent_type: "explore", + prompt: "Look around.", + title: "Explore task", + run_in_background: false, + }; + const firstPage = pageToolText(report, 0, 10); + const awaitPage = (taskId: string, offset: number, text = report) => { + const page = pageToolText(text, offset, 10); + return createToolMessage({ + toolName: "task_await", + args: { task_ids: [taskId], ...(offset > 0 ? { offset_chars: offset } : {}) }, + result: { + results: [ + { + status: "completed", + taskId, + reportMarkdown: page.text, + ...(page.startCharOffset != null ? { startCharOffset: page.startCharOffset } : {}), + ...(page.nextCharOffset != null ? { nextCharOffset: page.nextCharOffset } : {}), + }, + ], + }, + }); + }; + const renderCard = (result: unknown, laterMessages: DisplayedMessage[]) => { + workspaceContextMock = { workspaceMetadata: new Map() }; + const linking = computeTaskReportLinking([ + createToolMessage({ toolName: "task", args: foregroundArgs, result }), + ...laterMessages, + ]); + const AgentTaskToolCall = getToolComponent("task", foregroundArgs, undefined); + const view = render( + + + + ); + fireEvent.click(view.getByText("task")); + return { linking, text: view.container.textContent ?? "" }; + }; + const singleResult = (taskId: string) => ({ + status: "completed", + taskId, + reportMarkdown: firstPage.text, + nextCharOffset: firstPage.nextCharOffset, + }); + + test("shows the whole report after the agent rereads every page from 0", () => { + const card = renderCard(singleResult("fg-reread"), [ + awaitPage("fg-reread", 0), + awaitPage("fg-reread", 10), + awaitPage("fg-reread", 20), + ]); + expect(card.linking.suppressReportInAwaitTaskIds.has("fg-reread")).toBe(true); + expect(card.text).toContain(report); + }); + + test("joins continuation pages onto a best-of candidate's first page", () => { + const card = renderCard( + { + status: "completed", + taskIds: ["fg-a", "fg-b"], + reports: [ + { + taskId: "fg-a", + reportMarkdown: firstPage.text, + nextCharOffset: firstPage.nextCharOffset, + }, + { taskId: "fg-b", reportMarkdown: "Short report." }, + ], + }, + [awaitPage("fg-a", 10), awaitPage("fg-a", 20)] + ); + expect(card.linking.suppressReportInAwaitTaskIds.has("fg-a")).toBe(true); + expect(card.text).toContain(report); + expect(card.text).toContain("Short report."); + }); + + test("joins continuation pages onto a completed candidate of a pending best-of group", () => { + // One candidate finished with a report longer than one page; the other was backgrounded, + // so the foreground result has a pending status (buildPendingTaskResult). + const continuation = pageToolText(report, 10, 100); + const pendingResult = { + status: "running", + taskIds: ["fg-done", "fg-bg"], + tasks: [ + { taskId: "fg-done", status: "completed" }, + { taskId: "fg-bg", status: "interrupted" }, + ], + note: "Tasks were sent to background because a new message was queued.", + reports: [ + { + taskId: "fg-done", + reportMarkdown: firstPage.text, + nextCharOffset: firstPage.nextCharOffset, + }, + ], + }; + const continuationRead = createToolMessage({ + toolName: "task_await", + args: { task_ids: ["fg-done"], offset_chars: firstPage.nextCharOffset }, + result: { + results: [ + { + status: "completed", + taskId: "fg-done", + reportMarkdown: continuation.text, + startCharOffset: continuation.startCharOffset, + }, + ], + }, + }); + const linking = computeTaskReportLinking([ + createToolMessage({ toolName: "task", args: foregroundArgs, result: pendingResult }), + continuationRead, + ]); + expect(linking.reportByTaskId.get("fg-done")?.reportMarkdown).toBe(report); + expect(linking.suppressReportInAwaitTaskIds.has("fg-done")).toBe(true); + expect(renderCard(pendingResult, [continuationRead]).text).toContain(report); + }); + + test("skips malformed foreground report entries instead of throwing", () => { + // A partly corrupted chat.jsonl can hold null or non-object `reports` entries; the + // transcript must still render (self-healing), and the valid entry still joins. + const corruptResult = { + status: "completed", + taskIds: ["fg-ok"], + reports: [ + null, + "not a report", + 7, + { + taskId: "fg-ok", + reportMarkdown: firstPage.text, + nextCharOffset: firstPage.nextCharOffset, + }, + ], + }; + const linking = computeTaskReportLinking([ + createToolMessage({ toolName: "task", args: foregroundArgs, result: corruptResult }), + ]); + expect(linking.reportByTaskId.get("fg-ok")?.reportMarkdown).toBe(firstPage.text); + expect(renderCard(corruptResult, []).text).toContain(firstPage.text); + }); + + test("ignores tool rows named after Object.prototype members", () => { + // A persisted toolName is untrusted: "__proto__" or "constructor" must not resolve to a + // prototype member and throw while the transcript renders. + const linking = computeTaskReportLinking([ + createToolMessage({ toolName: "__proto__", args: {}, result: {} }), + createToolMessage({ toolName: "constructor", args: {}, result: {} }), + createToolMessage({ + toolName: "task", + args: foregroundArgs, + result: singleResult("fg-ok"), + }), + awaitPage("fg-ok", 10), + awaitPage("fg-ok", 20), + ]); + expect(linking.reportByTaskId.get("fg-ok")?.reportMarkdown).toBe(report); + }); + + test("starts a new join when a woken task reports again", () => { + // The second attempt's first page matches the first attempt's text. + const secondReport = "Page zero. Fresh end."; + const card = renderCard(singleResult("fg-again"), [ + awaitPage("fg-again", 10), + awaitPage("fg-again", 20), + createToolMessage({ + toolName: "task_send_message", + args: { task_id: "fg-again", message: "Once more." }, + result: { status: "reactivated", taskId: "fg-again" }, + }), + awaitPage("fg-again", 0, secondReport), + ]); + expect(card.linking.reportByTaskId.get("fg-again")?.reportMarkdown).toBe("Page zero."); + expect(card.text).not.toContain("Page one."); + }); + }); + test("joins a report page at the start the pager rounded to", () => { // offset_chars 10 points inside the emoji's surrogate pair: the pager starts at 9. const report = `${"a".repeat(9)}😀tail`; @@ -383,6 +624,38 @@ describe("TaskToolCall", () => { expect(linking.suppressReportInAwaitTaskIds.has("task-gap")).toBe(false); }); + test("keeps a later page's await row visible when the first page is read after it", () => { + const report = "Page zero. Page one."; + const awaitPage = (offset: number) => { + const page = pageToolText(report, offset, 10); + return createToolMessage({ + toolName: "task_await", + args: { task_ids: ["task-late-first"], offset_chars: offset }, + result: { + results: [ + { + status: "completed", + taskId: "task-late-first", + reportMarkdown: page.text, + ...(page.nextCharOffset != null ? { nextCharOffset: page.nextCharOffset } : {}), + }, + ], + }, + }); + }; + const linking = computeTaskReportLinking([ + createToolMessage({ + toolName: "task", + args: { subagent_type: "explore", prompt: "p", title: "t", run_in_background: true }, + result: { status: "running", taskId: "task-late-first" }, + }), + awaitPage(10), + awaitPage(0), + ]); + expect(linking.reportByTaskId.get("task-late-first")?.reportMarkdown).toBe("Page zero."); + expect(linking.suppressReportInAwaitTaskIds.has("task-late-first")).toBe(false); + }); + test("falls back to result-carried settings after workspace cleanup", () => { workspaceContextMock = { workspaceMetadata: new Map() }; diff --git a/src/browser/features/Tools/TaskToolCall.tsx b/src/browser/features/Tools/TaskToolCall.tsx index a5280fd236e..db56037ee74 100644 --- a/src/browser/features/Tools/TaskToolCall.tsx +++ b/src/browser/features/Tools/TaskToolCall.tsx @@ -573,6 +573,25 @@ const TaskAiSettingsDisplay: React.FC(entries: readonly T[]): T[] { + return entries.filter((entry) => typeof entry === "object" && entry !== null); +} + function collectTaskToolResultDisplayData(result: TaskToolSuccessResult | null): { taskIds: string[]; statusByTaskId: Map; @@ -816,6 +843,7 @@ function collectTaskToolResultDisplayData(result: TaskToolSuccessResult | null): ownReportsByTaskId.set(singleTaskId, { reportMarkdown: result.reportMarkdown, title: result.title, + isFirstPage: result.nextCharOffset != null, }); } @@ -837,12 +865,13 @@ function collectTaskToolResultDisplayData(result: TaskToolSuccessResult | null): } if ("reports" in result && Array.isArray(result.reports)) { - for (const report of result.reports) { + for (const report of objectEntries(result.reports)) { const taskId = rememberTaskId(report.taskId); if (taskId) { ownReportsByTaskId.set(taskId, { reportMarkdown: report.reportMarkdown, title: report.title, + isFirstPage: report.nextCharOffset != null, }); rememberWorkspace(taskId, report.workspaceId); rememberAiSettings(taskId, report); @@ -1010,9 +1039,7 @@ export const TaskToolCall: React.FC = ({ const linkedReport = taskReportLinking?.reportByTaskId.get(taskId); const openWorkspaceId = workspaceIdByTaskId.get(taskId); const metadata = findWorkspaceForTaskTarget(workspaceMetadata, taskId, openWorkspaceId); - const reportMarkdown = hasNonEmptyText(ownReport?.reportMarkdown) - ? ownReport.reportMarkdown - : linkedReport?.reportMarkdown; + const reportMarkdown = pickTaskReportMarkdown(ownReport, linkedReport); const reportTitle = ownReport?.title ?? linkedReport?.title; const derivedStatus = (ownReport ?? linkedReport) diff --git a/src/browser/utils/messages/taskReportLinking.ts b/src/browser/utils/messages/taskReportLinking.ts index 3b91c930a41..551a7240691 100644 --- a/src/browser/utils/messages/taskReportLinking.ts +++ b/src/browser/utils/messages/taskReportLinking.ts @@ -108,10 +108,24 @@ function getReportPageOffset(result: unknown, args: unknown): number { return fromArgs ?? 0; } +/** + * Whether a page with more text after it (nextCharOffset) repeats the text already joined at + * its offset: the agent reread an earlier page, so the later pages it read stay joined. + */ +function isRereadPage( + previous: string | undefined, + page: string, + offset: number, + hasMore: boolean +) { + return hasMore && previous?.startsWith(page, offset) === true; +} + /** * Join a report page onto the report read so far. A page at offset 0 starts the report over; - * a page at or before the end replaces the text from its offset on (a page read twice). A page - * past the end leaves a gap: null, and the caller keeps the await rows visible instead. + * a page at or before the end replaces the text from its offset on (the last page, or another + * report under the same task ID). A page past the end leaves a gap: null, and the caller keeps + * the await rows visible instead. */ function joinReportPage(previous: string | undefined, page: string, offset: number) { if (offset === 0) return page; @@ -206,6 +220,119 @@ function readReportAiSettings(r: { }; } +/** Report joins in progress, across the transcript's tool calls in history order. */ +interface ReportJoinState { + reportByTaskId: Map; + /** Tasks whose pages left a gap: their await rows keep showing each page. */ + incompleteReportTaskIds: Set; + /** + * Tasks the agent sent a message to after their last report. A reused task ID reports again + * after task_send_message wakes it, and agent reports carry no attempt ID, so the next page + * starts a new join instead of matching the previous attempt's text. + */ + newAttemptTaskIds: Set; +} + +/** + * Join one completed report page (a task_await result entry, or a foreground task result's + * first page) into reportByTaskId. + */ +function linkReportPage(r: unknown, args: unknown, state: ReportJoinState): void { + const page = readCompletedAwaitReport(r); + if (page == null) return; + const pageOffset = getReportPageOffset(r, args); + const newAttempt = state.newAttemptTaskIds.has(page.taskId); + const previous = newAttempt ? undefined : state.reportByTaskId.get(page.taskId)?.reportMarkdown; + const hasMore = toPageOffset((r as { nextCharOffset?: unknown }).nextCharOffset) != null; + if (isRereadPage(previous, page.reportMarkdown, pageOffset, hasMore)) return; + const joined = joinReportPage(previous, page.reportMarkdown, pageOffset); + if (joined == null) { + state.incompleteReportTaskIds.add(page.taskId); + return; + } + state.newAttemptTaskIds.delete(page.taskId); + // Only a join that reaches the report's end clears a gap: a first page read after a later + // one does not cover the later page, so its await row stays visible. + if (!hasMore) state.incompleteReportTaskIds.delete(page.taskId); + // Last-wins (history order) + state.reportByTaskId.set(page.taskId, { ...page, reportMarkdown: joined }); +} + +/** + * The first pages a foreground `task` result returned (entries with nextCharOffset), shaped + * like task_await entries. Later task_await pages join onto them, so the card can show the + * whole report even when the agent never reread page 0. + */ +function readForegroundFirstPages(result: unknown): unknown[] { + if (typeof result !== "object" || result === null) return []; + // A best-of group whose other candidates were backgrounded, interrupted or timed out has a + // pending aggregate status, but each `reports` entry is still a completed report. Only a + // single-task result needs its own completed status. + const reports = (result as { reports?: unknown }).reports; + const isCompleted = (result as { status?: unknown }).status === "completed"; + const entries: unknown[] = Array.isArray(reports) ? reports : isCompleted ? [result] : []; + // Persisted results can be corrupted (a null or non-object entry): skip those entries so + // the transcript still renders (self-healing). + return entries + .filter( + (entry): entry is { nextCharOffset?: unknown } => typeof entry === "object" && entry !== null + ) + .filter((entry) => toPageOffset(entry.nextCharOffset) != null) + .map((entry) => ({ ...entry, status: "completed" })); +} + +const DELIVERED_SEND_STATUSES: ReadonlySet = new Set([ + "accepted", + "queued", + "reactivated", +]); + +/** The task ID a delivered task_send_message woke, else null. */ +function getDeliveredSendTaskId(result: unknown): string | null { + if (typeof result !== "object" || result === null) return null; + if (!DELIVERED_SEND_STATUSES.has((result as { status?: unknown }).status)) return null; + const taskId = (result as { taskId?: unknown }).taskId; + return typeof taskId === "string" && taskId.trim().length > 0 ? taskId.trim() : null; +} + +type ToolMessage = Extract; + +/** + * How each tool that carries or resets a report feeds the report joins. A Map, not an object + * literal: a persisted toolName such as "__proto__" or "constructor" must miss, not resolve to + * an Object.prototype member and throw while the transcript renders. + */ +const REPORT_JOIN_HANDLERS = new Map void>([ + [ + "task", + (msg, state) => { + for (const page of readForegroundFirstPages(msg.result)) + linkReportPage(page, undefined, state); + }, + ], + [ + "task_send_message", + (msg, state) => { + const taskId = getDeliveredSendTaskId(msg.result); + if (taskId != null) state.newAttemptTaskIds.add(taskId); + }, + ], + [ + "task_await", + (msg, state) => { + const results = (msg.result as { results?: unknown } | null | undefined)?.results; + if (!Array.isArray(results)) return; + for (const r of results) linkReportPage(r, msg.args, state); + }, + ], +]); + +/** Feed one tool message into the report joins. */ +function linkToolMessage(msg: DisplayedMessage, state: ReportJoinState): void { + if (msg.type !== "tool") return; + REPORT_JOIN_HANDLERS.get(msg.toolName)?.(msg, state); +} + /** * Render-time helper that links completed task reports (from `task_await`) back to the * original `task` tool call that spawned the background work. @@ -249,38 +376,15 @@ export function computeTaskReportLinking(messages: DisplayedMessage[]): TaskRepo } } - // Second pass: collect completed reports from `task_await` results. - const reportByTaskId = new Map(); - // Tasks whose pages left a gap: their await rows keep showing each page. - const incompleteReportTaskIds = new Set(); - for (const msg of messages) { - if (msg.type !== "tool" || msg.toolName !== "task_await") continue; - - const rawResult = msg.result; - if (typeof rawResult !== "object" || rawResult === null) continue; - if (!("results" in rawResult)) continue; - - const results = (rawResult as { results?: unknown }).results; - if (!Array.isArray(results)) continue; - - for (const r of results) { - const page = readCompletedAwaitReport(r); - if (page == null) continue; - const pageOffset = getReportPageOffset(r, msg.args); - const joined = joinReportPage( - reportByTaskId.get(page.taskId)?.reportMarkdown, - page.reportMarkdown, - pageOffset - ); - if (joined == null) { - incompleteReportTaskIds.add(page.taskId); - continue; - } - if (pageOffset === 0) incompleteReportTaskIds.delete(page.taskId); - // Last-wins (history order) - reportByTaskId.set(page.taskId, { ...page, reportMarkdown: joined }); - } - } + // Second pass: collect completed reports from `task_await` results (and the first pages of + // paged foreground `task` results), in history order. + const state: ReportJoinState = { + reportByTaskId: new Map(), + incompleteReportTaskIds: new Set(), + newAttemptTaskIds: new Set(), + }; + for (const msg of messages) linkToolMessage(msg, state); + const { reportByTaskId, incompleteReportTaskIds } = state; // If a task has both a visible spawn card and a non-empty report, suppress the report // duplication under `task_await`. diff --git a/src/common/types/tools.ts b/src/common/types/tools.ts index 8c38b918f2c..1c75d8c7f82 100644 --- a/src/common/types/tools.ts +++ b/src/common/types/tools.ts @@ -136,7 +136,8 @@ export type SkillsCatalogReadToolResult = owner: string; repo: string; path: string; - frontmatter: AgentSkillFrontmatter; + /** On the first page only (no offset_chars). */ + frontmatter?: AgentSkillFrontmatter; body: string; url: string; /** Set when body is one page of a longer body (see offset_chars). */ diff --git a/src/common/utils/tools/pageToolText.ts b/src/common/utils/tools/pageToolText.ts index 68b36695100..1aaee731d63 100644 --- a/src/common/utils/tools/pageToolText.ts +++ b/src/common/utils/tools/pageToolText.ts @@ -68,3 +68,19 @@ export function pageToolTextForResult( note: `Text continues: read the next page with ${call.replace("", next)}.`, }; } + +/** + * One page of a completed task report. Reports are persisted, so the note points at the + * task_await call that re-fetches the next page (from task_await itself or a foreground task). + */ +export function pageTaskReportForResult( + taskId: string, + reportMarkdown: string, + offsetChars: number | null | undefined +): ToolTextPage & { note?: string } { + return pageToolTextForResult( + reportMarkdown, + offsetChars, + `task_await(task_ids: ["${taskId}"], timeout_secs: 0, offset_chars: )` + ); +} diff --git a/src/common/utils/tools/toolDefinitions.ts b/src/common/utils/tools/toolDefinitions.ts index 75153f7a419..66c55905c2d 100644 --- a/src/common/utils/tools/toolDefinitions.ts +++ b/src/common/utils/tools/toolDefinitions.ts @@ -581,6 +581,9 @@ const TaskToolCompletedReportSchema = z .object({ taskId: z.string(), reportMarkdown: z.string(), + /** Set when reportMarkdown is the first page of a longer report; note names the next read. */ + nextCharOffset: z.number().optional(), + note: z.string().optional(), title: z.string().optional(), structuredOutput: z.unknown().optional(), planFilePath: z.string().optional(), @@ -634,6 +637,8 @@ export const TaskToolCompletedResultSchema = z taskId: z.string().optional(), taskIds: z.array(z.string()).min(1).optional(), reportMarkdown: z.string().optional(), + /** Set when reportMarkdown is the first page of a longer report (see note). */ + nextCharOffset: z.number().optional(), title: z.string().optional(), structuredOutput: z.unknown().optional(), planFilePath: z.string().optional(), diff --git a/src/node/services/taskService.test.ts b/src/node/services/taskService.test.ts index c86497235e6..d46897d120c 100644 --- a/src/node/services/taskService.test.ts +++ b/src/node/services/taskService.test.ts @@ -11,6 +11,8 @@ import { recordAgentWorkflowRunReference } from "@/node/services/agentWorkflowRu import { Ok, Err, type Result } from "@/common/types/result"; import { SCRATCH_PROJECT_CONFIG_KEY } from "@/common/constants/scratch"; import { formatSubagentReportEnvelope } from "@/common/utils/subagentReportEnvelope"; +import { TOOL_TEXT_PAGE_MAX_CHARS } from "@/common/constants/toolLimits"; +import { pageTaskReportForResult } from "@/common/utils/tools/pageToolText"; import type { SendMessageError } from "@/common/types/errors"; import { createMuxMessage } from "@/common/types/message"; import { type DynamicToolPart } from "@/common/types/toolParts"; @@ -211,6 +213,7 @@ describe("TaskService", () => { const reportMarkdown = "The delegated investigation is complete."; const completed = { status: "completed", taskId, reportMarkdown }; const awaitOutput = { results: [completed] }; + const pagedCompleted = { ...completed, nextCharOffset: reportMarkdown.length }; const toolPart = (toolName: string, output: unknown): DynamicToolPart => ({ type: "dynamic-tool", toolCallId: "await-report", @@ -372,6 +375,10 @@ describe("TaskService", () => { true, 1, ], + // A long foreground report is one page plus nextCharOffset. A projection without the + // marker hides that more text exists, so it does not deliver the report. + ["paged report without its marker", "task", pagedCompleted, completed, true, 1], + ["paged report with its marker", "task", pagedCompleted, pagedCompleted, true, 0], ] satisfies Array<[string, string, unknown, unknown, boolean, number]>) { test("kernel report visibility: " + name, async () => { const fixture = await setup(); @@ -502,6 +509,78 @@ describe("TaskService", () => { } } + // A foreground workspace turn (and a task_await read) returns one page of a long report. + // The page proves delivery only when it is that exact report's text at the page's offset. + const longTurnReport = `${"a".repeat(TOOL_TEXT_PAGE_MAX_CHARS)}${"b".repeat(1_000)}`; + const firstTurnPage = pageTaskReportForResult("wst_paged", longTurnReport, undefined); + const lastTurnPage = pageTaskReportForResult( + "wst_paged", + longTurnReport, + firstTurnPage.nextCharOffset + ); + for (const [name, toolName, page, expectedWakes] of [ + [ + "a foreground first page with its continuation note", + "task", + { + reportMarkdown: firstTurnPage.text, + nextCharOffset: firstTurnPage.nextCharOffset, + note: firstTurnPage.note, + }, + 0, + ], + [ + "a task_await continuation page", + "task_await", + { reportMarkdown: lastTurnPage.text, startCharOffset: lastTurnPage.startCharOffset }, + 0, + ], + [ + "a page of a different report", + "task", + { + reportMarkdown: "c".repeat(TOOL_TEXT_PAGE_MAX_CHARS), + nextCharOffset: TOOL_TEXT_PAGE_MAX_CHARS, + }, + 1, + ], + [ + "a page at another offset", + "task_await", + { reportMarkdown: lastTurnPage.text, startCharOffset: 0 }, + 1, + ], + ] satisfies Array<[string, string, Record, number]>) { + test("paged workspace-turn receipt: " + name, async () => { + const record = workspaceTurnRecord("owner", taskId, "wst_paged", "completed", { + messageId: "workspace-final", + reportMarkdown: longTurnReport, + }); + const fixture = await setup({ + generationId: [record.handleId, record.status, record.updatedAt].join(":"), + }); + await new TaskHandleStore(fixture.config).upsertWorkspaceTurn({ + ...record, + ownerWorkspaceId: fixture.parentId, + }); + const receipt = { + status: "completed", + taskId: record.handleId, + handleKind: "workspace_turn", + workspaceId: taskId, + messageId: record.messageId, + ...page, + }; + fixture.assistant.parts = [ + toolPart(toolName, toolName === "task" ? receipt : { results: [receipt] }), + ]; + await fixture.historyService.updateHistory(fixture.parentId, fixture.assistant); + await fixture.drain(); + await fixture.drain(); + expect(fixture.resumeStream).toHaveBeenCalledTimes(expectedWakes); + }); + } + test("reconciles a covered response in the drain without a stream-end callback", async () => { const fixture = await setup(); await fixture.historyService.appendToHistory( diff --git a/src/node/services/taskService.ts b/src/node/services/taskService.ts index 2a0e704bd27..958726fcd6c 100644 --- a/src/node/services/taskService.ts +++ b/src/node/services/taskService.ts @@ -1708,15 +1708,41 @@ function collectReferencedTaskIdsFromTaskToolOutput(output: unknown, into: Set): number | undefined { + if (typeof row.startCharOffset === "number") return row.startCharOffset; + return typeof row.nextCharOffset === "number" ? 0 : undefined; +} + +/** + * Whether a continuation receipt shows the owner this exact report: the whole text, or any + * non-empty page of it at the page's own offset. Reading one page (with its continuation note) + * delivers the result, so its terminal notification must not wake the owner again. + */ +function receiptMatchesReport( + receipt: Extract, + reportMarkdown: string | undefined +): boolean { + if (receipt.reportMarkdown === reportMarkdown) return true; + if (reportMarkdown == null || receipt.pageOffset == null) return false; + return ( + receipt.reportMarkdown.length > 0 && + reportMarkdown.startsWith(receipt.reportMarkdown, receipt.pageOffset) + ); +} /** Read only canonical task-result containers, never arbitrary JSON or prose. */ function completedTaskReports(toolName: unknown, output: unknown): CompletedTaskReportReceipt[] { @@ -1747,15 +1773,38 @@ function completedTaskReports(toolName: unknown, output: unknown): CompletedTask if (taskId == null) return []; if (typeof messageId === "string") { return [ - { kind: "continuation", taskId, handleId, messageId, reportMarkdown: row.reportMarkdown }, + { + kind: "continuation", + taskId, + handleId, + messageId, + reportMarkdown: row.reportMarkdown, + pageOffset: readReportPageOffset(row), + }, ]; } return handleId == null - ? [{ kind: "initial", taskId, reportMarkdown: row.reportMarkdown }] + ? [ + { + kind: "initial", + taskId, + reportMarkdown: row.reportMarkdown, + pageOffset: readReportPageOffset(row), + }, + ] : []; }); } +/** + * Same text and same page position. A visible projection that drops a page's offset fields + * (nextCharOffset/startCharOffset) hides that more text exists and where to read it, so it does + * not prove delivery of the genuine paged row. + */ +function sameReportPage(a: CompletedTaskReportReceipt, b: CompletedTaskReportReceipt): boolean { + return a.reportMarkdown === b.reportMarkdown && a.pageOffset === b.pageOffset; +} + function sameTaskReportReceipt( a: CompletedTaskReportReceipt, b: CompletedTaskReportReceipt @@ -1763,7 +1812,7 @@ function sameTaskReportReceipt( return ( a.kind === b.kind && a.taskId === b.taskId && - a.reportMarkdown === b.reportMarkdown && + sameReportPage(a, b) && (a.kind === "initial" || (b.kind === "continuation" && a.messageId === b.messageId && a.handleId === b.handleId)) ); @@ -13211,7 +13260,7 @@ export class TaskService implements AgentTaskIntegration { report.kind === "continuation" && (report.handleId == null || report.handleId === record.handleId) && report.messageId === record.messageId && - report.reportMarkdown === record.reportMarkdown + receiptMatchesReport(report, record.reportMarkdown) ); } } diff --git a/src/node/services/tools/skills_catalog_read.test.ts b/src/node/services/tools/skills_catalog_read.test.ts index 2021bac6c9c..1064f2b61af 100644 --- a/src/node/services/tools/skills_catalog_read.test.ts +++ b/src/node/services/tools/skills_catalog_read.test.ts @@ -54,8 +54,8 @@ describe("skills_catalog_read", () => { expect(result.success).toBe(true); if (result.success) { - expect(result.frontmatter.name).toBe("test-skill"); - expect(result.frontmatter.description).toContain("test skill"); + expect(result.frontmatter?.name).toBe("test-skill"); + expect(result.frontmatter?.description).toContain("test skill"); expect(result.body).toContain("Test Skill"); expect(result.url).toContain("test-owner/test-repo"); } @@ -85,6 +85,9 @@ describe("skills_catalog_read", () => { )) as SkillsCatalogReadToolResult; if (!result.success) throw new Error(result.error); expect(result.body.length).toBeLessThanOrEqual(TOOL_TEXT_PAGE_MAX_CHARS); + // Only the first page carries the (unbounded) frontmatter. + if (call === 0) expect(result.frontmatter?.name).toBe("test-skill"); + else expect(result).not.toHaveProperty("frontmatter"); pages.push(result.body); offset = result.nextCharOffset; snapshot = result.snapshot; @@ -211,8 +214,8 @@ describe("skills_catalog_read", () => { expect(result.success).toBe(true); if (result.success) { - expect(result.frontmatter.name).toBe("test-skill"); - expect(result.frontmatter.description).toContain("test skill"); + expect(result.frontmatter?.name).toBe("test-skill"); + expect(result.frontmatter?.description).toContain("test skill"); expect(result.body).toContain("Test Skill"); expect(result.url).toContain("test-owner/test-repo"); } @@ -240,8 +243,8 @@ describe("skills_catalog_read", () => { expect(result.success).toBe(true); if (result.success) { - expect(result.frontmatter.name).toBe("hidden-skill"); - expect(result.frontmatter.advertise).toBe(false); + expect(result.frontmatter?.name).toBe("hidden-skill"); + expect(result.frontmatter?.advertise).toBe(false); } fetchContentSpy.mockRestore(); diff --git a/src/node/services/tools/skills_catalog_read.ts b/src/node/services/tools/skills_catalog_read.ts index 574d5e4d6f8..738c2143aab 100644 --- a/src/node/services/tools/skills_catalog_read.ts +++ b/src/node/services/tools/skills_catalog_read.ts @@ -120,6 +120,17 @@ function snapshotField( return paging.nextCharOffset != null || paging.startCharOffset != null ? { snapshot } : {}; } +/** + * Only the first page carries the frontmatter: its license and metadata strings have no size + * bound, and later pages read the same pinned copy anyway. + */ +function firstPageFrontmatter( + paging: Omit, + frontmatter: PinnedSkill["parsed"]["frontmatter"] +): { frontmatter?: PinnedSkill["parsed"]["frontmatter"] } { + return paging.startCharOffset == null ? { frontmatter } : {}; +} + function joinNotes(...notes: Array): { note?: string } { const present = notes.filter((note): note is string => note != null); return present.length > 0 ? { note: present.join(" ") } : {}; @@ -146,7 +157,7 @@ export const createSkillsCatalogReadTool: ToolFactory = (_config: ToolConfigurat owner, repo, path: fetched.path, - frontmatter: parsed.frontmatter, + ...firstPageFrontmatter(paging, parsed.frontmatter), body, url: `https://github.com/${owner}/${repo}/blob/${fetched.branch}/${fetched.path}`, ...paging, diff --git a/src/node/services/tools/task.test.ts b/src/node/services/tools/task.test.ts index 7b57708a8ec..71c0bc4274d 100644 --- a/src/node/services/tools/task.test.ts +++ b/src/node/services/tools/task.test.ts @@ -5,6 +5,7 @@ import { tool } from "ai"; import { z } from "zod"; import { createTaskTool, markBuiltInTaskTool, isBuiltInTaskTool } from "./task"; +import { createTaskAwaitTool } from "./task_await"; import { createFakeWorkspaceTurnManager, createTestToolConfig, @@ -13,6 +14,7 @@ import { } from "./testHelpers"; import { Ok, Err } from "@/common/types/result"; import { KNOWN_MODELS } from "@/common/constants/knownModels"; +import { TOOL_TEXT_PAGE_MAX_CHARS } from "@/common/constants/toolLimits"; import { ForegroundWaitBackgroundedError, type TaskService } from "@/node/services/taskService"; function expectQueuedOrRunningTaskToolResult( @@ -49,6 +51,12 @@ function expectGroupedQueuedOrRunningTaskToolResult( expect(typeof obj.note).toBe("string"); } +interface ReportPage { + reportMarkdown: string; + nextCharOffset?: number; + note?: string; +} + // The task tool requires a TaskService up front, but workspace-turn launches never call it. const unusedTaskService = {} as unknown as TaskService; @@ -376,8 +384,9 @@ describe("task tool", () => { using tempDir = new TestTempDir("test-task-tool-workspace-turn-supersede-completed"); const baseConfig = createTestToolConfig(tempDir.path, { workspaceId: "parent-workspace" }); const executeForegroundFollowUp = async ( - maySupersedeTaskId?: string - ): Promise<{ status?: string; note?: string }> => { + maySupersedeTaskId?: string, + reportMarkdown = "done" + ): Promise => { const createWorkspaceTurn = mock(() => Promise.resolve( Ok({ @@ -394,7 +403,7 @@ describe("task tool", () => { taskId: "wst_follow_up", workspaceId: "child-workspace", updatedAt: "2026-08-11T00:00:00.000Z", - reportMarkdown: "done", + reportMarkdown, }) ); const workspaceTurnManager = createFakeWorkspaceTurnManager({ @@ -417,7 +426,7 @@ describe("task tool", () => { }, mockToolCallOptions ) - )) as { status?: string; note?: string }; + )) as ReportPage & { status?: string }; }; const announced = await executeForegroundFollowUp("wst_previous_turn"); @@ -427,6 +436,15 @@ describe("task tool", () => { const silent = await executeForegroundFollowUp(); expect(silent.status).toBe("completed"); expect(silent.note ?? "").not.toContain("wst_previous_turn"); + + // A long report comes as its first page; the note keeps the supersede text too. + const paged = await executeForegroundFollowUp("wst_previous_turn", "x".repeat(60_000)); + expect(paged.reportMarkdown).toHaveLength(TOOL_TEXT_PAGE_MAX_CHARS); + expect(paged.nextCharOffset).toBe(TOOL_TEXT_PAGE_MAX_CHARS); + expect(paged.note).toContain("wst_previous_turn"); + expect(paged.note).toContain( + `task_await(task_ids: ["wst_follow_up"], timeout_secs: 0, offset_chars: ${TOOL_TEXT_PAGE_MAX_CHARS})` + ); }); it.each([ @@ -1070,6 +1088,64 @@ describe("task tool", () => { }); }); + it("returns the first page of a long foreground report and a task_await continuation", async () => { + using tempDir = new TestTempDir("test-task-tool-foreground-report-pages"); + const baseConfig = createTestToolConfig(tempDir.path, { workspaceId: "parent-workspace" }); + const longReport = `${"a".repeat(TOOL_TEXT_PAGE_MAX_CHARS)}${"b".repeat(1_000)}`; + const reports = new Map(); + let createCount = 0; + const taskService = { + create: mock(() => { + createCount += 1; + const taskId = `child-task-${createCount}`; + // The single task and the first best-of candidate write a long report. + reports.set(taskId, createCount <= 2 ? longReport : "short report"); + return Ok({ taskId, kind: "agent" as const, status: "running" as const }); + }), + waitForAgentReport: mock((taskId: string) => + Promise.resolve({ reportMarkdown: reports.get(taskId) ?? "" }) + ), + listActiveDescendantAgentTaskIds: () => [], + isDescendantAgentTask: () => Promise.resolve(true), + getAgentTaskStatuses: () => new Map(), + getAgentTaskStatus: () => "reported" as const, + } as unknown as TaskService; + const config = { ...baseConfig, taskService }; + const runTask = async (n: number) => + (await Promise.resolve( + createTaskTool(config).execute!( + { subagent_type: "explore", prompt: "p", title: "t", run_in_background: false, n }, + mockToolCallOptions + ) + )) as ReportPage & { taskId?: string; reports?: Array }; + const expectFirstPage = (page: ReportPage) => { + expect(page.reportMarkdown).toBe("a".repeat(TOOL_TEXT_PAGE_MAX_CHARS)); + expect(page.nextCharOffset).toBe(TOOL_TEXT_PAGE_MAX_CHARS); + expect(page.note).toContain(`timeout_secs: 0, offset_chars: ${TOOL_TEXT_PAGE_MAX_CHARS})`); + }; + + const single = await runTask(1); + expectFirstPage(single); + expect(single.note).toContain('task_await(task_ids: ["child-task-1"]'); + + const group = await runTask(2); + const [longCandidate, shortCandidate] = group.reports ?? []; + expectFirstPage(longCandidate); + expect(longCandidate.note).toContain('task_await(task_ids: ["child-task-2"]'); + expect(shortCandidate).toMatchObject({ reportMarkdown: "short report" }); + expect(shortCandidate.nextCharOffset).toBeUndefined(); + + // The continuation the note names reads the rest of the report. + const awaitResult = (await Promise.resolve( + createTaskAwaitTool({ ...config, workspaceTurnManager: createFakeWorkspaceTurnManager() }) + .execute!( + { task_ids: ["child-task-1"], timeout_secs: 0, offset_chars: single.nextCharOffset }, + mockToolCallOptions + ) + )) as { results: ReportPage[] }; + expect(single.reportMarkdown + awaitResult.results[0].reportMarkdown).toBe(longReport); + }); + it("prefers report-time AI settings over the launch snapshot in completed results", async () => { using tempDir = new TestTempDir("test-task-tool-report-time-settings"); const baseConfig = createTestToolConfig(tempDir.path, { workspaceId: "parent-workspace" }); diff --git a/src/node/services/tools/task.ts b/src/node/services/tools/task.ts index 2445fa3ca4f..8c81bdcee36 100644 --- a/src/node/services/tools/task.ts +++ b/src/node/services/tools/task.ts @@ -20,6 +20,7 @@ import { log } from "@/node/services/log"; import { ForegroundWaitBackgroundedError } from "@/node/services/taskService"; import { buildTaskGroupLaunches } from "@/common/utils/tools/taskGroups"; +import { pageTaskReportForResult } from "@/common/utils/tools/pageToolText"; import { emitChatEventBestEffort, parseToolResult, @@ -235,9 +236,14 @@ function toAggregatePendingStatus( } function serializeCompletedReport(report: CompletedTaskInfo) { + // A foreground result holds the first page of a long report, like task_await does; the + // note names the task_await call that reads the next page. + const page = pageTaskReportForResult(report.taskId, report.reportMarkdown, undefined); return { taskId: report.taskId, - reportMarkdown: report.reportMarkdown, + reportMarkdown: page.text, + nextCharOffset: page.nextCharOffset, + note: page.note, structuredOutput: report.structuredOutput, title: report.title, agentId: report.agentId, @@ -248,6 +254,21 @@ function serializeCompletedReport(report: CompletedTaskInfo) { }; } +/** The first page of a foreground workspace turn's report, plus any supersede note. */ +function pageWorkspaceTurnReport( + taskId: string, + reportMarkdown: string, + supersedeNote: string | undefined +): { reportMarkdown: string; nextCharOffset?: number; note?: string } { + const page = pageTaskReportForResult(taskId, reportMarkdown, undefined); + const notes = [supersedeNote, page.note].filter((note) => note != null); + return { + reportMarkdown: page.text, + nextCharOffset: page.nextCharOffset, + ...(notes.length > 0 ? { note: notes.join(" ") } : {}), + }; +} + function serializeCompletedReports(reports: readonly CompletedTaskInfo[]) { return reports.map(serializeCompletedReport); } @@ -328,6 +349,8 @@ function buildCompletedTaskResult(params: { status: "completed", taskId: report.taskId, reportMarkdown: report.reportMarkdown, + nextCharOffset: report.nextCharOffset, + note: report.note, structuredOutput: report.structuredOutput, title: report.title, agentId: report.agentId, @@ -511,11 +534,14 @@ export const createTaskTool: ToolFactory = (config: ToolConfiguration) => { taskId: created.data.taskId, workspaceId: report.workspaceId ?? created.data.workspaceId, handleKind: "workspace_turn" as const, - reportMarkdown: report.reportMarkdown, title: report.title, messageId: report.messageId, finalMessageRef: report.finalMessageRef, - ...(completedSupersedeNote != null ? { note: completedSupersedeNote } : {}), + ...pageWorkspaceTurnReport( + created.data.taskId, + report.reportMarkdown, + completedSupersedeNote + ), }, "task" ); diff --git a/src/node/services/tools/task_await.test.ts b/src/node/services/tools/task_await.test.ts index a0ed41525a0..3e7892a539d 100644 --- a/src/node/services/tools/task_await.test.ts +++ b/src/node/services/tools/task_await.test.ts @@ -1348,6 +1348,74 @@ describe("task_await tool", () => { }); }); + it("returns a workflow's structuredOutput on the first report page only", async () => { + using tempDir = new TestTempDir("test-task-await-tool-workflow-pages"); + const baseConfig = createTestToolConfig(tempDir.path, { workspaceId: "parent-workspace" }); + const report = "r".repeat(TOOL_TEXT_PAGE_MAX_CHARS + 10); + const structuredOutput = { findings: "x".repeat(1_000) }; + const completedRun = createWorkflowRun("completed", [ + { + sequence: 1, + type: "result", + at: "2026-01-01T00:00:04.000Z", + result: { reportMarkdown: report, structuredOutput }, + }, + ]); + const tool = createTaskAwaitTool({ + ...baseConfig, + ...createFakeTaskServices({}), + workflowService: { + getRun: mock(() => Promise.resolve(completedRun)), + } as unknown as TestWorkflowService, + }); + const read = async (args: Record) => { + const result = (await Promise.resolve( + tool.execute!({ task_ids: ["wfr_demo"], timeout_secs: 0, ...args }, mockToolCallOptions) + )) as { results: Array }; + return result.results[0]; + }; + + const first = await read({}); + expect(first.structuredOutput).toEqual(structuredOutput); + expect(first.nextCharOffset).toBe(TOOL_TEXT_PAGE_MAX_CHARS); + const second = await read({ offset_chars: first.nextCharOffset }); + expect(second.reportMarkdown).toBe("r".repeat(10)); + expect(second).not.toHaveProperty("structuredOutput"); + // Rereading the first page with an explicit 0 still returns it. + expect((await read({ offset_chars: 0 })).structuredOutput).toEqual(structuredOutput); + }); + + it("drops structuredOutput from a later-page read of an empty report", async () => { + using tempDir = new TestTempDir("test-task-await-tool-empty-report-pages"); + const baseConfig = createTestToolConfig(tempDir.path, { workspaceId: "parent-workspace" }); + const structuredOutput = { findings: "x".repeat(60_000) }; + const tool = createTaskAwaitTool({ + ...baseConfig, + ...createFakeTaskServices({ + listActiveDescendantAgentTaskIds: mock(() => ["t1"]), + isDescendantAgentTask: mock(() => Promise.resolve(true)), + getAgentTaskStatus: mock(() => "reported" as const), + waitForAgentReport: mock(() => Promise.resolve({ reportMarkdown: "", structuredOutput })), + }), + }); + const read = async (offset: number) => { + const result = (await Promise.resolve( + tool.execute!( + { task_ids: ["t1"], timeout_secs: 0, offset_chars: offset }, + mockToolCallOptions + ) + )) as { results: Array }; + return result.results[0]; + }; + + // An empty report reads back the same empty text at any offset: the requested offset, + // not the text, decides which read is the first page. + expect((await read(0)).structuredOutput).toEqual(structuredOutput); + const later = await read(TOOL_TEXT_PAGE_MAX_CHARS); + expect(later.reportMarkdown).toBe(""); + expect(later).not.toHaveProperty("structuredOutput"); + }); + it("returns retry_from_checkpoint guidance for checkpoint-retryable failed workflow runs", async () => { using tempDir = new TestTempDir("test-task-await-tool-workflow-retryable-failed"); const baseConfig = createTestToolConfig(tempDir.path, { workspaceId: "parent-workspace" }); diff --git a/src/node/services/tools/task_await.ts b/src/node/services/tools/task_await.ts index 64741d6c2de..8803a32d2ca 100644 --- a/src/node/services/tools/task_await.ts +++ b/src/node/services/tools/task_await.ts @@ -18,7 +18,7 @@ import { } from "@/common/types/workflow"; import { fromBashTaskId, isWorkflowRunTaskId, toBashTaskId } from "./taskId"; -import { pageToolTextForResult } from "@/common/utils/tools/pageToolText"; +import { pageTaskReportForResult } from "@/common/utils/tools/pageToolText"; import { formatBashOutputReport } from "./bashTaskReport"; import { dedupeStrings, @@ -117,15 +117,29 @@ function pageCompletedReport( result: T, offsetChars: number | null | undefined ): T { - if (result.status !== "completed" || fromBashTaskId(result.taskId) != null) return result; - if (!("reportMarkdown" in result) || typeof result.reportMarkdown !== "string") return result; - const { text, ...paging } = pageToolTextForResult( + if (!isPageableReport(result)) return result; + const { text, ...paging } = pageTaskReportForResult( + result.taskId, result.reportMarkdown, - offsetChars, - `task_await(task_ids: ["${result.taskId}"], timeout_secs: 0, offset_chars: )` + offsetChars ); - // A whole report keeps its result as is (including the re-fetch note). - return text === result.reportMarkdown ? result : { ...result, reportMarkdown: text, ...paging }; + const firstPage = (offsetChars ?? 0) === 0; + // A whole report read from the start keeps its result as is (including the re-fetch note). + if (firstPage && text === result.reportMarkdown) return result; + const page = { ...result, reportMarkdown: text, ...paging }; + // structuredOutput has no size bound: only a read from offset 0 carries it, so reading a + // report page by page does not repeat it. Decided by the requested offset, not by the text: + // an empty report reads back the same empty text at any offset. + if (!firstPage) Reflect.deleteProperty(page, "structuredOutput"); + return page; +} + +/** A completed, non-bash result with a string report: the results report paging applies to. */ +function isPageableReport( + result: T +): result is T & { reportMarkdown: string } { + if (result.status !== "completed" || fromBashTaskId(result.taskId) != null) return false; + return "reportMarkdown" in result && typeof result.reportMarkdown === "string"; } function getWorkflowRunElapsedMs(run: WorkflowRunRecord): number | undefined {