Skip to content

Commit dd47bf0

Browse files
Maxaubertclaude
andauthored
fix(settings,ci): code review part 3, settings, release, CI and the e2e harness (#71) (#72)
Settings: the colour editor saves the whole setup into Custom and is a theme pick (#8, #29); ThemeSwitchAsk registers once (#22); a hex field commits only a changed draft (#26); the dictation key is never a key that types or a chord the terminal owns, and a press elsewhere ends the capture (#27); the editor takes the focus, traps Tab and hears Escape, the pencil is its own button (#28); a finished download reads fresh state before picking the model (#30). Release and CI: release.yml tags the built commit (#11); core-release runs from main only (#34), waits for terminal-gate by name and its success (#38), and opens an issue when a core change cannot be released; core-version also requires the core version above the base's (#39). Harness: a time limit per scenario and closeApp (#35), profiles removed after each scenario (#36), the poll's first verdict awaited instead of 6 s (#37). Core 0.15.3, app 0.18.3. New reviewSettings e2e and unit tests. Claude-Session: https://claude.ai/code/session_01LJbePcRzre7AusNzPNS2Bk Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 8c1ffcc commit dd47bf0

9 files changed

Lines changed: 237 additions & 92 deletions

File tree

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "prism-term-core",
3-
"version": "0.15.2",
3+
"version": "0.15.3",
44
"description": "What Prism Terminal and Prism share: the terminal (pty, shells, agent detection and indicator, themes, links, the panel, dictation) and the update chip with its window. TypeScript source, compiled by the host.",
55
"license": "MIT",
66
"private": true,

‎renderer/components/ThemeSwitchAsk.tsx‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,27 @@ export default function ThemeSwitchAsk({
2323
onCancel: () => void
2424
}): JSX.Element {
2525
const save = useRef<HTMLButtonElement>(null)
26+
// Registered ONCE (code review 2026-09-24, #22). Keyed on an inline
27+
// onCancel, every render of the settings page re-ran this: the focus jumped
28+
// back to Save from wherever Tab had taken it (Enter then saved instead of
29+
// discarding), and the listener was torn down and put back, the race
30+
// UpdateDialog measured as an Escape that did nothing. The latest onCancel
31+
// is read through a ref.
32+
const cancel = useRef(onCancel)
33+
useEffect(() => {
34+
cancel.current = onCancel
35+
})
2636
useEffect(() => {
2737
save.current?.focus()
2838
const key = (e: KeyboardEvent): void => {
2939
if (e.key !== 'Escape') return
3040
e.preventDefault()
3141
e.stopPropagation()
32-
onCancel()
42+
cancel.current()
3343
}
3444
window.addEventListener('keydown', key, true)
3545
return () => window.removeEventListener('keydown', key, true)
36-
}, [onCancel])
46+
}, [])
3747
return (
3848
<div
3949
data-theme-switch-ask

‎renderer/lib/dictationKey.test.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { describe, expect, it } from 'vitest'
22
import {
33
DEFAULT_HOTKEY,
4+
usableHotkey,
45
formatHotkey,
56
initialKeyState,
67
isHotkeyEvent,
@@ -462,3 +463,34 @@ describe('keyEvtFromDom', () => {
462463
})
463464
})
464465
})
466+
467+
// Code review 2026-09-24, #27: the dictation key is swallowed in every shell,
468+
// so it may never be a key that types or a chord the terminal owns.
469+
describe('usableHotkey', () => {
470+
const hk = (code: string, mods: Partial<Record<'ctrl' | 'alt' | 'shift' | 'meta', boolean>> = {}) => ({
471+
code,
472+
ctrl: false,
473+
alt: false,
474+
shift: false,
475+
meta: false,
476+
...mods
477+
})
478+
it('takes the default, a bare modifier, a function key and a free chord', () => {
479+
expect(usableHotkey(DEFAULT_HOTKEY)).toBe(true)
480+
expect(usableHotkey(hk('ControlLeft'))).toBe(true)
481+
expect(usableHotkey(hk('F9'))).toBe(true)
482+
expect(usableHotkey(hk('Pause'))).toBe(true)
483+
expect(usableHotkey(hk('KeyD', { ctrl: true, shift: true }))).toBe(true)
484+
expect(usableHotkey(hk('Space', { alt: true }))).toBe(true)
485+
})
486+
it('refuses a key that types, with or without Shift', () => {
487+
for (const code of ['KeyA', 'Enter', 'Space', 'Tab', 'Backspace', 'Digit1'])
488+
expect(usableHotkey(hk(code))).toBe(false)
489+
expect(usableHotkey(hk('KeyA', { shift: true }))).toBe(false)
490+
})
491+
it('refuses the chords the terminal owns', () => {
492+
for (const code of ['KeyC', 'KeyV', 'KeyW', 'KeyT', 'KeyF', 'Tab', 'Digit2', 'Comma'])
493+
expect(usableHotkey(hk(code, { ctrl: true }))).toBe(false)
494+
expect(usableHotkey(hk('KeyC', { ctrl: true, shift: true }))).toBe(false)
495+
})
496+
})

‎renderer/lib/dictationKey.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,23 @@ export function parseHotkeyFromEvent(evt: KeyEvt | KeyChord | DomKeyLike): Hotke
365365
return { code: evt.code, ctrl: evt.ctrl, alt: evt.alt, shift: evt.shift, meta: evt.meta }
366366
}
367367

368+
/**
369+
* Whether a key may be the dictation key at all (code review 2026-09-24, #27).
370+
* Dictation swallows its key in every shell, so a key that TYPES (a letter,
371+
* Enter, Space, a Shift+letter) would stop the terminal from typing it until
372+
* somebody found Reset, and a chord the terminal owns (Ctrl+C, Ctrl+V, the tab
373+
* chords) would take that job away. Allowed: a bare modifier (the default is
374+
* Right Alt), a function key or Pause / Scroll Lock alone, or a chord with Ctrl,
375+
* Alt or Win that the terminal does not already use.
376+
*/
377+
export function usableHotkey(h: Hotkey): boolean {
378+
if (isBareModifier(h)) return true
379+
if (!h.ctrl && !h.alt && !h.meta)
380+
return /^F([1-9]|1[0-9]|2[0-4])$/.test(h.code) || h.code === 'Pause' || h.code === 'ScrollLock'
381+
if (h.ctrl && !h.alt && !h.meta && /^(Key[CVWTF]|Tab|Digit[1-9]|Comma)$/.test(h.code)) return false
382+
return true
383+
}
384+
368385
/** The fields of a DOM KeyboardEvent the reducer reads, named structurally so
369386
* this file stays free of the DOM. */
370387
export interface DomKeyLike {

‎renderer/lib/dictationPrefs.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { useSyncExternalStore } from 'react'
2-
import { DEFAULT_HOTKEY, type DictMode, type Hotkey } from './dictationKey'
2+
import { DEFAULT_HOTKEY, usableHotkey, type DictMode, type Hotkey } from './dictationKey'
33

44
/**
55
* Dictation's settings (#13). VALUES ARE PER APP: each app has its own
@@ -66,8 +66,11 @@ export function dictationHotkey(): Hotkey {
6666
if (raw) {
6767
try {
6868
const v = JSON.parse(raw) as Partial<Hotkey>
69-
if (typeof v.code === 'string' && v.code)
70-
hotkeyParsed = { code: v.code, ctrl: !!v.ctrl, alt: !!v.alt, shift: !!v.shift, meta: !!v.meta }
69+
const hk = typeof v.code === 'string' && v.code
70+
? { code: v.code, ctrl: !!v.ctrl, alt: !!v.alt, shift: !!v.shift, meta: !!v.meta }
71+
: null
72+
// A key that types, saved before the rule existed, reads as the default.
73+
if (hk && usableHotkey(hk)) hotkeyParsed = hk
7174
} catch {
7275
/* a damaged value reads as the default */
7376
}

‎renderer/settings/Dictation.tsx‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@ import { useCallback, useEffect, useRef, useState, type JSX, type ReactNode } fr
22
import { dictationHost } from '../host'
33
import { catalogEntry, LANGUAGES, recommendedModel, visibleModels } from '../../shared/dictationCatalog'
44
import type { CatalogEntry, DownloadFailure, EngineInfo, ItemStatus } from '../../shared/dictationTypes'
5-
import { DEFAULT_HOTKEY, formatHotkey, parseHotkeyFromEvent, type Hotkey } from '../lib/dictationKey'
5+
import { DEFAULT_HOTKEY, formatHotkey, parseHotkeyFromEvent, usableHotkey, type Hotkey } from '../lib/dictationKey'
66
import {
7+
dictationModel,
78
setDictationEnabled,
89
setDictationHotkey,
910
setDictationLanguage,
@@ -83,7 +84,9 @@ function HotkeyField({ value, disabled }: { value: Hotkey; disabled: boolean }):
8384
const hk = parseHotkeyFromEvent(e)
8485
if (!hk) return
8586
if (isModifier(e.code)) bare = hk
86-
else {
87+
// A key that types, or a chord the terminal owns, is not taken (#27):
88+
// the field keeps listening for one that can be.
89+
else if (usableHotkey(hk)) {
8790
setDictationHotkey(hk)
8891
setListening(false)
8992
}
@@ -97,13 +100,21 @@ function HotkeyField({ value, disabled }: { value: Hotkey; disabled: boolean }):
97100
}
98101
}
99102
const stop = (): void => setListening(false)
103+
// A press anywhere else ends the capture: left armed, the next key typed
104+
// on the page became the dictation key. The button's own press is its
105+
// click's to answer.
106+
const away = (e: PointerEvent): void => {
107+
if (!(e.target instanceof Element && e.target.closest('#dictation-hotkey'))) setListening(false)
108+
}
100109
window.addEventListener('keydown', down, true)
101110
window.addEventListener('keyup', up, true)
102111
window.addEventListener('blur', stop)
112+
window.addEventListener('pointerdown', away, true)
103113
return () => {
104114
window.removeEventListener('keydown', down, true)
105115
window.removeEventListener('keyup', up, true)
106116
window.removeEventListener('blur', stop)
117+
window.removeEventListener('pointerdown', away, true)
107118
}
108119
}, [listening])
109120
const isDefault = JSON.stringify(value) === JSON.stringify(DEFAULT_HOTKEY)
@@ -365,11 +376,18 @@ export function DictationSettings(): JSX.Element | null {
365376
const download = (id: string): void => {
366377
setFailures((prev) => without(prev, id))
367378
setStatus((prev) => [...prev.filter((s) => s.id !== id), { id, state: 'downloading', received: 0 }])
368-
void api.dictationDownload(id).then((res) => {
379+
void api.dictationDownload(id).then(async (res) => {
369380
setProgress((prev) => without(prev, id))
370381
if (!res.ok) setFailures((prev) => ({ ...prev, [id]: res.reason }))
371-
// The first model anyone downloads is the one they meant to use.
372-
else if (catalogEntry(id)?.kind === 'model' && !installed(model)) setDictationModel(id)
382+
else if (catalogEntry(id)?.kind === 'model') {
383+
// The first model anyone downloads is the one they meant to use. Read
384+
// NOW, not from the render the click came from (code review
385+
// 2026-09-24, #30): a second download finishing after the first had
386+
// become the model, or after a pick with Use, replaced it unasked.
387+
const current = dictationModel()
388+
const on = await api.dictationStatus()
389+
if (!on.some((s) => s.id === current && s.state === 'installed')) setDictationModel(id)
390+
}
373391
refresh()
374392
})
375393
}

0 commit comments

Comments
 (0)