Cover the mobile terminal toolbar's input contracts - #947
Merged
Conversation
The toolbar is the only input surface on mobile and had zero unit tests after #935 reworked it with API-backed Paste/Submit. Almost everything it does is a side effect on something outside its own markup — the escape sequence handed to the terminal socket, the ref the terminal's onData handler reads to fold in the ctrl modifier, and the inject-text POST that deliberately does not go over the terminal WS — so none of it was observable from the existing E2E check. Mounts the real tree (including the copy-mode banner) with only the HTTP seam, the toaster and the audio cue mocked, and pins: the exact escape sequence for all seven shortcut keys; the ctrl modifier arming/disarming ctrlPendingRef and clearing on the terminal's ctrl-consumed announcement without writing the ref back; the 420ms press flash and its next-frame re-arm; the copy-mode banner appearing for copy/exiting but not for the "unknown" state the terminal reports before its first poll; the sound-cue short-circuit; and the composer's inject-text payload, empty-draft short-circuit, no-agent guard, in-flight gating, and failure path keeping the draft, the overlay and focus. 33 of 35 mutants killed. The two survivors are diagnosed, not gaps: the !isConnected guard inside sendKey is redundant with the button's disabled attribute (mutating the pair together dies), and the onSuccess draft clear is unobservable because the overlay unmounts on close. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review catch: the test named "does not flash the ctrl toggle" only
asserted on the other controls, so it stayed green if the ctrl button
started rendering the flash class or the ctrl handler started arming a
flash. Now it asserts the ctrl button carries neither the flash token nor
the animation class, and — since the flash slot is shared and
triggerFlash blanks it first — that arming ctrl mid-flash leaves the key
the user just pressed still lit.
That last assertion is what makes a flashing ctrl handler observable at
all: adding flashButtonClass("ctrl") to the button alone is inert,
because nothing ever sets a flash keyed "ctrl".
Co-Authored-By: Claude Opus 5 (1M context) <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 freeto 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.
Recurring Test Enforcer run. The local suite was green on first pass (25th consecutive) and the CI scan was clean (50/50), so the whole run went to the top
next_focuscoverage gap.Why this file
apps/web/src/components/app/mobile-terminal-toolbar.tsxis 396 lines, the only input surface on mobile, and had zero unit tests — including after #935 reworked it with API-backed Paste/Submit. Nearly everything it does is a side effect on something outside its own markup, which is exactly what the existinge2e/mobile-layout.spec.tscheck cannot see:onDatahandler reads to fold in the ctrl modifierWhat is pinned
28 tests mounting the real tree (including the real copy-mode banner), with only the HTTP seam, the toaster, the audio cue and framer-motion mocked:
ctrlPendingRef, clearing after any key press without rewriting the key, and clearing only the indicator when the terminal announcesctrl-consumed— the toolbar must not write the ref backcopy/exitingbut not for theunknownstate the terminal reports before its first poll, and staying inert while disconnectedMutation battery
35 mutants, 33 killed. Two survivors, both diagnosed rather than papered over:
if (!isConnected) returninsidesendKeyis redundant with the button'sdisabledattribute — mutating the pair together does dieonSuccessdraft clear is unobservable, because the overlay unmounts on closeOne real gap surfaced mid-battery and was fixed: the failure-path refocus assertion was passing either way, because the composer still held focus from the open. A second was caught by review 680 and fixed in the follow-up commit.
Notes for readers of this test file
Fake timers use an explicit
toFakelist that addsrequestAnimationFrame— the component drives both the flash and the focus workaround through rAF, and leaving rAF real whilesetTimeoutis faked would strand those callbacks. jotai state is injected with a per-testcreateStore()becausesoundCuesEnabledAtomreads localStorage at module-eval time.Validation
check,format(on a warm tree),test(server 2715 / web 807→835 / ext 60 / scripts 4),test:e2e(180 passed, 12 skipped),test:e2e:live(11 passed, no leakede2e-*tmux sessions),finalize:web. The new file was run three times standalone with identical results.No product code changed.
🤖 Generated with Claude Code