* fix(shell-ready): honor ZDOTDIR without breaking zsh scoping
Fixes#1866
This reimplements PR #1737 (reverted in #1864) with a safer approach
that preserves normal zsh startup semantics.
**Core fix**: Discover ZDOTDIR by sourcing user ~/.zshenv in a subshell
instead of inside a wrapper function. This preserves top-level zsh
scoping for common patterns like `typeset -U path` that broke in the
original implementation.
**Shell safety improvements**:
- Use `printf '%s\n'` instead of `echo` for capturing ZDOTDIR (handles
special characters in paths safely)
- Subshell isolates early returns and side effects from wrapper
**Code quality**:
- Extract duplicated zsh wrapper template to `src/main/shell-templates.ts`
- Both local-pty and daemon paths now share identical wrapper logic
**Test coverage**:
- Add live zsh subprocess tests that spawn real zsh to verify:
- XDG ZDOTDIR discovery works
- `typeset -U path` in .zshrc preserves top-level scoping
- Early returns in .zshenv don't crash the wrapper
- Vanilla (non-XDG) configs fall back to HOME correctly
- Template structure tests validate subshell discovery logic
Before (broken):
```zsh
__orca_source_user_zshenv() {
source "$HOME/.zshenv" # typeset becomes function-scoped
}
```
After (fixed):
```zsh
_orca_discovered_zdotdir=$(
unset ZDOTDIR
[[ -f "$HOME/.zshenv" ]] && source "$HOME/.zshenv" 2>/dev/null
printf '%s\n' "${ZDOTDIR}"
)
export ORCA_ORIG_ZDOTDIR="${_orca_discovered_zdotdir:-${_orca_spawn_orig_zdotdir:-$HOME}}"
```
The subshell sources .zshenv at top-level (preserving normal scoping),
captures only the ZDOTDIR value, then exits. User rcfiles (.zshrc, etc.)
are still sourced at the wrapper's top level, so all scoping works normally.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* Add shell-script-literal test framework and improvements
Adds a new declarative test framework for shell-ready tests that uses
literal shell scripts (copy-pastable into terminals) with inline snapshots.
Framework features:
- Shell scripts as string literals with # Run: marker to split setup/test
- Direct script execution (no brittle parsing) via temp files
- Path normalization for reproducible snapshots (<HOME>, <WRAPPER_DIR>)
- Auto-detects shell from command, supports bash/zsh/sh
- Inline snapshot testing with vitest toMatchInlineSnapshot()
Code quality improvements:
- Extract escapeRegex to shared string-utils.ts (deduplicates 2 copies)
- Refactor shell-templates.ts for readability (condense comments, add structure)
- Pre-compile regex patterns to avoid hot-path allocation
- Fix path normalization to sort by length (prevent nested path corruption)
- Fix actualUserHome handling to skip empty values
All tests passing (64/64 shell-ready tests, 55/55 affected tests).
Files added:
- src/main/providers/__tests__/shell-ready-framework/shell-script-test.ts
- src/main/providers/__tests__/shell-ready-framework/README.md
- src/main/providers/__tests__/shell-ready-framework-example.test.ts
- src/shared/string-utils.ts
Files modified:
- src/main/shell-templates.ts (readability cleanup)
- src/main/codex/config-toml-trust.ts (use shared escapeRegex)
- src/main/daemon/shell-ready.test.ts (updated for new framework)
- src/main/providers/local-pty-shell-ready.test.ts (updated for new framework)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* fix(shell-ready): preserve zshenv semantics
---------
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>
Adds OMP as a first-class TUI agent and keeps Pi/OMP overlay state isolated across local and relay PTY paths.\n\nValidated locally with focused Vitest coverage, typecheck, lint, git diff --check, and Electron manual verification using installed omp (omp/15.3.2).
Strip ELECTRON_RUN_AS_NODE from daemon-spawned PTY environments so user shells do not inherit Electron's internal Node-mode flag. Adds regression coverage for the daemon PTY env boundary.
Fixes#2414.
Fix Linux/bash agent status cleanup by emitting OSC 133 command lifecycle markers from bash shell wrappers, including local, daemon, and SSH relay paths.
Reverts #1737 because sourcing user .zshenv inside a wrapper function can change zsh scoping behavior for common startup-file patterns. We will revisit with a safer targeted fix.
The wrapper .zshenv used to capture ORCA_ORIG_ZDOTDIR before sourcing the
user's ~/.zshenv, so users who set ZDOTDIR=$XDG_CONFIG_HOME/zsh in .zshenv
ended up with the wrapper sourcing ~/.zshrc instead of
~/.config/zsh/.zshrc. Functions, aliases, and PATH edits silently
disappeared inside every shell-ready PTY (startup-command, attribution
shim, SSH).
Reorder: clear ZDOTDIR, source user .zshenv inside a function (so an early
`return` doesn't abort our wrapper), recapture from user-resolved →
spawn-env → HOME, then re-pin ZDOTDIR to the wrapper dir baked into the
template. Trailing-slash normalization now matches the Node-side
normalizer. Same change in the renderer- and daemon-side wrappers.
New zsh-subprocess tests (gated on `which zsh`) cover the XDG case,
early-return survival in user .zshenv, spawn-env preservation, HOME
fallback, and self-loop normalization including pathological multi-
trailing-slash paths.
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
* WIP: Changes before auto-review fixes
Co-authored-by: Orca <help@stably.ai>
* fix: address auto-review-fix-multi-agent findings
- Replace local ORCA_WORKTREE_ID_SEPARATOR with shared WORKTREE_ID_SEPARATOR
- Make hydrateLocalPtyRegistryAtBoot idempotent (one-shot per process,
but stays retry-eligible until daemon provider is available)
- Strengthen daemon-pty-adapter strict-parser test to actually exercise
the new short-circuit (test would have passed under the old loose
parser too without the change)
- Add eslint-disable max-lines directive to oversized merge test file
Co-authored-by: Orca <help@stably.ai>
* chore: archive auto-review context to .context/
Co-authored-by: Orca <help@stably.ai>
* fix: address auto-review-fix findings
Drop the destructive reconcileOnStartup call from boot-time PTY registry
hydration: a transient listRepoWorktrees failure (returns [] and only
warns) would otherwise let the reconcile pass kill live local sessions.
The boot path is now read-only against the daemon — listSessions() only.
Also: tighten parsePtySessionId to reject degenerate `::` halves; replace
stale pty.ts:1005 references and a misleading local-unknown comment in
the hydrate module; narrow Store dependency to Pick<Store, 'getRepos'>;
log adapter listSessions failures instead of silently swallowing them;
re-anchor design-doc references on stable symbols and align §1b/§1c/§1d
with the implementation.
Co-authored-by: Orca <help@stably.ai>
* docs(resource-usage): update remote badge spec
Co-authored-by: Orca <help@stably.ai>
* test(resource-usage): cover boot hydration failure modes + warm-reattach e2e
Adds the regression coverage flagged in PR #1667's test plan that wasn't
already locked down.
vitest (`hydrate-local-pty-registry.test.ts`):
- daemon offline at first call → no-op, hasHydrated stays false so a
later macOS dock re-activation can retry.
- listSessions rejection caught and logged, does not throw.
- pid-write ordering: a pre-existing registry entry with pid=12345 is
not clobbered by a stale `pid: null` from listSessions (§1d).
- SSH-gate: a session whose repo has a non-null connectionId stays out
of the registry, mirroring the spawn-time gate in pty.ts.
- Happy-path: a local session is registered with the daemon's pid.
Playwright e2e (`resource-usage-warm-reattach.spec.ts`):
Full quit→relaunch cycle against the same userDataDir; asserts that
on the second launch the snapshot includes the warm-reattached PTY
with a real pid before any pane mount, and that the seeded repo
resolves as local (no connectionId). Mirrors the existing
terminal-restart-persistence pattern.
Co-authored-by: Orca <help@stably.ai>
* fix(test): satisfy Pick<Store, 'getRepos'> in hydrator vitest
CI typecheck failed because FakeStore's getRepos returned objects missing
Repo's required fields (path, displayName, badgeColor, addedAt). Fill with
placeholder values; the hydrator only reads id + connectionId, but the
type signature still has to line up.
Co-authored-by: Orca <help@stably.ai>
* chore(resource-usage): drop bug-doc files; strip dead doc refs from comments
Remove docs/resource-usage-remote-mislabel.md (new in this PR) and revert
docs/resource-usage-merge-spec.md to the PR-base state. Strip the
matching `docs/...md §N` pointers from code/test comments, keeping the
surrounding "why" explanations intact so readers still get the
warm-reattach mislabel context.
Co-authored-by: Orca <help@stably.ai>
---------
Co-authored-by: Orca <help@stably.ai>
Added process.on('exit') restore handler so the renamed spawn-helper is restored even if the vitest worker is killed mid-test.
Findings addressed:
- [low] src/main/daemon/node-pty-fd-leak.test.ts:57 — Test renames helper binary; crash leaves it hidden
Rebased onto current main to resolve conflicts.
Co-authored-by: orca-bot <bot@stably.ai>
* Improve node-pty spawn diagnostics
* Preserve original error stack when adding node-pty recovery hint
Mutate the existing Error's message instead of replacing the object so
the original stack trace and custom fields survive into telemetry/logs.
Co-authored-by: Orca <help@stably.ai>
---------
Co-authored-by: Orca <help@stably.ai>
Addresses two SIGKILL/wake-routing bugs:
- setWorkspaceSession now merges prior tab.ptyId / ptyIdsByLeafId
when the incoming snapshot's binding is empty, closing the second
half of the SIGKILL race that could clobber persistPtyBinding.
- DaemonPtyRouter.shutdown skips sessionAdapters.delete when
keepHistory=true so legacy-adapter sessions wake on the right
adapter on resume.
Co-authored-by: orca-bot <bot@stably.ai>
* fix(pty): release ptmx fd on natural exit + defuse SIGHUP-to-recycled-pid
Daemons accumulated ptmx fds over time because node-pty's UnixTerminal
only releases the master fd when destroy() runs. On the natural-exit
path (the common case — user closes a tab, shell runs `exit`) nothing
ever calls destroy(), so the fd leaks until GC. On macOS this
eventually hits kern.tty.ptmx_max=511 and all new terminals fail to
spawn.
Fix: release the fd synchronously on every teardown path (natural
exit, explicit kill, stale SSH spawn, daemon shutdown) and close the
concurrent SIGHUP-to-recycled-pid hazard inside node-pty's
UnixTerminal.destroy().
- src/main/daemon/pty-subprocess.ts: synchronous POSIX proc.kill
neutralization inside proc.onExit; dead guards on forceKill/signal
so they never target a reaped-and-possibly-recycled pid
- src/main/daemon/session.ts: new disposeSubprocess() for already-
exited sessions (fd release only, no SIGKILL) — avoids sending
SIGKILL to a recycled pid during daemon shutdown
- src/main/daemon/terminal-host.ts: dispose loop routes on isAlive —
live sessions get forceKillAndDisposeSubprocess (SIGKILL + fd
release), exited sessions get disposeSubprocess (fd release only)
- src/main/providers/local-pty-provider.ts: same POSIX kill
neutralization at top of onExit for the legacy local path
- src/relay/pty-handler.ts: same neutralization in wireAndStore;
disposed flag guards all public entry points; dispose() uses
SIGKILL (not SIGTERM) before destroy since the relay is exiting;
killTimer fallback + immediate-shutdown + stale-spawn cleanup all
call disposeManagedPty + ptys.delete so wedged children (D-state,
bad NFS) can't leak map entries against the 50-PTY cap
Windows is exempt everywhere — WindowsTerminal.destroy IS a kill()
call internally (closes the ConPTY agent), so neutralizing would
turn destroy into a no-op and leak the agent.
See docs/fix-pty-fd-leak.md for the full design.
Co-authored-by: Orca <help@stably.ai>
* fix(pty): patch node-pty native off-by-one leaking /dev/ptmx per spawn
node-pty 1.1.0's pty_posix_spawn on macOS walks low_fds[0..2] in an
allocation loop that breaks at the first fd >= STDERR_FILENO, then
cleans up via `for (; count > 0; count--) close(low_fds[count])`. In
the typical case (break at count=0) the cleanup body never runs and
low_fds[0] — a /dev/ptmx handle — leaks per spawn. Fixed upstream in
microsoft/node-pty af053f2 (PR #882), not in any 1.1.0 release.
Backport the 3-line cleanup-loop fix as a pnpm patch. E2E validated
against a dev daemon: 200 spawn/kill cycles kept the daemon's ptmx
fd count flat at baseline; prior runs reproduced linear 1-per-spawn
growth. Also documents the native root cause as a status addendum in
docs/fix-pty-fd-leak.md — the JS-side destroy() discipline previously
landed is still load-bearing for the SIGHUP-to-recycled-pid hazard and
for synchronous fd release on daemon shutdown.
Co-authored-by: Orca <help@stably.ai>
* fix(pty): capture stable kill spy ref in pty.test.ts
destroyPtyProcess reassigns proc.kill = () => {} on POSIX to defuse
the SIGHUP-to-recycled-pid hazard (see docs/fix-pty-fd-leak.md). After
that reassignment, proc.kill.mock is undefined and the assertions
crashed in CI. Capture a stable reference to the vi.fn() before it
gets reassigned.
Co-authored-by: Orca <help@stably.ai>
---------
Co-authored-by: Orca <help@stably.ai>
* fix(daemon): guard killStaleDaemon against pid recycling
The daemon's pid file carried only a bare integer, so killStaleDaemon had
no way to verify the current owner of that pid was still the process it
forked. Unix pids recycle — on macOS the default ceiling is ~2048 and
wraps in minutes under load — so blind SIGTERM/SIGKILL on a recycled pid
could hit an unrelated user process (editor, language server, background
task) silently.
Three moving pieces:
1. Pid file now carries startedAtMs. daemon-spawner exports DaemonPidFile
and serializeDaemonPidFile; daemon-init fills startedAtMs from
getProcessStartedAtMs(child.pid) right after the daemon signals ready.
parseDaemonPidFile tries JSON first and falls back to bare-integer for
backward compatibility (startedAtMs: null on legacy files).
2. isDaemonProcess takes expected startedAtMs. In addition to the cmdline
match, it consults getProcessStartedAtMs(pid) and compares with a
±1.5s tolerance. Null on either side is fail-open so the guard never
strengthens existing behavior negatively (Windows, legacy pid files,
kernel-thread /proc failures).
3. SIGKILL re-check. The SIGTERM-then-wait window is up to 3s — long
enough for the pid to be recycled if the original daemon dies during
the wait. Before escalating to SIGKILL, isDaemonProcess runs again;
on mismatch we log reason=pid_recycled and skip SIGKILL.
Carried forward from #1323 as a standalone safety fix — independent of
that PR's router/drain machinery, which is not shipping.
Co-authored-by: Orca <help@stably.ai>
* test(daemon): cover pid-recycling guard surface in daemon-health
Adds unit coverage for the new Phase 0 surface:
- parseDaemonPidFile: JSON round-trip, JSON without startedAtMs,
bare-integer fallback for legacy pid files, malformed-input rejection.
- startTimeMatches: null-expected fail-open, null-actual fail-open,
within-tolerance match, outside-tolerance rejection.
- killStaleDaemon: with a mismatched startedAtMs in the pid file,
assert that no SIGTERM/SIGKILL is sent even though the liveness probe
(process.kill(pid, 0)) runs.
startTimeMatches was promoted from module-private to exported so it can
be exercised directly — it's a pure function with no internal state.
Co-authored-by: Orca <help@stably.ai>
* feat(settings): manage sessions panel for daemon staleness UX
Add a Manage Sessions settings panel with list/kill-all/kill-one/restart
backed by a new pty:management IPC surface. Rows are hover-highlighted and
click to reveal the corresponding terminal pane, mirroring the bottom
status-bar sessions popover. Kill-all and restart-daemon are icon buttons
(Trash2, RotateCw) with tooltips so the restart action doesn't collide with
the row-refresh RefreshCw icon.
Co-authored-by: Orca <help@stably.ai>
* fix(settings): honest killAll counts + suppress post-kill spawn toast
killAll now snapshots the initial session IDs and polls listSessions every
100ms for up to 6.5s — past the daemon's 5s SIGTERM→SIGKILL ladder — so
well-behaved shells hosting long-running agents finish their SIGTERM
handlers before we classify them as "refused to exit." Shutdowns fire once
per initial session (no retry spam), and fresh session IDs that appear
mid-poll (renderer remounts) don't inflate remainingCount.
pty-transport's connect() catch now detects the adapter's
TerminalKilledError tombstone rejection ("...was explicitly killed") and
suppresses the red "file an issue" toast. After Kill All, a pane remount
would call pty:spawn on the dead session ID; surfacing the tombstone as
a scary error misrepresented an intentional user action. The pane still
renders "Process exited" via the normal lifecycle.
Co-authored-by: Orca <help@stably.ai>
* feat(daemon): foundation for pty:management IPC surface
Adds the plumbing the Manage Sessions settings panel depends on:
- daemon-init exports getDaemonProvider / replaceDaemonProvider /
restartDaemon / cleanupDaemonForProtocol so the pty:management
handlers can access the current provider and coordinate a clean
restart without importing window-services internals.
- daemon-pty-router exports getAllAdapters so the killAll / listSessions
handlers can fan across the current adapter plus any legacy-protocol
adapters still attached for in-flight sessions.
- daemon-pty-adapter gains a listSessions RPC and readonly
protocolVersion so the handlers can annotate each session with the
adapter it belongs to and route killOne back to the right adapter.
- types.ts exports DaemonSessionInfo (SessionInfo + protocolVersion)
as the shared shape the preload API surface mirrors.
- ipc/pty.ts, attach-main-window-services, TerminalPane and
terminal-search pick up the small bindings required to wire the
router through existing code paths without regressions.
Co-authored-by: Orca <help@stably.ai>
* fix(settings): remove high-session-count warning banner
The banner nagged at 20 sessions, which is well within normal use for
users with many open worktrees. Count is already visible in the header
bar, and the table supports per-row and bulk kills, so the banner added
noise without actionable value.
Co-authored-by: Orca <help@stably.ai>
---------
Co-authored-by: Orca <help@stably.ai>