feat: fix #272 add memory feature #420
Merged
Conversation
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
seratch
added
enhancement
🦋 Changeset detectedLatest commit: 27e74c4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
dkundel-openai
approved these changes
Sep 3, 2025
|
Yeah, I'll take some time to further tests before merging this |
|
This PR is stale because it has been open for 10 days with no activity. |
|
This PR is stale because it has been open for 10 days with no activity. |
|
How's this going? Would be great to get sessions in there to get more parity with the Python SDK! Plus, I could really use it :) |
|
+1 for this feature! We're very interested in session/memory management for the TS SDK. Having parity with the Python SDK for conversation persistence would be incredibly valuable for our use case. |
|
Would love to see this feature soon! |
|
+1 |
seratch
force-pushed
the
issue-272-memory
branch
from
269c2fb to
f55dac8
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
packages/agents-core/src/run.ts
Outdated
Comment on lines
1345
to
1354
| let preparedInput: typeof input = input; | ||
| if (!(preparedInput instanceof RunState)) { | ||
| preparedInput = await prepareInputItemsWithSession( | ||
| preparedInput, | ||
| session, | ||
| sessionInputCallback, | ||
| ); | ||
| } else if (session) { | ||
| throw new UserError( | ||
| 'Cannot provide a session when resuming from a previous RunState.', |
There was a problem hiding this comment.
The new session support unconditionally throws whenever Runner.run is called with a previously returned RunState and a session (packages/agents-core/src/run.ts lines 1345‑1354). Interruption flows such as tool approval require you to resume from the prior RunResult.state so the agent can continue, but the guard now raises UserError('Cannot provide a session when resuming from a previous RunState.'). That makes the memory feature unusable for exactly the multi-turn, human-in-the-loop scenarios it is meant to support: if you drop the session to get past the error the continuation will never be written to the session, so subsequent turns see an incomplete history. The fix is to allow sessions when resuming (skip re-preparing the input, but still persist the resumed turn) instead of throwing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@codex i've fixed this. can you review the changes again?
- [P1] Honor zero limit in getItems — packages/agents-openai/src/memory/openaiConversationsSession.ts:103-112
When getItems is called with limit = 0, the current code enters the if (!limit) branch and streams the entire conversation history. Callers generally treat limit literally (0
meaning "return nothing"), so returning the full history is both incorrect and can result in unexpectedly large fetches. Please change the guard to check specifically for limit
=== undefined before falling back to the unbounded branch so that passing 0 (or any other falsy numeric limit) respects the requested limit.
- [P1] Restore message type on session inputs — packages/agents-core/src/runImplementation.ts:1495-1506
When normalizing a string input for session persistence we now emit { role: 'user', content: ... } without the type: 'message' field. Any run that supplies a session with
a plain string input (the default usage) will persist that shape via prepareInputItemsWithSession, and the same objects get reused when the next turn is assembled. Downstream
logic—especially the new callModelInputFilter hook that callers will commonly implement by checking item.type === 'message'—now fails to recognize user turns. Before this patch
we always kept type: 'message' (see toAgentInputList in the runner), so this is a regression. Please keep type: 'message' on the objects returned here so hooks and providers can
continue to identify user messages correctly.
- [P0] Delay streaming session persistence until after filter — packages/agents-core/src/run.ts:936-969
For streaming runs we call ensureStreamInputPersisted?.() on the first turn before #prepareModelCall runs (line 936). ensureStreamInputPersisted immediately persists
sessionInputItemsToPersist and flips its internal persisted flag. Later, #prepareModelCall invokes applyCallModelInputFilter, which may redact or truncate the turn input and
updates sessionInputItemsFiltered. However, the second ensureStreamInputPersisted?.() (line 968) is a no-op because the persisted flag is already true, so the session history
now contains the unfiltered, unredacted input that the filter was supposed to scrub. This leaks exactly the data the caller asked us not to keep. We need to defer the first
persistence until after the filter (or otherwise allow the post-filter call to write the sanitized items) so that streaming memory mirrors the filtered payload.
- [P1] Prevent duplicating run items on approval resume — packages/agents-core/src/runImplementation.ts:460-462
When we resume a turn after tool approval, originalPreStepItems already contains the assistant message/tool-call items that were generated before the interruption.
Initializing newItems with processedResponse.newItems reuses those same instances, so SingleStepResult.generatedItems ends up concatenating [...preStepItems, ...newItems] where
the message/tool-call appear twice. This causes duplicate entries to be persisted in session history and re-emitted to callers every time an approval is resolved. The previous
implementation started from the tool results only (const newItems = functionResults.map(...)) to avoid this duplication. We should revert to building newItems from the newly
produced tool results instead of cloning the prior processedResponse.newItems array.
- [P0] Preserve pending MCP approvals when resuming — packages/agents-core/src/runImplementation.ts:414-418
When we resume an interrupted turn, this block removes every RunToolApprovalItem from originalPreStepItems, but we only re-add the ones tied to function tools. For
hosted MCP approvals without an on_approval handler, if the user has not yet approved, approved stays undefined, so nothing gets pushed back into newItems. As a result
maybeCompleteTurnFromToolResults sees no interruptions, returns next_step_run_again, and the runner keeps looping until it hits the max-turns guard. In practice you can
reproduce this with any HostedMCPTool HITL flow: run once, do not call state.approve, resume the run, and the approval request disappears so the user can no longer respond. We
need to keep outstanding hosted approval items in preStepItems (or push them back into newItems) whenever they are still waiting on human input; otherwise HITL with hosted MCP
tools is broken.
- [P0] Fix assistant item conversion — packages/agents-openai/src/memory/openaiConversationsSession.ts:100-100
When conversations.items.list returns assistant turns they come back as type: 'response' objects whose real messages live under the output array. Passing the whole
conversation item into convertToOutputItem falls through to the fallback branch and yields { type: 'unknown' }. That means every assistant turn we read from Conversations
gets stored as an invalid history item, so the very next turn we try to run will send an unknown input back to the model and the Responses API rejects the call. To unblock the
feature we need to convert the item.output payload instead (e.g. convertToOutputItem(item.output ?? [])).
- [P1] Preserve new input when callbacks drop items — packages/agents-core/src/runImplementation.ts:1728-1734
When sessionInputCallback returns a list that omits the new turn items (e.g. it returns a filtered history and relies on our fallback to re-include the latest user message),
appended stays empty. For non server-managed runs we still send the turn because preparedInput falls back to newInputSnapshot, but in the server-managed path (conversationId/
previousResponseId) we take prepared.sessionItems ?? [] (run.ts L465). With sessionItems set to [] we end up issuing the model call with no user input at all, so the
conversation cannot progress. Please mirror the fallback here (e.g. return sessionItems: appended.length > 0 ? appended : newInputSnapshot) so server-managed sessions keep the
new input.
- [P1] Preserve input when server session callback drops additions — packages/agents-core/src/run.ts:467-476
In this branch we replace the entire model input with prepared.sessionItems. That works when the callback append returns the new turn items, but it breaks valid callbacks that
only reuse existing history. For example, running with conversationId and a session callback such as (history) => history (or any callback that mutates the history array in-
place without appending new items) yields sessionItems === []. In the non-server-managed path we fall back to prepared.preparedInput, so the new user utterance is still sent.
Here we instead send an empty array to the model, effectively discarding the caller’s input whenever they reuse that callback alongside conversationId. That contradicts the
comment above about reusing callbacks and causes empty requests. Please fall back to prepared.preparedInput whenever sessionItems is empty so the user input is preserved.
- [P1] Keep original inputs when filters prepend items — packages/agents-core/src/run.ts:400-440
When callModelInputFilter adds any new items in front of the originals (for example, by returning [primer, ...modelData.input]), the bookkeeping in this loop lets the inserted
item consume the only counter entry for the underlying user message. Then, when we reach the real input a few iterations later, remaining has already been decremented to zero,
so the actual user message is skipped and never added to sessionInputItemsFiltered. The session layer therefore persists only the synthetic primer and silently drops the user’s
input from memory, so the next turn will run without the user message in history. Please adjust the allocation logic so that prepended/augmented items do not steal the slots
needed to persist the original inputs.
- [P1] Persist streaming session input on model errors — packages/agents-core/src/run.ts:1045-1086
When we stream a run with a session, ensureStreamInputPersisted is only invoked in the final_output and interruption branches. If the model stream rejects before either
branch is reached (for example a transient provider/network error, an abort signal, or a thrown tool error), we exit the loop via the catch path without ever calling
ensureStreamInputPersisted, so saveStreamInputToSession never runs. That drops the user’s latest turn from the session, which prevents resuming the conversation precisely when
we need memory most. We should persist the input as soon as we hand it to the model (or at least in a finally block) so streaming sessions remain consistent even when the stream
fails early.
- [P1] Preserve file_data when translating conversation items — packages/agents-openai/src/memory/openaiConversationsSession.ts:85-90
When OpenAIConversationsSession#getItems converts a conversation.items.list response back into AgentInputItems, input_file content only copies file_url or file_id. If the
original user turn supplied inline data (file_data)—which the Conversations API returns as file_data per the SDK typings—we drop it here. The next time the runner uses this
history, getInputItems() produces an input_file with no data, so the model call either loses the attachment or stalls with a 400. Please propagate c.file_data (and associated
filename) into the returned AgentInputItem so inline file inputs survive across turns.
- [P1] Resume approved computer actions — packages/agents-core/src/runImplementation.ts:343-386
In resolveInterruptedTurn we now only execute the resumed function tool calls. Any computer tool invocations that were interrupted for approval never get dispatched when the
turn resumes, because processedResponse.computerActions is ignored in the resumed path (compare with resolveTurnAfterModelResponse, which calls executeComputerActions). As a
result, a computer-use call that requires human approval will remain stuck forever after approval—no computer action runs and the turn cannot progress. Please mirror the non-
interrupted path by invoking executeComputerActions for the computer calls whose approvals have been granted.
- [P1] Rewind only pending approval items — packages/agents-core/src/runImplementation.ts:352-358
In this resume path we subtract approvalItemCount computed over all RunToolApprovalItems in originalPreStepItems. When a turn experiences multiple approval interruptions in
sequence, _generatedItems already contains older approval placeholders plus their resolved outputs. Subtracting the total count drops _currentTurnPersistedItemCount below the
run items we already persisted (e.g., the first tool’s output). The next call to saveToSession then slices from that lower index and re-adds those older outputs to the session,
producing duplicate tool results in persisted history. You can reproduce by running with session persistence, triggering two approvals back-to-back, approving the first, letting
the agent request a second, then approving again — the session now contains the first tool result twice. Please only rewind the counter for the approval items that are still
pending (e.g., the trailing approvals for the current interruption) rather than every approval seen so far.
- [P0] Convert stored tool outputs back to function results — packages/agents-openai/src/memory/openaiConversationsSession.ts:99-110
When we fetch history from an OpenAI conversation we run each item through convertToOutputItem here. Items that we previously added via session.addItems for tool results
arrive from the Conversations API as type === "function_call_output", but convertToOutputItem only understands real response output types and falls back to returning an
UnknownItem. On the very next turn the runner feeds these history items back through getInputItems, which throws UserError: Unsupported item { type: "unknown" }, so any tool
usage breaks the session after the first turn. Please deserialize function_call_output into a proper FunctionCallResultItem before returning it from getItems.
- [P0] Respect sessionInputCallback redactions — packages/agents-core/src/runImplementation.ts:1986-1992
If a session input callback intentionally omits or redacts the new turn inputs (e.g. it returns only the prior history to avoid persisting sensitive data), we
should not write the raw inputs to the session. However, when appended.length === 0 this code falls back to newInputSnapshot, so we still persist the original turn
items even though the callback removed them. This leaks exactly the data the developer tried to drop, defeating the purpose of the callback. Please respect the
callback’s decision by persisting nothing (or only the items it returned) when no new items were appended.
- [P0] Always capture previousResponseId — packages/agents-core/src/run.ts:1791-1796
When we start a Responses conversation without an explicit conversationId or previousResponseId, trackServerItems should capture the first response ID so that
follow-up turns can pass it back to the API. The new guard this.previousResponseId !== undefined prevents that first assignment, so previousResponseId stays
undefined forever and second turn requests go up without any conversation context even though we only send the delta. That means the model loses all history and
breaks server-managed conversation flows. Please drop that guard and always record the latest response ID whenever conversationId is absent.
- [P0] Reset session persistence counter — packages/agents-core/src/runImplementation.ts:1823-1844
saveToSession now slices result.newItems using _currentTurnPersistedItemCount, but that counter is never reset when we advance turns. After turn 1 we
increment the counter to the number of generated items, then on turn 2 we replace state._generatedItems with the new turn’s array without touching the counter.
As a result alreadyPersisted still equals the full length of turn 1, result.newItems.slice(alreadyPersisted) becomes [], and we never write any later turn’s
outputs to the session. Any multi-turn run using FileSession/PrismaSession/OpenAIConversationsSession will therefore persist only the first turn. Please reset
_currentTurnPersistedItemCount whenever we start a fresh turn (e.g. right after assigning the new generatedItems) before calling saveToSession.
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
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.
This pull request resolves #272
references: