fix: walk past an empty-string cursor in the legacy task refresh (#2220)
`ManagedRequestorTasksState.refresh()` drives the legacy `tasks/list` walk itself, and both of its cursor tests were truthiness checks — so the fixed adapter was never reached. `while (cursor)` ended the walk on a `""` `nextCursor`, leaving the user-visible task list on page one, and `cursor ? [...this.tasks, ...page] : page` would have replaced the accumulated list rather than extended it had the walk continued. Both now test `=== undefined`. The other four paged/managed stores already did (`pagedToolsState`, `pagedPromptsState`, `pagedResourcesState`, `pagedResourceTemplatesState`, `pagedRequestorTasksState`, `managedSkillsState`); this was the last walk on the truthiness side. Covered by a new case in `managedRequestorTasksState.test.ts` that paginates `"" → "c2" → end` and asserts the cursor each call received, so a walk that terminated correctly but re-requested page one still fails. Reverting either guard on its own fails exactly that case. Reported by Copilot on #2277. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EN2UfrkQZvtrviMtqURRm4 Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
9b35e530fc
commit
3b4eb447c9
3 changed files with 35 additions and 5 deletions
|
|
@ -101,6 +101,27 @@ describe("ManagedRequestorTasksState", () => {
|
|||
expect(client.listRequestorTasks).toHaveBeenCalledTimes(3);
|
||||
});
|
||||
|
||||
it("refresh walks past an empty-string cursor instead of stopping at page one", async () => {
|
||||
// A cursor is opaque and `""` is a legal `nextCursor` (#2220). Under the
|
||||
// truthiness guard this walk used to carry, page one's empty cursor ended
|
||||
// the loop — so the task list stopped at `t1` against a conforming server,
|
||||
// with nothing to show for it. The cursor each call receives is asserted
|
||||
// too, because a walk that stopped correctly but re-requested page one
|
||||
// would still produce three tasks with the wrong three requests.
|
||||
client.setStatus("connected");
|
||||
client.queueTaskPages(
|
||||
{ tasks: [task("t1")], nextCursor: "" },
|
||||
{ tasks: [task("t2")], nextCursor: "c2" },
|
||||
{ tasks: [task("t3")] },
|
||||
);
|
||||
|
||||
const result = await state.refresh();
|
||||
expect(result.map((t) => t.taskId)).toEqual(["t1", "t2", "t3"]);
|
||||
expect(client.listRequestorTasks.mock.calls.map((call) => call[0])).toEqual(
|
||||
[undefined, "", "c2"],
|
||||
);
|
||||
});
|
||||
|
||||
it("connect event triggers a refresh", async () => {
|
||||
client.setStatus("connected");
|
||||
client.queueTaskPages({ tasks: [task("t1")] });
|
||||
|
|
|
|||
|
|
@ -116,9 +116,12 @@ export class FakeInspectorClient
|
|||
listResourceTemplates = vi.fn(
|
||||
async () => this.resourceTemplatePages.shift() ?? { resourceTemplates: [] },
|
||||
);
|
||||
listRequestorTasks = vi.fn(
|
||||
async () => this.taskPages.shift() ?? { tasks: [] },
|
||||
);
|
||||
// Typed by its signature rather than inferred, so `mock.calls` carries the
|
||||
// cursor a test asserts on — the empty-string pagination case (#2220) turns
|
||||
// on *which* cursor each call received, not just how many there were.
|
||||
listRequestorTasks = vi.fn<
|
||||
(cursor?: string) => Promise<ListResult<"tasks", Task>>
|
||||
>(async () => this.taskPages.shift() ?? { tasks: [] });
|
||||
listSkills = vi.fn(async () => this.skillPages.shift() ?? { skills: [] });
|
||||
// `skills/get` echoes a minimal entry; tests that care override the mock.
|
||||
getSkill = vi.fn(async (uri: string) => ({
|
||||
|
|
|
|||
|
|
@ -181,7 +181,13 @@ export class ManagedRequestorTasksState extends TypedEventTarget<ManagedRequesto
|
|||
? { ...t, status: "cancelled" as const }
|
||||
: t,
|
||||
);
|
||||
this.tasks = cursor ? [...this.tasks, ...page] : page;
|
||||
// `=== undefined`, not truthiness, on both the append test and the loop
|
||||
// condition: a cursor is opaque and `""` is a legal value (#2220). Under
|
||||
// a truthiness check an empty `nextCursor` ended the walk after page one
|
||||
// AND made the next page replace the list rather than extend it — the
|
||||
// user-visible task list silently stopping at four rows against a
|
||||
// conforming server.
|
||||
this.tasks = cursor === undefined ? page : [...this.tasks, ...page];
|
||||
cursor = result.nextCursor;
|
||||
pageCount++;
|
||||
if (pageCount >= MAX_PAGES) {
|
||||
|
|
@ -189,7 +195,7 @@ export class ManagedRequestorTasksState extends TypedEventTarget<ManagedRequesto
|
|||
`Maximum pagination limit (${MAX_PAGES} pages) reached while listing requestor tasks`,
|
||||
);
|
||||
}
|
||||
} while (cursor);
|
||||
} while (cursor !== undefined);
|
||||
this.dispatchTypedEvent("tasksChange", this.tasks);
|
||||
return this.getTasks();
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue