Skip to content

fix(web): the publish-repository path no longer submits on an IME composition Enter - #28277

Open
only21mil wants to merge 1 commit into
pingdotgg:mainfrom
only21mil:fix/audit-m008
Open

only21mil wants to merge 1 commit into
pingdotgg:mainfrom
only21mil:fix/audit-m008

Conversation

@only21mil

@only21mil only21mil commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Problem

On macOS, pressing Return to confirm a Japanese or Chinese IME candidate in the Publish repository path publishes right away with a half-typed name. Safari, Chromium and the desktop app report that Return as key: "Enter", and the path input submits on every Enter. The pull request thread dialog and the sidebar rename (#6280, fixed in #6281) already ignore it.

Change

The path input's Enter handler returns early when event.nativeEvent.isComposing is true or keyCode is 229, the guard PullRequestThreadDialog and the sidebar rename use. Plain Enter still publishes. PublishRepositoryDialog is exported so the test can render it alone.

Scope and approval

A very small fix for an obvious bug, under the CONTRIBUTING exception for focused bug fixes. Nothing changes outside composition input.

Real run on a Mac

macOS 27.0.1 (Apple silicon), Apple's Japanese (Romaji) input source, real key presses through the OS. I typed nihon in the path and pressed Return once while the kanji candidate showed. A stub gh logs calls and refuses credentials, so a publish attempt shows "Publish failed" and nothing reaches GitHub. Main is the base 15fe283272; head is 8b3dba13fa.

Client Confirming Return, as logged Main Head
Brave 157 Enter, 229, isComposing: true, before compositionend publishes demo-user/にほn no publish
Desktop app same as Brave publishes demo-user/にほn no publish
Safari 27.0.1 Enter, 229, isComposing: false, after compositionend publishes demo-user/にほん no publish
Firefox 160 Process, 229, isComposing: true no publish no publish
  • Safari's Return comes after compositionend, so only the keyCode === 229 half catches it.
  • Pinyin behaves the same: in Brave, main published demo-user/ni hao; the head did not.
  • Korean 2-Set commits the syllable, then sends a plain Enter (13), so both builds publish the full demo-user/한.
  • On the head, a plain Return still publishes in Brave and Safari.

Verification

  • vp test run src/components/GitActionsControl.composition.browser.test.tsx: fails on main (publish.run called on the composing Enter), passes with the fix.
  • Negative control: with only the isComposing half, the keyCode: 229 row fails.
  • vp run check:changed passes.

Main, Safari, before Return: にほn is still composing.

Main, Safari: にほn still composing in the repository path

Main, after one Return: Safari, Brave and the desktop app each publish and show "Publish failed".

Main, Safari: Publish failed with demo-user/にほん

Main, Brave: Publish failed with demo-user/にほn

Main, desktop app: Publish failed with demo-user/にほn

Head, after the same Return: the text commits and the dialog stays on the Repository step.

Head, Brave: no publish

Head, Safari: no publish

Head, desktop app: no publish

Event order for the same keys on a plain input, in Safari, Brave and Firefox:

Safari: compositionend, then Enter with keyCode 229

Brave: Enter with isComposing true, then compositionend

Firefox: the confirming key is Process

Not checked:

  • Windows (Microsoft IME) and Linux (IBus or fcitx).
  • Mobile. Its publish form submits through the native return key (onSubmitEditing), so this guard has no mobile counterpart.

Prior PRs

Generated with Claude Opus 5.5 and GPT-6.1 Terra in T3 Code Nightly

…position Enter

Pressing Enter to confirm an IME candidate in the publish wizard's repository path published the repository mid-typing. The keydown handler now ignores composition Enter (isComposing or keyCode 229), matching the other inputs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Oct 31, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 31, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 8b3dba1

Macroscope's review found this PR approvable — The repository-path field now ignores Enter during IME composition while preserving normal Enter-to-publish behavior. Existing publishing safeguards and visibility defaults remain unchanged.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Walkthrough

The repository publish dialog is now exported. Enter events during IME composition do not submit or prevent the default. A browser test checks composing and regular Enter events.

Changes: Repository publishing

Layer / File(s) Summary
Composition-aware Enter handling
apps/web/src/components/GitActionsControl.tsx, apps/web/src/components/GitActionsControl.composition.browser.test.tsx
The dialog ignores Enter events marked as composing or with key code 229. Other Enter events prevent the default and submit. The browser test checks both cases.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested labels: bugfix, size:XS

Suggested reviewers: juliusmarminge, macroscopeapp

Merge Risk: Minimal · up to 8b3db

No actionable merge-blocking risk was found.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title clearly and concisely describes the main fix: preventing the publish-repository path from submitting when Enter completes IME composition.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description check Passed The description covers the problem, change, scope rationale, and focused verification. It includes before-and-after screenshots and states what was not checked. The scope section explains why this sma…
Full details: Description check

Explanation

The description covers the problem, change, scope rationale, and focused verification. It includes before-and-after screenshots and states what was not checked. The scope section explains why this small fix qualifies for the focused-bug exception.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 15fe283 and 8b3dba1.

Files selected for processing: 2

  • apps/web/src/components/GitActionsControl.composition.browser.test.tsx
  • apps/web/src/components/GitActionsControl.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

@maria-rcks

maria-rcks commented Nov 1, 2026 •

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

verified as-is at 8b3dba13fa3460f03788b1ddfa7847d52eb65350 on 2026-11-01. used the repository playwright dependency with /usr/bin/google-chrome, real dev servers, app-created projects and real codex gpt-6.1-sol turns. no mocks, fixtures or demo data.

  • reproduced on github/main (955e2bfbb5) in a separate worktree. chrome Input.imeSetComposition followed by keyboard enter generated a trusted isComposing: true keydown. it prematurely published a private github repository before composition ended.
  • on the exact pr head, the same repository text and trusted composing enter stayed in the repository field. no git remote was created. a separate synthetic keyCode: 229 enter also did not submit or prevent default.
  • ordinary enter reached github. an existing repository produced the expected error; an unused name successfully published over https. github and local head both equal c18ab21f4f70f2f5577e89c6f4f24d07108f7dcf, and github confirms private visibility.
  • empty-path enter kept publish disabled. back / close / reopen cleared the error and retained private visibility. reloading after successful publication showed the commit action.
  • blacksmith passed the composition browser test and git actions logic tests: 14 tests across 2 files. pr ci reports 31 successful checks, zero pending or failing. no source changes or pushes were needed.

before: composing enter prematurely published.

main prematurely publishes on composition enter

after: the same text stays editable during composition.

pr preserves the composition in the repository field

ordinary enter successfully publishes to github

composition guard, normal enter, error reset and successful publication

short runtime transcript

Not verified

native desktop/mobile, macos/windows, safari and an operating-system ime candidate picker. the chromium composition event was trusted and browser-generated; the safari-style 229 fallback was synthetic. ssh and other source control providers were not exercised in this run. this checks the shared web dialog through github over https.

@only21mil

Copy link
Copy Markdown
Contributor Author

Note

🤖 Opus 5.5 replying on behalf of Victor.

@maria-rcks thanks for the thorough run; we covered your macOS, Safari and OS candidate-picker items on a real Mac with Apple's Japanese (Romaji) input source and real key presses through the OS, in Brave, Safari, Firefox and the desktop app. On main, the Return that confirms the candidate publishes in Brave, Safari and the desktop app; on this head it does not, a plain Return still publishes, and Pinyin behaves the same (Safari sends that Return after compositionend, so the keyCode === 229 half is what catches it). Firefox reports the key as Process and Korean 2-Set ends with a plain Enter, so neither changes; Windows and Linux input methods are still unchecked, and the description now carries the captures.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants