From 5f9a521d60a8fcbb4728589c5549eae3b29338c2 Mon Sep 17 00:00:00 2001 From: TYY-2005 Date: Mon, 19 Oct 2026 07:32:43 +0800 Subject: [PATCH] fix(frontend): stop stalled subtask pagination without losing steps (#6729) * fix(frontend): stop stalled subtask pagination without losing steps * docs(frontend): note the non-advancing page stop in the subtask backfill The AGENTS.md backfill sentence only said the loop pages until a short page. It also stops on a full page that fails to advance the after_seq cursor, so record that stop condition as well. The file sits at 49,149 of its 49,152-byte hard limit, so this is the minimal wording change that fits. --------- Co-authored-by: TYY-2005 <334029581+TYY-2005@users.noreply.github.com> --- frontend/src/AGENTS.md | 2 +- frontend/src/core/tasks/api.ts | 24 +++++++--- frontend/tests/unit/core/tasks/api.test.ts | 52 ++++++++++++++++++++++ 3 files changed, 72 insertions(+), 6 deletions(-) diff --git a/frontend/src/AGENTS.md b/frontend/src/AGENTS.md index f1d8998919..e413e67dbd 100644 --- a/frontend/src/AGENTS.md +++ b/frontend/src/AGENTS.md @@ -211,7 +211,7 @@ export on a page error or non-advancing cursor; demos keep their fixture path. - Web-search steps accept either a provider result array or a Webz-style `{ query, returned_results, results }` envelope. Unwrap only in the web-search renderer; skip entries without string titles/URLs, preserve other tool payloads and apply the same link allowlist to both formats. UI regressions pin source titles/links and browser reloads, including malformed neighboring entries. - Tool-step links in `message-group.tsx` (`web_fetch` args, `web_search` / `image_search` result URLs) are model- or provider-controlled, so they pass the markdown `isSafeHref` allowlist; every surface renders a rejected href through the shared `UnsafeLink` marker. - **Environment validation** uses `@t3-oss/env-nextjs` with Zod schemas (`src/env.js`). Skip with `SKIP_ENV_VALIDATION=1` -- **Subtask step history and runtime metadata** (`core/tasks/`) — cards interleave subagent reasoning and tool steps (#3779). Task `description` is optional progress text; `MessageList` titles use the required `prompt`, then a localized generic label, so valid calls never show a blank title. `Subtask.steps[]` appends live `task_running` events via `mergeSteps`. On historical-card expansion, `fetchSubtaskSteps` backfills task-scoped events (GET `/runs/{runId}/events?event_types=subagent.step&task_id=…&after_seq=…`) until a short page; run-wide limits must not truncate the timeline. `task_started` carries the effective `model_name`; `task_running` carries a cumulative usage snapshot after each completed LLM call. `core/tasks/lifecycle.ts` normalizes these additive events, and `computeNextSubtask` keeps the largest cumulative total so replayed or late SSE frames cannot double-count or roll the folded card backward. Terminal ToolMessage metadata (`subagent_model_name` / `subagent_token_usage`) restores the same values from normal history after reload; no per-card event fetch is needed. `core/tasks/steps.ts` is the pure step model: `messageToStep` (live), `eventsToSteps` (reload), `mergeSteps` (dedup by `message_index`), and `stepsForDisplay` (what the card renders — keeps tool steps + AI steps with text, drops the trailing final-answer AI step when completed since it's shown as `result`). `core/tasks/context.tsx`'s `useUpdateSubtask` applies updates against a `tasksRef` mirroring the latest state (not a closure snapshot), so a late-resolving `fetchSubtaskSteps` backfill merges into current state instead of clobbering SSE steps or sibling subtasks that arrived meanwhile. The owning `run_id` is carried onto history content messages in `buildVisibleHistoryMessages` so the card can resolve the events endpoint. +- **Subtask step history and runtime metadata** (`core/tasks/`) — cards interleave subagent reasoning and tool steps (#3779). Task `description` is optional progress text; `MessageList` titles use the required `prompt`, then a localized generic label, so valid calls never show a blank title. `Subtask.steps[]` appends live `task_running` events via `mergeSteps`. On historical-card expansion, `fetchSubtaskSteps` backfills task-scoped events (GET `/runs/{runId}/events?event_types=subagent.step&task_id=…&after_seq=…`) until a short page or a non-advancing full page; run-wide limits must not truncate the timeline. `task_started` carries the effective `model_name`; `task_running` carries a cumulative usage snapshot after each completed LLM call. `core/tasks/lifecycle.ts` normalizes these additive events, and `computeNextSubtask` keeps the largest cumulative total so replayed or late SSE frames cannot double-count or roll the folded card backward. Terminal ToolMessage metadata (`subagent_model_name` / `subagent_token_usage`) restores the same values from normal history after reload; no per-card event fetch is needed. `core/tasks/steps.ts` is the pure step model: `messageToStep` (live), `eventsToSteps` (reload), `mergeSteps` (dedup by `message_index`), and `stepsForDisplay` (what the card renders — keeps tool steps + AI steps with text, drops the trailing final-answer AI step when completed since it's shown as `result`). `core/tasks/context.tsx`'s `useUpdateSubtask` applies updates against a `tasksRef` mirroring the latest state (not a closure snapshot), so a late-resolving `fetchSubtaskSteps` backfill merges into current state instead of clobbering SSE steps or sibling subtasks that arrived meanwhile. The owning `run_id` is carried onto history content messages in `buildVisibleHistoryMessages` so the card can resolve the events endpoint. - Structured subtask status parsing in `core/tasks/subtask-result.ts` accepts only own keys of the status mapping. Prototype names such as `constructor`, `toString`, and `__proto__` use the same `in_progress` fallback as other unknown diff --git a/frontend/src/core/tasks/api.ts b/frontend/src/core/tasks/api.ts index 8e35e6af84..b91dcc83e5 100644 --- a/frontend/src/core/tasks/api.ts +++ b/frontend/src/core/tasks/api.ts @@ -16,10 +16,11 @@ type FetchedEvent = Parameters[0][number] & { * Fetch a subtask's persisted step history for a historical run (#3779). * * Scoped server-side to this `taskId` (and to `subagent.step` events) and paged - * forward with an `after_seq` cursor until a short page, so the run-wide event - * limit can never truncate a subagent's step timeline — even for long runs or - * runs with several subagents. Used by the subtask card to backfill steps on - * expand when the live SSE steps are gone (e.g. after a page reload). + * forward with an `after_seq` cursor until a short page or a page that fails to + * advance the cursor, so the run-wide event limit can never truncate a + * subagent's step timeline — even for long runs or runs with several subagents. + * Used by the subtask card to backfill steps on expand when the live SSE steps + * are gone (e.g. after a page reload). */ export async function fetchSubtaskSteps( threadId: string, @@ -49,15 +50,28 @@ export async function fetchSubtaskSteps( throw new Error(`Failed to fetch subtask steps: ${res.status}`); } const batch = (await res.json()) as FetchedEvent[]; - events.push(...batch); if (batch.length < pageSize) { + events.push(...batch); break; } const lastSeq = batch[batch.length - 1]?.seq; if (lastSeq === undefined) { + events.push(...batch); break; // can't advance the cursor; stop rather than refetch page 0 forever } + if (afterSeq !== undefined && lastSeq <= afterSeq) { + // The store contract is `seq > after_seq`, so a full page whose last seq + // fails to advance the cursor is stale or malformed. Drop that page and + // return the pages already accumulated, matching the missing-`seq` + // fallback above: the expand-backfill caller swallows a rejection and + // resets its one-shot guard, so throwing here would render an empty card + // and replay the same futile requests on every expand. `mergeSteps` + // dedupes by `message_index`, so the retained prefix stays correct. + break; + } + + events.push(...batch); afterSeq = lastSeq; } diff --git a/frontend/tests/unit/core/tasks/api.test.ts b/frontend/tests/unit/core/tasks/api.test.ts index f691784983..9183f009d6 100644 --- a/frontend/tests/unit/core/tasks/api.test.ts +++ b/frontend/tests/unit/core/tasks/api.test.ts @@ -90,6 +90,58 @@ describe("fetchSubtaskSteps", () => { expect(mockedFetch).toHaveBeenCalledTimes(1); }); + test("stops and keeps the accumulated prefix when a full page does not advance the cursor", async () => { + mockedFetch.mockImplementation(() => + Promise.resolve( + jsonResponse(200, [ + stepEvent(10, 0, "web_search"), + stepEvent(11, 1, "read_file"), + ]), + ), + ); + + const steps = await fetchSubtaskSteps("t", "r", "A", 2); + + // The repeated page is dropped instead of being re-requested up to the + // 100-page bound, and the valid first page still reaches the caller. + expect(steps.map((s) => s.message_index)).toEqual([0, 1]); + expect(mockedFetch).toHaveBeenCalledTimes(2); + }); + + test.each([12, 13])( + "keeps all prior pages when a later full page ends at non-advancing seq %s", + async (lastSeq) => { + mockedFetch + .mockResolvedValueOnce( + jsonResponse(200, [ + stepEvent(10, 0, "web_search"), + stepEvent(11, 1, "read_file"), + ]), + ) + .mockResolvedValueOnce( + jsonResponse(200, [ + stepEvent(12, 2, "bash"), + stepEvent(13, 3, "write_file"), + ]), + ) + .mockImplementation(() => + Promise.resolve( + jsonResponse(200, [ + stepEvent(lastSeq - 1, 4, "web_search"), + stepEvent(lastSeq, 5, "read_file"), + ]), + ), + ); + + const steps = await fetchSubtaskSteps("t", "r", "A", 2); + + expect(steps.map((s) => s.message_index)).toEqual([0, 1, 2, 3]); + expect(mockedFetch).toHaveBeenCalledTimes(3); + expect(mockedFetch.mock.calls[1]![0] as string).toContain("after_seq=11"); + expect(mockedFetch.mock.calls[2]![0] as string).toContain("after_seq=13"); + }, + ); + test("throws when a page request fails", async () => { mockedFetch.mockResolvedValueOnce(jsonResponse(500, { detail: "boom" }));