Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 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
59 changes: 59 additions & 0 deletions components/terminal/runtime/createXTermRuntime.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1029,3 +1029,62 @@ 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("Command+Period 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, /\{ \.\.\.aliasedRelease\.event, type: "keyup" \},\s*aliasedRelease\.identity/);
assert.match(source, /releaseForwardedKittyPress\(toKittyKeyboardEvent\(releaseEvent\)\) \|\| releasedInterrupt/);
// 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 \}/,
);
// Native keyups retain the physical event even after releasing an alias.
assert.match(source, /const hasForwardedWin32KeyDown = win32InputModeForwardedKeys\.delete\(identity\);/);

});

test("Command+Period 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 Command+Period (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*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\);/,
);
});
160 changes: 143 additions & 17 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>();
// Command+Period 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 Command+Period 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
// Command+Period 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 Command+Period 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 @@ -1770,6 +1812,15 @@ export const createXTermRuntime = (ctx: CreateXTermRuntimeContext): XTermRuntime
releaseForwardedKittyPress({ ...deferredKittyEvent, type: "keyup" });
}
}
// Release the normalized interrupt separately: the same physical key
// may already have a forwarded press from before Command was held.
// Keep the saved layout-independent event rather than translating KeyC
// through the current keyboard layout again.
const aliasedRelease = resolveKittyNormalizedPressRelease(e);
const releasedInterrupt = aliasedRelease !== null && releaseForwardedKittyPress(
{ ...aliasedRelease.event, type: "keyup" },
aliasedRelease.identity,
);
const identity = kittyKeyIdentity(releaseEvent);
const hasForwardedWin32KeyDown = win32InputModeForwardedKeys.delete(identity);
if (broadcastLegacyDataPending === identity) clearBroadcastLegacyDataPending();
Expand All @@ -1792,7 +1843,7 @@ export const createXTermRuntime = (ctx: CreateXTermRuntimeContext): XTermRuntime
};
return true;
}
if (releaseForwardedKittyPress(toKittyKeyboardEvent(releaseEvent))) {
if (releaseForwardedKittyPress(toKittyKeyboardEvent(releaseEvent)) || releasedInterrupt) {
e.preventDefault();
e.stopPropagation();
return false;
Expand Down Expand Up @@ -1981,10 +2032,27 @@ 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: Command+Period interrupts the running command like
// Ctrl+C (#3408), including while text is selected. A
// user-assigned snippet or configured shortcut on this chord keeps
// precedence: the editors accept Command+Period (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 =
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 thread
binaricat marked this conversation as resolved.
const id = ctx.sessionRef.current;
if (id && ctx.statusRef.current === "connected") {
const rendererKeyAt = Date.now();
Expand Down Expand Up @@ -2035,32 +2103,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
// Command+Period 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 thread
binaricat marked this conversation as resolved.
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);
Comment thread
binaricat marked this conversation as resolved.
// 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;
Comment thread
binaricat marked this conversation as resolved.
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,
});
Comment thread
binaricat marked this conversation as resolved.
if (forwarded) {
upsertKittyKeyboardForwardedPress(
broadcastForwardedKeys,
identity,
pressIdentity,
kittyEvent,
forwarded.targetSessionIds,
);
broadcastKittyInput({
kind: "legacy",
data: "\x03",
keyIdentity: identity,
keyIdentity: pressIdentity,
urgentInterrupt: true,
});
}
Expand All @@ -2069,10 +2189,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 +2595,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
77 changes: 77 additions & 0 deletions components/terminal/runtime/kittyKeyboardBroadcast.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1021,3 +1021,80 @@ test("a key pressed while disconnected cannot produce an orphan release after re
});
assert.deepEqual(writes, []);
});

test("a dedicated keyIdentity pairs the interrupt press with its release (#3409)", () => {
const options = () => {
// Disambiguate + event-type flags so Ctrl+C press/release encode as CSI-u.
const mode = createKittyKeyboardModeState();
setKittyKeyboardModeFlags(mode, 1 | 2);
return {
kittyProtocolEnabled: true,
kittyMode: mode,
applicationCursorMode: false,
encodedKeys: new Set<string>(),
legacySuppressedKeys: new Set<string>(),
};
};

// A Command+Period interrupt is normalized to a Ctrl+C event but paired under a
// dedicated identity, so it must not collapse with an outstanding physical
// KeyC press on the peer: the press, the legacy suppression and the release
// all use the propagated identity, while the physical KeyC state survives.
const pressOptions = options();
const interruptPress = resolveKittyKeyboardBroadcastInput({
kind: "key",
event: { type: "keydown", key: "c", code: "KeyC", ctrlKey: true },
keyIdentity: "KeyC mac-period-interrupt",
urgentInterrupt: true,
}, pressOptions);
assert.equal(interruptPress?.data, "\x1b[99;5u");
assert.ok(pressOptions.encodedKeys.has("KeyC mac-period-interrupt"));
assert.ok(pressOptions.legacySuppressedKeys.has("KeyC mac-period-interrupt"));

// The physical KeyC press keeps its own entry.
assert.equal(pressOptions.encodedKeys.has("KeyC"), false);

// The legacy \x03 fan-out under the dedicated identity is suppressed on a
// Kitty peer (already encoded) but delivered on a legacy peer.
assert.equal(resolveKittyKeyboardBroadcastInput({
kind: "legacy",
data: "\x03",
keyIdentity: "KeyC mac-period-interrupt",
urgentInterrupt: true,
}, pressOptions), null);

const legacyPeerOptions = options();
const legacyInterrupt = resolveKittyKeyboardBroadcastInput({
kind: "legacy",
data: "\x03",
keyIdentity: "KeyC mac-period-interrupt",
urgentInterrupt: true,
}, legacyPeerOptions);
assert.equal(legacyInterrupt?.data, "\x03");
assert.equal(legacyInterrupt?.urgentInterrupt, true);

// The release pairs under the dedicated identity, leaving the physical
// KeyC press entry intact for its own keyup.
const releaseOptions = options();
releaseOptions.encodedKeys.add("KeyC mac-period-interrupt");
releaseOptions.encodedKeys.add("KeyC");
releaseOptions.legacySuppressedKeys.add("KeyC");
const interruptRelease = resolveKittyKeyboardBroadcastInput({
kind: "key",
event: { type: "keyup", key: "c", code: "KeyC", ctrlKey: true },
keyIdentity: "KeyC mac-period-interrupt",
}, releaseOptions);
assert.equal(interruptRelease?.data, "\x1b[99;5:3u");
assert.ok(releaseOptions.encodedKeys.has("KeyC"));
assert.equal(releaseOptions.encodedKeys.has("KeyC mac-period-interrupt"), false);

// Without an explicit identity, pairing still keys from the event code.
const defaultOptions = options();
defaultOptions.encodedKeys.add("KeyA");
const defaultRelease = resolveKittyKeyboardBroadcastInput({
kind: "key",
event: { type: "keyup", key: "a", code: "KeyA" },
}, defaultOptions);
assert.equal(defaultRelease?.data, "\x1b[97;1:3u");
assert.equal(defaultOptions.encodedKeys.has("KeyA"), false);
});
Loading
Loading