fix(client): preserve approval-responded part across input.requested replays - #1527
Open
iroiro147 wants to merge 1 commit into
Open
fix(client): preserve approval-responded part across input.requested replays#1527iroiro147 wants to merge 1 commit into
iroiro147 wants to merge 1 commit into
Conversation
…replays When the event stream replays `input.requested` after a user has already responded (e.g. during a resume), the reducer previously rebuilt the tool part wholesale with `state: "approval-requested"`, wiping the stored `eve.inputResponse` metadata written by `client.input.responded`. The UI then displayed an unanswered approval prompt even though the user had already answered. Partition on the existing tool-part state: - `approval-responded` → preserve `approval` / `input` / `toolMetadata` / `toolName` verbatim; only refresh `stepIndex` and `toolCallId` from the latest event. Never collapse the approval shape into `approval-requested`'s narrower variant. - Otherwise (fresh request or a replay before any response) → emit a fresh `approval-requested` part, merging any existing `toolMetadata` with the newly-derived descriptor so `eve.inputRequest` stays current. Includes a regression test that drives the full replay sequence (input.requested → client.input.responded → replayed input.requested) and asserts the part stays in `approval-responded` with the original `inputResponse` intact. Closes vercel#1507 Signed-off-by: Sarthak Singh <sarthak.singh@juspay.in>
Contributor
|
@iroiro147 is attempting to deploy a commit to the Vercel Team on Vercel. A member of the Team first needs to authorize it. |
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.
fix: preserve approval-responded tool part across replayed input.requested events
What
This is a partial fix for #1507. The issue describes two coupled reducer-idempotency bugs:
upsertRunto detect "same-step done run with identical text" without breaking the multi-run pattern that lets a step emit text, call tools, then emit more text. Worth a follow-up PR with its own coverage.This patch: preserve
approval-respondedacrossinput.requestedreplaysWhen the event stream replays
input.requested(e.g. after a resume),defaultMessageReducerpreviously rebuilt the tool part wholesale withstate: "approval-requested", discarding theeve.inputResponsemetadata thatclient.input.respondedwrites. The UI then showed an unanswered approval prompt even though the user had already responded.The handler now partitions on the existing tool-part state:
approval-responded→ restore, don't rebuild. The priorapproval,input,toolMetadata, andtoolNameare carried forward verbatim; onlystepIndexandtoolCallIdrefresh from the event. We deliberately do not mergeapprovalfields: the union member forapproval-requested(approved?: never; reason?: never) is incompatible with the member forapproval-responded(approved?: boolean; reason?: string), so blending them erases the recorded answer.approval-requested. Still emits the part, but merges any priortoolMetadatawith the newly-derived descriptor viamergeToolMetadata— the same patternaction.resultuses a few branches below.One nuance: parts in terminal states (
output-available/output-error/output-denied) also fall into the "otherwise" branch today. That mirrors upstream behavior (a re-request after a terminal result legitimately starts a new approval cycle in the reducer's coarse state machine), and matching it keeps this patch low-blast-radius. Tightening terminal-state replay handling is part of the same follow-up as the duplicate-text fix.Why this approach
Fixing the resume-from-stale-
streamIndexbug (the other half of #1507) would plug the source but wouldn't make the reducer itself idempotent to duplicateinput.requestedevents from any cause (multi-session, reconnect, hot-reload). Making the reducer idempotent is the narrower, protocol-level fix — and it's a strict superset of what the resume flow needs.Tests
defaultMessageReducer › replayed input.requested does not erase a prior approval-responded part (#1507)inpackages/eve/src/client/message-reducer.test.ts. It drivesinput.requested → client.input.responded → input.requested (replay)and asserts the part staysapproval-respondedwith the originaleve.inputResponseintact andeve.inputRequestpreserved.vitest run --config vitest.unit.config.tsinpackages/eve: 5872 / 5876 passing, with 4 pre-existingbin-bootstrap.test.tsfailures that gate on Node 22+ (unrelated).tsc -p tsconfig.json --noEmitis clean.Linked issue
Refs #1507 (partial — see "What" above for the duplicate-text scope split).