fix: default LANG to UTF-8 for terminal character display (#423)

* fix: address review findings

* fix: improve error handling in terminal e2e helper JSON parsing

* fix: remove unused catch parameter in terminal e2e helper

* fix: use nullish coalescing assignment for LANG default in PTY spawn

The object spread approach ({LANG: default, ...process.env}) produced
undefined on CI (Ubuntu/Node 22) despite working locally. Switch to
explicit ??= assignment after the spread for reliable cross-platform
behavior.
This commit is contained in:
Jinjing 2026-04-09 14:02:02 -07:00 committed by GitHub
parent df54597cc3
commit c908bdfbb2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 313 additions and 3 deletions

View File

@ -0,0 +1,240 @@
#!/usr/bin/env node
/**
* Terminal E2E helpers for agent-browser + CDP testing against a running Orca
* dev build. Encapsulates patterns discovered during manual terminal testing:
*
* - CDP key events do NOT work with xterm.js (canvas-based renderer)
* - ClipboardEvent paste simulation does NOT work
* - Direct PTY write via `window.api.pty.write(id, data)` DOES work
* - PTY IDs are sequential integers starting from 1
* - The visible terminal's PTY ID must be discovered (not guessed)
*
* Usage:
* import { OrcaTerminal } from './terminal-e2e-helpers.mjs'
*
* const term = new OrcaTerminal(9444) // CDP port
* await term.connect()
* const ptyId = await term.discoverActivePtyId()
* await term.send(ptyId, 'echo hello\r')
* const screenshot = await term.screenshot('/tmp/test.png')
* await term.waitForOutput(ptyId, 'hello')
*
* Or run directly:
* node config/scripts/terminal-e2e-helpers.mjs --port 9444 --command 'echo hello'
*/
import { execFileSync } from 'child_process'
const AGENT_BROWSER = 'agent-browser'
/** Thin wrapper around `agent-browser --cdp <port> <subcommand>`. */
function ab(port, args) {
const result = execFileSync(AGENT_BROWSER, ['--cdp', String(port), ...args], {
encoding: 'utf-8',
timeout: 15_000
})
return result.trim()
}
/** Run JS in the renderer via `agent-browser eval`. */
function evalInRenderer(port, js) {
return ab(port, ['eval', js])
}
export class OrcaTerminal {
/** @param {number} cdpPort — the --remote-debugging-port used when launching Orca */
constructor(cdpPort) {
this.port = cdpPort
}
/** Verify connection by taking a snapshot. */
connect() {
ab(this.port, ['snapshot', '-i'])
}
/**
* Discover the PTY ID of the currently visible terminal pane.
*
* Why: PTY IDs are opaque sequential integers and the mapping from
* visible tab PTY ID isn't exposed in the DOM. We send a unique
* marker to candidate IDs and see which one appears in the active pane.
*
* @param {number} maxId highest PTY ID to probe (default 10)
* @returns {string} the PTY ID string
*/
discoverActivePtyId(maxId = 10) {
const marker = `__PTY_PROBE_${Date.now()}__`
// Send Ctrl+C + marker echo to each candidate PTY
const js = `
(function() {
for (let i = 1; i <= ${maxId}; i++) {
window.api.pty.write(String(i), '\\x03\\x15echo ' + '${marker}_' + i + '\\r');
}
return 'probed 1-${maxId}';
})()
`
evalInRenderer(this.port, js)
// Wait for output to render
execFileSync('sleep', ['1.5'])
// Read the visible xterm buffer to find which marker appeared
const bufferJs = `
(function() {
const xterms = document.querySelectorAll('.xterm');
const visible = Array.from(xterms).find(x => x.offsetParent !== null);
if (!visible) return JSON.stringify({error: 'no visible xterm'});
// Read the screen buffer's text via the DOM text layer or serialize addon
// xterm renders to canvas, so read from the buffer API
// We check if the serialize addon exposed the buffer text
const screen = visible.querySelector('.xterm-screen');
// Fallback: read textContent from the accessibility tree
const accessibilityEl = visible.querySelector('.xterm-accessibility');
const text = accessibilityEl?.textContent || '';
return JSON.stringify({text: text.slice(-2000)});
})()
`
const bufferResult = evalInRenderer(this.port, bufferJs)
// Parse the marker from the buffer
let parsed
try {
parsed = JSON.parse(bufferResult.replace(/^"|"$/g, '').replace(/\\"/g, '"'))
} catch {
throw new Error(`discoverActivePtyId: failed to parse buffer result: ${bufferResult}`)
}
if (parsed.error) {
throw new Error(`discoverActivePtyId: ${parsed.error}`)
}
// Find all markers, take the last one (most recent = the visible terminal)
const markerRe = new RegExp(`${marker}_(\\d+)`, 'g')
const matches = [...parsed.text.matchAll(markerRe)]
if (matches.length === 0) {
// Fallback: take screenshot and try OCR-free approach by probing write
throw new Error(
'discoverActivePtyId: no marker found in buffer. ' +
'The accessibility tree may be disabled. ' +
'Try using probePtyIdWithScreenshot() instead.'
)
}
return matches.at(-1)[1]
}
/**
* Alternative PTY discovery: send markers, take a screenshot, and let the
* caller visually identify which PTY responded.
*
* @param {number} maxId highest PTY ID to probe
* @param {string} screenshotPath where to save the screenshot
*/
probePtyIdWithScreenshot(maxId = 10, screenshotPath = '/tmp/orca-pty-probe.png') {
for (let i = 1; i <= maxId; i++) {
evalInRenderer(
this.port,
`window.api.pty.write('${i}', '\\x03\\x15echo PTY_ID_${i}\\r')`
)
}
execFileSync('sleep', ['2'])
this.screenshot(screenshotPath)
return screenshotPath
}
/**
* Send text to a specific PTY.
*
* @param {string} ptyId the PTY ID (from discoverActivePtyId)
* @param {string} text text to send (use \r for Enter, \x03 for Ctrl+C, etc.)
*/
send(ptyId, text) {
// Escape for JS string literal inside eval
const escaped = text.replace(/\\/g, '\\\\').replace(/'/g, "\\'").replace(/\r/g, '\\r')
evalInRenderer(this.port, `window.api.pty.write('${ptyId}', '${escaped}')`)
}
/**
* Send a shell command and press Enter.
*
* @param {string} ptyId
* @param {string} command the shell command (Enter is appended)
*/
exec(ptyId, command) {
this.send(ptyId, `${command}\r`)
}
/** Send Ctrl+C to a PTY. */
interrupt(ptyId) {
this.send(ptyId, '\x03')
}
/** Clear the current line (Ctrl+U). */
clearLine(ptyId) {
this.send(ptyId, '\x15')
}
/**
* Take a screenshot of the Orca window.
*
* @param {string} path output file path
* @returns {string} the screenshot path
*/
screenshot(path = '/tmp/orca-terminal.png') {
ab(this.port, ['screenshot', path])
return path
}
/**
* Open a new terminal tab in Orca.
* @returns {void}
*/
newTerminal() {
ab(this.port, ['click', '@e7']) // "New terminal (Cmd+T)" button
execFileSync('sleep', ['2'])
}
/**
* Read the LANG value from a PTY's shell environment.
*
* @param {string} ptyId
* @returns {string} the LANG value
*/
readLang(ptyId) {
this.exec(ptyId, 'echo __LANG__=$LANG')
execFileSync('sleep', ['1'])
// Screenshot and return for inspection
return this.screenshot('/tmp/orca-lang-check.png')
}
}
// ---------------------------------------------------------------------------
// CLI entrypoint
// ---------------------------------------------------------------------------
if (process.argv[1]?.endsWith('terminal-e2e-helpers.mjs')) {
const args = process.argv.slice(2)
const portIdx = args.indexOf('--port')
const cmdIdx = args.indexOf('--command')
const port = portIdx >= 0 ? Number(args[portIdx + 1]) : 9444
const command = cmdIdx >= 0 ? args[cmdIdx + 1] : null
const term = new OrcaTerminal(port)
console.log('Connecting to Orca on CDP port', port, '...')
term.connect()
console.log('Connected.')
if (args.includes('--discover')) {
const screenshotPath = term.probePtyIdWithScreenshot()
console.log('Sent PTY probes. Check screenshot:', screenshotPath)
}
if (command) {
const ptyId = args[args.indexOf('--pty') + 1]
if (!ptyId) {
console.error('--command requires --pty <id>. Use --discover first to find PTY IDs.')
process.exit(1)
}
term.exec(ptyId, command)
const shot = term.screenshot()
console.log('Executed. Screenshot:', shot)
}
}

View File

@ -81,6 +81,70 @@ describe('registerPtyHandlers', () => {
})
})
/** Helper: trigger pty:spawn and return the env passed to node-pty. */
function spawnAndGetEnv(
argsEnv?: Record<string, string>,
processEnvOverrides?: Record<string, string | undefined>
): Record<string, string> {
const savedEnv: Record<string, string | undefined> = {}
if (processEnvOverrides) {
for (const [k, v] of Object.entries(processEnvOverrides)) {
savedEnv[k] = process.env[k]
if (v === undefined) {
delete process.env[k]
} else {
process.env[k] = v
}
}
}
try {
// Clear previously registered handlers so re-registration doesn't
// accumulate stale state across calls within one test.
handlers.clear()
registerPtyHandlers(mainWindow as never)
handlers.get('pty:spawn')!(null, {
cols: 80,
rows: 24,
...(argsEnv ? { env: argsEnv } : {})
})
const spawnCall = spawnMock.mock.calls.at(-1)!
return spawnCall[2].env as Record<string, string>
} finally {
for (const [k, v] of Object.entries(savedEnv)) {
if (v === undefined) {
delete process.env[k]
} else {
process.env[k] = v
}
}
}
}
describe('spawn environment', () => {
it('defaults LANG to en_US.UTF-8 when not inherited from process.env', () => {
const env = spawnAndGetEnv(undefined, { LANG: undefined })
expect(env.LANG).toBe('en_US.UTF-8')
})
it('inherits LANG from process.env when already set', () => {
const env = spawnAndGetEnv(undefined, { LANG: 'ja_JP.UTF-8' })
expect(env.LANG).toBe('ja_JP.UTF-8')
})
it('lets caller-provided env override LANG', () => {
const env = spawnAndGetEnv({ LANG: 'fr_FR.UTF-8' })
expect(env.LANG).toBe('fr_FR.UTF-8')
})
it('always sets TERM and COLORTERM regardless of env', () => {
const env = spawnAndGetEnv()
expect(env.TERM).toBe('xterm-256color')
expect(env.COLORTERM).toBe('truecolor')
expect(env.TERM_PROGRAM).toBe('Orca')
})
})
it('rejects missing WSL worktree cwd instead of validating only the fallback Windows cwd', () => {
const originalPlatform = process.platform
const originalUserProfile = process.env.USERPROFILE
@ -107,9 +171,7 @@ describe('registerPtyHandlers', () => {
rows: 24,
cwd: '\\\\wsl.localhost\\Ubuntu\\home\\jin\\missing'
})
).toThrow(
'Working directory "\\\\wsl.localhost\\Ubuntu\\home\\jin\\missing" does not exist.'
)
).toThrow('Working directory "\\\\wsl.localhost\\Ubuntu\\home\\jin\\missing" does not exist.')
expect(spawnMock).not.toHaveBeenCalled()
} finally {
Object.defineProperty(process, 'platform', {

View File

@ -164,6 +164,14 @@ export function registerPtyHandlers(mainWindow: BrowserWindow, runtime?: OrcaRun
FORCE_HYPERLINK: '1'
} as Record<string, string>
// Why: When Electron is launched from Finder (not a terminal), the process
// does not inherit the user's shell locale settings. Without an explicit
// UTF-8 locale, multi-byte characters (e.g. em dashes U+2014) are
// misinterpreted by the PTY and rendered as garbled sequences like "<22>~@~T".
// We default LANG to en_US.UTF-8 but let the inherited or caller-provided
// env override it so user locale preferences are respected.
spawnEnv.LANG ??= 'en_US.UTF-8'
let ptyProcess: pty.IPty | undefined
try {
ptyProcess = pty.spawn(shellPath, shellArgs, {