Fix: don't submit comment on IME composition Enter key - #30
Open
ulysses0604 wants to merge 1 commit into
Open
Conversation
Every Enter-key keydown handler that submits a comment (the comment composer, inline comment editing, the send-batch shortcut, and the artifact SDK's link-input) treated a bare Enter as "submit" without checking whether an IME composition was in progress. Japanese/CJK users pressing Enter to confirm kanji conversion had their in-progress comment submitted prematurely instead. Add an isImeCommitEnter(event) guard (event.isComposing, with the legacy keyCode === 229 fallback) before each of these handlers acts, so a composition-confirming Enter no longer triggers preventDefault() or the submit action.
petergyang
added a commit
that referenced
this pull request
Sep 2, 2026
From community PR #30 by @ulysses0604.
2 tasks
petergyang
added a commit
that referenced
this pull request
Sep 3, 2026
* Wait for Send with one background poll instead of timed re-polls The skill told agents to run poll --timeout 600 in the foreground and re-run it on every timeout. Now it starts one open-ended poll in the background and ends the turn; the harness wakes the agent when the command exits, which only happens on Send or when the review is closed. To make an open-ended wait safe, the CLI reconnects (restarting the server if needed) when the connection drops instead of giving up after three tries, and treats a socket close without end as a drop so the promise cannot be left unsettled. * Retry the atomic rename when Windows briefly locks the temp file From community PR #40 by @rzilem. * Harden review frame: drop popup/download rights, validate external hrefs From community PR #37 by @dacoldest. * Don't submit a comment on an IME composition Enter From community PR #30 by @ulysses0604. * Use the document filename as the browser tab title From community PR #36 by @NuclearManatee. * setup: treat any node_modules bin as transient; fix caret, details, heading-label bugs - Port community PR #7 by @noammigdali-hio onto the invocation(run) signature: a bin under any node_modules segment is transient, not just npm's _npx cache. - Issue #12: clicks inside the compose textarea place the caret instead of being swallowed by the card's focus handler. - Issue #25: <summary> keeps its native toggle so collapsed content stays reachable. - Issue #26: a heading labels itself instead of being named after the heading before it. * Server: make every wait terminate, persist delivery, queue second sends, one server per state dir - A tab that unloads tells the server; unless it reconnects within a short grace the review ends with reason window_closed and the waiting poll is released. A tab that silently vanishes ends the review after the 30-minute TTL instead of being deleted with the poll left hanging. - A poll with no review open waits a short grace for one, then closes with reason no_review_open. Closed answers carry the unsent counts. - A new poll for a target supersedes an older one, so stale background waits cannot double-apply a batch. - delivered is persisted, so a server replaced between delivery and --ack honors the ack instead of re-shipping the batch. A batch written by an overlapping server is read from disk. - Sending while the agent is applying a batch queues only the new items and carries the old cleanup forward; the agent's next --ack clears both. - edits_saved and markdown flags per page; edit text capped at 200k with an explicit truncated marker instead of a silent 4k cut; Markdown and self-rendering pages keep unsent edit rows when the file changes on disk. - Artifact URLs carry a per-run secret segment; a page key derived from the file path is no longer enough to read sibling files. - Localhost reviews proxy the app's own requests (API calls, prefetches, srcset images) to the app origin instead of 404ing. - One server per state directory: a lock file plus a health-checked takeover of an older-protocol server. Protocol bumped to 9. - New routes: DELETE edit (undo), POST mode (dynamic), POST discard, POST away. - Comment rewording updates an undelivered batch in place, or takes a fresh id after delivery so the rewrite ships next time. * Browser and SDK: tab-close beacon, session re-bootstrap, undo, leftover banner, multi-block edits - The chrome tells the server when the tab unloads (keepalive fetch on pagehide) and re-opens a session when the server has forgotten it, instead of turning into a dead tab that still looks alive. - Undo for the two irreversible gestures: a toast with an Undo button after a block delete or move restores the block and drops its edit row. - Feedback left over from a review that ended without a Send shows a Keep/Discard banner on the next open, noting that HTML text edits are already in the file either way. - A second Send while the agent works shows as queued, not as nobody listening. Comments get an Edit button and an edited badge; rewording after delivery explains it ships with the next Send. - Typing over a selection that spans blocks reports every block it touched, including a deleted row for one that vanished; a paragraph turned into a list keeps its label and captured original. - CLI: an open-ended wait ends after 12 hours with a timeout status that says to check status and start again; superseded polls just print. - SKILL.md: closed/superseded handling, edits_saved, deleted and truncated rows, rendered quotes with anchors, staged assets before ack, note-only batches. Codex block updated to match. - srcset URLs are absolutized for localhost reviews. * Fix phantom sessions after a review ends; pin edit-list ordinals to load order - After the server ended a session, the tab's event stream reconnected, got a 404, and the new re-bootstrap path opened a fresh session with no tab behind it, which kept the review open and the poll waiting. The chrome now never re-bootstraps once the review ended or the tab is unloading, and the server drops a session no browser connected to within a minute. - Label ordinals ("p 3") come from sibling order at load, so deleting a paragraph does not hand two blocks the same label within one review. - A list command now retracts the row the fresh <li> emitted about itself and reports the change under the paragraph's own label with its original text. * Make the reconnect test fail fast instead of stalling CI; per-test timeout On a slow Windows runner the poll's reconnect could spawn its own server before the test's replacement announced itself, and then wait forever for a batch sent elsewhere. The first reconnect now waits 3s, the test races the poll against a 30s timer, and every test gets a 60s timeout so a hang fails the file instead of the whole job. * agent-loop: bound the reconnect test's setup and instrument it for the Windows stall * agent-loop: bound child shutdown waits; log cleanup state * agent-loop: a signal-killed child counts as stopped; drop the debug markers * SKILL: background wait is Claude Code only; Codex and others stay on the poll in the foreground Codex returns a detached session id and nothing wakes the agent when that session finishes, so a Send sat unread until the user nudged. The skill and the AGENTS.md block now say which harness gets which wait, and how to cope with a shell tool that caps command duration. * SKILL: stay on the foreground poll through incoming messages; state the ended-turn limit plainly * setup: own the AGENTS.md block with markers and replace a legacy one A project set up months ago kept the instructions from that day forever, because setup skipped any AGENTS.md that already mentioned human-review. The block is now written between marker comments and rewritten on every setup; the unmarked block an older setup wrote is recognized by its heading and replaced; a project that wrote its own human-review guidance is left alone. * v0.7.0 * state: keep each page's pristine copy in its own file Every edit flush rewrote state.json in full, and it embedded the complete HTML of every page reviewed in the last month. The copy now lives in pristine/<key>.html, is written only when it changes, and is removed with its page. A state file that still embeds copies loads as before and moves them out on its next save. * Editor smoke tests under jsdom; fix ordinal snapshot missing the body Three tests boot the SDK against a jsdom document: block delete with undo, typing over a selection that spans blocks, and stable labels after a deletion. The last one caught that the load-order snapshot skipped the body element, so top-level paragraphs still renumbered.
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.
Problem
Every Enter-key handler that submits a comment treated a bare Enter as "submit," with no check for whether an IME composition was in progress. Japanese/CJK users pressing Enter to confirm kanji conversion had their in-progress comment submitted prematurely instead of just confirming the conversion.
This affected:
#composeTextinchrome-client.js)editCommentinchrome-client.js)chrome-client.js)sdk.js)Fix
Added a small
isImeCommitEnter(event)guard (event.isComposing || event.keyCode === 229) before each handler's submit action, so a composition-confirming Enter no longer triggerspreventDefault()or the submit action.Testing
npm test— all existing tests pass, no regressions