Skip to content

Commit 5489573

Browse files
committed
fix(cli): restore stdin to original state after OSC theme detection
Track whether stdin was already flowing before OSC detection resumed it, and only pause stdin in cleanup if we were the ones who resumed it. This prevents leaving stdin paused when the CLI or libraries like readline/ink had already resumed it before detection ran.
1 parent 8290dea commit 5489573

File tree

2 files changed

+99
-68
lines changed

2 files changed

+99
-68
lines changed

cli/src/hooks/use-theme.tsx

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,6 @@ export function initializeThemeStore() {
9393

9494
setThemeResolver(detectSystemTheme)
9595
setupFileWatchers()
96-
initializeOSCDetection()
9796

9897
const initialThemeName = detectSystemTheme()
9998
setLastDetectedTheme(initialThemeName)
@@ -104,10 +103,17 @@ export function initializeThemeStore() {
104103
themeConfig.plugins,
105104
)
106105

107-
useThemeStore = create<ThemeStore>((set) => ({
106+
useThemeStore = create<ThemeStore>((set, get) => ({
108107
theme: initialTheme,
109108

110109
setThemeName: (name: ThemeName) => {
110+
const currentTheme = get().theme
111+
112+
// Skip if theme name hasn't changed
113+
if (currentTheme.name === name) {
114+
return
115+
}
116+
111117
const baseTheme = cloneChatTheme(chatThemes[name])
112118
const theme = buildTheme(
113119
baseTheme,
@@ -119,9 +125,16 @@ export function initializeThemeStore() {
119125
},
120126
}))
121127

128+
// IMPORTANT: Set up the theme watcher BEFORE starting OSC detection
129+
// OSC detection is async and calls recomputeSystemTheme() when done,
130+
// which needs the themeStoreUpdater to be set
122131
initializeThemeWatcher((name: ThemeName) => {
123132
useThemeStore.getState().setThemeName(name)
124133
})
134+
135+
// Start OSC detection AFTER the theme watcher is set up
136+
// This ensures recomputeSystemTheme() can update the store when OSC completes
137+
initializeOSCDetection()
125138
}
126139

127140
export const useTheme = (): ChatTheme => {

cli/src/utils/terminal-color-detection.ts

Lines changed: 84 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
* OSC 11: Query background color
1010
*/
1111

12-
import { openSync, closeSync, writeSync, readSync, constants } from 'fs'
12+
import { openSync, closeSync, writeSync, constants } from 'fs'
1313

1414
// Timeout constants
1515
const OSC_QUERY_TIMEOUT_MS = 500 // Timeout for individual OSC query
@@ -88,20 +88,24 @@ function buildOscQuery(oscCode: number): string {
8888
}
8989

9090
/**
91-
* Query the terminal for OSC color information via /dev/tty
92-
* Uses synchronous reads with polling to avoid blocking forever
93-
* @param oscCode - The OSC code (10 for foreground, 11 for background)
91+
* Query the terminal for OSC color information
92+
* Writes query to /dev/tty and reads response from stdin using event-based reading
93+
* Terminal responses come back through the PTY, which appears on stdin
9494
* @returns The raw response string or null if query failed
9595
*/
9696
async function sendOscQuery(
9797
ttyPath: string,
9898
query: string,
9999
): Promise<string | null> {
100100
return new Promise((resolve) => {
101-
let ttyFd: number | null = null
101+
let ttyWriteFd: number | null = null
102102
let timeoutId: NodeJS.Timeout | null = null
103-
let pollIntervalId: NodeJS.Timeout | null = null
104103
let resolved = false
104+
let wasRawMode = false
105+
let wasFlowing = false
106+
let didResume = false
107+
let response = ''
108+
let dataHandler: ((data: Buffer) => void) | null = null
105109

106110
const cleanup = () => {
107111
if (resolved) return
@@ -111,17 +115,34 @@ async function sendOscQuery(
111115
clearTimeout(timeoutId)
112116
timeoutId = null
113117
}
114-
if (pollIntervalId) {
115-
clearInterval(pollIntervalId)
116-
pollIntervalId = null
118+
// Remove data handler from stdin
119+
if (dataHandler) {
120+
process.stdin.removeListener('data', dataHandler)
121+
dataHandler = null
122+
}
123+
// Restore raw mode state if we changed it
124+
if (process.stdin.isTTY && process.stdin.setRawMode) {
125+
try {
126+
process.stdin.setRawMode(wasRawMode)
127+
} catch {
128+
// Ignore errors restoring raw mode
129+
}
117130
}
118-
if (ttyFd !== null) {
131+
// Only pause stdin if we were the ones who resumed it
132+
if (didResume && !wasFlowing) {
119133
try {
120-
closeSync(ttyFd)
134+
process.stdin.pause()
135+
} catch {
136+
// Ignore pause errors
137+
}
138+
}
139+
if (ttyWriteFd !== null) {
140+
try {
141+
closeSync(ttyWriteFd)
121142
} catch {
122143
// Ignore close errors
123144
}
124-
ttyFd = null
145+
ttyWriteFd = null
125146
}
126147
}
127148

@@ -132,76 +153,73 @@ async function sendOscQuery(
132153
}
133154

134155
try {
135-
// Open TTY with O_RDWR and O_NONBLOCK for non-blocking reads
136-
// O_NONBLOCK = 0x0004 on macOS, 0x0800 on Linux
137-
const O_NONBLOCK =
138-
process.platform === 'darwin' ? 0x0004 : constants.O_NONBLOCK || 0x0800
139-
const O_RDWR = constants.O_RDWR
156+
// Check if stdin is a TTY - required for reading responses
157+
if (!process.stdin.isTTY) {
158+
resolveWith(null)
159+
return
160+
}
140161

162+
// Open TTY for writing the query
163+
const O_WRONLY = constants.O_WRONLY
141164
try {
142-
ttyFd = openSync(ttyPath, O_RDWR | O_NONBLOCK)
165+
ttyWriteFd = openSync(ttyPath, O_WRONLY)
143166
} catch {
144167
resolveWith(null)
145168
return
146169
}
147170

171+
// Save current raw mode state and enable raw mode to capture escape sequences
172+
try {
173+
wasRawMode = process.stdin.isRaw ?? false
174+
if (!wasRawMode && process.stdin.setRawMode) {
175+
process.stdin.setRawMode(true)
176+
}
177+
} catch {
178+
// Continue anyway - some terminals might work without raw mode
179+
}
180+
148181
// Set overall timeout
149182
timeoutId = setTimeout(() => {
150-
resolveWith(null)
183+
resolveWith(response.length > 0 ? response : null)
151184
}, OSC_QUERY_TIMEOUT_MS)
152185

153-
// Write the OSC query
186+
// Set up event-based reading from stdin
187+
dataHandler = (data: Buffer) => {
188+
if (resolved) return
189+
190+
const chunk = data.toString('utf8')
191+
response += chunk
192+
193+
// Check for complete response
194+
const hasBEL = response.includes('\x07')
195+
const hasST = response.includes('\x1b\\')
196+
const hasRGB =
197+
/rgb:[0-9a-fA-F]{2,4}\/[0-9a-fA-F]{2,4}\/[0-9a-fA-F]{2,4}/.test(
198+
response,
199+
)
200+
201+
// A complete response has RGB data AND a terminator (BEL or ST)
202+
// Some terminals might send RGB without proper terminator, so we accept that too
203+
if (hasRGB && (hasBEL || hasST || response.length > 20)) {
204+
resolveWith(response)
205+
}
206+
}
207+
208+
// Track if stdin was already flowing before we resume
209+
// readableFlowing is true if flowing, false if paused, null if not yet consumed
210+
wasFlowing = process.stdin.readableFlowing === true
211+
212+
process.stdin.on('data', dataHandler)
213+
process.stdin.resume()
214+
didResume = true
215+
216+
// Write the OSC query to TTY
154217
try {
155-
writeSync(ttyFd, query)
218+
writeSync(ttyWriteFd, query)
156219
} catch {
157220
resolveWith(null)
158221
return
159222
}
160-
161-
// Poll for response using non-blocking reads
162-
let response = ''
163-
const buffer = Buffer.alloc(256)
164-
let pollCount = 0
165-
const maxPolls = OSC_QUERY_TIMEOUT_MS / 10 // Poll every 10ms
166-
167-
pollIntervalId = setInterval(() => {
168-
pollCount++
169-
170-
if (ttyFd === null || pollCount > maxPolls) {
171-
resolveWith(response.length > 0 ? response : null)
172-
return
173-
}
174-
175-
try {
176-
const bytesRead = readSync(ttyFd, buffer, 0, buffer.length, null)
177-
if (bytesRead > 0) {
178-
const chunk = buffer.toString('utf8', 0, bytesRead)
179-
response += chunk
180-
181-
// Check for complete response
182-
const hasBEL = response.includes('\x07')
183-
const hasST = response.includes('\x1b\\')
184-
const hasRGB =
185-
/rgb:[0-9a-fA-F]{2,4}\/[0-9a-fA-F]{2,4}\/[0-9a-fA-F]{2,4}/.test(
186-
response,
187-
)
188-
189-
// A complete response has RGB data AND a terminator (BEL or ST)
190-
// Some terminals might send RGB without proper terminator, so we accept that too
191-
if (hasRGB && (hasBEL || hasST || response.length > 20)) {
192-
resolveWith(response)
193-
return
194-
}
195-
}
196-
} catch (error: unknown) {
197-
// EAGAIN/EWOULDBLOCK means no data available yet - this is expected
198-
const code = (error as NodeJS.ErrnoException)?.code
199-
if (code !== 'EAGAIN' && code !== 'EWOULDBLOCK') {
200-
// On actual error, stop polling
201-
resolveWith(response.length > 0 ? response : null)
202-
}
203-
}
204-
}, 10)
205223
} catch {
206224
resolveWith(null)
207225
}

0 commit comments

Comments
 (0)