Skip to content

fix(process-table): read ps under a pinned locale - #27005

Closed
outof-place wants to merge 1 commit into
stablyai:mainfrom
outof-place:fix/ps-locale-independent-start-time
Closed

outof-place wants to merge 1 commit into
stablyai:mainfrom
outof-place:fix/ps-locale-independent-start-time

Conversation

@outof-place

Copy link
Copy Markdown
Contributor

ELI5

Orca finds out what runs in each terminal by asking the ps tool and reading its printout. ps prints dates the way the user's language settings say, so on a Mac set to New Zealand, German or Japanese English the dates come out in a different order or language. Orca's reader then lost track of which column was which and stopped recognizing agents. With no language setting at all, ps garbled non-English characters in program arguments instead. Orca now tells ps exactly which locale to print in, so the output always has the shape Orca reads.

What Changed

  • Before: with LANG or LC_* set to a non-US locale, the macOS full process capture read every row wrong. Under en_NZ, ps -o lstart= prints Sat 10 Oct 00:04:51 2026 rather than Sat Oct 10 …. The parser's start-time pattern then fails, and the row falls through to a pattern that puts the terminal name and the start time into command. A real row read like this: command: '?? Fri 9 Oct 16:56:02 2026 /Applications/Orca.app/…', with no tty and no startTime. Consequences:

    • Agent recognition saw ?? or ttys003 as the program, so no agent was found in the full capture.
    • Pane fingerprints never formed, because the start marker was missing.
    • Date.parse returned NaN for most localized forms (Sa. 10 Okt., 土 10/10), which broke the agent-session start-time probe and the daemon pid identity check.

    Orca gets a locale whenever it starts from a terminal: pnpm dev, open from a shell, or a relay on an SSH host with a forwarded LANG. With no locale at all (a Dock-launched Orca), ps used the C locale and printed non-ASCII argv in vis(3) meta notation, so x-ł-日本 read as x-M-EM^B-M-fM^WM-%M-fM^\M-,.

  • After: every ps whose lstart= or command= Orca parses gets a pinned locale. Start times always read Sat Oct 10 00:04:51 2026, and argv keeps its real UTF-8.

  • How: a new src/shared/ps-locale-environment.ts builds the environment, used by:

    • the full and shell-foreground captures (src/shared/process-table-snapshot-reader.ts)
    • the cheap tier (src/shared/cheap-process-table-snapshot-reader.ts)
    • the launch-paste terminal lookup (src/main/runtime/terminal-foreground-group.ts)
    • the agent-session start-time probe (src/main/runtime/agent-session-process-identity-probe.ts)
    • the local-time daemon identity query (src/main/daemon/daemon-process-identity-query.ts)

    Per platform:

    • macOS: drop LANG and every LC_*, then set LC_ALL=en_US.UTF-8.
    • Linux: set LC_TIME=C. A user's LC_ALL is moved to LC_CTYPE, because it would otherwise override LC_TIME and its character set still has to apply.
    • Windows: unchanged.

    Call sites that already pin LC_ALL=C, such as the hook identity capture or the UTC daemon identity, are untouched. Their strings already have this shape.

Why

  • macOS needs a uniform locale. I ran macOS ps -o lstart=,command= on a process with x-ł-日本 in its argv under eight user locale setups:

    • nothing set
    • LANG=en_NZ, LANG=de_DE, LANG=ja_JP, LANG=C
    • LC_ALL=en_NZ
    • LANG + LC_TIME + LC_CTYPE mixed
    • LC_ALL + LC_TIME + LC_MESSAGES mixed

    Results:

    • Leaving the environment alone breaks either the date (any non-US locale) or the argv (no locale).
    • LC_TIME=C alone fails whenever LC_ALL is set, and still mangles argv when nothing else is set. macOS ps falls back to the C character set as soon as the categories differ.
    • Stripping the locale variables and setting LC_ALL=en_US.UTF-8 gave a US lstart and intact argv in all eight. en_US.UTF-8 ships with every macOS.
  • Linux can keep the user's character set. In Debian with procps-ng 4.0.4, lstart keeps its field order but localizes the names (Fr Okt 9 22:08:59 2026 under de_DE). LC_ALL=C replaces non-ASCII argv with ?. glibc honors a lone LC_TIME=C, so Linux gets English names while keeping the user's character set.

  • Alternatives considered:

    • A locale-tolerant parser cannot know every %c. Japanese and Polish orders differ, and Date.parse rejects most of them.
    • macOS ps has no locale-independent start-time column: etime changes every second and start is coarser.
    • Reading the kernel directly is what perf(macos): avoid ps during terminal repaint signals #26985 does, for macOS only, and it still needs this for its ps fallback and for Linux.

Linked Issue

Fixes #27004

Visual Proof

N/A: no UI change. The difference is which agents the panes and sidebar recognize when Orca runs with a non-US locale.

Testing

Tested on macOS 27 (M4 Max). Linux procps behavior was checked in Docker (debian:stable-slim, procps-ng 4.0.4, de_DE/en_NZ/en_US generated).

  • src/shared/process-table-snapshot-locale.test.ts: the regression test with an en_NZ fixture.

    • The parser cannot read an en_NZ row: tty and startTime are lost and command starts with the terminal.
    • The full capture, with ps mocked to answer in en_NZ unless the pinned locale is passed, returns the right tty, startTime and agent command while LANG=en_NZ.UTF-8.
  • src/shared/ps-locale-environment.test.ts:

    • Unit cases: macOS pinning, Linux LC_ALL → LC_CTYPE with LC_TIME=C, Windows untouched.
    • On macOS, the real ps with en_NZ, ja_JP, mixed LC_ALL/LC_TIME and no locale. Each case must give a Date.parse-able US lstart and argv equal to orca-ł-日本 30.
    • A case showing the user's locale alone breaks one or the other.
  • daemon-process-identity-time-zone.test.ts: the local-time daemon identity now passes the pinned environment.

  • Commands run: pnpm tc, every test file that touches the changed modules (35 files, 412 passing), oxlint, oxfmt, the max-lines ratchet, and ORCA_CODE_QUALITY_BASE=upstream/main pnpm run check:code-quality:changed.

  • Not tested by hand: Linux beyond the Docker check of procps itself, and Windows, which has no ps and keeps its environment as is.

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

AI Disclosure

Claude Code (Claude Opus 5.5) found the bug while checking ps parity for #26985, wrote the fix, the tests and this description, and ran the verification above.

Review

Start with src/shared/ps-locale-environment.ts; each call site only adds env: psLocaleEnvironment().

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • Security: only locale variables change, and only for ps.
  • Cross-platform: macOS and Linux each get their own recipe; Windows is untouched.
  • SSH: the relay runs these readers on the remote host, so a forwarded LANG on an SSH host no longer breaks them.
  • Backwards compatibility: start-time strings now always have the en_US/C shape. That is what a Dock-launched Orca already produced and what the LC_ALL=C call sites produce, so comparisons with them keep holding. TZ is not touched.
  • Performance: unchanged.
  • Interaction with perf(macos): avoid ps during terminal repaint signals #26985: that PR's in-process reader formats lstart in the C locale to match this.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred). Typecheck, lint on the changed files, the quality gate and the affected test files pass locally; CI covers the full suite.

🤖 Generated with Claude Code

macOS ps prints lstart with the locale's %c, so en_NZ gives
"Sat 10 Oct 00:04:51 2026" where the parsers expect "Sat Oct 10 ...". The
full capture then folds tty and the start time into command, which hides
every agent and stops pane fingerprints from forming. Date.parse rejects
most localized forms too, which breaks the agent-session and daemon
identity probes. With no locale at all, ps instead mangles non-ASCII argv
into vis(3) meta notation.

psLocaleEnvironment gives every ps whose lstart or command Orca parses one
uniform en_US.UTF-8 on macOS. A lone LC_TIME=C does not work there: LC_ALL
overrides it, and ps falls back to the C character set once categories
differ. On Linux, glibc honors LC_TIME=C; procps keeps the field order but
localizes the names, and LC_ALL is moved to LC_CTYPE so it cannot override
LC_TIME. Call sites that already pin LC_ALL=C are unchanged.

Fixes #27004

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

📝 Walkthrough

Walkthrough

The change adds psLocaleEnvironment, which applies platform-specific locale settings. Process identity queries, foreground process-group queries, and process-table readers now pass the helper’s environment to ps. Tests cover the helper’s platform rules, macOS locale behavior, and process-table parsing and capture.

Fixed issue severity: Medium


Priority: ➖ Normal

Merge Risk: 🔵 Low · up to c1f2c

The locale change appears mergeable with a small test-cleanup fix: restore an absent LANG variable by deleting it so later tests inherit the intended environment.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 9 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description check Passed The description follows the repository template. It explains the problem, changes, rationale, linked issue, testing, platform considerations, and checklist status. The unchecked full-suite command ite…
Title check Passed The title is concise, specific, and accurately summarizes the primary change: running process-table ps commands under a pinned locale.
Linked Issues check Passed The PR satisfies the coding requirements in [#27004]. psLocaleEnvironment pins macOS output to LC_ALL=en_US.UTF-8, removes conflicting locale variables, and preserves the required UTF-8 command ar…
Out of Scope Changes check Passed The changes stay within [#27004]. The new helper, its tests, the process-table regression tests, and the listed ps call-site updates directly support stable process-output parsing and identity check…


  • Fix all pre-merge checks with AI
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 2090775b-3f12-40c2-87f5-421205651133
📥 Commits

Reviewing files that changed from the base of the PR and between 8848ca9 and c1f2c73.

📒 Files selected for processing (9)
  • src/main/daemon/daemon-process-identity-query.ts
  • src/main/daemon/daemon-process-identity-time-zone.test.ts
  • src/main/runtime/agent-session-process-identity-probe.ts
  • src/main/runtime/terminal-foreground-group.ts
  • src/shared/cheap-process-table-snapshot-reader.ts
  • src/shared/process-table-snapshot-locale.test.ts
  • src/shared/process-table-snapshot-reader.ts
  • src/shared/ps-locale-environment.test.ts
  • src/shared/ps-locale-environment.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

Comment on lines +24 to +33
beforeEach(() => {
Object.defineProperty(process, 'platform', { value: 'darwin' })
process.env.LANG = 'en_NZ.UTF-8'
execFileMock.mockReset()
resetProcessTableSnapshotForTests()
})
afterEach(() => {
Object.defineProperty(process, 'platform', platform)
process.env.LANG = lang
})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Restore LANG correctly when it was originally unset.

Line 32 assigns process.env.LANG = lang. If LANG was unset, lang is undefined. Node then stores the string "undefined" in process.env. This pollutes the environment for later tests in the same worker. Delete the key when the original value is undefined.

Proposed fix
 afterEach(() => {
   Object.defineProperty(process, 'platform', platform)
-  process.env.LANG = lang
+  if (lang === undefined) {
+    delete process.env.LANG
+  } else {
+    process.env.LANG = lang
+  }
 })
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
beforeEach(() => {
Object.defineProperty(process, 'platform', { value: 'darwin' })
process.env.LANG = 'en_NZ.UTF-8'
execFileMock.mockReset()
resetProcessTableSnapshotForTests()
})
afterEach(() => {
Object.defineProperty(process, 'platform', platform)
process.env.LANG = lang
})
beforeEach(() => {
Object.defineProperty(process, 'platform', { value: 'darwin' })
process.env.LANG = 'en_NZ.UTF-8'
execFileMock.mockReset()
resetProcessTableSnapshotForTests()
})
afterEach(() => {
Object.defineProperty(process, 'platform', platform)
if (lang === undefined) {
delete process.env.LANG
} else {
process.env.LANG = lang
}
})

@nwparker

Copy link
Copy Markdown
Contributor

Thanks so much for this fix, @outof-place! I couldn't push to your fork's branch, so I rebased it onto current main, tightened it up a little (macOS-only LC_ALL=en_US.UTF-8, plus one more daemon-inspection call site), and opened #33898 to land it. Your authorship is kept on the commit. Closing this one in favor of #33898.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: ps output is parsed in the user's locale, so non-US locales hide agents and break process identity

2 participants