fix(tools): route Responses API memory saves through /v4/conversations#1286
Open
abhay-codes07 wants to merge 1 commit into
Open
fix(tools): route Responses API memory saves through /v4/conversations#1286abhay-codes07 wants to merge 1 commit into
abhay-codes07 wants to merge 1 commit into
Conversation
When a customId is configured, the chat-completions path hands addMemoryTool the messages plus API credentials, so the save routes through /v4/conversations with the internal "conversation:" prefix stripped back off. The Responses path built the same prefixed memoryCustomId but passed neither messages nor credentials, so the conversation branch never ran and the save fell through to client.add() with "conversation:<id>" as the stored customId — a value customId validation rejects (alphanumeric/hyphen/underscore only). The error is swallowed by design (memory saves must not break the user's API call), which means every Responses API call with a customId silently saved nothing. Hand the input over as a single user message together with the credentials, mirroring the chat path, so conversation grouping works for the Responses API too. Also strip the internal prefix in the client.add fallback so the routing marker can never leak into a stored customId on any path (previously the chat path had the same leak whenever SUPERMEMORY_API_KEY was unset). Covered with mocked-SDK middleware tests: a customId save routes through addConversation with the un-prefixed conversation id and never touches client.add; the fallback stores the user's configured id, not the prefixed marker; and the no-customId path is unchanged. Two of the three tests fail against the previous implementation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Responses API memory saves silently failed whenever a
customIdwas configured.The chat-completions path hands
addMemoryToolthe messages plus API credentials, so a configuredcustomIdroutes the save through/v4/conversations(with the internalconversation:routing prefix stripped back off). The Responses path built the same prefixedmemoryCustomIdbut passed neither messages nor credentials:So the conversation branch never ran and the save fell through to
client.add()withconversation:<id>as the stored customId — a value customId validation rejects (alphanumeric/hyphen/underscore only). Since save errors are swallowed by design (memory persistence must never break the user's API call), everyresponses.createcall with acustomIdsaved nothing, invisibly.Fix
client.addfallback strips the internalconversation:prefix so the routing marker can never leak into a stored customId on any path (the chat path had the same leak wheneverSUPERMEMORY_API_KEYwas unset)Testing
src/openai/middleware.test.ts): a customId save routes throughaddConversationwith the un-prefixed conversation id and never touchesclient.add; the fallback stores the user's configured id, not the prefixed marker; the no-customId path is unchanged — two of the three tests fail against the previous implementationvitest run— 3 passtsc --noEmiterror count unchanged vsmain(148 pre-existing),biome checkcleancc @MaheshtheDev