Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions components/terminal/runtime/createXTermRuntime.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1029,3 +1029,73 @@ test("multi-character plain text goes out as per-character writes (#3077)", asyn
assert.match(writeSite, /ctx\.onOutputTriggerUserInputRef\?\.current\?\.\(outData\)/);
assert.match(source, /onBroadcastInput\?\.\(broadcastData, ctx\.sessionId\)/);
});

test("⌘. interrupt press is keyed apart from an outstanding physical KeyC press (#3409)", async () => {
const { readFileSync } = await import("node:fs");
const source = readFileSync(new URL("./createXTermRuntime.ts", import.meta.url), "utf8");

// The normalized Ctrl+C press shares its event identity with a possibly
// outstanding physical KeyC press, so it must be recorded under a dedicated
// map key: upserting under "KeyC" replaced the held key's press and the
// Period keyup deleted that shared entry while C was still down.
assert.match(
source,
/const pressIdentity =\s*macCommandPeriodInterrupt && identity !== kittyKeyIdentity\(e\)\s*\?\s*kittyNormalizedPressIdentity\(identity\)\s*:\s*identity,?/,
);
assert.match(source, /kittyNormalizedPressAliases\.set\(kittyKeyIdentity\(e\), pressIdentity\)/);
assert.match(
source,
/upsertKittyKeyboardForwardedPress\(\s*kittyForwardedKeys,\s*pressIdentity,/,
);
assert.match(
source,
/upsertKittyKeyboardForwardedPress\(\s*broadcastForwardedKeys,\s*pressIdentity,/,
);
// The aliased Period keyup releases the interrupt press under its dedicated
// key, leaving the physical KeyC press entry intact.
assert.match(source, /aliasedReleaseIdentity = aliasedRelease\.identity/);
assert.match(
source,
/releaseForwardedKittyPress\(toKittyKeyboardEvent\(releaseEvent\), aliasedReleaseIdentity\)/,
);
// The dedicated identity crosses the broadcast boundary: peers key their
// pairing state from it, so the interrupt cannot collapse with an
// outstanding physical KeyC press on legacy or Kitty peers (#3409).
assert.match(
source,
/broadcastKittyInput\(\{\s*kind: "key",\s*event: kittyEvent,\s*keyIdentity: pressIdentity,\s*\}\)/,
);
assert.match(
source,
/broadcastKittyInput\(\{\s*kind: "legacy",\s*data: "\\x03",\s*keyIdentity: pressIdentity,/,
);
// The paired release carries the identity the press was recorded under.
assert.match(
source,
/\{ kind: "key", event, keyIdentity: identity \}/,
);
// The aliased release's Win32 lookup uses the physical key's identity so it
// neither consumes the held KeyC's native pairing nor leaks a native Ctrl+C
// keyup for a keydown ConPTY never received (#3409).
assert.match(
source,
/const win32LookupIdentity =\s*aliasedReleaseIdentity !== undefined \? physicalIdentity : identity;/,
);
assert.match(
source,
/const hasForwardedWin32KeyDown = win32InputModeForwardedKeys\.delete\(win32LookupIdentity\);/,
);
});

test("⌘. interrupt yields to a user-assigned snippet or shortcut chord (#3409)", async () => {
const { readFileSync } = await import("node:fs");
const source = readFileSync(new URL("./createXTermRuntime.ts", import.meta.url), "utf8");

// The snippet/app-shortcut editors accept ⌘. (their conflict checks only
// cover configured bindings), so the hard-coded interrupt must give a
// configured chord precedence instead of silently swallowing it (#3409).
assert.match(
source,
/const macCommandPeriodInterrupt =\s*!hasCopyableSelection\s*&& isMacPlatform\(\)\s*&& isMacCommandPeriodInterruptChord\(e\)\s*&& !\(ctx\.snippetsRef\?\.current \?\? \[\]\)\.some\(\(snippet\) => \(\s*snippet\.shortkey && matchesKeyBinding\(e, snippet\.shortkey, isMac\)\s*\)\)\s*&& !\(currentScheme !== "disabled"\s*&& checkAppShortcut\(e, ctx\.keyBindingsRef\.current, isMac\) !== null\);/,
);
});
181 changes: 161 additions & 20 deletions components/terminal/runtime/createXTermRuntime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,10 @@ import {
type TerminalOutputHistoryPreview,
} from "./terminalOutputHistory";
import { shouldPassThroughCopyShortcut } from "./terminalCopyShortcut";
import { shouldUseUrgentTerminalInterrupt } from "./terminalInterruptShortcut";
import {
isMacCommandPeriodInterruptChord,
shouldUseUrgentTerminalInterrupt,
} from "./terminalInterruptShortcut";
import {
createTerminalInterruptTrace,
logTerminalInterruptTrace,
Expand Down Expand Up @@ -1486,6 +1489,14 @@ export const createXTermRuntime = (ctx: CreateXTermRuntimeContext): XTermRuntime
const kittyForwardedKeys = new Map<string, KittyKeyboardForwardedPress>();
const broadcastForwardedKeys = new Map<string, KittyKeyboardForwardedPress>();
const win32BroadcastForwardedKeys = new Map<string, KittyKeyboardForwardedPress>();
// ⌘. interrupt presses are recorded under their normalized Ctrl+C identity
// (KeyC) so broadcast legacy pairing stays matched (#3408), but under a
// dedicated map key so they cannot clobber an outstanding physical KeyC
// press; map the physical chord identity (Period) to it so the later keyup
// can pair the release.
const kittyNormalizedPressAliases = new Map<string, string>();
const kittyNormalizedPressIdentity = (identity: string): string =>
`${identity}\u0000mac-period-interrupt`;
const broadcastEncodedKeys = new Set<string>();
const broadcastLegacySuppressedKeys = new Set<string>();
const kittyKeyIdentity = (event: KeyboardEvent): string => event.code || event.key;
Expand Down Expand Up @@ -1671,13 +1682,20 @@ export const createXTermRuntime = (ctx: CreateXTermRuntimeContext): XTermRuntime
*/
const releaseForwardedKittyPress = (
event: Pick<KittyKeyboardEvent, "code" | "key"> & KittyKeyboardEvent,
identityOverride?: string,
): boolean => {
const identity = event.code || event.key;
// The ⌘. interrupt press is keyed independently from its normalized
// Ctrl+C event identity, so its release must delete the entry it was
// stored under rather than the physical key's (#3408).
const identity = identityOverride ?? (event.code || event.key);
const forwardedPress = broadcastForwardedKeys.get(identity);
if (forwardedPress) {
broadcastForwardedKeys.delete(identity);
broadcastKittyInput(
{ kind: "key", event },
// Carry the identity the press was recorded under so peers pair this
// release with that press instead of the event's physical code — the
// ⌘. interrupt press lives under a dedicated normalized key (#3409).
{ kind: "key", event, keyIdentity: identity },
true,
forwardedPress.targetSessionIds,
);
Expand All @@ -1693,6 +1711,30 @@ export const createXTermRuntime = (ctx: CreateXTermRuntimeContext): XTermRuntime
}
return false;
};
/**
* Resolve a forwarded press whose recorded identity is not the physical
* key identity: the ⌘. interrupt press was recorded as the normalized
* Ctrl+C event (KeyC), but the browser delivers the physical release as
* Period. Pair that release from the stored event so Kitty consumers do
* not see Ctrl+C held until focus loss (#3408).
*/
const resolveKittyNormalizedPressRelease = (
physicalEvent: KeyboardEvent,
): { event: KittyKeyboardEvent; identity: string } | null => {
const physicalIdentity = kittyKeyIdentity(physicalEvent);
const normalizedIdentity = kittyNormalizedPressAliases.get(physicalIdentity);
if (!normalizedIdentity) return null;
const forwardedPress =
broadcastForwardedKeys.get(normalizedIdentity)
?? kittyForwardedKeys.get(normalizedIdentity);
// Consume the alias on every path once it has been paired (or
// invalidated): a later keyup for the same physical key while another
// KeyC press is outstanding must not be rewritten as the normalized
// release (#3408).
kittyNormalizedPressAliases.delete(physicalIdentity);
if (!forwardedPress) return null;
return { event: forwardedPress.event, identity: normalizedIdentity };
};

term.attachCustomKeyEventHandler((e: KeyboardEvent) => {
// Preserve mouse selection across keystrokes when enabled. xterm.js
Expand Down Expand Up @@ -1744,6 +1786,7 @@ export const createXTermRuntime = (ctx: CreateXTermRuntimeContext): XTermRuntime
// key they consumed (or drop the keyup), and an exact key match left the
// deferral armed so every later keypress was swallowed (#3103).
let releaseEvent: KeyboardEvent = e;
let aliasedReleaseIdentity: string | undefined;
if (
imeTextInputDeferredKey !== null &&
shouldFlushDeferredImeTextInputOnKeyUp(imeTextInputDeferredKey, e)
Expand All @@ -1770,14 +1813,36 @@ export const createXTermRuntime = (ctx: CreateXTermRuntimeContext): XTermRuntime
releaseForwardedKittyPress({ ...deferredKittyEvent, type: "keyup" });
}
}
// A ⌘. interrupt press was recorded under its normalized Ctrl+C
// identity; pair the physical Period release from the stored event
// instead of leaving the press unmatched (#3408). The release must
// delete the press under its dedicated key so an outstanding physical
// KeyC press is left intact (#3409).
const aliasedRelease = resolveKittyNormalizedPressRelease(e);
if (aliasedRelease) {
releaseEvent = {
...aliasedRelease.event,
type: "keyup",
} as unknown as KeyboardEvent;
aliasedReleaseIdentity = aliasedRelease.identity;
Comment on lines +1823 to +1827

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep aliased releases out of physical Win32 tracking

When the source terminal has negotiated Win32 input mode, broadcasting is enabled, and a physical KeyC remains held while Command-period is pressed, the Period keyup is rewritten here to the normalized KeyC event. The subsequent win32InputModeForwardedKeys.delete(identity) therefore consumes the outstanding physical C press and returns true, causing a premature native C release while the eventual real C keyup is dropped. Fresh evidence after the source-map alias fix is that the separate win32InputModeForwardedKeys lookup still uses the rewritten identity; preserve the physical e identity for that lookup and suppress the aliased interrupt's native keyup.

Useful? React with 👍 / 👎.

Comment on lines +1821 to +1827

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve a held Period press when normalizing the interrupt

If Period is already held and auto-repeating when Command is pressed, its original keydown has been forwarded under Period, while the repeated keydown creates this normalized interrupt alias. The eventual physical keyup is then replaced exclusively with the Ctrl+C release: Kitty and broadcast paths leave the original Period press pending until blur, and Win32 deletes the Period entry but emits a Ctrl+C keyup instead, leaving ConPTY's Period logically held. The aliased keyup must release both the pre-existing physical press and the normalized interrupt press, or repeated keydowns must not create the alias in this situation.

Useful? React with 👍 / 👎.

}
// The aliased ⌘. release must be looked up under the physical key's
// identity: the interrupt keydown was consumed by the shortcut and
// never handed to ConPTY, so the rewritten KeyC identity must not
// consume an outstanding physical KeyC press's Win32 entry (which
// would release C natively and drop the real C keyup later), and the
// aliased interrupt release must stay orphaned (#3409).
const physicalIdentity = kittyKeyIdentity(e);
const identity = kittyKeyIdentity(releaseEvent);
const hasForwardedWin32KeyDown = win32InputModeForwardedKeys.delete(identity);
const win32LookupIdentity =
aliasedReleaseIdentity !== undefined ? physicalIdentity : identity;
const hasForwardedWin32KeyDown = win32InputModeForwardedKeys.delete(win32LookupIdentity);
if (broadcastLegacyDataPending === identity) clearBroadcastLegacyDataPending();
if (term.modes.win32InputMode) {
// Broadcast peers may still need a paired Kitty release for a keydown
// consumed by a Netcatty action (notably the urgent Ctrl+C path).
releaseForwardedKittyPress(toKittyKeyboardEvent(releaseEvent));
kittyForwardedKeys.delete(identity);
releaseForwardedKittyPress(toKittyKeyboardEvent(releaseEvent), aliasedReleaseIdentity);
kittyForwardedKeys.delete(win32LookupIdentity);
// Only let xterm emit a Win32 key-up when its matching keydown was
// previously handed to xterm. Netcatty shortcuts, sudo controls and
// autocomplete consume their keydown and must not leak an orphaned
Expand All @@ -1792,7 +1857,7 @@ export const createXTermRuntime = (ctx: CreateXTermRuntimeContext): XTermRuntime
};
return true;
}
if (releaseForwardedKittyPress(toKittyKeyboardEvent(releaseEvent))) {
if (releaseForwardedKittyPress(toKittyKeyboardEvent(releaseEvent), aliasedReleaseIdentity)) {
e.preventDefault();
e.stopPropagation();
return false;
Expand Down Expand Up @@ -1981,10 +2046,28 @@ export const createXTermRuntime = (ctx: CreateXTermRuntimeContext): XTermRuntime
kittyKeyboardProtocolEnabled
? encodeKittyKeyEvent(kittyKeyboardMode, toKittyKeyboardEvent(e))
: null;
if (
const urgentInterrupt =
(!kittySequenceForKeyDown || kittySequenceForKeyDown === "\x03") &&
shouldUseUrgentTerminalInterrupt(e, { hasSelection: hasCopyableSelection })
) {
shouldUseUrgentTerminalInterrupt(e, { hasSelection: hasCopyableSelection });
const currentScheme = ctx.hotkeySchemeRef.current;
// Use shared utility for platform detection when hotkey scheme is disabled
const isMac = currentScheme === "mac" || (currentScheme === "disabled" && isMacPlatform());
// macOS Terminal convention: ⌘. interrupts the running command like
// Ctrl+C (#3408). Only when nothing is selected so copy wins first. A
// user-assigned snippet or configured shortcut on this chord keeps
// precedence: the editors accept ⌘. (their conflict check only covers
// configured bindings), so the hard-coded interrupt must not silently
// swallow a chord the user actually assigned (#3409).
const macCommandPeriodInterrupt =
!hasCopyableSelection
&& isMacPlatform()
&& isMacCommandPeriodInterruptChord(e)
&& !(ctx.snippetsRef?.current ?? []).some((snippet) => (
snippet.shortkey && matchesKeyBinding(e, snippet.shortkey, isMac)
))
&& !(currentScheme !== "disabled"
&& checkAppShortcut(e, ctx.keyBindingsRef.current, isMac) !== null);
if (urgentInterrupt || macCommandPeriodInterrupt) {
Comment on lines +2061 to +2070

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reserve Command-period before accepting custom shortcuts

On macOS, a user can still assign Meta+. to a snippet or configurable terminal action, but this branch executes before both the snippet loop and checkAppShortcut() and returns after sending an interrupt. The snippet editors accept this chord because findActiveSystemShortcutConflict() only checks configured key bindings and this new hard-coded shortcut was not registered there, so the UI can save a shortcut that silently becomes unusable; either reject/reserve this chord during validation or give configured shortcuts precedence.

Useful? React with 👍 / 👎.

const id = ctx.sessionRef.current;
if (id && ctx.statusRef.current === "connected") {
const rendererKeyAt = Date.now();
Expand Down Expand Up @@ -2035,32 +2118,84 @@ export const createXTermRuntime = (ctx: CreateXTermRuntimeContext): XTermRuntime
} else {
ctx.terminalBackend.writeToSession(id, "\x03");
}
const kittyEvent = toKittyKeyboardEvent(e);
const identity = kittyKeyIdentity(e);
// Report the interrupt to Kitty as Ctrl+C even when it came from the
// ⌘. chord: the broadcast legacy \x03 is keyed by this identity, so
// forwarding Super+Period would leave peers with an unmatched
// Super+Period press and suppress the interrupt instead (#3408).
const interruptEventForKitty: KeyboardEvent = macCommandPeriodInterrupt
? {
type: e.type,
key: "c",
code: "KeyC",
Comment on lines +2128 to +2129

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid layout lookup for synthesized Ctrl+C

When macOS uses a non-QWERTY layout (for example, Dvorak), assigning the synthesized event code: "KeyC" makes toKittyKeyboardEvent() obtain that physical key's layout character and getUnicodeKeyCode() prioritizes it. The normalized interrupt can therefore encode as Ctrl+J (or another layout character) instead of Ctrl+C; a Kitty-enabled broadcast peer then suppresses the legacy \x03 fallback and is not interrupted. Construct the normalized Kitty event without unrelated layout metadata, or explicitly force its unshifted character to c.

Useful? React with 👍 / 👎.

location: e.location,
repeat: e.repeat,
isComposing: e.isComposing,
keyCode: 67,
shiftKey: false,
altKey: false,
ctrlKey: true,
metaKey: false,
getModifierState: (key: string) => key === "Control" && e.getModifierState("Control"),
} as unknown as KeyboardEvent
: e;
const kittyEvent = toKittyKeyboardEvent(interruptEventForKitty);
if (macCommandPeriodInterrupt) {
// The synthesized event's code ("KeyC") is the physical QWERTY
// position of the chord key, so toKittyKeyboardEvent()'s layout
// lookup returns that position's character on non-QWERTY layouts
// (e.g. "n" under Dvorak) and getUnicodeKeyCode() prioritizes it,
// encoding the interrupt as the wrong key instead of Ctrl+C's 99
// — a broadcast peer would then suppress the legacy \x03 fallback
// and never be interrupted. Force the layout-independent identity.
kittyEvent.unshiftedKey = "c";
}
const identity = kittyKeyIdentity(interruptEventForKitty);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Pair the normalized Ctrl+C press with the Period keyup

When Kitty event reporting is enabled locally or on a broadcast peer, this records the normalized press under KeyC, but the browser later delivers the physical release as Period; the keyup handler looks up only e.code || e.key, so it neither removes this entry nor sends the paired Ctrl+C release. Although the prior Super+Period mismatch is now normalized, the new identity mismatch leaves Ctrl+C logically held for Kitty consumers; retain a mapping from the physical chord to the normalized event or synthesize its release.

Useful? React with 👍 / 👎.

// The normalized press shares the Ctrl+C event identity (KeyC) with a
// possibly outstanding physical KeyC press; record it under a dedicated
// map key so the Period keyup releases (and deletes) only the
// interrupt press instead of the held physical key's entry (#3409).
const pressIdentity =
macCommandPeriodInterrupt && identity !== kittyKeyIdentity(e)
? kittyNormalizedPressIdentity(identity)
: identity;
if (pressIdentity !== identity) {
// The physical release will arrive under the Period identity while
// the press was recorded as the normalized Ctrl+C event; pair them
// at keyup so the interrupt release is not lost (#3408).
kittyNormalizedPressAliases.set(kittyKeyIdentity(e), pressIdentity);
}
if (
!term.modes.win32InputMode &&
kittyKeyboardProtocolEnabled &&
shouldTrackKittyKeyRelease(kittyKeyboardMode, kittyEvent)
) {
upsertKittyKeyboardForwardedPress(
kittyForwardedKeys,
identity,
pressIdentity,
kittyEvent,
[],
);
}
const forwarded = broadcastKittyInput({ kind: "key", event: kittyEvent });
// The dedicated press identity must cross the broadcast boundary:
// peers key their pairing state from it, so broadcasting only the
// normalized event would collapse the interrupt with an outstanding
// physical KeyC press on every peer (#3409).
const forwarded = broadcastKittyInput({
kind: "key",
event: kittyEvent,
keyIdentity: pressIdentity,
});
if (forwarded) {
upsertKittyKeyboardForwardedPress(
broadcastForwardedKeys,
identity,
pressIdentity,
kittyEvent,
forwarded.targetSessionIds,
);
broadcastKittyInput({
kind: "legacy",
data: "\x03",
keyIdentity: identity,
keyIdentity: pressIdentity,
urgentInterrupt: true,
});
}
Expand All @@ -2069,10 +2204,6 @@ export const createXTermRuntime = (ctx: CreateXTermRuntimeContext): XTermRuntime
}
}

const currentScheme = ctx.hotkeySchemeRef.current;
// Use shared utility for platform detection when hotkey scheme is disabled
const isMac = currentScheme === "mac" || (currentScheme === "disabled" && isMacPlatform());

// Check snippet shortcuts first (even if hotkeys are disabled)
const snippets = ctx.snippetsRef?.current;
if (snippets && snippets.length > 0) {
Expand Down Expand Up @@ -2479,6 +2610,16 @@ export const createXTermRuntime = (ctx: CreateXTermRuntimeContext): XTermRuntime
win32InputModePendingEvent = null;
win32InputModeForwardedKeys.clear();
kittyForwardedKeys.clear();
// broadcastForwardedKeys is retained so pending peer releases still pair
// after a reconnect; keep the aliases whose normalized press is still
// owed a broadcast release, otherwise the physical keyup can no longer
// find the dedicated identity and peers keep the key logically pressed
// until blur (#3409).
for (const [physicalIdentity, normalizedIdentity] of kittyNormalizedPressAliases) {
if (!broadcastForwardedKeys.has(normalizedIdentity)) {
kittyNormalizedPressAliases.delete(physicalIdentity);
}
}
clearKittyKeyboardBroadcastPairingState(
broadcastEncodedKeys,
broadcastLegacySuppressedKeys,
Expand Down
Loading