From b202862ea498f2549daec1856ef171d808085fbf Mon Sep 17 00:00:00 2001 From: Cintu07 <178455858+Cintu07@users.noreply.github.com> Date: Thu, 27 Aug 2026 15:42:17 +0530 Subject: [PATCH] fix(tools): keep the forget-request timeout when a caller passes a signal forgetMemoryRequest combined the caller signal and the 30s timeout with `??`, so passing a cancellation signal dropped the timeout and made the DELETE /v4/memories request unbounded again, undoing #1451. compose them with AbortSignal.any so both still apply. updates the existing signal test to assert the composed behavior. closes #1549 --- packages/tools/src/shared/forget-memory.ts | 7 ++++++- packages/tools/src/tool-operations.test.ts | 12 ++++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/packages/tools/src/shared/forget-memory.ts b/packages/tools/src/shared/forget-memory.ts index 8691c92a..6df9c0bc 100644 --- a/packages/tools/src/shared/forget-memory.ts +++ b/packages/tools/src/shared/forget-memory.ts @@ -33,7 +33,12 @@ export async function forgetMemoryRequest( Authorization: `Bearer ${apiKey}`, }, body: JSON.stringify(params), - signal: options?.signal ?? AbortSignal.timeout(FETCH_TIMEOUT_MS), + // compose the caller's signal with the timeout instead of choosing + // between them: `??` dropped the 30s bound whenever a signal was passed, + // which reopened the unbounded-request hang that #1451 closed. + signal: options?.signal + ? AbortSignal.any([options.signal, AbortSignal.timeout(FETCH_TIMEOUT_MS)]) + : AbortSignal.timeout(FETCH_TIMEOUT_MS), }) if (!response.ok) { diff --git a/packages/tools/src/tool-operations.test.ts b/packages/tools/src/tool-operations.test.ts index 136a19be..5194795b 100644 --- a/packages/tools/src/tool-operations.test.ts +++ b/packages/tools/src/tool-operations.test.ts @@ -112,7 +112,7 @@ describe("memoryForget", () => { expect(init.signal).toBeInstanceOf(AbortSignal) }) - it("uses a caller-provided signal instead of creating a timeout", async () => { + it("composes the caller signal with the timeout so both still bound the request", async () => { const fetchMock = stubFetch() const controller = new AbortController() @@ -124,7 +124,15 @@ describe("memoryForget", () => { ) const [, init] = fetchMock.mock.calls[0] as [string, RequestInit] - expect(init.signal).toBe(controller.signal) + const signal = init.signal as AbortSignal + // #1549: a composed signal, not the raw caller signal. the 30s timeout + // must not be dropped just because a caller passes their own signal. + expect(signal).toBeInstanceOf(AbortSignal) + expect(signal).not.toBe(controller.signal) + expect(signal.aborted).toBe(false) + // the caller's signal still aborts the request through the composite + controller.abort() + expect(signal.aborted).toBe(true) }) it("throws a descriptive error on non-2xx responses", async () => {