Skip to content

fix(tools): keep the forget-memory timeout when a caller passes a signal - #1567

Open
addyCooks wants to merge 1 commit into
supermemoryai:mainfrom
addyCooks:fix/forget-memory-signal-timeout
Open

fix(tools): keep the forget-memory timeout when a caller passes a signal#1567
addyCooks wants to merge 1 commit into
supermemoryai:mainfrom
addyCooks:fix/forget-memory-signal-timeout

Conversation

@addyCooks

@addyCooks addyCooks commented Aug 19, 2026

Copy link
Copy Markdown

Closes #1549

Problem

#1451 bounded DELETE /v4/memories with a 30-second abort, but the caller
signal and the timeout were selected between with ??:

signal: options?.signal ?? AbortSignal.timeout(FETCH_TIMEOUT_MS)

They were mutually exclusive. Passing a cancellation signal silently dropped
the timeout and the request became unbounded again - the exact hang #1451 set
out to remove - with no way to ask for both through the API. Latent today
(ai-sdk.ts:332 and openai/tools.ts:490 both omit options), live the
moment anyone wires up cancellation.

Fix

Compose the two with AbortSignal.any, building the timeout signal once so
the bare path reuses it. AbortSignal.any is available in Node 20+ (the
repo's engines floor), Bun, and workerd.

Test

tool-operations.test.ts replaces the case that asserted the old select-one
behaviour with five:

  • the caller signal is composed rather than substituted, and its abort reason
    reaches the request
  • the timeout still fires while the caller signal stays open
  • an already-aborted caller signal is forwarded with its reason
  • the bare path still receives the 30s timeout signal itself, not a composition
  • an aborted fetch surfaces to the caller rather than being swallowed

18 tests in the file pass. The timeout case spies on AbortSignal.timeout
(shortened to 5ms) while asserting the code still requested 30_000 - Node
captures its internal timer at module load, so fake timers cannot intercept
AbortSignal.timeout.

Each case was checked against mutants of the fix, so the suite fails for a
wrong-but-plausible fix and not only for a full revert:

mutant result
AbortSignal.any([options.signal]) - timeout leg dropped 1 case fails
composed with a signal that never fires 1 case fails
revert to options?.signal ?? ... 2 cases fail
timeout shortened to 300ms 2 cases fail
the fix as submitted 18/18 pass

Verification against a real socket

Against a server that accepts the connection and never responds:

scenario before after
caller signal present, never aborts hangs indefinitely TimeoutError at the deadline
no caller signal TimeoutError TimeoutError
caller aborts early AbortError AbortError
already-aborted caller signal immediate AbortError immediate AbortError
200 response with a caller signal resolves resolves

Note (not in this PR)

apps/mcp/src/server/client/index.ts:370 (getDocuments) has the identical
?? pattern and is equally latent - no caller there passes options either.
Left out to keep this diff scoped to the issue and to avoid colliding with
#1564, which already touches that file.

supermemoryai#1451 bounded `DELETE /v4/memories` with a 30s abort, but the two signals
were selected between with `??`:

    signal: options?.signal ?? AbortSignal.timeout(FETCH_TIMEOUT_MS)

so they were mutually exclusive. Any caller who passed a cancellation
signal silently dropped the timeout and the request went unbounded
again - exactly the hang supermemoryai#1451 set out to remove - and there was no way
to ask for both through the API. No production call site passes options
today (ai-sdk.ts and openai/tools.ts both omit it), so this was latent.

Composes the two with `AbortSignal.any` instead, so a caller signal
cancels the request and the 30s ceiling still applies. The timeout
signal is built once and reused for the bare case.

Verified against a server that never responds: before this change the
caller-signal path hangs indefinitely; after it, it rejects with
TimeoutError at the deadline, an early caller abort still wins with
AbortError, and an already-aborted signal rejects immediately.

tool-operations.test.ts replaces the case that asserted the old
select-one behaviour with five: the caller signal is composed rather
than substituted and its abort reason reaches the request, the timeout
still fires while the caller signal stays open, an already-aborted
caller signal is forwarded, the bare path still gets the 30s timeout
signal itself, and an aborted fetch surfaces to the caller.

Each case was checked against mutants of the fix - dropping the timeout
from AbortSignal.any, composing with a signal that never fires, reverting
to ??, and shortening the timeout - and every mutant fails at least one
of them.
@addyCooks
addyCooks force-pushed the fix/forget-memory-signal-timeout branch from 349a821 to 73cf9f4 Compare August 19, 2026 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

forgetMemoryRequest drops its 30s timeout whenever a caller passes a signal

1 participant