Repository navigation
fix(purchases): a late execute response re-enables Execute in a newer review modal #1724
Description
Summary
Follow-up from the review of #1718. In handleExecutePurchase (frontend/src/app.ts, the finally block at ~:622), the Execute button state is always restored when the request settles. That includes the case where the user closed the review modal and opened a new purchase while the request was in flight. #1718 already keeps a late failure from closing or clearing the newer modal: it checks the modal open token. But the finally still re-enables Execute on the new modal, using the old request's submitting state.
Steps
- Start a direct execute and keep the request pending.
- Close the review modal and open a new purchase.
- Let the first request fail with a response a purchase may have followed (5xx, 409, or one that carries
execution_id).
Expected
The finally only touches the button for the modal session that sent the request (the same open-token check as the catch branch). A newer modal keeps its own button state.
Notes
Low severity: the newer modal's own selection decides what a click sends. Related: #1694, #1718.
Activity
cristim commentedon Nov 4, 2026
Checked before fixing, and I think the premise doesn't hold; the proposed fix would introduce a stuck button. (claude-ui-fix-2)
#execute-purchase-btncarriesdata-submitting=true. Opening a new single or fan-out modal keeps it (resetExecuteModeAndLabeland the fan-out summary inrecommendations.tsskip the label/disabled update while submitting). So the newer modal can't start its own request until the old one settles.finallyis the only thing that clears that flag.setExecutePurchaseSubmitting(…, false, …)then recomputesdisabledfrom the current selection (getPurchaseModalRecommendations()or the fan-out buckets) and the label from the current mode. So it applies the newer modal's own state, not the old request's.purchase-execution-toast.test.ts, run on main and then with the proposedif (getPurchaseModalSession() === modalSession)guard around thefinally): start a request, switch the session to a newer modal, then reject with a 502.So what happens today is correct, and the session check belongs only on the close/clear branch, where #1718 already has it.
I suggest closing this as not planned. Alternatively, rephrase it as "pin the current behaviour with a regression test"; the probe above would be that test, and I can open it as a small test-only PR if wanted. I'm leaving the close to the issue's owner.
cristim commentedon Nov 4, 2026
Closing as not planned (coordinator decision): main already restores the newer modal's Execute button correctly, and the proposed guard would leave it stuck on 'Sending...'. See the probe in the comment above.