mirror of
https://github.com/stablyai/orca.git
synced 2026-10-25 08:04:05 +00:00
98bdd653abcb12e2b8abba57fa0f2aa9ed636110
3406
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Brennan Benson
|
98bdd653ab |
fix(native-chat): stop the spinner on a not-yet-flushed transcript (#16493)
* fix(native-chat): stop the spinner on a not-yet-flushed transcript A brand-new agent session can take minutes to write its first JSONL line, and one that is never prompted never writes it at all. The host emitted no stream frame until the file resolved, so every native-chat client sat on a bare spinner with the composer enabled but the transcript blank -- forever, in the never-prompted case. The resolve poll now reports the transcript as pending after a short grace, and both host handlers emit a `pending: true` snapshot. It is deliberately not a plain empty snapshot: an empty window sold as a settled read would capture over retained history and unblock consumers that require a trustworthy transcript (the launch-draft adoption would re-offer a prompt the agent may already have taken). Clients render it as the "start a chat" empty state while keeping the read unsettled -- `awaiting-transcript` on mobile, an `awaiting` read phase on desktop, which also stops the seed loop expiring into an error card for a session that is simply new. New optional field only, so older clients ignore it and still stop spinning. * fix(native-chat): negotiate pending transcript frames |
||
 Jinjing
|
e361da7fb7 |
Deleting skill (#16357)
* Add skill deletion with cross-platform transaction safety Implements end-to-end skill removal with placement enumeration, dependency guards, and transactional recovery. Covers native, WSL, and remote hosts; users can delete canonical directories and alias placements (symlinked directories or files) in a single atomic batch. Includes UI selection flow, preview, confirmation, and results band. Block reasons (bundled, plugin, unowned, stale) gate deletions that would fail or contradict user intent. * Organize IPC handlers into module subdirectories Move register-core-handlers and skill-delete-ipc-handlers into dedicated subdirectories for improved code organization and to reduce the flat structure in src/main/ipc/. * Make skill deletion recovery transactions idempotent Defer journal cleanup until both staging removal and receipt cleanup succeed, leaving the journal in place for startup to retry if either operation fails. This ensures the recovery process is safe to run multiple times without leaving partially-deleted skills. * Consolidate skill-delete files into dedicated module Reorganize skill deletion functionality into a modular structure under `src/main/skills/skill-delete/` with simplified file names. Remove the redundant `skill-delete-` prefix from file names since they now live in the dedicated directory. Update all import paths throughout the codebase to reflect the new structure, including imports from IPC handlers and RPC methods. * Fix broken import paths and add deletion robustness improvements Import paths using `..//'` were invalid and broken. Replace with explicit module names (`skill-discovery-sources`, `skill-install-filesystem`, etc.) to clarify dependencies. - Bind WSL filesystem methods to preserve `this` context - Keep recovery journal when rollback rename fails, so startup can retry - Skip symlink-based tests on Windows where they cannot run - Only treat ENOENT/ENOTDIR as empty directories; propagate other errors - Fix cross-platform path parent calculation to handle drive roots - Replace shared constant with localized string for user-facing message - Use `runProcess` for WSL integration test instead of bare `execFile` * Add batch limit for skill deletion and improve host availability checkin - Limit concurrent deletions to prevent remote host overload - Add retry logic for capability probing to handle transient unavailability - Add reprobe() method to recheck capability after errors or user refresh - Fix status logic: receipt cleanup is best-effort, completion depends only on content removal - Improve error message for unreachable hosts |
||
 Neil
|
4218d5068e |
fix(cli): seed nvm's default version, not the newest install (#16420)
* fix(cli): seed nvm's default version, not the newest install #16314 stopped the login-shell probe inheriting the seeded PATH, but left the seed itself picking the newest installed nvm version. That ordering decides which node a CLI runs under whenever the probe does not land — a timeout, or a login shell whose rc never initializes nvm — and newest is precisely the wrong guess: it is usually the version the user just added and has installed nothing into. That is the root cause reported in #10932. Resolve `alias/default` instead, mirroring nvm: follow the alias chain (`default` -> `lts/*` -> `lts/krypton` -> a version), resolve a partial version like `24` to the highest matching install, and treat `system`/`node`/`stable` as no preference. The chain is bounded and cycle-guarded because nvm's own resolver tracks seen aliases and hand-edited files can point at each other. Ordering is a preference, not a restriction: the remaining versions stay behind the default, so a CLI installed outside it is still reachable. Measured on a real machine with nvm default=24 and a bare v26.7.0 installed: the old resolver seeds v26.7.0/bin (no CLIs), the new one seeds v24.18.0/bin (every CLI). Tests were written first and verified to fail on the three bug cases against main before the fix existed. Also raise the probe budget from 5s to 10s. The old value was never measured against a real profile: a bash -ilc loading nvm, rvm, conda and gcloud takes ~1s idle but 6-7s on a loaded machine, so a cold start under load silently fell back to the seed. Startup does not block on the probe, and the one awaited consumer is agent detection, which is better served by a probe that finishes late than one that gives up early. * fix(cli): reject non-version alias tokens instead of matching v0.x Review finding, and a real bug I introduced. parseVersionSegment coerces every unparseable segment to 0, so an unresolvable default alias — `garbage`, `iojs`, `lts/nonexistent`, any hand-named alias — became [0] and prefix-matched a `v0.12.x` install, or any stray non-version directory. Orca would then seed a decade-old node as the preferred runtime. Real nvm answers N/A for all of them. The `wanted.length === 0` bail could never have caught this: ''.split('.') is [''], never empty. Replaced with a shape check that still admits legitimate numeric prefixes — verified against nvm itself, which resolves `24` to v24.18.0 and `0` to an installed v0.x while answering N/A for the rest. Also corrects two comments that no longer described the code: the seed is no longer "newest install", and the probe budget note claimed startup never blocks on hydration, which is false on packaged Windows where it gates terminal services and git. The traversal-guard comment claimed a containment join() already normalizes away; the real guarantee is that matchNvmVersion can only return an entry of the versions directory. * fix(cli): match nvm's version-token grammar, not just its first character Round-2 review finding, and the same bug one layer down. The previous guard anchored only the first character, but parseInt stops at the first non-digit, so `0x18`, `00` and `0abc` still parsed to [0] and prefix-matched a v0.12.x install — the decade-old-node seed the earlier fix was supposed to close. Reachable: `nvm alias default 0x18` warns that the version does not exist and writes the alias anyway, then resolves it to N/A. Use nvm's actual grammar, leading zeros included — nvm calls `00` and `024` N/A while parseInt reads them as 0 and 24. Verified by executing 17 tokens against a five-version fixture: every one now agrees with nvm, including the legitimate prefixes `0`, `0.12`, `24` and `v24.18.0`. Also drops a dead disjunct (the hop bound already caps the loop, so seen.size can never exceed it) and corrects the log comment in index.ts, which still told the reader a failed probe leaves the newest install in front. It leaves the default version in front now, which is usually survivable but still not what the shell would have resolved. * test(cli): skip the lts/* chain fixture on Windows Round-3 review finding. makeNvmHome materializes each alias as a real file, and the chain case uses nvm's actual `lts/*` alias — `*` is a reserved Win32 filename character, so writeFileSync fails with EINVAL. PR CI runs a Windows allowlist that excludes this file, so the breakage only reaches a Windows developer running the suite locally. Skipped rather than renamed: `lts/*` is the alias nvm really ships, and the assertion pins platform: 'darwin' anyway, so the real name costs no coverage. Matches the skipIf convention already used across src/shared. Also reflows a comment line that a previous edit ran to 143 characters; oxfmt does not reflow comments, so nothing would have caught it. |
||
 NeilandNeil
|
822087c8ec |
refactor(git): split runner.ts into focused command-runner modules (#16395)
* refactor(git): split runner.ts into focused command-runner modules * chore(ratchets): repoint child_process and wsl.exe allowlists at the split modules --------- Co-authored-by: Neil <n@example.com> |
||
|
Neil
|
1f39c93b01 | refactor(ipc): split ssh.ts into focused modules (#16394) | ||
 NeilandSeongho.Bak
|
fcf55f2d68 |
fix(terminal): stop Orca mangling the OMP/Pi title it writes itself (#16381)
* fix(terminal): collapse identity group in the title churn signature Replaces the ingest-time title rewrite from #16373 with a non-destructive fix at the actual cause. The churn suppressor `isDecorativeAgentTitleFrameChange` keyed on the literal label, so `working:OMP` and `working:Pi` compared unequal and every alternating frame from a wrapped harness committed a store patch. #16373 made the labels agree by rewriting the stored title to the tab's launch owner — but `runtimePaneTitlesByTabId` is also the Windows Shift+Enter byte-encoding input, so normalizing at ingest destroyed evidence other consumers read (fixed separately in #16376). Collapse the identity group inside the signature instead. Which member of a group a frame names is decoration, exactly like the spinner glyph the signature already strips, so frames compare equal without touching what is stored. Suppression now changes only WHETHER a frame commits, never WHAT it says. Also fixes the flap under a multiplexer (#8032): the collapse runs over wrapper segments, so "zsh | ⠋ Pi" and "zsh | ⠙ OMP" compare equal, which the anchored owner-relabel in #16373 never matched. Reverts the store changes from #16373 and drops the helper it added. Co-authored-by: Seongho.Bak <49228032+psh4607@users.noreply.github.com> * fix(terminal): fold only bare identity frames into the group token A legacy "π - <session> - <cwd>" title is Pi-compatible too, so folding every profile match collapsed two different sessions to the same signature and suppressed the change outright — reintroducing #16093 through the churn signature. Fold only exact bare identity frames, matched per wrapper segment, so semantic session titles keep comparing on their own text. Co-authored-by: Seongho.Bak <49228032+psh4607@users.noreply.github.com> * docs(terminal): correct the flap diagnosis in the repro header Verified against the OMP source: it emits only π-glyph frames (`DEFAULT_TERMINAL_TITLE = "π"`, title-generator.ts:25), and on an Orca-hosted pane its native titler cedes to Orca's own injected extension, which writes `⠋ π - <session> - <cwd>`. So OMP emits neither "OMP" nor "Pi". Both flap sides are Orca's: "OMP" from driveSyntheticTitleFromHook, "Pi" from normalizeTerminalTitle collapsing our own extension's output to a hardcoded literal. The prior header credited the wrapped harness for frames it never sends, which is the same wrong narrative that produced eight fixes at eight layers. No behavior change. * fix(terminal): stop Orca mangling the OMP/Pi title it writes itself Verified against the OMP source: it emits only π-branded frames (`DEFAULT_TERMINAL_TITLE = "π"`, title-generator.ts:25), and on an Orca-hosted pane its native titler cedes to Orca's OWN injected extension, which writes `π - <session> - <cwd>` / `⠋ π - <session> - <cwd>` at 80ms. So neither flapping string came from OMP. Orca made both: "Pi" — normalizeTerminalTitle collapsing our extension's output to a hardcoded literal, discarding the session name and cwd (#16093) "OMP" — driveSyntheticTitleFromHook injecting over it every 80ms Fixed at the source: - normalizeTerminalTitle canonicalizes only the rotating braille frame and keeps the rest, in both spinner positions and through a multiplexer prefix (#8032). Status still round-trips through normalization. - detectAgentStatusFromTitle reads the π state separator, so `π ! <label>` is permission instead of the blanket idle that hid a blocked agent. - normalizeCompatibleAgentTitleForOwner swaps only the brand for the owner's label, so a pane still reads as its launch owner (#6689, #7633, #9077) without losing the session text. - pi/omp set synthesizeWorkingTitle: false — the agent animates its own working title. Terminal states still synthesize; they carry the pane's agent identity downstream. Reverts the ingest-time title rewrite from #16373, whose normalization of runtimePaneTitlesByTabId also changed Windows Shift+Enter bytes (#16376). Co-authored-by: Seongho.Bak <49228032+psh4607@users.noreply.github.com> * fix(terminal): match the state separator only in exact profile casing The separator check runs on every title, so `omp - deploy notes` and `pi - refactor the parser` read as an idle agent. The owner rewrite only ever emits the exact profile labels, so dropping case-insensitivity keeps `OMP - tmp` classifying while ordinary prose stops matching. Co-authored-by: Seongho.Bak <49228032+psh4607@users.noreply.github.com> * test(terminal): pin one real OMP turn to two committed patches Drives 30 working frames as Orca's injected extension emits them plus the idle transition, and asserts what survives the churn gate. Before the fix every frame alternated "⠋ Pi"/"⠋ OMP" and each one committed — ~12 store patches per second on a working tab. Co-authored-by: Seongho.Bak <49228032+psh4607@users.noreply.github.com> * fix(terminal): carry the permission guard inside the separator reader `-` is both a π state separator and the delimiter in the synthetic permission label, so `OMP - action required` read as idle. It resolved correctly only because detectAgentStatusFromTitle happens to check the synthetic label first — and the separator fn is exported, so a direct caller inherited the bug. Also pins the owner rewrite's fixed-point property, which holds only because getAgentLabel does not tokenize omp/pi, and corrects a comment that overstated how tightly the brand swap is scoped. Co-authored-by: Seongho.Bak <49228032+psh4607@users.noreply.github.com> * docs(terminal): name the flag the code actually sets The suite header cited `synthesizeTerminalTitle: false`; the profiles set `synthesizeWorkingTitle: false`. The distinction is the whole reason the narrower flag was chosen — terminal-state frames still carry the pane's agent identity downstream — so the wrong name buried the rationale. Co-authored-by: Seongho.Bak <49228032+psh4607@users.noreply.github.com> --------- Co-authored-by: Seongho.Bak <49228032+psh4607@users.noreply.github.com> |
||
|
Neil
|
33587fb77d | refactor(git): split status.ts into source-control modules (#16393) | ||
|
Neil
|
127fa7fae0 |
refactor(ipc): split repos.ts into focused modules (#16392)
* refactor(ipc): split repos.ts into focused modules
* test: point repo notification mocks at the extracted module
* fix(ipc): repoint the child-process allowlists after the repos split
The type-only `import type { ChildProcess }` moved from repos.ts to
repos/repo-clone-lifecycle.ts, so the import-boundary entry follows it and the
windows-console entry (now stale, and that list only shrinks) is dropped.
Fixture-only; the base file had no runtime child_process use at all.
|
||
|
Neil
|
e7c5263a93 |
perf(git): make local Git metadata observation event-driven (no scheduler) (#16404)
* perf(git): make local Git metadata observation event-driven Replaces the recurring per-repo metadata scan with native filesystem events on macOS, Linux, and Windows. Polling is retained purely as a fallback. - Narrow @parcel/watcher stream over <common>/worktrees, extended from macOS to Linux and Windows, with the Windows backend pinned explicitly. - New shallow watcher mode over the allowlisted primary metadata leaves. It watches the containing directory rather than each file, so Git's atomic write-and-rename does not orphan the binding. - Selected upstream refs stay on the existing bounded stat poll. Verified on real hosts rather than in principle: - Windows: `git worktree remove` and `git worktree prune` both succeed while the narrow stream holds the directory. The historical concern that an open handle would block prune does not reproduce. - Linux: inotify costs one instance per event loop, not one per watch, so the watch budget is not a constraint. - macOS/Linux/Windows: shallow events survive repeated commit, checkout, config, and pack-refs cycles. Failure handling, each reproduced before being fixed: - fs.watch binds an inode and reports nothing once that inode is replaced, with no error. Directory bindings are re-checked on a bounded cadence and rebound. - A host whose notification path is dead accepts registrations and stays mute forever. Observed on a macOS machine whose fseventsd had grown to ~15GB and saturated a core. A one-shot delivery probe now fails the shallow subscribe on such a host so it falls back to polling instead of showing stale metadata. This change stands alone on main and does not depend on the metadata poll scheduler. * test(git-watch): hold reserved inodes across root replacements Linux returns a released reservation to the free list, so the second replacement could land back on the first replacement's inode and look unchanged to reconciliation. Verified on ext4: releasing yields inodes [N, N+43, N+43] while holding yields [N, N+43, N+44]. macOS never recycles, which is why this only failed on CI. * refactor(git-watch): share one single-flight helper between watcher fallbacks Both fallbacks tracked their in-flight promise with the same self-comparison on settle, duplicated verbatim. Hoisting it removes a subtle invariant that was being hand-maintained in two places. * fix(git-watch): close the silent-staleness paths in primary metadata Two independent reviews converged on the same root cause: nothing bounded how long primary metadata could stay wrong once the shallow watcher stopped reporting. Four distinct paths led there. - A terminal watch error arriving while the status-ref poll was still starting left the repo with status-ref coverage only. handleWatcherError ran its teardown against nulls, then the in-flight poll installed itself, and the fallback guard mistook it for coverage and discarded the fallback. Primary metadata was then never observed again. The guard no longer treats status-ref polling as primary coverage, and startup re-checks watcher liveness after its awaits. - Nothing re-read the six primary files while the watcher was nominally live. A lossy notification path, a dropped batch, or inotify queue overflow raises no error, so the error-driven fallback never fired and the inode rebind sweep does not detect loss. A 15-tick backstop re-stats them, turning permanent staleness into one tick. Measured cost is ~0.2 stats/s/repo against the 3/s the old poll cost. - Reconciliation treated any late-observed entry create as a root replacement, so an ordinary tore down a healthy stream ~30s later and opened a deaf window. It now also requires the root itself to be recreated. - The shallow watcher recorded directory identity from a stat issued after binding, so a replacement in that gap pinned the dead inode's watcher to the new identity and the sweep would never rebind. Identity is now read first, which errs toward a harmless extra rebind. Test helper: replacing the worktrees root frees several inodes at once, so reserving one still let the recreated root reuse its own. It now verifies the inode actually changed. Confirmed on ext4, where three holds were needed. |
||
|
Neil
|
d456549c22 |
refactor runtime Git bridges (#16189)
* refactor runtime git bridges * fix(test): avoid computed namespace import access in runtime Git client contract test |
||
|
Neil
|
fc0d947983 |
refactor(rate-limits): split Codex and Claude fetchers (#16140)
* refactor(rate-limits): split Codex and Claude fetchers * refactor(rate-limits): restore base error-message defaulting The split moved the 'Unknown error' fallback from inside String() to the call site, which changed behavior for an Error with an empty .message: base surfaced '', head surfaced 'Unknown error'. Restore the base form. |
||
|
Neil
|
83ffc0df24 |
refactor Electron facilities modules (#16333)
* refactor oversized Electron facilities * fix interactive process timeout and shortcut repeat guard * chore(child-process): drop stale cli-installer allowlist entry cli-installer.ts now routes privileged spawns through runProcess via cli-privileged-processes.ts, so the shrink-only ratchet flags it as stale. * refactor(child-process): extract the bounded output sink runProcess's timeoutMs opt-out (required to preserve the unbounded osascript admin prompt) pushed run-process.ts past the 300-line cap. Move createOutputSink to its own module rather than add a max-lines bypass, which AGENTS.md forbids. Moved verbatim; no behavior change. |
||
|
Neil
|
1cf562deea | refactor: split source control AI modules (#16179) | ||
|
Neil
|
6103b78e3c |
fix(crash-reporting): correlate concurrent process deaths instead of blaming the renderer (#15251)
* fix(crash-reporting): correlate concurrent process deaths on a renderer report
Two 1.4.184 reports (a326935a, 1862f316) are renderer "crashed"/-1 crash reports
whose renderer only died alongside a sibling Chromium child that died at the same
instant:
F0BQMB30GJX network.mojom.NetworkService crashed/-1 -21ms -> renderer crashed/-1
F0BRPP8TC0Y audio.mojom.AudioService crashed/-1 -2ms -> renderer crashed/-1
GPU crashed/-1 +180ms
process-gone-classification.ts classifies each event in isolation:
isRecoverableChromiumChildProcess discards the utility/GPU halves as recoverable
churn, and `if (reason !== 'killed') return true` then reports the renderer half as
a genuine renderer crash before any cross-source signal exists. Triage reads
"renderer crashed" for what died with three other processes.
process-gone-sibling-correlation keeps a bounded ring of child deaths, populated
before the suppression early-return so churn-suppressed siblings stay visible, and
matches a renderer death against child deaths sharing its failure signature.
What the timing can and cannot support:
- The window is asymmetric. 1s of lookback (a child that died first can plausibly
have taken the renderer with it), but only 250ms of lookahead: a child dying well
after the renderer is at least as likely to be an effect of it — Chromium tearing
down the dead renderer's channels, or renderer_recovery_reload at +264ms — and a
symmetric window would retro-label a genuine lone crash as collateral.
- crashAttribution is 'concurrent-process-deaths', not a causal claim. The largest
1.4.184 cluster is an external taskkill /T where renderer and children are
co-victims; no sibling caused anything there.
- The verdict is not derived from timing alone. A host with a child looping at the
observed 1459/min drops a death into every window, so crashAttribution is set only
when the nearest sibling is within 250ms and no identity repeats. Looser or
repeating deaths still ship as evidence (siblingProcessDeathCount, signed offsets,
siblingProcessDeathRepeats) with no attribution.
- The signature match buckets `crashed` with `abnormal-exit` and gates on the exit
code only on win32. Both fixtures are win32, where every process in a collateral
pair reports crashed/-1; POSIX surfaces a per-process wait status, so an equality
gate would mean this never fires on macOS or Linux.
The report stays reportable and gains evidence rather than being suppressed
(#14667). Both arrival orders are covered without delaying persistence: a sibling
that dies first is folded into the initial record, a sibling that dies after amends
the record already on disk through attachDetails, the same way the minidump
signature does. Late amends are capped at two per report and skipped when the
rendered evidence is unchanged, so a crash-looping child cannot rewrite the store
during renderer recovery, and a failed amend now leaves a
sibling_attribution_attach_failed breadcrumb instead of vanishing.
Relationship to #12484: it is still OPEN and adds process-tree-kill-window.ts, the
same ring/lookback bookkeeping with a 250ms settle, patching the same recorder
hunks with the opposite policy (suppress the killed/1 renderer report instead of
keeping it). #14667 is test-only — it pinned the keep-the-report policy in tests, it
did not remove a shipped implementation. #12484 has to be closed or rebased out
before this lands.
* chore: remove merge hook formatting drift
|
||
 Neilandinnocarpe
|
49acb93e2e |
fix(daemon): let PTY inventory recover from a dead terminal host (#16363)
Worktree removal inventories PTYs through DaemonPtyAdapter.listProcesses. That called ensureConnected bare, so once the terminal-host pipe was dead the removal failed with `connect ENOENT \\?\pipe\orca-terminal-host-...` and stayed broken until the whole app was restarted. spawn already wrapped its work in withDaemonRetry and recovered from exactly this. Inventory did not — so the one path that must not get stuck was the only one that could not heal itself. Both the connect and the listSessions request go inside the retry: a host that dies between them throws the same daemon-gone error, so retrying only the connect would still fail. The reconciliation after the request is deliberately outside it; retrying that would be wrong. Reproduced first, with a real daemon killed mid-test: listProcesses threw DaemonConnectionLostError while a control asserting spawn recovery from the identical kill passed. Both are now regression tests, so the asymmetry cannot come back silently. Co-authored-by: innocarpe <innocarpe@users.noreply.github.com> |
||
 
|
deae7212d9 |
fix(orchestration): derive inject's agent guidance from the recognized-agent roster (#15874)
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: vam <a@a.com> |
||
|
Neil
|
48e63c015f |
refactor agent config and auth services (#16195)
* refactor: split agent config and auth services * chore: repoint wsl and global-fetch guards at split module paths * fix: restore merge-base Claude CLI error propagation Drop the secret-redaction rewriting added to Claude CLI error paths in the refactor: spawn errors again reject with the original Error (preserving .code/.errno/.syscall/.stack) and command output/auth-status logs are no longer rewritten. |
||
|
Neil
|
2b1b094aa8 |
fix(cli): pair every resolved CLI with its runtime, and ratchet it (#16383)
Follow-up to #16365, which paired 8 spawn sites by hand. Hand-pairing is how the class got introduced, so close it structurally instead. cliPath is now required on CodexAppServerInvocation, `null` only for the guest-side wsl.exe launcher where a host path pairs nothing. Optional let a native builder omit it and silently fall back to pairing against a cmd.exe wrapper with no type error. Every production site already passed it; only test fixtures needed updating, which is the type doing its job. Four more sites now pair. codex-state-db-backfill-recovery spawns the same `codex app-server` subcommand #16365 fixed elsewhere. cli/handlers/account was the worst case: addAgentNodePaths prepends the *newest* version-manager bin, which is not necessarily where the CLI being launched lives, so it actively created the mismatch — pairing now runs last so the CLI's own node wins. commit-message-text-generation and skills/skill-update-run spawn resolved binaries with inherited env. cli/handlers/skills had grown its own buildNpxPath: a weaker local copy that prepended unconditionally, ignored the Windows `Path` key, and special-cased a '.' dirname. Deleted in favor of the shared helper, which checks the sibling node actually exists — the behavior change one test had pinned. The ratchet is the point: any file that resolves a CLI and spawns must reference withCliRuntimeOnPath, with a shrink-only allowlist. It caught skill-update-run, which I had missed. Its first draft required a call paren and so let dependency-injected resolvers (`resolveCommand: resolveCodexCommand`) through — verified by removing a pairing and watching it stay green, then widened until it failed. A second assertion fails on a stale allowlist entry so an exemption cannot outlive its reason. external-editor-launch stays allowlisted: it launches a GUI editor, not a Node CLI whose ABI matters. |
||
|
Neil
|
20c1a61401 |
fix(orcad): answer host paths honestly and ship the watcher child (#16369)
orcad's AppEnvironment implemented three of seven AppPathNames and returned the userData directory for the rest — including 'exe', where a data directory is not an executable. Every name now has a Node answer: 'appData' is the platform's per-user application-data root, 'logs' lives inside the data root so a headless deployment stays one removable directory, 'downloads' honours XDG_DOWNLOAD_DIR, and 'exe' is the Node binary. getAppPath() is the directory orcad was launched from rather than cwd, so children resolve against the bundle instead of wherever the supervisor happened to be. The watcher child was the load-bearing consequence: resolveWatcherProcessEntryPath probed for the adjacent entry only when !isPackaged, so orcad resolved a desktop out/main path that no deployment has — and build-orcad never emitted the child anyway. isPackaged stays true (consumers read it as "production, not a dev checkout" and it gates HTTPS-only skill downloads); the resolver now asks whether the app root is an asar archive, which is the question it actually meant. The child ships beside orcad.js, and the build forks it to prove it runs. |
||
|
Neil
|
c4988fa81c |
refactor workspace lifecycle modules (#16334)
* refactor(workspaces): split lifecycle modules
* preserve workspace cleanup consent contract
* restore workspace delete shortcut hint in context menu view
* restore host-qualified visit recency and viewed-candidate predicate
* test(cleanup): pin the viewed-mark upgrade path and host-qualified visit reads
Two invariants a refactor broke in this PR, both silent:
- viewed marks are persisted, so gating `shouldPreserveCleanupInspection` on any
newer field voids the grace period for every entry written by an older build
- visits are stamped under `${hostId}|${worktreeId}` whenever the host is known
(the normal case, including 'local'), so a bare map[worktreeId] read misses
every modern entry and yields 0, disabling the recent-visible-context blocker
Verified discriminating: reintroducing each bug fails exactly its own test.
|
||
|
Neil
|
a7505fd911 |
fix(cli): spawn a version-manager CLI with its own node runtime (#16365)
* fix(cli): spawn a version-manager CLI with its own node runtime resolveCliCommand falls back to scanning every version-manager install when PATH misses, so it can hand back ~/.nvm/versions/node/v20.x/bin/codex while PATH still leads with v22. Nothing paired the binary with the runtime it was installed against, so its `#!/usr/bin/env node` shebang loaded a v20-built native module under a v22 ABI and the agent died on first require (#10932). Reproduced with a real addon rather than asserted: a CLI requiring a cpu-features build for NODE_MODULE_VERSION 115, spawned with v24 leading PATH, fails with ERR_DLOPEN_FAILED and exit 1. With the CLI's own bin directory prepended it runs clean. withCliRuntimeOnPath prepends the resolved command's directory when that directory ships a sibling node, and is a no-op otherwise — so a Homebrew or /usr/local CLI is untouched, and the WSL paths pass a bare `codex`/`claude` that is not absolute and so never matches. Host CLI resolution in the Claude login path is now lazy, keeping the WSL branch from resolving a host binary it never spawns. * fix(cli): split PATH on the delimiter we join with, pair app-server too Readiness review findings, all four addressed. withCliRuntimeOnPath chose its join delimiter from the platform option but split with the host's. Passing platform:'win32' from a posix host turned `C:\Windows;C:\Windows\System32` into `C;\Windows;C;\Windows\System32` — every drive letter torn off at its colon. Latent, since no shipped caller passes platform, but the sole win32 test was written against the corrupted value and asserted one split segment, so it green-lit the shredding. That test's other assertion was vacuous: it seeded only `Path`, so the `PATH` key it asserted absent could never exist. Deleting the whole case-dedupe block left the suite green. It now seeds both keys and asserts the full joined string; removing the block fails it. Nothing covered the wiring, and the argument choice is the easy thing to get silently wrong. Note it only diverges on win32 — on posix getSpawnArgsForWindows returns the CLI itself, so pairing the spawn command is indistinguishable there. The new test drives the win32 branch with a .cmd fixture; pairing spawnCmd or dropping the wrapper both fail it now. codex-trust-grant-host and codex-session-index-heal spawn the same `codex app-server` subcommand through runCodexAppServerSession and were left unpaired. Pair centrally there via a new optional cliPath, since invocation.command may be a cmd.exe wrapper. Pairing tests live in their own file: adding them inline pushed codex-fetcher.test.ts past the 800-line ratchet. * fix(cli): read the Windows path key the child will actually use Round-2 review finding. The read was narrower than the delete: the key was picked from exactly two spellings (`Path`, else `PATH`), while the twin dedupe removed every key whose lowercase form is `path`. A block spelling it `path` or `pATh` therefore had its value deleted without ever being read, handing the child a PATH containing only the CLI's own directory — a strictly worse outcome than not pairing at all. Win32 resolves env names case-insensitively and object order preserves block order, so the entry the child reads is the first case-insensitive match. The repo already encodes that rule in resolvePathEnvKey (src/main/pty/windows-path-segment-merge.ts); src/shared cannot import from src/main, so mirror it locally. Verified by execution across six env shapes: lowercase, mixed-case, Path-only, PATH-only, both twins, and a PATHEXT control that must not be touched. All preserve the original PATH; before the fix the first two lost it entirely. Reverting the selector fails the new test and nothing else. |
||
|
Neil
|
e217fdd10f |
build(orcad): gate orcad's own graph, and prove it loads under plain Node (#16368)
* fix(orcad): close the browser-provider gaps The providers landed without enforced coverage, so a regression in either path would have landed silently. - CI: the external-Chromium integration test was gated on ORCA_BROWSER_EXECUTABLE and nothing ever set it, so it skipped forever. It now runs in its own job against the runner's Chrome and FAILS when Chrome is absent rather than skipping, because an unset variable is exactly how it went uncovered. Timeout raised to 120s: a warm run is ~7s but the first launch against an unseeded profile took 30s and hit Vitest's default, and CI is always that cold case. - Electron provider had no test at all. It is the path anyone with the desktop app hits. - Browser unavailability reported one message for four causes, including telling an operator to set a variable they had already set. Fixes a live defect found while covering it: the runtime advertises browser.tabCreate.known-id.v1 unconditionally, so a web client sends a provisional page id for a page that does not exist yet — and the sidecar's generic requestedPageId branch ran require() on it first and threw. Every known-id create against the Electron provider failed. The adoption logic was already there; only the ordering was wrong. Also updates the workflow-parallelism guard, which correctly caught the new job missing from verify's required-check list, and asserts verify actually reads it. * build(orcad): gate orcad's own graph, and prove it loads under plain Node Two gaps the artifact's own comment asked for. The ratchet measured only orca-runtime + runtime-rpc, but orcad imports ipc/pty directly to install the PTY controller, so its graph is strictly larger. The gate could read zero while the shipped artifact regressed. orcad's entry is now a ratchet entry point, and the baseline stays empty with it included. orcad cannot join plain-node-entry-guard — that is a rollup plugin keyed on electron-vite input names, and orcad is an esbuild artifact. But the half that matters here is the guard's smoke-load: scanning the metafile proves no module NAMES electron, not that the graph resolves under plain Node. A dynamic require, a missing native or a top-level throw all pass the scan and fail at runtime. build-orcad now runs the bundle with a bogus flag and requires the argv rejection that only a fully loaded graph can produce. Verified: a bundle that builds but throws on load fails the gate. |
||
|
Neil
|
8043b5f705 |
Refactor worktrees IPC into cohesive modules (#16190)
* refactor worktrees IPC into cohesive modules * test update PTY waiver invariant paths * revert unrelated oxfmt reflow from merge commit |
||
|
Neil
|
d8807801ff |
Refactor GitHub Project surfaces and mutations (#16166)
* refactor github project modules * restore dropped issue #4756 guard comments |
||
|
Neil
|
087b895524 |
refactor(daemon): split oversized PTY services (#16160)
* refactor(daemon): split oversized PTY services * revert(daemon): restore merge-base session listing and canceled-spawn behavior Two behavior changes rode along with the file-splitting refactor: - listLiveTerminalHostSessions dropped sessions with isTerminating, not just dead ones, hiding sessions the merge base still advertised. - spawnAndPublishSession called session.beginTermination() before publishing a canceled spawn into the host map. Both hunks are reverted to the merge base; the refactor is untouched. |
||
|
Neil
|
09048c63d4 |
feat(orcad): add headless browser providers (#16193)
* feat(orcad): add headless browser providers * fix(orcad): merge the duplicate runtime-browser type import |
||
|
Neil
|
788575e300 | fix(crash-reporting): stop destroying user crash notes (#15252) | ||
|
Neil
|
b6a24ecac1 | refactor: split GitHub backend modules (#16194) | ||
|
Neil
|
9bcaf09869 | refactor loading store into cohesive domains (#16192) | ||
|
Neil
|
b516300b8c | refactor agent hook listener modules (#16187) | ||
|
Neil
|
07b13c8468 | refactor git repository host boundaries (#16177) | ||
|
Neil
|
1c436a8084 | refactor(rpc): split terminal methods into cohesive modules (#16188) | ||
|
Neil
|
4371aaf722 | refactor provider clients into domain modules (#16168) | ||
|
Neil
|
5c116b6ec2 |
fix(startup): stop the PATH seed pinning nvm to its newest install (#16314)
* fix(startup): stop the PATH seed pinning nvm to its newest install patchPackagedProcessPath prepends the newest nvm version dir to process.env.PATH, then hydrateShellPath probes the login shell with that same env. nvm's startup `use` honors whatever node is already on PATH instead of the user's `default` alias, so the probe returns a PATH pinned to the newest install and every terminal pane inherits it. A user whose newest nvm node is a bare install then loses every global CLI (codex, claude, gemini, vercel...) inside Orca while they still resolve in Ghostty/Terminal, which start from the bare GUI PATH and fall through to `default`. Probe with the PATH the process launched with. Windows already gets this through WindowsShellPathOwnership. * test(startup): pin the platform in the probe-env test shellProbeEnv short-circuits on win32, so the POSIX-only assertion failed for anyone running the suite on Windows. Matches the convention in hydrate-shell-path.windows.test.ts. Also narrows the win32 exemption comment: WindowsShellPathOwnership snapshots its baseline after the seeds land, so it does not unwind them. The exemption holds because Windows keys PATH as `Path` and no Windows seed pins a node version. * fix(startup): give win32 the same probe insulation, keyed by Path The previous commit exempted win32 on the stated grounds that no Windows seed pins a node version. That is wrong: getVersionManagerDirectories calls getNvmVersionDirectories on every platform, so a Git Bash user whose nvm uses the POSIX ~/.nvm/versions/node layout gets the newest version dir seeded on Windows too, and the -ilc Git Bash probe inherits it. Record the PATH key alongside the value and overwrite that entry in place, so Windows never carries both `Path` and `PATH` — which was the only real reason to skip win32. * fix(startup): snapshot the launch PATH at module load, log probe failures Three loose ends from the review, folded in rather than deferred. The probe's clean PATH was handed over by an explicit recordLaunchPath call from the seeding site, so the invariant lived across three files and a refactor that moved the seed call would silently re-pin nvm. Snapshot PATH during module init instead: that runs while the import graph is evaluated, strictly before any statement in main's body, so it cannot observe the seeds and there is no call ordering left to break. All seven importers are static, so no lazy import can defeat it. A test asserts the probe ignores later process.env mutation, and fails if the live read is reintroduced. A failed startup probe leaves the seeded newest-nvm dir in front and said nothing, so the population whose rc files blow the 5s budget hit the original symptom with no diagnosable trace. Log the failureReason. The probe is an interactive login shell, so rc files that exec into a multiplexer or start a heavy prompt can outrun that budget with no way to opt out. Set ORCA_SHELL_PATH_PROBE=1 so they can take a fast path. * fix(startup): drop the other-cased PATH key from the Windows probe env Caught by running the suite on a real Windows machine, not a mocked platform. The spread of process.env is a plain, case-sensitive object, while Windows resolves env names case-insensitively. Writing the captured `Path` back onto it left the seeded value still live under `PATH`, so the probe shell could read either one — the exact duplicate-key hazard the win32 branch was supposed to prevent. Drop any other-cased variant of the key before writing. * fix(startup): preserve launch PATH across app restarts |
||
|
Neil
|
0bfd3808f6 |
test(pty): pin the renderer-liveness guard STA-2373 relies on (STA-5373) (#16345)
* test(pty): pin the renderer-liveness guard STA-2373 relies on (STA-5373) STA-5373 reported that #15927 dropped the `webContents.isDestroyed()` half of the guard #10065 added for STA-2373, and that the app therefore dies when a daemon death fans out to every pane. The guard is present on main: #15172 restored it at both senders when it split the PTY monolith, and a per-file count across v1.4.188..HEAD shows only relocation (pty.ts:4 -> write-input.ts:2 + bind-listeners.ts:2). The reported 4 -> 0 was scoped to `src/main/ipc/pty.ts`, now a 39-line barrel. No production fix is needed. What was real is that nothing pinned the guard — it survived #15927 only because #15172 happened to re-expand it — and the shared PTY test fake made that invisible: its `webContents` had no `isDestroyed` at all, so both guards passed vacuously in every suite. That is also why they are written defensively as `typeof ... === 'function' && ...`. - Give the fake an `isDestroyed` mock, re-stubbed to `false` each beforeEach rather than left `undefined`, so "alive" is stated rather than accidental. - Cover both senders red/green: the daemon-death fan-out (bind-listeners.ts:37) and the per-write reporter (write-input.ts:57). Verified red against a window-only guard and green with the real one. The per-write case needs a chunked write. A single-chunk write is already fenced by `isPtyWriteEventFromMainWindow`, which rejects the event once the WebContents is gone; only the multi-chunk path yields a macrotask mid-write, letting the renderer die after the sender check passes. That is the sole route to this sender with a dead WebContents, and what makes its own guard load-bearing. Also deletes `src/main/ipc/pty-renderer-surface.ts`: zero importers repo-wide, and its `isRendererGone` is exactly the weakened predicate. Its comment claims the headless path "now passes null", but register-headless-runtime.ts:26 still fakes `{ isDestroyed: () => true }` — #15172 rolled that back too. Adopting it would reintroduce STA-5373 for real. Verified: 17 tests across the two pty write suites; full src/main/ipc run 3156 pass (4 pre-existing @parcel/watcher failures in filesystem-watcher-real and worktree-base-directory-poller, unrelated and failing identically on a pristine tree); pnpm typecheck clean; oxlint clean. * docs(pty): correct headless-path comments the #15172 rebase left stale #15927 made `registerPtyHandlers` accept `BrowserWindow | null` so the headless path could pass null instead of faking a window. #15172 reverted that signature while splitting the PTY monolith, but the comments describing it survived, so three of them now document an API that does not exist. - orcad-entry.ts: the module docstring claimed orcad installs its controller via `registerPtyHandlers(null, …)`. It uses `registerHeadlessPtyRuntime`, and null is not accepted. It also claimed desktop surfaces are "declared rather than faked" — true except for the renderer window, which is still faked. - register-headless-runtime.ts: record why the fake is safe rather than leaving `isDestroyed: () => true` looking arbitrary. It is load-bearing: every renderer-liveness guard reads it and skips, so no send is attempted. Also gives the fake a `webContents.isDestroyed`. The real guards check both, and a missing method reads as "alive" — the same gap this PR fixes in the test fake. No behavior change: the window-level check already short-circuits. Verified: 45 tests across the liveness-guard, startup-barrier, management and kill/exit suites; pnpm typecheck clean; oxlint clean. |
||
  OrcaWinandm4air
|
1a7934c54f |
Fix deleted remote worktree reappearing due to host ID mismatch (#16039)
* Fix deleted remote worktree reappearing due to host ID mismatch Paired clients and servers may use different spellings for execution hosts (e.g., client 'runtime:env-1' vs server 'local'). Resolve these spellings before worktree.rm and worktree.forceDeleteBranch to prevent failures and orphaned worktrees. * Validate runtime kind before comparing environment IDs Ensure parsed host IDs are actually runtime environments before accessing their environmentId property. Fixes incorrect host ID matching during worktree cleanup that caused deleted remote worktrees to reappear. * Use hostId directly when same-ID surviving host exists When a surviving host has the same ID as the deleted worktree's original host, use the hostId directly instead of qualifying it through the runtime call host. This prevents worktrees from reappearing due to host ID mismatch. --------- Co-authored-by: m4air <m4air@m4airs-Air.localdomain> |
||
|
Neil
|
50438041a8 | fix(rate-limits): safely surface Codex RPC exit reasons (#16023) | ||
|
Brennan Benson
|
31562c5b27 |
fix(windows): attach interactive login children to console input
Verified on native Windows awin at the exact PR head with Electron CDP/Playwright: the Claude sign-in console is visible, cancellation after console launch restores Add Account state, and the login process/PID/temp cleanup completes. |
||
|
Neil
|
7e7947665c | fix(relay): gate relay control work on proven broker liveness (#16021) | ||
 Jinwoo Hong
|
fba910f2ea | fix(crash-reporting): scope renderer crash evidence (#16313) | ||
|
Brennan Benson
|
723158a519 |
feat(workspace-cleanup): blockers become labels, not refusals (#16282)
* wip(workspace-cleanup): PR 3 blockers-become-labels, recovered from a dead worker Uncommitted work recovered from a worker that died with 'Agent process stop was requested but never confirmed'. Committed as-is to preserve it; NOT verified yet. * Fix workspace cleanup review regressions * fix(workspace-cleanup): drop filter chips for the safety fields this PR removes The cleanup dialog crashed on every open. My merge of #15300 brought in the applied-filter chips, which read `safety.tiers` and `safety.selectableOnly` -- the exact fields this PR deletes. Electron QA caught it; the chip derivation runs on open, so it threw before anything rendered. Removed the two chip branches, their formatter entries, and the now-dead 'tier' chip-kind label. The test that swept every chip keeps its breadth by using `safety.dismissed`, which survives. Worth noting what did not catch this: typecheck flagged only the test file, not the source, because the QA agent had already patched the source locally without committing. A merge that compiles can still remove a field a caller reads at runtime, and only opening the dialog proved it. |
||
|
Neil
|
beb1d45198 |
test(windows): run PTY IPC suites in Windows lane
Merged after clean CI, Windows PTY IPC validation, and readiness review. |
||
|
Jinjing
|
3bc13f7b8c |
Split monolithic PTY IPC module into organized submodules (#15172)
* rm unused files * remove unused files * Refactor PTY IPC and add host environment paths - Split PTY handlers out of inline baseline checks - Rename local PTY shell provider for clarity - Pass userDataPath and resourcesPath to host environment * Establish PTY daemon identity before first await in spawn flow Move identity setup, session ID minting, and hidden delivery state to the beginning of preflight, ensuring these complete synchronously before any awaited operations. Defer async operations like folder workspace validation; add liveness tracking for SSH provider failures. Refactor pane spawn reservation to prevent concurrent spawns from creating duplicate providers. * Add incarnationId tracking throughout PTY exit lifecycle Track PTY incarnation IDs in exit messages sent to renderer, and add cause tracking for exit events. This enables proper lifecycle state management when PTYs can be respawned or have multiple concurrent instances. Also adds deadline support to process listing operations and stop-request tracking for better shutdown observability. * Use fake timers in SFTP namespace tests for deterministic abort handling Tests now use `vi.useFakeTimers()` to control time during abort scenarios, advancing timers explicitly instead of waiting on real async delays. Ensures more reliable test execution without flakiness from timing-dependent behavior. * Fix PTY spawn lifecycle: handle concurrent races and cleanup abandoned a Properly release Agent Teams leader handles when spawns are abandoned or fail, restore provisional PTY sizes on reattachment, and settle concurrent spawn races for the same pane. Add validation guards for destroyed renderers and improve handler re-registration to reset delivery state before bridging a new window. * Move PTY cleanup to localized error boundaries Restore provisional PTY size when build-options fails and guard pre-allocated handle registration. This ensures cleanup happens at the point of error, not deferred to the general catch block. * Replace Promise.resolve() with vi.waitFor in PTY claim test Wait explicitly for the providerSpawn call to be made using vi.waitFor() instead of relying on event-loop yielding. This makes the test more deterministic and reduces flakiness from timing assumptions. * Redact PTY IDs in pending data drop diagnostics Prevent workspace paths embedded in session IDs from leaking through diagnostic logs by using redactPtyIdForDiagnostics. * Mark PTY exit events as observed by provider Exit handlers now receive `providerExitObserved: true` to distinguish definitive provider-witnessed exits from inferred state changes. Preserves optional exit cause when present. * Add defensive input validation to PTY IPC handlers Validate that IPC arguments are present and the correct type before passing them to handler logic. Uses optional chaining and type checks to safely handle malformed requests from the renderer process. * Replace direct Electron imports with PTY host bindings Abstract app, ipcMain, and powerMonitor access through getter functions to support multiple host environments and improve testability. * Defend against transient PTY setup failures with state cleanup Host-env setup failures now trigger cleanup of runtime-allocated PTY state. Cached PTY geometry is preserved after transient reattach failures but cleared when the provider reports the PTY exited before the spawn reply—preventing stale geometry from corrupting future operations. Error handling now distinguishes expired SSH sessions and early-exit conditions to preserve geometry appropriately. |
||
|
Brennan BensonandNeil
|
ec4687c434 |
feat(agents): distinguish Claude background monitoring (takes over #14205) (#16201)
* feat(agents): distinguish Claude background monitoring Adds an optional `workingMode: 'monitoring'` discriminator for a Claude session whose lead turn finished but which still has background shell tasks or session crons registered. The wire state stays `working`, so older peers that never read the field keep rendering Working. (cherry picked from commit |
||
|
Brennan Benson
|
1921ba2250 |
fix(agent-status): clear the pane when a Claude compact finishes (STA-2915, STA-4613) (#15202)
* fix(agent-status): clear the pane when a Claude compact finishes (STA-2915, STA-4613) A manual /compact ends at an idle prompt without emitting Stop, so nothing in the compact window could ever clear the pane. A worktree that entered the compact `working` stayed `working` until the 30-minute stale sweep -- and the summarizer's start-less SubagentStop kept republishing the row, resetting that clock each time. The correlation added by #12332 was supposed to own this, but it could never run: PreCompact and PostCompact were never added to CLAUDE_EVENTS, so they were never registered with Claude. compactTrigger was always undefined, and the transition guard, the ownership cache, the relay wire field and the ingest branch were all unreachable. Five test files exercised the logic by injecting events past the registration boundary, so the suite stayed green over code that could not execute. Register PostCompact -- and deliberately NOT PreCompact. Measured on Claude Code 2.1.227, a successful manual compact emits PreCompact, a start-less SubagentStop, SessionStart(source=compact), then PostCompact; an ABORTED compact ("Not enough messages to compact") emits PreCompact ALONE. Mapping PreCompact to `working` would strand the pane on every aborted compact, which is the bug being fixed, so the abort guard is structural: Orca never subscribes to the pre-validation event. PostCompact carries its own trigger, so no anchor is needed to tell manual from auto and the correlation machinery is deleted rather than repaired. Manual becomes a `done` with sessionBoundary set -- a finished compact is a session-shaped boundary, not a completed turn, so completion notifications, unread counts and automation-run evidence stay out of it. Auto claims nothing: it runs inside a turn that resumes and emits its own Stop. The source-blind early return that dropped compact events for EVERY provider before its normalizer ran is narrowed to Claude, so it keeps failing closed on a malformed payload without pre-empting other providers. Ownership is kept where the deleted guard had it: a valid provider prompt id is required, a completion clears a row but never creates one (a retired pane must not be resurrected), and a hydrated row is matched on provider session only -- it carries the previous session's connectionId, and older rows carry no session at all, so a strict check would reject the restart case this fixes. A consumed prompt id keeps relay duplicates from refreshing the row. Mixed versions: no new wire field and no new opcode. An older relay normalizes with its own shipped mapping and forwards the event, so ingest drops `auto` envelopes and stamps the boundary on `manual` ones; its replay strips the trigger entirely, so payload state stands in for it while ownership is still enforced. The relay now caches a completion with its compact identity removed, so a client that was offline during the compact still receives the clearing row on reconnect. Tests go red before this change and green after: 6 of 12 in the new registration-gated suite and 5 of 8 in the relay/ingest suite. The harness delivers only events present in CLAUDE_EVENTS, so a fix that is never registered cannot pass -- the failure mode that let the original correlation ship unreachable. * test(agent-status): restate the compact reliability gate around the new invariant The gate pinned a test file this change deletes, so the manifest check failed. Repointing the path alone would have left the gate describing an invariant that no longer exists: it required a manual PostCompact to match its exact PreCompact generation, and PreCompact is no longer consumed at all. Restate it. The invariant is now that PreCompact never moves a pane, that only a manual PostCompact marks done and does so as a session boundary, that a completion clears an existing row but never creates one, and that a relay predating the contract has its automatic envelopes dropped and its trigger- stripped replays classified by payload state under the same ownership checks. Evidence runs are the real ones: the 105-test suite from this branch, and the Claude Code 2.1.227 PTY capture that measured PreCompact arriving alone on an aborted compact. * fix(agent-status): clear the restart-stuck pane a compact was meant to clear Review found the completion did not clear the pane STA-2915 actually reports, and that republishing it was a strict regression. - A manual completion now retires a subagent that exists only as a disk snapshot: a /compact only completes at an idle prompt, so a restored child is proof of nothing. Live evidence -- a child observed in this runtime, an unclassifiable running background task, a registered session cron -- still holds the pane. - A completion that cannot clear now publishes nothing instead of restating the row, which was stripping restoredUnconfirmed off a hydrated row and restarting the staleness clock for work the compact never observed. - The relay defers compact ownership to the client that owns pane identity, so a cold relay cache can no longer swallow the one event that clears a remote pane. - claudeConsumedCompactPromptIdByPaneKey joins all three pane-scoped teardown routes, and an auto compact no longer spends the pane's consumed-compact slot. - The promptless completion keeps the summarized turn's label with or without a trigger on the envelope. Tests: the two restart cases now deliver the completion while the hydrated row is still cached, so they exercise the restored-row branch instead of passing through the strict one; the triggerless working replay is asserted from a FINISHED pane so it can fail. Reverting the four source files turns 12 of 21 registration-gated and 8 of 12 relay/ingest tests red, and 18 of 18 targeted mutations are caught. * fix(agent-hooks): preserve compact identity across relay replay * docs(reliability): describe compact replay ownership |
||
|
Jinjing
|
08f5570a43 |
Fix mail pointer repoint scheduler tests with pending count getter (#16292)
Add pendingCount getter to MailPointerRepointScheduler to expose the number of handles awaiting repoint. Replace flaky vi.getTimerCount() assertions with direct scheduler state checks. |
||
|
Brennan Benson
|
43461fca46 |
fix(ai-vault): replace scanner internals with actionable panel copy (#16229)
* fix(ai-vault): replace scanner internals with actionable panel copy Agent Session History painted the scanner's supervision errors verbatim: "AI Vault service restart circuit is open." and "AI Vault service timed out after 130000ms." Neither tells a user what happened or what to do. Add a shared mapper that rewrites the supervision family into copy tied to an action, passing anything unrecognized through so scanner-authored messages (host name, remote path, cap) keep their own wording. It also strips Electron's `Error invoking remote method` wrapper, which this path never handled. Applied at both surfaces the panel paints: the local leg's scan-issue row and the thrown-rejection banner. Humanizing in main covers remote clients too; the raw text moves to the main log. Also make "Refresh to try again" true. The relay path lets a forced refresh reopen the restart circuit, but on the local path `force` stopped at the cache layer and never reached the supervisor, so the refresh button was inert for the full 60s fault window. The client file sat at 299/300 lines, so extract the child-listener and init-frame wiring into `attachAiVaultServiceChild`, next to the ready-waiter and retirement helpers it belongs with, rather than bumping the max-lines ceiling. * fix(ai-vault): humanize runtime scanner errors * fix(ai-vault): preserve runtime error metadata * fix(ai-vault): normalize wrapped scan errors * fix(ai-vault): preserve non-scanner relay errors * fix(ai-vault): make forced retries cancel backoff |
||
|
Jinjing
|
7e76bb3aec |
Fix rebase race by fetching to private ref before rebasing (#15990)
* Fix rebase race by fetching to private ref before rebasing
`git pull --rebase` is vulnerable to concurrent fetches modifying remote-tracking refs during execution. Fetch to a temporary private ref (refs/orca/rebase/*) first, then rebase from that stable ref to avoid the race condition.
* Fix rebase race by fetching to private ref with timeout
Concurrent fetches can interfere with remote-tracking refs between
fetch and rebase. Use a unique private ref and 60-second timeout to
isolate each rebase operation and prevent hangs on stalled remotes.
Extract gitPullRebaseFromBase to a dedicated module.
* fix rebase race by fetching to private ref with timeouts
Concurrent fetches can replace FETCH_HEAD and remote-tracking refs between
fetch and rebase, causing the rebase to fail. Fetch to a temporary private
ref instead, use --no-write-fetch-head when available (Git 2.29+), and
serialize FETCH_HEAD access for older versions. Add process termination
barriers to ensure proper cleanup and extend timeouts for SSH operations.
* Fix rebase race by fetching to both private and tracking refs
Concurrent fetches between source and rebase can replace remote-tracking refs,
causing rebases to use stale bases. Now fetch to both a private ref and the
remote-tracking ref simultaneously, ensuring the tracking ref stays current.
Also improves process termination for WSL guests with process-group tracking,
fixes process-tree termination timeouts on POSIX, and serializes FETCH_HEAD
operations for linked worktrees through their shared Git directory.
* Add WSL setsid --wait probe and barrier termination timeout
Probe for `setsid --wait` support and fall back to unwrapped execution for BusyBox compatibility. Add a deadline for process termination barriers to prevent hanging when tree termination cannot be verified. Update tests for cross-platform compatibility.
* Add wsl-process-group-termination to WSL invocation allowlist
* Serialize per-worktree git mutations to fix rebase race
Introduce operation locking for each worktree to prevent concurrent
mutations (like rebase) from interfering with each other. Ensures
rebasing a linked worktree doesn't affect the source worktree state.
Add SIGKILL fallback if process termination barriers cannot verify
tree termination.
* Serialize pull and fastForward operations per-worktree
- Extract generic git operation lock to reuse locking pattern
- Refactor existing locks to use the generic implementation
- Apply per-worktree serialization to pull and fastForward to prevent races
* Route WSL group termination through runWslProcess
|
||
|
Jinjing
|
aa32871a61 |
Improve cmd j ranking with recency (#16281)
* Track container-only tokens and tab focus for cmd+j ranking Previously ranked by whether any container-only matches existed (boolean); now counts tokens matching only containers for finer-grained ranking. Tab focus recency is now tracked explicitly so recent refocuses rank above stale worktree activity. Preserves worktree grouping by input order while applying focused-group MRU within each block. * fix(cmd-j): preserve duplicate recent tab occurrences * fix(cmd-j): preserve host scope during worktree purge * fix(cmd-j): scope repo purge for exact-id host twins * fix(cmd-j): scope ssh visit recency to local to survive restarts Boot hydration loads only local + runtime:* partitions, so routing ssh-qualified recency to ssh partitions strands it across restarts. - Keep ssh-qualified visit timestamps in local partition - Route runtime-qualified keys to their partition - Remove groupId from recent tab occurrence base (unstable on regroup) - Collapse bare and host-qualified timestamps, preserving max - Simplify repo pruning host-match logic - Add robustness: optional chaining, helper function * Scope focused tab recency by worktree to fix Cmd+J ranking Tab ids can be duplicated across worktrees; scoping recency keys to per-worktree prevents one worktree's MRU position from overwriting another's in Cmd+J. Scope worktree order blocks to (hostId, worktreeId) to keep same-id worktrees on different hosts separate. Also fix recency preservation during partial identity migrations and prune orphaned host keys on removal. |
||
|
Brennan Benson
|
94f231737d |
fix(agent-status): retire panes whose agent process is gone (STA-4612) (#15212)
* fix(agent-status): retire panes whose agent process is gone (STA-4612) Agent status can hold `working` on a pane where no work is outstanding, and nothing closes the gap. A pane's Claude state is a join of a lead turn and three latches — the subagent roster, the background-task gate and the session-cron gate — and each is set by a hook and cleared only by another hook. Claude Code emits no terminating hook on `/exit`, `/clear`, Ctrl+C, crash, SIGKILL or terminal close, so every one of those latches is a claim with no owner and no expiry. The join is also materialised at ingest time and persisted, so a stale `working` survives restart and blocks hibernation, which requires `done`. Registering `SessionEnd` is not the fix: it covers roughly a third of exit paths (measured on 2.1.231/2.1.233; upstream anthropics/claude-code#17885 and #6428 are both closed as not planned). Nor is a TTL — `AGENT_STATUS_STALE_AFTER_MS` only decays the sidebar dot at read time while the stored row stays non-terminal. So the backstop is built from evidence Orca already owns. A session id that changes means the conversation was replaced. On the first hook of the new session — whatever that hook is — the previous session's own claims are void: its session crons and its one-shot subagents. Deliberately not voided: the background-task gate (a background shell is an OS process that survives `/clear`, and the previous inventory is positive evidence it was running), and `confirmedTeammate` rows (persistent in-process teammates a lead swap cannot end). The lead record is left to the incoming event's own fold. A certified process exit retires the pane. Orca already does this on every attributable PTY exit — `clearProviderPtyState` resolves the pane key and calls `clearPaneState` — but that resolution depends on the spawn-time `ptyPaneKey` mapping, which a restored or reattached PTY may never rebuild. Those panes keep their row and latches for good. `onPtyExit` knows the keys teardown could not resolve, so it reconciles them from its own records. The certificate is `exitCode >= 0 || hostExitConfirmed || providerExitObserved`: a synthetic `-1` from a failed stop is not a death (the PTY can have survived it), while a real exit can also report `-1`, so neither the code nor the SSH surface predicate is sufficient alone. `providerExitObserved` is additive and separate from `hostExitConfirmed`, which also drives the liveness verdict and the SSH surface decision. A confirmed shell foreground is the `/exit` case: the agent died, the shell lived. That already dropped the row, but through `agentStatus:drop`, which by its own contract preserves a live pane's caches — so every latch survived and the next event resolved the pane back to `working`. It now routes through the reconciler instead, gated on a per-pane accepted-status generation rather than row identity: the confirming process read can take seconds, and `updatedAt` cannot order two writes inside one millisecond (the store deliberately admits equal timestamps). Cold start generalises the same way. The startup sweep required a restored subagent roster, so a stranded lead row, background-task gate or cron gate — the shapes with no child event left to reap them — were never candidates. Hibernation needs no change: with the above, those rows become genuinely `done` and the lockout resolves through the front door. A `restoredUnconfirmed` bypass in the planner would let it reclaim the heap of an agent that may be working. Not included: folding `background_tasks` from a child-attributed `SubagentStop`. Writing its test surfaced #11838's deliberate assertion that child inventories are not authoritative for lead-owned background work, and the listener says the same — "background_tasks is trusted only where unambiguous". An empty list on a `SubagentStop` does not prove the lead's shell ended, so the fold would have cleared a gate on evidence that establishes nothing. STA-4119's live-side question — whether a genuinely live background shell should hold the lead row after the lead turn ends — is untouched. This change extends gate-clearing to zero new triggers. * fix(agent-status): make the confirmed-shell reconcile survive its own drop The /exit leg never fired. `settleDeferredCommandFinishedStatusDrop` runs the paired drop before the reconcile, and `dropAgentStatus` cleared the per-pane accepted-status counter the reconcile's guard then read — so the guard compared a live anchor against a zeroed counter and skipped itself on every pane that had a status row, which is every pane worth reconciling. The existing test passed only because it used a pane with no row, where the drop early-returns and both sides read 0. Stop keying the guard on a counter a sibling teardown path can reset: the ordinal is now stamped on the row itself, derived from the row it replaces, so there is no side table to clear and a batched burst lands the same ordinals as the equivalent sequential writes. A removed row means "nothing reported", which is exactly what the paired drop leaves behind. Also: - Keep the `providerSessionOnly` resume identity that the paired dismissal mints when the shell outlived the agent; a certified PTY exit still takes it, since there is no pane left to resume into. - De-vacuum two guard tests. The confirmed-teammate pin never anchored a session owner, so the void it claimed to survive never ran; the unavailable-inspection pin asserted before the confirm ladder settled. Both now fail when their guard is removed. - Derive `hasLiveClaimsForPaneKey` from a predicate that lives beside `clearPaneCacheState`, so a new latch cannot be added to the teardown and silently missed by the claim check. - Drop the unreachable compact-`trigger` clauses; SessionStart is the whole guard. - Cover the connectionId arm of the exit certificate, where a provider-observed death and a preserved SSH surface are deliberately independent. * fix(agent-status): keep agent-status-types under its line cap main already sits exactly at the 300-line max-lines cap for this file, so the single `acceptedStatusSeq` field this branch adds pushed it to 301 once main's observation facet merged in. Declared the field as a mixin beside the observation facet instead. Both are per-write facets mixed into `AgentStatusEntry` rather than fields a reporter supplies, so they belong together — and the capped file loses a line rather than gaining one, since it already imports from that module. No lint suppression. * fix(agent-status): collapse the entry facets into one intersection The previous attempt still tripped max-lines: two mixins on one intersection wrap across two lines under oxfmt, so removing the field line bought nothing. Expose a single AgentStatusRowFacets that already includes the observation facet, so the entry intersects one short name on one line. The payload keeps intersecting the observation facet alone — it must not carry the renderer-local ordinal. Verified by formatting first and then linting, which is the order that catches this. * fix(agent-status): retire resume authority with dead panes |