mirror of
https://github.com/stablyai/orca.git
synced 2026-09-29 16:02:50 +00:00
e7d23ea8cd151264f773dc9bcfa2bcc90fc13ef1
7974
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Neil
|
e7d23ea8cd | chore(persistence): keep the derivation predicate module-private | ||
|
Neil
|
6a05862e73 |
perf(persistence): drop the identity twin, not the locator row
Reverses the projection direction: `worktreeMeta` stays complete on disk and `worktreeMetaByIdentity[K]` is omitted instead, only when the locator row regenerates K by construction (`wt2:<hostId>:<instanceId>`) and the two rows are equal. That removes the format marker, the deliberate-absence list, both raw-file reader patches and every downgrade hazard, because "alias present, identity row absent, locator derives it" is a shape every shipped build already heals to exactly this state. Keeps 88% of the byte win (541 KB vs 613 KB) and the whole heap-sharing win. |
||
|
Neil
|
6e42c141eb |
fix(persistence): keep the lineage maps when a profile file has no worktreeMeta key
The rebuild returned `parsed.worktreeMeta` untouched when it was not a plain
record, so an absent key became an explicit `worktreeMeta: undefined` that
outranked the defaults spread. `normalizeWorktreeLinkedItemMetadata` reads a
non-object `worktreeMeta` as corruption and wipes that file's
`worktreeLineageById` and `workspaceLineageByChildKey` with it, then marks the
state dirty so the wipe is persisted. Before this branch the spread supplied
`{}` and the lineage survived.
Also pins the two raw-file readers the projection made load-bearing: the
history GC recovering projected ids from the alias keys (a miss deletes shell
history a live workspace is using), and the profile-transfer read rebuilding
the omitted locator rows (a miss transfers workspaces with no metadata).
|
||
|
Neil
|
59385b3e85 |
perf(persistence): stop writing every worktree metadata row twice
`setWorktreeMetaForHost` assigns one object to both `worktreeMeta` and `worktreeMetaByIdentity`, so the profile serialized every metadata row twice. On a measured 3.64 MB install, 1,347 of 1,349 locator rows were byte-identical to their identity twin. The serializer now omits a `worktreeMeta` row the identity map can rebuild, and the load path rebuilds it — reinstating the shared object reference `JSON.parse` splits in two. A row is only omitted when exactly one alias claims the locator and that alias names exactly one identity key, so the rebuild is a pure function of the file with no winner selection to disagree about. Omission rather than an in-value sentinel: a downgraded build reads a non-object `worktreeMeta` value as corruption and deletes that locator's lineage companions with it. An absent key is a shape every build already tolerates, and it falls back to the untouched identity map. |
||
|
Neil
|
c11c6878c1 |
perf(persistence): stop dead SSH leases pinning metadata, retire unreachable tombstones (#18430)
Two unbounded-growth fixes in the persisted profile, which is re-serialised in full on every save. `collectPersistedWorkspaceOwners` registered every SSH lease's worktreeId as a live persisted owner with no state filter, so a route-retired lease — the operator-close `terminated` tombstone, or an `expired` row already marked `supersededBy`/`relayIdRecycled` — pinned its worktree's metadata row permanently. The prune gate's own doc names that failure: "Rows pinned by a persisted session are never removable, so the repetition cannot even make progress." Reuses `sshRemotePtyLeaseAllowsReattach`, the predicate that already decides which leases still name a route. `sshRemotePtyLeases` had no pruning path at all: removal happens in three explicit places, none age- or state-based, so `terminated` rows accumulated forever (137 rows / 54 KB on the reported profile, ~38/day from one target). Marking a lease `terminated` scrubs its pane bindings in the same write, so once no persisted binding names the id the row routes nothing — reattach, pane recovery, the orphan sweep, `ssh:reset` and `ssh:terminateSessions` all behave identically on an absent row. Delete it then, gated on that reachability check because a lease freezes its tabId and the tab-qualified scrub cannot reach a pane that was detached into a new tab. `expired` rows are deliberately untouched, superseded ones included: `sweepOrphanedRelayPtys` reads those ids as its leave-alone list, so dropping one would authorize stopping a remote shell that supersession left running on purpose (docs/reference/ssh-execution-boundary.md). |
||
|
Neil
|
ab32c2c0c5 |
perf(startup): stop the persistence milestone from timing its own details closure (#18439)
`logPersistenceStartupMilestone` resolved the lazy `details` closure before reading `performance.now()`, so the 1.6 MB `JSON.stringify` that `persistence-load-done` uses to report `workspaceSessionBytes` was billed to the milestone it measures. Snapshot `t` first. Diagnostics output is unchanged; only the recorded timestamp moves. |
||
|
Neil
|
6415b1dc22 |
perf(images): probe raster headers instead of decoding whole payloads, memoize repo icon validation (#18421)
* perf(images): measure raster headers from a probe and memoize repo icon validation
`getRepos()` re-sanitizes every repo on every call, and an uploaded/file repo
icon costs a full base64 decode of its data URI each time. Three fixes:
- `writeQuartet` destructured a mutable array, which sends V8 through the
iterator protocol once per four input characters; index reads plus a length
counter produce identical bytes.
- `decodeBase64Prefix` decoded the whole payload despite only the first bytes
being needed. `exceedsRasterImagePreviewLimits` now probes 64 bytes and
widens x16 until the header measures, and only re-runs the original
full-payload decode when the verdict would suppress a preview.
- `sanitizeRepoIcon`'s src validation is memoized per icon source with a
bounded FIFO map, reusing the `memoizeTitleClassification` idiom (now a
shared `memoizeByStringKey`).
* perf(images): key icon-validation memo on the persisted icon object
Replaces the per-source 64-entry FIFO string-key memo with a WeakMap keyed on
the persisted repoIcon object that hydrateRepo already receives, storing
{src, source, supported} and re-checking both fields on a hit.
Retention becomes zero by construction (entries die with state.repos[i].repoIcon),
so there is no cap to evict live icons and no dead icon strings held after a repo
or icon is replaced. The identity re-check makes an in-place mutation unable to
serve a stale verdict. Drops bounded-string-key-memo.ts and reverts the collateral
terminal-title-classification-memo refactor.
|
||
|
Neil
|
97e5eb8886 |
perf(paths): guard the no-op regex passes on the path-comparison hot path (#18418)
* perf(paths): guard the no-op regex passes and hoist the loop-invariant root `normalizeRuntimePathForComparison` ran two whole-string regex passes on every call — `/\/+/g` and `/\/+$/` — that cannot change a path with no doubled slash and no trailing slash, which is nearly every path. `parseWslUncPath` likewise folded backslashes and ran an anchored UNC regex over every POSIX path. Substring/char-code probes skip all of them, and a `createRelativePathInsideRootResolver` factory (mirroring the existing `createNormalizedPathInsideOrEqualMatcher`) folds a fan-out's root once instead of once per candidate. Outputs are unchanged; a seeded 200k-path differential fuzz against a pre-guard copy proves it. * perf(paths): drop the root hoist, land the guards alone The three in-module guards are the whole win: 5000-op batches, CPU time, median of 9 --- normalize 541 -> 239 ns/op, relativePathInsideRoot 1778 -> 899, isPathInsideOrEqual 1076 -> 572, parseWslUncPath 57 -> 14. The loop-invariant root hoist added 176 ns/op on top of that (899 -> 723) at 7 hand-picked call sites, and cost a new exported factory whose input contract is the opposite of the one next to it, plus a function substitution in worktree/ownership.ts. Not worth 0.9 ms per storm. Prod diff: 2 files. New ratchet pins the single-factory surface. * docs(paths): point the fixture header at the real guards test |
||
|
Neil
|
7ed86a98ae |
perf(ipc): index worktree owners instead of rescanning the repo list per lookup (#18416)
Two hot lookups rescanned a whole table once per repo. `getLocalRepoForRegisteredWorktree` (59 IPC call sites, including Quick Open keystrokes and every File Explorer expand) walked the entire worktree-meta table once per repo. One pass now collects the owning repo ids, built lazily so a repo whose own path matches still never touches the table. `createRepoRowExecutionHostLookup` re-filtered the repo array on every `byId` / `byHost` call. Rows are grouped into a Map once at construction, preserving repo-list order so `rows[0]` still picks the same owner. |
||
|
Neil
|
63aee7f1ee |
perf(terminals): spend one inspection start on a whole cadence round (#18438)
The inspection rate limiter counted panes when it should have counted host observations. `MAX_INSPECTION_STARTS_PER_SECOND = 8` is global, and it was spent one pane at a time, so N due panes meant an effective per-pane period of max(tier, N/8 seconds) — ~37.5s at 300 panes for a pane the code polls at 750ms. Agent-completion latency degraded monotonically as panes were added. Every local pane's inspection resolves out of the same TTL-and-in-flight- deduped process-table capture, so a whole round of them is one host observation. The queue now drains all shared-observation tasks as one round on one start, launched in a single tick. Remote panes each cost their own execution-host round trip and stay admitted one at a time. Both the budget and the cadence tiers are numerically unchanged. Disposed tasks are also compacted out in one pass instead of a splice per drop, so the per-round predicate cost is linear rather than quadratic at pane scale. No IPC, preload, wire, or main-process change: each pane keeps its existing per-pane `pty:inspectProcess` invoke. |
||
|
Neil
|
07e50e9513 |
perf(terminal): scan only new tail lines for the wait-blocked sentinel (#18437)
* perf(terminal): scan only new tail lines for the wait-blocked sentinel The wait-blocked scan must prove a signal is ABSENT, so it could not early-exit and re-tested all 2000 retained lines with a 13-alternative regex on every scan (20/s per streaming PTY) even though only ~20 lines were new. Index the matching line indices per tail-array identity and carry them across appends, testing only the lines each append produced. Also carries the retained character total and the redraw prefix's right-trimmed state across appends, so a saturated tail is no longer re-summed and re-scanned per chunk. * perf(terminal): build the carried tail window and its match index from one constructor |
||
|
Neil
|
a711cb8b60 |
perf(renderer): gate the tab strip's worktree subscriptions and fix the orchestration batch's self-invalidating cache (#18428)
* perf(renderer): gate the tab strip's worktree subscriptions and stop the orchestration batch invalidating itself Two store-subscription hot paths. The tab strip subscribed to projects/repos/worktreesByRepo for the Windows shell menu's local project runtime, which is never built unless that menu is on. On macOS/Linux every worktree write therefore re-rendered and re-committed every mounted tab strip. Gate the three on the condition that already gates their only consumer. The runtime-orchestration batch keyed its cache on agentStatusByPaneKey identity, which `agentStatus:set` replaces by definition, so it missed 100% of the time on the only event that calls it. Key on the paneKey -> worktreeId pairs the batch actually reads instead, and hang the requested-id array off the existing activeWorkspaces memo so the O(worktrees) prologue stops running per event. * refactor(renderer): make the orchestration batch's cache key its build's only inputs buildRuntimeBatch no longer receives agentStatusByPaneKey/retainedAgentsByPaneKey. It takes a RuntimeBatchInputs record whose paneWorktreeIds projection is its whole view of those maps, and that same record is the cache key, so the key cannot drift from the read set. Adds a guard asserting one read per orchestrated pane per map. * refactor(renderer): move the orchestration projection key onto the shared index The batch builder and `worktree-agent-orchestration-index.ts` were near-duplicate implementations of the same attribution walk, and both had the self-invalidating `liveSource === agentStatusByPaneKey` gate. Fixing only the batch left the index — which every mounted WorktreeCard hits on every `agentStatus:set` — still rebuilding per publication. Put `paneWorktreeIds` on the index instead and reduce the batch to a `.get`-compatible view of it. That deletes the whole `requestedWorktreeIds` apparatus the batch fix needed (the `worktreeIds` memo threading, the optional `selectDashboardOrchestration` param, the `uniqueWorktreeIdsByInput` WeakMap and its no-mutation contract, `getRequestedTabMembership`), leaves one builder guarded by the index's randomized oracle test, and extends the fix to the sidebar. The projection is memoised on the live/retained map identities so it is computed once per publication rather than once per card, and a successful ordered compare adopts the new array so the remaining cards compare by identity. |
||
|
Neil
|
34222e0137 |
perf(orchestration): project explicit columns so the graph publish stops recompiling SQL (#18420)
* perf(orchestration): cache the prepared statements the graph publish recompiles SyncDatabase refuses to cache any `SELECT *` — node:sqlite can build the first row after a schema change from stale column names — so every wildcard read in the orchestration DB recompiles its SQL on each call. The graph publish runs that fan-out once per pane, ~0.7 times a second, forever. Add a per-connection prepared-statement cache scoped to the orchestration DB, whose schema is frozen in the constructor (createTables/migrate/trigger) and whose resets are DELETE-only, and route the buildByPaneKey -> getForHandle -> getRecent path through it. 5 publishes over 2 panes: 30 compilations -> 2. * perf(orchestration): project explicit columns so the existing cache covers the hot path Replaces the branch's second statement cache. The six graph-publish reads were uncacheable only because they were spelled `SELECT *` / `SELECT t.*`, which SyncDatabase refuses to cache (node:sqlite can build the first row after a schema change from stale column names). Spelling the projection out from type-checked column tuples makes them cacheable by the SyncDatabase LRU that is already merged, already bounded, and already clears on DDL — so the WeakMap and its documented cross-connection ALTER hazard both go away. Drift is caught at build time: `satisfies readonly (keyof Row)[]` plus an `Exclude<keyof Row, Cols[number]> extends never` assertion pins list vs type at tsc, and a PRAGMA table_info test against a freshly migrated OrchestrationDb pins list vs schema. Same win, verified: 6 compilations per publish -> 2 total then 0, identical to the WeakMap branch; 92/96/91 us CPU per 2-pane publish before, 11-12 us after on both. |
||
|
Neil
|
8c1a28d39c |
fix(i18n): repair French locale drift breaking static analysis (#18550)
The French UI locale landed with two catalog drifts that fail `static analysis` on every PR in the repo: - `fr.json` carried 15 keys absent from `en.json` (and from every other locale), so `verify:localization-catalog` rejected it. They are stale entries generated against an older `en.json` snapshot; none is referenced anywhere in the source. - `settings.appearance.language.french` had no call site supplying a literal default, which promotes it to a boot-bundle-required entry that `en-runtime-required.json` does not ship, so `verify:localization-runtime-catalog` rejected it. Registering the key in settings search alongside its siblings fixes the runtime-catalog failure at its source and closes the real gap the drift exposed: French was the only supported language not findable in settings search. `en-runtime-required.json` is deliberately untouched — the sync script regenerates it wholesale and would drop 925 entries the check itself documents as harmless. |
||
  Brennan BensonandMerge Sim
|
90780acb85 |
refactor(agents): one pane-identity resolver behind six thin adapters (tranche 0) (#18243)
* feat(agents): pane-identity canonical adapter, comparison telemetry, inventory ratchet phase 1 * fix(agents): preserve canonical coverage provenance * refactor(agents): unify pane identity adapters for tranche 0 * fix(agents): keep title resolver cache-free after rebase * Fix ladder tranche zero review findings * fix(agents): restore title classifier memoization * fix(agents): fence unknown canonical evidence sources * docs: drop the ladder plan and decision table from the PR Design docs stay out of the shipped tree; the code carries its own comments and the decision table lives in the test fixture. --------- Co-authored-by: Merge Sim <sim@local> |
||
|
Neil
|
8463dcb7b9 |
fix(terminal): make wrapped-line search rewind iterative and bound its scans (#18402)
Patches @xterm/addon-search so one very long un-newlined line no longer overflows the stack, freezes the renderer, or goes unsearched. Submitted upstream as xtermjs/xterm.js#6149 (issue #6148); drop the patch once a release ships it. See the PR for measurements and the differential fuzz. |
||
 foXaCe
|
49d6d35b16 |
feat(i18n): add French UI locale
foXaCe <290678+foXaCe@users.noreply.github.com> |
||
 hwantage
|
48cb575db5 |
feat(i18n): localize Orca Account settings and navigation to Korean
hwantage <82494320+hwantage@users.noreply.github.com> |
||
 Jinwoo Hong
|
dec1a1d788 |
fix(i18n): ship onboarding integration capability strings in boot catalog
Jinwoo-H <73622457+Jinwoo-H@users.noreply.github.com> |
||
|
Neil
|
e42c60e8a3 |
fix(ssh): resolve a pane's binding from the target partition, not the stale local copy (#18546)
One SSH pane accumulated one extra reattachable lease per relay restart (2, 3, 4, 5, 6 across five), and every one of them costs a `pty.attach` round trip on every later connect, forever. Nothing prunes `sshRemotePtyLeases`, so the fan-out only grows. `supersedeSiblingLeasesForPane` is fenced on the PTY the pane is durably bound to, and `durablyBoundPtyIdForPane` read `state.workspaceSession` (local) before `workspaceSessionsByHostId['ssh:<target>']`. But `persistPtyBinding(binding, hostId)` updates ONLY the host partition: AFTER-PERSIST local= ssh:t@@pty2:old:1 host= ssh:t@@pty2:new:1 So for the length of a reconnect the local copy still names the predecessor, the fence resolved to it, supersession took an already-`expired` lease as its winner, and returned having marked nothing. Both partitions agree again once the renderer republishes its layout, which is why the settled store looks consistent and hid this. Read both partitions as an ordered list, target's own first, and test the fence by membership rather than by equality with whichever was read first. Pick the winner preferring a lease this client still has a route to, since the stale partition names an expired one. Never retire a lease that is both bound and live, so a partition disagreement can't strand a running remote process. Superseded predecessors stay `expired` and are never `terminated`: losing a lease is not evidence the shell died (docs/reference/ssh-execution-boundary.md). A pane with no binding is skipped rather than pruned, so a genuine orphan stays askable. Also re-runs supersession from the binding side after each spawn commit's binding write, so the lease/binding order at a call site no longer decides, and reconciles every pane for a target immediately before `reattachKnownPtys` reads the set it feeds to `pty.attach` — that repairs stores which already accumulated these rows. The guard suite could not catch this: every assertion bound the pane BEFORE upserting the lease, an order no caller uses. Rewritten to the spawn commits' real order (lease, then binding, then the binding-side trigger); it fails 8 assertions without this change. Added a suite that drives the real `persistPtyIpcSpawnCommit` rather than the store primitives, including the exact stale-partition state written by production's own binding writer. Verified on the Docker SSH lane: five `relay.js` SIGKILLs with recovery between each, reattachable leases flat at one per pane. Note: this bounds the reattach SET, not the store. `sshRemotePtyLeases` still has no cap or TTL and rows still accumulate; pruning is left alone deliberately, since an `expired` row without `supersededBy` is a genuine orphan and must not be dropped on age. |
||
|
Brennan BensonandMerge Sim
|
e85ebb0086 |
feat(native-chat): restore the terminal/chat switcher for bridge chat only (#18532)
* feat(native-chat): restore the terminal/chat switcher for bridge chat only #16729 removed every user-facing terminal<->chat switching affordance as a side effect of the structured Codex restructure ("renderer switching affordances and their dead leftovers"). That was right for structured Codex sessions, which render their own transcript with no live TUI underneath, but it also took the switcher away from bridge native chat, which still reads the terminal and has one to return to. Restore all four surfaces, each gated so structured sessions keep the removal: - pane header chat/terminal button (TerminalPaneHeaderOverlay) - pane context-menu "Switch to chat/terminal view" (TerminalContextMenu) - tab context-menu equivalent (SortableTabContextMenu) - the keyboard chord, whose hook had survived uncalled since #16729 Gating is one rule in one place: `canSwitchNativeChatView` refuses whenever a `structuredSessionId` is present, over the existing `canToggleNativeChat` eligibility. Standalone structured tabs are already excluded by the `contentType === 'terminal'` check; the new guard covers a terminal tab that adopted a structured session. The shortcut hook applies the same rule. The state plumbing (`viewMode`, `setTabViewMode`, `toggleTabViewMode`, host mirroring, `native_chat_toggled` telemetry) was never removed, so this rewires live actions rather than reintroducing logic. SortableTab.tsx sat exactly at its 400-line cap, so its inline-rename state and the window rename-request listener move to `use-sortable-tab-rename.ts` to make room. No behavior change; its rename tests pass unmodified. Two ratchets move for real, explained in place: - store-subscription budget: per-pane listeners stay pinned at 17 (the folded action bundle is still one listener); only the counterfactual pre-fold constant grows 48 -> 49 for the added `toggleTabViewMode` key. - hook-order parity: 204 -> 208 hooks for the four added `useCallback`s, useMemo count unchanged at 8. * fix(native-chat): restore bridge chat escape hatch * test: update pane agent identity inventory --------- Co-authored-by: Merge Sim <sim@local> |
||
|
Neil
|
5d8532f6d3 |
fix(worktrees): resolve the execution host at both worktree-create entry points (#18545)
Two entry points create the same workspace and disagreed about how to read its host. `orca-runtime-create-managed-worktree.ts:63` resolved through `getRepoSshConnectionId` and then normalized the row; the `worktrees:create` IPC handler branched on raw `repo.connectionId` (`register-worktree-create-handlers.ts:66-69`). So a repo naming its owner only as `executionHostId: 'ssh:<target>'` created remotely through the runtime and ran `git worktree add` on the client against a remote path through IPC (#11163). Same repo, two entry points, different answers. Both now take one route, resolved through the existing layer (`getRepoExecutionHostId` -> #18296's `resolveGitRouteForHost`). No new resolver. The row normalization on the `ssh` variant is kept, and it is a **workaround, not the pattern**. `createRemoteWorktree` and its callees re-read `repo.connectionId!` at five depths in `ipc/worktree-remote.ts` (1627, 1847, 1848, 1865, 2029), so the resolved connection has to reach them through the field they already read. It travels only as far as that object does — anything downstream that re-reads the row from the store still sees the unnormalized one, and it cannot express the `runtime:` refusal on its own. Proper fix, deliberately not done here: give that pipeline an explicit connection parameter and delete `repo.connectionId!` from it so every reader becomes a compile error, the technique #18307/#18325 used. That is a change inside a 2800-line module plus its callers, and it wants its own PR. Three answers that used to collapse into one, now distinct at both entry points: - `executionHostId: 'ssh:*'` with no `connectionId` -> that SSH host (IPC used to create locally); - `executionHostId: 'local'` with a surviving `connectionId` -> local, since a local row cannot nest an SSH namespace. This is what `getRepoSshConnectionId` and therefore the runtime sibling already answered; IPC used to go remote; - `runtime:<env>` -> refused. Its worktree is created by that environment's own server and the SSH target on its repo row is that server's nested one, addressable only as (environmentId, targetId). The renderer already routes runtime-environment creates over `worktree.create` RPC rather than this IPC channel, so reaching either entry point with one is a routing mistake. Matches `workspace-cleanup-git-route` and `runtime-git-command-target`. Folder-workspace creation is untouched on both sides: it is a registration, not a filesystem create, so the route is resolved after that branch on the IPC side, and on the runtime side only the agent trust write consumes it — where a `runtime:` host now yields `null` instead of the nested target, so the write stops going to a same-named target in this client's table. No wire or persistence change: the normalized row is a local value passed to the create pipeline, never stored, and `CreateWorktreeResult` is untouched. |
||
|
Neil
|
3c91404820 |
fix(worktrees): route managed worktree removal by resolved execution host (#18529)
`removeManagedWorktree` resolved its host once — for the metadata prune (`cleanupHostId ?? getRepoExecutionHostId(repo)`) — and then read raw `repo.connectionId` for every step that touches the filesystem: the `git worktree list` deciding whether the path is registered, the provider handed to the unregistered-removal branch, the registered-remote-vs-local fork, and the PTY/history teardown. One function, two spellings. For a row naming its owner only as `executionHostId: 'ssh:<target>'` — the exact class #18296 names — the list ran on the client against a remote path, `removeRuntimeUnregisteredWorktree` was entered with `provider: null`, and the metadata was pruned under `ssh:<target>` while a same-named *local* directory was the one considered for deletion (#11163). #18358 made this reachable: it migrated the cleanup scan, so `executionHostId`-only rows now surface as removable candidates, but removal did not move with it. Routing is now one answer for the whole removal, taken from the host the prune already used, through #18296's host-keyed dispatch. The ambiguous `provider: SshGitProvider | null` carrier is deleted from the callees rather than supplemented, so every remaining reader is a compile error in the typed modules that do the destructive work (`runtime-unregistered-worktree-removal`, `runtime-registered-remote-worktree-removal`, `runtime-worktree-filesystem`). The orchestrator itself carries `@ts-nocheck` from its mechanical split, so that guarantee does not reach it — tests cover it instead. Every change is in the refusing direction; nothing became more aggressive: - an `ssh:` host with no registered provider throws instead of deleting a client-side path (`requireSshGitProvider` already threw for rows that spelled the same host as `connectionId`); - `runtime:<env>` throws rather than dialling a same-named target in this client's namespace, matching `workspace-cleanup-git-route` and `runtime-git-command-target`; - the folder-workspace teardown resolves its connection instead of reading the raw field, so a `runtime:` row stops dialling the wrong namespace. No wire or persistence change: `removeWorktreeMetadataAndHistory` already took the resolved host, and the removal RPC result shape is untouched. |
||
|
Neil
|
04ae62202a |
fix(ssh): close the macOS relay's per-terminal pty fd leak (#18534)
The relay asset from #17920 only rewrote the forkpty `default:` call site, which sits in the `#else` arm of PtyFork's `#if defined(__APPLE__)`. macOS takes `pty_posix_spawn`, so the asset had never patched anything a Mac executes -- and `applyNodePtyMasterCloexecPatch` returned 'fixed' for any non-Linux host without running the script at all, which is what publishes a tree to the shared native-deps cache. Stock `pty_posix_spawn` opens up to three throwaway ptys to push the real master off fds 0-2 and never closes them: the cleanup loop is `for (; count > 0; count--)`, but the first `posix_openpt()` in a running process already returns >= 2, so it breaks with `count == 0` and the body never runs -- and where it does run it closes `low_fds[count]`, never `low_fds[0]`. One orphaned /dev/ptmx fd per terminal, for the life of the relay. Ports the `low_fds` fix and the Apple-branch `pty_cloexec(master)` call from the app's `config/patches/node-pty@1.1.0.patch`, byte-identical, and runs the gate on darwin. macOS needs a different build layout than Linux: it has no `build/` at all, so the fallback moved aside is `prebuilds/darwin-<arch>` -- which is also what makes node-pty's install script fall through from "prebuild found" to node-gyp -- and the compile writes a `build/Release` the loader checks first. Verification is per-platform too: Linux's leak is inheritance (/proc), macOS's is self-held (lsof). Also corrects the asset's claim that "macOS re-opens the tty through uv_tty_init's cloexec dup". Measured false: FD_CLOEXEC is not set on the master. What protects it is POSIX_SPAWN_CLOEXEC_DEFAULT, one option away from gone since uid/gid drops libuv back to fork()/exec() -- so the master is now marked there too. Measured on darwin-arm64, one PTY per open/close cycle in a relay-shaped dir running the relay's own commands: before cycle:ptmx 1:1 2:2 3:3 ... 10:10 (10 after a settle) after cycle:ptmx 1:0 2:0 3:0 ... 10:0 (0 after a settle) Linux re-verified in docker node:22: inherited before, isolated after, `already-patched` on the second run. Refs #17915 Refs #8362 |
||
|
Neil
|
a5d6114baf |
fix(ssh): stop pane adoption certifying a death from the relay's not-found union (#18531)
* fix(ssh): stop pane adoption certifying a death from the relay's not-found union
`attachStablePaneOwner` was the last reader that synthesised a runtime exit
from a reattach refusal, and it published code `0` — which
`orca-runtime-on-pty-exit` records as `rememberPtyLivenessVerdict(exited)`, a
death certificate whose only legitimate writer is a host-delivered exit frame.
The refusal it acted on is a union. `pty.attach` answers `PTY "<id>" not found`
both for a pid the relay probed with `isProcessAlive` and for an id its session
map simply never had — which, because ids carry a per-start mint epoch, is every
id minted before a relay restart, checked against nothing. So a relay restart
plus a reconnect certified a shell that was still running under the old daemon's
orphaned process tree, retired the pane binding, and cold-started a second agent
onto the same transcript. The sibling `handlePtyReattachFailure` has always
refused to certify from that union; this path did not.
- The relay marks the one refusal it backed with a liveness check
(`PTY_ATTACH_PROVEN_EXITED_MARKER`). The marker is additive, so an unmarked
answer — including an older relay's — stays ambiguous, which is the safe
direction.
- The client mints that half as `SshPtyProvenExitedOnRelayError`, a subclass so
every existing `isSshPtyAbsentFromRelayError` consumer is unchanged.
- Pane adoption publishes `UNVERIFIED_PROCESS_EXIT_CODE` (-1), the sentinel its
sibling publishes, and passes `hostExitConfirmed` only for evidence that
observed the process: the marked relay refusal, or `SessionNotFoundError` from
the registry that owns the PTY. The ambiguous half now records `unverifiable`
instead of `exited`.
- The gone-branch keys on the error type rather than the bare `PTY ".+" not
found` text, so an untyped string can no longer authorise abandoning a
binding — the discriminator `pty-connect-limits.ts` already documented.
Refs docs/reference/ssh-execution-boundary.md
* test(pty): make the pane-adoption fixtures throw what real providers throw
These four fixtures rejected with bare `new Error('Session not found: ...')` and
`new Error('PTY "..." not found')`. No provider produces either untyped:
`local-pty-spawn` and `decodeDaemonResponseError` both mint
`SessionNotFoundError`, and the SSH reattach path types the relay's wire text
before any pane sees it. Fixtures that skip the type were the reason a
message-shaped gate looked adequate.
The exit-code expectations move with it: the pane path now publishes the -1
stop sentinel plus `hostExitConfirmed`, so a certificate follows the evidence
rather than a synthesized zero.
|
||
|
Brennan BensonandMerge Sim
|
98e77ef1a7 |
feat(mobile): structured native Codex chat (#18074)
* feat(mobile): finalize structured native Codex chat * fix(mobile): close structured chat lifecycle gaps * wip(mobile): fence stale structured inventory and bound operation-id retention Fence local structured-session inventory and subscription responses with a sync generation so a toggle-off clear, reconnect restore, or retry cannot apply a mirror from a superseded instance. Bound mobile ambiguous operation-ID retention at 128 with unmount cleanup. Staged on the reconcile branch only: the sync module is now 312 lines and needs a real split before this can reach the PR head. * fix(ci): split the structured session-tabs sync and give static analysis mobile types The local structured session-tabs sync module outgrew the 300-line cap once it took on generation fencing, so split it along its real seams instead of raising the cap: the generation/cursor fence, snapshot projection, snapshot apply, inventory refresh, and the subscription loop. The original path stays as a barrel so no importer moves. Repoint the host-session-mirror settle census at the apply module, which owns two receipts now — the snapshot it mirrors in, and the toggle-off teardown that retracts what it published. The teardown receipt is named rather than anonymous so the pin says which direction it settles. The changed-code quality gate lints mobile files and resolves their types from mobile/node_modules, but mobile is a separate pnpm project that the root install never populates, so every mobile type degraded to an `error` type and the gate reported phantom findings. Install mobile dependencies in static analysis when the diff touches mobile, gated on a new classifier output. * fix(mobile): let a slow capability handshake still reach connected The mobile capability update is an advisory whose result is discarded, yet an unanswered one was fatal while an explicit rejection was tolerated. A 5s timeout on the direct client force-closed the socket, and on the relay path it failed `confirmResume` before `connected` was ever published, so a consistently slow link redialled forever. Both paths now share one helper that settles every ambiguous outcome (timeout, mid-flight drop) like a rejection and rejects only when the frame never reached the wire — the one case nothing else recovers from, since the socket's own desync force-close is gated on already being connected. The generation guard still keeps a replaced session from connecting. Retained structured-session operation ids were capped at 128 with oldest-first eviction, but every retained id belongs to a send whose outcome is unknown, so eviction turned a user's retry into a second message on the host. Bound the map by expiry against the id's own embedded timestamp instead, mirroring the host's operation ledger, so no id is released while the host would still honour it. Also give the mobile CI install the root install's lockfile drift guard (mobile's lockfile carries patchedDependencies a silent rewrite would drop), gate mobile_dependencies on should_run, and key the pnpm store cache on both lockfiles. * refactor(mobile): extract the relay pending-request registry The merge composed two independently-sized changes — this branch's capability handshake settle and main's dial-stage tracking — pushing the relay session file to 304 lines against a 300 cap. Neither side broke it alone. Move the in-flight request registry (id generation, tracking, settlement, and reject-all with its delivery-ambiguity marking) into RelayPendingRequests, matching the existing collaborator pattern alongside RelayDialStageTracker and RpcSessionLivenessWatchdog. No behavior change. --------- Co-authored-by: Merge Sim <sim@local> |
||
 mors
|
c79e1c097b |
fix(i18n): localize remaining onboarding UI
Reviewed and approved by Codex. |
||
 韦编三绝
|
f13f2472c6 |
fix(i18n): distinguish Duplicate from Copy in Simplified Chinese
Reviewed and approved by Codex. |
||
 Ilya Gusev
|
8262fb147f |
fix(i18n): extract translateSearchKeyword calls so settings-search keywords reach en.json
Reviewed and approved by Codex. |
||
 Jinjing
|
b1186c6beb |
Fix scope of workspace-creation-project tour target (#18502)
* fix: scope workspace-creation-project tour target to project picker only The tour target was previously applied to a container that included both the project picker and the run target picker below it. Restructure the layout to scope the target to only the project-related section, and add a test to verify the tour target does not span into the run target picker. * fix: scope workspace-creation-project tour target to project picker only Move the tour target attribute from the outer project section to an inner wrapper around just the combobox and its messages, excluding the header label and "Add project" button. Update tests to verify the narrower scope. |
||
|
Neil
|
f35015d0c8 |
fix(ssh): measure pane idleness in the unit the sweep's kill operates on (#18415)
The orphan-relay-PTY sweep authorizes `pty.shutdown { immediate: true }`, which
runs `forceKillPosixPtyProcessGroups`: collect every process group on the pane's
tty, then `killpg` each one. The blast radius is therefore (groups on the tty) x
(members of those groups, wherever they are). The idleness evidence measured only
the first factor, so three shapes read as idle and were SIGKILLed:
- with job control off (`set +m`) a background job keeps the SHELL's pgid, so the
tty carries exactly one process group and that group is running the user's build;
- a child that drops the controlling terminal (`ioctl(TIOCNOTTY)` without `setsid`)
keeps the pgid, reports `tpgid == -1`, and never appears in `ps -t <tty>`;
- a double-forked grandchild keeps the pgid and tty but reparents to pid 1, so the
`ppid` walk cannot reach it and the named-process backstop never fires.
`shellOwnsEveryTtyProcessGroup` now also requires the shell's own process group to
hold no other member anywhere in the table, indexed in the same single pass. A
pids-per-tty set would catch the first and third but not the second, which is why
the count is pgid-wide rather than tty-scoped. The wire field keeps its tty-shaped
name: the value only ever became stricter, so an old client skips more, never less.
Second, unrelated-in-mechanism but same file family: `foregroundSkipReason` summed
`capturedAgeMs + evidenceAgeSinceListingMs` without validating either. A non-numeric
`capturedAgeMs` makes the sum `NaN`, and `NaN > 5000` is false, so a malformed record
PASSED the freshness gate and proceeded toward the stop — the one place in the file
that defaulted toward kill. Nothing validated it on this path
(`mapSshPtyProcessList` checks the ownership fields and spreads the rest through;
`PtyProcessListAdmission` is not on the sweep path). It now runs
`isForegroundProcessEvidence` and fails closed.
Verified on real Linux, not only in mocks: a container drives `bash -i` on a real
pty, builds each construction, runs the real publisher and planner, and then calls
the real `forceKillPosixPtyProcessGroups`. Before, all three published
`shellOwnsEveryTtyProcessGroup: true`, planned SWEEP, and the planted pid was gone
after the signal. After, all three skip and survive, and an idle shell is still
reclaimed.
Residuals are written down at the predicate and in ssh-execution-boundary.md: the
capture is a snapshot (bounded by the evidence-age budget, not removed), and a
process the host's own `ps` cannot enumerate stays unobservable while `killpg`
still reaches it.
|
||
|
Neil
|
95eed52801 |
fix(cli): report which hosts a worktree listing covered, and stop the cap starving remote ones (#18417)
`orca worktree list` returned zero of 24 SSH worktrees at the default limit (#18104). Rows are resolved repo by repo, so every SSH repo's rows land contiguously at the end of the fleet order — the 24 remote rows sat at indices 496-520 of 521 and a plain `slice(0, 200)` never reached them. The omission was not fully silent: text output printed `truncated: showing 200 of 521` and JSON carried `totalCount` / `truncated`. What was missing is that the omission was *categorically every remote host* — no host column, no `hostScope`, nothing to distinguish "200 of 521" from "one host is entirely absent". Per docs/reference/ssh-execution-boundary.md, a listing that does not name its scope reads as absolute. Adopt the mechanism `terminal list` already has rather than inventing a second one: - `RuntimeTerminalListHostScope` becomes an alias of a shared `RuntimeListingHostScope`, now also carried (optional, so old hosts are unaffected) on `worktree.list` and `worktree.ps` results. - `src/shared/host-balanced-listing-page.ts` round-robins the row cap across hosts and returns the survivors in the caller's original relative order, so the page stays a subsequence of the unbounded listing and nothing downstream re-sorts. An uncapped listing is returned unchanged. - `worktree list` / `worktree ps` text output gains a `host=` column and the same trailing `scope:` line `terminal list` prints. Third defect, same mechanism: `hostScope.omittedHostIds` is built from the runtime's own bookkeeping, so it names `runtime:` ids for servers that are no longer paired — 6 of 9 in the recorded QA run hard-error when queried. Since `hostScope` is *the* documented way to complete a partial listing, that makes the mechanism unreliable for its intended use. Annotate rather than filter. Dropping an id would shrink what the listing admits it did not cover, and the boundary doc requires a listing to name its gaps — the gap is real whether or not this machine can name the host that owns it. `src/cli/omitted-host-scope-selectors.ts` resolves each omitted id against this machine's pairing store and the runtime's SSH-target registry and attaches the exact flag that reaches it, or `null` marked "not selectable from this machine". This is a client-side annotation: nothing new goes over the wire, it answers "can I select it" and never "is it up", and the SSH round trip is only paid when an `ssh:` host was actually omitted. No `--host` filter was added; the host column plus scope line covers the reported need without a new selector axis. |
||
|
Neil
|
9bed758e36 |
fix(cli): reject runtime selectors on host list and environment list (#18405)
`orca host list --environment m4air` was not ignoring the flag — it was applying it to half the answer. `shouldIgnoreRemoteSelection` never pinned the `host` family, so the SSH-target lookup was routed to m4air while paired servers were still read from this machine's own pairing store, and the handler stamped the envelope `_meta.runtimeId: "local"` regardless. The result was one listing describing two hosts: the openclaw row silently disappeared, which reads as "m4air has no SSH targets". `environment list --environment X` had the pin but no guard, so the flag vanished with no signal at all. Reject rather than route. `host list` answers "what can this machine target and with what flag"; its paired-server half comes from a client-local store and cannot be routed at all, so any routed answer is necessarily half-substituted — rule 1 of docs/reference/ssh-execution-boundary.md. `environment list` is entirely client-local, so there is no other host to ask. This matches the `account` and `artifacts` precedent, the only two pinned families that already paired the pin with a rejection guard. - pin the `host` family so an ambient ORCA_ENVIRONMENT cannot produce the same two-machine listing with no flag to reject; `runtimeId: "local"` is now true - extract the duplicated `rejectRemoteSelectionFlags` from account.ts and artifacts.ts into src/cli/remote-selection-flag-rejection.ts - `environment show` / `environment rm` / `environment add` are untouched: there `--environment` and `--pairing-code` name the row to act on, not a route |
||
|
Neil
|
232d04f541 |
fix(dashboard): open remote sessions from every agent reveal path (#18403)
Three reveal paths called bare setActiveWorktree + activateTabAndFocusPane,
skipping setActiveView('terminal'), ensureWorktreeHasInitialTerminal and
resumeSleepingAgentSessionsForWorktree. A parked SSH workspace has no resident
tab until those run, so the reveal landed on a workspace with no terminal.
Route all three through the incumbent activateAndRevealWorkspace dispatcher
(which the sidebar and "Jump to workspace" already use, and which also handles
folder workspaces). The Activity row-click additionally early-returned when the
thread's tab was absent from tabsByWorktree/unifiedTabsByWorktree, which made a
cold-parked remote thread a silent no-op; residency is now probed after
activation, so a revived tab is focused and a genuinely retained thread still
activates its workspace instead of doing nothing.
Also stop asserting `exited` from an absence of local state: SshPtyProvider
reports no authoritative buffer snapshot and the relay has no snapshot RPC, so
a null preview snapshot for a remote pty is loss of contact. The preview and
the no-pty dialog branch now say the remote preview is unavailable rather than
claiming the pane closed. Adding the relay snapshot RPC stays out of scope --
it needs capability negotiation.
Fixes #16731
|
||
|
Neil
|
5a626dcdf4 |
refactor(git): share push-target resolution between local and the SSH relay (#18406)
`src/relay/git-handler-push-target.ts` and `src/main/git/remote.ts` carried
identical ~160-line copies of the resolver that decides which remote a plain
`git push` hits. Identical today is exactly when to share it: the cost of a
future divergence is pushing to the wrong remote, which retrying does not undo.
Move the resolver to src/shared/git-push-target-resolution.ts, parameterized on
a `(args) => Promise<{ stdout }>` runner — the only thing the two hosts actually
differ in — and delete both copies. The relay entry point keeps only the work
that is genuinely relay-side: re-validating an explicit target that arrived over
the wire and running `check-ref-format` on it.
No behavior change on either path, and nothing new or different is published, so
this engages no rule in remote-wire-compatibility. No git command changes.
src/relay/git-push-target-local-parity.test.ts scripts one repository's config
and requires `git.push` over the real relay dispatcher and the desktop's
`gitPush` to emit the same push argv, plus the argv each case should produce.
|
||
|
Neil
|
53adf5e2e6 |
fix(git): share one failed-command error-text reader between local and the SSH relay (#18398)
* fix(git): share one error-text reader between the local and relay branch-delete fallbacks The relay and the desktop each carried their own `getErrorText`, and they had drifted: the relay read `message` + `stderr` + `stdout`, the desktop only `message` + `stderr`. A `git branch -d` refusal arriving on `stdout` therefore routed the SSH removal through prune-and-retry while the local removal gave up and preserved the branch. Against a real binary the two agree, because Git prints the refusal through `error()` on every supported version — verified on 2.25.1, 2.38.1, 2.49.1 and 2.55.0, none of which put a byte of it on stdout. What the desktop copy actually missed is that Orca classifies errors it built itself, with the Git output on `.stdout`: `worktree remove`'s submodule retry attaches `git status --porcelain` that way on both paths. The stdout-reading form is also already the shared spelling — `isSubmoduleWorktreeRemovalRefusal` uses it for both hosts — so this converges on it rather than on the shorter one. Move the reader to src/shared/git-command-failure-text.ts and the predicate it feeds to src/shared/git-branch-delete-refusal.ts, and delete all three copies. The predicate carries both refusal wordings live in the supported range: Git through 2.40 says "checked out at", 2.43+ says "used by worktree at". The real-binary contract now pins that boundary: the refusal is recognized, it lands on stderr, and stdout stays empty on every Git in the matrix. * fix(test): consolidate the duplicate worktree import in the parity test |
||
|
Jinwoo Hong
|
a35451f5b9 |
fix(relay): stop self-closing the control socket on unknown messages (#18400)
* fix(relay): stop self-closing the control socket on unknown messages
The desktop control client tore its own relay control WebSocket down with
code 4401 "unknown control message" for any well-formed control frame it
did not recognize. handleMessage() funneled everything that was not
ping / conn-open / drain / a tracked request reply into
failProtocol('unknown control message'), which closes the socket and
orphans the origin.
Three real frames hit that branch:
- A relay reply that arrives after the desktop's 10s request deadline
already deleted the pending entry. Relay control operations run DB
transactions that can exceed 10s under load, so resolveMessage() finds
no waiter and returns false.
- A control-error carrying no reqId (or an unknown one), including the
relay's own 'unknown_control_message' reply to a host command it could
not route.
- A newer relay's opcode that this build predates.
Fleet telemetry shows ~15 of these closes per day across app versions
1.4.175..1.4.197, so it is version-agnostic. The self-close was also far
more costly than the message that caused it: the relay session dropped to
'orphaned' and answered the phone with HOST_OFFLINE (4404) for the orphan
grace window, then the desktop had to re-register through the director's
503 reconnect throttle, stretching a single stray frame into minutes of
mobile downtime.
Per docs/reference/remote-wire-compatibility.md Rule 2, an unknown but
well-formed control frame must be dropped, not treated as fatal. Log and
ignore it; malformed JSON, binary frames, and messages before activation
still close as protocol violations.
Adds unit tests for the unknown-opcode drop, the timed-out-reply drop,
and the preserved malformed-frame teardown.
* docs(relay): correct the ignore rationale, drop the Rule 2 misattribution
Rule 2 of remote-wire-compatibility governs the SENDER of a new terminal-
stream opcode and treats the receiver's silent drop as a hazard, not a
mandate. Reframe the comment around the actual justification: the decoder
convention of dropping unknown frames, the control channel's lack of an
opcode negotiation step, and the incident cost asymmetry.
|
||
|
Brennan BensonandMerge Sim
|
91c5a615c5 |
fix(settings): indent the Agent sleep "Sleep after" sub-setting (#18379)
"Sleep after" rendered flush with its parent toggle, unlike the Agent Dashboard and Chat UI sub-settings which sit inside the indented, left-bordered group. Reuse that same wrapper and drop the row's extra vertical padding so the block matches its siblings. Co-authored-by: Merge Sim <sim@local> |
||
|
Jinwoo Hong
|
16e2624578 | fix(terminal): flush xterm's parked renderer resize when releasing the pause latch (#18510) | ||
|
Jinjing
|
0f22e1e905 |
Fix table header transparency with opaque background (#18499)
Replace the translucent bg-muted/25 with an opaque color-mix blend (40% muted on background) to ensure scrolled rows don't show through the sticky header. Add test coverage for header styling and layout. |
||
|
Neil
|
4cc0b8de61 |
perf(hot-paths): delete allocation-only work in sort, explorer, monaco, rpc, snapshots (#18372)
* perf(hot-paths): delete allocation-only work in sort, explorer, monaco, rpc, snapshots * fix(perf): revert snapshot revision fast-path — same revision can carry a new session * perf(hot-paths): drop the unproven rpc buffer rewrite, dedupe the equality helpers - Revert the unix-socket chunk-carry change. Its comment claimed it avoided O(n^2) rescans, but chunks is reset to [remainder] every data event, so the join plus the tail byteLength is two passes where the old code did one; benchmarks showed no win. It also moved consumed-frame bookkeeping out of the closure, so a synchronous throw from the handler would re-dispatch frames. - project-host-compatibility: fold the two byte-identical array comparators into one generic arraysEqualByJson. - smart-attention: drop the leftover byTab.size === 0 branch that returned the same value as the line after it. |
||
|
Neil
|
f70f580627 |
perf(renderer): park hibernation and panel-watchdog work behind a hidden window (#18373)
* perf(renderer): stop five timers from ticking behind a hidden window * perf(renderer): park hibernation and panel-watchdog work behind a hidden window Narrowed from five timers to two, and made both correct: - Gate on getWindowParkVisible(), not raw document.visibilityState. macOS can wedge visibilityState at 'hidden' with no further visibilitychange, which would park these for the rest of the session. - Add a real becoming-visible pass via subscribeWindowParkVisibility, so resume does not wait out the remaining interval. Unsubscribed on stop. Dropped the other three gates: - terminal-delivery-watchdog: it is the recovery lane for the byte-drop bug the stale-visibility latch exists for; parking it costs a frozen terminal. - crash-diagnostics: the dashboard-popout surface is normally occluded, so it would sample once at startup and never again. - use-contextual-tour: attempts only increments past the gate, so a hidden window turned a self-clearing 20-attempt interval into a permanent one. Tests stub visibilityState and the stale latch; both watchdog cases are RED against the previous raw-visibilityState implementation. |
||
|
Neil
|
10b5ac8c90 |
perf(renderer): narrow vault and editor subscriptions off the every-write path (#18374)
* perf(renderer): narrow vault and editor subscriptions off the every-write path * fix(perf): revert EditorPanel narrowing — downstream hooks need the full openFiles list * perf(renderer): make the vault session-id cache resettable between tests Every production writer replaces agentStatusByPaneKey, but test fixtures commonly mutate it in place, which would keep serving the key cached for that identity. Fold the WeakMap into the existing reset hook. |
||
|
Neil
|
7b530f1eb5 |
fix(crash-reporting): record Orca-initiated tree kills so a killed renderer is decidable (#18367)
* fix(windows): refuse tree-kills of Orca's own Chromium pids and record the rest G2 is 20 field reports that share only a symptom. It is at least four fingerprints: ~15 Windows `reason=killed exitCode=1`, 3 POSIX SIGKILL under memory pressure (G4-oom), 2 duplicate reports of one macOS V8 Proxy Resolver SIGKILL, and 1 `0x80000003` install-dir ACL crash (G1; #17740 ships in v1.4.196 only, not 1.4.195). Nothing here claims to fix all of them. Two changes: 1. Behaviour. `classifyWindowsTreeKillTarget` returns `own` for any direct child of the main process — which our renderer, GPU and network-service utility all are — so PTY teardown could `taskkill /T /F` Orca's own UI (#10680). Both that classifier and `terminateWindowsProcessTree` now refuse any pid Electron is currently accounting for in `getAppMetrics()`. 2. Diagnosis. An Orca-issued kill and an external one are byte-identical in every field the crash report records today, so the cluster is undecidable. Every main-process force-kill choke point now records a durable `self_tree_kill` breadcrumb, and `process_gone` reports carry `selfInitiatedTreeKills` naming the pid and its offset from the death. A refused kill records `self_tree_kill_refused_own_chromium`, which is falsifiable: if it ever shows up in the field, we were the killer. * fix(crash-reporting): coalesce self-kill breadcrumbs and scope the discriminator Round-1 review remediation. Three blocking findings, all accepted. 1. Breadcrumb flood (accepted). recordSelfInitiatedTreeKill wrote an uncoalesced durable crumb from two routine teardown paths, and the reviewer reproduced 12 terminal closes x 3 process groups completely evicting the 30-slot ring — including this PR's own refusal crumb — plus a forced writeSync per killed group. It now uses the existing recordCoalescedDurableCrashBreadcrumb (5s window for pid-addressed taskkills, 60s for routine group/job teardown), so a burst costs one ring slot and one flush. The refusal crumb is coalesced per victim pid, so a retry loop cannot flood while a distinct pid always gets its own crumb. Regression test replays the reviewer's exact 12x3 reproduction and asserts the refusal crumb and a pre-existing gpu_process_crashed both survive. 2. Undifferentiated count (accepted). posix-process-group and win-pty-job are structurally incapable of reaching a Chromium process, and scope was absent from the persisted string. Scope is now in every entry (`<scope>/<site>/pid<N> +Nms`), and the count is split: selfInitiatedTreeKillCount now counts only pid-addressed taskkills — the kills that can land on a recycled pid that is now our renderer — with pty-scoped sweeps in selfInitiatedGroupKillCount. The list is renamed selfInitiatedKills because it carries both, and sorts pid-addressed kills first so truncation never drops the discriminating ones for teardown noise. The reviewer's repro (routine macOS terminal close + unrelated exit-133 crash) now yields selfInitiatedTreeKillCount undefined. 3. Recording gaps and a false comment (accepted). New admitSelfInitiatedTreeKill gate: it refuses own-Chromium pids and records the rest, and all three main-process taskkill families now go through it — terminateWindowsProcessTree plus codex-accounts/service.ts and claude-accounts (which keep their own spawn lifetimes). The false "single taskkill choke point" comment is gone. The runProcess choke point the investigation asked for is instrumented via a setProcessTreeKillObserver seam in src/shared/child-process — shared code runs in the CLI and relay so it cannot import the main breadcrumb store — registered in main preflight. The codex app-server POSIX group teardowns and the claude POSIX branch record too. The module doc no longer claims absence is discriminating: it enumerates what is instrumented and names the direct process.kill(-pid) sites that are not. Non-blocking, also fixed: - Breadcrumb calls moved out of the try blocks whose catch is the ESRCH contract (posix-pty-process-groups, codex teardown, claude POSIX), so a throw from the diagnostic path can never be reported as a failed kill. - Detail truncation now bounds the first entry too, matching its comment. - own-chromium-tree-kill-refusal.test.ts renamed to own-chromium-tree-kill-guard.test.ts, colocated with the module it tests. Not changed, with reasons: - Date.now() vs performance.now(): kept. Offsets are computed against goneAt = Date.now() in process-gone-recorder; a monotonic clock here would make the offsets meaningless. The reviewer verified this and agreed it is not a defect. - app.getAppMetrics() per force-kill remains unbenchmarked. It reads in-process browser state rather than enumerating the OS process table, and a TTL cache would let a recycled pid slip past the refusal, so it stays uncached. - The ~15 remaining direct process.kill(-pid) sites (browser routes, notebooks, automation prechecks, ephemeral VM recipes) are not instrumented. Rather than claim coverage this PR does not have, the module doc names them. claude-command-process.ts crossed the 300-line cap, so terminateClaudeProcess moved to claude-login-process-termination.ts. No max-lines suppression added. * fix(crash-reporting): scope the self-kill guard to its real host topology Round-2 review findings on the own-Chromium tree-kill guard. BLOCKING 1 — "the own-Chromium refusal is a no-op in the process that issues the pty-descendant-sweep taskkill". Correct on the mechanism, wrong on the consequence; REBUTTED in part and documented in full. Confirmed: the only non-test `setAppEnvironment` installs are main-process-preflight.ts:177 (Electron) and orcad-entry.ts:84 (Node, whose `getAppMetrics()` is `[]`); daemon-init-fresh-import.ts is a test harness. So in the standalone daemon `readOrcaChromiumProcessPids()` is empty and `admitSelfInitiatedTreeKill` always admits. But that is not a live hazard. `killWithDescendantSweep` reaches `terminateWindowsProcessTree` only when `verifyWindowsTreeKillTarget` returns `own`, and that walks ancestry back to `deps.ownerPid ?? process.pid` — the KILLING process's pid. In the daemon that is the daemon's pid. Orca's Chromium processes are children of Electron main, a sibling of the daemon, so their chain never reaches it: hop 0 lands on main, and within MAX_ANCESTOR_HOPS the walk dead-ends and returns `foreign`. The reviewer's probe passes `ownerPid: 1000` with the renderer as a direct child of 1000 — that is the Electron-main topology, where the AppEnvironment IS installed and the guard DOES fire, not the daemon's. On an orcad/SSH host there is no Chromium on the box at all, so `[]` is accurate rather than degraded. Locked in as tests rather than prose (own-chromium-tree-kill-guard.test.ts): a renderer classifies `foreign` from a daemon ownerPid with an empty pid set, and `own` from main's ownerPid with an empty set — the falsifiable pair showing the pid set is load-bearing in main and nowhere else. Documented the host coverage in orca-chromium-process-pids.ts and own-chromium-tree-kill-guard.ts. One genuine hole the finding exposes: `signalProcessTree`'s `taskkillTree` is a fourth pid-addressed taskkill family (non-blocking item 2), it runs in the daemon/relay/CLI where the guard cannot run, and it guarded only on `!child.pid`. Reusing the predicate the codex login teardown already uses, the win32 branch now refuses a reaped child and falls back to `killRoot` — the same shape as the existing `!child.pid` branch. That closes the reaped-then-recycled pid path in every host. BLOCKING 2 — module doc overstates coverage. Rewritten: the ring is per-process and its only reader lives in Electron main, so a count on a `render-process-gone` covers main-issued kills only. Sites are now split into main-only, main-and- other-hosts (runProcess choke point, POSIX PTY group sweep, Windows Job Object — which record into a ring nothing reads when they run in the daemon or relay), and never-instrumented, with the note that a daemon/relay omission is a diagnostics gap, not a missed suspect, per the topology argument above. BLOCKING 3 — the three out-of-main instrumentation sites were untested. Added regression coverage: the runProcess seam on both branches plus the reaped-child refusal (process-tree-termination.test.ts), the group sweep recording only groups it actually signalled and skipping an ESRCH group (posix-pty-process-groups.test.ts), and the Job Object recording the shell pid only on `terminated` (windows-pty-job.test.ts). Verified red: reverting the three production files to origin/main fails 7 of the new tests. BLOCKING 4 — the Windows evidence validates a single-process model. Accepted. The main2.js arms exercise `pty-descendant-sweep` inside one Electron process; that models the in-process/degraded daemon and the local PTY provider, not the standalone daemon. Arm C's "the 449351d6 shape is not producible with the guard" holds for main-issued kills only. In the daemon the shape is blocked one layer earlier, by the ancestry check, which the arms do not exercise. NON-BLOCKING taken: `recordSelfInitiatedTreeKill` moved outside the native `terminateJob` try in windows-pty-job.ts, so a diagnostics throw can no longer downgrade a real termination to `unavailable` and escalate callers to a broader kill; covered by a test. The "all three families" parenthetical is gone with the doc rewrite. `pnpm build:relay` run: exit 0, all seven targets built. NON-BLOCKING declined: codex-accounts/service.ts records before the spawn because a refusal must prevent the spawn — the crumb means "we were about to kill this pid", which is the artifact worth having; the existing comment already says so. `app.getAppMetrics()` perf is unbenchmarked and unchanged by this round. Verification: pnpm tc clean; oxlint clean on touched paths; check:code-quality:changed 0 new findings; oxfmt applied. 730 tests pass across shared/child-process, main/crash-reporting, main/pty, main/windows and the guard and descendant-sweep suites. The 4 failures in providers/git/codex-integration reproduce on HEAD without these changes. * fix(crash-reporting): keep the reaped-pid skip from flipping the termination barrier The win32 hasExited short-circuit correctly avoids taskkill on a pid Windows may have reissued, but it resolved `true` — verified tree termination. A taskkill against a reaped pid already resolved `false`, and run-process turns `true` into barrierTerminationVerified + terminationReporter.report(), which releases the git admission grant on root exit instead of on `close`. That admits the next git command while a descendant holding the inherited pipes is still writing the repo. Resolve `false` so the skip changes only which process we refuse to signal, not what the barrier claims. |
||
|
Neil
|
3a32e084dd |
perf(renderer): index diff comments, skip no-op hydration, drop duplicate normalizes (#18375)
* perf(renderer): index diff comments, skip no-op hydration, drop duplicate normalizes * fix(perf): keep tree-path stability hook render-pure for react-doctor * fix(perf): publish the returned array from the tree-path stability hook The ref was written with the raw input but read in render to pick the return value, so it trailed one commit and a wave of content-equal arrays flipped identity every render — re-firing the uncancellable full-tree git check-ignore it exists to prevent. Publish `stable` instead, keyed on `[stable]`. Also drops the hydrateOverrides no-op skip: notifyChange is not a bare wakeup (it drives getPanesNeedingOverrideFit -> safeFit and the remote viewport re-claim), and the branch never fires in production anyway. |
||
|
Neil
|
860ee73a11 |
fix(git): parse sparse and cquoted paths on the SSH relay (#18389)
The relay carried its own copies of the worktree-list and unmerged-entry porcelain parsers, and both had drifted from the desktop originals: the relay copy had no `sparse` branch, so SSH sparse checkouts were never marked, and it never C-quote-decoded a conflict path, so a conflicted file with a space or non-ASCII byte was published under its raw quoted name and probed as missing. Move both parsers into src/shared and delete the relay copies, so there is one implementation each. Type the relay's worktree-list plumbing on GitWorktreeInfo instead of Record<string, unknown> so a field-copying step can no longer silently drop a newly parsed field. `isSparse` is a new optional field on the git.listWorktrees result (remote-wire-compatibility Rule 1); Git <2.28 omits the porcelain line and the field stays absent. No new git subcommand or option. Closes #18280 |
||
|
Neil
|
a9f2fbb684 | chore(workspaces): drop the dead workspaceCleanup:hasKillableLocalProcesses IPC (#18386) | ||
|
Neil
|
b8da193b7a | fix(ssh): route the remaining expired-lease readers through the reattach predicate (#18378) | ||
|
Neil
|
d05dd8ef50 |
fix(source-control): route hosted reviews by resolved execution host (#18382)
`ForgeProvider.createReview(repoPath, input, connectionId, options)` and the `connectionId` on `ForgeProviderRepositoryContext` carried the same collapse the five prior migrations closed: `string | null` spells "genuinely local", "runtime host" and "could not resolve" with one value. Because it was decided two layers up -- `repo.connectionId ?? null` at the `hostedReview:*` IPC handlers and in `RuntimeHostedReviewCommands` -- a row naming its owner only as `executionHostId: ssh:<target>` ran the whole review path against this machine's copy of a remote path (#11163): `git rev-parse`, `git status`, the base-on-remote ref probe, the upstream divergence read, and `gh`/`glab` with no host flags. Replace it with a required `ExecutionHostId` threaded from the decision point through the contract, routed by #18296's `resolveGitRouteForHost`. The parameter is removed rather than added beside, so all five implementations -- GitLab, GitHub, Bitbucket, Azure DevOps, Gitea -- and every caller became a compile error. None of these families carries `@ts-nocheck`, so unlike #18325 that guarantee is real here; `orca-runtime-file-commands.ts` does, but it only constructs `RuntimeHostedReviewCommands` with unchanged deps. Also fixed at the sites: - The branch cache scoped entries on `connectionId ?? ''`, so two rows at one path on different hosts shared one cached review, one backoff deadline and one invalidation. Keyed on the resolved host now, as #18377 did for its probe key. - `hostedReview:create` resolved shared symlink paths and normalized worktree paths off the raw field, so an `executionHostId`-only SSH row read `orca.yaml` and `resolve()`d a remote POSIX path on the client. Those ask the file-holder question -- `getRepoSshConnectionId` -- not the dialable one. - An SSH host with no provider now refuses inside the git-state layer instead of reaching the local branch, keeping "remote and unreachable" distinct from "local" (docs/reference/ssh-execution-boundary.md). `runtime:` is a routing mistake inside `hostedReviewSshConnectionId` -- that environment's server runs its own git, and the SSH target on its repo row is nested in that server's namespace, so dialing it here reaches a same-named box of ours. But store-backed callers ask `getRepoHostedReviewExecutionHostId` first, which is "what may this client dial" and answers `local` for a `runtime:` row. That is deliberate and matches #18377: the runtime registration controller only adopts a `runtime:` stamp onto a row with no `connectionId` (`runtimeRepoMatchesExecutionHost` refuses to match an SSH row), so the checkout really is in this process and refusing would regress a runtime server creating reviews for its own rows. No wire change. `connectionId` on `CreateHostedReviewArgs`, `CreateStackedHostedReviewArgs` and `HostedReviewCreationEligibilityArgs` in src/shared/hosted-review.ts is untouched -- every host already ignores it in favor of the repo row, and removing it from the request types would only churn the schema older clients still populate. The main-side eligibility input `Omit`s it so nothing on this side can read the ambiguous field again. |
||
|
Jinwoo Hong
|
573537ecd4 |
feat(cli): make terminal close the canonical workspace teardown (#18073)
* fix(runtime): recover stale session owners and await retirement * fix(runtime): preserve session hydration and smoke compatibility * test(runtime): cover empty and unindexed session owners * feat(cli): make terminal close the canonical workspace teardown * fix(preload): align ssh termination result type * test(runtime): assert folder hydration owner * fix(runtime): fence legacy terminal stop by worktree host * fix(preload): reconcile ssh result import with main * fix(runtime): keep same-id sibling hosts out of workspace close The stale-owner fallback in the session controller re-routed any worktree whose catalog partition had no tabs to whichever other partition held tabs. Only `runtime:` environment ids rotate across relay restarts; `repoId::path` legitimately repeats across hosts, so an SSH workspace close could retire the local copy's tabs and resume records, or flip owners mid-close and strand the SSH PTY. Restrict the fallback to runtime hosts, and pin the session partition once per workspace close so record clearing targets the partition that owned the tabs. * test(runtime): give the cross-host close fixture a real resume record * fix(preload): take main's ssh-bridge import order so the merge stays duplicate-free |