mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-18 04:41:56 +00:00
fix(tui): harden Terminal.app render behavior
Avoid Terminal.app paint corruption by disabling fast-echo in that terminal, sanitizing non-SGR control sequences before ANSI rendering, and defaulting Apple Terminal back to the safer 256-color path unless truecolor is explicitly requested.
This commit is contained in:
parent
3b39096904
commit
290bf93104
9 changed files with 214 additions and 10 deletions
|
|
@ -1080,7 +1080,7 @@ def _make_tui_argv(tui_dir: Path, tui_dev: bool) -> tuple[list[str], Path]:
|
|||
return [node, str(bundled)], bundled.parent
|
||||
|
||||
# 2. Normal flow: npm install if needed, always esbuild, then node dist/entry.js.
|
||||
# --dev flow: npm install if needed, then tsx src/entry.tsx (no build).
|
||||
# --dev flow: npm install if needed, then tsx src/entry.tsx.
|
||||
if _tui_need_npm_install(tui_dir):
|
||||
npm = _node_bin("npm")
|
||||
if not os.environ.get("HERMES_QUIET"):
|
||||
|
|
@ -1102,10 +1102,30 @@ def _make_tui_argv(tui_dir: Path, tui_dev: bool) -> tuple[list[str], Path]:
|
|||
sys.exit(1)
|
||||
|
||||
if tui_dev:
|
||||
# Keep the local @hermes/ink package exports in sync with source.
|
||||
# --dev runs src/entry.tsx directly, but @hermes/ink resolves through
|
||||
# packages/hermes-ink/dist/entry-exports.js. If that dist bundle is
|
||||
# stale after a pull, newer hooks/components can exist in src while
|
||||
# being missing at runtime (e.g. useCursorAdvance). Prebuild it here.
|
||||
npm = _node_bin("npm")
|
||||
ink_dir = tui_dir / "packages" / "hermes-ink"
|
||||
result = subprocess.run(
|
||||
[npm, "run", "build"],
|
||||
cwd=str(ink_dir),
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
if result.returncode != 0:
|
||||
combined = f"{result.stdout or ''}{result.stderr or ''}".strip()
|
||||
preview = "\n".join(combined.splitlines()[-30:])
|
||||
print("TUI dev prebuild failed.")
|
||||
if preview:
|
||||
print(preview)
|
||||
sys.exit(1)
|
||||
|
||||
tsx = tui_dir / "node_modules" / ".bin" / "tsx"
|
||||
if tsx.exists():
|
||||
return [str(tsx), "src/entry.tsx"], tui_dir
|
||||
npm = _node_bin("npm")
|
||||
return [npm, "start"], tui_dir
|
||||
|
||||
# Always rebuild — esbuild is fast and this avoids staleness-edge-case bugs.
|
||||
|
|
|
|||
|
|
@ -523,6 +523,34 @@ def test_launch_tui_exports_model_provider_and_toolsets(monkeypatch, main_mod):
|
|||
assert env["NODE_ENV"] == "production"
|
||||
|
||||
|
||||
def test_make_tui_argv_dev_prebuilds_hermes_ink(monkeypatch, main_mod, tmp_path):
|
||||
tui_dir = tmp_path / "ui-tui"
|
||||
tsx = tui_dir / "node_modules" / ".bin" / "tsx"
|
||||
ink_dir = tui_dir / "packages" / "hermes-ink"
|
||||
tsx.parent.mkdir(parents=True)
|
||||
ink_dir.mkdir(parents=True)
|
||||
tsx.write_text("#!/usr/bin/env node\n", encoding="utf-8")
|
||||
|
||||
monkeypatch.setattr(main_mod, "_ensure_tui_node", lambda: None)
|
||||
monkeypatch.setattr(main_mod, "_tui_need_npm_install", lambda _tui_dir: False)
|
||||
monkeypatch.delenv("HERMES_TUI_DIR", raising=False)
|
||||
monkeypatch.setattr(main_mod.shutil, "which", lambda bin_name: f"/usr/bin/{bin_name}")
|
||||
|
||||
calls = []
|
||||
|
||||
def fake_run(cmd, cwd=None, **_kwargs):
|
||||
calls.append((cmd, cwd))
|
||||
return types.SimpleNamespace(returncode=0, stdout="", stderr="")
|
||||
|
||||
monkeypatch.setattr(main_mod.subprocess, "run", fake_run)
|
||||
|
||||
argv, cwd = main_mod._make_tui_argv(tui_dir, tui_dev=True)
|
||||
|
||||
assert argv == [str(tsx), "src/entry.tsx"]
|
||||
assert cwd == tui_dir
|
||||
assert calls == [(["/usr/bin/npm", "run", "build"], str(ink_dir))]
|
||||
|
||||
|
||||
def test_print_tui_exit_summary_includes_resume_and_token_totals(monkeypatch, capsys):
|
||||
import hermes_cli.main as main_mod
|
||||
|
||||
|
|
|
|||
|
|
@ -52,6 +52,50 @@ describe('forceTruecolor', () => {
|
|||
)
|
||||
})
|
||||
|
||||
it('downgrades Apple Terminal when truecolor is only advertised by env', async () => {
|
||||
await withCleanEnv(
|
||||
() => {
|
||||
process.env.TERM_PROGRAM = 'Apple_Terminal'
|
||||
process.env.COLORTERM = 'truecolor'
|
||||
process.env.FORCE_COLOR = '3'
|
||||
},
|
||||
async () => {
|
||||
const mod = await import('../lib/forceTruecolor.js?t=downgrade-' + importId++)
|
||||
expect(
|
||||
mod.shouldDowngradeAppleTerminalTruecolor({
|
||||
TERM_PROGRAM: 'Apple_Terminal',
|
||||
COLORTERM: 'truecolor',
|
||||
FORCE_COLOR: '3'
|
||||
} as NodeJS.ProcessEnv)
|
||||
).toBe(true)
|
||||
expect(process.env.COLORTERM).toBeUndefined()
|
||||
expect(process.env.FORCE_COLOR).toBeUndefined()
|
||||
}
|
||||
)
|
||||
})
|
||||
|
||||
it('keeps non-Apple terminals untouched when they advertise truecolor', async () => {
|
||||
await withCleanEnv(
|
||||
() => {
|
||||
process.env.TERM_PROGRAM = 'vscode'
|
||||
process.env.COLORTERM = 'truecolor'
|
||||
process.env.FORCE_COLOR = '3'
|
||||
},
|
||||
async () => {
|
||||
const mod = await import('../lib/forceTruecolor.js?t=keep-non-apple-' + importId++)
|
||||
expect(
|
||||
mod.shouldDowngradeAppleTerminalTruecolor({
|
||||
TERM_PROGRAM: 'vscode',
|
||||
COLORTERM: 'truecolor',
|
||||
FORCE_COLOR: '3'
|
||||
} as NodeJS.ProcessEnv)
|
||||
).toBe(false)
|
||||
expect(process.env.COLORTERM).toBe('truecolor')
|
||||
expect(process.env.FORCE_COLOR).toBe('3')
|
||||
}
|
||||
)
|
||||
})
|
||||
|
||||
it('sets COLORTERM=truecolor and FORCE_COLOR=3 when explicitly enabled', async () => {
|
||||
await withCleanEnv(
|
||||
() => {
|
||||
|
|
@ -79,6 +123,30 @@ describe('forceTruecolor', () => {
|
|||
)
|
||||
})
|
||||
|
||||
it('lets explicit opt-in keep Apple truecolor advertisement', async () => {
|
||||
await withCleanEnv(
|
||||
() => {
|
||||
process.env.TERM_PROGRAM = 'Apple_Terminal'
|
||||
process.env.COLORTERM = 'truecolor'
|
||||
process.env.FORCE_COLOR = '3'
|
||||
process.env.HERMES_TUI_TRUECOLOR = '1'
|
||||
},
|
||||
async () => {
|
||||
const mod = await import('../lib/forceTruecolor.js?t=apple-explicit-on-' + importId++)
|
||||
expect(
|
||||
mod.shouldDowngradeAppleTerminalTruecolor({
|
||||
TERM_PROGRAM: 'Apple_Terminal',
|
||||
COLORTERM: 'truecolor',
|
||||
FORCE_COLOR: '3',
|
||||
HERMES_TUI_TRUECOLOR: '1'
|
||||
} as NodeJS.ProcessEnv)
|
||||
).toBe(false)
|
||||
expect(process.env.COLORTERM).toBe('truecolor')
|
||||
expect(process.env.FORCE_COLOR).toBe('3')
|
||||
}
|
||||
)
|
||||
})
|
||||
|
||||
it('respects NO_COLOR', async () => {
|
||||
await withCleanEnv(
|
||||
() => {
|
||||
|
|
|
|||
|
|
@ -8,12 +8,15 @@ import {
|
|||
estimateRows,
|
||||
estimateTokensRough,
|
||||
fmtK,
|
||||
hasAnsi,
|
||||
isToolTrailResultLine,
|
||||
lastCotTrailIndex,
|
||||
parseToolTrailResultLine,
|
||||
pasteTokenLabel,
|
||||
sanitizeAnsiForRender,
|
||||
sameToolTrailGroup,
|
||||
splitToolDuration,
|
||||
stripAnsi,
|
||||
thinkingPreview
|
||||
} from '../lib/text.js'
|
||||
|
||||
|
|
@ -84,6 +87,27 @@ describe('estimateTokensRough', () => {
|
|||
})
|
||||
})
|
||||
|
||||
describe('ANSI sanitizers', () => {
|
||||
const ESC = String.fromCharCode(27)
|
||||
const BEL = String.fromCharCode(7)
|
||||
|
||||
it('strips CSI/OSC/control bytes from plain previews', () => {
|
||||
const sample = `A${ESC}[31mB${ESC}[39m${ESC}[2J${ESC}]0;title${BEL}C${ESC}[?25lD`
|
||||
|
||||
expect(stripAnsi(sample)).toBe('ABCD')
|
||||
})
|
||||
|
||||
it('keeps SGR color spans but removes cursor controls for Ansi rendering', () => {
|
||||
const sample = `A${ESC}[31mB${ESC}[39m${ESC}[2J${ESC}]0;title${BEL}${ESC}[?25lC`
|
||||
|
||||
expect(sanitizeAnsiForRender(sample)).toBe(`A${ESC}[31mB${ESC}[39mC`)
|
||||
})
|
||||
|
||||
it('detects non-CSI escape prefixes too', () => {
|
||||
expect(hasAnsi(`ok${ESC}Ppayload${ESC}\\`)).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe('thinkingPreview', () => {
|
||||
it('adds paragraph breaks before markdown thinking headings', () => {
|
||||
const raw =
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { canFastAppendShape, canFastBackspaceShape } from '../components/textInput.js'
|
||||
import { canFastAppendShape, canFastBackspaceShape, supportsFastEchoTerminal } from '../components/textInput.js'
|
||||
|
||||
// The fast-echo path bypasses Ink and writes characters directly to stdout
|
||||
// for the common case of typing plain English at the end of the line. These
|
||||
|
|
@ -172,3 +172,14 @@ describe('canFastBackspaceShape', () => {
|
|||
expect(canFastBackspaceShape('hello ', 'hello '.length)).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe('supportsFastEchoTerminal', () => {
|
||||
it('disables fast-echo in Apple Terminal', () => {
|
||||
expect(supportsFastEchoTerminal({ TERM_PROGRAM: 'Apple_Terminal' } as NodeJS.ProcessEnv)).toBe(false)
|
||||
})
|
||||
|
||||
it('keeps fast-echo enabled in VS Code and unknown terminals', () => {
|
||||
expect(supportsFastEchoTerminal({ TERM_PROGRAM: 'vscode' } as NodeJS.ProcessEnv)).toBe(true)
|
||||
expect(supportsFastEchoTerminal({ TERM: 'xterm-256color' } as NodeJS.ProcessEnv)).toBe(true)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ import {
|
|||
compactPreview,
|
||||
hasAnsi,
|
||||
isPasteBackedText,
|
||||
sanitizeAnsiForRender,
|
||||
stripAnsi
|
||||
} from '../lib/text.js'
|
||||
import type { Theme } from '../theme.js'
|
||||
|
|
@ -85,13 +86,14 @@ export const MessageLine = memo(function MessageLine({
|
|||
if (msg.role === 'tool') {
|
||||
const maxChars = Math.max(24, cols - 14)
|
||||
const stripped = hasAnsi(msg.text) ? stripAnsi(msg.text) : msg.text
|
||||
const safeAnsi = hasAnsi(msg.text) ? sanitizeAnsiForRender(msg.text) : msg.text
|
||||
const preview = compactPreview(stripped, maxChars) || '(empty tool result)'
|
||||
|
||||
return (
|
||||
<Box alignSelf="flex-start" borderColor={t.color.muted} borderStyle="round" marginLeft={3} paddingX={1}>
|
||||
{hasAnsi(msg.text) ? (
|
||||
<Text wrap="truncate-end">
|
||||
<Ansi>{msg.text}</Ansi>
|
||||
<Ansi>{safeAnsi}</Ansi>
|
||||
</Text>
|
||||
) : (
|
||||
<Text color={t.color.muted} wrap="truncate-end">
|
||||
|
|
@ -129,13 +131,13 @@ export const MessageLine = memo(function MessageLine({
|
|||
{msg.text.length.toLocaleString()} chars
|
||||
</Text>
|
||||
</Box>
|
||||
{systemOpen && <Ansi>{msg.text}</Ansi>}
|
||||
{systemOpen && <Ansi>{sanitizeAnsiForRender(msg.text)}</Ansi>}
|
||||
</Box>
|
||||
)
|
||||
}
|
||||
|
||||
if (msg.role !== 'user' && hasAnsi(msg.text)) {
|
||||
return <Ansi>{msg.text}</Ansi>
|
||||
return <Ansi>{sanitizeAnsiForRender(msg.text)}</Ansi>
|
||||
}
|
||||
|
||||
if (msg.role === 'assistant') {
|
||||
|
|
|
|||
|
|
@ -283,6 +283,12 @@ export function canFastBackspaceShape(current: string, cursor: number, columns?:
|
|||
return ASCII_PRINTABLE_RE.test(removed)
|
||||
}
|
||||
|
||||
export function supportsFastEchoTerminal(env: NodeJS.ProcessEnv = process.env): boolean {
|
||||
// Terminal.app still shows paint/cursor artifacts under the fast-echo
|
||||
// bypass path. Fall back to the normal Ink render path there.
|
||||
return (env.TERM_PROGRAM ?? '').trim() !== 'Apple_Terminal'
|
||||
}
|
||||
|
||||
function renderWithCursor(value: string, cursor: number) {
|
||||
const pos = Math.max(0, Math.min(cursor, value.length))
|
||||
|
||||
|
|
@ -559,7 +565,7 @@ export function TextInput({
|
|||
}, 16)
|
||||
}
|
||||
|
||||
const canFastEchoBase = () => focus && termFocus && !selected && !mask && !!stdout?.isTTY
|
||||
const canFastEchoBase = () => supportsFastEchoTerminal() && focus && termFocus && !selected && !mask && !!stdout?.isTTY
|
||||
|
||||
const canFastAppend = (current: string, cursor: number, text: string) =>
|
||||
canFastEchoBase() && canFastAppendShape(current, cursor, text, columns, lineWidthRef.current)
|
||||
|
|
|
|||
|
|
@ -19,12 +19,42 @@ export function shouldForceTruecolor(env: NodeJS.ProcessEnv = process.env): bool
|
|||
return TRUE_RE.test(override)
|
||||
}
|
||||
|
||||
const isAppleTerminal = (env: NodeJS.ProcessEnv = process.env) => (env.TERM_PROGRAM ?? '').trim() === 'Apple_Terminal'
|
||||
|
||||
const isAdvertisedTruecolor = (env: NodeJS.ProcessEnv = process.env) => {
|
||||
const colorTerm = (env.COLORTERM ?? '').trim().toLowerCase()
|
||||
const forceColor = (env.FORCE_COLOR ?? '').trim()
|
||||
|
||||
return colorTerm === 'truecolor' || colorTerm === '24bit' || forceColor === '3'
|
||||
}
|
||||
|
||||
export function shouldDowngradeAppleTerminalTruecolor(env: NodeJS.ProcessEnv = process.env): boolean {
|
||||
if (!isAppleTerminal(env)) {
|
||||
return false
|
||||
}
|
||||
|
||||
if (shouldForceTruecolor(env)) {
|
||||
return false
|
||||
}
|
||||
|
||||
return isAdvertisedTruecolor(env)
|
||||
}
|
||||
|
||||
if (shouldForceTruecolor()) {
|
||||
if (!process.env.COLORTERM) {
|
||||
process.env.COLORTERM = 'truecolor'
|
||||
}
|
||||
|
||||
process.env.FORCE_COLOR = '3'
|
||||
} else if (shouldDowngradeAppleTerminalTruecolor()) {
|
||||
// Terminal.app may advertise truecolor even when RGB SGR paths render
|
||||
// incorrectly. Keep Hermes on the safer TERM-driven 256-color path unless
|
||||
// users explicitly opt back in via HERMES_TUI_TRUECOLOR=1.
|
||||
delete process.env.COLORTERM
|
||||
|
||||
if ((process.env.FORCE_COLOR ?? '').trim() === '3') {
|
||||
delete process.env.FORCE_COLOR
|
||||
}
|
||||
}
|
||||
|
||||
export {}
|
||||
|
|
|
|||
|
|
@ -9,12 +9,27 @@ import { VERBS } from '../content/verbs.js'
|
|||
import type { ThinkingMode } from '../types.js'
|
||||
|
||||
const ESC = String.fromCharCode(27)
|
||||
const ANSI_RE = new RegExp(`${ESC}\\[[0-9;]*m`, 'g')
|
||||
const BEL = String.fromCharCode(7)
|
||||
const ANSI_CSI_RE = new RegExp(`${ESC}\\[[0-?]*[ -/]*[@-~]`, 'g')
|
||||
const ANSI_CSI_WITH_CMD_RE = new RegExp(`${ESC}\\[[0-?]*[ -/]*([@-~])`, 'g')
|
||||
const ANSI_OSC_RE = new RegExp(`${ESC}\\][\\s\\S]*?(?:${BEL}|${ESC}\\\\)`, 'g')
|
||||
const ANSI_STRING_RE = new RegExp(`${ESC}[PX^_][\\s\\S]*?(?:${BEL}|${ESC}\\\\)`, 'g')
|
||||
const ANSI_STRAY_ESC_RE = new RegExp(`${ESC}(?!\\[)[\\s\\S]?`, 'g')
|
||||
const CONTROL_RE = /[\x00-\x08\x0B\x0C\x0E-\x1A\x1C-\x1F\x7F]/g
|
||||
const WS_RE = /\s+/g
|
||||
|
||||
export const stripAnsi = (s: string) => s.replace(ANSI_RE, '')
|
||||
export const stripAnsi = (s: string) =>
|
||||
s.replace(ANSI_OSC_RE, '').replace(ANSI_STRING_RE, '').replace(ANSI_CSI_RE, '').replace(ANSI_STRAY_ESC_RE, '').replace(CONTROL_RE, '')
|
||||
|
||||
export const hasAnsi = (s: string) => s.includes(`${ESC}[`) || s.includes(`${ESC}]`)
|
||||
export const sanitizeAnsiForRender = (s: string) =>
|
||||
s
|
||||
.replace(ANSI_OSC_RE, '')
|
||||
.replace(ANSI_STRING_RE, '')
|
||||
.replace(ANSI_CSI_WITH_CMD_RE, (seq, cmd: string) => (cmd === 'm' ? seq : ''))
|
||||
.replace(ANSI_STRAY_ESC_RE, '')
|
||||
.replace(CONTROL_RE, '')
|
||||
|
||||
export const hasAnsi = (s: string) => s.includes(ESC)
|
||||
|
||||
const renderEstimateLine = (line: string) => {
|
||||
const trimmed = line.trim()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue