Skip to content

Commit c754661

Browse files
jahoomajahooma
committed
cli: fixes for error propagation, reset sequences
1 parent 869f5c4 commit c754661

File tree

4 files changed

+168
-21
lines changed

4 files changed

+168
-21
lines changed

‎cli/release/index.js‎

Lines changed: 60 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,40 @@ const tar = require('tar')
1313

1414
const packageName = 'codebuff'
1515

16+
/**
17+
* Terminal escape sequences to reset terminal state after the child process exits.
18+
* When the binary is SIGKILL'd, it can't clean up its own terminal state.
19+
* The wrapper (this process) survives and must reset these modes.
20+
*
21+
* Keep in sync with TERMINAL_RESET_SEQUENCES in cli/src/utils/renderer-cleanup.ts
22+
*/
23+
const TERMINAL_RESET_SEQUENCES =
24+
'\x1b[?1049l' + // Exit alternate screen buffer
25+
'\x1b[?1000l' + // Disable X10 mouse mode
26+
'\x1b[?1002l' + // Disable button event mouse mode
27+
'\x1b[?1003l' + // Disable any-event mouse mode (all motion)
28+
'\x1b[?1006l' + // Disable SGR extended mouse mode
29+
'\x1b[?1004l' + // Disable focus reporting
30+
'\x1b[?2004l' + // Disable bracketed paste mode
31+
'\x1b[?25h' // Show cursor
32+
33+
function resetTerminal() {
34+
try {
35+
if (process.stdin.isTTY && process.stdin.setRawMode) {
36+
process.stdin.setRawMode(false)
37+
}
38+
} catch {
39+
// stdin may be closed
40+
}
41+
try {
42+
if (process.stdout.isTTY) {
43+
process.stdout.write(TERMINAL_RESET_SEQUENCES)
44+
}
45+
} catch {
46+
// stdout may be closed
47+
}
48+
}
49+
1650
function createConfig(packageName) {
1751
const homeDir = os.homedir()
1852
const configDir = path.join(homeDir, '.config', 'manicode')
@@ -526,18 +560,24 @@ async function checkForUpdates(runningProcess, exitListener) {
526560
term.clearLine()
527561

528562
runningProcess.removeListener('exit', exitListener)
529-
runningProcess.kill('SIGTERM')
530563

531564
await new Promise((resolve) => {
532-
runningProcess.on('exit', resolve)
565+
let exited = false
566+
runningProcess.once('exit', () => {
567+
exited = true
568+
resolve()
569+
})
570+
runningProcess.kill('SIGTERM')
533571
setTimeout(() => {
534-
if (!runningProcess.killed) {
572+
if (!exited) {
535573
runningProcess.kill('SIGKILL')
574+
// Safety: resolve after giving SIGKILL time to take effect
575+
setTimeout(() => resolve(), 1000)
536576
}
537-
resolve()
538577
}, 5000)
539578
})
540579

580+
resetTerminal()
541581
console.log(`Update available: ${currentVersion} → ${latestVersion}`)
542582

543583
await downloadBinary(latestVersion)
@@ -547,8 +587,14 @@ async function checkForUpdates(runningProcess, exitListener) {
547587
detached: false,
548588
})
549589

550-
newChild.on('exit', (code) => {
551-
process.exit(code || 0)
590+
newChild.on('exit', (code, signal) => {
591+
resetTerminal()
592+
process.exit(signal ? 1 : (code || 0))
593+
})
594+
595+
newChild.on('error', (err) => {
596+
console.error('Failed to start codebuff:', err.message)
597+
process.exit(1)
552598
})
553599

554600
return new Promise(() => {})
@@ -565,12 +611,18 @@ async function main() {
565611
stdio: 'inherit',
566612
})
567613

568-
const exitListener = (code) => {
569-
process.exit(code || 0)
614+
const exitListener = (code, signal) => {
615+
resetTerminal()
616+
process.exit(signal ? 1 : (code || 0))
570617
}
571618

572619
child.on('exit', exitListener)
573620

621+
child.on('error', (err) => {
622+
console.error('Failed to start codebuff:', err.message)
623+
process.exit(1)
624+
})
625+
574626
setTimeout(() => {
575627
checkForUpdates(child, exitListener)
576628
}, 100)

‎cli/src/index.tsx‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ import { initializeAgentRegistry } from './utils/local-agent-registry'
3333
import { clearLogFile, logger } from './utils/logger'
3434
import { shouldShowProjectPicker } from './utils/project-picker'
3535
import { saveRecentProject } from './utils/recent-projects'
36-
import { installProcessCleanupHandlers } from './utils/renderer-cleanup'
36+
import { installProcessCleanupHandlers, TERMINAL_RESET_SEQUENCES } from './utils/renderer-cleanup'
3737
import { initializeSkillRegistry } from './utils/skill-registry'
3838
import { detectTerminalTheme } from './utils/terminal-color-detection'
3939
import { setOscDetectedTheme } from './utils/theme-system'
@@ -363,11 +363,43 @@ async function main(): Promise<void> {
363363
)
364364
}
365365

366+
// Install early error handlers BEFORE renderer creation.
367+
// If the renderer crashes during init, these ensure the error is visible
368+
// by exiting the alternate screen buffer before printing the error.
369+
const earlyFatalHandler = (error: unknown) => {
370+
try {
371+
if (process.stdin.isTTY && process.stdin.setRawMode) {
372+
process.stdin.setRawMode(false)
373+
}
374+
} catch {
375+
// stdin may be closed
376+
}
377+
try {
378+
if (process.stdout.isTTY) {
379+
process.stdout.write(TERMINAL_RESET_SEQUENCES)
380+
}
381+
} catch {
382+
// stdout may be closed
383+
}
384+
try {
385+
console.error('Fatal error during startup:', error)
386+
} catch {
387+
// stderr may be closed
388+
}
389+
process.exit(1)
390+
}
391+
process.on('uncaughtException', earlyFatalHandler)
392+
process.on('unhandledRejection', earlyFatalHandler)
393+
366394
const renderer = await createCliRenderer({
367395
backgroundColor: 'transparent',
368396
exitOnCtrlC: false,
369397
useAlternateScreen: true,
370398
})
399+
400+
// Remove early handlers — proper cleanup handlers (with renderer access) take over
401+
process.removeListener('uncaughtException', earlyFatalHandler)
402+
process.removeListener('unhandledRejection', earlyFatalHandler)
371403
installProcessCleanupHandlers(renderer)
372404
createRoot(renderer).render(
373405
<QueryClientProvider client={queryClient}>

‎cli/src/utils/renderer-cleanup.ts‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ let terminalStateReset = false
2121
* - \x1b[?2004l: Disable bracketed paste mode
2222
* - \x1b[?25h: Show cursor (safety measure)
2323
*/
24-
const TERMINAL_RESET_SEQUENCES =
24+
export const TERMINAL_RESET_SEQUENCES =
2525
'\x1b[?1049l' + // Exit alternate screen buffer
2626
'\x1b[?1000l' + // Disable X10 mouse mode
2727
'\x1b[?1002l' + // Disable button event mouse mode
@@ -43,12 +43,21 @@ function resetTerminalState(): void {
4343
if (terminalStateReset) return
4444
terminalStateReset = true
4545

46+
try {
47+
if (process.stdin.isTTY && process.stdin.setRawMode) {
48+
process.stdin.setRawMode(false)
49+
}
50+
} catch {
51+
// Ignore errors - stdin may already be closed
52+
}
4653
try {
4754
// Reset terminal title to default
4855
resetTerminalTitle()
4956
// Write directly to stdout - this is synchronous and will complete
5057
// before the process exits, ensuring the terminal is reset
51-
process.stdout.write(TERMINAL_RESET_SEQUENCES)
58+
if (process.stdout.isTTY) {
59+
process.stdout.write(TERMINAL_RESET_SEQUENCES)
60+
}
5261
} catch {
5362
// Ignore errors - stdout may already be closed
5463
}
@@ -124,21 +133,23 @@ export function installProcessCleanupHandlers(cliRenderer: CliRenderer): void {
124133

125134
// uncaughtException - Safety net for unhandled errors
126135
process.on('uncaughtException', (error) => {
136+
cleanup() // Exit alt screen FIRST so error output is visible on the main screen
127137
try {
128138
console.error('Uncaught exception:', error)
129139
} catch {
130140
// Ignore logging errors
131141
}
132-
cleanupAndExit(1)
142+
process.exit(1)
133143
})
134144

135145
// unhandledRejection - Safety net for unhandled promise rejections
136146
process.on('unhandledRejection', (reason) => {
147+
cleanup() // Exit alt screen FIRST so error output is visible on the main screen
137148
try {
138149
console.error('Unhandled rejection:', reason)
139150
} catch {
140151
// Ignore logging errors
141152
}
142-
cleanupAndExit(1)
153+
process.exit(1)
143154
})
144155
}

‎freebuff/cli/release/index.js‎

Lines changed: 60 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,40 @@ const tar = require('tar')
1313

1414
const packageName = 'freebuff'
1515

16+
/**
17+
* Terminal escape sequences to reset terminal state after the child process exits.
18+
* When the binary is SIGKILL'd, it can't clean up its own terminal state.
19+
* The wrapper (this process) survives and must reset these modes.
20+
*
21+
* Keep in sync with TERMINAL_RESET_SEQUENCES in cli/src/utils/renderer-cleanup.ts
22+
*/
23+
const TERMINAL_RESET_SEQUENCES =
24+
'\x1b[?1049l' + // Exit alternate screen buffer
25+
'\x1b[?1000l' + // Disable X10 mouse mode
26+
'\x1b[?1002l' + // Disable button event mouse mode
27+
'\x1b[?1003l' + // Disable any-event mouse mode (all motion)
28+
'\x1b[?1006l' + // Disable SGR extended mouse mode
29+
'\x1b[?1004l' + // Disable focus reporting
30+
'\x1b[?2004l' + // Disable bracketed paste mode
31+
'\x1b[?25h' // Show cursor
32+
33+
function resetTerminal() {
34+
try {
35+
if (process.stdin.isTTY && process.stdin.setRawMode) {
36+
process.stdin.setRawMode(false)
37+
}
38+
} catch {
39+
// stdin may be closed
40+
}
41+
try {
42+
if (process.stdout.isTTY) {
43+
process.stdout.write(TERMINAL_RESET_SEQUENCES)
44+
}
45+
} catch {
46+
// stdout may be closed
47+
}
48+
}
49+
1650
function createConfig(packageName) {
1751
const homeDir = os.homedir()
1852
const configDir = path.join(homeDir, '.config', 'manicode')
@@ -513,18 +547,24 @@ async function checkForUpdates(runningProcess, exitListener) {
513547
term.clearLine()
514548

515549
runningProcess.removeListener('exit', exitListener)
516-
runningProcess.kill('SIGTERM')
517550

518551
await new Promise((resolve) => {
519-
runningProcess.on('exit', resolve)
552+
let exited = false
553+
runningProcess.once('exit', () => {
554+
exited = true
555+
resolve()
556+
})
557+
runningProcess.kill('SIGTERM')
520558
setTimeout(() => {
521-
if (!runningProcess.killed) {
559+
if (!exited) {
522560
runningProcess.kill('SIGKILL')
561+
// Safety: resolve after giving SIGKILL time to take effect
562+
setTimeout(() => resolve(), 1000)
523563
}
524-
resolve()
525564
}, 5000)
526565
})
527566

567+
resetTerminal()
528568
console.log(`Update available: ${currentVersion} → ${latestVersion}`)
529569

530570
await downloadBinary(latestVersion)
@@ -534,8 +574,14 @@ async function checkForUpdates(runningProcess, exitListener) {
534574
detached: false,
535575
})
536576

537-
newChild.on('exit', (code) => {
538-
process.exit(code || 0)
577+
newChild.on('exit', (code, signal) => {
578+
resetTerminal()
579+
process.exit(signal ? 1 : (code || 0))
580+
})
581+
582+
newChild.on('error', (err) => {
583+
console.error('Failed to start freebuff:', err.message)
584+
process.exit(1)
539585
})
540586

541587
return new Promise(() => {})
@@ -552,12 +598,18 @@ async function main() {
552598
stdio: 'inherit',
553599
})
554600

555-
const exitListener = (code) => {
556-
process.exit(code || 0)
601+
const exitListener = (code, signal) => {
602+
resetTerminal()
603+
process.exit(signal ? 1 : (code || 0))
557604
}
558605

559606
child.on('exit', exitListener)
560607

608+
child.on('error', (err) => {
609+
console.error('Failed to start freebuff:', err.message)
610+
process.exit(1)
611+
})
612+
561613
setTimeout(() => {
562614
checkForUpdates(child, exitListener)
563615
}, 100)

0 commit comments

Comments
 (0)