602 Commits

Include renames

Author	SHA1	Message	Date
OrcaWinandOrca Worker	ec030f1d35	fix(windows): sign the NSIS uninstaller via SignPath (#17868) ... * fix(windows): sign the NSIS uninstaller via SignPath `Uninstall Orca.exe` ships NotSigned, and MDE's whole update cluster is that one file: electron-builder copies it to `old-uninstaller.exe` and runs it silently during every update. The cause is narrower than "NSIS generates the uninstaller at install time". app-builder-lib already builds the uninstaller in its own makensis pass and calls `packager.signIf(uninstallerPath)` on it before embedding it (NsisTarget.computeScriptAndSignUninstaller). Orca signs nothing during electron-builder — SignPath signs afterwards, behind a human approval — so that hook is a no-op and the file is deleted before CI can reach it. Use the hook as a relay instead of a signer: the first Windows build exports the uninstaller, it rides the existing inner-binaries SignPath request (no third approval wait), and the rebuild-from-signed-tree pass swaps the signed bytes back in before makensis embeds them. Every added step is fail-open. A missing export, a SignPath artifact configuration that does not cover `uninstaller/`, or a relay error costs only the uninstaller signature — the inner-binary chain and the shipped installer are unchanged. * fix(windows): keep the uninstaller relay out of the packed checkout Review fixes on the uninstaller signing chain. The export path lived at `${{ github.workspace }}\uninstaller-signing\`. `files` in the electron-builder config is all-negation, so app-builder prepends `**/*` and packs whatever is left in the checkout root, and the build step retries up to three times — attempt 1 wrote the file after packing, attempts 2 and 3 would have packed an unsigned `.exe` into app.asar. All seven relay sites move to `runner.temp`, and a contract test now fails if any of them points back into the checkout. The uninstaller staging block guarded with `Test-Path` but left `New-Item` and `Copy-Item` able to throw. That step's outcome gates the upload of every inner binary, so a locked file there would have cost all of them their signatures — worse than before the chain existed. It is wrapped in try/catch, asserted. Also: test `signWindowsUninstallerViaSignPath` itself (it runs in a step with no continue-on-error, so its no-throw property is load-bearing) and the sha1+sha256 double invocation; make the rehearsal verify the uninstaller the installer actually writes to disk rather than only the relay receipt, whose digest comparison is equal by construction; correct the staged-name comment, which asserted a collision that does not reproduce; count what was reported rather than what was extracted; and note two traps — a custom sign hook replaces signtool outright, and the single-env-var relay would race if a second NSIS target or arch is added. * fix(windows): stop the signing rehearsal failing on its own artefact The rehearsal is the merge gate for this chain, so it must not be able to fail on something that is not the thing under test. It trusted whatever 7-Zip's NSIS handler emitted. That handler produces partial or garbled output on some NSIS builds, and a truncated extract would score NotSigned and be reported as "the shipped uninstaller is unsigned" when nothing was wrong. It now has to reproduce the digest the sign hook recorded before its output is trusted; otherwise it falls through to the silent-install route, which is ground truth. A name miss falls through the same way. The install route only checked the signature. Comparing the on-disk file against the receipt is what actually proves the shipped installer embedded the SignPath-signed bytes — the release job's own comparison is equal by construction, so this is the only place the claim is really tested. Also: bound the silent install (a bare `-Wait` on an installer that ever prompts hangs to the 360-minute job cap) and poll before stopping Orca, since the oneClick installer launches the app as it finishes and the process can appear after the installer has already exited. Two smaller ones: `-ErrorAction Stop` on the staging New-Item/Copy-Item so the catch above them does not depend on GitHub's $ErrorActionPreference default; and the relay-path test now counts every occurrence rather than the first, so a step carrying two paths cannot root one in RUNNER_TEMP and leave the other bare-relative — the exact shape of the bug it guards. * test(windows): stop a pre-existing elevate.exe defect masking the gate The first real rehearsal (run 33484703381) proved the uninstaller relay works end to end — the 7-Zip route read the embedded uninstaller, the digest guard did not trip, SignPath accepted the new uninstaller/ zip entry, and the shipped `Uninstall Orca.exe` came back signed. It also failed, on `resources\elevate.exe`, for a reason that predates this PR. app-builder-lib re-copies the pristine cached elevate.exe over `resources\elevate.exe` on every nsis pack — `AppPackageHelper.packArch` calls `elevateHelper.copy()` before `buildAppPackage`, and `CopyElevateHelper.copy` does `copyFile(elevatePath, outFile, false)` then `signIf(outFile)`, which signs nothing because this build configures no certificate. The signed copy restored into win-unpacked is clobbered by the rebuild. That is not the sign hook displacing a signtool call: with no `sign` hook, `signFile` already returned false at "no signing info identified", so nothing was signing elevate.exe before either. release-cut.yml mitigates it separately by pre-seeding the electron-builder cache; this workflow has no such step, which is why the clobber is visible here and not there. Downgrade elevate.exe alone to advisory so it cannot mask the uninstaller result, and record it in the evidence artifact so downgrading stays distinguishable from deleting the check. Both uninstaller verdicts stay fatal, pinned by a contract test that also holds the escape hatch to exactly one file. The underlying defect gets its own PR — it is a UAC elevation helper and deserves more scrutiny than a footnote here. * docs(windows): warn against relaxing the elevate.exe cache guard The tempting edit, for anyone who finds the rehearsal red on resources\elevate.exe, is to relax release-cut's `Valid` + `CN=SignPath Foundation` guard so the cache swap runs under test-signing and the rehearsal goes green. That guard is the only thing stopping a test certificate from being seeded into a cache a real release restores from — both workflows share the key `electron-builder-win-<lockfile hash>`. Shipping users a binary signed by "Test certificate for 'Orca agent ide [OSS]'" is worse than shipping it unsigned, so say so at the place someone would make that edit. --------- Co-authored-by: Orca Worker <orca-worker@localhost>	2026-09-05 21:44:28 -07:00
OrcaWinandOrca Worker	8415d53a05	fix(release): stop shipping an unsigned elevate.exe on Windows (#18044) ... * fix(release): stop shipping an unsigned elevate.exe on Windows The release cut swaps the SignPath-signed elevate.exe into the electron-builder toolset cache so the NSIS rebuild's CopyElevateHelper re-copy becomes a no-op. It searched `<cache>\nsis`, a directory no app-builder-lib layout creates, and `-ErrorAction SilentlyContinue` plus `exit 0` turned that miss into a green step — v1.4.193 and v1.4.194 shipped an unsigned UAC elevation helper. Move the lookup into a script that covers the real layouts (`nsis-3.0.4.1/…`, `nsis@<toolset>/…`, `ELECTRON_BUILDER_NSIS_DIR`), asks app-builder-lib for the authoritative path, and exits non-zero with an ::error:: annotation when it finds nothing. The step stays continue-on-error so the inner-signing chain remains fail-open. * fix(release): make the elevate.exe swap prove it replaced the packed copy Success was "some cached copy was replaced", which a stale release directory carried in by the `electron-builder-win-` prefix restore can satisfy on its own while the bundle the rebuild packs stays unsigned. The app-builder-lib probe returns the exact path CopyElevateHelper will pack, so make that the check and the directory scan the fallback: exit non-zero when the probed copy was not replaced, and annotate a warning when the probe could not run at all, so a green step never quietly means the authoritative check was skipped. Also pin both shebang scripts to LF: `core.autocrlf=true` gives a Windows checkout CRLF, and CRLF plus a shebang breaks vite's transform, so resolve-7za-path.test.mjs currently runs zero tests there. --------- Co-authored-by: Orca Worker <orca-worker@localhost>	2026-09-05 21:44:20 -07:00
OrcaWinandOrca Worker	c252d855ac	fix(windows): resolve npm/pnpm .cmd shims past cmd.exe (#17869) ... * fix(windows): resolve npm/pnpm .cmd shims past cmd.exe A `.cmd` target forces every spawn through `cmd.exe /c` with each argument caret-escaped, and Microsoft Defender for Endpoint scores a long `cmd.exe /c` line carrying caret-escaped natural language as obfuscation. `codex.cmd` is named in the spawn cluster of the MDE incident this addresses. npm's `cmd-shim` and pnpm's `@zkochan/cmd-shim` generate files whose whole body is "find node, run this script". Read one, and the spawn can go straight to `node.exe <script> <args>` — no cmd.exe, no caret escaping. Anything the parser does not recognise exactly, or whose target cannot be confirmed on disk, keeps the existing cmd.exe path. Incidentally fixes a real bug: cmd ends its command at a raw CR/LF whatever the quote state, so a multi-line agent prompt through a `.cmd` shim had to be rejected. Resolved shims have no such limit. * fix(windows): refuse drive-relative shim paths and run the win32 tests in CI Two blocking findings from review. A drive-relative path defeated the absolute-path guard: `win32.isAbsolute('D:evil.js')` is false, but `win32.resolve` reads the drive letter and lands on `D:\evil.js`, outside the shim directory. cmd would have built `C:\shim\D:evil.js` and failed; we would have executed the wrong file. Adding `:` to the unsafe-character set closes it, and the alternate-data-stream spelling `a.js:zone` with it. It costs no coverage: 84 of the 91 real shims on this box still resolve, the same seven fall back. Neither `windows-cmd-shim-resolution.test.ts` nor its `.win32` sibling was in the Windows package job's file list, so the whole filesystem/resolution half and the real-spawn equivalence suite ran nowhere. Both are now in `WINDOWS_PACKAGE_TESTS` and in the pr.yml step. Also from review: clear `windowsVerbatimArguments` explicitly on the resolved branch rather than inheriting it, since there is no caller-built command line there; document the kill switch and the PTY/hook-wrapper scope limits in docs/reference; and cover drive-relative, BOM, line-ending, casing and `%*` tampering in the platform-independent half of the tests. * docs(windows): justify the shim-path colon guard from the filesystem rule The guard was argued empirically ("none of the 91 shims on this box has one"), which invites a future reader to relax it for a shim we have not seen. Windows reserves `:` within a path segment, so a relative path cannot carry one at all: the only spellings that can are drive-qualified, an alternate data stream, or a `\?\` device path, and the last is already refused as absolute. That makes a false refusal impossible rather than unobserved. * refactor(child-process): move resolveSpawn into its own module The merge with main pushed run-process.ts one line past the 300-line cap: both sides grew it. The spawn-argv decision is already a pure, separately tested unit, so it moves out rather than the cap moving up. run-process.ts re-exports it, so no caller changes. * perf(child-process): cache the shim interpreter lookup The parse cache spared the shim read but not the PATH walk, so a second resolution of the same .cmd did 0 reads and one statSync per PATH entry -- 30 on a 30-entry PATH, synchronous on resolveSpawn, where one dead network mount blocks the calling thread on every spawn. Keyed by shim directory AND PATH, since the shim's own rule is %~dp0\node.exe first then PATH, and a PATH edit between spawns must miss. Corrects the stat comment, which accounted only for the shim itself. * fix(child-process): revalidate a cached shim interpreter before using it The node cache was held for process life and never rechecked, so a cached node.exe that was later uninstalled -- or dropped from PATH by a version manager -- was still handed to resolveSpawn, failing the spawn with ENOENT. An uncached process in the same state returns null and falls back to cmd.exe successfully, so the cache was strictly worse than no cache. One statSync on a non-null hit, not one per PATH entry, so the walk this cache exists to skip is still skipped. The stale-null direction stays uncorrected on purpose: it only keeps the working cmd.exe fallback. Both directions are now stated in the comment, along with the known miss for callers that vary PATH per spawn. * fix(child-process): honour PATHEXT when resolving the shim interpreter The doc claimed a node.com/.bat/.cmd on PATH returned null and fell back to cmd.exe. The scan actually skipped those entries and kept looking for a node.exe, so PATH=C:\A;C:\B with C:\A\node.com and C:\B\node.exe resolved to B's node.exe while the shim runs A's node.com -- a different binary, chosen silently, on the one axis this module must not get wrong. The scan now follows cmd's rule: first PATH directory holding any PATHEXT spelling wins, PATHEXT order decides within it, and only an .exe winner is returned. Anything else gives up and keeps the cmd.exe path, which restores the strict-subset-of-cmd property everywhere except the documented cwd case. PATHEXT is read from the child's env and joined into the cache key, since it now changes the answer. Costs one stat per PATHEXT entry per node-less directory, paid once per process behind the cache. --------- Co-authored-by: Orca Worker <orca-worker@localhost>	2026-09-05 21:33:16 -07:00
Neil	ba86e1c83c	perf: avoid repeated whitespace scans in diagnostic redaction (#18908) ... * perf: scan diagnostic environment lines without repeated whitespace searches * perf(observability): skip redundant terminator scans in environment-line redaction After a successful match ENV_LINE.lastIndex already sits at end of input or a line terminator, and a failed start's whitespace skip plus terminator scan collapse into one LINE_CONTENT exec. Byte-identical output; verified against the pre-change regex over 280k generated inputs. * perf(observability): find environment-line ends without entering the regex engine	2026-09-05 21:17:46 -07:00
3 people	0cbb01ef4b	fix(security): apply the Windows path-hardening ACL that never ran (#17884) ... * fix(security): apply the Windows path-hardening ACL that never ran `buildWindowsRestrictAclArgs` invoked the hardening script as `powershell.exe -Command <script> <path> <sid> <isDir>`. `-Command` does not populate `$args`; it appends the trailing tokens to the command text. The script therefore read `$args[1]` as `$null`, threw `NullArrayIndex` at `$allowedSids[$sidText] = $true` under `$ErrorActionPreference = 'Stop'`, and exited 1. Both callers swallowed that: the async callback was empty and `applySecurePathRestriction` returned `true` regardless, while the sync `catch` returned `false` and nobody logged. Every Windows secure path has been left on its inherited ACL since the ACL was introduced (#5006), and nothing said so. Replace PowerShell with `icacls.exe`, which takes plain argv. That removes the quoting surface entirely rather than escaping it: interpolating a path into the command text would have turned a dead no-op into arbitrary PowerShell on a filesystem path, since `-Command` executes what it appends. It also drops the execution-policy dependency and the `powershell.exe` spawn an EDR flags, and runs ~25x faster than the PowerShell cold start. Hardening is now three passes: `/reset` to purge explicit ACEs that `/inheritance:r` leaves behind, `/inheritance:r` plus a `/grant:r` per allowed SID, then a read-back that checks the DACL is protected and grants only the intended rights. The predecessor's verification block was equally dead, and an apply that is never read back is only half a control. Failures stay non-fatal — non-NTFS volumes, network paths and restricted tokens fail legitimately and must not break startup — but they are no longer invisible: every failure is logged, and a failed async apply now evicts its cache entry so the next call retries instead of trusting a success that never happened. Routing through `runProcess`/`runProcessSync` also retires this file's `node:child_process` allowlist entry. * fix(security): verify the hardened ACL by identity, not by shape Review found the bug class this PR fixes surviving inside the fix. The verify pass checked rule count, absence of the inherited marker, and exact rights — never *who* the rules named. Granting Everyone full control satisfies all three, so hardening reported success on a DACL that handed the credential to every local account, and most of the real-filesystem tests still passed. Verification now reads the descriptor back with `icacls /save`, which emits SDDL with raw SIDs, and compares the principal set exactly. That is also locale-independent by construction: the previous parse read localized account names out of icacls' OEM-codepage stdout, where a non-ASCII path survived by accident rather than by the documented mechanism. SDDL parsing moves to `windows-security-descriptor.ts`. Two further self-inflicted problems, both measured: The post-rename re-harden led with `/reset`, which re-widened a DACL that was already correct — the staged file's protected DACL survives the rename, so the pass had nothing to do but open a window. Polling an external process during a write into a relocated root caught it: the e2ee keypair dropped to `BUILTIN\Users:(RX)` plus `Authenticated Users:(M)` — read *and* write — before tightening again. Hardening now verifies first and returns early when the DACL already reads back correct, which closes the window and cuts the steady state from three spawns to one. Re-measured: 158 samples, one DACL state, zero broad. Evicting the cache on every failed async apply reintroduced #4901. The env store re-hardens on the read path at ~2/s, so on a host where hardening cannot work (FAT32, network path, restricted token) that was two icacls spawns and two warnings a second, forever. Async retries now take a retry floor and a hard per-path attempt cap. The write path keeps retrying unthrottled — it is user-driven, and a failed credential ACL must still be retried on the next write. Also: failures route through a reporter hook that the main process points at the diagnostic tracer, because `console.warn` reaches nothing in a packaged GUI-subsystem build; `writeSecureFile` returns whether hardening took, and the async branch reports `pending` rather than claiming `applied`; a transient `whoami` failure no longer disables hardening for the process lifetime, and the SID is shape-validated; the `/c` guard now covers the synchronous runner too. * fix(security): re-probe hardening instead of latching a transient failure The per-process attempt cap added for the read-path storm was a permanent latch: one AV scan, momentary lock or %TEMP% blip and every later credential write in that session went unhardened, silently, on a host where hardening would now succeed. Same defect class as #17858's computer-use host, and worse here because what stops happening is security hardening on credential files and nothing said so. The retry budget now bounds the *rate*, not the lifetime: at most three attempts per path per minute, re-probing in every later window, forever. The transition is announced in both directions — `throttled` once per window on entry, `recovered` when a rate-limited path hardens again — so a host stuck in the degraded state is diagnosable rather than merely quiet. The reporter type covers both, and the main process ends the `recovered` span successfully rather than failing it. Extracted to secure-path-hardening-retry-budget.ts, which keeps secure-file.ts under its line cap without a max-lines disable. Also confirms the second flagged risk rather than assuming it: a real unwritable %TEMP% is now covered by a test proving verification fails closed, reports at the `verify` stage, and still leaves the ACL applied — so that path loses proof, not protection, and with the lifetime cap gone it can no longer combine into a permanent-off state. * fix(security): verify a directory's whole inheritance flag set The flag check tested only that `OI` was present — never that `CI` was, nor that nothing else was. That was harmless while `/reset` + `/grant` ran on every pass and repaired whatever was there. The verify-first short-circuit made it load-bearing: what verification accepts is now left alone, so a latent under-check went live because a different fix started depending on it. Two directory DACLs passed while being wrong — both protected, three non-inherited full-control rules, correct SIDs, differing from correct only in their flags: (OI)(F) - no CI, so subdirectories are left unprotected (OI)(CI)(IO) - inherit-only, so the directory object itself grants nobody anything; the next writeFileSync into it fails with EPERM, on a directory just cached as hardened Verification now compares the whole flag set, which also rejects IO and NP, and names the offending flags in the failure. Both shapes are planted in real-filesystem regression tests, including an assertion that a write into the repaired directory succeeds and its child inherits. Confirmed both tests fail against the old check and pass against this one. * fix(security): back the hardening retry off exponentially The fixed one-minute window bounded the retry rate but left a standing floor of three attempts per path per minute on a host where hardening can never succeed — FAT32/exFAT, a network path, a redirected profile. That budget is per path and there are several secure files, so the floor multiplied into tens of thousands of icacls spawns a day for work guaranteed to fail. The delay now doubles after each consecutive failure, from a one-minute floor to a thirty-minute ceiling, and the attempt cap is gone entirely: once the backoff elapses the path is re-probed however long it has been failing. A permanently incapable host settles at ~2 attempts/hour. Slowing the backstop costs almost nothing, because it is not the recovery mechanism: the synchronous write path is deliberately unthrottled, so a host that recovers hardens on its very next credential write regardless of what the read-path budget says. The `throttled`/`recovered` reports are unchanged and matter more here, since the quiet periods between probes are now much longer. The curve is pinned in a new unit test against the exported delay function rather than a copy of its constants, covering the doubling, the ceiling holding at 5000 consecutive failures, a 30-day failing path still re-probing, one announcement per degraded episode, and per-path isolation. The integration tests keep only what they uniquely prove: that the read path is wired to the budget, and that a day of failures still re-probes. Confirmed four of these fail against a reinstated lifetime cap. * ci(windows): run the real-icacls DACL suite in CI The win32 suite only self-skips off Windows, so it passed vacuously in every lane. Register it the way the cmd-shim suite is registered. * fix(security): describe the cache's real cost, which is icacls now Both cache comments still justified themselves with PowerShell -- "~1-1.5s" and "a PowerShell spawn every read" -- in the same file whose PR removed PowerShell from this path. The caches are still right, but for different numbers, and the old ones are the kind an engineer would reasonably delete a cache over. The real shape: hardening verifies first and returns early, so an already-correct DACL costs one synchronous icacls spawn and a rewrite costs four (verify, reset, grant, verify). Still worth caching on the read path, which polls at ~2/s. * test(security): make the DACL suite safe to schedule Registering this spec in the Windows lane put it under two rules it had never been measured against. Teardown now goes through `removeTreeSync`, which the lane's boundary test requires, and repairs the DACLs the suite plants on purpose first: those retries only cover transient locks, so a regressed `(OI)(CI)(IO)` repair leaves the root un-removable and `afterAll` throws EPERM. And the no-permission case decides by elevation before it writes anything. `windows-2022` runs elevated, where hardening succeeds: the old branch asserted nothing about denial and instead replaced the `hosts` DACL, then `icacls /reset` -- which is not a restore, it drops the explicit `SYSTEM:(F)` that file ships with. Ephemeral in CI; permanent for a developer running the lane from an elevated shell. Now it asserts or it skips. The probe reads the token integrity SID rather than `icacls /save`, which succeeds unelevated (`BUILTIN\Users:(RX)` carries READ_CONTROL) and would have skipped the case on every machine. * fix(security): measure the hardening latches on a clock that cannot go backwards `mayAttemptHardening` compared wall-clock times, so any backwards step -- an NTP correction, a VM snapshot restore, a user changing the clock -- made the elapsed time negative and held every failing path below its delay until the clock caught up. Measured at the 30-minute ceiling with the clock stepped back a year, the path was refused at +0d, +1d, +30d, +180d and +364d, and re-probed only at +366d. That is the permanent latch the exponential backoff was added to remove, and it contradicts the module's own "bounds the rate without ever bounding the lifetime". The SID lookup's own one-minute window had the identical shape and is worse: a failed lookup makes `planFor` return null, which disables the synchronous *write* path too, so the write-path exemption that recovers the read-path budget cannot recover it. Both now measure elapsed monotonic time, following the repo's existing `monotonicNowMs` spelling. Two things the write path was not doing, both found in the same pass: - A successful synchronous apply now records the outcome. It is exempt from the budget, but it was also invisible to it, so a host that had demonstrably recovered kept the read path backing off for up to 30 minutes and no `recovered` transition ever came from that lane. Only success is recorded; recording failure would put the exempt lane back under the budget. - `writeSecureFile`'s JSDoc now says its boolean covers the file only. The directory harden is fire-and-forget and answers `pending` on Windows regardless, so a `true` says nothing about the directory's ACL. * fix(security): stop the hardening test doubles from faking a no-op Three CI failures on this branch, one failure shape: hardening silently does nothing and the check that should have caught it agrees. The auth critical-path test hand-rolled a `node:child_process` factory with `execFileSync`/`execFile`. The rewritten ACL path goes through `runProcessSync`, i.e. `spawnSync`, which the factory never returned — so every spawn threw into the SID lookup's bare catch, `planFor` returned null, and hardening no-opped. It mocks `child-process/run-process` now, the boundary production code actually calls and the one sibling ACL tests already mock: an export missing there fails loudly by name instead of returning undefined. Its fake icacls writes a real UTF-16LE SDDL file, so the pinned spawn count per write is a property of the ACL path rather than of the double. The test forces `platform='win32'`, so this failed on every platform, Linux CI included. `windowsSystem32Binary` is a production bug, not a test bug: it builds a Windows path with the host `join`, which off-platform yields the mixed `C:\Windows/System32/whoami.exe`. On Windows the two joins agree, which is why it survived; on Linux the SID lookup's whoami match missed and 27 of secure-file's 32 tests exercised a lane that never ran. These are always Windows paths, so `path.win32.join` is what it should have been. The import-boundary pin still read 160 after this branch migrated secure-path-windows-acl.ts off `node:child_process`; the ratchet correctly refuses a pin left above reality. * fix(security): resolve the machine-relative SDDL alias, and stop a denied read destroying the file Path hardening verified the DACL it wrote by comparing the SIDs `icacls /save` reports. SDDL substitutes two-letter aliases for well-known SIDs, and the resolution table could only hold constants -- but `LA` and `LG` name an account by RID inside the *machine's own* SID, so on a box whose user is the built-in Administrator (a CI runner, an Administrator-only install) the current user read back as `LA`, matched nothing, and hardening reported failure for every path. Resolve those two against the machine authority derived from the user SID; without one they stay unresolved and the comparison still fails closed. Three secret stores treated any read failure as "malformed -- regenerate" and overwrote. A hardened file granting a SID this process does not hold reads as EPERM while its directory stays writable, so the overwrite succeeds: renaming over an unreadable file needs FILE_DELETE_CHILD on the parent, not DELETE on the file. That destroyed the E2EE secret key, every paired device's bearer token, and the plugin vault. Distinguish EPERM/EACCES from a parse failure and refuse. Also close the async lane's unhandled rejection: `void p.then(onSettled)` turned a throw from `onSettled` into a dead main process, and the retry budget it calls threw whenever nothing had configured it -- a contract held only by import order. The budget now defaults its own bounds. * test(windows): say which ACEs icacls listed when a planted DACL fails `toHaveLength` reports only a count and vitest elides the array, so three preconditions failing on the CI runner said "expected 3, got 6" and nothing about what the sixth entry was. Name the entries in the failure. * fix(security): stop three more stores overwriting what they were denied Same swallow-default-overwrite shape as the readers already fixed, found by sweeping every store that reads under a hardened root. - plugin-storage-store.ts returned `{}` on any read failure and set()/delete() wrote it back, losing the plugin KV store. It is the secrets store's shape line for line, so the two now behave identically. - relay-revoke-outbox.ts returned [] and save() wrote it, dropping revocations that never reached the relay -- a revoked device stays live. - profile-cloud-session-store.ts mapped an EPERM read onto `decrypt-failed`, which fails the `status === 'found'` guard in clearCloudSessionIfUnchanged and falls through to an rmSync of the account session. A denied read now reports `unreadable`, which licenses nothing; the refresh path bails on it and the auth status surfaces it rather than reporting a bare reconnect. All reuse isPermissionDeniedError. The predicate stays an EPERM/EACCES allow list rather than "ENOENT defaults, everything else throws": these stores are meant to self-heal a truncated or malformed file, and inverting it would turn a corrupt keypair into an app that cannot start. The distinction that matters is "could not read it" versus "read it and it was garbage". * test(windows): plant fixture DACLs that cannot inherit what they did not plant %TEMP% grants [SYSTEM, Administrators, <user>] (OI)(CI)(F) by default, and those propagate into every fixture. Three preconditions read back 4 and 6 ACEs where 3 were planted, and the extras looked like Orca's own hardening because the shape is identical -- on a runner whose user is the built-in Administrator, the inherited trio IS the trio production grants. Combining /inheritance:r with /grant:r leaves the argument order to icacls, and that combined form drops the inherited ACEs on Windows 11 but keeps them as explicit ones on the Windows Server runner. Removing inheritance in its own invocation makes the grant the whole DACL on either host, and the fixture root is de-inherited once up front so nothing propagates in. Rooting the fixtures outside %TEMP% would not have fixed this: any directory inherits from wherever it lives. The fix is to stop inheriting, not to move. No assertion is relaxed -- the counts stay exact. * test(windows): pick a foreign SID that stays foreign on an elevated runner `S-1-5-32-544` is only foreign to a token that is not an administrator. The CI runner is elevated AND logged in as the built-in Administrator, so granting Administrators granted the reader full control: the file stayed readable, and all six preservation assertions went vacuous rather than proving anything. BUILTIN\Guests is resolvable everywhere and no interactive token is a member, so the read is denied on an unelevated developer box and on the runner alike. An unresolvable SID would have been the stronger choice but icacls rejects one with ERROR_NONE_MAPPED (1332). The premise guard is what caught this -- it asserted the file was actually unreadable instead of trusting the grant, and named elevation as the suspect. * fix(security): refuse on any read that never reached the contents, not just a denied one isPermissionDeniedError becomes isUnreadableError, because "permission denied" was never the concept -- "could not read it", as opposed to "read it and it was garbage", is. EBUSY, EMFILE, ENFILE and EIO say exactly as little about a file's contents as EACCES does, and they fell into the branch that regenerates and overwrites. On Windows EBUSY is the likelier of the two: antivirus holding a credential open at the moment of a startup read produces it, which makes it a commoner path to the same permanent loss than the ACL case that motivated the original fix. Still an allow list, deliberately: ENOENT keeps licensing a create, and a parse failure keeps self-healing. The stores are built to recover from a truncated write, and turning that into a refusal would trade a recoverable state for an unrecoverable one on the startup path. Also fixes the regression suite's own premise on an elevated runner: makeUnreadable combined /inheritance:r with /grant:r, and that form keeps %TEMP%'s inherited [SYSTEM, Administrators, user] as explicit ACEs on Windows Server -- so the file stayed readable and all six assertions were vacuous. Same split-the-invocation fix as the ACL suite's planter. * test(windows): skip the preservation suite where a read cannot be denied An elevated token logged in as the built-in Administrator reads straight through a DACL that grants it nothing -- confirmed on the CI runner against both BUILTIN\Administrators and BUILTIN\Guests, and with the grant split into its own icacls invocation so the DACL really was the planted one. On such a host the premise these tests rest on does not hold, and every assertion would pass while proving nothing. So probe once at module scope and skip rather than assert vacuously -- the same trade the ACL suite already makes for its unelevated-only case. The gate stays in the compound `<win32 check> && <flag>` form the win32 lane ratchet detects, so the file stays registered in both lane lists. Coverage is not lost where it counts: isUnreadableError has unit tests that run on every platform and every host, and the stores' refusal is exercised in full on any machine where a denial is reproducible -- which is every developer box. --------- Co-authored-by: Orca Worker <orca-worker@localhost> Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>	2026-09-05 21:13:06 -07:00
OrcaWinandOrca Worker	975bbdedcc	fix(windows): scan ports natively instead of encoded PowerShell (#17861) ... * fix(windows): scan ports natively instead of encoded PowerShell Microsoft Defender for Endpoint scored the relay's Windows port scan as suspicious PowerShell plus network discovery (T1049). The command line was `-ExecutionPolicy Bypass -EncodedCommand <base64>` around a Get-NetTCPConnection/Get-Process join -- base64 next to a policy override is the highest-weighted token pair on a PowerShell command line, and netstat only ever ran as its fallback. Invert the chain. `netstat.exe -ano` is now the primary reader and the owning process name comes from the shared native process table, which exists to keep PID lookups off PowerShell. The payload survives only as a last resort, and without the override: execution policy gates script files, never `-Command`, so nothing needed it (verified: `-ExecutionPolicy Restricted -Command` runs). Drop `-p tcp` while inverting: on Windows that protocol name means IPv4 only, so as a primary reader it would have hidden every `[::]` listener the payload used to report. Names arrive as `sshd.exe` from the table and are published as `sshd`, keeping the sshd filter and old clients' rendering intact. Routes both spawns through runProcess, removing the file from the child_process and windowsHide ratchets. * fix(windows): read netstat state by shape and refuse a truncated table Review of the port-scan inversion found two ways the new primary path could be silently wrong, both of which would have kept the flagged PowerShell payload running on exactly the hosts this change targets. `LISTENING` is not in netstat.exe. It lives in System32\<locale>\netstat.exe.mui and MUI selection follows the UI language, so the pinned-locale env in relay-command-env.ts cannot reach it -- a German host prints `ABHOEREN` and the word test parsed zero rows. The zero-listeners guard then read that as a blocked reader and ran `Get-NetTCPConnection` every 12-30s forever, or returned nothing at all where PowerShell is also restricted. Keep the word as the fast path and, when it finds nothing over output that did contain TCP rows, re-read by shape: only a listening socket has no peer. Measured on this host across all four states present (LISTENING 47, ESTABLISHED 49, CLOSE_WAIT 29, TIME_WAIT 213): zero non-listening rows with a zero peer, zero listening rows without one, and the same 47 rows parse after substituting the German state words. Shape stays the fallback because `BOUND` also prints a zero peer. Truncation was invisible: createOutputSink discards overflow, ProcessResult carries no flag, so a capped read still exits 0 and its head still parses. netstat orders IPv4 TCP, then IPv6 TCP, then UDP, so a host with tens of thousands of TIME_WAIT rows would have lost every `[::]` listener -- the exact loss dropping `-p tcp` exists to prevent, and one the zero-listeners guard cannot see. Refuse the read instead. A `truncated` flag on the shared sink would be cleaner and is left as a follow-up rather than widened into this PR. Also: decline to wait on the shared process table once the request is aborted (it takes no signal and must not be cancelled for other callers); note the name lookup as best-effort, since a TTL-cached snapshot can hand a recycled PID its previous owner name; log once on either fall-through, because both are permanent and invisible when wrong; and drop a stderr assertion that any PowerShell autoload banner would redden. Correcting the cost claim in the previous commit: the aggregate win holds with the native addon (netstat 21ms vs the retired payload 860ms at 532 processes), not without it. The addon is optional, the snapshot TTL is 500ms and the scan cadence is 12-30s, so a relay with no active agent pane never warms its own cache and pays ~1.4s cold on the CIM path -- slower than what it replaced. * fix(windows): log the port-scan fall-through on the relay diagnostic stream Checked where this code actually runs before trusting the log. `console.warn` did reach a file, but relayLogLine is the right call and the reasoning is worth recording. `scanWindowsListeningPorts` runs only in the detached relay daemon: relay.ts returns early for --connect and --orca-cli, so PortScanHandler is reached only through runRelayDaemon, and both launchers start it detached with a log file (POSIX `> relay.log 2>&1`, Windows `1>relay.log 2>relay.err.log` via Win32_Process.Create). installRelayLogRotation then wraps both streams into relay.log, which is the file the documented diagnostics tail reads. Verified by installing the real rotation over a temp path and reading the file back. So the line surfaced -- but untimestamped, in a log whose format exists so reconnect flaps can be correlated with the events around them (#7773). relayLogLine is that format and the relay idiom in 41 other places, and "since when has this host been stuck on PowerShell" is most of what this line is for. The test spies on process.stderr to pin the stream and the ISO stamp rather than just asserting something was called, since a fall-through logged somewhere unread is the failure being guarded against. Also fixes a comment that ended its own block early: `relay-*/relay.log` in a doc comment contains `*/`. * fix(windows): keep the dominant zero-peer state when reading a localized netstat Shape alone promoted any zero-peer TCP row, not just listeners. `BOUND` and `CLOSED` print a zero peer too, and on a localized host their state words are exactly as unreadable as the listening one -- so a German host with listeners plus one BOUND socket published a phantom listener. Reachable on an English host too: with zero listeners a lone BOUND row is promoted AND, because the result is then non-empty, it suppresses the blocked-reader fall-through. Group the zero-peer rows by state word and keep only the largest group. A transient BOUND or CLOSED socket cannot outnumber the listeners (51 against 0 on this host), so this removes the class rather than special-casing the words, which would just be the localization bug again. An exact tie keeps every tied group rather than guessing -- no worse than reading shape alone. Verified against real netstat output: injecting a BOUND row into the localized capture leaves the result identical to the English answer (47 rows, no phantom 65001). The new test has teeth -- reverting the grouping fails it and nothing else. Corrects two claims that were slightly wrong: the docblock said shape was the fallback because BOUND prints a zero peer, which described the hazard without saying it was unhandled; and a test comment said an English host "never sees a bound socket", true only when it has at least one readable LISTENING row. Also gates the fall-through log per reason instead of per module, so a host that parses nothing today and truncates tomorrow reports both faults. Same one-shot cost, and the vocabulary is two fixed strings so the set cannot grow. That guard matters more than it looks: --log-file rotates stdout only, so the file stderr can land in is unrotated. * docs(windows): note the direction the zero-peer majority rule can fail in The docblock described the tie case and stopped there, which reads as a complete account of the limits when it is not: a majority rule inverts if the majority is wrong, and enough transient zero-peer sockets would publish the phantoms and drop the real listeners. Someone would reasonably have concluded the rule was safe in both directions. Trigger numbers and the repro stay in the PR discussion; the code only needs the reader to know the rule has a direction, and the hatch (defer to the PowerShell reader, which reads the state word instead of inferring it) since that is the part a future editor would otherwise re-derive. * ci(windows): run the real-netstat port scan suite in CI The win32 suite only self-skips off Windows, so it passed vacuously in every lane. Register it the way the cmd-shim suite is registered. * test(windows): lower both child-process ratchets to the ground this PR took Migrating the port scan off `node:child_process` onto `runProcess` drops `src/relay/windows-port-scan.ts` from both allowlists, so both offender counts fall by one. Each ratchet pins the count from below as well as above, so a pin left above reality fails and re-opens room for the next direct import to land for free. * docs(windows): qualify the no-PowerShell claim on the netstat scan The scan starts no PowerShell of its own, but no released relay carries the optional `windows-process-tree.node` addon (only dev-channel-win-build.yml builds it), so the shared process-table read falls back to a CIM scan that forks one `powershell.exe`. The EDR win is the removal of the `-EncodedCommand` / `-ExecutionPolicy Bypass` shape, not the elimination of PowerShell. Comment-only. * docs(windows): record the identity-reader follow-up and the perf table's addon attachWindowsProcessNames reads only `name`, so it should move to `readWindowsProcessIdentityTable` once #17866 lands -- on that PR's detailed reader it would open per-process handles for a field it discards. The reader does not exist on this branch, so the call stays as-is with the follow-up recorded rather than pulling #17866 in. The process-table perf table's two Toolhelp32 rows assume the optional `windows-process-tree.node` addon. The desktop bundles it; no released relay does, so on an SSH host the CIM row is the operative number. Comment-only. * docs(windows): state the CIM scan as the relay's normal path, not a fallback No released relay carries the optional `windows-process-tree.node` addon -- release-cut.yml has zero references to it and only dev-channel-win-build.yml builds it -- so the PowerShell CIM scan is what every SSH host runs. The call-site docstring read as a conditional fallback standalone. Comment-only. --------- Co-authored-by: Orca Worker <orca-worker@localhost>	2026-09-05 21:12:59 -07:00
3 people	bfc6a262a7	fix(windows): read command lines from the kernel, not each process's PEB (#17886) ... * fix(windows): read command lines from the kernel, not each process's PEB MDE incident D scored Orca for suspicious memory activity: the vendored `@vscode/windows-process-tree` recovered every process's command line by opening it with `PROCESS_QUERY_INFORMATION | PROCESS_VM_READ` and chaining three `ReadProcessMemory` calls through the PEB and `RTL_USER_PROCESS_PARAMETERS`. On a 750ms/2s cadence over the whole table that is the credential-dumping primitive, whatever the intent. Windows 8.1 added `NtQueryInformationProcess`'s `ProcessCommandLineInformation` class (60), which returns the same string as a kernel-built `UNICODE_STRING` under `PROCESS_QUERY_LIMITED_INFORMATION` alone. Electron's floor is Windows 10, so every supported OS has it. The PEB reader stays behind a process-wide latch that only `STATUS_INVALID_INFO_CLASS`/`NOT_SUPPORTED`/`NOT_IMPLEMENTED` can set; a pid that merely denied a handle does not re-arm it, because `PROCESS_QUERY_INFORMATION` implicitly grants the limited right and so cannot be obtained where the weaker open already failed. The same hunk drops `PROCESS_VM_READ` from `GetProcessMemoryUsage` and `GetCpuUsage`, which acquired it and never read an address space. Measured on Windows 11 (514 processes), counted in-process by swapping the addon's import table entries for counting stubs, per CommandLine scan: `ReadProcessMemory` 1128 -> 0, desired access 0x0410 -> 0x1000, p50 12.7ms -> 9.3ms. Command lines were byte-identical on every process both readers recovered (376/376, 379/379 across runs), including a 24,068-character argv with quotes, non-ASCII and trailing whitespace, and a WOW64 target. Three processes that refused the old rights granted the new one; none went the other way. * chore(deps): refresh the windows-process-tree patch hash in the lockfile * fix(windows): drop the PEB fallback and detect the unpatched prebuilt Review of #17886 found three ways the reader could still perform, or silently resume, the primitive it exists to remove. The class-missing latch was a permanent, process-wide, one-way downgrade back to the PEB read, and any single target returning STATUS_INVALID_INFO_CLASS / NOT_SUPPORTED / NOT_IMPLEMENTED could trip it. On an EDR-hooked ntdll -- the entire premise of this change -- a hook that does not recognise class 60 would have restored PROCESS_VM_READ plus three ReadProcessMemory per pid per scan for the life of the process, unobservably, on precisely the machines this was written for. The fallback is deleted rather than guarded: GetProcessCommandLine now returns false and leaves the command line empty, which callers already handle, so the addon imports no ReadProcessMemory at all. That absence is what makes the property checkable on the artifact. The published 0.8.0 tarball ships a loadable prebuilt built from unpatched source; it is node-addon-api, so a bare require() accepts it, allowBuilds is false and CI installs with --ignore-scripts, and a rebuild that soft-exits on a Windows file lock leaves it in place. Source-text guards could never see it. windowsProcessTreeAddonReadsProcessMemory() checks the compiled binary instead, and is wired into the install check, the rebuild, and the relay build. The repair itself never worked: `git apply` run inside a work tree prefixes patch paths with the cwd-relative prefix, skips what does not match, and exits 0, so the branch always fell through to its own post-check throw. The package dir is always under the project root, while the fixture that covered it was in %TEMP%, outside any repo. Blinding git with GIT_DIR fixes it, and the test now runs inside a real work tree. Also from review: bounds-check the returned UNICODE_STRING against the allocation (not the size the second query clobbers) and cap the probe so a bogus length cannot bad_alloc a whole scan; test NT_SUCCESS explicitly; value- initialize ProcessInfo, which left `memory` as stack garbage -- measured, 82 processes reported the same bogus working set; and correct a comment in windows-process-table.ts that still described the command line as a PEB read. Re-measured on Windows 11 (543 processes): ReadProcessMemory 1128 -> 0, with the symbol absent from the import table so the IAT hook finds no slot to count; desired access 0x0410 -> 0x1000 on all 543 opens; p50 13.5 -> 12.3ms; 405/405 command lines byte-identical including a 24,087-character quoted non-ASCII argv and a WOW64 target; 3 processes recovered only by the new path, 0 only by the old. * chore(deps): refresh the windows-process-tree patch hash in the lockfile * test(scripts): stage a script's local imports into the native-runtime fixture ensure-native-runtime.mjs gained an import of windows-process-tree-gyp-rebuild.mjs, but the fixture copied only the script itself, so every case in the suite died with ERR_MODULE_NOT_FOUND before reaching its own assertions. copyScriptWithLocalModules already walks a script's co-located imports for exactly this reason -- its own doc comment names this failure -- so use it rather than listing files by hand. The two Windows cases still fail here, on a missing node-pty ConPTY runtime that also fails on main; this only stops a resolution error from standing in front of whatever they were meant to catch. * fix(windows): route a locked stale addon to the Windows file-lock message `pnpm install` with Orca running aborted with a raw EPERM stack. The stale-binary guard -- which deletes an addon that still imports ReadProcessMemory so a skipped rebuild cannot use it -- ran outside the try whose catch classifies Windows file locks, and whose message is literally "Close running Orca/Electron/dev processes for this worktree": exactly this situation. Measured rather than assumed: rmSync against a loaded (memory-mapped) addon throws EPERM, and `force: true` does not help, since it only swallows ENOENT. Cold copies of the same file delete fine. So the delete threw a page before the handler that knows what it means. Moving the guard inside the try is the whole fix; the classifier already matches the EPERM text. The new case runs the real script against a temp project whose stale addon is held open by a live child process, and fails against the old placement with the raw `syscall: 'rm'` stack the report described. * feat(windows): warn once when command-line recovery is refused host-wide Removing the PEB fallback removed a total-defeat vector, but it left a cliff: if NtQueryInformationProcess(ProcessCommandLineInformation) is refused -- a hooked ntdll that does not know class 60 -- every command line comes back empty and agent identity matching silently degrades to image names. The addon still loads and still enumerates, so every health check the app has stays green. A cliff nobody can see is the failure mode this area keeps producing. The querying process is the unambiguous probe. A process can always open itself with PROCESS_QUERY_LIMITED_INFORMATION, so its own command line coming back empty means the query is refused for every process -- not that some target denied a handle, which is normal for roughly a quarter of the table. Keying on our own row rather than a fraction means no threshold to tune and no false positive on a hardened box where most processes deny. One warning per session, gated on the CommandLine flag actually being requested so a future identity-only reader cannot trip it. The suite's own SELF fixture gains a command line for the same reason: a self row without one is the alarm, not a detail. * fix(windows): check the relay's staged addon at load, and answer tri-state Two gaps in the ReadProcessMemory check, both about what it does not see. It only ever looked at node_modules/@vscode/windows-process-tree. A relay host has no node_modules of ours: it loads ./windows-process-tree.node staged beside the bundle. The relay build asserts the symbol on the artifact it produces, but a bundle and the addon beside it redeploy independently, so a host that has not taken a new bundle keeps whatever binary is already there -- and the published prebuilt is node-addon-api, so it binds cleanly and then walks every process's address space. loadWindowsProcessTree now checks that file too and refuses it, falling back to the CIM scan: slower, but not the thing an EDR quarantines a host for. The predicate is duplicated rather than imported, because the config-script copy is install-time tooling that drags in node-gyp and child_process, and this module is bundled into the app and the relay. And it returned false for a binary that is not there. All three callers happened to be safe, but the name read as a safety predicate, so a future caller would take a missing binary as verified. inspectWindowsProcessTreeAddon() now answers clean/unpatched/missing over an explicit binary path -- which is also what lets the relay's staged addon be checked at all -- and each caller states which state it acts on. Both are covered by cases that fail against the old code: without the load-time check the unpatched staged addon is bound and the CIM fallback never runs, and with 'missing' folded back into 'clean' the absence case fails outright. * test(windows): load the addon in beforeAll, not at collection time loadAddon() ran while the file was being collected, so on a Windows checkout with no built addon the require threw before any case existed and took the seven patch-text cases down with it -- cases that read only the patch file and need no binary at all. Verified both ways against a deliberately unresolvable addon path: at collection time vitest reports "no tests" for the file; from beforeAll the seven text cases pass and only the three addon cases go. * fix(deps): normalize the windows-process-tree patch to LF and let pnpm own its hash `pnpm install --frozen-lockfile` failed on this branch on every platform with ERR_PNPM_LOCKFILE_CONFIG_MISMATCH, which breaks CI and the release build. Two coupled defects. The patch file was committed with CRLF -- 174 CR bytes, against zero on main -- and `.gitattributes` pins `/config/patches/*.patch -text` precisely so checkout cannot convert it, so those bytes reached every runner. And pnpm hashes a patch **LF-normalized**, so the raw sha256 of a CRLF file is a value pnpm never computes: raw sha256 322965470c05f63d8527f7d8e892ee26ee444136b66b57fd64c362a9f2ff05d1 LF-normalized f8ea245391c94da5770045aeea01fa6de466c2199c6ef46b5b769b398aa9823e The lockfile carried the raw one, at all three sites. It is the only one of the seven patches where the two digests differ, which is why the other six passed. Normalized the patch to LF and took pnpm's own value from `pnpm install --no-frozen-lockfile`; nothing here is hand-computed. With the file LF-only the two interpretations coincide, so the lockfile, the contract test's no-CR assertion and its hash assertion all agree at one number -- and `config/scripts/windows-process-tree-patch-contract.test.mjs`, which was red on this branch for the same reason, is green again. The lockfile diff is exactly the three hash lines. The regression check is the installer, not a digest. Two separate reviews "verified" the shipped hash by recomputing sha256(patchBytes) and matching the lockfile; both were wrong, because both repeated the same wrong assumption about which bytes pnpm hashes. A check that reproduces the original mistake is not independent. So the new case runs `pnpm install --frozen-lockfile --lockfile-only --ignore-scripts` against a copy of the manifest, lockfile and patches, and asserts exit 0 -- verified by deletion: restoring the shipped hash fails it with the exact ERR_PNPM_LOCKFILE_CONFIG_MISMATCH from the branch's package (windows) job. Also corrected the `.gitattributes` comment claiming pnpm hashes patches byte-for-byte. The `-text` setting is right -- `git apply` needs the exact bytes -- but that sentence is the claim that produced the wrong hash twice. * ci(windows): run the process-tree patch suites in CI Both suites only self-skip off Windows, so the binary-level check that the addon carries no ReadProcessMemory passed vacuously in every lane. * fix(windows): force core.autocrlf=input for the patch repair My LF normalization of the windows-process-tree patch broke the `git apply` repair path introduced in this PR. The two are coupled and I checked only one. Those 174 CR bytes were not editor noise. They sat on exactly the pre-image lines and nowhere else -- 107/107 in src/process.cc, 67/67 in src/process_commandline.cc, 0 on every added or context line -- because @vscode/windows-process-tree@0.8.0 ships those two sources as CRLF. Normalizing the patch made its pre-image stop matching the file it is applied against. Measured, reconstructing the true CRLF pre-image from the pre-normalization blob and applying the current LF patch: core.autocrlf plain -c core.autocrlf=input true exit 0 exit 0 input exit 0 exit 0 false exit 1 exit 0 `false` is Git's own built-in default and what "checkout as-is" selects in the Git for Windows installer -- on this box the `true` that hides it comes from the installer's system gitconfig, not from anything in the repo. There the repair throws, ensureWindowsProcessTreeCommandLinePatch reports "still reads the PEB, and repairing it ... failed", isWindowsNativeLockError does not match that text, and `pnpm install` dies with no path forward. Forcing the mode rather than `--ignore-whitespace`: both fix every cell and both leave the applied file fully LF, but `input` relaxes line endings only, so a hunk whose real content drifted is still rejected. The repair rewrites a security-relevant source file; it should stay strict about everything except the thing that is legitimately ambiguous. Not reverting the patch to CRLF: windows-process-tree-patch-contract.test.mjs (pre-existing on main) forbids CR bytes in it, and pnpm computes the same hash either way. LF plus the forced mode is the end state. The suite could not have caught this. The fixture built its pre-image from the patch itself and joined with '\n', so fixture and patch agreed by construction on any encoding -- once again a test that passes without its fix. It now emits the CRLF the real package ships, and the case runs under both autocrlf modes pinned through a temp HOME gitconfig, because the repair blinds git to the repo and so reads global config. Verified by deletion in both directions: with the flag removed the autocrlf=false case fails with the exact "still reads the PEB" dead end while autocrlf=true still passes, and with the fixture back on LF all eight cases pass with no fix present at all. Also corrected the .gitattributes comment I added last commit. It said `git apply` needs the bytes the patch was written against, which is now false -- the pinned bytes are LF and the bytes it was written against are CRLF. That is the same class of confident-and-wrong claim that produced the bad hash twice. * fix(windows): assert the rebuilt addon, and install the patch for real in tests Three follow-ups from review. **The packaged binary had no check.** The relay build asserts its own artifact and ensure-native-runtime asserts what it loads, but nothing looked at the addon copied into the packaged app -- so a rebuild that silently produced the upstream reader shipped. `rebuild-native-deps.mjs` now asserts `clean` on it after `rebuild()`. This is also the caller D4's tri-state was missing: every existing site branches on `=== 'unpatched'`, so `missing` still behaved exactly like `clean` everywhere, which was the thing making it a state rather than a boolean. Here both non-clean states fail, and they fail differently: after a rebuild that reported success, an absent binary is a broken build, not an absence to shrug at. The fake `rebuild()` had to start producing a binary for that to mean anything, so it now emits stand-in bytes and takes `addon: 'clean' | 'unpatched' | 'none'`. Verified by deletion: with the assertion removed both new cases pass. **The frozen-install case could not see a patch at all.** `--lockfile-only` resolves and never applies one, so its coverage stops at hash consistency. Added a case that installs `@vscode/windows-process-tree@0.8.0` for real with the patch and asserts the materialized `src/process_commandline.cc` carries the marker and no longer carries `ReadProcessMemory` -- about 1.5s for the pair. Correcting the brief on that one: it does **not** catch the `git apply` breakage from the previous commit. Measured -- with `-c core.autocrlf=input` removed it passes cleanly, because `pnpm install` uses pnpm's own patch applier and never runs our repair script. What it does catch is a patch pnpm can no longer apply: corrupting one pre-image line fails both cases. The repair path stays covered by the CRLF fixture in rebuild-native-deps-node-pty.test.mjs. Worth recording, since it decides whether the LF normalization was safe at all: pnpm applies the LF patch to the CRLF tarball sources without complaint, and materializes them as LF with the marker present and `ReadProcessMemory` absent. The primary install path was never affected -- only the `git apply` fallback was. **Dead timeout.** The frozen-install case passed `timeoutMs: 300_000` to the spawn while vitest capped the case itself at 30s, so on a cold runner vitest would have killed it first. Both cases now declare the budget they use. * test(windows): route the frozen-install check through the pnpm invocation owner The new patched-dependencies check hand-rolled a PATH walk naming 'pnpm.cmd', which the windows batch shim spawn boundary ratchet rejects: pnpm-cli-invocation already owns that decision for every other script, and its allowlist only shrinks. Reuse resolvePnpmCliInvocation for the command and prefixArgs, and the shared resolveCliCommand for the presence check, so no shim name is spelled here. Its `shell` flag is dropped because runProcessSync refuses it and already drives a shim through the interpreter itself. --------- Co-authored-by: Orca Worker <orca-worker@localhost> Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>	2026-09-05 21:12:47 -07:00
OrcaWinandOrca Worker	687a22e1ee	fix(computer-use): run the Windows runtime as one persistent helper (#17858) ... * fix(computer-use): run the Windows runtime as one persistent helper Microsoft Defender for Endpoint raised multi-stage Execution + Collection incidents against Orca on Windows ("Screenshots were taken unexpectedly on this device... Screen capture code was found in a script launched by powershell.exe", factor "Executes suspicious MSIL code"). The desktop script provider spawned a fresh powershell.exe per operation, so a single computer-use session produced a burst of short-lived PIDs and re-emitted runtime.ps1's inline Add-Type P/Invoke assembly on every click. runtime.ps1 gains a -Serve mode that loads its assemblies once and then reads NDJSON requests from stdin, and a new DesktopScriptRuntimeHost owns one long-lived child: lazy spawn, strict serialization, a 30s per-request timeout, restart on crash, a 120s idle shutdown, and dispose() on provider teardown. The one-shot -OperationPath path stays as the fallback, and Linux keeps its python3 bridge unchanged. Both Windows spawn sites now use -ExecutionPolicy RemoteSigned instead of Bypass, falling back once to Bypass (and logging) when a Restricted host refuses the unsigned script. * fix(computer-use): recover the runtime host instead of latching it off Review follow-up on the persistent Windows computer-use helper. A helper that died before producing a line set an unavailable flag nothing ever cleared, and the client then dropped the host for the life of the session. One transient bad spawn — a Defender scan, a locked CSC temp directory — silently restored the per-click powershell.exe burst and per-operation MSIL emission this work exists to remove, with computer use still working so nothing looked wrong. Start failures are now retried, then cool down for 60s, then re-probed; the client keeps the host so it can come back. Repeated post-answer crashes cool down too, and a single reply no longer clears the failure count. The one-shot bridge decided its execution-policy retry from a message that fell back to stdout, so a window title containing "SecurityError" could replay a non-idempotent operation — a double click, keystroke or paste — and stick the session on Bypass. The retry now requires empty stdout and a matching stderr. Serve-mode replies carry an echoed request id. Without one a single stray stdout line would make every later response answer the previous request, acting on stale element indexes with no error raised; a mismatch now kills the child. Non-JSON noise is ignored rather than counted as the helper having answered. Also: warnings reach the main process over the sidecar's IPC channel rather than its piped, unread stdio; the child is watched on close rather than exit; dispose latches so a queued request cannot respawn during teardown; and the host is split into a serve channel and an availability policy to stay under max-lines. * fix(computer-use): prove a helper never started before replaying its request The retry that replaced the permanent-latch bug could deliver unrequested input. send() re-sent the same request whenever the helper died without replying, but "no reply came back" is not "the operation did not run": runtime.ps1 synthesizes the click and only then builds the snapshot, which allocates a full-window bitmap and walks the UIA tree — a native GDI+/UIA fault there is uncatchable, and leaves the click already delivered. A deterministic fault meant three clicks from the host plus a fourth from the one-shot bridge, surfaced as a single failed operation. -Serve now writes one {"ready":true} line after its Add-Type work and before its first read, so "never started" is a fact rather than an inference. A request is replayed only when the helper died before announcing. A runtime.ps1 that predates the announcement — reachable through the provider path override — is covered by an observation-tool allowlist until a ready line proves otherwise. Host-detected aborts (timeout, desynchronised reply, oversized line) suppress the exit handler, so they were bypassing failure accounting entirely and a helper failing that way was respawned once per operation forever. They now count and are logged. Also stop charging twice for one outage: entering the cooldown resets the failure count, so the first death after recovery no longer re-enters a full cooldown and an interleaved workload cannot be stranded on the one-shot bridge. * fix(computer-use): ignore a stdin write callback from a torn-down helper stop() destroys stdin, so a write still queued at teardown calls back with ERR_STREAM_DESTROYED. The callback carried no channel or request identity and write() had no closed guard, so it ran abortChannel a second time: stopChannel no-opped but recordFailure and the warning did not, charging two failures for one operation and reaching the 3-strike cooldown at half the intended rate. That feeds the same accounting that keeps a persistently broken helper from respawning once per operation. The same root also allowed a late callback landing after a replacement channel existed to stop that channel and reject a different request with the previous one's error. Node fires the destroyed-stream callback on the next tick, well before a new request arrives, so the double-count is the reachable effect; binding the callback closes both. write() now drops payloads and error reports once closed, and the host ignores any report whose channel or request id is no longer current. * test(computer-use): pin each stale-write guard independently The channel's closed guard and the host's request-identity check are redundant by design, and the existing tests only failed when both were absent. Someone deleting one, believing the other was the covered one, would have got a green suite and a live regression — the same shape as a test that passes without the fix it was written for. Each is now pinned on its own. The channel's half is tested against the channel directly: after stop() it takes no writes and reports no error from one already queued, which the host cannot observe because it drops the channel at the same moment. The host's half is pinned by the case the channel cannot see — a live channel whose request was already answered, where backpressure delivers a write callback for a request that is no longer pending. Removing either guard alone now fails a test. Both carry a comment saying they are deliberately redundant and separately pinned, so the next reader does not have to rediscover this from the diff. * ci(windows): run the computer-use runtime host suite in CI The win32 suite only self-skips off Windows, so it passed vacuously in every lane. Register it the way the cmd-shim suite is registered. * fix(computer-use): time the runtime host cooldown on a monotonic clock The start-failure cooldown was a wall-clock deadline, so a backwards step — an NTP correction, a VM snapshot restore, a user changing the clock — left `remainingCooldown()` returning the cooldown plus the whole step. A one-hour step measured 3,660,000ms, and ten real minutes later still 3,060,000ms. Nothing shortens it from there. Only `recordSuccess()` clears the cooldown on a non-dispose path, and no request can reach a helper to succeed while it holds, so every `send()` throws `runtime_host_unavailable` first. The host is built with no `now` override and its lifecycle is a module-level singleton that shuts down at process exit, so the latch held for the sidecar's life — computer use kept working via the one-shot bridge while the per-click powershell.exe burst this host exists to remove came back silently. Store the instant the cooldown began and compare elapsed monotonic time, following the two fixes in #17884. The field is `number | null` rather than sentinel 0 because `performance.now()` legitimately returns 0. Both new tests leave `now` unset, because the bug was in the default the host picks and a test that injects a clock cannot see it. * fix(computer-use): give a queued request its own deadline The 30s request timeout was armed only in `sendOnce`, once a request reached a helper. A request behind N timing-out ones therefore waited roughly N times that with no deadline of its own: bounded, but the caller sees an `await` that looks hung for minutes and gets no error to act on. Move the serialization tail into its own class and arm a deadline at enqueue time. Only the wait is bounded — a request that reaches a helper still gets its full execution budget, so nothing that used to succeed now fails. An expired request is dropped rather than sent late: the caller has already been told it failed, and a click delivered after that is worse than no click. The tail keeps its never-rejecting shape and chains on the turn rather than on the raced promise, so a caller giving up early cannot release the next request while its predecessor is still in flight. * fix(computer-use): stop reading a locked file as an execution policy block `UnauthorizedAccess` is the FullyQualifiedErrorId PowerShell reports for a policy block, and it is also a strict prefix of `UnauthorizedAccessException`, which .NET raises for any ordinary locked or ACL-denied file. The predicate matched the token unanchored, so an AV scan holding runtime.ps1 or a locked CSC temp directory was read as a policy block. Two consequences, both bad. `escalateExecutionPolicy()` has no path back, so one false match spent the rest of the session on `-ExecutionPolicy Bypass` — the exact command line token this stack exists to stop emitting. And on the one-shot path `isPolicyBlockedStart` re-runs the operation: one-shot mode writes stdout only after the operation returns, so a crash partway through an action is indistinguishable from a helper that never started, and the click lands twice. Measured on Windows against all three records, which the test carries verbatim as fixtures: policy/Restricted FullyQualifiedErrorId: UnauthorizedAccess policy/RemoteSigned FullyQualifiedErrorId: UnauthorizedAccess genuine access denied FullyQualifiedErrorId: UnauthorizedAccessException `\b` is the whole discriminator: between `s` and `E` both sides are word characters, so no boundary exists there and the exception cannot match. Dropped two alternatives that measurement showed were wrong. `PSSecurityException` never appears — the record surfaces through a native-command wrapper and reports `ParentContainsErrorRecordException`. The prose is wrong three times over: it differs by policy, it is localized, and PowerShell hard-wraps it mid-sentence. Anchoring on the `FullyQualifiedErrorId:`/`CategoryInfo:` labels would be more precise again, but those labels are localized where the values are not, so it would lose a real block on a non-English host and strand it with no fallback. Matching the values with word boundaries keeps both directions; a fixture with translated labels pins it. The escalation stays sticky. With the predicate correct, it only fires on a machine that really does block, where re-probing the preferred policy would buy a guaranteed failed spawn per operation. * fix(computer-use): route a malformed request back to the request that caused it `ConvertFrom-Json` throws before `$requestId` is read, so the serve loop answered an unparseable request with an untagged error. On the client that is not an error at all: `deliver()` sees no matching id, calls `abortChannel`, kills the helper and charges a failure — and the helper's own message is discarded. A parse failure was reported as a stream desync with no trace of the real cause, and three of them walked into the 60s cooldown behind three misleading "did not match" messages. Recover the id from the raw line when the parse fails. No wire change: the response shape is untouched and `BridgeResponse.requestId` already documents this echo. It is the same shape the helper already returns for `not_a_tool`, where the id survives because it is read before the operation runs. Both mixed pairings degrade safely — a new script with an old client resolves the error normally, and an old script with a new client still aborts, but now reports what the helper said. When the line is mangled past recovering an id, the desync abort is the honest outcome, so keep it and carry the helper's text into it rather than replacing it. A line the helper could not tag is usually the only account of the cause. Proven against the real `runtime.ps1 -Serve`: the host can only write well-formed JSON, so the parse-failure branch is unreachable through it and the test drives the channel directly. * fix(computer-use): keep the Bypass escalation only when Bypass actually works AppLocker and WDAC constrained language mode raise PSSecurityException under the same SecurityError category a real execution-policy block uses, so the predicate matches them - correctly, on the evidence available. But those block the script at parse time, which `-ExecutionPolicy Bypass` cannot lift. The escalation was sticky unconditionally, so on a WDAC host we misdiagnosed, retried, failed again, and then latched: every later command line carried the most heavily weighted MDE token there is, on exactly the hardened, monitored enterprise machine that is watching for it. Treat the escalation as the diagnosis it is. A fallback that cannot start a helper either disproves it - the policy was not what stopped the first attempt - so revert to RemoteSigned instead of latching. When Bypass does start a helper the diagnosis is confirmed and it stays sticky exactly as before, so a genuinely Restricted machine still never pays a re-probe per operation. The revert lands inside the outage rather than only at its end, so a misdiagnosis costs one Bypass command line instead of one per attempt, and an escalation that never proved itself does not outlive the cooldown that ends the outage. Deliberately not a permanent "fallback is useless" flag: a Bypass attempt that failed for a transient reason would then disable the fallback for the session, which is the same latch in the other direction. Only `runtime_host_unavailable` proves no helper started, so only that reverts; a helper that started and then died proves Bypass works. That also makes the policy branch reachable on a final attempt for the first time, so it now rejects as unavailable rather than a generic error - that code is what routes the operation to the one-shot bridge, which carries its own policy fallback, and without it an all-blocked host would fail operations outright instead of degrading. The pre-existing "reports itself unavailable when Bypass is also refused" test pins that. --------- Co-authored-by: Orca Worker <orca-worker@localhost>	2026-09-05 21:12:33 -07:00
OrcaWinandOrca Worker	fba90e017c	fix(windows): copy the daemon host exe verbatim instead of renaming it (MDE T1036) (#17865) ... * docs(windows): document the EDR signal surface Six Microsoft Defender for Endpoint incidents fired against Orca 1.4.192 in eight days on one enterprise Windows 11 / Intune tenant. All six were behavioural process-tree scoring, not signature hits; two escalated to multi-stage incidents mapped to ATT&CK Execution and Collection. Add a reference doc mapping each attack-technique-shaped behaviour to the code that produces it and to why it exists: the renamed daemon image (T1036), the per-process PEB read, encoded policy-bypassed PowerShell (T1049), caret-escaped cmd.exe lines, and computer-use screen capture plus runtime-compiled MSIL (T1113). Records that signing is not the gate -- reputation is signer plus hash-keyed prevalence -- and carries the two evidence gaps the report noted. Adds an engineer checklist, deployment guidance for admins (AV path exclusions do not suppress EDR behavioural alerts; an MDE alert suppression rule does), and an explicit pre-deployment warning about computer use. * docs(windows): correct the PowerShell flag inventory and admin paths Review corrections to the EDR posture doc. The "encoded, policy-bypassing PowerShell" list conflated three different shapes and was incomplete. Split it into the three tiers an EDR actually scores differently -- bypass plus encoding, encoding alone, and bypass alone -- and add the sites it missed, including windows-mobile-firewall.ts, which encodes a script and launches it elevated through Start-Process -Verb RunAs. system-fonts.ts (-Command) and desktop-script-provider-bridge.ts (-File) were listed as encoded and are not. Notes that a raw grep under-reports, because the hook sites reach -EncodedCommand through wrapWindowsPowerShellEncodedCommand. Attribute the in-payload Set-ExecutionPolicy move to #16576 rather than to #16003's measurement, which keyed on -WindowStyle Hidden + -EncodedCommand, and record that the launcher's own tradeoff is unverified on a real box. Admin guidance was missing two ways a suppression rule pinned to one full path misses real activity: the .staging-<hex> sibling that exists mid-update, which is when the update-cluster incidents fire, and the userData fallback when LOCALAPPDATA is unset. Also: state the measurement conditions on the process-table timings, note that Hermes has surface even though we have no telemetry for it, note that the uninstaller names are electron-builder-generated and in no repo file, drop a volatile line count, and mark the per-operation computer-use shape as being addressed by an unmerged change. Drops the duplicated AGENTS.md section, keeping the indexed bullet. * docs(windows): reconcile the EDR posture doc with the shipped remediation Three claims in this doc became false once the rest of the Windows EDR set landed, and two told engineers the opposite of what the release does. The process-table section still described one shared snapshot taken with `Memory | CommandLine | CreationTime`, argued that splitting the cache per field set "would restore exactly the fan-out it exists to prevent", and concluded the shape was unfixable because "the information is only in the PEB". The split shipped (identity opens no handle at all), `Memory` is retired, and the command line now comes from the kernel through `ProcessCommandLineInformation` -- `ReadProcessMemory` is absent from the compiled addon and a ratchet asserts it against the import table. An engineer reading the old text would have concluded both fixes were dead ends. The PowerShell site inventories were stale in three of four lists: the port scan went native, every `-ExecutionPolicy Bypass` + `-EncodedCommand` pair was dropped as a measured no-op, and of the unencoded-bypass list only `wsl-cli-scripts.ts` survives. Regenerated against the merged tree, including the sites that reach the flag through `wrapWindowsPowerShellEncodedCommand` and never spell it, which a raw `rg` misses. Incident-evidence sections are left alone: they record what the tenant observed on 1.4.192, not what the code does now. * fix(windows): copy the daemon host exe verbatim instead of renaming it Microsoft Defender for Endpoint flagged `orca-terminal-daemon.exe` as MITRE T1036 (Masquerading): Orca copied its own `Orca.exe` into %LOCALAPPDATA% under a different name, specifically so the NSIS updater's `taskkill /IM Orca.exe` could not match, then ran it detached. Because that process is what every other flagged action was attributed to, the name mismatch acted as a reputation multiplier on unrelated findings. The rename was never what made the daemon survive. In app-builder-lib 26.15.3 the installer's FIND_PROCESS/KILL_PROCESS select processes whose image path is under $INSTDIR; `taskkill /IM` is only the fallback for hosts where PowerShell is missing or blocked. Survival is a property of the path, and %LOCALAPPDATA%\Orca\daemon-host is outside $INSTDIR whatever the file is called. Derive the host exe name from process.execPath so the copy is byte-for-byte, name included — it keeps its Authenticode signature and carries no renamed-image signal. On the no-PowerShell fallback the daemon is now killed with the app and terminals cold-restore, which is the documented pre-relocation outcome the update harness already asserts, not a regression. The uninstall macro no longer needs a distinct name to find the daemon; it kills the app's own image name (plus the legacy name, for hosts left by older builds). Adds docs/reference/windows-daemon-host-relocation.md with the survival contract, the rejected alternatives and their measured costs, and the invariants to keep. * fix(windows): apply daemon-host relocation review corrections Scope the uninstall taskkill to the current user with `/FI "USERNAME eq %USERNAME%"` via cmd.exe, matching upstream's per-user KILL_PROCESS — without it an elevated machine-wide uninstall reaches another logged-on user's session, so the "no collateral" claim in the comment was overstated. Comment the rmSync-before-publish: Windows refuses to delete a running image, so a live daemon already hosted in this version's dir (same-version reinstall, or a dev channel reusing a version) throws and materialization fails open. Doc corrections: - The fallback selector is the full per-user `taskkill /F /IM "<app>.exe" /FI "PID ne $pid" /FI "USERNAME eq %USERNAME%"`, not a bare `taskkill /IM`. - The probe reads `Get-ExecutionPolicy -Scope Process`, not the effective policy, and GPO writes MachinePolicy/UserPolicy — so GPO-managed hosts take the primary path-scoped branch. Narrow the fallback triggers accordingly. - Drop the Authenticode sentence: the old name was equally byte-identical and equally signed, so a filename has no bearing on signature validity. - Name the new update-abort path: the daemon now matches FIND_PROCESS, so on the fallback branch an unkillable host reaches the retry loop's MessageBox /SD IDCANCEL and Quits, aborting a silent update. - Correct the customCheckAppRunning rejection. It is ~6 lines, not a rewrite; it is wrong because forcing the PowerShell branch where PowerShell is absent makes FIND/KILL silently no-op and leaves the real app running with files in use. - Bound the win honestly: OriginalFilename is empty on the shipped binary, so the strongest T1036 indicator never fired, and the residual copy-and-run-detached shape still maps to T1036.005. Reconcile docs/reference/windows-edr-posture.md, which documents the rename as a live finding and would otherwise contradict this change. Content-only edit: markdown under docs/reference/ is not oxfmt-formatted as a matter of practice and nothing in CI gates it, so the file is left consistent with its neighbours. * fix(windows): expand USERNAME in NSIS instead of spawning cmd.exe The uninstall macro routed both taskkills through `"$SYSDIR\cmd.exe" /C` purely so `%USERNAME%` would expand — two extra interpreter spawns on the uninstall path, in a change whose whole point is not adding scored behaviour, and the exact `cmd.exe /c` shape the new AGENTS.md EDR bullet warns about. NSIS reads the variable itself with ReadEnvStr, so the spawns buy nothing. Verified on Windows 11 that the generated command line does what the filter is there for: a copy of cmd.exe running as orca-nonexistent-probe.exe (pid 34244) was terminated by `taskkill /F /IM "orca-nonexistent-probe.exe" /FI "USERNAME eq <user>"` — SUCCESS, exit 0, process gone. Guarded on an empty USERNAME because the degenerate case is silent: taskkill rejects an empty filter value outright ("The search filter cannot be recognized") and kills nothing, which would leave exactly the orphaned daemon this macro exists to reap. `*` is rejected as a filter value too, so there is no branchless spelling. With no USERNAME to scope by it kills unfiltered, as the macro did before the filter was added. Stack stays balanced: three pushes, two nsExec pops, three restores. Also strike the last stale row in windows-edr-posture.md's remediation table. "Copying our own image under a different name" read as outstanding work; it is done by this change, so the row now points at the relocation doc. Same class of staleness as the section reconciled in the previous commit, and git would not have flagged it either. * fix(windows): port the daemon-host uninstall sweep into the live NSIS include The uninstall macro this branch rewrote lived in config/nsis/daemon-host-uninstall.nsh, which main no longer includes: #17906 consolidated every Windows installer hook into config/nsis/orca-installer-hooks.nsh because electron-builder accepts exactly one `nsis.include`. Merged as-is, the rewritten macro would have been dead code while the shipped uninstaller kept running main's stale sweep — `taskkill /F /IM orca-terminal-daemon.exe`, which matches nothing now that the relocated host is a verbatim Orca.exe copy. The RMDir that follows then cannot delete the running image, so a live orphaned daemon and its ~224 MB tree would survive every uninstall. Ported into the live include: the ${APP_EXECUTABLE_FILENAME} kill, the USERNAME filter that keeps an elevated machine-wide uninstall out of another logged-on user's session, and the register save/restore around both. The legacy orca-terminal-daemon.exe kill stays so hosts left by older builds are still reaped. The ratchet that was meant to catch exactly this pinned only the legacy image name, which main's stale macro already satisfied, so it passed both ways. It now asserts the app-exe kill and the USERNAME filter, against comment-stripped script — the prose above the macro names both image names, so a toContain over the raw file proves nothing. --------- Co-authored-by: Orca Worker <orca-worker@localhost>	2026-09-05 21:11:28 -07:00
Neil	8ed81ceb8d	perf(tabs): index saved tab order during hydration repair (#18964)	2026-09-05 20:04:11 -07:00
Neil	56626e7daa	perf(ssh): reuse and release relay startup buffers (#18953) ... * perf(ssh): reuse the searched relay startup prefix * perf(ssh): release startup banners after relay readiness	2026-09-05 20:03:58 -07:00
Neil	bf073b833e	perf(skills): skip symlink probes beyond discovery depth (#18937)	2026-09-05 20:03:37 -07:00
Neil	6f28e019b5	perf(hooks): use native reverse search for transcript lines (#18936)	2026-09-05 20:03:33 -07:00
Neil	9b76ff9217	perf(explorer): avoid redundant dotfile path filtering (#18929)	2026-09-05 20:03:24 -07:00
Neil	fb7b75d55d	perf(cli): skip feature formatters during help and error startup (#18923) ... * perf(cli): load error reporting without feature formatters * test(cli): follow extracted error reporter in import guard * chore(cli): track cli-error.ts in deferral equivalence baseline The equivalence script restores TOUCHED files from the baseline rev to rebuild the pre-deferral CLI. reportCliError/formatCliError moved from format.ts into cli-error.ts, so the baseline arm must also drop cli-error.ts (absent at older revs) or the old tree would still compile against the new module.	2026-09-05 20:03:19 -07:00
Neil	388e9fb776	perf: avoid rescanning emitted source in analysis guards (#18920)	2026-09-05 20:03:14 -07:00
Neil	4e8e14424d	perf: avoid splitting every path during file autocomplete (#18919)	2026-09-05 20:03:08 -07:00
Neil	4204bdf717	perf: avoid repeated Quick Open exclusion string allocations (#18916)	2026-09-05 20:02:57 -07:00
Neil	e9d9d42ccc	perf: find mobile Markdown placeholder prefixes in one scan (#18914) ... * perf: find mobile Markdown placeholder prefixes in one scan * style: format mobile Markdown benchmark guard * docs: explain collision-free Markdown prefix length	2026-09-05 20:02:53 -07:00
Neil	a07d8fe19c	perf: avoid file-to-file scans when selecting deletion roots (#18913)	2026-09-05 20:02:48 -07:00
Neil	7b44f3c0e3	perf: decode fragmented CLI replies without repeated scans (#18909) ... * perf: decode fragmented CLI replies without rescanning accumulated text * bench: require an explicit CLI framing baseline	2026-09-05 20:02:44 -07:00
Neil	926f3ff585	perf(browser): assemble fragmented tunnel frames once (#18893)	2026-09-05 20:02:30 -07:00
Neil	bf87b1290f	perf(repos): avoid quadratic icon source scans (#18892) ... * perf(repos): avoid quadratic icon source scans * perf: avoid repeated malformed HTML icon scans * bench: balance icon parser timing samples	2026-09-05 20:02:25 -07:00
Neil	1c41d59203	perf(relay): drain fragmented frame buffers in linear time (#18891) ... * perf(relay): drain fragmented frame buffers in linear time * style: follow block-body lint in relay buffer checks	2026-09-05 20:02:20 -07:00
Neil	ef3f507903	ci: verify release ref trust and preserve case twins during checkout (#18980)	2026-09-05 18:38:45 -07:00
Neil	6031c19e9f	ci: reduce dependency, checkout, and test deadline overhead (#18968) ... * ci: reduce dependency, checkout, and test deadline overhead * ci: avoid generic E2E jobs for native-only IME changes	2026-09-05 18:20:12 -07:00
Neil	b852aa74a6	fix(ci): store vetted refs in a reftable so case-twin branches don't fail the fetch (#18970) ... The adhoc mac and dev-channel Windows builds vet the requested ref by mirroring every branch and tag of this repo into a scratch bare repo and proving the commit is reachable. Both runner disks are case-insensitive, and the repo now has two branches differing only in casing, so the files backend refuses the fetch outright — the whole job dies before checkout. reftable keys refs in a table rather than as file paths, so both refs store and every ref stays in the reachability set.	2026-09-05 18:03:01 -07:00
OrcaWinandOrca Worker	b6ca8dad99	fix(hooks): register the Claude hook script directly on Windows (#18875) (#18905) ... * fix(hooks): register the Claude hook script directly on Windows (#18875) The Windows Claude Code lifecycle hook was registered as `powershell.exe -NoProfile -EncodedCommand <...>` whose entire decoded payload was a `Test-Path` and a call to `~/.orca/agent-hooks/claude-hook.cmd`. Every hook event paid a full PowerShell start-up to reach a script that exits at its first `ORCA_PANE_KEY` guard, so sessions outside Orca paid it to do nothing. Register the script path itself instead, with `|| echo {}` for the neutral-JSON-when-missing contract (#14818). Measured on Windows 11, invoked as Claude Code invokes it (`printf payload | bash -c -l "<command>"`): idle (n=12) baseline 177ms | before 471ms | after 213ms 10-way conc (n=40) -- | before 656ms | after 296ms p95 under load -- | before 696ms | after 337ms It also drops an interpreter from the chain the hook's timeout kill must tear down. Killing the hook does not kill its PowerShell grandchild, which still holds the stdout handle the agent reads to EOF -- measured, EOF arrived 352ms AFTER the kill, when the orphan exited by itself. msys2 creates children suspended and resumes them after, so a kill landing in that window strands one that never exits and EOF never comes; that is the reported frozen session. The encoded launcher stays as the fallback for profile paths the shells cannot carry bare (space, `%`, `^`, `&`, non-ASCII) and for hosts where Git Bash is not resolvable, because PowerShell 5.1 rejects `||`. Every other agent's hook is untouched, as is the remote/SSH path. Not adopted from the report: `cmd.exe /d /c <path>` (MSYS rewrites the `/c` under Git Bash -- measured, the invocation fails), and raising the 10s timeout (the orphan survives the kill regardless; the fast path puts the hook 30x under the budget so the kill effectively stops firing). * fix(build): list the new hook launcher modules in the CLI tsconfig project config/tsconfig.cli.json enumerates its files explicitly, so the two new imports reached by src/main/claude/hook-settings.ts failed tc:cli with TS6307. src/main/git-bash.ts pulls in only node:fs, node:path and a shared constant, so it adds nothing heavy to the CLI project. * fix(hooks): address review of the direct Windows Claude hook launcher - Make the Windows hook suites host-independent. A box with a cmd.exe AutoRun (HKCU\...\Command Processor\AutoRun) failed them at HEAD too: the tests redirect USERPROFILE, the AutoRun target vanishes, and MSYS spawns a .cmd without /d so AutoRun runs and lands on the hook's stderr. Seed an empty target, including under the deliberately-absent profile. - Note in managed-hook-stdin-lifecycle why the "missing managed script" case no longer exercises the fallback for the direct shape (it carries an absolute path, so a redirected profile changes nothing); that path is covered live in windows-direct-cmd-hook-command.test.ts. - Keep the direct shape off UNC profiles: WINDOWS_CMD_SAFE_PATH admits them, but //server/share/... is not a command cmd.exe reliably starts. - Correct the comments: `|| echo {}` also fires when cmd.exe itself exits non-zero (failing AutoRun), printing {} twice. The encoded launcher exited 1 on that same box, so neither shape is clean there. - Test the contract that replaced runtime %USERPROFILE% resolution (STA-3348): a stale absolute path reports not_installed and is rewritten on install. - Record the standing unmeasured assumption in windows-edr-posture.md: `||` does not parse in Windows PowerShell 5.1, so a compat consumer that hosts hook strings there would fail closed. Measure before widening to another agent. - Trim the launcher comments per AGENTS.md; the numbers live in the doc. * test(win32): register the new Windows-gated hook test in the CI lane win32-test-lane-registration guards against exactly this: a Windows-gated file that self-skips on ubuntu and reports success, so it runs on no machine. The new windows-direct-cmd-hook-command.test.ts needs both entries — WINDOWS_PACKAGE_TESTS decides whether package_windows runs for a diff, and the workflow argv decides whether the file runs once that job started. * test(win32): remove the hook temp tree through the retrying helper windows-lane-tree-removal-boundary scans exactly the specs in the Windows CI lane, so registering windows-direct-cmd-hook-command.test.ts subjected it to the rule: cmd.exe and bash have just exited in that tree, and a raw recursive rm throws EPERM on Windows while their handles drain, turning a green spec into a lane failure. Use removeTreeSync, which carries the repo's maxRetries policy. --------- Co-authored-by: Orca Worker <orca-worker@localhost>	2026-09-05 17:50:33 -07:00
Neil	9f0054d89c	ci: skip idle Mac allocations and redundant native compiler setup (#18954) ... * ci: avoid idle Mac allocations and cached native toolchain installs * test: anchor artifact fixtures before their fixed expiry	2026-09-05 17:35:20 -07:00
Neil	7bb54cc2f7	ci: reduce runner overhead and disposable package compression (#18948) ... * ci: reduce PR runner overhead and package compression time * ci: validate mobile when its dependency action changes	2026-09-05 16:56:57 -07:00
Neil	a730becd7a	fix(automation): keep explicit background launches off screen (#18898)	2026-09-05 16:12:52 -07:00
Neil	1924c8f5b1	feat(perf): lint repeated sort setup and schedule regression contracts (#18822) ... * feat(perf): audit comparator setup and schedule performance contracts * test(sqlite): close readers after expected busy failures * ci(perf): trigger contract workflow on the contract files themselves Without these paths a contract rename lands green on PR CI and only breaks the next nightly, where nobody owns the failure. Also run the OS-independent source audit once instead of on all three runners.	2026-09-05 13:56:06 -07:00
Neil	9c3d957ae3	perf(automations): reuse collation setup for list name sorting (#18823)	2026-09-05 13:19:25 -07:00
Neil	4c5077d57a	perf(persistence): skip rewriting unchanged terminal scrollback snapshots (#18764) ... * perf(terminal): tighten the partial-escape-tail benchmark and equivalence test * perf(terminal): spell the ESC gate the same way as the sibling ingest gates * test(terminal): differential-fuzz the ESC-free partial-escape-tail gate against the unguarded fold * test(terminal): make the escape-tail fuzz exhaustive at symbol depth, and cap the fold expectation Two review findings on the differential fuzz, both about the test faithfully modelling the function it guards. The odometer generated strings by symbol depth but the caller filtered on `chunk.length`, which is the UTF-16 code-unit count. An astral symbol is two code units, so every depth-4 string containing one was silently skipped and the corpus was not exhaustive at depth 4 the way the test name claimed. The generator now yields `{ depth, text }` and the caller filters on depth. That restores the missing strings and takes the pinned corpus from 516,566 to 593,468 - exactly the count CodeRabbit derived for the intended corpus. The pairing assertion in the sibling suite compared the capped `advancePartialEscapeTail` against an uncapped `extractPartialEscapeTail(pending + chunk)`. It passed only because no pairing in that corpus crosses MAX_PARTIAL_ESCAPE_TAIL_LENGTH; it would have stopped modelling the function the moment one did. The cap now lives in the expectation, matching the fuzz oracle. Re-verified the fuzz still fails on a wrong guard: mutating the gate to a bracket check fails all four tests with a `gate diverged` assertion on a lone ESC chunk. Reported by CodeRabbit and pullfrog on #18748.	2026-09-05 00:34:36 -07:00
Neil	320f4c8b87	perf(renderer): index four projections that rescanned their inputs per keystroke (#18747) ... Four renderer projections scanned a collection inside a loop over another collection, each on a path that reruns per keystroke or per store write. All four now build the index once per stable input, which is what the surrounding code already does for its other lookups. `workspace-kanban-search.ts` called `searchWorktrees`, the convenience wrapper that builds the palette document index inline. The board's filter hook memoized the whole call on the query, so every character re-normalized and re-segmented every indexed field of every worktree — and did it again on every agent-status tick while a query was active, since those churn board identities. `buildWorkspaceBoardPaletteDocuments` splits out, memoized on `[worktrees, repoMap]`; only the match reruns per keystroke. This is the shape `worktree-jump-palette-document-index.ts` already provides for Cmd-J. `useTabGroupItemProjections` resolved each editor tab against `state.openFiles` — the global list across every worktree — and each `tabOrder` entry against the group's tabs, with a `.find` per element. Both are now `Map` lookups, alongside the `terminalTabById` index the same file already built. The memo key `groupTabs` gets a new identity on any unified-tab write, so this ran on title, label and colour changes. `buildSourceControlTree` rebuilt every ancestor path with `segments.slice(0, i + 1).join('/')` per segment, making tree construction O(files x depth^2) in characters copied — on the path the Source Control file filter rebuilds per keystroke. The path now accumulates. `worktree-header-section-boundaries.ts` ran a full `findIndex` over the render rows for every header row, plus an `indexOf` over the bucket ordering, in two `useMemo`s keyed on `renderRows` — so it recomputed on every sidebar row-model change, not just during a drag. One indexing pass each, first-match-wins to match `findIndex`/`indexOf`. The successor index is keyed per bucket because a repo or group id can appear in more than one bucket ordering; a flat id-keyed map would pick whichever bucket was iterated first. `worktree-header-section-boundaries.test.ts` pins that and the first-wins duplicate-header case. Measured by `pnpm bench:renderer-quadratic-scans`. Three scenarios time the production export against a reproduction of the pre-change function; the tab-group scenario is modelled on both sides because the projection lives inside a React hook. Each asserts before/after agree first. | projection | drives | scale | before | after | | | --- | --- | --- | --- | --- | --- | | workspace board filter (per keystroke burst) | production | 300 worktrees x 12 keystrokes | 15.4 ms | 4.4 ms | 3.5x | | tab-group projections (per unified-tab write) | modelled | 60 tabs x 120 open files | 1.23 ms | 0.34 ms | 3.6x | | source-control tree build (per filter keystroke) | production | 5000 changed files | 6.2 ms | 3.9 ms | 1.6x | | sidebar header boundaries (per row-model rebuild) | production | 80 repos x 600 rows | 2.0 ms | 0.8 ms | 2.6x | The tree and sidebar wins are smaller than the scans they remove because the rest of each function (tree finalize/sort, per-row size estimation) is linear and now dominates. 4,537 existing sidebar, tab-group and right-sidebar tests pass unmodified.	2026-09-05 00:11:51 -07:00
Neil	c36dd5f4c7	perf(terminal): skip the partial-escape-tail walk on ESC-free PTY chunks (#18748) ... `advancePartialEscapeTail` runs once per PTY chunk, on the main thread, for every terminal — visible, hidden or parked — inside `HeadlessEmulator`'s write path. It unconditionally concatenated the pending tail with the whole chunk and then walked the result one code unit at a time through a VT500 state machine. `extractPartialEscapeTail` only leaves `ground` on an ESC byte, so with no pending tail and no ESC in the chunk the answer is always ''. Taking that case up front skips both the full-chunk concat and the walk. `String.prototype.includes` is a native scan, so the gate costs essentially nothing on the chunks it does not short-circuit. This is the same gate its two neighbours on the very same ingest path already apply — `TerminalOscCwdTitleScanner.scan` and `TerminalMouseModeMirror.scan`, both carrying a comment citing their measured share of a 2.2x ingest regression. This call was simply missed. Measured by `pnpm bench:terminal-partial-escape-tail` over 640 x 16 KB chunks (10 MB), median of 7 rounds: | stream shape | before | after | | | --- | --- | --- | --- | | ESC-free (build logs, `cat`, piped output) | 40.3 ms | 0.19 ms | 213x | | SGR-coloured output (gate does not apply) | 30.6 ms | 32.1 ms | 1.0x | The benchmark proves equivalence over a 226-case corpus before timing, and a new unit test pairs every pending-tail state the scanner can be left in against every chunk shape, asserting the gate is indistinguishable from the unconditional fold.	2026-09-05 00:11:48 -07:00
Neil	e1599c94b8	perf(terminals): let idle panes share one process-table capture instead of forking their own (#18742) ... * perf(terminals): let idle panes share one process-table capture instead of forking their own Every visible local pane runs an agent-completion cadence that resolves through `getStrictProcessTableSnapshot`, and the inspection queue already collapses every shared-observation task enqueued in the same tick onto a single whole-host `ps`. Independent ±10% jitter per pane defeated that: the jitter was re-rolled on each reschedule, so panes drifted permanently apart, each landing in its own tick and each missing the snapshot's 500 ms TTL. Four idle panes cost four captures where one would have served all of them. Idle panes now aim at a deadline grid anchored at the epoch. The pull-forward is clamped to the snapshot TTL, so no interval is ever longer than its tier and none is more than 500 ms shorter: a pane off the grid walks onto it over at most `tier / TTL` steps, costs at most one extra inspection in total, and no inspection is ever delayed. Scoped deliberately. A pane with a foreground agent, or one still inside the 10 s post-activity hot window, keeps its exact interval and its own phase, so the bounded hot cadence is unchanged. The error-backoff path keeps its jitter, where spreading retries across panes is the point. Measured by `pnpm bench:agent-inspection-cadence` — whole-host `ps` captures over 60 s at the 2 s idle tier, median of 21 rounds: | visible panes | before | after | reduction | | --- | --- | --- | --- | | 1 | 29 | 29 | 0% | | 2 | 42 | 30 | 29% | | 4 | 62 | 31 | 50% | | 8 | 82 | 32 | 61% | `process-table-snapshot-reader.ts` measures the `command=` column at 1.15 s of work for 1,948 processes, so these are captures a quiet app was paying for continuously. All 4,202 existing terminal-pane tests pass unchanged, including the no-evidence cadence suite that pins the relaxed and hot intervals. * test(terminals): report n/a instead of dividing by a zero baseline in the cadence benchmark A window shorter than one cadence tier leaves the baseline capture count at zero, and the reduction line then divided by it and printed a meaningless percentage. Reported by CodeRabbit on #18742.	2026-09-05 00:05:29 -07:00
Neil	149732df6d	perf(persistence): stop the session write re-scanning and rebuilding unchanged state (#18739)	2026-09-04 23:39:02 -07:00
Neil	b33d1972bc	docs(relay): correct why the ConPTY teardown asset diverges from the desktop patch (#18636) ... The divergence pinned by #18601 is real and worth keeping, but its stated reason was wrong. It claimed the desktop patch carries the early conin placement "and therefore the +2 File / +1 Process regression, measured against its exact installed tree" -- i.e. that the shipped desktop app leaks because of its own leak fix. It does not, for any terminal a user opens. node-pty defaults `_useConptyDll` to false. Every desktop site that opens a pane sets it true (`local-pty-utils.ts` twice, `native-pty-spawn.ts`), as does the `windows-conpty-warmup.ts` warm-up, so they take the `else` branch, where upstream already destroys the input socket. The relay passes no such option (`src/relay/pty-handler.ts`) and takes the `!useConptyDll` branch -- the one both this asset and the desktop patch edit. The desktop is not entirely off that branch, though: the hidden rate-limit probes in `src/main/rate-limits/claude-pty.ts` and `codex-pty-rate-limit-probe.ts` omit the option, recur, and tear down through `kill()`, so the hunk is live there -- just never for a visible pane. Whether the early placement costs the same +2 File / +1 Process across a probe's lifecycle is unmeasured; the numbers in this comment were taken on relay-style spawn/kill cycles, and the comment now says so. What is settled is the replaced claim: not every Windows user, and not every terminal. Those two probes were missed three enumerations running because they use `await import('node-pty')`, which no static-import grep finds. The comment now tells the next reader to grep for `node-pty` instead. The measurement that produced the wrong claim was taken by a standalone harness that passed no `useConptyDll` and so defaulted into the branch it was not trying to measure -- the same standalone-is-not-the-real-host trap #18601's own body warns about, one level down. Also refreshes the self-exit paragraph, which #18635 made stale. That leak is now fixed for the desktop, and the note records why the fix cannot reach a Windows relay. The fix is mostly native (`src/win/conpty.cc`) and this asset only rewrites `lib/*.js`, and all three delivery paths stop short of Windows: pnpm patches do not cross the SSH boundary; `MATRIX_SLOTS` in `build-orcad-prebuilds.mjs` has no win32 entry; and the one relay asset that does patch native source and rebuild on the host (`node-pty-1.1.0-master-cloexec-patch.cjs`) returns `skipped:unsupported-platform` for anything but linux/darwin. #18635's flat self-exit relay numbers were measured against a locally rebuilt binary, so they describe the relay code path on a patched tree, not the tree a relay host installs -- the note says so explicitly rather than leaving the next reader to conflate them. Assertion and hashes unchanged: the relay must still release conin after the console-list fork, and a patch sync must still not copy the early placement onto the relay's branch, where it does cost +2 File and +1 Process per terminal. Only the justification changes, plus the test name, which said "like the desktop patch" where it meant "unlike the desktop patch placement".	2026-09-04 22:40:38 -07:00
Neil	41b520259e	ci(package): retry apt fetches and docker builds behind the Ubuntu mirror (#18797) ... The package job builds three Docker images whose apt-get update/install hit archive.ubuntu.com with no retry, timeout, or mirror fallback. When the mirror is mid-sync every build dies in one of three ways: - per-package fetch stalls (~64 s each, `Ign:` lines) until the runner's 10-minute docker build timeout fires: https://github.com/stablyai/orca/actions/runs/33935104546/job/101221447425 - `apt-get update` exit 100 with `Hash Sum mismatch` on noble-updates/restricted/Packages.gz: https://github.com/stablyai/orca/actions/runs/33935104546/job/101226099525 - `apt-get update` exit 100 with `File has unexpected size ... Mirror sync in progress?`: https://github.com/stablyai/orca/actions/runs/33935244026/job/101231083497 Each Dockerfile now retries `apt-get update` up to five times with Acquire::Retries and a 30 s HTTP timeout, clearing /var/lib/apt/lists between attempts so a half-synced index is never reused, and passes the same acquire options to `apt-get install`. Each runner script retries the whole `docker build` once when the first attempt fails or times out.	2026-09-04 22:24:18 -07:00
Neil	0a821e5bc8	fix(crash-reporting): make the own-Chromium gate a real choke point, and stop a refusal leaking the root (#18459) ... * fix(crash-reporting): make the own-Chromium gate a real choke point Round-3 review found the guard was not the choke point its own comments claimed: six pid-addressed `taskkill /pid <pid> /t /f` families in main were ungated and uninstrumented, so the stale-pid shape stayed producible and a `selfInitiatedTreeKillCount: 0` could read as exculpatory when it was not. - Gate the remaining main-process families: the git command-runner abort, the notebook-cell and automation-precheck timeouts. - Turn the `src/shared` seam into the gate itself (`process-tree-kill-gate`), so the runProcess choke point, the codex app-server deadline kill and the ephemeral-VM recipe kill ask the same decision. Those three are compiled into the CLI/relay too and cannot import main; main installs the guard at preflight. - Ratchet (`main-process-tree-kill-gate.test.ts`): a new pid-addressed taskkill in main that skips the gate fails, and the allowlist entries must still exist. - Give pid-addressed kills eviction priority in the 32-entry ring: 32 routine `win-pty-job` teardowns from a window-close burst no longer evict the one entry that discriminates a self-kill from an external one. - Correct the coverage doc, which described the uninstrumented Windows sites as POSIX `process.kill(-pid)` group kills and omitted the git and codex paths. * fix(crash-reporting): keep a refused tree-kill from leaking the root it owns A refusal must block the pid-addressed tree walk, not the termination. Five of the six gated sites returned on refusal with no fallback, so a refused `taskkill /pid /t /f` left git.exe, a timed-out notebook cell, an automation precheck or an ephemeral-VM recipe running while the caller reported it stopped. The root kill is addressed by the child handle, which cannot reach the recycled pid the refusal is about, so it stays correct and required on that path. Also fixes the ring eviction the scope preference introduced: with the ring saturated by pid-addressed kills, the only non-pid-addressed entry is the one just pushed, so the splice evicted itself and the detail came back `{}` -- byte-identical to the external-kill arm, in the window-close case the guard exists for. Eviction now excludes the newest entry and falls back to FIFO. Tests: refusal now asserts the root kill at all six sites, and the ring covers the saturated-pid ordering as well as round 3's group-burst ordering. * fix(crash-reporting): stop a refused tree-kill leaking the commit-message agent, and count call sites Two round-5 blocking findings, both open on main and on both branches. `killSourceControlAgentProcess` had no root-kill fallback on its win32 arm: the taskkill was the only termination, so once the own-Chromium gate could refuse it the promise resolved having killed nothing. Both callers do `terminationComplete ??= killSourceControlAgentProcess(child)` and then release the managed-home lock on that promise, so a refusal left the local Codex/Claude commit-message agent running while the caller reported it stopped -- the lock-contention failure the taskkill was added for. Same fix as the six sibling sites: the handle-addressed root kill cannot reach the recycled pid the refusal is about, so it stays correct and required on that path. The ratchet was file-granular, not call-site granular: one gate mention anywhere in a file exempted every taskkill in it, which left the six files that now ask the gate ratchet-blind -- the inverse of what it is for. It now counts `/pid` call sites against gate admissions per file, so a second ungated kill inside an existing family fails. Keying on the `/pid` argument rather than a quoted `taskkill` also catches a kill whose program name comes from a constant. The three comments that claimed more than the old scan enforced now state the rule and its two remaining blind spots. Also: the recording in `admitSelfInitiatedTreeKill` is now wrapped the way the `admitProcessTreeKill` seam already wraps it, with the refusal decision taken before anything that can throw so a diagnostics failure cannot flip it; and `orca-chromium-process-pids` documents the false-positive direction (a stale `getAppMetrics()` entry plus pid reuse refuses a live unrelated child), which is the mechanism the root-kill fallback exists to bound. Tests: refusal now asserts the root kill at all seven sites; the ratchet asserts call-site counting and the constant-program form. * test(crash-reporting): run the own-Chromium gate against real Windows trees Nothing on this branch had ever executed on Windows. The unit tests pin the gate's decision against a mocked taskkill, which cannot show that the decision does anything to a real process: that `/T /F` reaps a detached grandchild, that a refusal leaves that tree standing, or that the handle-addressed root kill the refusal path falls back to reaps the root while orphaning descendants. Adds a win32-gated live test covering all four, registered in both the `package_windows` CI lane and `WINDOWS_PACKAGE_TESTS` as `win32-test-lane-registration` requires. Also completes the coverage doc's "never instrumented" list, which omitted the macOS keyboard-input-source probe's POSIX group kill in `ipc/app.ts`. * fix(crash-reporting): pin the commit-message root kill on the Windows arm The first Windows run of this branch found nine failures the macOS suite cannot see: `commit-message-text-generation-test-harness` asserts `expect(child.kill).not.toHaveBeenCalled()` on `process.platform === 'win32'`, which is the contract the previous commit deliberately replaced — and it branches on the real platform, so it is dead code everywhere CI runs today. The harness now asserts the handle-addressed root kill on every platform. On win32 it lands after the tree walk, so the expectation waits rather than reading one tick early, and its ten call sites await it. Red against the pre-fix arm at all seven sites; the production code is unchanged. * test(crash-reporting): remove the Windows lane marker tree through the retrying helper The new win32 spec teardown used a raw rmSync, which the windows-lane-tree-removal boundary ratchet rejects — and which is exactly the EPERM the ratchet exists to prevent, since this spec's marker directory is written by processes it has just force-killed. * fix(crash-reporting): only refuse pid-addressed tree walks, disclose the handle-less codex site The own-Chromium gate refused the POSIX process-group arm of signalProcessTree as well, which was new macOS/Linux behaviour: a stale getAppMetrics() entry plus pid reuse would orphan a group that main reaps today. A POSIX group only holds what Orca put in it, so the refusal is now scoped to win-taskkill-tree and the POSIX arm is recorded and admitted like the other group kills in main. That also drops the synchronous getAppMetrics() read from every POSIX termination. codex-turn-added-roots kills roots found by a table walk, so a refusal has no handle to fall back to. Pin that the refusal is visible - crumb written, turn reported as not cancelled - rather than fixing what cannot be fixed. * test(crash-reporting): detach the Windows survival fixture and observe real spawns	2026-09-04 16:42:42 -07:00
3 people	a65332a8bd	feat(claude): move structured native chat onto the Claude Agent SDK and enable it on macOS and Linux (#18560) ... * Join structured attach teardown through journal bind * fix: restore structured chat parity * feat: add Claude structured session adapter * fix: harden Claude structured adapter * fix: close Claude adapter edge cases * fix: start Claude init deadline after launch * feat: wire Claude structured sessions * fix: harden Claude structured runtime * fix: fence Claude structured compatibility * fix: preserve Claude free-text prompt answers * fix: decode addressed Claude prompt text * feat: enable Claude structured chat on mobile * fix(mobile): keep structured chat provider-aware * fix(mobile): negotiate Claude structured tabs * fix: keep scoped RPC tests native-free * fix: secure mobile structured image delivery * fix: close structured session data-loss gaps * fix: prove real Claude structured startup * fix: consume pre-spawn proof before retry * feat(native-chat): add desktop structured sessions * fix(native-chat): satisfy structured session cleanup gates * fix(native-chat): keep structured renders pure * fix(native-chat): open composer pickers upward * fix(native-chat): use existing view for structured sessions * fix: harden structured desktop status projection * fix: close structured desktop lifecycle gaps * fix: fence structured AI Vault resumes * fix: fence structured AI Vault resumes * fix: preserve structured tabs during activation * feat: toggle structured sessions between chat and TUI * fix: harden structured session handoffs * fix: bind structured TUI before rollout proof * fix: complete structured chat round trips * fix: align structured TUI return readiness * fix(native-chat): make reverse handoff transactional * Add Claude structured TUI handoff seams * fix(native-chat): clear sticky handoff recovery * fix(native-chat): complete mobile reverse after TUI exit * fix(native-chat): keep TUI transcripts readable * fix(native-chat): recover TUI transcript gaps * fix(native-chat): recover claimed TUI owners * fix(native-chat): retain cold TUI proof authority * fix(native-chat): preserve Claude handoff authority * fix(native-chat): recover TUI transcripts read-only * fix(native-chat): harden Claude handoff recovery * fix(native-chat): serialize structured handoff recovery * fix(native-chat): close handoff admission races * fix(native-chat): validate pinned launch environment * fix(native-chat): revalidate restored and retried owners * fix(native-chat): gate restart recovery publications * fix(i18n): catalog Claude session controls * fix(native-chat): wait for structured TUI process proof * fix(native-chat): queue stale idle TUI handoffs * fix(native-chat): route structured Codex options directly * fix(native-chat): persist structured session options * fix(native-chat): hydrate resumed structured options * fix(native-chat): preserve options across structured handoffs * fix(native-chat): replay pending option mutations * fix(native-chat): rotate settled handoff operations * fix(native-chat): rotate refused send operations * test(native-chat): derive refusal retry state from host * test(native-chat): give the host-oracle matrix test an explicit timeout * fix(native-chat): keep Claude option controls idle * fix mobile structured first-send hydration race * fix(native-chat): preserve handoff launch authority * fix(native-chat): harden shared handoff recovery * fix(native-chat): serialize structured handoff recovery * fix(native-chat): close handoff admission races * fix(native-chat): validate pinned launch environment * fix(native-chat): revalidate restored and retried owners * fix(native-chat): gate restart recovery publications * fix(i18n): catalog structured session recovery control * fix(native-chat): wait for structured TUI process proof * fix(native-chat): queue stale idle TUI handoffs * fix(native-chat): keep structured recovery provider-neutral * fix(native-chat): drop local terminal topology from structured sync * fix structured outbox and tab restore races * fix(native-chat): preserve Claude question groups * fix structured provider visibility and request handling * fix structured session TUI handoff recovery * fix reverse structured session handoff * fix(native-chat): recover Claude outbox and resume state * chore(mobile): preserve the working-tree lockfile state before the main merge Carries the pre-existing uncommitted mobile/pnpm-lock.yaml modification into history so the main merge cannot overwrite it. Verified benign pnpm drift (babel 7.29.7->7.29.8 transitives plus deprecation metadata); drops no patchedDependencies (the mobile lockfile declares none). * test(native-chat): drop orphaned Claude handoff-auth test left by the main merge 'pins Claude handoff auth through the terminal provider boundary' is absent from main and its production counterpart preserveClaudeAuthEnv no longer exists outside this test - orphaned residue of the terminal/native handoff work this PR excludes by scope. Removed rather than repaired: the failure was a renamed field (providerHome -> providerRoot), and renaming it would have carried out-of-scope handoff code into the merge. Body preserved as evidence and logged in CLAUDE-STRUCTURED-DISPOSITION-TABLE.md. * Fix mobile structured turn state * fix Claude structured session blockers * fix claude structured lane blockers * fix Claude acquisition exit proof * fix(claude): route stream-json launch through process wrapper * fix(claude): gate structured chat support * Fix Claude structured launch gating * fix(claude): split session acquisition and prune mobile scope * test(claude): align structured session fixtures * fix(agent-session): preserve handoff launch arguments * fix(claude): open journals through the factory after origin/main split The journal opener moved to journal-store-factory on main; retarget the Claude structured tests that still imported the old path. * fix(claude): resolve Claude structured launch args, auth, and win32 proof The origin/main merge re-expressed the lane's Claude wiring onto main's split orca-runtime facade and dropped three wires past green typecheck and lint. - resolveLaunchArgs discarded its provider parameter, so structured Claude sessions were launched with Codex app-server flags; Claude exits on --dangerously-bypass-approvals-and-sandbox, and a Codex arg-parse throw could block Claude session creation outright. - resolveClaudeLaunchEnv was no longer supplied, so the launch resolver fell back to the whole process env as configuredEnv and buildClaudeChildProcessEnv re-applied every auth var it had just stripped. The resolver now merges the Claude overlay onto a strip-applied copy of the inherited env, which also keeps PATH intact for withCliRuntimeOnPath. - The windowsProcessStartTimeAvailable producer was gone while the contract field and both consumers survived, so the renderer gate fail-closed and structured native chat was unreachable on every win32 host. Separately, structured Claude pinned CLAUDE_CONFIG_DIR unconditionally. An explicit pin makes the CLI abandon the macOS Keychain even when it names the CLI's own default, so a default claude.ai account could not authenticate where the legacy Claude terminal could. Pin only a home the CLI would not resolve on its own, matching ClaudeRuntimePathResolver, and compare against the env the child would otherwise inherit so a diverging overlay cannot outrank the record's account home. Also await the now-async revealNativeSession in its regression test, and set the native status before revealing so a rejecting reveal cannot leave a session released but never marked native. Claude-Session: https://claude.ai/code/session_013UqKCRB6k5e8UaYhXUHeWY * fix(claude): scrub case-insensitive Windows auth env * fix(native-chat): settle handoff outcome-write failures instead of leaking them A store write failure while recording a handoff outcome escaped the flow runner's catch handler, so the client never received the failure and the flow surfaced as an unhandled rejection (seen as an intermittent agent_session_store_corrupt error in the proven-dead-retry suite, whose teardown raced the flow's trailing outcome write). Record the failed outcome best-effort, and drain the coordinator before that test's teardown removes the store root. Claude-Session: https://claude.ai/code/session_011aXkcHyeiRJuezupQdjZaM * fix(native-chat): make the structured close-failure toast provider-neutral The structuredSessionCloseFailed toast fires for any structured session, but its copy said 'Codex chat', so a Claude structured session that fails to close showed the wrong provider name. The launch-failure toast is only reachable behind the agent === 'codex' gate, so its copy stays as is. Claude-Session: https://claude.ai/code/session_013ugSpCx4AWkySaJb69BQax * fix(native-chat): wire structured handoff proof recovery * fix(native-chat): wire structured handoff proof recovery * fix(native-chat): correct the structured chat opt-in copy The one `experimentalStructuredNativeChat` toggle gates both providers — `useStructuredAgentSessionCreate` runs `canUseStructuredNativeChat` for `'claude'` as well as `'codex'` — but its description named only Codex. Its scope line also said Windows keeps using terminal chat, while the gate refuses win32 only until the host proves it can read a process start time. `structured-native-chat-availability.test.ts` already pins that Windows is allowed once the proof is cached, so the two contradicted each other. Claude-Session: https://claude.ai/code/session_01RJFsidQWmKYFmeoUuVu4Tp * test(claude): pin @anthropic-ai/claude-agent-sdk 0.3.251 contracts against a scripted CLI PR 1 of the SDK migration: dependency + test-only harness, no product wiring. - Pin @anthropic-ai/claude-agent-sdk to exactly 0.3.251 — not the newest release — because 0.3.251 (published 2026-08-28) clears the repo's 3-day minimumReleaseAge supply-chain gate with no exclusion, while the newest release was minutes old and would have required excluding a brand-new publish from the exact control built to catch brand-new malicious publishes. Every contract this design depends on was verified identical on 0.3.251: the full option surface, no pid on SpawnedProcess (custom spawner stays mandatory), env defaulting to process.env when omitted, and --replay-user-messages appearing only via extraArgs. - Exclude all eight bundled CLI platform binaries via ignoredOptionalDependencies. The setting lives in pnpm-workspace.yaml because pnpm 12 no longer reads the package.json "pnpm" field (it warns and ignores it; verified by install ablation). Excluding the binaries is what makes Orca's pathToClaudeCodeExecutable override mandatory rather than merely preferred. Note: pnpm 12.0.0 honors the ignore list when reconciling an existing lockfile but not on fresh resolution of a new dependency, so the lockfile's SDK entry was pinned surgically; both 'pnpm install' and 'pnpm install --frozen-lockfile' verify clean and stable against the committed lockfile. - Contract-pin suite drives the real SDK against a scripted fake CLI and pins: unknown type/field/content-block pass-through (and keep_alive interception), spawner env fidelity plus the omitted-env process.env inheritance sharp edge, extraArgs producing --replay-user-messages, argument parity for every CLAUDE_STRUCTURED_BASE_ARGS entry plus --session-id/--resume/ --resume-session-at, canUseTool wire request_id stability and abort on control_cancel_request, one spawn per query, pathToClaudeCodeExecutable honored by the default spawner, the exact SDK version, and the eight platform binaries staying uninstalled. Claude-Session: https://claude.ai/code/session_01FGCRfYUnb4hbvfTAHGtJKQ * feat(claude): drive the structured transport through the agent SDK Replaces the hand-rolled `claude -p --input-format stream-json` transport with @anthropic-ai/claude-agent-sdk 0.3.251, keeping the existing connection interface for this commit so the acquisition path changes minimally. The control-plane rewrite is a separate change. Orca still supplies the process. `spawnClaudeCodeProcess` routes through `spawnProcess`, retains the child and its pid — the triple the durable lease adjudicates on — drains stderr so exit errors keep their tail, and hands `.cmd` shims to Orca's Windows argument encoder rather than the SDK's plain spawn. `close()` keeps Orca's own bounded tree-kill and exit deadline, so it still resolves true only after an observed exit. Launch resolution emits an SDK options object instead of argv; durable `launchArgs` translate to a typed option where one exists and to `extraArgs` otherwise, refusing a token neither can carry rather than dropping it. The child env is always passed explicitly — omitting it would let the SDK inherit `process.env` and reintroduce the ambient `ANTHROPIC_*` leak. The stdout line parser is deleted; the SDK owns framing, and unknown frames still reach the translator verbatim. Claude-Session: https://claude.ai/code/session_01JMhFjh9HEnkcJ5YTfCdgD3 * fix(claude): settle the frame the SDK pulled but never wrote The SDK's input pump is `for await (frame of prompt) { await transport.write(frame) }`. When that write rejects — the child dies between Orca's liveness guard and the write — the for-await ends abruptly and calls the generator's `return()`, so the code after `yield` never runs. The frame was already shift()ed out of `queued`, so the later `fail()` from the exit path could not reach it and `send()` never settled: `dispatchClaudeTurn` awaits that send before it can return `unknown`, wedging the caller and the durable outbox. The pre-SDK transport rejected on the stdin write callback instead. Retain the in-flight entry and settle it from the generator's cleanup, and let fail() reach it too for the pump that never resumes at all. Claude-Session: https://claude.ai/code/session_01AobxxokqQ3qcxS7sy7ckum * fix(claude): keep the agent SDK behind the structured-Claude boundary The ordinary OrcaRuntimeService graph statically reaches the Claude adapter and so the transport module, whose first line imported @anthropic-ai/claude-agent-sdk. The SDK is evaluated whenever the regular runtime loads, before any structured Claude session is chosen: it sets process.env.NoDefaultCurrentDirectoryInExePath, changing Windows executable resolution for later subprocesses, and a missing or incompatible install would break normal runtime startup — for a user who never leaves the terminal/TUI path. Defer the SDK to the connection, memoized so it loads once per process, and add the import-graph ratchet: a walk from the Electron main entry that fails on any static import of the package, plus a clean-fork check that loading the runtime leaves the Windows search variable untouched and a child-process pin that the side effect is still real. Claude-Session: https://claude.ai/code/session_01AobxxokqQ3qcxS7sy7ckum * fix(claude): answer list_models so the picker stops serving the seed sendControlRequest had no list_models case, so every request hit the default reject; readClaudeStructuredSessionOptions swallows that with .catch(() => null) and falls back to the static catalog. Every structured session therefore served a hardcoded model list with no per-model effort levels, no resolvedModel and no default detection, and nothing surfaced the failure. The pre-SDK transport got the live catalog from the CLI. Route it through the SDK's supportedModels(), wrapped in the { models } envelope the existing parser reads. Claude-Session: https://claude.ai/code/session_01AobxxokqQ3qcxS7sy7ckum * fix(claude): reap the child's descendants before killing it The forced step of the exit ladder went through the Codex helper, which spawns `pkill -KILL -P <pid>` and SIGKILLs the parent in the same tick: the parent usually dies first, the descendants reparent to pid 1, and `-P` matches nothing. An MCP or launcher descendant of a stubborn Claude child was left running. The test named for that requirement declined to assert it and killed the survivor by hand instead, so it could not fail for the thing it was named after. Route the Claude reap through Orca's existing sweep, which snapshots descendants while their parent link still exists and signals them before the root goes, and on Windows uses the identity-gated `taskkill /T /F`. The test now asserts the descendant is dead; the manual kill stays only as a failure-safe. close() still returns true only on an observed exit. Claude-Session: https://claude.ai/code/session_01AobxxokqQ3qcxS7sy7ckum * fix(native-chat): merge the duplicated handoff type import CI's static-analysis lint (`oxlint --config config/oxlint-code-quality-native-plugins.json src config tests mobile --deny-warnings`) exits 1 on the two separate `import type` statements from the same module. Claude-Session: https://claude.ai/code/session_01AobxxokqQ3qcxS7sy7ckum * fix(claude): answer a permission callback whose signal already aborted settleFrom registered the abort listener and then delivered the request. A callback that arrives already aborted never fires that event, so the promise stayed pending behind a durable prompt with no cancel path. Check the signal first, emit the cancel, and resolve the SDK's null sentinel without registering. Claude-Session: https://claude.ai/code/session_01AobxxokqQ3qcxS7sy7ckum * test(claude): wait for the child to record the frame, not just for its report The scripted CLI writes its report at startup, so `until(readReport)` returned a report with no user messages whenever the child had not yet read the line. The assertion then failed under parallel load. Poll for the frame instead of for the file. Claude-Session: https://claude.ai/code/session_01AobxxokqQ3qcxS7sy7ckum * fix(claude): coalesce partial deltas onto one assistant item and stop painting result frames Under --include-partial-messages every stream_event frame carries its own uuid, and the final assistant frame for a block carries yet another; only message.id ties them. The translator keyed each delta by its frame uuid, so a reply painted as one bubble per delta chunk followed by a complete duplicate under the final frame's uuid. The block's first stream frame now mints the claude:(sessionId, uuid) identity, deltas coalesce onto it through the shared 60ms seam, and the final frame reconciles onto that same item. Known SDK bookkeeping no longer reaches the provider-fallback row: result subtypes are catalogued and settled by the turn lifecycle, an empty thinking block (redacted thinking) is a modeled kind, a string-content user replay is a text block, and an empty user frame paints nothing. An unmodeled result subtype or content kind still lands on the bounded fallback row. Claude-Session: https://claude.ai/code/session_01GaP5HpYQbvy2hYehVhwfEW * fix(claude): prove descendant exit at the close boundary instead of on an unref'd timer close() reported proven=true as soon as the direct child exited while the descendant sweep's SIGKILL sat on an unref'd 2 s timer, so a SIGTERM-resistant MCP server outlived the lease release. The reaper now composes the same shared primitives the Codex structured provider uses: snapshot, verified bounded descendant termination on POSIX, taskkill /T /F on Windows. The proof is false whenever descendants outlive the deadline, a retried close re-verifies the retained snapshot rather than trusting the dead root, and the raw pipe child no longer goes through the PTY job sweep it never owned a job for. Measured on macOS: a killed child of a SIGSTOPped parent stays a matching zombie row in ps, so the root is killed while verification runs rather than stopped first as the Codex non-group path does. Claude-Session: https://claude.ai/code/session_0161QFm3KVRNJKfdzWVGVNWk * feat(claude): replace the hand-rolled control plane with the SDK's native surface PR 3 of the Claude structured SDK migration removes the wire-frame scaffolding PR 2 kept, so Orca drives the SDK's typed control surface directly. Inbound permissions move from a rebuilt control_request dispatch to the SDK's canUseTool / onUserDialog callbacks. The prompt registry now carries the callback's own resolver: a decodable can_use_tool becomes a durable prompt whose answer settles the callback; a malformed one is denied without registering; the SDK's abort signal (fired on control_cancel_request, which the SDK matches and dedups itself) forgets the prompt and settles it null, and a late answer after abort finds no prompt and is refused. Closing settles every in-flight callback so no promise dangles. The claude-agent-sdk-control-bridge that rebuilt the wire frame is deleted. Outbound control maps to Query methods: interrupt() for cancel, setModel / setPermissionMode / applyFlagSettings for options, supportedModels for the model list, initializationResult() for init proof, each under Orca's own request deadline and error classification. Cancel is interrupt-receipt aware: a CLI advertising interrupt_cancel_queued_v1 gets cancel_queued in one round trip, otherwise the receipt's still_queued uuids are swept with cancel_async_message so a cancelled turn cannot spawn a later unexpected turn; older CLIs resolve no receipt. Init keeps the 10s deadline and the unauthenticated-startup guidance. Every behavior is failing-first and ablation-proven; the toggle-off import boundary and the accepted loss of unknown-control visibility rows are unchanged. Claude-Session: https://claude.ai/code/session_01Pqjduxt5G4rr9aYvtp7rNm * fix(claude): arm the descendant snapshot before stdin closes and make the tree verdict unproven by default A healthy Claude root leaves within the graceful window, and the close ladder only snapshotted descendants when the root was still alive after that window. So the common close never looked at the tree: `treeExited` stayed null, `!== false` passed it, and close() reported a proven exit with an MCP child still running. A root that died before the walk made the snapshot vacuous too. The proof is now unproven by default. The reaper holds one verdict in Orca's vocabulary (exited / live / unverifiable), assigned in exactly one place from the bounded verification, and close() returns true only on `exited`. The snapshot is armed before stdin closes, while the root can still be walked, and is verified after the root exits; a root that left before any snapshot could be armed stays unverifiable rather than vouching for descendants it never showed us. The shared verifier gains the three-way verdict behind its boolean face, and the connection reports the root and tree verdicts separately along with the child's exit status. One verification per close attempt: the retried close re-verifies, so the intra-attempt re-reap is gone from the teardown budget. Claude-Session: https://claude.ai/code/session_01BSmXgkWSsNHft8jFkdBFG9 * fix(claude): verify the Windows tree after taskkill instead of trusting that it ran `terminateWindowsProcessTree` resolves from taskkill's callback whatever the error says, so a timeout, an access denial, a recycled root and a surviving descendant all looked identical to the reaper — which then returned a proven exit unconditionally. close() reported true and the lease was released with an MCP descendant potentially still live. The Windows branch now snapshots the root's descendants while it is alive and, after taskkill, polls a fresh process table to a bounded deadline: a row still matching by pid AND creation time is `live`, an unreadable table is `unverifiable`, and only a table with no match is `exited`. Creation time is the PID-reuse guard the POSIX path gets from ps lstart, so a descendant that denied a creation-time query is omitted rather than signalled on a bare pid. A root already observed exited is never taskkilled: `/T /F` on a recycled pid would take an unrelated tree down with it. The captured tree is tagged by platform so neither verifier can be handed the other's rows. Claude-Session: https://claude.ai/code/session_01BSmXgkWSsNHft8jFkdBFG9 * fix(claude): release a reservation on a first-hand root exit instead of latching it into manual recovery Making close() strict about the descendant tree exposed a second defect at the same boundary. A create-time acquisition has no ownerProcess until publication, so an unproven cleanup mapped to handoffStage `manual-recovery`, and adjudication then refuses every later attach with agent_session_ownership_unknown. A user who was merely signed out, or whose --resume the CLI rejected, wedged the session id permanently. Each question now answers from its own evidence. close() is unchanged and stays strict about the tree. Separately, the lease is keyed on the root's pid and start time, so when Orca's own child handle observed that root exit and no descendant snapshot was ever admissible, the reservation is released and the CLI's exit code and stderr reach the user. A descendant observed still alive, or a root Orca never saw leave, stays unproven and keeps the reservation. The settlement records only what was observed: the released lease says the provider process exited and its descendants were not verifiable, rather than reusing the wording that claims cleanup proved no child remains. Claude-Session: https://claude.ai/code/session_01BSmXgkWSsNHft8jFkdBFG9 * fix(claude): surface an API error a result frame reports instead of settling the turn on it The SDK models an API failure as a SUCCESS-subtype result whose `result` string is the user-facing error text, with no assistant frame behind it. The translator suppressed every catalogued result subtype as turn bookkeeping, so that turn tombstoned its lifecycle and showed the user a completed, empty reply with no sign anything had failed. Suppression is now by meaning. A result reporting a failure routes to the bounded provider-error surface, leading with the provider's own sentence and keeping the raw frame behind the row's disclosure; ordinary successful results stay off the timeline as before. A turn the user aborted also stays suppressed: its interrupt frame already says so, and its execution diagnostic would only be noise on every stop. Claude-Session: https://claude.ai/code/session_01BSmXgkWSsNHft8jFkdBFG9 * fix(claude): drop the stream state of turns that never received their final frame Every streamed delta recorded its block's identity, latest text and checkpoint length. Only the final assistant frame removed them, so an interrupted turn left its whole accumulated reply reachable until the session was disposed, and a long session with repeated interruptions grew those maps without bound. The partial text was already journaled by the flush that precedes settlement, so the live copy was pure retention. That state now lives in its own module, named for what it does — grow a streamed block's journal row between its deltas and its final frame — and turn settlement drops every block still awaiting a final. The translator reports how many remain, which is the invariant: a settled turn leaves none. Also makes a timed-out process-table read retryable while the root is still alive. A loaded host can miss the table's one-second deadline, and latching that as "no descendants" both lost the descendant sweep and, on a busy machine, made the close ladder report unproven for a tree it never actually looked at. Only the root's death still makes a missing snapshot final. Claude-Session: https://claude.ai/code/session_01BSmXgkWSsNHft8jFkdBFG9 * perf(claude): capture the Windows descendant tree from one process-table read The capture walked the descendant tree and then read the table again for the creation times the walk's projection drops. Each read is bounded in seconds and both run inside the close ladder's budget, so the second one cost the worst-case teardown three seconds for data the first read already held. The walk is now exported from the module that owns it and runs over rows the caller has already read, which is also what lets the snapshot keep the PID-reuse guard the projection cannot carry. Claude-Session: https://claude.ai/code/session_01BSmXgkWSsNHft8jFkdBFG9 * fix(pty): spend the descendant verification window instead of surrendering on one slow table read The verification abandoned the whole check the first time a process-table read missed its own one-second deadline, with seconds of its window still unspent. On a loaded host that reported a tree unverifiable without ever having looked at it, which the Claude close ladder then turned into an unproven close and a retried teardown. It also made the descendant-exit tests flake under a parallel suite run, for the same reason and with the same honest-but-premature verdict. A read that missed its deadline is now simply not an answer: the loop waits and reads again until its own deadline, and only a window that ends without a readable table reports unverifiable. This can only turn a premature verdict into one backed by evidence; it never manufactures a proof. Claude-Session: https://claude.ai/code/session_01BSmXgkWSsNHft8jFkdBFG9 * fix(claude): never let a later failed look collapse an observed live descendant into unverifiable The reaper's single assignment site latched only 'exited', so a second reap whose table reads all missed their deadline overwrote an earlier completed verification's 'live' with 'unverifiable'. The acquisition release gate discriminates on exactly that pair, so a root exit after such a decay released the lease over a descendant that had been observed alive. The latch is now monotone in trust order: exited is final, and live is only ever raised to exited. Claude-Session: https://claude.ai/code/session_01HfdhsvSJucLw4cTZxzg2CP * fix(claude): never prove a Windows tree gone while a descendant denied identification The Windows snapshot dropped rows that denied the creation-time query, and an emptied snapshot was judged exited without any table read: a descendant Orca was refused information about was treated as one that had left. The snapshot now counts the unidentified rows it saw, and verification caps its verdict at unverifiable while any exist. Nothing is ever signalled on a bare pid, as before. Claude-Session: https://claude.ai/code/session_01HfdhsvSJucLw4cTZxzg2CP * fix(claude): classify cleanup after a first-hand exit as a root exit instead of a proven tree When the CLI died between a successful acquire and the host's commit or proof of the lease, handleExit had already removed the session, so releaseAcquisition found nothing and reported true. The attach flow then settled exit-proven with deathEvidence claiming cleanup proved no provider child remains, though the tree was never verified. The adapter now keeps the exit that removed a published session until the session is acquired again; acquisition cleanup runs that connection's close ladder and classifies its verdict exactly as a start-time failure would be, so the record reads root-exit-observed. The wire helper keeps that typed classification and its provider diagnostic instead of wrapping it as unproven, and the router gives up its owner even when the release throws. Claude-Session: https://claude.ai/code/session_01HfdhsvSJucLw4cTZxzg2CP * fix(claude): integrate SDK teardown and picker lifecycle fixes * fix(claude): preserve resume leaf and settle processless spawns * fix(claude): reacquire from persisted resume leaf * fix(native-chat): restore Claude grouped question handling * fix(claude): persist only resumable transcript leaves * fix(claude): recover structured session exits safely * fix(claude): close remaining structured session P1s * fix(claude): harden transcript branch proof * Remove superseded root fix reports * fix(windows): restore indexed descendant row walk * fix(router): forward force-close lifecycle * fix(claude): fence stale turn cancellations * fix(claude): fence cancellation after unknown dispatch * fix(claude): fence replay and option recovery races * fix(claude): block replay fallback after waiter eviction * fix(claude): fence evicted slash results * fix(claude): fence ambiguous results and restore options safely * fix(claude): scrub SDK child env and localize pending launch * fix(claude): pin transcript roots and exit recovery proofs * fix(claude): retain unproven SDK exits * fix(claude): settle retained exit before reacquire * fix(claude): resume from settled retained cursor * chore: remove tracked review artifact * fix: harden Claude SDK transport session cleanup * fix: close Claude sessions safely * fix(claude): close races with fresh child snapshots * fix(claude): fail closed on recycled child identities * fix(claude): gate root cleanup on process identity * fix(claude): fence same-second root identity reuse * fix(claude): restore the root SIGKILL fallback the identity gate took away The direct root kill goes through the handle Node owns, not through a pid: libuv drops that handle in the same turn it reaps, so the signal either reaches the process Orca spawned or reaches nothing at all. Gating it on a process-table probe therefore bought no safety and cost the tree its only fallback whenever the probe declined -- a first capture landing in the fork's own second, a recycled descendant pid voiding the snapshot, or a process table that could not be read on either platform. Identity verification stays where a bare pid is genuinely addressed: Windows `taskkill /T /F`, and the descendant sweep's own revalidation before it signals. Also stops a declined root probe from collapsing an observed `live` or `exited` descendant verdict into `unverifiable`, and stops a successful taskkill from reporting `unverifiable` because a later probe found the root correctly dead. * docs(claude): rewrap the root-kill ordering comment * Match the Claude structured launch to the terminal path's managed-account auth rules The SDK path stripped ambient Anthropic auth unconditionally, let an explicit agentDefaultEnv override beat a pinned managed account, and had no account-switch guard. Reuse the terminal preflight's own predicate and messages so both transports strip, refuse, and report identically, and cover the CLI transcript location that mobile native chat depends on. * Reach the Claude structured chat lane from the desktop UI The main process has had a complete, correctly gated Claude Agent SDK lane for a while, but no renderer ever asked for it: the launch route accepted only `codex`, and the create path was typed `agent: 'codex'` end to end. Widen both to the structured provider union that already exists (`AgentSessionHandleProvider`), and generalize the codex-named create path instead of adding a Claude twin beside it. The pending-launch registry is now keyed by agent as well as workspace — a shared key handed a second caller the first agent's intent, so a Claude and a Codex launch in one worktree collided. Windows, per agent. Codex's client-side win32 refusal is deliberate and settled elsewhere, so it stays exactly as it was. Claude's answer is no longer guessed from the client's platform: a structured session fences its provider child on that child's process start time, and only the executing host knows whether it can read one. `agentSession.createSupport` already answers precisely that, per agent, and had no renderer caller — so the Claude create path asks it before creating and turns a "no", or a probe it cannot get answered, into the definitive refusal the launch fallback already handles. Fail closed either way. That refusal mapping also closes a real gap: the host reports an unsupported location by throwing `structured_agent_session_unsupported`, which reaches the client as a transport rejection rather than a refusal envelope, so `StructuredAgentSessionCreateRefusalError` never fired. The launch would retry the create, strand itself in `visibilityUnknown`, run no legacy fallback, and show an error toast. Close a fail-open hole while Claude and win32 become reachable: `create` with a client-supplied location, and `ensure`, both skip the worktree-resolving support check. They now ask the executing host the same question directly, so a host that cannot fence a provider child no longer creates one on a client's say-so. Also deletes `structured-agent-session-provider-routing.ts`, a duplicate of `structured-agent-session-provider-support.ts` with no importers. WSL, SSH and paired hosts, floating workspaces, draft prompt delivery, explicit TUI customization and initial session options all keep refusing; folder workspaces keep working. * P1-1: make the structured Claude auth policy required and testable The optional dep plus a {stripAuthEnv:false} fallback meant a dropped wiring under-stripped silently. Required at all three hops, asserted at install time for the @ts-nocheck caller, and the settings-to-policy mapping is now a named tested function. * P2-3: mobile's default Claude transcript root must follow CLAUDE_CONFIG_DIR session-file-resolver's default ignored the variable the pinned account home follows, so a CLAUDE_CONFIG_DIR launch wrote one tree and mobile read another. The Task-4 test now resolves with no root override (mobile's own call) and checks the answer against the root the CLI itself reports, instead of mirroring the code under test's own expression. * P2-1/P2-2/P3: close the teardown window, join the live-auth gate, align the refusal P2-1: a switch beginning inside the acquire teardown left a dead chat and no replacement. Past that point the launch waits the swap out and refuses only if it never settles; the entry guard still refuses outright, because nothing is torn down there yet. P2-2: structured children now hold the same OAuth-refresh gate a Claude PTY does, so a managed refresh cannot rotate the token out from under a live turn. P3: the refusal now matches the strip it guards (case-folded on win32, presence not truthiness), and the dead structured-to-TUI builder states its auth policy instead of silently signing a system-auth user out. * Make the live-auth gate tests independent of sibling connection teardown order * Do not offer structured Claude under a WSL-only managed account Structured Claude launches against the ambient Claude config, which the account service keeps in sync with the selected HOST account. A WSL-bound managed account lives inside the distro and is never synced there, so on Windows a structured session would authenticate as whatever the ambient identity happens to be while the UI names the WSL account — the user is told one identity and given another. That was unreachable only because nothing offered structured Claude on win32. Enabling it makes it reachable, so gate it here rather than patching the auth layer: refuse the structured path when the active managed Claude account is WSL-bound, and let the terminal-backed path — which resolves the account per runtime — handle that account shape. The answer rides the agentSession.createSupport seam the renderer already consumes, so no new capability and no renderer knowledge of account internals. A create the host declines becomes the definitive refusal the launch fallback already turns into a legacy native chat tab, with no error toast. Unknown answers refuse. An install with no managed accounts claims no identity and is fine, but an active selection that cannot be resolved — or account state that cannot be read at all — is not evidence that the ambient identity is right. Claude only. Codex resolves its account through a different path and its createSupport answer is untouched, as is every Codex routing decision. * Read the structured Claude account gate through the auth policy's accessor The gate resolved the active account from the account-service snapshot's runtime map; the auth policy resolves it with getSelectedClaudeAccountIdForTarget(settings, { runtime: 'host' }). Those are two sources and two resolution rules, and they disagree on a legacy settings blob that carries the selection only in the flat activeClaudeManagedAccountId: the accessor falls through to it, a direct read of the runtime map does not. The gate would then refuse a launch the policy would have run under host-1 — and in the mirror case a session could be admitted under a policy computed from a different account than the gate approved. Read the same settings through the same accessor so agreement is structural rather than coincidental, and drop the controller accessor that existed only to reach the snapshot. No behaviour change for any state both already agreed on; Codex is untouched. * Round-3 review fixes: N-1 empty-value regression, N-2 gate leak window, N-4 lost history N-1: my presence-based conflict predicate refused a terminal launch that works today. 'ANTHROPIC_API_KEY=' is how a user blanks a variable and the settings pipeline preserves that empty value; an empty override cannot beat the pinned account and the strip removes the name anyway. Back to truthiness for the value, keeping the win32 case folding. N-2: enter the live-auth gate only after the exit/close handlers that release it, so no throw in between can leave an entry nothing reconciles. N-4: the Claude transcript resolver searches config-dir-then-default and de-dupes, matching the Codex sibling in the same file, so adopting CLAUDE_CONFIG_DIR no longer hides history written before it. * Run the managed-account gate on every Claude acquisition, not just create createSupport gates the create path, but a session's account state can change while it lives. A reacquire after an unexpected child exit re-resolves the launch and re-derives auth, with nothing re-checking the gate — so a session created while supported could come back up in the refused shape. With the strip predicate keyed on there being an active non-WSL account, the WSL-only user's normalized steady state (accounts exist, none active) does not strip, and that reacquire reaches the child with ambient auth while the UI names the account. Gate at resolveLaunch, the one choke point every acquisition passes through, refusing with the pre-spawn error the caller already handles. Same predicate as create-time, now sharing one settings reader so the two cannot drift. Claude only; Codex resolves its account on a different path and is untouched. The runtime class that wires this does not typecheck its own `this` calls — a missing hookup compiles clean — so the wiring is pinned behaviourally rather than trusted to the compiler. * Move the structured Claude gate out of the @ts-nocheck runtime files Both call sites of the managed-account gate sat in files whose first line is `// @ts-nocheck`, so neither was typechecked: three arguments to a one-argument function plus an undeclared identifier compiled clean. New auth-identity decision logic had no compiler behind it. Move the verdict into a checked module that takes the two facts the runtime owns — the adapter's answer and a settings getter — and decides. The runtime class now only forwards. Move the gate reader's construction into the checked installer too, so the nocheck file passes a plain settings closure and never names a gate symbol. Every reference to the gate predicate and its reader now lives in a checked file, so the ablation that used to pass silently is a compile error at both the create-support and reacquire sites. Removing the file-level @ts-nocheck is a separate, larger job and is not attempted here. * Derive the gate test's auth policy from the settings under test A hardcoded stripAuthEnv asserts a gate/policy pairing production cannot produce, and false additionally lets launch.env inherit the runner's real process.env. Derive via claudeStructuredAuthPolicyForSettings instead: the gate settings type is the same Pick the policy takes, and both resolve the account through getSelectedClaudeAccountIdForTarget. * Pin the absent-vs-empty distinction in the managed-account gate An empty claudeManagedAccounts array is a real answer: the user has no managed accounts, nothing claims an identity, and the ambient path is legitimate. A readable settings object with no such field is settings we failed to parse — the same unknown as unreadable — so it refuses. The two are one character apart in the code and the difference is invisible without the reasoning, so record it at the branch and pin both sides. The test fails under the obvious "consistency fix" of treating a missing field as empty. * fix(claude): keep command queue bookkeeping out of the transcript Claude Code 2.1.258 emits a `command_lifecycle` frame for every uuid-stamped command it starts, completes or cancels. The frame carries a command uuid and a state and no content, and the CLI keeps it out of its own transcript -- but it is absent from the SDK's SDKMessage union and so from Orca's frame catalogue, where an uncatalogued kind defaults to a substantive row. Every structured turn therefore painted raw JSON rows into the user-visible transcript. Catalogue it and disposition it as status chrome. The unknown-kind default stays `timeline-substantive`: a kind we have never seen is likelier to carry content than to be chrome, and a visible row we can catalogue later beats content we silently dropped. A lifecycle state that reads as a failure still surfaces, because the payload error check in `classifyProviderFrame` outranks the catalogue. * fix(claude): let a re-walked descendant become eligible for the forced sweep A descendant first observed by a capture inside its own birth second could never be SIGKILLed: `ps lstart` is second-resolution, so that capture cannot rule out a pid recycled later in the same second, and the merge pinned each retained row to the boundary of the walk that first saw it. SIGTERM-resistant children forked in that window were signalled and then never escalated -- they survived close, quit and restart, reparented to init, and had to be killed by hand. Advancing that boundary on any later capture would be unsound: a later capture matching pid, pgid and start-second is exactly what an impostor would also show. But a capture is not a match -- it is a fresh ppid walk from a root Node pins through its own handle, so a row it re-derives is proved ours at that instant without appealing to its start time. Chain the fence from there instead, and take that walk at the close boundary while the root certainly still lives: the root may leave inside the grace window, and the post-timeout refresh never runs. A row absent from the later walk still keeps its earlier boundary, and a row no walk has ever re-derived in a later second is still never escalated. * Treat an absent managed-account list as empty, not as unreadable An empty claudeManagedAccounts array and a missing one are the same answer: this user has no managed Claude accounts, so nothing claims an identity and ambient auth is the truth. Refusing on absence strands any profile that simply never wrote the key, and it disagrees with the auth policy, whose own predicate takes `(accounts ?? [])` for exactly this reason. Only settings that cannot be READ stay unknown, and those still refuse — as do a WSL-bound active account and a selection naming an account the list does not explain. The earlier reasoning treated a missing field as settings we failed to parse. That conflated "not present" with "not readable"; only the second is unknown. * Support structured Claude when accounts are registered but none is selected Registered-but-deselected Claude accounts were refused, which is behaviourally identical to having no accounts at all: the auth policy does not strip, ambient auth is the truth, and the UI names no host identity. A user who deselected their accounts silently got legacy chat with nothing explaining why. Nothing selected for the host runtime is two states the settings cannot tell apart after the fact, because pruneInvalidClaudeRuntimeSelection empties the host slot and persists null in the second one: honest deselection -> ambient auth, UI names nothing -> SUPPORTED the WSL-only steady state -> ambient auth, UI names the WSL account -> REFUSED The presence of any WSL-bound account in the list decides. Simplifying this to "none active -> supported" re-opens the auth-identity misrepresentation, so the tests fail loudly on exactly that: five of them, across the unit rule and the createSupport path. * Stop treating an unanswerable create-support probe as a refusal A worktree is not resolvable for a beat after createWorktree resolves, so a probe fired immediately after creation fails the RPC with selector_not_found instead of answering. The catch collapsed that into `supported = false`, so the composer refused and quietly built a terminal session — the gate never said no, it was never asked successfully. Elapsed time was the only input that decided whether a Claude launch went structured. "Could not answer" and "answered no" are different states and only the second is a verdict. Retry while the host cannot yet resolve the selector, with a bounded backoff that covers the measured window with margin, and keep refusing on the first ask for everything else. Fail-closed is unchanged: a probe that still cannot be answered when the budget is spent refuses. The retry is narrowed with the shared error-code matcher, which classifies a token that transports re-wrap into a longer message without matching prose that merely mentions it. Codex never probes, so this race has never been able to refuse a Codex launch — the race itself is identical for it. Recorded at the early return, because whoever gives Codex a probe inherits the bug. * fix(claude): fence the forced sweep on re-derivation, not on lstart's second A descendant forked in the same wall-clock second as every walk that sees it was signalled with SIGTERM and then never escalated, so a SIGTERM-resistant child survived tab close, app quit and a full relaunch. Two children of one parent 96ms apart across a second boundary took opposite paths. The leak predates this branch: it reproduces with the change reverted. `ps lstart` has one-second resolution, so a walk landing inside a row's birth second can never rule out a pid recycled later in that same second. But a walk is not a match: a ppid walk only reaches what the root actually parents, and the root is pinned by Node's own handle, so a row the walk re-derived is ours whatever second it was born in -- a stranger would have to have been forked into our tree, and then it is not a stranger. Fence the escalation on that. Rows a merge retained from an earlier walk are not re-derived and still answer to the start-time fence, which remains correct for them. Scoped to callers that revalidate identity before signalling, which is the Claude close path. Codex teardown reaches this same verifier and is unchanged; the argument holds there too, but widening it is its own deliberate change. Also reverts two changes from the previous attempt at this leak. Advancing the capture boundary on a later walk is inert once the sweep fences on re-derivation -- both key on the same set of rows, so the new term short-circuits for exactly the rows whose boundary it advanced. The extra ladder refresh was a duplicate full process-table read: close() already awaits tree.refresh() immediately before proveClaudeChildExit, on the only path that reaches it. Known property: the kill lands roughly a grace window after the walk that proved membership, so a pid recycled inside that gap could in principle be signalled. It is bounded -- matchingSnapshotRows already requires the live row to carry the same start-second and pgid, so an impostor must be born in the remainder of that one second, land on that exact pid, and sit in the same process group, and it has already received the unfenced SIGTERM from the same loop. * Run the Claude structured integration suite as a runtime client The suite exercises agentSession.* for Claude, not the mobile surface: nothing in it asserts anything mobile-specific and its sibling integration suites use 'runtime'. Mobile now additionally requires the experimental structured-chat setting, which structured-agent-session.test.ts pins in both states, so the stale 'mobile' fixture was claiming coverage it never had. * fix(claude): report effort from get_settings, which is the only frame that has it The composer's Effort pill rendered blank in every structured session. This is not a missing source: the publication reads `effortLevel` off the `system/init` frame, and that frame has never carried an effort of any kind, while the correct value is already fetched at acquisition and thrown away on the auth diagnostic. Verified two ways -- a live get_settings probe against Claude Code 2.1.258, and the shipped binary's own init frame construction, which lists `model` and no effort. So `reportedOptions.effort` was always empty, the options reader dropped the key, and the pill had no value. Model survived only because `currentModelId()` has a fallback chain. The get_settings call acquisition already makes reports the session's current effort as `effective.effortLevel`; pass that into the publication instead. Selecting an effort already worked, so this is the arrival value only. The legacy PTY path is unaffected and must not be "fixed" to match: it reads its effort by parsing the startup banner (`CLAUDE_MODEL_EFFORT` in src/renderer/src/components/native-chat/claude-terminal-session-options.ts), which is why it shows a value where the structured path does not. Also removes the fixture that hid this: the fake init frame invented `effortLevel: 'high'`, a field the CLI does not send, which is why every gate stayed green over a value that is always empty in production. The fixture's get_settings now returns the real {applied, effective, sources} shape instead of a bare `{env: {}}`, so the two adapter tests that asserted an effort keep asserting it through the path production actually uses. The reader returns null rather than defaulting: an effort nothing measured would repeat the fixture's mistake, and a blank pill is the honest degradation if the provider ever renames the key. * fix(claude): only record an effort the child confirms it adopted apply_flag_settings answers `success` for an effort it then ignores. Measured against Claude Code 2.1.258: applying `bogus-effort-xyz` returns subtype "success" with no error while `applied.effort` stays at its previous value, and a valid `low` moves it. The option write treated the absence of a throw as adoption and recorded the requested value unconditionally, so Orca would show and persist an effort the child was not using, with nothing anywhere reporting a problem. Read the effort back after applying it, through the same reader the arrival value uses, and reject when the child reports a different one. A readback that could not be taken is not evidence of a refusal -- the apply itself succeeded -- so it still records; only a readback that disagrees rejects. Not reachable from today's picker, which offers catalog values only, but the CLI's effort catalog is server-delivered and has changed before, so a retired id would otherwise become a pill confidently displaying a setting that never took. * test(claude): assert the effort contract against the real binary The blank pill survived every gate because the only tests that touched it were fixture-backed, and the fixture invented the field. A test that pins the shape we read cannot catch the provider renaming the key, which is the failure mode that produced this defect. Asserts both halves against a live authenticated CLI: that no frame it publishes carries an effort at all, and that the session's current effort arrives through get_settings. Which frame proves the session varies by host -- this machine proves it with a SessionStart hook rather than a system/init frame -- so the negative half asserts over every published frame rather than picking one. Skips with the rest of the file when no authenticated CLI is present. * fix(claude): stop the synthesised content-part kinds leaking into the transcript Sending an image put a bare `claude · message:user:content:image` row between the user's bubble and the answer. Two causes, and only the second is a family. An image part counted as modelled only when `source.type === 'url'`, but claudeDispatchMessageContent sends a local attachment as a base64 source and the CLI replays that shape back, so every attached image was classified unmodelled. Accept the base64 and file sources Orca itself sends. The family is the real defect. `message:<role>:content:<type>` kinds are synthesised at runtime from whatever `part.type` arrives, so unlike the top-level frame catalogue they can never be enumerated ahead of time -- the `?? 'timeline-substantive'` default then prints the synthesised name at a user who cannot act on it. That default is right for top-level frames, where "substantive" means show the frame; here it meant show our own vocabulary, which drops the content AND leaks the opcode. So an unrenderable part now renders a sentence saying exactly that, with the kind and payload still on the row's disclosure. A part that carries its own readable sentence keeps it -- the placeholder is a fallback, not an override. An unknown future part type is therefore visible, never silently dropped and never printed as a kind: the same principle as the effort readback, which records only what the provider confirms. * Declare agentSession.requestHandoff on the cross-version wire surface The manifest is a ratchet for cross-version reachability, so the method is declared with real HandoffParams rather than counted. requestHandoff is capability-gated through requireStructuredHost and has no client caller, so declaring it is the whole of the change. Also model two host capabilities the harness omitted: the stub host's supportsCreate, and the fake adapter's, without which adapterSupportsCreate falls through to a supportsLocation the fake also lacks. Every ensure was refused for the harness's silence rather than for its location. * Gate structured Claude session tabs on the client capability that names them The Claude structured lane deleted the projection's `agent !== 'codex'` filter and added CLAUDE_STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY in the same commit, but never wired the constant to anything. Paired clients then received agent-session tabs for Claude, which no shipped client renders -- mobile's resolveMobileNativeChat returns null for every agent but codex, so the row listed and selected into a pane with neither chat nor terminal. Restore the filter behind the declared capability instead of the bare agent name. No client advertises it yet, so this matches main's behaviour today and becomes a negotiation a future client can opt into. * Confirm the structured Claude model against the model the CLI reports set_model answers success for any string, including a model it cannot resolve — the failure only surfaces when the turn runs — and get_settings reports the settings-file model, not the session's. The init frame that opens each turn is the only channel carrying the adopted model, so keep the session's reported model current from it instead of reading it once at acquisition. Also stop rejecting an effort the readback cannot represent: max is session-scoped and excluded from the persisted effortLevel, so a readback reporting the level underneath it is an absence of evidence, not a refusal. * Clear the session-option hedge when the provider confirms the value The pill claimed every option was unconfirmed for the life of the session: the renderer recorded each write as dispatched and nothing ever moved it, so a model the CLI had already reported back still read as unconfirmed. Carry the provider's own confirmation to the surface. Main reports which option ids the provider named rather than merely accepted, and the client re-reads options as a turn changes, because the frame that opens a turn is where the adopted model arrives. A value the provider has not reported stays hedged, including an effort whose readback could not be taken. The confirmed list is optional on the wire: a host that predates it sends nothing and the client keeps hedging, which is the behaviour it had. * Keep the model report current across an acquisition fence bump * Show the picked session-option value and let the provider report correct it The pill showed a "not confirmed" second tooltip line for any value we had sent but not yet seen reported back. Nothing acts on it, and for the PTY lane it was permanent — that transport has no report channel. The pill now shows the picked value immediately and the provider's per-turn report corrects it when the two disagree; a newer local write still outranks a report that precedes it. `dispatched` stays as a provenance member rather than collapsing into `applied`: it is produced independently by the PTY lane, and it is where the `confirmed` wire field lands, which would otherwise be unobservable. Effort keeps its readback and its rejection path. That matters more now, not less: with the hedge gone the rejection is the only user-visible failure signal on this surface, so a spurious one would be the loudest bug here. Skipping the readback for an effort the settings response structurally cannot echo is what prevents it — the response carries the persisted level, so reading it back for a session-scoped value would report the level underneath and fail a valid write. * Hedge a session-option value only when the terminal transport sent it Both lanes emit `dispatched`, so it could never say which one produced a value. The descriptor now carries the transport that built it, set once in the shared snapshot builder from a parameter that is required rather than defaulted — the builder is the only place a descriptor is constructed, so a new producer has to name its lane or fail to compile. The structured lane confirms every value from the provider's own per-turn report, which makes the hedge transient noise there. The terminal lane can only learn an outcome by parsing the screen back, and only for Claude: every other agent's `dispatched` value stays unconfirmed for the life of the session, so the line is the only signal that we sent something we never saw land. * Refuse an effort the session's model advertises no control for * Refuse tab mutations on a Claude row the client never negotiated The branch added a case asserting a client advertising only agent-session.structured.v1 may mutate a claude row. That is the same ungated behaviour the projection gate removes, encoded a second time — mutation authorization reads the projection, so hiding the row refuses the write. Assert that contract instead, and add the positive case for a client that does negotiate Claude rows. * Resolve the Claude session's current model in one place so the effort guard and the pill agree * Record an effort the child did not adopt instead of refusing the write apply_flag_settings answers success for an effort it then ignores, so the readback exists to detect that. Refusing on it made the detection a veto, and a veto is only correct if the readback can never be wrong about which model is current -- which it was, twice. The pre-flight guard already refuses a level the model advertises no control for, so the veto guarded a door that is now locked upstream. Keep the detection, drop the refusal: a disagreement records the child's own answer and omits the option from confirmed, so main stops vouching for a value the provider rejected without blocking the user's write. * Stop a slow whole-machine ps from being read as an absent process `ps -axo ...command=` pays a per-pid argv read: measured 1.15s for 1,948 processes (0.03s without `command=`), and CPU contention stretched the same capture to 6.0s. Two budgets sized for a cheap look then misreport a readable machine. The reader's 3s ceiling killed 6 of 20 consecutive captures at load 27, so every consumer answered "unverifiable" about a table it could read. Raise it to 15s, and stamp the capture instant at ps START so `capturedAgeMs` is the upper bound its contract promises -- a 6s capture used to report itself as freshly taken, understating staleness against a 5s kill gate. The TTL keys on completion so a slow capture still coalesces instead of forking ps per caller. `readStructuredTuiProcessIdentity` then spent its whole 5s wait inside one capture and concluded "no exact child" after a single look taken before the child existed (observed landing at ~3.5s). Absence needs a look that did not race the spawn, so require two captures before the deadline can end the loop. Both surfaced by the real-binary Claude TUI resume test, which failed ~1 in 5 under load; 14/14 now, 8 of those runs containing a capture the old 3s budget would have killed. * Let the desktop renderer negotiate Claude structured tabs The paired-client gate hides agent-session rows an agent the client cannot render. The desktop renderer's own IPC dispatches as clientKind 'runtime' advertising only agent-session.structured.v1, so the gate hid Claude rows from the surface this feature ships on. It renders them; it should say so. * Stop a slow process table from silently blinding every freshness gate Stamping `capturedAgeMs` at ps START made the number honest, and honest broke both consumers that read it. `ps -axo ...command=` measured 2.5-9.0s on an idle 2,002-process laptop and 4.0-18.6s at load 46, so the age it now reports lands past every budget: `planRelayPtySweep` refuses the stop as "too old", and the renderer's `admitRemoteForegroundEvidence` refuses the record outright. That second one is the expensive half and was outside the diff -- a refusal bumps `consecutiveInspectionErrors`, the poll scheduler backs off to its 10s floor, and agent-completion detection stops for the pane. The subsystem went blind on exactly the loaded hosts the honest stamp was meant to serve. The evidence-publishing read now gives up at 1,200ms instead of waiting out `PS_TIMEOUT_MS`. It is one budget for one question: these consumers ask whether an observation describes NOW, and past this it does not -- a late answer is refused by the age gate anyway, having first blocked a polled path for the whole capture, so a prompt `unverifiable` is both the truthful verdict and the cheap one. Both relay call sites already produce it from a rejection, and an admitted `unverifiable` costs a poll where a refusal costs the cadence. Identity proof keeps the full 15s through `getFreshProcessTableSnapshot`, because it asks whether a process EXISTS and must never read slow as absent. The budget bounds the wait, never the capture: the reader coalesces, so an abandoned wait leaves its capture running to fill the cache rather than forking a second whole-machine `ps` on the host that can least afford one. 1,200ms is bracketed rather than picked. The floor is the capture's own cost -- `command=` measured 1.15s for 1,948 processes on an idle host, and a budget under that answers `unverifiable` about a machine nobody is straining. The ceiling is the consumer's: 2,000ms, less the 500ms a TTL-shared capture may already have aged, leaves 1,500ms, and transit takes the rest. That ceiling only fits once the capture stops being charged twice. `ps` runs inside the RPC round trip, so its duration is already in `receiveDelay`, and `capturedAgeMs` is that same duration on the host's clock; summing them halved the budget this gate grants a host from ~2.0s of `ps` to ~1.0s, which is why a 1.2s capture arriving at 1.3s read as 2.5s old and was refused. Admission now takes the larger of the two. The sweep's gate keeps its sum, which is correct there: `evidenceAgeSinceListingMs` is stamped after the listing ARRIVES, so it measures planning time and overlaps nothing. A stated limit rather than an assumed one: 15s is not proven sufficient for identity proof. The same capture reached 18.6s at load 46, so that path can still time out and answer "no exact child" about a host it simply could not read in time. Narrowing it needs a cheaper question than a whole-machine argv read, not a larger number. The one test guarding this field could not fail. `beginPtyHandlerTest` installs fake timers, so `Date.now()` is frozen, the real reader reports exactly +0, and `0 <= 500` held identically for a hardcoded zero, for completion-stamping and for start-stamping -- while the real reader on that host returns thousands of ms. It now drives a measured age in and asserts the handler publishes it rather than restamping; that the reader MEASURES it correctly stays pinned separately, against a controllable clock. Both consumers get boundary coverage either side, and each new gate was ablated red before it went green. * Keep the compatibility fields off the capture the budget just abandoned inspectProcess falls back to processHasChildren and listProcesses to getForegroundProcessName, and both read the same TTL-shared capture with no budget of their own. On a slow host they joined the in-flight capture the budgeted evidence read had just given up on, so the call still blocked for the full 6-18s and the budget bought nothing -- once for inspectProcess and once per managed PTY for listProcesses. Use the degraded answers those helpers already give for an unreadable table, reached promptly. pty.hasChildProcesses keeps its unbudgeted fresh probe: it is a one-shot destructive gate that can afford to wait. --------- Co-authored-by: Merge Sim <merge-sim@local> Co-authored-by: Merge Sim <sim@local>	2026-09-04 15:55:20 -07:00
Jinwoo Hong	3e4fd4a7af	Shorten orchestration skill description under the Agent Skills 1024-char limit (#18683) ... * Shorten orchestration skill description under the Agent Skills 1024-char limit The folded description was 1038 chars, so spec-conforming installers such as SkillStar rejected the bundled orchestration skill. Drop the two clauses already covered elsewhere in the same description: "decomposing work across agents" (implied by "structured multi-agent coordination") and "automation of the browser embedded inside Orca" (restated by the locked `orca-cli` embedded-pages sentence). Every routing trigger asserted by orchestration-skill-guidance.test.mjs, the orca-cli handoff boundary, and the Computer Use boundary are unchanged. Result: 958 chars. Add config/scripts/skill-description-length.test.mjs, which parses every skills/*/SKILL.md frontmatter with `yaml` and fails on an empty or >1024 char description, so the regression cannot return. orca-cli sits at 1015 and is left as is. Fixes #17935 * Keep the embedded browser in the orchestration description's orca-cli routing Restores the word "browser" in the orca-cli sentence ("and the Orca embedded browser") so agents scanning for it still route embedded-browser control to orca-cli. Description is 985 chars, 39 under the spec limit.	2026-09-04 14:27:07 -04: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	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	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
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	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
foXaCe	49d6d35b16	feat(i18n): add French UI locale ... foXaCe <290678+foXaCe@users.noreply.github.com>	2026-09-03 17:32:59 -07:00
Neil	04ae62202a	fix(ssh): close the macOS relay's per-terminal pty fd leak (#18534) ... The relay asset from #17920 only rewrote the forkpty `default:` call site, which sits in the `#else` arm of PtyFork's `#if defined(__APPLE__)`. macOS takes `pty_posix_spawn`, so the asset had never patched anything a Mac executes -- and `applyNodePtyMasterCloexecPatch` returned 'fixed' for any non-Linux host without running the script at all, which is what publishes a tree to the shared native-deps cache. Stock `pty_posix_spawn` opens up to three throwaway ptys to push the real master off fds 0-2 and never closes them: the cleanup loop is `for (; count > 0; count--)`, but the first `posix_openpt()` in a running process already returns >= 2, so it breaks with `count == 0` and the body never runs -- and where it does run it closes `low_fds[count]`, never `low_fds[0]`. One orphaned /dev/ptmx fd per terminal, for the life of the relay. Ports the `low_fds` fix and the Apple-branch `pty_cloexec(master)` call from the app's `config/patches/node-pty@1.1.0.patch`, byte-identical, and runs the gate on darwin. macOS needs a different build layout than Linux: it has no `build/` at all, so the fallback moved aside is `prebuilds/darwin-<arch>` -- which is also what makes node-pty's install script fall through from "prebuild found" to node-gyp -- and the compile writes a `build/Release` the loader checks first. Verification is per-platform too: Linux's leak is inheritance (/proc), macOS's is self-held (lsof). Also corrects the asset's claim that "macOS re-opens the tty through uv_tty_init's cloexec dup". Measured false: FD_CLOEXEC is not set on the master. What protects it is POSIX_SPAWN_CLOEXEC_DEFAULT, one option away from gone since uid/gid drops libuv back to fork()/exec() -- so the master is now marked there too. Measured on darwin-arm64, one PTY per open/close cycle in a relay-shaped dir running the relay's own commands: before cycle:ptmx 1:1 2:2 3:3 ... 10:10 (10 after a settle) after cycle:ptmx 1:0 2:0 3:0 ... 10:0 (0 after a settle) Linux re-verified in docker node:22: inherited before, isolated after, `already-patched` on the second run. Refs #17915 Refs #8362	2026-09-03 16:13:07 -07:00