RALPH: TUI Composer double-Enter dedup guard (#159)#470
Open
a9a4k wants to merge 1 commit into
Open
Conversation
Fixes #159: pressing Enter twice in rapid succession on a Composer (reply or top-level annotation) produced a duplicate annotation. The inline submit handler in src/tui/app.tsx awaited the disk write + bundle reload before clearing the Composer in a `finally`, leaving the focused <input> mounted to receive a second submit. Worst on replies, where the latency is enough to invite an impatient Enter. Two layers of defence (per the issue's "Key interfaces"): 1. Synchronous dismiss. `setComposer(null)` now fires BEFORE the first await, not in `finally`. React unmounts the input on the next render, so most second-Enter events never reach the handler. 2. Reentrancy guard. Cache-miss-style race protection: the extracted submitter closes over an `inFlight` flag in a `useRef`, so even when the second Enter slips in before React's flush (event-loop timing varies across opentui / React batching), the second call returns immediately as a no-op. Acceptance criteria walk: * Rapid double-Enter on a reply: exactly one write — covered by `tests/tui/composer-submit.test.ts` "ignores a second submit while the first is in flight (reply path)". Top-level variant in the next test. The guard test holds the write promise pending, fires submit twice, then resolves — writeAnnotation is called once. * Esc-cancel is unchanged: app.tsx keymap path is untouched. * Empty-body silent close preserved: trimmed-empty short-circuits before the guard arms, dismisses with no write. * Error-path doesn't leave Composer stuck open: the synchronous dismiss already cleared it; the `finally` resets `inFlight` so a follow-up submit isn't permanently blocked. * Regression test exercises the rapid-double-Enter path against the Composer submit handler with a slow async write mock — 10 cases total (guard, sync-dismiss, retry after error, retry after success, empty-body, top-level vs reply applyTopLevelCreated, bundle reload, no-writeAnnotation graceful no-op). Key decisions: * Extracted submit orchestration into `src/tui/composer-submit.ts` rather than inlining a `useRef` boolean in app.tsx. Cleaner unit test surface — the issue explicitly asked for a regression test against the submit handler, which the inline-closure form blocks. The new module is ~70 lines and takes callbacks for every state-touching effect (dismiss / applyBundleReload / applyTopLevelCreated) so it stays pure. * The submitter is held in a `useRef`, not `useMemo`, so its in-flight flag survives any future state-only re-render of App. * `bundle` continues to ride along on the top-level write path (CLI tui.ts consumes `input.bundle` to skip a redundant bundle reload — PRD #140 slice 4 #144). Cast to WriteAnnotationInput in the new module to keep typecheck quiet about the excess property (the type sits in app.tsx and src/tui is excluded from tsc anyway, so this is belt-and-braces). Files changed: * src/tui/composer-submit.ts (NEW) — `createComposerSubmitter()` factory; reentrant-safe Promise<void> submit fn. * src/tui/app.tsx — drops inline `submitComposer`, replaces with a `composerSubmitterRef` + thin wrapper that passes current state and callbacks. Net -15 lines. * tests/tui/composer-submit.test.ts (NEW) — 10 cases. Acceptance: 1028/1028 unit tests pass. `npm run typecheck` green. Pre-existing integration failures (tests/integration/{tui,webapp, reply-loop}.test.ts) all root-cause to sandbox-only `bun`/`tsx` install issues, unrelated to this change — same set noted in recent commits. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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
1 participant
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.
Demo PR mirroring commit
d2bb312against a synthetic base (demo/159-base=d2bb312^), so the 3-file diff renders for the Tour demo recording.The real merged change is
d2bb312onmain; this PR is throwaway and can be closed/deleted after the recording.Diff
src/tui/app.tsx(modified, net -15 lines)src/tui/composer-submit.ts(new, 95 lines)tests/tui/composer-submit.test.ts(new, 272 lines, 10-case regression matrix)See
d2bb312's commit message for the full rationale.