8001 Commits

Include renames

Author	SHA1	Message	Date
Jinjing	67999dcaae	Keep attention glyph knockout white when row is selected (#18679) ... * Simplify palette attention glyph styling Remove visual styling from the container so the glyph appears as a lightweight overlay on the row icon, not a selection bubble. * Keep attention glyph knockout white when row is selected The glyph now uses a white background (bg-popover) with a ring to create a visual knockout effect that separates it from the icon. This prevents the glyph from inheriting the row selection styling, ensuring it stays visible and distinct regardless of selection state. * Correct attention glyph knockout color description to popover-colored	2026-09-04 10:05:45 -07:00
Neil	0d2375a7ff	fix(remote): stop a disclosure list latching the mirror completeness gate (#18619) ... * fix(remote): stop a disclosure list latching the mirror completeness gate `hostScope.omittedHostIds` was doing two jobs with opposite requirements. As disclosure it must over-name: `omitted-host-scope-selectors.ts` deliberately keeps ids for servers that are no longer paired so a caller can still see the gap, and `docs/reference/ssh-execution-boundary.md` requires a listing to name what it did not cover. As a completeness gate it must name only coverage that was owed and not delivered, or it latches. It latched. `workspaceSessionsByHostId` keeps a partition for every runtime a machine has ever paired with and nothing prunes it, and a mirrored `remote:` row names its peer too — so any client that has ever paired outward publishes a permanently non-empty `omittedHostIds`. `probeHostLiveTerminals` read that as `unverifiable`, `markHostSessionMirrorHydrated` never fired, and panes parked on `parkUntilHostSessionMirrorHydrates` never drained. `hostScopeCensusIsComplete` gives the gate its own answer and leaves the disclosure list alone. A `runtime:` host is never owed coverage by the runtime answering: a paired runtime is a peer with its own control plane reached with `--environment`, and there is no paired-runtime PTY provider for this runtime to have queried. Two other branches stay load-bearing — an absent scope is a host too old to claim one, and a listing that covered no host proves nothing. No wire change: the host publishes byte-identical content and only the client's reading moves, so this reaches the reporter by updating their client alone rather than waiting for their remote. That also avoids a new field's fallback rule, where "absent means complete" would recreate the bug with the polarity flipped. `queried-host-kinds.test.ts` pins the invariant the predicate rests on at its source, because the consolidation moving the SSH path onto orcad is the change most likely to introduce a runtime-backed PTY provider and quietly invalidate it. Fixes #18595 * test(remote): pin the orphan-recovery host-scope gate and narrow the invariant claim The readiness review found the second gate unpinned: reverting `web-session-terminal-orphan-recovery-inventory.ts` alone to the pre-PR expression left the whole renderer suite green, because every existing fixture passes `omittedHostIds: []`. The commit claimed two gates and proved one. Four cases now drive `resolveTerminalOrphanInventory` through a non-empty scope. Reverting that gate alone fails the peer-runtime case. Note the absent-scope case deletes the key rather than passing `undefined`, because `listResult` substitutes its default for `undefined` — routing through the fixture there silently tests the default instead. `queried-host-kinds.test.ts` also claimed more than it caught: a runtime-backed transport registered under an SSH connection id reports as `ssh:` and passes, which is the shape the orcad consolidation is expected to take. It pins the spelling this function emits, which is what the gate keys on, and now says so. * fix(remote): require a legible covered host before believing a census CodeRabbit found a real asymmetry: the predicate refused an omitted host id it could not parse, but accepted an unparseable *covered* id as proof of coverage. `isTerminalListResult` validates only that `hostIds` is an array, so `{hostIds: ['runtime:'], omittedHostIds: ['runtime:env-7']}` was `unverifiable` before this PR and would have become `complete` after it. Taken as "at least one legible covered host" rather than the suggested "every id parses". A host that later gains a kind this client cannot parse would otherwise report an incomplete census forever — which is this bug in a new coat, and the failure mode the predicate exists to prevent. The check exposed four tests publishing `hostIds: ['remote-runtime']`, a bare environment id that `parseExecutionHostId` rejects. No host emits that: a runtime answering `terminal.list` names the execution hosts it covered, which is `local` — verified against a live paired runtime. Those fixtures are corrected to the shape the wire actually carries, which is why the assertions move.	2026-09-04 06:09:07 -07:00
Neil	14e4031948	test(pty): make the F24 patch pins catch the regressions they name (#18660) ... Two of the pins added in #18635 did not discriminate. Found by review of the merged change; both are test-only defects, the fix itself is unaffected. `resolves the ConPTY DLL before it claims the close` searched the whole patch for `HANDLE hLibrary = LoadConptyDll(info, useConptyDll);`. That line occurs twice -- PtyConnect's copy comes first -- so indexOf always matched PtyConnect, and the ordering assertion held no matter where PtyKill resolved the DLL. Verified by simulation: moving PtyKill's resolve back below the claim left the suite green. `reaches hShell only under the null check` used a marker as a slice END bound without checking it existed. If that marker vanished the slice ran to the end of the patch, the stray-line filter found nothing, and the test passed silently. Both now anchor inside the PtyKill hunk only, located by its header's function context rather than line numbers. `indexIn` throws on a missing marker instead of returning -1, so a marker that moves fails the assertion that depends on it rather than making it vacuous. Adds the pin that was missing entirely: PtyKill's half of the two-sided baton free. Without it a self-exit followed by kill() -- the ordinary pane close -- leaks one baton and one entry in the vector get_pty_baton scans linearly. Mutation-tested rather than only revert-tested, because wholesale reverting the patch is what hid this: it fails every assertion for the trivial reason that nothing matches. Simulating each specific regression instead: - move PtyKill's DLL resolve below the claim -> 1 failed (was: 0) - drop PtyKill's baton free, line-count-neutral -> 2 failed (was: 0) Wholesale revert still fails all 9. Refs F24.	2026-09-04 06:08:33 -07:00
Neil	f7e3af254a	fix(pty): close the pseudoconsole and dispose the conout worker on Windows self-exit (F24) (#18635) ... * fix(pty): close the pseudoconsole when a Windows shell exits by itself `ClosePseudoConsole` is the only thing that reaps a ConPTY's console host. node-pty calls it from one place, `PtyKill`, which starts by looking the baton up by id -- and the exit watcher in `SetupExitCallback` erased that baton the moment the shell died. So on the self-exit path (typing `exit`, how panes usually close) the lookup missed, `PtyKill` did nothing at all, and the pseudoconsole was never closed. The baton now survives until BOTH the shell has exited and `kill()` has run; whichever arrives second frees it. `PtyKill` copies `hpc` out under the lock and closes it afterwards, guards `TerminateProcess` on a shell handle the watcher may already have closed, and duplicates that handle rather than reordering, so upstream's close-then-terminate sequence is unchanged. Measured on Windows 11, 20 self-exit cycles driven exactly as Orca drives them (`onExit -> destroy()`), handles bucketed by NT object type: relay spawn (no useConptyDll) 225 -> 285 (+1 Process +2 File/term) after 219 -> 219 FLAT desktop spawn (useConptyDll) 239 -> 439 (+1 Process +2 Thread +5 File/term) after 235 -> 395 (+2 Thread +4 File/term) The desktop residue is a separate defect in the `useConptyDll` branch of `WindowsPtyAgent.kill()`, which disposes the conout worker only from an `_outSocket.on('data')` handler -- and no data arrives after the shell has gone. Fixing that line as well takes the desktop to 222 -> 222 FLAT, but it lives in the `kill()` hunk owned by F23, so it is left to that change. Refs F24. * fix(pty): dispose the conout worker when a Windows shell exits by itself Second, independent defect on the same self-exit path, and the larger half of the desktop's leak. The `useConptyDll` branch of `WindowsPtyAgent.kill()` disposed the conout worker only from an `_outSocket.on('data')` handler -- and once the shell has gone no more data ever arrives, so the worker was never disposed. The non-DLL branch three lines above already disposed unconditionally, which is why only the desktop (the only spawner that sets `useConptyDll`) hit it. Measured on Windows 11, 20 cycles, handles bucketed by NT object type, totals: self-exit, relay spawn 225 -> 285 now 219 -> 219 FLAT self-exit, desktop spawn 239 -> 439 now 222 -> 222 FLAT explicit kill, relay spawn 225 -> 285 now 219 -> 219 FLAT explicit kill, desktop spawn 235 -> 395 now 219 -> 219 FLAT Neither fix alone is enough on the desktop: the pseudoconsole close is worth +1 Process +1 File per terminal, this dispose +2 Thread +4 File. The relay asset (config/relay-assets/node-pty-1.1.0-windows-pty-teardown-patch.cjs) deliberately gets no counterpart: the relay takes the non-DLL branch, where the dispose is already unconditional. Its reconstruction table needs the new hunk though, or un-applying the desktop hunks no longer yields published node-pty. Taken over from F23 at win-relay-qa's request after they verified that the desktop never executes the non-DLL branch F23 was scoped around. Refs F24. * fix(pty): harden PtyKill against a failed handle duplication and a missing DLL Both from review of #18635. DuplicateHandle's result was dropped. On the live explicit-kill path a failed duplication left hShellDup null, which the guard below could not tell apart from the self-exit case, so TerminateProcess was skipped and the shell kept running after its pane closed -- a worse outcome than the handle leak this patch exists to fix. The failure now terminates through handle->hShell under the lock, where it is valid and where TerminateProcess does not block. The only cost is that the rare path kills before the console closes instead of after. LoadConptyDll is now resolved BEFORE any baton state is touched, matching what PtyConnect already does for the same reason. It throws when conpty.dll is missing, and a throw after consoleClosed was set would strand the pseudoconsole permanently: the retry finds the work claimed and does nothing. Also corrects three comments the earlier commits made stale: - the ptyJobMutex note still said PtyKill reads the table unlocked - PtyListJobProcessIds said the baton is gone once the shell exits; it now outlives the shell, and the nulled hJob is what makes the answer null - windows-pty-job.ts said node-pty drops its handle record on exit Re-measured on Windows 11 with the rebuilt binary, 20 cycles, all four paths still flat: self-exit relay 219->219, self-exit desktop 222->222, explicit-kill relay 219->219, explicit-kill desktop 219->219. Both explicit-kill runs report 22/22 shells exited, so the kill still lands. Refs F24.	2026-09-04 05:32:19 -07:00
Neil	886fcf083f	fix(runtime): let a scoped worktree listing report the host it could not cover (#18645) ... * fix(runtime): let a scoped worktree listing report the host it could not cover `orca worktree list --repo <id>` passed `[]` as `knownHostIds`, so a scoped listing could never report a gap — for any host kind, reachable or not. With zero matched rows both scope lists are empty by construction, and the answer is `{hostIds: [], omittedHostIds: []}`: byte-identical to a repo that genuinely has no worktrees. docs/reference/ssh-execution-boundary.md forbids a listing from implying exactly that. Measured on one runtime with one refusing SSH host, in the same second: unscoped totalCount 0 omittedHostIds ["local","ssh:<target>"] (+ --host selectors) scoped totalCount 0 hostScope {"hostIds":[],"omittedHostIds":[]} The same runtime reports nine omitted hosts unscoped and zero scoped against a live profile, so this is not a subtle inconsistency: it is one runtime giving two contradictory answers about its own coverage. A scoped listing now names the one host the caller asked about. That costs nothing when rows come back — the host lands in `covered`, so it is never reported omitted — and is the whole answer when they do not. Hosts the caller scoped out are still never named, which a test pins, because naming them all is the obvious over-correction. * test(runtime): pin the scoped host derivation for local and executionHostId repos Review flagged that the host-scope cases only covered a connectionId repo. getRepoExecutionHostId reads two spellings, and a scoped listing naming the wrong host would be worse than naming none, so both are pinned. Both fail with the fix reverted.	2026-09-04 05:06:44 -07:00
Neil	637dc30a32	fix(relay): observe Windows PTY child processes instead of answering false (#18591) ... * fix(relay): observe Windows PTY child processes instead of answering false `processHasChildren` returned a hardcoded `false` on Windows, and a hardcoded negative is indistinguishable from a measurement. Every close guard reads it as "nothing is running in this pane", so an SSH-to-Windows tab running a build closed with no prompt. Measured on a real Windows SSH host: a live `PING.EXE` under the pane's `cmd.exe` still reported `hasChildProcesses: false`, while the identical harness on Linux reported `sleep` / `true`. Windows has no `ps`, but it does have a process table, and the pane walk over it already existed for the foreground reader. The answer now comes from `queryWindowsPaneProcessInventory`; a table it could not read reports `unverifiable` rather than a fabricated negative. `hasChildProcesses` is a boolean, which cannot hold the third answer, and it is read both as "busy, do not close" and as "the agent took the PTY, safe to type into" — so no single mapping of `unverifiable` is safe for both. The verdict moves to a new optional `childProcessEvidence` member that the close paths read; the boolean keeps its exact meaning for every client that cannot. Cost: `pty.inspectProcess` is the polled path and a relay host has no `@vscode/windows-process-tree`, so its table read falls back to the 1.36s CIM scan. Polling that would reinstate the fork storm the shared table exists to prevent, so only a caller whose answer decides something asks for the scan. * fix(runtime): forward scanChildProcesses through the environment inspection RPC `guardRunningTerminalClose` asks the host to pay for a real child-process read, but the environment path dropped the option before it reached the wire: the renderer sent only `expectedIncarnationId`, and the RPC schema — the shared `TerminalHandle` — silently stripped anything else. A host routing that pane through an SSH relay then declined to scan and answered `unverifiable`, which `inspectionReportsRunningWork` reads as running work. The result was a close confirmation on an idle pane, which is the nag this PR exists to avoid. Forwarded through all four layers: renderer payload, RPC schema, method handler, and the runtime/controller signatures. The schema is a dedicated extension rather than a field on `TerminalHandle`, so `clearBuffer`/`agentStatus`/`isRunningAgent` keep refusing an option they have no use for. The silent strip is not itself the defect — it is what makes a new optional member safe to send to an old host, per docs/reference/remote-wire-compatibility.md. The defect was the schema and its caller drifting inside one version, so the tests pin the registered method rather than the schema alone: pointing it back at `TerminalHandle` compiles, parses, and drops the option. Found by review on #18591. * fix(terminal): teach the shared running-work probe the third child-process answer Rebasing onto main landed `probePtyRunningWork`, which is a better home for this than the close guard: it already speaks `live` / `unverifiable` / `exited`, and it exists so the tab-close and window-close guards cannot drift. The child-process verdict belongs there, not in a parallel predicate beside it. So the mapping moves into the probe and `inspectionReportsRunningWork` is deleted rather than kept alongside. The probe now asks for the scan, and a host that could not observe the pane reports `unverifiable` instead of collapsing onto `exited` -- which is what `hasChildProcesses: false` meant on every Windows relay. The pane-close path is routed through the same probe for the same reason; it was the third caller asking this question through a direct inspect of its own.	2026-09-04 02:11:09 -07:00
Neil	766b5b153c	fix(relay): release the ConPTY conin handle after teardown, not before it (#18601) ... A Windows SSH relay leaked one Windows File handle per terminal, for the life of the relay process, across reconnects. node-pty's `kill()` flips `readable` on the conin and conout sockets and destroys neither; `_cleanUpProcess` destroys `_outSocket`, so only conin is stranded, and it wraps a real named-pipe handle from `fs.openSync(term.conin, 'w')`. The obvious fix -- and the one config/patches/node-pty@1.1.0.patch ships for the desktop -- releases it at the top of the branch, before `_getConsoleProcessList()` forks and before the native kill. Measured against a real Windows SSH host, that is three times worse than leaving the leak alone: teardown aborts partway, the forked console-list agent is never reaped, and both pipe handles stay alive. Releasing it at the end of the branch instead is flat. 20 spawn/kill cycles, handles bucketed by NT object type, identical numbers standalone and through a real relay: published node-pty File +1/terminal, Process flat desktop patch placement File +2/terminal, Process +1/terminal released last (this) File flat, Process flat `windowsTerminal.js` takes the desktop's error-listener hunks verbatim. The conin listener is not what fixes the leak -- adding it alone changed nothing -- but it is what keeps a pipe error retiring one terminal instead of the host. The desktop patch has the early placement and therefore the regression, measured against its exact installed tree. Correcting it there needs its own verification on a Windows desktop build, so the trees diverge on this one hunk deliberately and a test pins that so a future patch sync cannot copy the bug back.	2026-09-04 01:56:42 -07:00
Neil	fb69f00b65	fix(hosts): resolve a folder workspace's SSH host from the repo's host, not its raw connectionId (#18598) ... * fix(hosts): resolve a folder workspace's SSH host from the repo's host, not its raw connectionId `resolveFolderWorkspaceHost` inferred a workspace's host by reading `repo.connectionId` directly. SSH ownership has two spellings on a repo row, and a row carrying only `executionHostId: 'ssh:<target>'` has no `connectionId` to read — so it counted as a local repo and the workspace resolved `{ kind: 'local' }`. That is an execute-here answer for a workspace whose files are on an SSH host, the #11163 class, and it fires on a well-formed row. Resolve the host first, then read the target off it. Every other row keeps its existing contribution, including a `runtime:` row's nested SSH target: that target is not this client's to dial, but narrowing it here would be a second behaviour change riding on this one. The runtime branch above still answers `local`, and now says so — `FolderWorkspaceHost` has no runtime variant, and widening the type is its own change, not an oversight to be silently corrected. Three smaller items that stand on their own: - `resolveWorktreeExecutionHost` gains a `malformed` reason distinct from `unknown`. `unknown` (nothing carries the id) is a verdict the launch path may legitimately dispose of as a plain local folder; `malformed` (the row named a host that cannot be parsed) must fail closed. One word for two situations is the shape that lost the distinction in #18006. The strict read is private to that module: `getRepoExecutionHostId` stays the answer everywhere else, since its fall-through to `local` is harmless for the grouping, label and index callers that are nearly all of its ~340 call sites. - `readAllWorktreeMetaForRepo` / `readWorktreeMetaForRepo` replace four open-coded copies of the same host-qualified read (the F7/F8 lockstep shape). - `getExecutionHostLabel` answers 'Unknown host' rather than 'All hosts' for an id that names no host. Showing one unroutable row as though it were on every host is wrong on its own terms. Plain English like every other label in that module, none of which resolve through the renderer's i18n catalog. * fix(hosts): resolve the host in candidate selection too, not just in resolution The first pass fixed how a repo row is classified once it reaches `resolveFolderWorkspaceHost`. The candidate filter decides which rows reach it at all, and it read `repo.connectionId` raw as well — so an SSH-only row outside the project-group subtree was dropped before the new logic could see it, and the execute-here bug survived for the population the fix was for, via a different path. Found in review by CodeRabbit. Three repo-row reads had the same root cause, not one: - the scope-connection filter, comparing a path repo's raw field against the workspace/group connection; - the group-connection set, built from group repos' raw fields; - that set's membership test against path repos' raw fields. The last two are one comparison with the mismatch on either side, so resolving only the path side would have reintroduced it from the other direction. All three, plus the resolution loop, now go through one `getRepoScopeConnectionId` helper. Non-SSH hosts still fall back to the raw field, so a `runtime:` row keeps contributing its nested target exactly as before. The new tests use a repo matched only by path, outside the subtree — the population every existing test missed, which is why four passing revert-tests did not catch this. One of them is labelled as pinning the resolver rather than the filter: under the old raw read both rows came back connectionless and matched each other by accident, so it survives a filter revert and must not be counted as coverage for it.	2026-09-04 01:34:47 -07:00
Neil	2ee507d744	fix(ssh): move Windows file writes off PowerShell 5.1 stdin onto sftp (#18596) ... * fix(ssh): move Windows file writes off PowerShell 5.1 stdin onto sftp #16432 was fixed by chunking writes to 32KB, on the belief that a `DefaultShell=cmd.exe` host caps one stdin at roughly 50KB. Re-measured on Windows 11 26200.9168 / OpenSSH_for_Windows_10.0p2, that premise is wrong in both directions, and the chunking does not fix the hang. The real constraint: a read on Windows PowerShell 5.1's redirected-stdin handle over a non-pty ssh exec can die permanently when it finds the stream momentarily empty, taking both the remaining data and the EOF with it. It is probabilistic per such read — not a size threshold, and not certain on the first one. Measured by swapping the copy loop for a counting reader: a 1.5s gap before any byte -> 0 bytes received, 6 of 6 1 byte, 1.5s gap, then 32767 -> exactly 1 byte 32768, 1.5s gap, then 32768 -> exactly 32768 a continuous 2MB -> 167936 / 270336 / 372736 Those three 2MB figures are one payload run three times under the same conditions, which is what rules out a threshold. Independently reproduced by a second harness where one 1.9MB counted read completed through 39 reads and another died after 11. A payload that fits one burst usually presents only one read that can find the stream empty, which is why 32KB mostly works — and it still failed 15 times in 120 under load, and 1 in 40 on a quiet host. Neither rate survives the 62 execs a 1.9MB file needs: even 2.5% compounds to about four uploads in five failing. No chunk size helps, because the defect is per blocking read, not per byte. Three controls on the same host, same DefaultShell, rule out both a size limit and cmd.exe: `findstr` took 2,016,000 bytes through one exec's stdin, sftp moved 1.9MB 5/5, and PowerShell 7 took 2MB in one exec. Windows writes now go over the sftp subsystem, whose batch script is read by the *local* client, so no remote process reads a pipe at all. PowerShell 7 is the fallback where sftp is unavailable, and Windows PowerShell 5.1 is last, still bounded, and now reports the host limitation and its remedy instead of a bare timeout. Measured on the same host, through this code: 1.9MB x20 all succeeded, hash-verified, median 315ms, against 0/6 before. 32KB x120 zero hangs, against 15/120. Also: - Stage under a unique name per attempt. An abandoned write leaves a remote process that may still hold the staging file, and losing contact is not evidence it died (docs/reference/ssh-execution-boundary.md), so a retry must not reuse a name its predecessor may own. Sweep is best-effort and never treated as proof of anything. - Create upload directories over sftp too; the JSON mkdir batch rode the same defective read. - Cover makeWindowsWriteFileCommand and the publish command against the 8000-char budget, which F11 flagged as untested. * fix(ssh): replace the staged Windows write atomically, and translate ssh -l Three review findings, all on the failure path that the success-path measurements say nothing about. CodeRabbit, Critical: the publish deleted the destination before moving the staged file onto it, so a failed move destroyed the user's existing file and left a window where a reader saw no file at all. That is worse than the truncated partial the staging discipline exists to prevent. Now File.Replace (Win32 ReplaceFile, atomic), falling back to a plain Move only when the destination is absent — and that race is safe, because a destination appearing in between makes Move throw with the staged file preserved. The exclusive branch already had it right: Move throwing on an existing destination is the exclusive contract. Append stays non-atomic and now says why. buildSshArgs can emit '-l <username>' for a config alias no Host block claims, and the translator threw on it. isSftpUnavailableError read that throw as 'this host cannot do sftp', so those hosts fell back to the defective PowerShell 5.1 path and had the refusal cached against them for 30 minutes, silently. '-l' now maps to '-o User=', with a test for the exact argument shape buildSshArgs produces in that case. CodeRabbit, minor: two assertions passed on an absent observation — an unmatched regex yields '' and every() is true of an empty list. Both now assert the positive form first, and the same audit was applied to the three other some()/every() assertions in the file. The temp-file test now asserts mode 0600 rather than only that the file is cleaned up. * fix(ssh): keep a path sftp cannot spell from becoming a verdict about the host Audit of isSftpUnavailableError, prompted by the '-l' gap having the same shape: a per-operation condition being written into a per-host cache that holds for 30 minutes. It had a second instance, and this one was mine. UnsupportedSftpPathError was classified as 'this host cannot do sftp', but it is thrown for a UNC or relative destination and for any path sftp's batch lexer cannot quote -- including a *local* filename containing a newline, which POSIX clients allow. One such file would have routed every later Windows write to that host down the defective PowerShell 5.1 path for the rest of the cache window. The host verdict is now only the errors that really are host-scoped: a refused subsystem, a client that will not start, and an untranslatable argument list. A path refusal falls back for that one write and leaves the cache alone, in both the file-write and directory-creation paths. Revert-tested. Removing the operation-scoped catch fails all three new tests, whether or not the predicate is also widened. Widening the predicate alone does not fail them, correctly: with the catch in place the predicate no longer gates that path, so keeping it narrow is defence-in-depth rather than the live mechanism. Flag audit at the same time: -F, -o, -T, -S, -p, -i, -J, -l and -- are now the complete set buildSshArgs can emit, and all are handled. * fix(ssh): make the atomic publish actually run, and unroll the mkdir batch Two runtime defects that only a real host could surface. Both were invisible to unit tests that assert the shape of the generated command string, because both are PowerShell rejecting an argument at execution time. File.Replace was passed a bare $null for destinationBackupFileName. PowerShell coerces $null to an empty string when binding a .NET string parameter, and Replace rejects that with 'The path is not of a legal form' -- so every create-mode publish failed. The Critical fix was inert as shipped. Now [NullString]::Value, which is the construct that exists for this. Measured on awin, same staging-file lock, opposite outcomes: old publish rc=1 destination MISSING <- prior contents destroyed new publish rc=1 destination PRESENT, sha 7f06b7e0... unchanged control, destination present, no lock rc=0 replaced exactly control, destination absent, no lock rc=0 Move fallback created it End-to-end through the real uploader afterwards: 1.9MB x15 all hashes exact, median 303ms; overwrite of an existing destination exact both times. Separately, the PowerShell mkdir fallback could not create a tree of more than one directory. '@($json | ConvertFrom-Json)' wraps the parsed array in another array, so the loop variable binds to the whole thing and [string] of it is the paths joined by spaces. It only ever worked for a one-element batch, where stringifying a single-element array happens to yield the element -- which is why no existing test caught it. Pre-existing on main; fixed here because this PR puts that command on the fallback tier and claims the ladder works. Both tiers now verified live against a three-directory tree.	2026-09-04 01:22:09 -07:00
Neil	cc9e9ed65f	fix(crash-reporting): sample system memory before the process is gone (#18356) ... * fix(crash-reporting): sample system memory before the renderer dies * fix(crash-reporting): make the pre-gone host sample decisive, not just present Round-1 review said the shipped field set could not decide G4-oom. Fixed. Decisive field (blocking #1). The investigation's own win-lowspec repro falsified "low available commit kills": at a 127 MB commit floor Windows grew the pagefile to 2029 MB and nothing died, and it named the missing datum — pagefile-growth headroom / system-drive free space. `getSystemMemoryInfo()` gives neither. Added `swap-volume-free-space.ts`: one `fs.statfs` on the volume backing the pagefile (SystemRoot on Windows, the root fs elsewhere, resolved via `path.parse().root`), published as `systemMemoryPreGoneSwapVolumeFreeMB`. Together with the already-emitted commit limit that separates "commit was low" from "commit was refused". Pagefile *max* size needs a registry read; skipped deliberately — per-operation interpreter spawning is exactly what docs/reference/windows-edr-posture.md says not to add for telemetry. Darwin honesty (blocking #2). Every reading now carries `systemMemoryPressureSignal`: `available-commit` on Windows (swapFree is ullAvailPageFile), `mem-available` on Linux when MemAvailable is present, `none` otherwise — which is always on darwin. A future analyst cannot now table `freeMB: 272` from a healthy Mac as evidence of exhaustion, because the same record says the platform gave no pressure verdict. Partial rebuttal on the suggested reuse: `host-memory.ts:86` was considered and rejected as a periodic source. It spawns `/usr/bin/memory_pressure` per call, and the sampler this PR needs runs every 10 s for the app's lifetime; a subprocess at that cadence is worse than the gap it closes. The reviewer conceded this tradeoff is arguable — what was not acceptable was shipping the darwin gap silently, so it is now in the data, not only in a comment. Staleness (blocking #3). Confirmed the measurement: four of five G4 reports carried a ~37 s-old sample (4872/36796/37332/38017/39715 ms). Host memory no longer rides the 60 s process-metrics sweep; `pre-gone-host-memory.ts` samples it on its own 10 s timer with its own `systemMemoryPreGoneSampleAgeMs`. One GlobalMemoryStatusEx-class call plus one statfs is cheap enough at that rate. A refusal shorter than the interval stays invisible and the module comment says so — no polling cadence fixes that. Non-blocking, all taken: renamed `gone-time-system-memory.ts` -> `system-memory-details.ts` with the now-false "reads AFTER the crash" framing scoped to the gone-time caller; pre-gone host keys moved out of the `processMetrics` namespace to `systemMemoryPreGone*`, so the string-surgery `preGoneDetailKey` helper is gone and a `systemMemory` prefix scan sees both reads; the bare catch no longer spans both halves of the sample, and a test pins that a throwing host read leaves the process-metric sample intact; the inert second test is replaced by three that go red without this change (verified: swap-volume, pressure-signal and cadence assertions all fail when the production hunks are reverted). Rebuttal, non-blocking #5 (duplicated electron mock across two test files): declined. `vi.mock` is hoisted per file, so the mock cannot be shared without a setup module, and this directory already has 26 focused test files that each re-declare it. Splitting by concern is the local convention. `startPreGoneProcessMetricsSampling` is renamed `startPreGoneCrashSampling` since it now starts two samplers. * fix(crash-reporting): test the arming, gate the swap volume, unblock the host read Round-2 review blocked on four items. All four addressed. WHAT THIS BRANCH ACTUALLY DOES, AT HEAD (blocking #4). The commit-1 message ("13 lines, 1 production file, no new module, new optional numeric fields only", `processMetricsPreGoneSystemMemory*` keys, a `preGoneDetailKey` helper, a `pre-gone-system-memory.test.ts`) describes a superseded revision; every one of those claims is false now, so it must not be used as the PR description. The change against origin/main is: 3 new production modules (`pre-gone-host-memory.ts`, `system-memory-details.ts`, `swap-volume-free-space.ts`), 1 deleted (`gone-time-system-memory.ts`), plus edits to `process-gone-diagnostics.ts` and `main-process-ready-runtime.ts` and 2 test files. It adds a second main-process interval timer that runs for the life of the app: every 10 s one synchronous GlobalMemoryStatusEx-class read, and on win32/darwin one `fs.statfs` on the swap-backing volume. Details are `systemMemoryPreGone*`, and two of them are STRINGS, not numbers: `systemMemoryPreGonePressureSignal` (enum) and `systemMemoryPreGoneSwapVolume` (a drive label, separator-trimmed so it is not a path). Both are assigned after `sanitizeCrashReportDetails`; neither carries user content. Arming is now tested (blocking #1). The reviewer deleted `startPreGoneSystemMemorySampling(...)` from `startPreGoneCrashSampling` and all 264 tests stayed green — confirmed and fixed. `pre-gone-host-memory.test.ts` now calls `startPreGoneCrashSampling()` with production defaults and asserts both `setInterval` calls, their literal periods `[60_000, 10_000]`, that both timers are unref'd, and that advancing 10 s takes a fresh host sample that reaches `buildProcessGoneCrashDetails` with `SampleAgeMs: 0`. Verified red on revert: deleting the arming line -> 1 failure; changing the interval constant to 30_000 -> 1 failure (the old assertion compared the constant to itself and caught neither). The tautological `10_000 < 60_000 / 2` test is gone, superseded by this one. Swap volume is win32/darwin only (blocking #2). On Linux swap is a fixed partition, a fixed-size swapfile, or zram; none grow into root-fs free space, so `SwapVolumeFreeMB: 380000` beside `SwapFreeMB: 0` would have invited exactly the wrong verdict on the two Linux cluster members. `swapVolumeAnchor` returns undefined off win32/darwin, so no field and no statfs at all. The comment claiming "elsewhere swap is on the root fs" was wrong and is gone. The Windows anchor is still the DEFAULT pagefile volume, so the measured volume now ships with the number (`systemMemoryPreGoneSwapVolume: 'C:'`) instead of being implied. The honesty label covers it: win32 reads `available-commit` only when the volume datum is present, and `available-commit-unqualified` otherwise — which also fixes non-blocking #5, where the synchronous gone-time read claimed a verdict its own fields could not support. Host read no longer waits on statfs (blocking #3). `samplePreGoneSystemMemory` now commits the synchronous memory reading first and merges volume free space in afterwards, so the cadence is 10 s regardless of disk-metadata latency and a hung volume can no longer stop host sampling — precisely the paging-storm case this exists for. The in-flight latch now guards only the statfs. Verified red on revert to the serialized shape (2 failures). A stale-but-slow-moving volume value merging into a newer memory sample is deliberate and commented. Non-blocking #3 (reset does not invalidate an in-flight sample): fixed with a generation counter bumped by `resetPreGoneSystemMemorySamplingForTest`, so a late statfs cannot repopulate a reset sample. Separately, the volume read now only runs after a host sample committed, which removes the real `statfs('/')` side effect from `process-gone-diagnostics.test.ts` entirely. REBUTTAL, darwin `memory_pressure` reuse (non-blocking #2): declined, with evidence. `readDarwinAvailableMemory` at src/main/memory/host-memory.ts:87 is reached only via `collectHostMemory` <- `runSnapshot` <- `collectMemorySnapshot`, whose only callers are the `memory:getSnapshot` IPC handler and orca-runtime-pty-foreground-process-reads.ts:170 — both on demand. There is no periodic snapshot, so there is no cached reading to reuse for free; adopting it means spawning `/usr/bin/memory_pressure` on a main-process timer for the life of the app, and its module-global `darwinAvailabilitySupported` latch is shared with the memory UI. The gap is not hidden: darwin ships `PressureSignal: 'none'` in the data, and the module comment now cites the existing reader and why it is not used here rather than claiming Orca lacks one. Verified: `vitest src/main/crash-reporting src/main/startup src/main/memory` = 769 passed / 6 skipped (crash-reporting re-run 5x, no flake); `tsc --noEmit -p config/tsconfig.node.json` 0; `oxlint` 0; `oxfmt --check` 0. * fix(crash-reporting): stop a stale statfs qualifying the commit verdict Round-3 adversarial review, 2 blocking. Both fixed with mutation-verified tests. 1. `mergeSwapVolumeFreeSpace` merged the volume reading into whatever sample was current at RESOLUTION time, and `pressureSignal` then upgraded win32 from `available-commit-unqualified` to the decisive `available-commit` on the strength of it. The `swapVolumeReadInFlight` latch makes every intervening tick skip the merge, so the lag is as old as the last STARTED statfs, not the last tick — and no age field exposed it, because `systemMemoryPreGoneSampleAgeMs` describes only the synchronous memory read. Reviewer's executed scenario: a statfs issued at t=0 on a healthy host (40 GB free) resolving at t=20 s of commit pressure emitted `SwapFreeMB: 200` beside `SwapVolumeFreeMB: 40000`, labelled `available-commit`, with `SampleAgeMs: 0`. That reads as "the pagefile had room, so this was not a commit refusal" — the opposite conclusion, wearing the branch's highest-confidence label, on exactly the win32 G4-oom reports this exists to decide. The datum still ships (it is the only pagefile-expandability signal there is), but now: - the sample carries `swapVolumeSampledAtMs` — the tick that ISSUED the statfs, never the one it resolved on — surfaced as `systemMemoryPreGoneSwapVolumeAgeMs`; - only a statfs that answers on its own tick may qualify the verdict. `withSwapVolumeFreeSpace` takes `coTimed`; false keeps `available-commit-unqualified`. The next tick issues a fresh statfs, so the verdict recovers on its own. 2. The branch's sole production entry point — `startPreGoneCrashSampling()` at main-process-ready-runtime.ts:128 — was untested. Deleting it left 691 tests across crash-reporting/ and startup/ green, while a comment in the new test file claimed that gap was why the test was written. This is pure instrumentation, so that one line is the whole of its value in the shipped app. Added a source-level wiring test (the pattern this repo already uses for arm-once ready-phase lines) that pins the import, exactly one call, the call at statement indent, and that `main-process-ready.ts` awaits the function it lives in. The misleading comment is gone. Mutation-verified — each goes red alone: coTimed -> always true 1 failed (verdict) drop swapVolumeSampledAtMs age 1 failed (verdict test) delete startPreGoneCrashSampling() 1 failed (wiring) wrap it in `if (!is.dev) { ... }` 1 failed (wiring) Verified: tsc -p config/tsconfig.node.json exit 0; oxlint src/main/crash-reporting src/main/startup exit 0; 268 tests in crash-reporting/ pass. Across crash-reporting/ + startup/ + memory/: 769 passed, 2 failed — both environment-dependent and failing identically on the unmodified tree (Xvfb rebind, and a whole-repo glob census that times out). * fix(crash-reporting): stop free disk standing in for pagefile growability The win32 reading was promoted to the decisive `available-commit` whenever a co-timed volume number merely existed, which the data cannot support: a fixed or disabled pagefile grows into no amount of empty disk, its maximum is unreadable here, and the measured volume is only the DEFAULT pagefile drive. A host with 180 MB of available commit, a commit limit at RAM and 812 GB free read as "the pagefile had room" — the opposite conclusion, under the branch's most confident label. The volume datum is now named for what it is (`available-commit-volume-cotimed`, context beside the commit number), and the one decisive win32 case — a commit limit at or below RAM, i.e. no pagefile behind it — gets its own label. Also: carry the last volume reading onto the sample that replaces it, aged and non-qualifying, so a statfs slower than one tick no longer makes the field vanish from the reports it exists for; don't commit a reading whose every memory field failed, which shipped an age and a disk-free number with no host memory beside them; and move the startup wiring test beside the file it pins, scoped to the ready-phase entry's own body so the call cannot satisfy it from a sibling export nothing calls. * fix(crash-reporting): co-time the statfs by tick, not sample identity A tick whose host read fails leaves the pre-gone sample object in place, so the identity check still read a 25 s-late statfs as co-timed.	2026-09-04 00:53:37 -07:00
Neil	7a714d1bd2	fix(terminals): add equality bailouts to the tab pane-expansion actions (#18332) ... * fix(terminals): bail out of no-op pane-expansion store writes * test(terminals): lock the root-state identity of the bailout A `return {}` bailout keeps the map reference but still allocates a new root state, so zustand walks every listener. Assert root identity too.	2026-09-04 00:53:34 -07:00
Neil	9acfba401a	fix(crash-reporting): stop claiming kills that never landed, and leave proof when the own-Chromium pid set is unreadable (#18578) ... * fix(crash-reporting): stop the codex POSIX teardown claiming a group that was already gone terminatePosixTree's default group signal swallowed every process.kill error and then recorded a self_tree_kill unconditionally, so an ESRCH — proof the group was already gone and this teardown killed nothing — still put a suspect in the five-second render-process-gone attribution window. Every sibling group-kill in the tree already records only on a proven signal: terminateDedicatedPosixGroup in this same file, forceKillPosixPtyProcessGroups, and the claude account-login teardown. This makes the outlier match them. * fix(crash-reporting): leave proof when the own-Chromium pid set cannot be read `readOrcaChromiumProcessPids` returns an empty set when `getAppMetrics()` throws, which is the right decision — refusing every kill would orphan every PTY, git, codex and notebook tree main tears down, and on main a refusal from `killSourceControlAgentProcess` releases the managed-home lock with the agent still alive. But the empty set was byte-identical to "no Chromium on this host", so the fail-open was invisible in a field bundle. Keeps the decision, adds a coalesced durable `own_chromium_pids_unreadable` crumb so the two cases are distinguishable. Coalesced because the gate reads this set on every tree kill. * style(crash-reporting): tighten the group-signal comments to the WHY	2026-09-04 00:53:31 -07:00
Neil	11e459e933	fix(crash-reporting): bound replay-guard wedge bursts in the ring without losing their spans (#18441) ... `terminal_replay_guard_wedged_release` was not in COALESCED_RENDERER_BREADCRUMB_NAMES, and its per-pane hashes give every entry a unique ring identity. One mount/reveal/wake transition expires every in-flight replay write at once, so a burst arrives as N distinct entries against a 30-slot FIFO ring. Measured, from the 09-02 corpus (121 `renderer.breadcrumb` spans across 9 of 55 diagnostic bundles): - bundle 26461769: 26 events in 0.96s - murlock1000: 62 events over 85s - 8907a508 mixes two call sites in one window (2 crumbs carry `tabIdHash`, 2 do not) Not measured: no captured report's ring actually lost slots to this crumb. All 57 reports have zero wedge crumbs in `Recent activity:`, and in all 9 bundles the burst predates the report's ring window — for 26461769 the burst ran 13:36:03.784Z-13:36:04.742Z while the ring-owning main process started at 13:43:44.380Z, 7m40s later. So this bounds a demonstrated hazard, not an observed loss. An earlier draft of this commit asserted "26 of 30 slots / 87% of the pre-crash trail" as a measurement; that was a model, and it is removed. The burst evidence lives entirely in the durable span stream, and suppressed repeats normally emit no span (see the 1000-emissions/1-span case in crash-reporting-renderer-breadcrumbs.test.ts), so coalescing alone would have cut that 121-event corpus to 13 with the multiplicity recorded nowhere. Instead: - the ring coalesces: one slot per call site, plus `suppressedSinceLast` - every wedge event still emits its own `renderer.breadcrumb` span, via PER_EVENT_TRACED_COALESCED_BREADCRUMB_NAMES. Span volume is unchanged at 121, and the span deliberately carries no count so a span-stream total cannot double-count what the ring already claims - the coalesce key is `ptyId`/`tabIdHash` *presence*, not name alone: those fields are absent on the restore call site (restoreScrollbackBuffers) and present on reattach, so name-only keying would collapse 8907a508's two call sites into whichever crumb landed last. Bounded at 4 slots per storm, matching the webgl `kind` and duplicate-tab `resolvedToActiveWorktree` precedents in the same file. Replaying the corpus timestamps: 121 events -> 14 ring writes. This is a diagnostics fix, not a crash fix. It does not stop panes wedging, and it does not explain the "can't type" reports in this round.	2026-09-04 00:53:24 -07:00
Neil	b85510f3a9	fix(terminal): warn about remote work when closing the window or quitting (#18593) ... The native window-close warning was built from a local-only pty set: any worktree with a connectionId was dropped whole, and any remote runtime pty was filtered out. A build, test run, or agent on an SSH or Orca Remote host was therefore structurally invisible to it, on every platform. The quit path skipped the check entirely (#524), so remote work got no prompt at all. Route both paths through the same probe the tab-close guard uses, so the two cannot drift, and keep the verdict vocabulary of the SSH execution boundary: only a host that answers "no children" suppresses the warning. An unreachable host is `unverifiable`, never `exited`, so it warns rather than quitting silently — with its own copy, because "could not reach the host" is a different claim than "processes are running". Quit still ignores local ptys, preserving #524: quitting is an unambiguous instruction to end this machine's processes, but not to end execution on someone else's, which a bounded relay grace period will SIGKILL once the countdown expires. The probe budget is 1.5s (vs the tab guard's 4s) because quit is time sensitive; expiry raises the prompt, so an unreachable host costs a click rather than the 15s RPC timeout or a silently orphaned build.	2026-09-04 00:51:21 -07:00
Neil	561a94038c	fix(ssh): stop the daemon's own services from blocking the superseded-relay reap (#18586) ... `isReapableRelayHusk` required `childCount === 0`, where `childCount` came from `pgrep -P <relay> | grep -c .`. But the relay forks service children of its own, and `relay-ai-vault-service.js` never exits once spawned. Any relay that had served a single AI Vault request therefore reported a non-zero child count forever, so the sweep answered `retained-live-work` for a superseded, disconnected relay holding no user work at all — and its version directory stayed pinned against GC by its own live socket. The probe now censuses each direct child instead of counting them, and the reap gate reads the count of children it could *not* positively identify as relay infrastructure. The asymmetry is the safety argument (docs/reference/ssh-execution-boundary.md): subtracting a child we can name is positive knowledge, assuming about one we cannot is not. An unrecognised argv, an argv `ps` would not print, and a host without `pgrep` all keep the relay unreapable. `reapEmptyRelayHuskCommand` re-runs the same census on the host immediately before signalling. Fixes #13614	2026-09-04 00:06:09 -07:00
Neil	3941edd4b6	perf(ipc): build the filesystem allowed-root list once per authorization (#18423) ... * perf(ipc): build the filesystem allowed-root list once per authorization * perf(ipc): keep the allowed-root snapshot lazy so granted external paths build nothing Hoisting getAllowedRoots to the top of resolveAuthorizedPath made every read of a path covered by an external grant build the full root list, where main built none (the grant answered before isPathAllowed reached the roots). Build on first use instead: still one build per authorization, zero when a grant already answers. * test(ipc): skip the allowed-root symlink escapes on Windows Unprivileged Windows cannot create symlinks (EPERM), so both cases failed in setup instead of exercising the escape check.	2026-09-03 22:20:35 -07:00
iverJistyandNeil	7574ee8403	fix(ports): route the status-bar popover scan to the workspace's host (#17048) ... * fix(ports): route the status-bar popover scan to the workspace's host - PortsStatusSegment resolved its runtime target from the global active runtime, so opening the popover on a paired-remote workspace scanned the client OS and reported zero workspace ports - Resolve the target from the active worktree's owner host, matching PortsPanel, PortRow, and WorktreeCardPorts - Add publishWorkspacePortScanForHost: store the host's scan under its own key, then republish the aggregate through setWorkspacePortScanProjection so a single-host refresh no longer drops every other host's ports - Publish through the projection setter instead of setWorkspacePortScan, which wrote the synthetic all-hosts key back into workspacePortScansByKey and made the next merge fold the aggregate into itself (duplicate rows) - Reuse the helper for the manual panel refresh and the post-stop refresh, and share the aggregate key constant with WorkspacePortScanner * test(ports): cover popover host routing and aggregate preservation - PortsStatusSegment.host-routing: popover scans the active workspace's owner host, keeps other hosts in the projection, publishes a failed scan under its own host, and stays local when the workspace has no owner - workspace-port-scan-publish: single-host key vs all-hosts projection, and repeated publishes never accumulate duplicate rows * fix(ports): surface a host whose port scan failed instead of dropping it - The merged projection only carries unavailableReason when every host failed, so one unreachable server read as "this workspace has no ports" - Add getUnavailableWorkspacePortHosts: hosts that failed while another host still answered, with the local host distinguished by a null environment id - Show one notice per failed host in the popover, named by its runtime environment or the local host label, above the surviving hosts' ports - Reuse the existing scan-unavailable string so no catalog entry is added * test(ports): prove the popover's own failed scan reaches the host notice - Make the mocked store setters write back, so a publish and the notice that reads it can no longer name different scan keys with every assertion green - Cover open popover -> remote scan rejects -> notice names the host, the seam the store-write and render-only tests each stopped short of - Drop an assertion comment that claimed to prove port preservation when it only exercised the render path * fix(ports): review nits — single-write publish, colon-safe host keys, failure port retention, docstrings * fix(ports): keep the popover count and body in agreement, name every failed host - A failed scan retains the host's last-good ports, and the badge/header count them; the notice now sits above the list instead of replacing it, so the popover no longer claims N ports over an empty body. - getUnavailableWorkspacePortHosts reports all-hosts-failed too, so total loss of contact names each host instead of printing raw scan keys under platform 'unknown'. - Scan keys parse to a discriminated host ref, so an unrecognised key is 'unknown' rather than silently blamed on the local machine. - Extract useWorktreeRuntimeTarget for the four ports surfaces that hand-rolled the same owner-settings spread. * fix(ports): label the local host from the failed scan's platform, not the renderer's userAgent A paired web client's browser is not the Orca host, so deriving 'Local Mac' from navigator.userAgent mislabels a Linux host. Carry each failed scan's own platform through the unavailable-host list instead. * fix(ports): keep the Ports panel list under its failure notice too The retained-ports change gave a failed scan both ports and an unavailableReason, and the right-sidebar panel hid every section behind the notice — stripping the stop and open actions for ports the status bar still counts. Gate the sections on whether anything is left to list, matching the popover, behind a testable predicate. * fix(ports): let a retained-port failure keep its debounce grace period The popover publishes the host's last-good ports alongside the failure reason the moment its own scan fails. reconcileTransientPortScanFailures treated any published result carrying unavailableReason as a spent grace period, so the very next background poll replaced those ports with an empty unavailable scan — the retention never survived one poll interval. Keep the grace while the published result still has ports; the tolerance still clears them on schedule. * fix(ports): prune stale hosts in the poll's single map write A manual publish (the ports popover) can resolve after the host-set change already pruned its key, re-adding it; the poll's per-key writes only ever added, so a removed host kept its ports in the count and held a permanent unavailable notice until the next host-set change. Publish the poll's already-pruned map in one replaceWorkspacePortScans instead, which also collapses N per-host notifications into one and drops any synthetic all-hosts key that leaked in. * fix(ports): fail closed for direct SSH workspaces --------- Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>	2026-09-03 22:19:40 -07:00
Neil	8854b5ded5	perf(ssh): coalesce concurrent git.listWorktrees reads (#18419) ... * perf(ssh): coalesce concurrent pty.inspectProcess and git.listWorktrees reads Both were the only reads in their provider class with no in-flight dedupe while their siblings already had it. Route them through the existing InFlightPromiseDedupe, keyed per (relay pty id, incarnation) and per repoPath, scoped to the provider instance so two hosts never share an entry. The worktree listing clears from invalidateGitReads(), and a signalled read keeps its own request so one caller's abort cannot cancel its joiners' scan. In-flight only, no TTL: the relay does answer inspectProcess from a 500ms TTL-cached process table, but a client TTL would compound with it rather than match it, so it is not a free win. * perf(ssh): drop the inspectProcess half, ratchet per-read host observations The `git.listWorktrees` dedupe ships unchanged. The `pty.inspectProcess` dedupe is reverted: the host mints one `observationEpoch` per request and the pane foreground reader commits it per read, so two overlapping probes sharing one reply make the second read a stale replay and `admitRemoteForegroundEvidence` rejects it -- a would-be `live` identity read becomes `unverifiable`. The pane foreground tracker overlaps its own probes by design (cancel-and-reissue after a 350 ms settle), so that path is reachable. Adds a ratchet that fails when the dedupe returns, driving the real reader through the real provider operations. * fix(ssh): move the inspect ratchet to the provider it guards The ratchet lived under src/renderer and imported src/main/providers/ssh-pty-provider-rpc-operations, dragging the whole main-process graph into config/tsconfig.tc.web.json (TS6307). Split it: the request-counter ratchet moves next to the provider it pins, and the renderer file keeps the why -- a shared host observation degrades the second overlapping read to unverifiable -- against the real reader with no cross-project import. * docs(ssh): document the worktree-list coalescing contract	2026-09-03 22:03:05 -07:00
3 people	f36c03e84a	fix(windows): make the install-dir ACL repair rescue the launch it runs in (#18361) ... * fix(windows): repair the poisoned install-dir ACL before the window, not after The install-dir LPAC ACL poison (electron/electron#51761) still costs every affected machine at least one crash: the probe that detects it is setImmediate-deferred and answers 0.9-3.0s in, while createMainWindow runs synchronously in the same frame and its renderer dies at init 48-1373ms later. - Persist the poison verdict the moment the probe reports it, and await the repair (bounded at 20s) before any window is created on a launch that already carries the marker. - Do not engage the GPU safe-graphics fallback while the install-dir ACL verdict is poisoned or still outstanding. Safe graphics does not rescue a poisoned tree, and --in-process-gpu removes the GPU child, erasing the sibling-death evidence that identifies the shape (4 field reports landed in 'misc' this way). - Clear the safe-graphics marker once the repair lands, so a repaired machine stops launching software-rendered for the rest of that build. - Give the repair marker a bounded retry budget: it was written on failure and matched regardless of outcome, so one transient failure pinned a machine to 'marker-hit' for the life of that version. * test(windows): pin the install-dir ACL repair against the real icacls binary * fix(windows): stop the install-DACL verdict from outliving the evidence Adversarial review round 1. Five blocking findings, all addressed. 1. gpu-lifecycle guard had only a source grep (green with the polarity inverted). The stated justification -- that gpu-lifecycle's import graph cannot be driven in-process -- was wrong: mocking `electron` plus `@electron-toolkit/utils` imports it fine. Replaced with gpu-lifecycle-install-dir-acl-guard.test.ts, which drives the real handleGpuChildCrash against a stub tracker. All four cases go red when the guard is flipped to `if (!isInstallDirAclSuspect())`. 2. A clean probe verdict retired the on-disk marker but not the in-memory `poison` verdict, so a machine the probe just proved healthy kept suppressing the GPU safe-graphics fallback and kept the dialog accusing the install folder -- permanently, since a `status:'failed'` probe deliberately keeps the marker. A positive clean reading now latches `installDirReadClean`, drops the verdict, and outranks a repair result that lands after it (a 'failed' from a repair with nothing left to fix must not re-accuse). 'repaired' is kept: it is not a contradiction and it is what tells the user to reload. 3. `noteWindowsInstallDirAclProbePending()` ran on every `openMainWindow` while the probe is once-per-process, so every tray/second-instance reopen armed a 15s window in which `recordGpuCrash` was never called at all -- on healthy machines. `probeWindowsInstallDirAcl` now reports whether THIS call dispatched, and only a dispatch arms the grace window. 4. The pre-window ordering guarantee was defeatable and untested. `focusExistingMainWindow` opens a window whenever there is none and the app is ready -- true for the whole 20s gate, which is exactly when a user double-clicks the shortcut again. Added a `canOpenWindow` seam (same 'pending' semantics as the existing `!app.isReady()` case) wired to `isBlockingInstallDirAclRepairInFlight()`, plus windows-install-dir-acl-startup-wiring.test.ts pinning the await ahead of both window-creation paths and both new call sites. 5. windows-install-dir-acl-repair.win32.test.ts was absent from the pr.yml win32 allowlist, so it ran nowhere. Added. Also from the non-blocking list: - The repair no longer clears a `userConfirmed: true` safe-graphics marker; "keep safe graphics" is a user choice, not Orca's automatic latch. - `repairWindowsInstallDirPackageAcl` now reports its dispatch too, so a second entry into the gate resolves immediately instead of eating the full 20s budget waiting on an `onDone` that is never coming. - The gate is wrapped in try/catch/finally, matching the contract the probe documents as mandatory for anything upstream of window creation. Rebutted, not applied: - "Gate should be conditioned on app.isPackaged." A dev launch only carries the poison marker if a dev launch actually probed that tree and found the signature, in which case the dev renderer is dying the same way and the repair is exactly what is needed. The adjacent `isPackaged` check guards a packaged-only early-window optimisation, not a correctness boundary. - "Fold the poison marker into the repair marker's `outcome`." They answer different questions with different lifetimes. The repair marker is a retry budget (`attempts >= 3` disables the repair for that version) and is never cleared; the poison marker is cleared by a successful repair and by a clean probe. A `'pending'` outcome written before the attempt would bump `attempts`, so three launches killed mid-repair would permanently disable a repair that never once ran icacls to completion. * fix(windows): keep counting GPU crashes while the install-DACL verdict is pending Adversarial review round 2. Both blocking findings addressed. 1. handleGpuChildCrash early-returned on isInstallDirAclSuspect() BEFORE recordGpuCrash, so the crash left no trace in the 30s rolling window. The suspect window is armed on every win32 non-serve launch, and the field bundles put it at 0.8-1.7s after main_window_created on hosts whose DACL is clean (matchesPoisonSignature=false) -- squarely inside the 2.1-6.2s bad-driver bursts this repo already pinned in gpu-crash-fallback-field-sessions.test.ts. A healthy machine with a failing driver could lose an entire coalesced burst and never engage safe graphics. The crash is now always recorded; only the engagement consults the verdict, and it waits for the verdict rather than acting on the suspicion (waitForInstallDirAclVerdict, resolved by the probe's onDone or by the existing 15s grace, whichever lands first). Deviation from the review's suggested shape, deliberately: awaiting the verdict before persisting anything reintroduces the exact race gpu-fallback-engagement.ts documents -- Chromium aborts the whole browser process on the 6th GPU crash, ~1.3s after the 3rd, which is less than the probe takes to answer. So the unconfirmed marker is written up front and withdrawn if the verdict comes back poisoned. A machine killed mid-wait still comes back software-rendered, and its marker is unconfirmed, which is the state the repair's own clear already retires. gpu-lifecycle-install-dir-acl-guard.test.ts now drives the real GpuCrashFallbackTracker and the real engagement path (the restart prompt firing is the signal) instead of a stub tracker, and covers the case the previous suite could not express: a burst that lands entirely inside the pending window still engages once the probe reports clean. Four reverts go red -- restoring the pre-record guard (2 tests), dropping the wait, dropping the post-wait re-check, and dropping the pre-wait marker write (2 tests). 2. The round-1 evidence block quoted commits, a test name and pass counts that no longer exist, and its real-icacls Windows run predated the commit that rewrote the gate. Re-run at this commit; counts and the live-Windows result are restated in the handoff rather than carried forward. Also from the non-blocking list: - 'marker-hit' conflated "already repaired" with "retry budget spent", because hasMarkerFor matches outcome === 'repaired' too. The result now carries alreadyRepaired, and the recovery maps that to stage 'repaired' -- so a launch killed between a successful repair and its marker clear no longer tells the user the folder needs an administrator, no longer latches isInstallDirAclSuspect() for the session, and does retire the poison marker. Not applied, with reasoning: - "clearGpuFallbackMarker narrowed to userConfirmed === false leaves the target population software-rendered after a repair." The summary was overstated and is corrected, but the narrowing stands: a userConfirmed marker now requires a clean DACL verdict, because the restart prompt that writes it is exactly what the gate above withholds while the install is a suspect. The population this family targets can no longer reach confirmMarker while poisoned. - "writeInstallDirAclPoisonMarker re-stamps on a budget-exhausted machine forever." True, but on that machine the tree really is still poisoned and the gate resolves immediately ('skipped', no icacls spawn, no 20s wait), so the marker is telling the truth. Retiring it would be wrong; only a clean probe reading should. * fix(windows): register the real-icacls spec and stop its teardown racing icacls Two ratchets were red: - windows-lane-tree-removal-boundary: the win32 spec's afterAll used raw rmSync on a tree two icacls.exe children had just rewritten DACLs on, which is the EPERM race removeTreeSync exists for. - win32-test-lane-registration: the spec was in the pr.yml argv but not in WINDOWS_PACKAGE_TESTS, so a future diff touching only test files would not select package_windows and the spec would self-skip on ubuntu and report success. * fix(windows): re-arm the GPU fallback latch when the install-DACL verdict withholds it recordGpuCrash reports the threshold crossing exactly once and latches `engaged`. handleGpuChildCrash consumes that report before consulting the DACL verdict, and installDirAclClearsGpuFallback then discards it — so nothing could ever engage safe graphics again in that process. A machine whose tree the repair fixes and whose driver is genuinely broken stayed hardware-accelerated through an unbounded crash loop, with no prompt and no marker. disengage() releases only the one-shot latch; the crash window is untouched, so a real driver burst is still never erased. Test is RED without the re-arm. * fix(windows): keep the safe-graphics marker while an install-DACL repair is in flight The gate dispatches a repair without arming the probe clock, so waitForInstallDirAclVerdict() returns immediately and the withdrawal deleted the marker inside Chromium's FATAL window (crash 6 lands ~1.3s after crash 3, well inside the 20s gate). The process then died mid-repair, spent no attempt, and relaunched hardware accelerated into the same gate — spawning the same GPU children, FATALing again, forever. Hold the marker while poison.stage is 'pending' so that launch comes back software rendered and the next gate runs to completion. Still not engaged this launch, so --in-process-gpu does not erase the sibling-death evidence. A terminal verdict has no next step to rescue, so it still withdraws. Both new tests are RED without the retention. * fix(windows): stop a repaired marker outranking a fresh poison verdict The probe reads the install DACL and finds it poisoned; `startRepair` dispatches; `markerHitFor` sees a repair marker recording `outcome: 'repaired'` for the same installDir+appVersion and reports `alreadyRepaired`, which the recovery module maps to stage 'repaired'. So the launch that just proved the tree poisoned runs no icacls, deletes the poison marker that arms the next launch's pre-window gate, clears the suspect flag so `--in-process-gpu` can engage on a tree safe graphics cannot rescue, and tells the user "Orca repaired the permissions." Reachable whenever the tree is re-poisoned after one successful repair of the same version, and whenever a repair reports success without clearing the tree — the silent icacls no-op this module exists to document. A DACL reading taken this launch now outranks the marker: `probeConfirmedPoisoned` stops `outcome: 'repaired'` short-circuiting the repair. The attempt budget still bounds it, so an unrepairable tree does not re-spawn icacls forever. The pre-window gate does not set the flag — it acts on a marker from an earlier launch, not on evidence of its own, so a recorded repair still outranks it there. Also drives the GPU-fallback re-arm test through a repair that actually completes 'repaired', rather than a later clean probe, which is the route the review exercised. * fix(windows): make the pre-window ACL gate act on the poison evidence it fired on The gate fired on a poison marker — an earlier launch's DACL reading that nothing has retired — but withheld `probeConfirmedPoisoned` from the repair, so a repair marker recording an older success still short-circuited it. On the three-launch shape the gate exists for (repair succeeds; tree is re-poisoned; the next launch's probe records the poison but dies before writing its repair marker) the gate ran no icacls, deleted the poison marker that arms every later gate, un-suspected the tree so --in-process-gpu could engage, and told the user "Orca repaired the permissions." `applyInstallDirAclProbeVerdict` then swallowed that launch's own reading behind `if (poison) return`. Both callers of `startRepair` hold outstanding poison evidence, so the flag is now unconditional (renamed `poisonEvidenceOutstanding`) and `marker-hit` means only that the attempt budget is spent. The probe guard is narrowed to an in-flight gate repair: a reading taken after the gate finished re-arms the poison marker and downgrades a claimed repair. Also: withholding safe graphics now ends with the repair budget. A machine whose attempts are spent while the signature persists was denied safe graphics on every launch for the life of that appVersion — and had its marker deleted each time — including the healthy installs the probe's flag-blind ACE match over-matches, where the driver really is broken. Non-blocking, same lane: re-read `isQuitting` after the up-to-15s verdict wait, and skip the recovered-launch prompt when the ACL gate retired the marker read before whenReady. * fix(windows): stop a timed-out gate repair outranking a later poison reading The gate's 20s budget expires while icacls runs on under its own 120s cap, so the probe can read the tree poisoned while that repair is still in flight. Its success claim then deleted the poison marker, un-suspected the tree and told the user their permissions were fixed. The reading is now latched and outranks it. * fix(windows): stop a gate repair claim pre-empting this launch's probe reading Round-7 adversarial findings, both driven against the real modules: - isInstallDirAclSuspect returned false the moment the pre-window gate set stage 'repaired', short-circuiting ahead of the probe-pending grace check. The GPU children die 48-1373ms after window creation while the probe answers 0.9-3.0s in, so an icacls that silently no-opped (exit 0, tree untouched) opened exactly that interval to --in-process-gpu on a still-poisoned tree - and a 'keep safe graphics' answer then pinned a userConfirmed marker no later repair may clear, with the poison marker already deleted so no later launch gates. The claim now stays provisional until this launch's probe corroborates it or the grace window lapses. - A probe reading that disproves a 'repaired' claim re-armed the poison marker but never restored the unconfirmed safe-graphics marker the claim had cleared, so the next launch relaunched hardware-accelerated into the re-armed gate. The clear is now captured and handed back on disproof. * test(windows): pin the nested and update-inherited grants against real icacls The live spec asserted the grant landed on the root-level module file only. It now also pins that the flagless /T pass reaches a nested file carrying its own protected DACL (the shape app.asar.unpacked and node_modules have), and that a file written after the repair inherits the (OI)(CI) root grant - the stated reason that grant form exists. * fix(windows): keep the recovered-launch prompt silent while the tree is the suspect Round-8 fresh-eyes finding, driven against the real modules: the prompt re-read the marker the pre-window gate may have retired, but never consulted isInstallDirAclSuspect() - so after a FAILED gate (tree still a live suspect, window blank behind the 10s reveal fallback, Keep as both defaultId and cancelId) a 'keep it' answer pinned a userConfirmed marker no later repair may clear, on the exact victim class the repair cannot help. The guard now covers both gate outcomes; staying silent leaves the marker unconfirmed, which a successful repair still retires. --------- Co-authored-by: Orca Worker <orca-worker@localhost> Co-authored-by: OrcaWin <alpha-eng@stably.ai>	2026-09-03 21:39:34 -07:00
Neil	36354f1742	perf(remote): read the repo catalog once per publish, not once per worktree (#18410) ... * perf(remote): read the repo catalog once per publish, not once per worktree `remoteWorkspace:setForConnectedTargets` costs 13 ms of main-thread time per call at 0.48 calls/sec — 0.63% of wall on a real session, the second most expensive IPC handler in the main process. Almost all of it is one line. `exportRemoteWorkspaceSession` asks `isTargetWorktree(worktreeId)` once per worktree in the session, and that callback called `targetForWorktree(store, ...)`, which called `store.getRepos()` — and `getRepos()` maps `hydrateRepo` over every repo row. So publishing to one SSH target re-hydrated the whole repo catalog once per worktree, then threw a fresh `createRepoRowExecutionHostLookup` (which itself `filter`s the catalog per lookup) away each time. The lookup is now built once per handler invocation and shared across targets: the rows cannot change inside one synchronous projection, and they are the same for every target. On the session that surfaced this — 413 worktrees, 13 repos, 1 connected target — that is 413 catalog hydrations (5369 `hydrateRepo` calls) per publish reduced to 1 (13 calls). The repo normaliser reached through `hydrateRepo` was the #2 self-time function in a 30 s main-process CPU profile at 0.25%. No user-facing trade-off: identical ownership resolution, identical exported session, identical stale-revision handling. * perf(remote): resolve each worktree's owning target once per publish Follow-up on the same handler: hoisting `store.getRepos()` removed the repeat hydration, but the ownership resolution itself was still repeated once per connected target. `targetForWorktree` computes a connection id from the repo catalog alone — only the final `=== targetId` differs — so exporting to N targets ran the identical resolution N times over every worktree key, and the projection asks the question once per key of `tabsByWorktree`, `activeTabIdByWorktree`, `lastVisitedAtByWorktreeId` and `defaultTerminalTabsAppliedByWorktreeId`. Resolutions are now memoised for the life of one publish, keyed on `(worktreeId, executionHostId)` because both participate in resolution. Test asserts 6 worktree keys resolve 6 times across 2 targets instead of 12. * perf(remote): skip the session and repo reads when no hydrated target is connected Hoisting the catalog read made a zero-connected-target publish pay for a full repo hydration it never did before. Return early instead.	2026-09-03 21:38:03 -07:00
Neil	0593f4e0ad	perf(persistence): stop writing every worktree metadata row twice (#18451) ... * 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. * 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). * 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. * chore(persistence): keep the derivation predicate module-private * test(persistence): pin that a file with no identity map never gains one Answers the review ask that the projection's absent-key contract be asserted on the bytes, not inferred: a profile whose file carries no `worktreeMetaByIdentity` must still not have one after a load+flush.	2026-09-03 21:30:45 -07:00
Neil	6c4797ca9f	perf(runtime): stop the expired-SSH-lease sweep from rescanning every tab layout (#18409) ... * perf(runtime): stop the expired-SSH-lease sweep from rescanning every tab layout The `runtime:syncWindowGraph` IPC handler is the most expensive thing the main process does: measured on a real session it costs 20.7 ms per call at 0.71 calls/sec, which is 1.47% of wall and ~17% of all main-thread JS. 76% of that sits in one subtree: `getHydrationTargets` -> `hasRuntimeOwnedPtyCandidate` -> `getRecentExpiredSshLease` -> `findTerminalTabIdForLeaf`. Three pieces of pure waste, none of which change an answer: 1. `getRecentExpiredSshLease` evaluated its cheapest and most selective filter LAST. `SSH_PANE_RECOVERY_GRACE_MS` is 30 s, so nearly every stored expired lease fails it — but only after the predicate had already resolved the lease's leaf to its current tab, which is the expensive part. The freshness and reattach-eligibility gates now run first; the predicate is otherwise identical and side-effect free, so the selected lease is unchanged. 2. The sweep ran once per tab. `workspaceSessionWorktreeHasRuntimeOwnedPtyCandidate` asked "does a recent expired lease name THIS tab" for every tab in a worktree, and each ask re-read and re-filtered the whole lease list. It now resolves the worktree's recoverable tab ids once, lazily, so a worktree whose first tab already owns a serve/SSH pty still never sweeps. 3. `findTerminalTabIdForLeaf` allocated a `Set` and walked a whole pane tree per tab to answer one leaf lookup. It now reads a leafId -> tabId index built once per layouts record and reused until a layout object is replaced, which keeps first-tab-wins ordering identical. Measured by replaying a real 414-worktree / 801-tab / 137-lease session: 2.51 ms -> 0.27 ms per publish for this subtree, a 9.3x cut. No user-facing trade-off: same leases selected, same tabs reported recoverable, same SSH pane recovery affordance. * fix(runtime): revalidate the leaf membership index on root identity persistPtyBinding grafts a leaf by assigning `layout.root` on the SAME layout object inside the SAME layouts record, so the layout-identity revalidation kept serving an index blind to the grafted leaf and findTerminalTabIdForLeaf answered `undefined` where the pre-index linear scan answered the tab. That fed the SSH reattach fence (restoreReattachedPtyRuntime) and the expired-lease pane recovery resolver, both of which then fall back to the frozen lease tabId. Membership is a pure function of the root tree and no writer mutates a node in place, so root identity is the exact revalidation key — same O(tabs) pointer compare, no new cap, cadence or staleness window. * perf(runtime): resolve a leaf's tab by scan instead of a cached membership index Fix #3 of this PR cached a leafId -> tabId map per layouts record and revalidated it by comparing every root reference on every read. It was the only mutable cross-call state in the change, the only piece carrying a staleness invariant, and it had already needed one follow-up fix (1c23c544) after a layout-identity key turned out to be blind to `persistPtyBinding`'s in-place `layout.root` graft. The index was never what produced the measured win. After fix #1 moves the freshness gate first, the reporter's replay never calls `findTerminalTabIdForLeaf` at all — every stored expired lease is older than the 30 s recovery grace, so the entire 24.9 ms -> 0.9 ms comes from fixes #1 and #2, both of which are unchanged. `findTerminalTabIdForLeaf` is now an allocation-free scan over the existing `layoutContainsLeafId`, which short-circuits on the first matching leaf instead of materialising a Set per tab. Same answers, same first-tab-in-record-order semantics, no revalidation key, nothing for a writer to invalidate. Re-measured on the same 414-worktree / 801-tab / 137-lease replay (process.cpuUsage deltas, median of 3; wall clock is useless on this box): scenario main index scan all leases stale (replay) 24.86 0.87 0.88 ms/publish one lease inside the grace 24.31 1.08 1.04 ms/publish all 137 inside the grace 18.04 3.71 4.31 ms/publish The measured win is unchanged. Only the synthetic worst case — every one of 137 leases expiring inside the same 30 s window — pays for the cache's absence, and even there the two ranges overlap because the index's own revalidation is O(tabs) per lookup. Removes 208 net lines. `terminal-leaf-tab-resolution.test.ts` keeps the parity cases and adds the guard the cache needed: a subtree replaced in place after an earlier read must be visible to the next one. That test fails against the index. * docs(runtime): say why the leaf scan keeps Object.keys 'Allocation-free' overstated it — Object.keys does allocate one key array. A guarded for...in trades that for a hasOwn call per tab and measures slower, so record the reason the next reader does not re-litigate it.	2026-09-03 21:21:22 -07:00
Neil	1c4c6b7fec	perf(startup): stop queueing window creation behind the proxy apply and i18n (#18436) ... * perf(startup): stop queueing window creation behind the proxy apply and i18n Three independent, measured startup wins, all free: 1. Park the initial Chromium proxy apply on `mainProcessState` instead of awaiting it mid-`initializeReadyFoundation`. `setProxy` still starts at the identical moment; the default-session request guard (which holds, not cancels) is what actually fences fetchers on it, so only window creation stops waiting. Runtime launch still awaits it before the desktop relay and before every headless-serve fetcher. 2. Run `initializeMainProcessI18nAndMenu` concurrently with `initializeMainProcessRuntimeLaunch`. Nothing in window creation reads a translated string or the native menu. 3. Load `emojibase-data` in main through `createRequire` on first use instead of a static import, keeping 166 KB of JSON off `out/main/index.js` and its ~2 ms parse off every launch. The renderer keeps its eager copy unchanged. out/main/index.js 7,210,071 -> 7,040,147 bytes. No renderer behaviour changes. * fix(packaging): ship the emoji shortcode dataset main lazily requires app.asar carries no node_modules, so main's bare requires resolve only out of Resources/node_modules. emojibase-data is a devDependency and is not in the packaged runtime allowlist, so the new createRequire in deferred-emoji-shortcode-dataset.ts threw MODULE_NOT_FOUND in every packaged build — breaking sanitizeWorktreeName, and with it workspace creation. Copy the single 166 KB dataset (not the 49 MB package root) into Resources/node_modules, and gate every createRequire'd bare specifier in src/main against the packaged resource plan. verifyPackagedMainRuntimeDeps cannot catch these: the bundler renames the require binding. * test(proxy): fail CI when a main-process fetcher escapes the default-session guard The hoist relies on installElectronProxyRequestGuard(session.defaultSession) holding every app-owned request until the persisted proxy lands. Nothing enforced that every fetcher actually lands on defaultSession. Two source-anchored rules do now: no net.fetch/net.request may name a session/partition, and every non-net .fetch( call site is counted against an allowlist. * test(proxy): close the shorthand and chained-receiver holes in the fetch call-site audit The audit caught `net.request({ session: x })` and `ident.fetch(`, but not the two shapes a real regression is just as likely to take: the `{ url, session }` shorthand that both `net.request` overloads accept, and a receiver with no bare identifier (`session.fromPartition(...).fetch(`, `ctx.session.fetch(`). Rule 1 now also matches the shorthand key; rule 2 scans every `.fetch(` and excludes only a literal `net`/`globalThis`/`global` receiver. Audited counts are unchanged (2/2/1). * fix(startup): scope the deferred emoji loader to the projects that own it TS6307: the composite web project lists src/main/ipc/worktree-logic.ts, which now imports the deferred dataset loader, and the shared lazy test reached into src/main from a project that has no src/main files. Add the loader to tsconfig.tc.web.json and move the cross-project case into a src/main test. Also close the last two review gaps: gate the runtime-RPC startup failure dialog (the only launch-phase translateMain reader) on a published i18n barrier so a concurrent i18n phase cannot leave a non-English user with the English fallback, and let the fetch call-site audit match `net.fetch (url)`.	2026-09-03 21:19:06 -07:00
Neil	ef9e9f3fd9	perf(main): take the idle ownership poll off the main thread and batch pending marker probes (#18425) ... * perf(main): take the idle ownership poll off the main thread and batch pending marker probes The runtime-metadata ownership watch ran existsSync + readFileSync + JSON.parse on the main thread every 10s for the life of the process. Move it to fs/promises with an ENOENT catch (dropping the existsSync pre-check, a TOCTOU race anyway) and guard overlapping ticks. The base-directory poller's pending `.git` marker probes ran serially, costing D x latency per tick for up to 300 ticks. Route them through the same forEachWithConcurrency bound the full scan already uses. * test(runtime): pin that a shutdown-straddling ownership read cannot republish CodeRabbit flagged the async read resuming after stop(). The cleared activeTransports guard already neutralizes it; this test pins that guard rather than the interval teardown.	2026-09-03 21:16:04 -07:00
Neil	0d42e3fc99	perf(persistence): stop double-traversing the persisted session at load (#18458) ... * perf(persistence): stop double-traversing the persisted session at load normalizeLoadedProfileState is the largest measured startup cost that scales with profile size, and almost all of it is zod-validating the 1.9 MB workspaceSession blob. Two redundant traversals removed, with no change to what is accepted: - The salvage containers wrapped `z.record(z.string(), z.unknown())` / `z.array(z.unknown())` around a transform that re-validates every entry itself, so zod validated and copied each map and array before the real per-entry parse even started. The containers now apply the same guards zod applied (`isPlainObject` plus its enumerable-symbol-key rejection, `Array.isArray`) and walk the input once. - The two recursive layout node schemas were plain unions, so every split node of every restored terminal and tab-group layout re-tried the leaf branch. They discriminate on `type`, which has the same accept and reject set. Cold parse of a 413-worktree / 801-tab profile: 52.5 -> 45.0 ms CPU (-14.3%, median of 25 interleaved processes). Steady state: 9.8 -> 7.1 ms. * test(persistence): pin absence-stays-fatal for the salvaging containers The comment on salvagingArray claimed a bare container in a z.object shape would read a missing key as an absence unless wrapped. Not true on zod 4.5.4: a bare transform sets neither optin nor optout, and handlePropertyResult only swallows an absent key's issues when a field is both. Assert it instead of documenting it, and correct the comment. Also add the .js extension the node16 CLI project needs on the two dynamic imports these tests added, which broke `pnpm tc`.	2026-09-03 21:15:28 -07:00
Neil	71721a6eef	perf(renderer): narrow the App-root badge and terminal pty-set subscriptions (#18444) ... The unread dock badge held the App root subscribed to `tabsByWorktree`, so every agent title frame re-rendered the whole shell for an integer that had not moved. The terminal snapshot-capability memo was keyed on the same raw maps plus `terminalLayoutsByTabId`, so title frames and active-leaf moves rebuilt the whole pty-id set — work its own value key then discarded. Both now gate on the exact fields their consumer reads, compared in place.	2026-09-03 21:11:21 -07:00
Neil	6a5aa1904f	perf(renderer): load the project-location and feedback dialogs on click (#18440) ... * perf(renderer): load the project-location and feedback dialogs on click Both are reachable only from an explicit click, but their chunks sat on the renderer boot graph and were fetched and parsed on every launch. Route them through the existing `lazy-with-retry` helper, keeping each trigger eager so the click target still exists, and keep the mount sticky once opened so the dialog's own close animation and repeat opens are unaffected. Renderer boot graph 4,473,242 -> 4,424,142 bytes (-49,100 B / -47.9 KiB). Trade-off: the first open per session now waits on a local chunk fetch — measured at ~0.53 ms (project location) and ~0.26 ms (feedback) of read plus V8 parse/compile, warm page cache. * test(renderer): flush the lazy set-location chunk in the ready-target test Without the flush this case only passed because an earlier test in the file had already resolved the shared lazy chunk; it fails under -t filtering. * perf(renderer): warm the lazy dialog chunks on their precursor Both deferred dialogs have a guaranteed, strictly-earlier precursor: the composer only renders "Set location" for a needs-setup host that can take one, and Send Feedback only exists inside an open help menu. Warm each chunk there with a swallowed `import()` (the `preloadCommentMarkdown` pattern) so the fetch/parse happens while the user is reading the picker or the menu, not on the click. Boot graph is unchanged in kind: `import()` never enters modulepreload, so the win holds at -48,958 B (was -49,100 B before the warm; the 142 B is the warm's own source on an already-preloaded chunk). * test(renderer): make the composer warm guard's no-mount assertion real The mock stubbed SetProjectLocationDialog as `() => null`, so the "warming must not mount the dialog" assertion could never fail — the testid it looked for was not rendered under any condition. Render a marker unconditionally instead, matching the sidebar guard, so the assertion actually pins the behaviour. Verified non-vacuous: forcing the lazy element to mount eagerly now fails with "expected <div /> to be null" rather than passing. * fix(renderer): latch the lazy dialog mounts in state instead of during render React Doctor's ref-mutated-during-render rule failed static analysis on both sticky-mount latches. Use the useState mount-flag idiom already in NewWorkspaceComposerModal (addProjectMounted), set from the open handler.	2026-09-03 21:11:02 -07:00
Neil	558f57de58	perf(source-control): sort branch entries before filtering, gate projections by view mode (#18426) ... * perf(source-control): sort branch entries before filtering, gate projections by view mode Two dead-work fixes in the Source Control file projection. 1. filterAndSortSourceControlPathEntries copied and re-sorted the uncapped branch entry list with Intl.Collator on every keystroke. Sort once on branchEntries, filter after: Array#filter preserves order and compareFileNames is a total order, so filter(sort(x)) === sort(filter(x)). 2. The tree projection was built in list mode and the list projection in tree mode, then discarded. Gate each memo on sourceControlViewMode and return a shared empty projection, matching the combined-diff file tree precedent. * docs(source-control): drop the total-order premise from the projection sort argument The sort-before-filter swap does not need compareFileNames to be a total order. A stable Array#sort places each element by (comparator result, original index) and Array#filter disturbs neither, so filter(sort(x)) === sort(filter(x)) for any self-consistent comparator -- which the previous filter-then-sort already required. Restating that removes a shared-module property (the code-unit tie-break in file-name-sort.ts) from this hook's correctness argument instead of defending it. Also record on the EMPTY_* singletons that the gates and both branching consumers read one sourceControlViewMode prop in one synchronous render, so the off-mode value cannot reach the screen, and warn against deriving the mode from a separate store read. New guard: matches filter-then-sort under a comparator that is not a total order. It ties every path sharing a top-level directory over 300 entries and fails against a correct-but-unstable sort. With the duplicate paths removed from ORDERING_FIXTURE the pre-existing equivalence test passes under that same mutant, so this is the only test that pins stability. No behaviour change: counters over first render + 8 keystrokes at n=2000 are identical before and after (34685 compareFileNames calls, 0 tree builds in list mode). * refactor(source-control): freeze the empty branch-tree singleton Object.freeze([]) matches the other three empty projection singletons and the combined-diff-file-tree precedent; readonly types keep it honest.	2026-09-03 21:05:55 -07:00
Neil	6815fed6d6	perf(worktrees): converge the trash sweep instead of re-walking doomed trees (#18429) ... * perf(worktrees): converge the trash sweep instead of re-walking doomed trees `transientLockRemovalOptions()` only asked for `maxRetries` on Windows, and `removeHostTree`'s retry ladder was gated on `process.platform === 'win32'`. A concurrent writer is not Windows-specific: Spotlight/`mds`, a scanner, or a live process writing under the tree surface the same EBUSY/ENOTEMPTY/EPERM on macOS and Linux. So on POSIX the startup sweep got exactly one attempt per entry, failed, and re-issued the same guaranteed-to-fail walk on every launch. - Extend the retry policy to every platform. Windows keeps its error set, its message fallback, and its delays; the message fallback stays Windows-only because POSIX always sets a code. - Persist a per-entry failure ledger in the trash root so a repeatedly failing entry is retried on a 15m/1h/6h ladder rather than on every launch. Nothing is abandoned: the ladder clamps, records are pruned when the entry goes, and a torn ledger fails open to a full sweep. - Defer the sweep behind first paint, so its recursive readdir/rm no longer competes with window creation and worktree-catalog hydration. * fix(worktrees): keep Node's per-level rm retries Windows-only Node's rimraf hands every child back to the retrying entry point (`_rmchildren` -> `rimraf`), so `maxRetries` is applied once per directory level and compounds: a permanently-failing leaf at depth d costs roughly `retryDelay * 36 * 9^(d-1)`. Measured on macOS against one `chflags uchg` file at depth 2, `{recursive, force}` rejected in 1 ms while `{maxRetries: 8, retryDelay: 150}` had not settled after 5 minutes. Handing those options to POSIX removals turned every `removeHostTree` on a worktree residue (`node_modules/.pnpm/...`, a dozen levels deep) into a promise that never settles -- wedging the serialized trash-deletion queue, hanging the sweep on its first failing entry so no backoff is ever recorded, and leaving the unregistered-worktree removal IPC pending forever. Keep the cross-platform retry where this PR put it -- the bounded outer ladders that re-issue one whole `rm` against the same already-chosen path -- and restore `transientLockRemovalOptions()` to Windows-only `maxRetries`. Also guard the deferred first-window task: off whenReady's promise chain a synchronous throw is an uncaughtException, which the pipe-error guard re-throws fatally. * fix(worktrees): make host tree removal see through Electron's asar shim The 267 stranded trash entries were not a concurrent-writer race. Electron patches `fs` so a `*.asar` file reports `isDirectory() === true`, so Node's recursive `rm` descends into the archive, `rmdir`s a real file, and fails the parent with ENOTEMPTY — deterministically, on every attempt. Every worktree that has run `pnpm install` carries a `default_app.asar`, which is why every residue stopped at the same path. Route `removeHostTree` through `original-fs` (Electron's unpatched `fs`, with a `node:fs/promises` fallback outside Electron) instead of retrying a failure that can never succeed. `removalPath`, `rmOptions` and the Windows retry ladder are byte-identical to `main`. Reverts the POSIX retry ladder, the `isTransientRemovalError` widening, the sweep backoff ledger and the inverted `does not retry host removal failures outside Windows` ratchet — none of them were fixing the actual failure. * fix(worktrees): drop the stray orchestration test and bundle the asar guard like production `orchestration-statement-compilation.test.ts` belongs to #18420 and was swept into this branch by accident. It imports `./prepared-statement-cache`, which does not exist here, so `tsc -p config/tsconfig.node.json` failed on this branch. Removed; typecheck is clean again. The Electron asar guard pre-externalized `original-fs` in its own Vite build, which is not what the shipped bundle does. Mirror `isExternalMainModule` from electron.vite.config.ts instead, so the guard also proves the production bundler leaves `createRequire(__filename)('original-fs')` as a runtime require — if that ever became a static import or got folded, production would silently degrade to the shimmed `fs` while the old test kept passing.	2026-09-03 21:04:54 -07:00
Neil	d247d6441b	perf(startup): overlap the runtime capability refresh with the session-tabs inventory (#18460) ... The startup structured-session restore chained `runtime:getStatus` before `session.tabs.listAll`, but the capability value is discarded at that call site — it only seeds the module cache later launch flows read, and the inventory fetch never reads it. On a profile with 413 worktrees / 801 tabs that serial leg cost a median 109 ms of the did-finish-load -> renderer-startup-hydration-done window. Issue both calls concurrently. `Promise.all` still resolves only after both settle, so the capability cache is populated no later than before.	2026-09-03 21:04:04 -07:00
Neil	949c9d3353	perf(worktrees): classify each worktree once, defer the SSH meta index, drop the conflict-path probe (#18433) ... * perf(worktrees): classify each worktree once, defer the SSH meta index, unserialise conflict probes Three redundancies on the worktree-catalog and git-status read paths: - buildDetectedGitWorktrees ran mergeWorktree + toDetectedWorktree twice for every visible row. Discovery backfill returns the same meta object when it wrote nothing, and both builders are pure over it, so skip the second pass on identity. - The SSH worktree-meta index parsed every worktree id on the host, then threw it away whenever the provider was connected. Build it lazily, memoised. - Unmerged `u` records were resolved one fs.access at a time. Resolve the prefix the cap can reach with 8-way concurrency, keyed by record index so Git's output order and error precedence are unchanged. * perf(git): read the porcelain worktree mode instead of probing conflicted paths Every porcelain-v2 `u` record already carries `mW`, the working-tree mode Git stat'ed for that row: `000000` means the conflicted path is absent. Reading it replaces the per-conflict `fs.access`, so the bounded-concurrency resolver, its `= 8` cap, and the order/error-precedence invariant are unnecessary rather than cheaper. `access()` stays only as a fallback for a malformed `mW`, so `parseUnmergedEntry` keeps its signature and neither status-read.ts nor the relay loop changes. Also corrects two fixtures that encoded `mW=100644` for a file that does not exist, which real Git never emits. * fix(test): import the conflict parser statically so the CJS cli project compiles	2026-09-03 21:02:11 -07:00
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).	2026-09-03 20:59:53 -07:00
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.	2026-09-03 20:57:57 -07:00
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.	2026-09-03 20:56:29 -07:00
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	2026-09-03 20:54:59 -07:00
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.	2026-09-03 20:41:17 -07:00
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.	2026-09-03 20:37:44 -07:00
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	2026-09-03 20:34:42 -07:00
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.	2026-09-03 20:25:22 -07:00
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.	2026-09-03 20:10:01 -07:00
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.	2026-09-03 18:24:43 -07:00
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>	2026-09-03 18:07:56 -07:00
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.	2026-09-03 17:59:16 -07:00
foXaCe	49d6d35b16	feat(i18n): add French UI locale ... foXaCe <290678+foXaCe@users.noreply.github.com>	2026-09-03 17:32:59 -07:00
hwantage	48cb575db5	feat(i18n): localize Orca Account settings and navigation to Korean ... hwantage <82494320+hwantage@users.noreply.github.com>	2026-09-03 17:32:55 -07:00
Jinwoo Hong	dec1a1d788	fix(i18n): ship onboarding integration capability strings in boot catalog ... Jinwoo-H <73622457+Jinwoo-H@users.noreply.github.com>	2026-09-03 17:32:48 -07:00
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.	2026-09-03 16:47:32 -07:00
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>	2026-09-03 16:28:45 -07:00
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.	2026-09-03 16:27:13 -07:00
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.	2026-09-03 16:21:52 -07:00