Clicking "Send edit" a second time while the first is still working starts a
second full truncate. Observed in the wild on a laggy instance: seven concurrent
POST /api/session/truncate, 5.2s / 8.3s / 6.7s / 7.4s / 8.1s / 8.5s, from one
user clicking repeatedly because the UI had not yet acknowledged the first click.
submitEdit's only guard was S.busy — but send() does not set that until the LAST
line of the function, after two multi-second awaits:
if(!S.session || S.busy) return; // S.busy still false
const absoluteKeepCount = _oldestIdx + msgIdx;
await _ensureAllMessagesLoaded(); // ~6.4s on a long session
await api('/api/session/truncate', ...); // 5-8s under load
await send(); // S.busy finally set
On a 2000-message session that is a 10s+ window in which every further click
runs the whole destructive path again.
The load is the visible symptom; the hazard is worse. absoluteKeepCount is
captured BEFORE the awaits deliberately — at click time _oldestIdx is the loaded
window's offset and msgIdx is window-relative, so the sum is the true absolute
index (pinned by test_submit_edit_captures_absolute_before_await, the #2184
pattern). But the first call's _ensureAllMessagesLoaded() sets _oldestIdx = 0.
A second call entering after that computes `0 + msgIdx` from a msgIdx that is
still window-relative — a far smaller keep_count. Truncating a long session to
that would delete most of its history.
Fix: a module-scope _submitEditInFlight flag, raised before the first await and
cleared in a finally. Guarded at the function rather than at the click handler
because the edit is also submittable with Enter (the keydown handler clicks the
button), so a handler-only guard would leave that path open, and because a future
caller would silently reopen the hole. The finally means an early return or a
throw cannot wedge editing off for the rest of the page's life.
Deliberately NOT changed:
- The pre-await capture stays where it is. Recomputing keep_count after
_ensureAllMessagesLoaded() looks like the obvious fix and is wrong: it would
pair _oldestIdx=0 with a window-relative msgIdx and break #2184 outright.
- regenerateResponse has the same S.busy-only shape, but no longer client-side
truncates (it delegates to the atomic startRegeneration) and validates the
clicked index against latestAssistantIndex, so a double click is refused
server-side rather than being destructive. Left alone to keep this diff to the
fault.
- No UI feedback added. Extra clicks are now harmless, but the button still does
not visibly acknowledge the first one, which is what provoked the clicking.
Worth a follow-up; it is a UX change, not this correctness fix.
Not reproduced end-to-end: this environment cannot run a real agent turn, so the
seven-truncate storm comes from the reporter's network log, not from a local
repro. The guard is verified by source-level test only.
test_submit_edit_is_guarded_against_reentry pins the flag, that it is raised
before the first await and cleared in a finally after send(). Confirmed to FAIL
when the guard is reverted. The three pre-existing tests in that file still pass.
Broad sweep: 2457 passed, 13 failed — all pre-existing test_update_banner_fixes
ordering pollution, identical on a clean tree. One earlier run of the same sweep
showed a 14th failure that did not reproduce across two further runs and whose
name I did not capture; flagging it rather than claiming a clean baseline.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018y9pr7ppZgDuCZCT6x96kY
Claude