731 Commits

Include renames

Author	SHA1	Message	Date
7 people	09073086a8	feat(terminal): inline images via @xterm/addon-image (perf-first) (#19512) ... * feat(terminal): inline images via @xterm/addon-image, perf-first Add opt-in inline terminal images (SIXEL, iTerm2 IIP, Kitty graphics) through @xterm/addon-image, designed to keep idle terminals unaffected. Performance: - The addon (base64-inlined wasm decoders + protocol handlers) loads off the boot critical path via a deferred loader that mirrors the WebGL addon: primed after first paint only when the setting is on, read back synchronously at attach, with a 3-attempt cap so a transient failure never disables images for the session and a missing chunk never refetches per pane. renderer-boot-graph guards against eager import. - enableSizeReports:false so the addon never sets windowOptions and double-answers Orca's own CSI 14t/16t responder. - Perf-tuned decode/storage limits (storageLimit, sixel/iip/kitty size caps) in one place. Correctness: - Orca's DA1 handler wins over the addon's (last-registered-first), and the default DA1 response never advertised Sixel (;4), so DA1-detecting tools (chafa, img2sixel, viu, timg) never emitted it. The winning handler now appends ;4 while the setting is on, resolved per query so a live toggle changes the next DA1; idempotent against the ConPTY response that already lists it. - ORCA_IMAGE_PROTOCOL=kitty is exported to spawned shells (local, daemon, relay/SSH) and forwarded across the WSL boundary, so image-capable agents can pick an encoder. Unknown image sequences are swallowed by xterm when the addon is detached, so this never garbles output. - Settings toggle (default on) gates rendering and DA1 advertisement. Cross-checked against community PRs #7775, #11706, and #19201 at the end; credited below. Co-authored-by: s546126 <s546126@users.noreply.github.com> Co-authored-by: XRX193 <XRX193@users.noreply.github.com> Co-authored-by: lmsh7 <lmsh7@users.noreply.github.com> * fix(terminal): bound inline image memory and classify Kitty replies * fix(terminal): bound image decode and release image resources on cleanup * fix(terminal): address image addon review feedback * test(terminal): stub setPaneInlineImagesEnabled in appearance manager fakes * fix(terminal): evict unplaced kitty payloads before displayed images Byte-budget eviction dropped the oldest transmitted blob regardless of placement, so a new upload could erase a visible image while abandoned blobs still held budget. Unplaced payloads now go first and displayed ones only when that is not enough. The incoming image is always stored, so an oversized one overshoots the cap by one payload instead of being dropped after the protocol already acked OK. * fix(terminal): gate DA1 Sixel on real addon attachment; claim SSH image spec in CI - DA1 advertised Sixel from the setting alone, so a pane whose lazy addon chunk was still loading (or had failed all three attempts) told feature-detecting tools to emit DCS that nothing could render. Track the attached decoder per terminal and require it before setting the ;4 bit. - tests/e2e/terminal-inline-images-ssh.spec.ts was Docker-gated but claimed by no lane runner, so pr-e2e-gate-contract failed and the spec would have self-skipped green forever. - Reject non-positive PNG IHDR dimensions before decode: they are parsed with signed shifts, so a dimension >= 0x80000000 came back negative and slipped past the pixel-limit comparison. - One resolveTerminalInlineImagesEnabled() for the default-on setting; the four call sites mixed '?? true' with '!== false', which disagree on null. - One readInlineImageResources() walk of the addon internals instead of two copies that could drift against the patched dependency. - Isolate the deferred-attach drain per pane; make the zoom-invariance and backing-storage e2e assertions fail when the feature is dead. * refactor(terminal): one lazy xterm addon loader for webgl and image terminal-image-addon-loader was a structural clone of the webgl one — same memo, attempt cap, and .then(ok,err)-clears-memo recovery. Both now wrap createLazyXtermAddonLoader; each keeps its literal import() specifier so the bundler still splits the chunk (verified against a fresh build: addon-image stays out of the boot graph). * refactor(terminal): name openTerminal's addon flags; pin image addon limits Two adjacent optional booleans could be swapped without a type error once inline images added the second one. * docs(terminal): state the real per-pane image ceiling; drop test ordering dependency storageLimit:32 reads like the pane's budget but keys three pools — decoded pixels, retained encoded Kitty blobs, and pending WASM decoders — so the worst case is ~98 MB per pane with no cross-pane governor. Say so at the constant. pane-inline-images.test.ts's deferred case needed to run first; it now takes a fresh module instead, and the rest prime in beforeAll. Verified by running the file with that test moved last. * fix(terminal): satisfy rebased static analysis gate * fix(terminal): complete casting gate cleanup * fix(terminal): recover failed image addon loads * fix(terminal): bound image decoder allocations --------- Co-authored-by: m4air <m4air@m4airs-MacBook-Air.local> Co-authored-by: s546126 <s546126@users.noreply.github.com> Co-authored-by: XRX193 <XRX193@users.noreply.github.com> Co-authored-by: lmsh7 <lmsh7@users.noreply.github.com> Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com> Co-authored-by: Neil <neil@stably.ai>	2026-09-18 16:32:49 -07:00
Jinwoo Hong	209d2d8df6	build(mobile): split the Route A page into per-route chunks (OTA phase C, C1.5) (#21475) ... * build(mobile): split the Route A page into per-route chunks (OTA phase C, C1.5) The page bundled as one 8.16 MB script because every route was a static import. The route manifest now defers each screen behind `import()`, the build is esm with splitting on, and the document loads the entry as a module. What the browser parses before the first route can paint drops from 8.16 MB to 908 KiB; the whole page still weighs the same. Two budgets hold it: the chunk count, which catches a split running away, and the bytes the entry reaches by static import, which catches it collapsing back. The second is the one that matters, and it is measured from esbuild's metafile because only that says which import is static. The RequireContext stays synchronous, since expo-router reads keys() to build the route tree before anything renders. A lazy module cannot answer `unstable_settings` or `ErrorBoundary`, which expo-router reads off the namespace, so a test holds that no route in the subtree exports either. The render check now waits for the route's own text: the entry's mount signal lands while the route chunk is still being fetched. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): read a route's synchronous exports from esbuild, not a regex `export { x as ErrorBoundary }`, `export class ErrorBoundary` and a re-export all reach the namespace without matching the declaration pattern the guard was matching, so the lazy manifest dropped the boundary and the page painted blank. A star re-export is now reported rather than read as clean. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): say that the entry budget is not a per-route opt-out Measured: statically importing one route already breaks the 3 MiB bound for 5 of the 14. The hatch only works for a layout node, which is the only place expo-router reads a synchronous export from. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * build(mobile): derive the chunk ceiling from the route count 64 was three routes of headroom over the 53 chunks 14 routes measure, so C2's routes would have failed on a number measured before they existed. Four per route plus 16 tracks the measured slope; the entry-bytes bound stays the real budget. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): drop the web entry's dead suspense boundary expo-router wraps every screen in its own, so this one never fires; all nine render checks stay green without it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): pin that a client-side navigation fetches the next route's chunk Goes red with splitting off: the tasks screen paints out of the entry and no new script is fetched. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): name every bundle output by its bytes, not by esbuild's path hash esbuild's [hash] is over the metafile's input keys, which are paths relative to absWorkingDir, so a checkout at another depth or with node_modules as a symlink named a byte-identical chunk differently and shipped a different buildId for one commit. Outputs are now renamed leaves-first to the sha256 of their final bytes. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): fail the build on a route the lazy manifest would strip The guard ran only in a test while the docstring said it failed the build. It now runs in bundleMobileWebApp and names the route and the export. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * build(mobile): derive the asset ceiling from the chunk ceiling and the images A flat 128 stopped agreeing with the chunk ceiling at 18 routes, where the asset count would have failed first and named the count instead of the split. Chunks plus images plus the document keeps the chunk ceiling the one that trips. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): split the route-manifest tests out of the bundle builder's The builder's test file passed 600 lines. The route manifest, the synthesized RequireContext and the web entry are their own subject and move together. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): give the export guard the builder's route-source loaders Without .js as jsx the guard reported a React Native .js route carrying JSX as "JSX syntax extension is not enabled" instead of reading its exports. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): assert the navigation fetches the tasks route's own chunk "some new script arrived" passed on any fetch. The builder now names the chunk each route lands in, read off the metafile, and the check asserts that exact path arrived and was not already loaded. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): resolve a route's realpath before matching it to its chunk esbuild writes metafile input keys after resolving symlinks, so every scratch route tree under /var on macOS reached no output and failed the build. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): fail the build when the asset ceiling outgrows the shell's map The derived ceiling had no upper bound, and the native shells return null for a manifest over their own 256 rather than truncating it. At 42 images the formula crosses that at 50 routes, inside what Phase C adds, so the build would stay green while the phone got nothing. The number is read from the contract through esbuild, not restated here. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): cover the two hard stops in the content-addressed naming Both throws only ran through a whole bundle before, where neither can be provoked. A cycle and a route no output claims are now asserted directly; each test goes red when its throw is removed. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): exit the app-bundle build on one line, not a stack The route-export guard fails this script by design, and a raw stack put the route and the export name under twelve frames of node internals. Mirrors the verifier's exit; the message is printed as thrown because every throw on this path already names its source. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-18 11:58:42 -04:00
Jinwoo Hong	381a3da46f	feat(build): Route A, the phone's host routes bundled for the web, dark (OTA phase C, C0.7) (#21449) ... * refactor(mobile-web): share the bundle manifest assembly with a second builder Manifest assembly and the on-disk write move to writeMobileWebBundleTree, and the helpers the Phase C app builder needs become exports. No behaviour change to the shipped bootstrap bundle. The CRLF guard grows two exemptions it needs once it is pointed at mobile/src: the image and font extensions .gitattributes already pins -text, and the gitignored webview engine modules the postinstall writes. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): web entry for the host route tree, and its two transport siblings The entry mounts app/h on react-native-web through expo-router's own ExpoRoot. It lives inside mobile/ so one React resolves, and supplies RpcClientProvider itself: the route tree starts below the native root layout that owns it. route-manifest.ts is a real typed module whose body the builder replaces -- esbuild has no require.context. A virtual specifier would need an ambient declaration and would leave the entry unchecked. Two .web.* siblings, both listed with a reason in web-overrides.json: the transport substitution point (a placeholder client until C0.4 lands BridgeRpcClient) and the device token store, whose native path imports expo-secure-store, which is {} on web. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(build): build:mobile-web:app, the phone's host routes bundled for the web Same builder shape as the Phase A bootstrap into a separate out/mobile-web-app, with the same manifest and the same two-scratch-build determinism check. Dark: build:mobile-web, packaging and the A2 census are untouched, and C1 is what flips build:release. Six shims, each a named Metro or RN Web gap. Images are emitted as same-origin hashed assets rather than data: URLs, because the shell's CSP sets img-src 'self'; the render check under that exact header is what found it. The script is referenced root-absolute for the same reason a <base> tag cannot be used: the document is served at every route depth and base-uri is 'none'. The budget sits below the contract's per-asset ceiling so growth trips a build rather than a refused asset on a phone. esbuild splitting does not lower it: one entry with only static imports emits one chunk (measured). Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): let React Native Web paint under the shell CSP RN Web 0.21.2 injects its stylesheet at runtime with no nonce support, so style-src 'self' blocks every rule and the page renders unstyled. Measured, not predicted: the render check serves the document under this exact header and reported the violation. 'unsafe-inline' is granted to style-src and nothing else. script-src 'self' holds, which is the directive that decides whether page code can arrive any way other than as a fetched same-origin script. The test now pins that scoping rather than rejecting the token everywhere. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * ci: prove the Route A app bundle on every PR A dedicated job, for the same reason the browser provider has one: it needs mobile/node_modules and a real browser, and the sharded test matrix would pay for both on every shard. It builds the bundle, verifies it, and runs the builder, override-census and render suites. It ships nothing. The mobile_web_app signal is lifted out of should_run the way static_analysis is. A mobile-only diff is desktop-irrelevant and skips every gated job, and that is exactly the diff that changes the page this job builds. Also the C0.6 review follow-up: mobile/package.json and mobile/pnpm-lock.yaml join the installer cache keys in the two workflows that build an installer off a hashFiles key, since beforePack requires out/mobile-web and a mobile-only change must miss those caches rather than reuse a stale build. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): pin the shipped builder against the app builder's own module name The assertion named a specifier that no longer exists, so it held vacuously. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): assert the RN Web style-src grant in the Swift checks The Swift twin of the Kotlin CSP test still required style-src 'self' and no unsafe-inline anywhere, so it trapped on the approved grant. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): make the Route A render check name what each route paints The check asserted only "some html, no errors", which expo-router's Unmatched screen satisfies: pointing HOST_ROUTE at /zzz/not-a-real-prefix stayed green. Each route now asserts content only its own component produces, and the unmatched case asserts the screen positively so the negatives discriminate. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): read the shell CSP past the comments that quote directives Both constants document themselves with // comments containing quoted directive text, which the quoted-string scan picked up as directives. One parser now drops comment lines, and iOS and Android go through it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(build): honour a .web.* route sibling in the app bundle Routes were imported by absolute path with the extension, so esbuild's resolveExtensions never applied and a .web.tsx under app/ was dead code the census still accepted. The manifest now carries a key and a module: the key stays the native filename so the URL does not move, and the module is the web sibling when one exists. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): tie each named shim to the esbuild option that implements it The shim list was asserted against a literal copy of itself, which passes however the build is configured. Each entry now carries an appliesTo that reads its own option, checked against the real options object. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * chore(build): line up the CRLF exemptions, the budget comment, and the job scope The builder loads .gif as a file but neither .gitattributes nor the CRLF scan exempted it, so the blanket eol=lf pin would have rewritten one. A test now keeps the two lists in step. The Phase C byte budget's comment sat on the asset count, and a root package.json edit could change build:mobile-web:app without running the job that proves it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * style(build): satisfy the index-check lint rule in the CSP parser Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * ci: key the installer caches on the mobile page trees too beforePack builds the mobile web bundle into the installer. Today those bytes are Phase A's, which src/** already covers, but once C1 flips the entry to mobile/app a page-only change would hit a cache holding a stale installer. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): skip the bundling tests where mobile dependencies are absent The sharded `test` job collects config/scripts/**/*.test.mjs and installs no mobile dependencies, so the two new suites failed there on "Could not resolve react-native-web". They now skip themselves with a message naming the job that runs them, and that job sets ORCA_MOBILE_WEB_APP_DEPS_REQUIRED so a missing install fails it instead of skipping everything it exists to prove. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(build): scan mobile/packages in the .web.* census The census claimed the app entry never resolves into packages/, but the dictation hook imports @orca/expo-two-way-audio and the built script carries ExpoTwoWayAudioModule.web.ts. That file is now listed with its reason, and planting a .web.* in each scanned tree proves the scan is not passing because a tree happens to be empty. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): assert the route exclusions against a tree that has them mobile/app holds no test, spec or +api file, so the exclusion rule was asserted against a tree it could not fire on. A scratch tree plants one of each; dropping the rule now fails this test. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): 404 unknown file paths in the render check's page server The server answered every path with the document, so pointing publicPath at /wrong-prefix still rendered three green routes: the script is fetched from the one prefix that is served. A path naming a file now has to come out of the bundle, which is what the shell's manifest map does. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): cover the app bundle verifier's own checks The verifier had no test. One doctors the buildId, which the packaged assert catches; the other rewrites the tree so every digest still agrees and only the two fresh builds can tell, which is what a stale out/ looks like. Deleting either check now fails a test. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * chore(build): tidy the app bundle comments and the job's path prefixes Drops an export nothing read, merges two comments that had drifted apart from the constant they describe, and corrects the claim that the job runs on every PR when it is path-gated. package.json leaves the prefix list because GLOBAL_FORCE_FILES already forces every job on it; mobile/packages/ joins it, since the page resolves a .web.ts out of there. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * style(build): merge the duplicate node:fs/promises import in the census Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): redirect the hybrid shell route on the web page app/h/[hostId]/web.tsx reaches OrcaMobileWebShellView, whose module calls requireNativeViewManager at import. In a browser that throws before React mounts, and the route manifest imports every route statically, so one native route left the whole page blank at every URL. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): fail the render check with the error that stopped the mount The check waited on "#root has children" with Playwright's animation-frame polling, so a route module that threw at import read as a bare 30s timeout naming nothing. It now waits on a mount attribute the entry sets after the router commits, polls on a timer, and races the wait against the first uncaught error so the failure carries it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): answer the favicon the render browser asks for CI resolves the runner's Google Chrome, which requests /favicon.ico; the bundled headless shell does not. The bundle carries no icon, so the server answers 204 rather than turning a browser habit into a console error the render assertions read as a page fault. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): settle the render check's uncaught-error race without rejecting The entry throws during goto, before anything awaits the race, so a rejected promise surfaced as an unhandled rejection beside the real failure. The same signal now resolves with the error. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * chore(mobile): list the page transport in the raw request port inventory The placeholder client implements the port, so the boundary test counts it as an unlisted file. It belongs under OWNERS until C0.4's BridgeRpcClient replaces it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-18 09:50:37 -04:00
Jinwoo Hong	ca2ae89011	ci: install mobile dependencies in every desktop packaging job, pin mobile page source to LF (OTA phase C, C0.6) (#21425) ... * chore(mobile): pin mobile page source to LF so a Windows checkout keeps buildId The Phase C web bundle hashes every text byte under mobile/src and mobile/app into its asset digests and from there into buildId. There is no global text=auto, so a CRLF checkout on Windows would give the Windows release a different buildId for identical source, the same failure the src/mobile-web pin above exists for. All 1924 tracked files in those directories are already LF in the index, so the pin renormalises nothing. mobile/web-entry does not exist yet; the pin is forward-looking for the Phase C entry point. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * ci: install mobile dependencies in every desktop packaging job Ten workflows reach build:release/build:desktop and none of them installs mobile/node_modules. Root has no react-native, react-native-web or expo, so once the mobile web bundle builds from mobile/ its packaging jobs would fail at electron-builder's beforePack with an unresolvable import. Extract the frozen mobile install that pr.yml's static analysis job already ran inline into .github/actions/install-mobile-dependencies, and invoke it from every job the packaging census enumerates, after the root install and before the build. Same --frozen-lockfile, same lockfile-drift guard, and still no --ignore-scripts: mobile's postinstall generates the gitignored webview engine modules that tracked source imports. pr.yml now uses the action too, so there is one definition. Where a packaging job's setup-node caches the pnpm store, mobile/pnpm-lock.yaml joins cache-dependency-path so a mobile lockfile change invalidates it. Two jobs (daemon-relocation-spike, win-update-survival-e2e) do not cache at all and are left alone. The census test grows a per-job assertion that the action is present, so a new packaging job has to add the install deliberately rather than discover it at beforePack. release-cut's composite-action restore is no longer Windows-only: every platform consumes this action now, so any of them can be the leg whose cut ref predates it. No job builds anything different; this only makes mobile/node_modules present. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(ci): assert the mobile install contract on the shared action pr.yml's static analysis job no longer carries the install inline, so the scope test's findIndex by step name resolved to -1. Match the step by the action it uses, and read working-directory and --frozen-lockfile off the action itself so the job cannot keep the step while the action stops installing anything. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * chore(mobile): exempt binary asset types from the mobile LF pin /mobile/{src,app,web-entry}/** text eol=lf would mark a future PNG or font as text and rewrite its bytes on a Windows checkout. Exempt the asset types an RN page carries, the same way src/mobile-web exempts its PNG. -text after text eol=lf wins: probed a CRLF-bearing .png under the pin, it stays i/crlf attr/-text while a sibling .ts still normalises to i/lf. No tracked file changes classification; the 1924 files under mobile/src and mobile/app stay i/lf. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * ci: gate the mobile install with the build it feeds in the cached lanes win-crash-survival, win-update-survival and daemon-relocation-spike all skip electron-builder on an installer/unpacked cache hit, so an unconditional mobile install spent time on node_modules nothing then consumed. Move each `uses:` below its cache step and carry the same cache-hit condition as the build. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-18 05:44:11 -04:00
Jinwoo Hong	f819ed96ca	fix(skills): keep the disposal verdict when staging cleanup fails, and retry release-cut installs (#21366) ... * fix(skills): keep the disposal verdict when staging cleanup fails `begin()` ended with `await this.removeOwnershipIfDisposed()` inside its `finally`, so when a caller raced `dispose()` the rejection it received was whatever that opportunistic `rmdir` threw -- not `skill-upload-service-disposed`. A caller could not tell "the service shut down" from "the filesystem broke", and the Windows release gate saw it as `EPERM: operation not permitted, rmdir`. Two causes, both fixed here: - The EPERM itself: an in-flight operation and disposal each call `ownership.remove()`, so two `rm -rf` run concurrently against the same owner directory. On POSIX the loser reads ENOENT and `force: true` swallows it; on Windows the loser reads a delete-pending directory and gets EPERM. `SkillUploadStagingOwnership.remove()` now joins one removal and forgets it on failure so a later caller still retries. - The masking: cleanup in a `finally` no longer replaces the outcome of the call it is cleaning up after. Disposal retries staging removal and reports its own failure, matching `removeUnpublished`/`retainFailedCleanup` in this class. Both regressions are pinned platform-independently: one injects a failing ownership removal and asserts the racing `begin` still rejects with `skill-upload-service-disposed` while `dispose()` reports the cleanup failure; the other models Windows delete-pending rmdir in the `node:fs/promises` mock, which turns a second removal into EPERM on every platform. * ci(release-cut): retry the installs that fetch node-gyp headers `golden e2e windows` installs with lifecycle scripts enabled, so pnpm runs node-gyp for the `native/windows-registry` workspace project, which downloads that Node version's headers from nodejs.org. A single `read ECONNRESET` on that fetch failed a blocking release gate, and the release build job one screen below already wraps its install in `nick-fields/retry@v4` for exactly this class of failure. Both remaining unretried installs in this workflow (the blocking platform golden and the non-blocking rendering-evidence lane) now use the same wrapper, and a contract test keeps every release-cut install retryable.	2026-09-18 03:12:46 -04:00
Neil	8d2f16856f	fix(session): scope agent resume to the host that captured the session (#21288) ... * fix(session): scope agent resume to the host that captured the session A provider session id names a transcript in one machine's agent state directory. Nothing in the resume path compared that machine against the one the resume executes on, so a record captured on host A reached a `--resume` run on host B, which answers `No conversation found with session ID`. Three things make the drift reachable: `worktreeId` is `repoId::path` with no host component, sleeping records are `'sleepingAgentKeyed'` so boot-time host-contention parking never arbitrates them and every partition merges into one map without retaining provenance, and both issuers resolve their launch target from the current catalog. Both issuers are gated. The activation sweep hands `quit`/`live` records whose pane still exists to the pane's own cold restore, so gating the sweep alone changed nothing in the SSH lane. Declines rather than guesses: the record is preserved and remains resumable by hand. A refused resume is recoverable, a forked transcript is not. The predicate fails open on anything it cannot positively rule out -- an unstamped record, an empty stamp, or a `runtime:` host, which a paired client uses to relabel its host's own SSH workspaces. The cold-restore gate consults both the pane's transport and the catalog. The transport alone was racy: it is unresolved on an early reattach frame, and that frame is exactly when a wrong resume escaped. * docs(session): name the inverted fail-open direction at the resume gate * fix(session): keep an unresolved catalog out of the resume host verdict The worktree form of the resume gate resolved the current host through getExecutionHostIdForWorktree, which answers 'local' for a worktree the catalog has no row for. Read as a host, that made every SSH-stamped record look foreign until its repo row landed, contradicting the module's own contract that it reports only a positively-known disagreement. Add getKnownExecutionHostIdForWorktree, which returns null in that silence (no repo row for a git worktree, no folder-workspace row for a folder workspace), and route the gate through it; the pair form already fails open on a null host. The routing resolver keeps its default unchanged. The CI red on the control case was a separate spec race: the ledger wait returned as soon as the ledger was non-empty, and it already held the first launch's `--version` probe, so the control read two probes and gave up before the cold-restore had typed `--resume` (the failure screenshot shows the command running in the pane). The spec now reads only the lines the relaunch appended, anchors on the relaunch's PTY binding and its own probe, and then waits for `--resume` for the control case or a bounded grace for the refusal case.	2026-09-17 22:12:55 -07:00
Jinwoo Hong	9641a1b544	feat(mobile-web-bundle): serve the packaged mobile web bundle over RPC (OTA phase A, 3/5) (#21348) ... * feat(mobile-web-bundle): serve the bundle manifest and chunks over RPC Two paired-runtime methods on the already-authenticated connection: `mobileWeb.bundle.manifest` returns this install's manifest plus the chunk size it advertises, and `mobileWeb.bundle.chunk` returns one aligned range of one asset with the whole asset's length and hash, so a single chunk describes what it belongs to. `path` is accepted only by exact match against a manifest member, so traversal is unreachable rather than mitigated. Each asset's on-disk sha256 is verified once and the verdict remembered, concurrent first readers sharing one hash. Reads are capped at four in flight per connection, and a disconnected client stops costing reads at the next checkpoint. No SSH or relay proxying: a runtime answers only out of its own install. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile-web-bundle): pin the three buildId serializers against each other The canonical serialization exists in the builder, the packaging guard, and the shared contract, because the two packaging scripts run on bare node before any build output exists and cannot import TypeScript. A divergence in any one would reject every honest bundle at packaging, or ship a bundle whose id the phone recomputes differently and re-downloads forever. Proved red by swapping the guard's code-unit sort for localeCompare: five of six cases fail. Exports the guard's serializer for the test; no packaging behaviour changes. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile-web-bundle): cover every error code and a multi-chunk paging round trip Against a synthetic bundle in a temp dir, because the real builder's largest asset is under one chunk and CI unit jobs never build out/mobile-web. The fixture's script spans three chunks, its stylesheet is exactly one, and one asset is empty, so paging, the eof boundary, and the zero-byte case are exercised rather than assumed. Reads in flight are held by latching `open`, so the four-per-connection cap and an abort arriving mid-read are deterministic rather than a race with a stopwatch. Both were proved red: dropping the abort check after verification fails the abort case, and keying the cap on connectionId alone fails the device-token case. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile-web-bundle): resolve the bundle root through the AppEnvironment port check:runtime-electron-ratchet caught this: the resolver sat beside getBundledWebClientRoot in src/main/startup and imported electron, and importing it from an RPC method pulled the first electron edge into a runtime graph whose baseline is zero. The runtime has to stay bootable on plain Node. So it reads app.getAppPath() through the port every other runtime module already uses, and moves next to its two callers under src/main/runtime. A host with no environment installed has no install root, which is the same answer as having no bundle. orcad answers getAppPath from its own install root, so a headless runtime that carries the artifact serves it with no special case. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile-web-bundle): cover the resolver's two probe layouts directly Also stops exporting the manifest filename, which nothing outside the resolver needs. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile-web-bundle): pin both methods on the mobile allowlist The scanner only checks mobile-used ⊆ allowlist, and no mobile source calls these until A5, so deleting both entries left every test green. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile-web-bundle): keep filesystem failures inside the six error codes An asset unlinked or truncated after its verdict was cached reached the client as runtime_error carrying the desktop's absolute install path. Both now answer mobile_web_bundle_asset_changed, with the cause warned host-side only. A short positional read is the truncation case, so it throws instead of paging the client past the end. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile-web-bundle): drop the unreachable release-idempotence guard The one caller releases exactly once in a finally; removing the flag left every test green, so it was defensiveness against a caller that does not exist. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile-web-bundle): prove a failed verify is not cached as a verdict The verdict cache never invalidates, so a transient read failure remembered as a verdict would poison the asset for the life of the process. Removing the delete left every test green until now. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile-web-bundle): delete the unsatisfiable manifest params schema The dispatcher substitutes `{}` for absent params, so `z.null()` could never parse; the method declares `params: null` instead. A comment on the method name records why there is no schema. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile-web-bundle): fill the read window instead of failing a partial read fs.read may answer short of what it was asked for before EOF, so the previous check turned a legitimate partial read into a spurious asset_changed. The loop mirrors the relay's readFullStreamChunk, which is not imported because it sits behind the relay dispatcher's module graph; only a read returning nothing is treated as truncation. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile-web-bundle): read the disconnect idiom with the shared predicate isClientDisconnectedError already exports exactly the check the catch needed, so the local error class goes away and the throw returns to the repo-wide idiom. The module doc now says asContractError is a total catch. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile-web-bundle): pin the four branches no test was holding Each one survived a mutation: the abort check before verification, the per-process manifest cache, the buildId component of the verdict key, and delete-at-zero in the admission map. The last two matter beyond hygiene — a verdict keyed by path alone carries a failed verdict onto the next build of index.html, and a map that never drops a key retains one pairing token per socket. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-18 01:04:27 -04:00
Neil	066b4951b9	fix(terminal): keep a split's real direction when the leaf set moves (#21294) ... resolveTerminalLayoutRoot discarded any known tree that did not cover the published leaf set exactly and rebuilt the tab as a flat chain with a guessed 'horizontal' direction, restacking side-by-side panes. The guess is then published, mirrored to every paired client, and written back over the real tree, so the direction is gone from disk. Prune a known tree to the leaves that survive and graft only the leaves no tree places, which is now the sole place a direction is invented and is still reported through onSynthesize.	2026-09-17 21:21:01 -07:00
Jinwoo Hong	ad4f26cdd4	feat(build): build, verify and package the mobile web bundle with every desktop release (OTA phase A, 2/5) (#21326) ... * feat(mobile-web): add the Phase A bootstrap web source A peer of src/ so the root workspace owns it and mobile's separate lockfile stays out of packaging. Four assets across four content types, enough to exercise multi-asset manifest handling rather than assume it. The page reads buildId from manifest.json at runtime: buildId hashes the asset list that index.html belongs to, so injecting it into a hashed asset would make that asset's hash depend on itself. Registered as a fourth typecheck project; without it the entry would be the only TypeScript in a release path that tsc never sees. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(build): build and verify the mobile web bundle from the root workspace Root esbuild over mobile-web/ into out/mobile-web/, content-addressed as assets/<sha256>.<ext> with index.html the only stable name. buildId is the sha256 of the canonical serialization of the sorted asset list, so it is a pure function of content and usable as a cache key with no further reasoning. The verifier builds twice into scratch dirs and compares: a timestamp, an absolute path, or an unstable ordering fails the build when someone introduces it, not the first time a phone gets a spurious cache miss. It also enforces the Phase A budget of 16 assets and 256 KiB, separate from the permanent contract ceiling. build:release does not call build:desktop, so build:mobile-web is wired into build:desktop, build:release, and build:release:parallel. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(packaging): fail the release when the mobile web bundle is missing or stale electron-builder only warns about a missing input, so without a beforePack guard a release ships an app that advertises the bundle capability and then errors on every request. The hash check, not the existence check, is what catches a half-written or stale out/. The source tree is excluded from app.asar; out/mobile-web ships inside it under the existing out rules, exactly as out/web does. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile-web): narrow the manifest with `in` instead of a cast The changed-code casting gate rejects assertions, and `in` narrows the same untrusted JSON without one. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile-web): move the bundle source under src/ so the root guard passes .github/scripts/check-root-directory-entries.mjs blocks any new top-level entry by name, so mobile-web/ could not live at the root. The source is excluded from app.asar by the existing '!src{,/**/*}' rule; the explicit '!src/mobile-web{,/**/*}' entry stays as a marker. out/mobile-web is unaffected and still ships under the out rules like out/web. No tsconfig includes src/**, so node, web, cli, and relay do not pick the tree up; it is registered as a knip entry so audit:dead-code does not call it unused. buildId is unchanged at 9d78435e: the builder hashes content, not paths. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(build): resolve the entry-script guard through pathToFileURL `file://${process.argv[1]}` never equals import.meta.url on Windows, where that url is file:///C:/... So the builder exited 0 having written nothing and the Windows packaging job failed later, at the guard, with no clue why. Every other script in config/scripts already uses pathToFileURL; this one now does too, via an exported predicate a posix runner can exercise with a win32 path. The verify script had no entry guard at all, so importing its budget constants ran the whole verification — including its process.exit — inside the test worker. It is now a function behind the same guard. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(ci): build the mobile web bundle in the PR package job That job assembles packaging inputs step by step instead of calling build:release, so the new beforePack guard hard-failed it. The census test added here is the oracle: it walks every workflow job that invokes electron-builder without --prepackaged (which short-circuits doPack before beforePack) and requires a bundle-producing script in the same job. It goes red on exactly pr.yml's package job when this step is removed. Ten jobs covered; the other nine already ran build:release, build:release:parallel, or build:desktop. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile-web): pin source line endings, because CRLF changes the buildId Every text byte under src/mobile-web is hashed into an asset digest and from there into buildId, so a CRLF checkout produces a different bundle id for the same commit: 91af2897 instead of 9d78435e. That would make a Windows-built desktop disagree with a mac-built one about which bundle a phone has cached. .gitattributes pins eol=lf for the text sources and -text for the PNG, matching the four trees already pinned for byte-hashing. The verify script asserts no source file carries a CR, so the build fails if the pin ever stops applying rather than silently shipping a second bundle identity. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * style(build): read the test's own path from import.meta.filename oxlint unicorn/prefer-import-meta-properties. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(test): census packaging jobs over raw workflow text, not re-serialized YAML yaml.stringify folds long lines, and in dev-channel-win-build.yml's build-win the fold landed between `electron-builder` and `--config`, so a real packaging job was invisible to the census: 11 jobs exist, the test saw 10. Slice each job's raw source by its parsed boundaries instead, and pin the inventory so a new packaging workflow has to be added here on purpose. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): assert the script chain the packaging census trusts The census only checks that a packaging job invokes one of ten build scripts; that those scripts still reach build:mobile-web was asserted nowhere, so a dropped link would leave every job looking covered while packaging failed at beforePack. Resolve each script for real, and pin pr.yml's hand-rolled step, since that job never calls build:release. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(build): realpath the entry path before the direct-invocation compare Node resolves symlinks in import.meta.url but not in argv[1], so `node /tmp/...` against a /private/tmp realpath compared two different strings: the builder and the verifier exited 0 having written and checked nothing. Same silent-success shape as the Windows file:// bug, so the fix sits next to it, with both seams injectable. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * style(mobile-web): format bootstrap.css with oxfmt It was the only tracked CSS failing oxfmt --check. The buildId is unchanged at 9d78435e8bb73c3341f833c20aaefbd7bfdfc414b68dadf87c1689d86728fe33, because esbuild's CSS minifier normalises the whitespace this touches before the asset is hashed. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(packaging): reject bundle files the manifest does not list The guard only walked the manifest, so a dropped assets/stale.js passed: assets are content-addressed, nothing ever overwrites a stale copy, and it would ship inside asar unreachable and unverified. Require every file under out/mobile-web to be the manifest or a listed asset. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(packaging): give beforePack an explicit mobile web bundle root The bundle guard read the repo's out/mobile-web unconditionally, so the two arch-aware packaging tests that call the real beforePack went red in the unit-test job, which never runs build:mobile-web. beforePack now takes the bundle root as a second parameter defaulting to out/mobile-web, which is what electron-builder gets, and those tests build a real bundle into a temp dir instead. The guard is neither skipped nor made tolerant of a missing bundle. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(packaging): census sees script-wrapped packers; dev verify reuses the guard The workflow census only matched a literal `electron-builder --config` line, so daemon-relocation-spike's `pnpm run build:unpack` (which packs and runs beforePack) was invisible to it. Jobs now count when any `pnpm run <script>` they invoke chains to electron-builder without --prepackaged; the spike joins the pinned list (12 jobs). verify-mobile-web-bundle.mjs re-implemented a weaker subset of the packaging guard (no safe-path check, no buildId recompute). It now calls assertMobileWebBundleBuilt, so a manifest edited after the build fails at `pnpm build:mobile-web` exactly as at beforePack. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-17 22:41:13 -04:00
Jinwoo Hong	a634bf9b49	test(bench): runtime-graph publication probe and optional CDP CPU throttle (#21107) ... * test(bench): count runtime-graph publications from main The build-provided `__orcaBenchmarkInstrumentation` is gone from the tree, so the typing bench could no longer report graph-publication counts at all. The renderer cannot supply them either: `window.api` is frozen by contextBridge, so `runtime.syncWindowGraph` is not wrappable. Count them where they land instead — main's `runtime:syncWindowGraph` invoke handler — behind ORCA_TYPING_BENCH_GRAPH_PROBE=1, and record the result in the bench report. Measured on an 870-worktree fixture: 21 publications over a 50 s metadata-only window versus ~1,205 with recurring OSC title/status traffic. The long-task fields ship unproven: an injected 250 ms renderer busy-wait produced zero entries even though `longtask` is in `supportedEntryTypes`, so their zeros mean "oracle unverified", not "no long task". The self-test knob exists to make that falsifiable, and the file says so; per-publication build time still needs a separate --cpu-profile run. * test(bench): optional CDP CPU throttle around the typing window * test(bench): report the throttle that ran and the long task the self-test caused Two ways the bench could misreport its own conditions. `cpuThrottleRate` was the requested rate, written into every report, but only two of the three scenarios wrapped their typing window in the throttle — a `--cpu-throttle 4` visible-split run claimed a 4x throttle it never applied. Recording the rate per scenario would have made the report honest; it would also have left one scenario silently ignoring the flag, and a fourth scenario would inherit the same omission. So both: every scenario now goes through one `measureTypingWindow` helper, and the value it returns is the rate the throttle actually applied. `writeBenchReport` takes that composite instead of a bare measurement, so a scenario cannot produce a report without saying what it ran under. Unthrottled runs are unchanged — rate 1 still opens no CDP session. `selfTestLongTaskMs` took the *earliest* long task starting before a cutoff captured after the busy-wait. The observer has been live since probe start, so any unrelated long task from fixture setup satisfied it — the field whose whole job is to prove the oracle is live was the easiest one to fake. The busy-wait now reports its own renderer-clock bounds and the matching entry is the one containing their midpoint: main-thread tasks never overlap, so at most one can, and it is the task the busy-wait ran in. That entry is then withheld from `longTasks`, `longestLongTasks`, and `longTasksAroundPublication`, which had been counting the oracle's injected 250 ms as workload. A zero still means "oracle unproven" — it now also means it honestly. * test(bench): stop the graph probe when the typing run throws * test(e2e): drain queued long-task records before the probe disconnects	2026-09-17 17:32:20 -04:00
Brennan Benson	abc8386e14	fix(mobile): name a create's launch so a lost reply cannot build two workspaces (#21137) ... * fix(mobile): name a create's launch so a lost reply cannot build two workspaces `agent.launch` admits a caller-supplied `operationId` through a durable ledger, so exactly one execution happens and every replay returns the recorded answer. No client sent one, so the machinery was inert and the original defect was still live: mobile retries a lost create by design, and a retried launch built a second agent in a second workspace. Mobile now mints an operation id per create candidate and sends it whenever the host advertises `agent.launch.replay.v1`. The invariant is one operation per candidate. `computeAgentLaunchFingerprint` folds `target` whole, so the workspace name is inside the fingerprint; carrying one id across a name-collision bump would meet its own row under a differing fingerprint and refuse `agent_session_operation_conflict`, failing the create outright on the second candidate. The id is therefore minted beside `clientMutationId` at the top of each loop iteration and reused verbatim by every retry arm inside that candidate — never re-minted, since a new id is a new operation. Admission runs ahead of every effect, so `_invalid` / `_expired` / `_capacity` prove nothing launched: those re-send the same candidate unnamed rather than let bookkeeping fail a create the host would have performed. `_unknown` is the one refusal that is not safe to re-send, and it surfaces. Also corrects a false comment: the legacy path caches the whole launch under `clientMutationId`, so inside its 60s window a replay adds neither a workspace nor a surface, and outside it adds both — not "a second surface, never a second workspace". * fix(mobile): preserve launch identity on refusals * fix(mobile): use launch receipts to authorize replay * test: move mobile launch replay coverage outside node project * fix(mobile): enforce replay-safe launch delivery at the host * test: run mobile launch contracts in mobile checks * test: cover mobile launch contract workflow dependencies	2026-09-17 10:06:11 -07:00
Neil	ea01cd0ccd	fix(windows): reject a node-pty addon that predates the MSYS breakaway denial (#20047) ... * docs(windows): record the measured MSYS job-breakaway mechanism The per-PTY job already denies JOB_OBJECT_LIMIT_BREAKAWAY_OK for Cygwin/MSYS shells (#19068), but nothing records why, and a conpty.node built before that commit fails windows-msys-job.win32.test.ts in a way that reads as a source defect. Measured on a real Windows 11 host: both the plain and the exec- replacement Git Bash shapes leak, the escape is the MSYS runtime's own spawn/exec (fork keeps membership), and a single-variable A/B on usesCygwinRuntime flips the result 0/2 -> 4/4. Also names the gap the failure hid behind: node-pty-job-ownership.cjs asserts symbol presence, which cannot distinguish patch revisions. * fix(windows): reject a node-pty addon that predates the MSYS breakaway denial The native-runtime gate asserted only that terminateJob, listJobProcessIds and assignCurrentProcessToJob were exported. All three predate the Cygwin/MSYS breakaway denial, so an addon built before it passes every gate, isPtyJobOwnershipAvailable() returns true, and windows-pty-job.win32.test.ts passes 6/6 -- while every Git Bash child is created outside its pane's job and survives terminatePtyJob. Read the resolved .node and require the wide msys-2.0.dll literal that usesCygwinRuntime holds, the way stagedRelayAddonIsUnpatched() already tells a patched windows-process-tree addon from a published one. An addon the caller cannot name is refused rather than skipped: a gate that cannot see its subject is not a gate. Verified against real binaries on a Windows 11 host: the shared checkout's pre-#19068 build errors, a build from current patched source passes, a missing path errors. Also closes the cross-host packaging skip. The export half has to load the addon so it cannot run when the packaging host is not the target, which is how a Windows release built elsewhere could ship this. The marker is a file read and needs neither; an unrecognised layout warns rather than fails a release that was packaging fine. * fix(windows): check the MSYS breakaway denial on the rebuild path too The Electron probe carried the marker check, but it lives inside probeElectronNativeModules, which returns early whenever the Electron package binary is unusable. Covered by another path is not this path checks -- and the defect this whole change closes was a gate that looked like it checked. Reading the binary needs neither a loadable Electron nor an executable target arch, so assert it after the rebuild, beside the windows-process-tree assertion that exists for the same reason: this is the addon copied into the packaged app. Absent warns (a cross-platform rebuild need not leave a win32 addon on this disk); present and unmarked is fatal. The fixtures now write a real addon file, because the gate reads the binary it was told about rather than trusting the exports. Verified against the two real binaries measured on the Windows host: the pre-#19068 build fails this path, the build from current patched source passes. * fix(windows): check the marker on every ConPTY path the packaged app can load The packaged marker check read one hard-coded path, `build/Release/conpty.node`, and warned when it was absent. `loadNativeModule` tries `build/Release`, then `build/Debug`, then `prebuilds/win32-<arch>`, swallowing each failure, and `prunePackagedNodePty` drops the published prebuild only when a same-arch `build/Release` exists to replace it. So the two packages the check was added for were the two it could not see: - cross-host: no host but Windows can build conpty.node, so there is no `build/Release` and the prebuild is what ships. The check warned and returned. - cross-arch: `build/Release` is the packaging host's own arch, patched and marked, so the check printed OK -- while the target app cannot load it and falls through to the unmarked prebuild underneath. Measured, not assumed: both published Windows prebuilds in the node-pty tarball contain neither `msys-2.0.dll` nor `cygwin1.dll` in any encoding. They are the binary that leaks every MSYS pane child out of its job. It now sweeps every candidate present for the *target* arch and refuses a package with no candidate at all, which is a package with no ConPTY backend rather than a layout to shrug at. It runs for every Windows slice instead of only the branch the export check skips, so deleting the export check cannot silently take it too. A stale source build keeps the rebuild advice; the prebuild gets the advice that actually works, which is to package the slice on a Windows host of that arch. Also: the marker constant was re-typed in four places and was tied to the C++ literal that produces it by nothing at all, so editing the patch would have left a gate that fails every correctly rebuilt addon and tells the developer to do the one thing that cannot help. The fixtures now take the constant from the gate, and a test asserts the patch still adds `L"msys-2.0.dll"` to conpty.cc. And the rebuild path treated a missing addon as a warning even on the host that will run the install, where node-pty would fall through to that same prebuild. The verdict is now a value, so it is tested without a platform gate. * fix(windows): resolve the packaged ConPTY the way its loader does Sweeping every candidate and demanding the marker on all of them was wrong in the one case it was meant to make safe. `beforeBuild` runs `rebuild-native-deps.mjs --platform=win32 --arch=<target>`, so a cross-arch slice normally does get a patched `build/Release` for the target; `prunePackagedNodePty` keeps the prebuild anyway because its guard is `electronArch === process.arch` rather than the arch of the binary. That package is correct and its leftover prebuild is never reached, and the sweep failed it -- telling whoever ran it to package on a Windows arm64 host, which is both the wrong remedy and one no runner here can offer. Presence cannot separate that package from the one whose cross-arch rebuild quietly emitted the host's architecture, because the only difference is the arch of `build/Release`. So the gate now resolves the addon the way `loadNativeModule` does -- first candidate whose PE `IMAGE_FILE_HEADER.Machine` matches the target, walking root-then-lib for each layout in node-pty's own order -- and checks the marker on the one that will actually run. A package with no candidate, or none of the target's architecture, is refused: it has no ConPTY backend either way, and the second is exactly what a silently host-arch cross-build looks like. The PE machine reader already existed, privately, in the relay addon builder that needed the same "a cross-build cannot silently emit host arch" guarantee. It is now shared rather than copied. Two seams were unreachable from anything but Windows, so nothing tested them: - the afterPack hook's win32 block was an inline if/else that only a source-text assertion could inspect, and that assertion could not tell the difference between the check running and the check being wrapped in `try {} catch {}`. It is now `verifyPackagedWindowsNodePty`, and "the marker check runs even where the export check cannot" is four spied assertions instead of a string match. - the rebuild path's verdict read `process` directly, so the branch that fires only on the host being rebuilt for was dead on every other host. It now takes the host as arguments, and the fs checks, the warning and the failure are all exercised from macOS. Fixtures write a real PE header rather than `MZ fake addon`, since the gate now reads one. The machine table is pinned to the documented IMAGE_FILE_MACHINE values, because every fixture builds its header from that table and a table wrong in both entries would otherwise agree with itself. * fix(windows): say why the packaged ConPTY fell back, not just that it did The previous commit resolved the addon by architecture but still had one message for every way the resolution could land on the published prebuild. Those ways want opposite remedies, and the one it printed was the remedy the commit before it had just called wrong: - no source build in the package at all — the slice has to be built somewhere that can build node-pty for the target arch. - a source build that is there but is the packaging host's architecture, because the cross-arch rebuild did not honour `--arch` — re-running that rebuild is the fix, and "package on a Windows arm64 host" is neither necessary nor possible. The second is the common one, since node-pty publishes a prebuild for both Windows arches and prune keeps the target's on every cross-arch package. So the old text fired mostly on the case it described least. It now reports which source builds were skipped and the machine field each carried, and names the rebuild command. "Nothing the target can load" had the same problem in reverse: a zero-length or truncated `conpty.node` got a cross-architecture diagnosis. Every candidate is now named with what was actually read, including "not a PE image". The rebuild path asserts the architecture too. A rebuild that ignored `--arch` was otherwise only visible at packaging, two steps from the command that fixes it. Arches with no known machine value are left unjudged rather than guessed at. Two things the extraction broke or nearly broke, both found by mutation: - the shared PE reader answers `null` where the relay builder's private copy returned a number, which would have turned its "node-gyp ignored --arch" error into a `TypeError`. Both callers now go through `describePeMachine`. - the rebuild fixtures stage a script's co-located modules by walking its imports, and the walker only understood `from '...'` — so the gate's new `require('./windows-pe-machine.cjs')` was left behind and every subprocess test failed with a resolution error, which is the exact failure its own comment warns about. It now follows `require` and bare side-effect `import` as well, and has tests; the fixture stages the gate by walking it rather than by naming one file. Fixtures write real PE headers through one shared builder instead of three hand-rolled ones. * fix(windows): run the node-pty addon gates on the Windows job that can `rebuild-native-deps-node-pty.test.mjs` carries four `skipIf(platform !== 'win32')` tests. The full suite runs on ubuntu, and the Windows PR job runs an explicit file list that never named this file -- so those tests were skipped on Linux and never reached anywhere else. Three of them predate this branch. The Windows job is added the four node-pty addon suites plus the module-walker one; the comment above that list already says why it is the right place, which is that the addon assertions only hold once natives have been rebuilt. Running the path-joining suites there also covers the separator this gate's candidate list is built from. The rest is round-three review: - the rebuild-time arch assertion told a reader "node-gyp did not honour --arch" about a file that was not a PE image at all, which is a truncated or quarantined artifact and a different command to run. The two now read differently, and neither claims the other's cause. Same fix the packaged gate had one commit ago, in the place that had not had it yet. - the missing-addon error said node-pty "would load" a prebuild without checking it is there. It says "fall through to" now, which is true either way. - `isLoadableByArch` had no caller left once the packaged gate started needing the raw machine field for its message. Removed rather than kept warm. - each candidate's header is read once instead of up to three times. - the module walker's comment claimed every shape that reaches a co-located module; it does not follow `projectRequire`/`requireLocal`, and it must not -- those specifiers resolve against the project root, so following one stages the wrong path and the copy fails. Proven by trying: widening the pattern to require-shaped names broke nine tests on `projectRequire('./config/scripts/...')`. The comment now says what it follows and why it stops there. - a new test resolved a file URL with `.pathname`, which keeps the drive-letter slash on Windows -- the very job this commit adds it to. * docs(windows): put the superseded export-only gate in the past tense It describes what used to pass a broken addon, so present tense reads as a description of the gate the same document then explains replacing it. * fix(windows): repair what running the node-pty suites on Windows exposed Putting these files on the Windows job turned four assertions red on the first run. Three of them were in tests that carried `skipIf(platform !== 'win32')` and had therefore never executed anywhere, on any branch. - `writeFakeElectronRebuild` emitted the `windows-process-tree` addon a real rebuild leaves but never node-pty's, so every Windows test of the rebuild path ran against a tree no real rebuild can produce: node-pty "rebuilt" with nothing in `build/Release`. The new same-host check reads that state correctly and said so. The fake rebuild now writes `build/Release/conpty.node` when it was asked to rebuild node-pty for win32, with the marker and the target machine. - `mkTempProject` never staged `windows-process-tree-creation-time.cjs`. The rebuild script reaches it through `projectRequire`, which resolves against the project root, so the module walker cannot follow it and must not try. Staged by name, with a comment saying which of the two it is. Without it the windows-process-tree probe failed to load its own checker and the module joined `modulesToRebuild`, which is the second and third red assertion. - the two `nodePtyAddonPath` cases compared against a literal POSIX string. `resolve` returns a drive letter and backslashes on Windows, so they could only ever pass off it. Built from segments now, which still pins the `..` traversal that is the point of the test. Verified on macOS: ensure-native-runtime-job-ownership, verify-packaged-node-pty-job-ownership, windows-pe-machine, script-module-dependencies, rebuild-native-deps-node-pty, rebuild-native-deps, rebuild-native-deps-windows-process-tree, ensure-native-runtime -- 109 passed, 6 skipped. The 6 are the Windows-gated rebuild tests, which is the job this change is aimed at; Windows CI is the arbiter. * fix(windows): give the packaged fallback a third verdict, for a file that is no image The packaged gate had two remedies for landing on the published prebuild and picked between them on `!prebuilt`, which puts a truncated, empty or quarantined `build/Release/conpty.node` in the cross-arch bucket: "the source build beside it is the wrong architecture ... re-run with --arch". It is not the wrong architecture, it is not an architecture, and `--arch` is not the command. The rebuild-path gate was split for exactly this a commit ago; this is the same split in the place that had not had it. Also from review of the settled state: - the stale-source-build branch ended in a call that happened to throw, so a reader could not see it was terminal and the file was read twice to get there. The verdict is now an Error the caller throws, built once from the read it already did, and shared with `assertCygwinBreakawayDenied` rather than copied. - four injection seams had no consumer in production or in tests (`deniesBreakaway`, `peMachine`, and `exists`/`peMachine` on the rebuild verdict). An unused seam is a way for the tested path and the real one to drift apart; the tests drive both with real files. Removed. - the loader table existed in a docblock and in the reference doc, already disagreeing about row four. The docblock cites the doc now. - `peImage` stamped machine `0x0000` for an arch it had no value for, because `writeUInt16LE(undefined)` coerces to zero. A fixture that quietly invents the field the gates read is the same species of silent lie the gates exist to catch; it throws, and a test holds it to that. - a test named for refusing an unreadable candidate asserted only that something threw. Renamed to what it proves. * fix(windows): make the rebuild fixtures represent a tree that can exist Second round of what running these suites on Windows exposed. The module the walker could not stage is now staged, so the probe reached its own checker and the real reasons surfaced: - `writeFakeWindowsProcessTree` exported `{}`. The creation-time gate reads `supportedProcessDataFlags` off the addon and calls its absence "the tarball prebuilt, not a build of the patched source" — correctly. The fixture predates that gate and, being Windows-only, never met it. The healthy fake now reports the flag, taken from the gate's own constant. Two tests were failing on this, the second only because the module then joined `modulesToRebuild`. - `rebuilds a loadable ConPTY native that lacks Orca job ownership` asked for a node-pty rebuild in a tree where node-pty had none of the payload its package ships. It gets `writeFakeNodePtyConptyPayload` like its two siblings. I also tried making the fake rebuild emit `build/Release/conpty.node` the way a real one does, and backed it out: `restoreNodePtyWindowsConptyRuntime` keys off that file and then reads `third_party/conpty`, so emitting it in a tree without the package payload turns one honest gap into an ENOENT two steps away. The payload fixture is where "node-pty has its addon" belongs. macOS: ensure-native-runtime-job-ownership, verify-packaged-node-pty-job-ownership, windows-pe-machine, script-module-dependencies, rebuild-native-deps-node-pty, rebuild-native-deps, rebuild-native-deps-windows-process-tree, ensure-native-runtime — 112 passed, 6 skipped. The 6 are the Windows-gated rebuild tests; Windows CI is the arbiter and is why they are on that job now. * fix(windows): register the node-pty addon suites in the scope list too Putting the five suites in the Windows lane's vitest argv gets them run once the job starts; `WINDOWS_PACKAGE_TESTS` in `pr-code-change-scope.mjs` is what decides whether the job starts at all. Only the argv was updated, so a PR touching just `rebuild-native-deps-node-pty.test.mjs` would not have started the Windows job, and its four Windows-only cases — including the same-host-absent one added here — would have run on no machine for that PR. Exactly the shape of gap this branch is about. Both lists now name all five, and `windows-pe-machine`, `windows-pe-image-fixture` and `script-module-dependencies` join `NATIVE_RUNTIME_PREFIXES` so a change to the modules themselves starts it too. `win32-test-lane-registration.test.mjs` exists to catch precisely this and did not, because its matcher only recognises suite-level gates (`describe.runIf` / `describe.skipIf`) and a `.win32.` filename. These tests gate per `it`. Widening it is not this branch's change to make: about thirty files across the repo carry per-`it` Windows gates and are unregistered, so the ratchet would move far beyond node-pty. Flagged rather than done. Message repairs from the same review: - the non-PE arm of the rebuild-time arch error read "... is not a PE image, so nothing can load it, so node-pty would fall back ...". The shared consequence clause already opens with ", so". - the no-source-build packaging error ended "Package this Windows slice on such a host", which is wrong advice for the case where the host IS such a host and the rebuild simply left nothing — reachable when the artifact is removed before prune runs. It now names both readings and points at the beforeBuild output. - the relay-addon builder blamed `--arch` for a build output that is not a PE at all, the same guess the node-pty gate was taught to stop making. - the patch-drift assertion was a bare `toBe(true)`, so a real drift read as "expected false to be true". It now names the two things that can have drifted and what happens until they agree.	2026-09-16 22:23:30 -07:00
Brennan Benson	fbe7b194b8	fix(quality-gate): let the changed-code gate see the focused import plugins (#20912) ... import/no-duplicates was reachable only through the repo-wide CI audit, so an author's first signal was a red static analysis job after push.	2026-09-16 20:54:27 -07:00
Brennan BensonandMerge Sim	97aa5ff19b	fix(mobile): open native chat when a new worktree launches a default agent (#19850) ... * refactor(agent-launch): make the launch-mode decision surface-neutral `decideWorkerStartMode` was the only shared answer to "structured chat session or terminal agent?", but it lived in an orchestration-named module and spoke orchestration's vocabulary, so the other launch surfaces could not call it. Move the decision to `main/agent-launch/agent-launch-mode` unchanged and leave `orchestration-worker-start-mode` as the adapter that supplies the noun. A worker is not a special kind of launch; it is the same launch with a dispatch attached. Naming the receipt's subject is the only thing orchestration actually contributed, so that is the only thing the adapter keeps: "worker" in both sentences, plus the `--terminal` wording, which reads as nonsense anywhere a `--terminal` flag does not exist. Both are pinned, because they are asserted. No behavior change. The receipts are byte-identical for every reachable case, proven by running the new pin against both implementations. Also pins the wording, which nothing was holding. The existing suites assert `toContain` fragments ('terminal agent', 'cannot create') and the CLI suite asserts a receipt handed to it by a mock rather than one this code produced; all six files stayed green against a deliberately corrupted vocabulary. A dispatch receipt is the only place a structured-to-terminal downgrade explains itself, so the whole sentence is the contract, not a fragment of it. * feat(agent-launch): add the launch intent and the one executor that runs it The sequencing around the launch decision was duplicated per surface, and the duplicate is where the bug lives. A new worktree was created agent-first, so its startup terminal WAS the agent and the structured branch below it could never be reached — every new-worktree launch was a PTY regardless of the user's default. Orchestration fixed that for itself in #19431; mobile and the CLI still have it. `executeAgentLaunch` inverts the order once, for everyone. When the preference is structured the worktree is created with NO startup agent, the executing host is then asked whether it can host a session for the workspace that now exists, and only then is a surface created. The host verdict cannot be hoisted above creation: `agentSession.createSupport` only answers for a workspace it can resolve, which is why the decision stays in two halves. Agent-first creation is deliberately preserved for PTY launches — it is what sequences the agent's startup command behind the setup runner, so wait-for-setup comes for free there. What actually differs per surface is only how a surface is built (an orchestration worker's session takes a dispatch hold and a mailbox a plain launch must not take), so that is injected as a factory rather than branched on. The intent also strips the reserved agent fields from a migrated create payload: a caller moving off `worktree.create` passes its existing params, and a stale `startupAgent` in there would re-create the very path this replaces. Tests assert order and arguments, not just the resulting mode. Reintroducing agent-first creation reddens 4 of 11. * feat(agent-launch): expose the launch executor as the agent.launch RPC Adds `agent.launch` — one host-side method that decides structured-vs-terminal and creates the surface — wired to the real runtime factories: `createManagedWorktree` for the workspace, forking on `startupAgent` exactly as the orchestration worker path does; `createStructuredAgentSessionForWorktree` for a chat session; and `createTerminal` for a PTY agent. Allowlisted for mobile, which is the surface the routing gap was reported on. `worktree.create` is untouched. Its `startupAgent` keeps meaning "spawn a PTY agent" verbatim, because it answers with `agentTerminalHandle` only on that path: a host that quietly routed it to a structured session would hand every older client a response with no handle and no error. All new behaviour sits behind `agent.launch.v1`, which the host now advertises and a remote client must negotiate, so a client that does not gets today's behaviour unchanged. * feat(mobile): route workspace creates through agent.launch Picking an agent on the mobile create sheet always produced a terminal, even when the user's default was native chat, because all three create paths put `startupAgent` on `worktree.create`. That means "create the worktree agent-first", so its startup terminal IS the agent and the structured branch below it is unreachable — while the same phone's in-workspace "+" button opened a chat. The blank, branch and new-branch creates now send the same payload through `agent.launch` and let the host settle the surface. `worktree.create` is untouched, and a host that does not advertise `agent.launch.v1` (read from the existing `status.get` probe) keeps today's path exactly. Work-item creates stay on `worktree.create`: they pre-fill the issue/PR URL as an unsent `startupDraft`, which a structured session cannot hold yet, so routing them would submit the URL as a first turn. * fix(agent-launch): drop the deleted draft-prompt blocker from the reason map main removed the draft-prompt blocker in #19681 (a structured session now holds an unsent draft), so the exhaustive Record no longer typechecks. * chore(agent-launch): carry a SAFETY rationale on the agent placement cast The type-assertion gate landed after this branch's base, so the new file's copy of the worker-start cast is now a changed-code finding. * chore(agent-launch): carry agent.launch through main's RPC typing and casting gates The typed-method contract, the generated params catalog and the `assertionStyle: never` casting scan all landed after this branch's base. - AGENT_LAUNCH_METHODS kept an `RpcMethod[]` annotation, which widened its method name to `string` and broke assignability; every sibling infers instead. - `agent.launch` binds a schema under src/main, so it joins the catalog's RPC_METHODS_WITHOUT_SHARED_PARAMS and the parity gate's hand-listed twin. - The now-typed methods make most test casts unnecessary; the few that remain carry the line-specific SAFETY rationale the casting gate requires. * test(mobile): supply the agent-launch fixture the create-submit recording needs The golden RPC recordings landed upstream while this branch was out, so they first met agent.launch here. Three things had to happen, and only one of them is a fixture bump. 1. workspace-settings-mounts.ts mounts useNewWorkspaceCreateSubmit against a fixture model that throws on any member it was not given. This PR added a required getAgentLaunchSupport, so the submit aborted with "Missing model fixture" before it ever issued the create, and three cleanup checkpoints vanished. That read like a product regression and was not one. Supplying the member restores the recording byte-for-byte; it is pinned false for the same reason the cutover probe is, so the baseline stays on worktree.create. 2. Editing that adapter moves adapterSha256 for the twelve settings goldens it mounts. Their recordings are unchanged - header only, by design: the digest is per-golden so editing a module fails exactly the goldens that mounted it. 3. Five goldens changed behaviourally, and both changes are this PR's: the capability probe now reports agentLaunch, and a create whose reply carries no worktree returns "Failed to create workspace" instead of throwing a TypeError off an unguarded result.worktree read. The launch route needs that guard, since a receipt can arrive without a worktreeId. * refactor(mobile): decode the launch receipt instead of asserting its shape The changed-code quality gate refuses type assertions, and the eight it flagged were worth removing rather than suppressing. The production one was the point. readAgentLaunchCreateOutcome asserted the RPC payload into Partial<AgentLaunchResult> and then runtime-checked it anyway, so the assertion bought nothing and claimed a contract the host had not proven. It now narrows with `in` and validates each hop, which is the same nullability question readCreateResult already answers on the sibling path - a launch receipt can legitimately arrive without a worktreeId. AgentLaunchCreateOutcome ties worktreeId to the shared contract so a change there fails this reader's typecheck rather than passing a differently-typed field through. The test fakes claimed a whole RpcClient via `as unknown as RpcClient` while implementing one member. They now build a typed literal, matching the pattern in use-mobile-structured-agent-options.test.ts. The read sites cast params and then read one field; they now assert the payload with toMatchObject, which removes the cast and pins more of the shape than the cast did. Also pins the warning passthrough, which nothing covered: a terminal launch that seats the workspace but cannot start the pty reports why, and the absent, blank, non-string and structured-surface cases report nothing. Writing that test caught a real drop I had introduced in the reader. * ci(mobile): re-run Mobile Checks when a shared capability changes Mobile Checks is path-filtered to mobile/**, but mobile imports the negotiated capability names straight from src/shared/protocol-version.ts and records the whole capability read verbatim in its goldens. So a capability added desktop-side rewrites a mobile fixture while never triggering the suite that would catch it. That is what happened here: #19849 introduced agent.launch.v1 and Mobile Checks never ran on it. Verified at the run level rather than by check name - the window-free check-runs API on 3837ae8d51 returns 49 check-runs across six runs (PR Checks x2, PR test LoC x2, Track Community PRs, Review) and no Mobile Checks among them. The breakage surfaced only in this PR, which happens to touch mobile/**. The workflow already concedes this pattern for terminal-file-link-conformance.ts; protocol-version.ts has the stronger claim, since mobile records its output. Also corrects the mount adapter's SAFETY comment. It claimed the recorder supplies only the members the hook reads, which was false the moment the hook gained a required getAgentLaunchSupport - and the assertion it annotates is exactly what stopped the compiler from saying so. The twelve goldens are adapterSha256 churn from that comment: every body is byte-identical, which is the digest doing its job. * docs(agent-launch): stop the receipt-wording comment claiming a migration The decision was never moved out of orchestration-worker-start-mode; this PR adds a second copy beside it. Say so, and name the unenforced agreement. * docs(agent-launch): stop the executor comment claiming a migration that has not happened The header asserted two things the tree does not support: that every launch surface routes through the executor, and that the mode decision "already lived" in `agent-launch-mode`. `agent.launch` is the executor's only consumer, and `orchestration-worker-start-mode.ts` is byte-identical (blob 92dc5c644a, 217 lines) at the merge base and all three stack heads, still used by workers.ts. Describe the two live copies and leave the cutover to later stack work. * fix(agent-launch): preserve setup and refusal fallbacks * refactor(mobile): parse the launch outcome into a named type at its boundary anti-slop/no-object-parameters flagged terminalLaunchWarning's `result: object`. The rule is pointing at a real seam rather than a style nit: the helper advertised a loose object and did the narrowing inside itself, so every caller handed it unparsed wire data and nothing downstream held a real type. Parsed at the boundary instead. parseTerminalLaunchOutcome takes `unknown` and returns TerminalLaunchOutcome | null, so the narrowing happens once, where the untrusted payload enters, and the consumer works with a named type. The type is taken from the shared contract rather than restated - a Pick over the terminal member of AgentLaunchOutcome - so a change to that union fails here instead of flowing through. `handle` is deliberately excluded: nothing reads it, and requiring it would drop the warning off a reply that omitted one, which is a behaviour change smuggled in under a typing change. No assertion and no config exemption: reintroducing `as Partial<AgentLaunchResult>` would trade this finding for the defect removed earlier in this branch, and the rule is correct here. The rule arrived with the merge-forward (#20781, newer than this branch's merge-base), and anti-slop is not one of the changed-code gate's six scans - it runs only repo-wide - which is why a clean local gate did not predict it. Behaviour is unchanged across all five warning cases, and the positive case was re-ablated on the new parser: dropping the warning reddens exactly it, 1 failed | 18 passed, restored byte-identical to 19 passed. * fix(agent-launch): dedupe complete launch and cancel setup wait * fix(agent-launch): memoize the whole launch so a replay cannot mint a second session A replayed agent.launch could create a second structured session in the same worktree, with activate: true. dedupeWorktreeCreate wrapped only the worktree half, inside the workspace factory. On a replay the create was reused, and the executor then continued to createSurface and built another surface inside it. The terminal route hid this: its cached create carries a startup terminal handle, so the executor returns on early. A structured create has no handle by construction - that is the whole point of the structured fork - so it fell through every time. Mobile replays this method deliberately on a delivery-ambiguous response, up to five attempts, so the path is reachable by design rather than in theory. The handler now wraps the entire launch in the same dedupe, on the same (repo, clientMutationId) identity, exactly as worktree.create wraps its own body. A replay returns the original AgentLaunchResult instead of re-running createSurface, which makes the two routes replay-identical. The inner dedupe is removed rather than kept. Wrapping both levels on one key deadlocks: dedupeWorktreeCreate stores the in-flight promise before the inner call runs, so the inner call would be handed the outer's promise, which is waiting on it. The launch-level memo subsumes the worktree-level one. Failures are still dropped rather than cached, so an unknown outcome stays unknown instead of replaying as a fabricated success. The guard replays a STRUCTURED launch: the terminal route cannot reproduce this and a test there would pass either way. Ablated against the pre-fix files - 1 failed | 22 passed, "expected vi.fn() to be called 1 times, but got 2 times", which is the duplicate session - then restored to 23 passed. The stub's dedupe had to be made faithful for that to be observable; the shared one passes through so other tests can see raw calls. * Revert "fix(agent-launch): memoize the whole launch so a replay cannot mint a second session" This reverts commit 59bc5e9b04. The same defect was already fixed upstream on this stack's base branch by 539e283c0f, which landed while this was being written. That change is broader (it also cancels the setup wait) and namespaces the dedupe key, so it supersedes this one. Reverting rather than hand-merging keeps a single implementation instead of a hybrid nobody chose. The behavioural guard from this commit is ported back on top of the upstream implementation separately: it asserts exactly one structured session survives a replay, where the upstream tests assert the dedupe wiring. * ci(mobile): close the round-1 signal gaps around agent.launch Three review findings, all narrow. Mobile Checks is path-filtered, and this branch made mobile's types depend on the shared RPC contract: rpc-params-contract.ts is a type-only re-export of the generated params catalog, and mobile/tsconfig.json includes **/*.ts. So a desktop-only edit under src/shared/rpc-contract/ could break mobile's typecheck with no mobile signal at all - the same blind spot the protocol-version.ts entry closed, one directory over. Added src/shared/rpc-contract/** to the paths filter. agent.launch had no cross-version trigger. Added the three prefixes a paired peer actually exchanges: the intent contract, the wire schema, and the RPC method. src/main/agent-launch/ is deliberately NOT listed - the executor shapes behaviour but is not itself wire, and AgentLaunchResult's shape is already covered by agent-launch-intent. Extending the cross-version SUITE to cover a negotiated handshake is separate work, not this. The break branch that answers an accepted-but-empty reply with "Failed to create workspace" had no unit coverage; the golden that used to discriminate it collapsed five partitions into one shared error when the null guard replaced the unchecked read. Covered on BOTH routes - worktree.create with no worktree.id and agent.launch with no worktreeId - since the branch serves both. Ablated by bypassing the guard: 2 failed | 11 passed, the two new cases returning a fabricated worktree instead of the error, restored to 13 passed. * fix(agent-launch): give a launch one place to say the workspace is incomplete createManagedWorktree reports an unspawned startup terminal or an uncopied working tree as a top-level `warning`, and worktree.create hands it straight to mobile. The launch path narrowed that result down to {worktreeId, startupTerminalHandle} and dropped it, so every agent.launch create lost a warning the old method surfaces - on both arms. The channel was also asymmetric by accident rather than design: a terminal outcome could carry `warning`, a structured one had nowhere to put it, so the arm this PR exists to enable was the arm that could not report an incomplete create at all. Now there is exactly one place a launch warning lives: AgentLaunchResult.warning, at the top level. It is about the create as often as the surface, it applies to a structured session and a terminal alike, and a reader should not branch on outcome.kind to discover the workspace it just opened is missing something. The terminal arm's own `warning?` is removed rather than left beside it - two homes for one fact is how they drift. Every producer folds in: the create, the surface, and the refusal downgrade. Consumer census before removing it: one production reader (mobile's readAgentLaunchCreateOutcome) and no others - the renderer and mobile launch call sites never read it. The mobile reader now reads the top-level field, which also lets its outcome parser go away entirely. Guard ablated by restoring the pre-fix narrowing: 2 failed | 24 passed, both carriers reporting `expected undefined`, which is the dropped warning itself; restored to 26 passed. The third case asserts an absence and stays green under the mutation by construction - it pins shape, not the defect. * fix(agent-launch): combine both launch warnings instead of dropping one Round 2 found the comment here was false. A create warning and a surface warning CAN both be set, on two reachable paths: 1. The create warns precisely BECAUSE it produced no startup terminal - didSpawnStartup stays false when that spawn throws, and orca-runtime-create-managed-worktree.ts:283 gates startupTerminal on it - so the executor's early return is skipped and a second surface is built, which can warn too. 2. An untracked-copy warning, then a definitive structured refusal downgrading to a terminal that also warns. `??` kept the first and lost the second with nothing saying so. They are now combined the way the create combines its own failures - appendFailure in runtime-local-worktree-terminal-startup.ts, and the startup-terminal catch in runtime-remote-managed-worktree-create.ts - which append rather than replace. The comment is rewritten to say what is true, and records the gap NOT fixed here: a create warning about a failed startup terminal is stale once the launch recovers by building a working one, so a user can be told the agent did not start while looking at it. Distinguishing those needs createManagedWorktree to stop multiplexing two unrelated failures into one string. Guarded and ablated: restoring `??` reddens exactly the new test, with the surface clause missing from the received string; restored to 27 passed. The structured-create stub had to admit its real ok-or-refusal union for the downgrade path to be modellable at all - it previously declared only the ok arm. Also: mobile.yml gains src/shared/agent-launch-intent.ts. It is the sole holder of the agent.launch RESULT shape - the rpc-contract catalog holds params only - and mobile imports it as a value. CROSS_VERSION_WIRE_PREFIXES already treats it as wire-critical; without this, one gate does and the other cannot see it. And the agent-first warning test no longer pairs "startup terminal failed" with a returned handle, a combination the producer cannot emit. * fix(mobile): read a launch warning an older host nests on the outcome agent.launch moved `warning` from the terminal outcome to the top level of the result. That is the right shape - a reader should not branch on `outcome.kind` to learn the workspace it just opened is incomplete - but on the wire it is a REMOVAL, and mobile only read the new place. A host built before the move still advertises the same `agent.launch.v1` capability, so the capability probe cannot tell the two apart and mobile takes this route against one: protocol-version.ts:360 AGENT_LAUNCH_RUNTIME_CAPABILITY is in RUNTIME_CAPABILITIES, the host list orca-runtime-get-status.ts:64 publishes it via status.get; the filter drops only browser.screencast.v1 and three E2E-gated capabilities, never agent.launch agent-launch-executor.ts such a host writes warning INSIDE outcome The result was a regression rather than a contract cleanup: the worktree.create path this replaces returned the warning at the top level and mobile read it, so a create that seated the workspace but could not start the agent surface - pty exhaustion, untracked files not copied - stopped explaining itself on the phone. Read both shapes for as long as such a host can be paired. Top level wins, and cannot be shadowed: AgentLaunchOutcome has no `warning` on either arm, so a current host cannot nest one. The test that pinned the old behaviour is inverted here. Its comment was the actual defect - it framed a legitimate warning from an older peer as a stale shape to defend against, which is what made dropping it look deliberate. * chore(mobile): raise the unchecked-reader ceiling for the agent.launch receipt main landed `unchecked-rpc-reader-inventory.ts`, a ratchet on RpcOperation readers that re-type their reply instead of validating it. Its ceiling for mobile-workspace-create-operations.ts is 4, counted on a tree without this branch's `agentLaunchRun`, so the merge produced "listed 4, found 5". The inventory's own header prescribes this case: a merge is the one time a line goes up without a migration undoing itself, and the instruction is to raise it and name the PR that brought it. It describes main landing an operation the branch never saw; here it is the mirror - the branch holds one main had not seen - so the line is annotated with #19850 rather than left bare. Not converted to `rpcResultVariant(variant, schema)`, which would lower the line instead. That is a validation change rather than a migration, which is exactly what the file's own comment says these five readers deliberately are not; the agent.launch reply is already guarded at the consumer, where readAgentLaunchCreateOutcome returns null on a malformed payload and the create surfaces "Failed to create workspace". Writing a schema now would also target a reply shape #20999 is actively redefining. Ablated: with the line back at 4 the ratchet fails "listed 4, found 5"; at 5 it passes. --------- Co-authored-by: Merge Sim <sim@local>	2026-09-16 13:15:20 -07:00
Jinwoo Hong	12d744f253	fix(skills): keep computer-use off filesystem and shell tasks (#21069) ... * fix(skills): keep computer-use off filesystem and shell tasks STA-7615: "On my desktop create a folder" was matching computer-use because discovery copy said OS/window-level and neighboring skills advertised desktop UI. Scope the trigger to visible GUI with no CLI path, and exclude files/folders/git/shell. * fix(skills): prefer programmatic paths over computer-use State the last-resort rule in discovery copy instead of enumerating files/folders/git/shell. computer-use prefers shell, filesystem, git, HTTP, CLIs, and Playwright/CDP; neighboring skills route to Computer Use only when a visible window needs GUI control those cannot do. * fix(skills): stop advertising computer-use from orchestration Orchestration coordinates workers; it does not drive a GUI. Drop Computer Use and Playwright/embedded-browser routing from its discovery description so those tools are not pulled in from a coordination skill. * fix(skills): drop Playwright from orca-cli discovery orca-cli should not prescribe Playwright or CDP. Those tools may not be installed, and page automation is not this skill's job. * fix(skills): drop the page-only ban from computer-use discovery Page automation is a preference, not a prohibition. If Playwright or CDP is not available, a visible browser window is valid Computer Use. Keep the hard split for Orca's embedded browser (`orca-cli`) only.	2026-09-16 15:43:11 -04:00
Jinwoo Hong	bdb18003e0	test: add accumulated-workspace terminal typing reproduction (#20934) ... * test: reproduce accumulated-workspace typing latency through real PTYs * test: make the bench harness self-checks falsifiable Review found four assertions that could not fail and one fixture gap: - `missingPtyArrivalCount`/`missingEchoCount` were hardcoded `0` and `validateExpectedSeqs` throws before them, so every assertion on them was vacuous and every report read `0`. The throw is the real guard and is already covered; drop the vestigial fields. - An absent status controller returned an all-zero result, which satisfied its own accepted-equals-generated equality. Assert presence first. - The byte-pacing control had only an upper bound, so a generator emitting no stream bytes passed. Add the lower bound. - `lineageEvery: 1` built zero lineage: no ordinal satisfies `% 1 === 1`. Offset the interval and cover the densest setting. - The documented control command never set ORCA_TYPING_BENCH, so it skipped instead of running.	2026-09-16 13:10:46 -04:00
Brennan Benson	170ebce1f2	fix(ci): run static analysis for every tree the repo-wide audits scan (#20918) ... A mobile-only diff is desktop-irrelevant, so should_run was false and every PR check skipped -- including the audits that do lint mobile/. The violation then landed on main and failed the same gate on every later PR's merge ref. Derive the trigger from the audit commands' own scan roots so the two cannot drift.	2026-09-16 01:13:06 -07:00
Neil	d62328aa4d	fix(codex): remove redundant Windows hook launcher for Unicode profiles (#20952) ... * fix(codex): reuse the Windows hook shell for Unicode profile paths * test(codex): register Unicode hook tests in Windows CI * test(codex): pin trust hash replacement during Windows upgrade * test(codex): retry transient Windows teardown locks	2026-09-16 00:30:52 -07:00
Jinjing	47bb473ec6	Remove agent map from dashboard popout (#20929) ... The agent map view was not functional and its components have been removed entirely. The dashboard popout now only supports the kanban board view, with all map-related code, utilities, types, and translations cleaned up accordingly.	2026-09-15 21:55:06 -07:00
Neil	13ba649c22	fix(terminal): let a runtime-created Windows terminal BE the requested shell (#20825) ... * fix(terminal): let a runtime-created Windows terminal BE the requested shell `orca terminal create --environment <windows-host> --command 'cmd.exe'` never created a cmd terminal. `--command` is text the provider TYPES into whatever shell it spawned, so the PTY stayed the host's default shell with cmd running inside it. Captured on `awin`, whose default is Git Bash: $ orca terminal create --environment awin --command 'cmd.exe' --json $ orca terminal send --environment awin --terminal term_10656cf7... \ --text exit --enter $ orca terminal read --environment awin --terminal term_10656cf7... --screen neil@awin MINGW64 ~/orca/orca ((30f820708f...)) $ cmd.exe Microsoft Windows [Version 10.0.26200.9445] C:\Users\neil\orca\orca>exit neil@awin MINGW64 ~/orca/orca ((30f820708f...)) $ The handle is alive the whole time and `terminal list` shows one healthy terminal, because the PTY never changed — so the only symptom is that the caller's terminal is now a shell it never asked for, and every later `send` is quoted for the wrong one. On `win-lowspec` (default pwsh) the same create lands cmd inside PowerShell. Root cause ---------- There are two spawn preflights and they are twins: - `src/main/ipc/pty/ipc/spawn-preflight.ts` — renderer/IPC spawns, i.e. a terminal tab opened in the app. - `src/main/ipc/pty/runtime/spawn-preflight.ts` — runtime spawns: the CLI's `terminal.create`, headless `orca serve`, and every paired remote environment. Only the IPC twin read the caller's requested shell. The runtime twin passed a literal `requestedShellOverride: undefined`, so a runtime-created terminal on Windows could only ever be the host default. Everything downstream of that point — `spawn-options`, the daemon, `resolvePtyShellOverride` in the relay, `local-pty-launch-plan` — already honoured `shellOverride`; nothing upstream could supply one. Change ------ - Thread `shellOverride` through the runtime lane: `RuntimePtySpawnArgs` -> runtime `spawn-preflight` -> `RuntimePtyController.spawn` -> `TerminalCreateOptions` -> the `terminal.create` RPC's new `shell` param -> `orca terminal create --shell`. - Thread it through the renderer-backed lane too (`createDesktopTerminal` -> `terminal:requestTabCreate` -> `store.createTab`), so `--shell --focus` is not silently dropped on a local Windows app. - An agent launch quotes its startup command for the shell it will actually run in, so a requested shell now owns the startup-shell family instead of the global `terminalWindowsShell` setting. - Lift the relay's `ALLOWED_WINDOWS_SHELL_OVERRIDES` into `isSupportedWindowsShellOverride` in `src/shared/windows-terminal-shell.ts` (membership unchanged) so the CLI, the zod param schema, and the relay refuse the same names. `--shell` therefore cannot carry a path or a command line into `pty.spawn`; only allowlisted bare shell names pass. - Gate on `TERMINAL_CREATE_SHELL_SELECTION_RUNTIME_CAPABILITY`. An older host strips the unknown `shell` param and answers with a healthy terminal running its default shell — a reply indistinguishable from success — so the CLI refuses before creating anything rather than creating the wrong shell quietly. `--shell` stays Windows-only; macOS and Linux hosts spawn the login shell and the relay drops the value off win32 rather than honouring it half-way. A WSL project runtime still outranks it, unchanged. Tests ----- - `pty-spawn-shell-override-parity.test.ts` pins both preflights against the exact drift that caused this (verified failing with the fix reverted). - `createTerminal` passes `shellOverride` to `ptyController.spawn` with no startup command. - CLI: sends `shell`, refuses a shell the host cannot spawn, and refuses a host without the capability — in both refusals without making the round trip. - Allowlist and `terminal.create` schema accept/refuse cases, including paths and appended arguments. * fix(terminal): refuse a requested shell the execution host cannot apply The first commit made `--shell` reach the spawn, but only a LOCAL win32 execution host applies it: `spawn-options` gates the override on `process.platform === 'win32' && !args.connectionId`. So `--shell cmd.exe` against an SSH-routed worktree, or against a macOS/Linux host, still returned a healthy terminal running that host's default shell — the same indistinguishable-from-success reply the capability gate exists to prevent, one layer down. Refuse instead, before anything spawns. The check sits at the top of `resolveAgentTerminalCreateOptions`, which every create lane funnels through, so neither lane has to remember it; the desktop lane additionally refuses a worktree-less create, which has no execution host to resolve a shell on. An SSH host's platform and installed shells are not visible to this runtime, and a POSIX host has no Windows shell to pick. Neither can honour the request, and saying so is the whole point of the flag. Docs and the CLI spec now say "refused", not "ignored". * fix(terminal): refuse a shell that contradicts the project execution runtime `resolveLocalWindowsTerminalRuntimeOptions` does not merely rank the project's execution runtime above a per-terminal pick -- it REWRITES the pick, in both directions, and says nothing: - a WSL project forces `wsl.exe`, discarding `--shell cmd.exe`; - a Windows-host project discards a WSL name and falls back to `COMSPEC` (`getHostShellForProjectRuntime`), so `--shell wsl.exe` spawns cmd. That is the common case, not an edge: `resolveProjectExecutionRuntime` resolves `windows-host` for every project that is not WSL, while a repo belonging to no project honours `wsl.exe` -- so the same flag behaved differently depending on whether the repo was in a project. Either rewrite returns a healthy terminal running a shell the caller did not ask for, which is the failure `--shell` exists to remove. It also split an agent launch's quoting from the shell that receives it. The previous commit made the startup-shell family follow the REQUESTED shell, so `--shell wsl.exe --command codex` on a Windows-host project typed POSIX-quoted launch args into cmd. Refusing the contradiction removes that case rather than papering over it. Refuse instead, alongside the SSH and non-Windows refusals, from the same `resolveAgentTerminalCreateOptions` seam every create lane funnels through. Also from review: - the allowlist test looped the list against itself; spell the members out. - the runtime spec case claimed to prove the pty's shell when it asserts the controller received the field; name it for what it checks. Reported by an adversarial review of the branch. * fix(terminal): canonicalize --shell and refuse a WSL-path rewrite Review of the --shell create path turned up two ways the terminal could still end up being a shell the caller never asked for -- the exact failure --shell exists to remove. Bare and mixed-case spellings passed the allowlist but reached consumers that exact-match the canonical name: resolveWindowsShellStartupFamily classified `cmd` as the PowerShell family, resolveWindowsShellLaunchArgs fell through to empty shellArgs (no `chcp 65001`, no OSC 133 bootstrap that Windows foreground status depends on), and resolveWindowsGitBashShellPath compares case-sensitively so `Git-Bash` spawned a literal `Git-Bash`. The allowlist is now one canonical-name map and terminal.create canonicalizes on parse, so the spawn path only ever sees `.exe` spellings. `pwsh` and `powershell` stay distinct binaries. A `\\wsl$\<distro>\...` cwd made the providers force wsl.exe regardless of the request, and terminalShellOverrideRefusal only inspected the project runtime -- undefined for a folder workspace with no project. Refuse on the resolved cwd and the workspace path, judging what the PTY actually gets. Also: the capability gate reported an unreachable host as too old rather than unavailable; the SSH CLI shim dropped capabilities from status, so --shell there blamed the host version instead of naming SSH; and --shell had no help entry, rendering bare in `orca terminal create --help`. Adding that entry crossed help.ts's max-lines cap, so the flag table moved to flag-help-text.ts rather than suppressing the rule. Adds a behavioural test for the runtime preflight (the one-line fix was pinned only by a source-text scan), plus coverage for the startup-command quoting family, the no-workspace refusal, and the WSL-path refusal. * fix(build): keep tests out of the RPC params catalog bundle The catalog walk under methods/ already skips *.test.ts, but the contract directory glob took every .ts. terminal-create-shell-param.test.ts is the first test to live there, so the bundle pulled vitest into a CJS build and the generator threw on require(). Same exclusion, same reason.	2026-09-15 16:34:16 -07:00
Neil	231e805b1e	fix(lint): enable anti-slop/no-shape-in-symbol-names (#20785) ... Flip `anti-slop/no-shape-in-symbol-names` from "off" to "error" and clear every violation under src, config, tests and mobile. What the rule bans ------------------ The case-insensitive substring "shape" in any JS/TS identifier: variables, functions, parameters, types, type parameters, class members, private names, object-literal keys and JSX identifiers. The one exemption is a statically accessed member read owned by another value (`zodObject.shape` is fine), so third-party APIs stay readable without a suppression. "Shape" names a value's structure rather than its domain role. `UserShape`, `validateArgShape` and `errorShape` all tell you the symbol is "an object with some fields" -- which is already what a type says -- while saying nothing about what the value is for or who owns it. The rule forces the name to carry the domain instead. Violations fixed ---------------- 689 violations across 109 files at baseline (verified by re-running the audit against the pre-change tree with the rule set to "error"). Fix pattern ----------- Rename for the domain role, not the structure: -type FieldShape = 'list' | 'map' | 'whole' -const FIELD_SHAPES = { ... } satisfies Record<keyof Observation, FieldShape> +type FieldEncoding = 'list' | 'map' | 'whole' +const FIELD_ENCODINGS = { ... } satisfies Record<keyof Observation, FieldEncoding> -function assertGitPushTargetShape(target: unknown): void +function assertValidGitPushTarget(target: unknown): void -function describeReadDirPathShape(p: string): ReadDirPathKind +function classifyReadDirPath(p: string): ReadDirPathKind Predicates became statements about the value (`isDeltaShapedProviderFrameKind` -> `isDeltaProviderFrameKind`, `isDeleteShapedDiscardEntry` -> `discardDeletesEntryFile`, `isSkillsCliAgentKeyShaped` -> `isUsableSkillsCliAgentKey`). Type aliases dropped the suffix where the remaining name was already unambiguous (`GhGraphqlErrorShape` -> `GhGraphqlError`). No wire-visible name was renamed: no IPC or RPC channel, stream opcode, request/response param, persisted field, or i18n key. The `--shape=symlink|copy` CLI flag read by .github/workflows/skill-update-roundtrip.yml is unchanged -- only the local variable holding it was renamed. Exemptions ---------- They are file-scoped entries in config/oxlint-anti-slop.json, not inline `oxlint-disable` comments. An inline directive naming an anti-slop rule reads back as an UNUSED directive under the root lint scan, which does not load this plugin -- the changed-code quality gate counts that warning, so the comment form cannot be used for a rule that lives only in this config. * src/renderer/src/components/browser-pane/annotate/**: in the screenshot annotator a "shape" is the drawn geometry -- pen, arrow, rect, ellipse, highlight. That is a genuine domain noun, and it pervades every symbol in the module. * repo-icon.tsx, repo-header-project-actions.tsx, mobile MobileRepoIcon.tsx: lucide exports the icon component as `Shapes`. The name is theirs, and the matching REPO_LUCIDE_ICONS key is the persisted icon name shared with the desktop picker -- renaming it would orphan saved repo icons. * src/shared/onboarding-state-types.ts, src/shared/constants.ts: `shapedSidebar` is a persisted onboarding-checklist field and a telemetry enum member; renaming it would orphan saved state. * src/shared/rpc-contract/rpc-send-params.ts: matching zod's own literal `shape` property is what selects the ZodObject branch of the conditional type. No exemption was added merely to avoid a rename. Eight symbols initially suppressed as "a cross-module refactor outside this change" were proven to have zero non-TypeScript references repo-wide and renamed instead. Zod's `ZodRawShape` needed no exemption at all: `Readonly<Record<string, z.ZodType>>` is its definition, so repo-update-params.ts and ui-update-value-tolerance-params.ts spell it out instead. Likewise telemetry-event-classification.ts now reads `.shape` through an `in` narrowing, which also retires two pre-existing type assertions; three more assertions the rename had dragged onto changed lines (two `JSON.parse` sites, one node:sqlite row read) became annotations and an explicit row mapping. Verified -------- * Audit reports zero violations; confirmed the rule genuinely fires by planting a probe violation. * node config/scripts/run-typecheck-projects-in-parallel.mjs exits 0. * Vitest over src/shared, src/main/github/project-view, the annotate module, the repo-icon components and the Chromium SameSite electron spec: all green. * All 66 removed "shape" identifiers grepped repo-wide across every file type; none survive. * node config/scripts/generate-rpc-params-catalog.mjs --check exits 0. * node --check on every changed .mjs; oxfmt clean on all changed files. * `pnpm run check:code-quality:changed` reports 0 findings. Not machine-verified: the 3 mobile/ files (its Vitest run cannot resolve `expo/tsconfig.base.json` in this worktree), and the WSL- and Playwright-gated specs. All are rename- or comment-only hunks, read in full.	2026-09-15 02:00:27 -07:00
Neil	bfdec26352	fix(lint): enable anti-slop/no-object-parameters (#20781) ... The rule rejects the broad `object` type on any function input (declarations, expressions, arrows, methods, call/construct signatures, function types), plus local aliases and unions that resolve to `object`. `object` accepts every non-primitive while exposing no properties, so it documents nothing and pushes callers into assertions at the boundary. Fixes all 185 violations across src, config, tests and mobile, and flips the rule from "off" to "error" in config/oxlint-anti-slop.json. Approach: replace each `object` input with the type its owner already has. Most sites took an existing domain type or a type-only import (36 added); 40 new aliases name shapes that had none. Where a value is genuinely only compared by reference, it gets a named identity token instead of a shape -- `Record<string, never>`, the built-in `WeakKey`, or a `unique symbol` brand, matching the branding already used in src/shared. Same treatment for WeakMap and Map key parameters. Two `as unknown as` casts became unnecessary once the parameter carried a real type and were removed; no new casts were added. Suppressions added: none. No `oxlint-disable` for this rule anywhere, and no max-lines disable or per-file bump. Three files sat exactly at their max-lines cap, so the added type imports were made line-neutral rather than suppressed: - src/main/ipc/browser.ts exports the existing guest-registration args type (renamed BrowserGuestArgs) so browser.test.ts reuses it on one line. - pane-scroll.ts takes TerminalScrollIntentTarget through the existing pane-manager-types import via a type-only re-export. - direct-rpc-client.ts drops the identity parameter entirely: the session check moved into the sendProbe callback that owns the token. Verified: anti-slop config reports zero violations over src config tests mobile; run-typecheck-projects-in-parallel exits 0; 144 affected test files pass (1749 tests); oxlint and oxfmt clean on all changed files. Mobile has no runnable test/typecheck target in this worktree (expo is not installed), so its 6 files were typechecked against a standalone config and diffed against the base branch -- error sets are byte-identical, including test files.	2026-09-15 01:59:58 -07:00
Neil	f7b2736d6d	fix(worktree): block removal when the archive hook fails (#20153) ... * fix(worktree): block removal when the archive hook fails A repo's orca.yaml archive hook is the user's last chance to save work off a checkout Orca is about to delete. A failed hook was logged as advisory and stepped over, so the removal went ahead with nothing archived — and the caller could still be told it succeeded. The hook is now a blocking precondition, evaluated while the checkout, its Git registration, its agents and Orca's ownership evidence are all still intact: it sits ahead of the registration re-read, the lock/dirty preflights, stopPtys() and removeWorktree in every orchestrator that runs it. Failure is typed (worktree_archive_hook_failed) and carries the worktree path, outcome, exit code where one was observed, and the hook's output. unverifiable stays distinct from exited, so loss of contact is never read as a pass. The waiver rides its own field at every layer and is never implied by --force, which already carries the PTY-stop waiver; when used, the waived failure comes back on result.archiveHookOverride rather than being swallowed. worktree.archive-failure-blocking.v1 is advertised so an integration can tell "accepts --run-hooks" from "safely propagates a failing hook" without risking the data loss to find out. The runtime's SSH path cannot run a hook at all, so rather than delete with the archive step silently skipped it refuses — waivable like every other refusal here. #18563 retires that gate by making the path run the hook for real. Stacked on #20559, which makes a timed-out hook report honestly; without it a hook that traps SIGTERM and exits 0 would defeat this gate. Fixes #19334 * fix(worktree): close the skip-confirm dead end and the client/hook timeout gap Four review findings on the gate. A retry from the failure toast could fail for a DIFFERENT reason than the one the user had just answered, and that second failure got a bare toast with no buttons. With skipDeleteWorktreeConfirm set, the delete helpers pass no force, so waiving a failed archive hook on a dirty checkout landed on the dirty preflight and stopped there. Retry failures now re-enter the same failure toast, so every retry stays as actionable as the first attempt. Third instance of this class. The renderer gave worktree.rm a 60s budget while an archive hook may run for 120s. A hook that took 90s and succeeded timed the client out and reported failure while the host went on to delete — telling the user their delete failed and their checkout was gone. The budget is now derived from the hook's, and only when a hook can run. The SSH fail-open is logged rather than silent, and the capability's doc comment scopes what it claims: a hook that RUNS and fails cannot delete the checkout; it is not a promise the hook was found. The SSH owner-resolution test now reads a real remote orca.yaml through a stubbed provider and asserts the returned script is the remote one. It previously stopped at the lookup key, which is the coverage that let this path break twice. It fails against the row-only resolution. * fix(worktree): name a signalled hook exit, and state why prunable cleanup skips the gate Two things the rebase onto #20617 and #20576 surfaced, both found by rerunning the real-repo harness rather than by reading the diff. - #20617 added a registration-cleanup branch that returns before the archive gate. That ordering is correct — both of its arms describe a row with no checkout behind it, so there is nothing to archive and running the hook would fail on the missing cwd — but the gate's ordering invariant is documented, so the exception should be too. - A signalled hook reported `Command failed with exit code null.`, which reads as a reporting glitch rather than the `unverifiable` verdict it is about to produce. It now says the command was terminated without reporting an exit code. Introduced by #20576; the withheld `exitCode` itself was always right. Fixes #19334	2026-09-15 01:19:32 -07:00
Neil	37a5b278b3	test(package): reject an Electron install takeover by exact command (#20799) ... * test(package): reject an Electron install takeover by exact command CodeRabbit was right about #20787. Replacing the pinned postinstall string with a /electron/i keyword check was wrong in both directions, verified: rebuild-native-deps.mjs && rebuild-native-deps.mjs PASSED (should fail) rebuild-native-deps.mjs && check-electron-version FAILED (should pass) The owner's own path contains no "electron", so duplicating it slipped through -- the one case the contract is named for. And a substring match rejects any later step that merely mentions Electron, which is the same over-tightness that broke every open PR in the first place, relocated. Later steps are now checked against the exact owned command plus the known Electron install commands. A second case pins the rejections themselves, because reading the real postinstall cannot show a bad chain would be caught -- that is how #20787 shipped with a guard that did not guard. Split into its own file rather than adding a max-lines disable (AGENTS.md). * test(package): match install commands as tokens and cover the rebuild:electron alias Both review comments were right, verified by running them: && check-install-app-deps-version.mjs rejected by substring match (should pass) && pnpm run rebuild:electron slipped through (should fail) package.json:101 aliases rebuild:electron to the owned script, so invoking it is the same takeover. Matching is now token-based with the owned command still checked as a phrase, and both cases are pinned.	2026-09-15 01:10:35 -07:00
Neil	22ce8d69a1	fix(lint): enable anti-slop/no-module-mocking (#20783) ... The rule rejects `vi.mock` / `vi.doMock` / `vi.unstable_mockModule` and the `jest` equivalents, on the argument that a test which rewrites the module graph asserts against a stand-in the production code never sees. It is already off for `**/*.test.{ts,tsx}`, `**/*.spec.{ts,tsx}`, `tests/**` and `**/__mocks__/**` via the existing override in config/oxlint-anti-slop.json; that override is unchanged here. What the rule actually catches is module mocking that has drifted out of a spec and into a first-party `.ts` support module, where nothing marks it as test-only. 73 violations at baseline, all of them in test-support code. 9 were relocated back into spec files the override already exempts; the remaining 64 sit in 10 files that are test-only but do not match the override globs, and carry a file-level disable naming the rule and the reason. Relocated: - terminal-hydration-store-test-bootstrap.ts: the sonner / sync-runtime-graph / pty-transport `vi.mock` calls moved into the two specs that import it (terminals-hydration-canonical-rows, terminals-hydration-canonical-pty-overlap). Vitest hoists `vi.mock` inside a test file, so registration is strictly earlier than the previous module-eval-time call; the bootstrap keeps only the preload API proxy. Both importers were updated. - ipc-events-ssh-authority-test-fixtures.ts: the 6 direct-ssh `vi.doMock` calls moved into useIpcEvents-agent-status-ssh-authority.test.ts as a local `stubDirectSshModules()` helper, which also de-duplicates the three copies the spec already had inline. The fixture now returns the store state and coordinator doubles it builds, typed via the exported DirectSshReconnectCoordinatorDouble. Suppressed, with justification (each is `/* oxlint-disable anti-slop/no-module-mocking -- ... */`, rule named, no blanket disable): - config/scripts/headless-serve-shutdown-matrix.test.mjs (1) - a genuine Vitest spec that the override misses only because its globs say {ts,tsx}. The script under test is a top-level CLI module; the alternative is spawning real docker. - src/main/codex-accounts/runtime-home-service-test-harness.ts (1) - stubs one probe predicate in ../pty/shell-startup-env, imported directly by several main-process readers; 17 specs share it. - src/main/computer/desktop-script-provider-test-harness.ts (2) - stubs child_process/fs-promises for a provider that shells out; 8 specs share it. - src/main/github/work-item-search-test-harness.ts (4) - one consumer lives in tests/e2e, where the relative mock ids resolve differently, so moving the calls into the specs would silently stop mocking there. - src/renderer/src/components/automations/automations-page-test-harness.tsx (14) - the mount rig for 10 AutomationsPage specs. - src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-test-harness.ts (1) - stubs refreshWebRuntimeSessionTabsSnapshot, imported directly by several renderer runtime modules; 18 specs share it. - src/renderer/src/hooks/ipc-events-agent-status-window-test-fixtures.ts (7) - stubReactSyncEffect/stubAuxiliaryModules, shared by 11 specs. - src/renderer/src/hooks/ipc-events-close-routing-test-harness.ts (11) - stubs and hook invocation are one unit; 4 specs share it. - src/renderer/src/hooks/ipc-events-terminal-create-test-harness.ts (13) - its only spec is at 799 of an 800 max-lines budget. - src/renderer/src/hooks/ipc-events-test-harness.ts (10) - shared by 8 specs. No violation was converted to real dependency injection, and no max-lines disable was added. Verified: the audit command exits 0 with no output (and reports errors on a planted probe, so the rule is live); node config/scripts/run-typecheck-projects-in-parallel.mjs exits 0; 354 spec files / 2506 tests covering every importer of every touched file pass. No mobile/ file was touched. The changed-code quality gate's root Oxlint scan runs without --config so it never loads the anti-slop JS plugin, which made all 10 of those file-level suppressions read as "Unused oxlint-disable directive". check-changed-code-quality.mjs now exempts directives naming an anti-slop rule from that unused-directive warning, the same carve-out isCastingDirectiveUnusedWarning already makes for the casting suppressions the casting config enforces. Such a directive can never suppress a root-config rule, so nothing the root scan would otherwise report is hidden; audit:anti-slop remains the scan that enforces the rule.	2026-09-15 00:41:17 -07:00
Neil	49e5fa597a	refactor(lint): enable anti-slop/no-reflect-apply (#20782) ... `anti-slop/no-reflect-apply` rejects `Reflect.apply(fn, thisArg, argsArray)`. It defeats the call-signature checks TypeScript applies to an ordinary call: the args array is checked as an array, not positionally against the callee's parameters, so arity and type errors pass silently. Dynamic dispatch belongs behind a named interface, not behind a reflective call. Flipped the rule from "off" to "error" and cleared all 17 baseline violations across `src config tests mobile` (16 sites; one file had two). Fix pattern: `Reflect.apply(fn, recv, args)` becomes `fn.call(recv, ...args)`, or a direct method call when the implicit receiver is already the right object. The receiver is preserved at every site. Where the callee is a captured built-in whose overloads split on an argument's shape (`String.prototype.split`, `JSON.stringify`), a call-signature capture no longer compiles once the args are passed positionally. Those three sites capture the function through a method-shaped type (`{ split(separator: unknown, limit?: number): string[] }['split']`), which keeps the forwarding call checked rather than asserted. Behaviour notes: - `diff-section-layout.test.ts` drops a `limit === undefined ? [sep] : [sep, limit]` conditional. Equivalent: `String.prototype.split` maps an undefined limit to 2^32-1, and the `Symbol.split` path forwards undefined either way. - `workspace-space-compaction.test.ts` forwards `reduce`'s two arguments unchanged, so the `arguments.length >= 2` initial-value branch is unaffected. - `agent-session-history-byte-accounting.test.ts` is the one site where the receiver is not literally preserved (`JSON` -> undefined). `JSON.stringify` never reads `this` per spec, and restoring `.call(JSON, ...)` would reintroduce the overload failure under strictBindCallApply. No suppression comments added — the rule has zero `oxlint-disable` sites. `Reflect.apply` still appears at electron.vite.config.ts:159, inside a template literal of generated bootstrap source. That is string content, not lintable code.	2026-09-15 00:10:11 -07:00
Neil	18d0afc918	test(package): let the postinstall contract allow unrelated chained steps (#20787)	2026-09-14 23:26:48 -07:00
Neil	11180fa532	chore(lint): add anti-slop oxlint plugin (pinned, all rules off) (#20726) ... * chore(lint): add anti-slop oxlint plugin (all rules off) Vendors dmmulroy/anti-slop (MIT) plus no-call-only-assertions and no-pass-through-type-alias from maharshi365/deslop (MIT). Every rule starts "off"; each follow-up PR fixes one rule's violations and flips it to "error". * fix(lint): actually exclude the vendored plugin from the anti-slop audit oxlint does not honour ignorePatterns supplied via --config, so the config/oxlint-plugins/anti-slop/** entry never matched and the vendored rule source was being linted as first-party code (505 violations). Move the exclusion to the --ignore-pattern CLI flag in audit:anti-slop, which does work, and drop the entry that gave a false sense of coverage. Keeping vendored source unlinted matters because anti-slop is updated by three-way merge against the upstream snapshot; reformatting it locally would conflict on every update. * chore(lint): pin anti-slop instead of vendoring it; drop deslop Replaces the ~5k vendored lines with a git-pinned devDependency: oxlint-plugin-anti-slop: github:dmmulroy/anti-slop#c44ef22 anti-slop ships raw .ts with no build step, and Node refuses to type-strip anything under node_modules (ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING), so oxlint cannot load it from there -- which is why upstream says to vendor it. A postinstall step copies the pinned package's source to .anti-slop-plugin/ (gitignored), which Node will type-strip because it sits outside node_modules. Upgrading is now a SHA bump rather than a re-vendor and three-way merge. Verified byte-identical rule output to the vendored copy across all 16 rules that fire. Drops maharshi365/deslop and its two rules (no-call-only-assertions, no-pass-through-type-alias). It is not on npm either, so it would need a second git pin and copy step, and it is a 5-star single-maintainer repo that is itself a re-namespaced copy of anti-slop. One upstream is enough. * ci(lint): run audit:anti-slop in PR CI config/scripts/pr-workflow-lint-parity.test.mjs requires every step in `pnpm lint` to have a matching step in .github/workflows/pr.yml; adding audit:anti-slop to lint without the workflow step failed that ratchet. Also makes audit:anti-slop sync the plugin itself before linting. The generated .anti-slop-plugin/ directory is gitignored and otherwise only created by postinstall, so a cached install that skips postinstall would leave oxlint unable to load the plugin.	2026-09-14 21:42:37 -07:00
Neil	b61a2347b9	feat(design-system): gate renderer UI with @shadcn/lint (#20731) ... * feat(design-system): gate renderer UI with @shadcn/lint Wires shadcn-ui/lint's Oxlint plugin into the two places this repo already ratchets: the changed-lines PR gate for rules the renderer can't satisfy today, and `pnpm lint` for the one that is already at zero. - config/oxlint-design-system.json: no-restyle (layout allowed), no-raw-colors, require-static-classes -- scoped to src/renderer/**/*.tsx, run over added lines only. Measured at 10 findings across the last 60 commits (771 changed files), so it holds the line without a migration. - config/oxlint-dead-classes.json: no-unknown-classes repo-wide, with the renderer's plain-CSS hook namespaces allow-listed. Now at zero. - no-inline-styles and no-arbitrary-values stay off; STYLEGUIDE says why. Fixes the three live bugs the linter found: - `--editor-surface` never reached `@theme inline`, so `bg-editor-surface` generated no CSS -- 12 editor/artifact/notebook panes fell through to the page background instead of #1e1e1e in dark mode. - `scrollbar-none` is not a Tailwind utility and was declared nowhere, so the remote file browser breadcrumbs showed the scrollbar they meant to hide. Declared as a real `@utility`. - Notebook markdown cells used `markdown-preview-body`, which no stylesheet defines; the styled class is `markdown-body`. They rendered unstyled. * ci: run the dead-class gate in PR CI `pnpm lint` gained check:dead-classes, and pr-workflow-lint-parity requires every `pnpm lint` step to have a matching step in pr.yml. * fix(notebook): keep markdown theme selectors working	2026-09-14 17:52:21 -07:00
Neil	20794ee785	ci: keep the baseline build off the compatibility matrix lanes (#20733) ... The compatibility gate started the pinned 2.25.5 source build inside the same step that runs the three measured lanes, so `make -j$(nproc)` competed with two container lanes whose wall clock is container starts, not Git. A boundary case that costs ~1.5s stretched past Vitest's 30s timeout and failed the job. Build the binary in its own step before the matrix, and pull both images before any lane starts so a lazy pull cannot stall whichever test its sibling is timing.	2026-09-14 17:32:33 -07:00
Neil	fc4519cda4	fix(omp): preserve zsh startup with global aliases (#20621) ... Validated and independently reviewed OMP integration fix.	2026-09-14 13:56:22 -07:00
Neil	1ba9801574	fix(ci): stop hourly versions dropping below a tagged or already-shipped build (#20699) ... * fix(ci): stop hourly versions dropping below a tagged or already-shipped build Hourly/daily/adhoc based their X.Y.Z on GitHub releases, not git tags. When v1.4.202 was tagged and then its GitHub release vanished, the next hourlies shipped as 1.4.202-hourly — below both stable 1.4.202 and the 1.4.203-hourly builds already installed, so electron-updater stopped offering updates. Read main's v* tags and already-published channel tags instead. * docs(ci): record that 1.4.202's release was unpublished for a bug The leftover tag is what hourly must still honor; this was not a failed cut.	2026-09-14 13:33:04 -07:00
Jinwoo Hong	eba56f2f69	feat(ai-vault-search): construct the session search indexer in the scanner service behind a setting (#20516) ... * feat(ai-vault-search): persist agent-session search consent and retention Two booleans and nothing else: `enabled` and `historyDays`, off by default because building the index reads every transcript on the machine. No `paused` -- the PR 3 indexer is immutable, so every change is close-and-construct. The settings IPC normalizes a write like every other field and hands the change to the index; there is no UI for it until PR 8. * feat(ai-vault-search): hold one indexer and engine pair per host The object that owns a host's live index and the three recipes that change it. The indexer is immutable, so a settings change is close-and-construct, disabling is close with no replacement, and clearing is close, remove the database, construct. The new instance's first sweep purges a narrowed window and admits a widened one, so neither needs a code path. The database sits beside the scanner's parse cache, one file per host. A runtime with no node:sqlite can hold no index at all, which the Node 18 floor on orcad and the relay makes a real case rather than a hypothetical one. * feat(ai-vault): let the scanner child own the session search index The transcript reader runs in that child, so the index consumer has to as well: one read serves both the session list and the index. Three request operations (search, status, reconcile) and one fire-and-forget settings message carry everything a parent needs; main never opens the database file. The init frame becomes a factory because it is read at every spawn, so a respawned child sees current consent rather than the first frame's. A child holding a running index is never idle from the parent's side, so idle retirement is suppressed while the index is on -- retiring it would stop the reconcile loop until some later scan happened to respawn one. Both files this lands in were already at the max-lines ceiling, so three collaborators move to where they belong rather than being disabled around: the invalidation deadline into the class that owns invalidations, call cancellation and the start requeue into the call-state module, and orcad's flag parsing into its own file. * feat(ai-vault-search): register a search service on every host that answers Without a registered service a host answers no-service, which means "this host does not have the feature" rather than "the index is off". All three hosts now answer the second thing. The desktop forwards to the scanner child. orcad and the SSH relay daemon have no such child -- orcad ships only the watcher and daemon entries, and the relay's AI Vault sidecar runs the remote scanner, which publishes nothing to the transcript channel -- so on those two the index lives in the process that would drive its reads, gated on a runtime that has node:sqlite at all. The relay registers with consent off and no way to turn it on: nothing carries a setting to a remote host yet. That is the honest state, and it is still worth registering, because it is what tells a client the difference between off and too old. * test(ai-vault-search): price a warm pass over five thousand transcripts The number the reconcile interval will be revisited against, measured rather than argued: a warm sweep stats every file under every root, a warm cycle stats the newest N per agent, and neither reads what the index already holds. It does not tune the interval. * fix(ai-vault-search): answer the casting gate without assertions main's new type-assertion rule reaches every file this branch touches. All nine sites drop the cast rather than carry a SAFETY: rationale: the operation guard narrows with `in`, the sqlite probe narrows the builtin it loads, the child test keeps the discriminated reply instead of widening it, and the settings resolver takes `unknown` -- which is what it really reads, since a persisted profile can hold a value no version of this code wrote. * fix(ai-vault-search): let a refreshed scan root reach the live index The parent re-resolves scan roots before every policy push, precisely so a WSL distro or extra Codex home that appeared since the child spawned enters the window. The child forwarded only the settings to a live instance and used the roots solely in its `??=` initializer, so those roots were dropped for the child's lifetime. The indexer stays immutable: a structurally different root set closes the pair and constructs a new one, the same way a changed databasePath already does. Compare via `sameSessionSearchRoots` rather than a plain JSON compare, because nothing fixes the key order two producers write; lists are sorted too, since the indexer walks every root and a re-enumeration that reorders is not a change. An unchanged set still never restarts a running index. The orcad and relay in-process hosts resolve roots once at install and never re-apply, so they have no such seam. * fix(ai-vault): restart the scanner child the index is holding Three review items. The hold keeps a child alive for the index, but only a queued call ever started one: `pump()` skipped a hold with an empty queue, so an idle indexing child that crashed, or an `ensureChild()` that failed at start, left indexing stopped until an unrelated request happened to arrive. `pump()` now starts the child the hold requires, which is also the restart callback the fault policy already schedules, so the existing delay and circuit bound the retry exactly as they bound a queued call's start. `updateSessionSearch` goes through the same seam instead of its own `ensureChild` call. A search registers no AbortController, so a cancel sent for a search id was added to the `cancelled` set and never consumed. Nothing can reach that today -- no caller passes a signal and the child answers in milliseconds -- so this is only a leak of ids: consume it when the search settles. The orcad argument doc claimed a `--`-prefixed value stays a flag. The parser takes the next token regardless, and orcad-launch-contract.test.ts pins that, so the doc is what was wrong. Behaviour is unchanged. * fix(ai-vault): recover search indexing and refresh scan roots * fix(ai-vault): defer search refresh policy reads * fix(session-search): stabilize paging and host enablement * fix(session-search): refresh host roots within full sweeps * docs(session-search): clarify initial root fallback	2026-09-14 13:38:37 -04:00
OrcaWinandOrca Worker	243f443155	fix(session-search): read oversized numeric file IDs on Windows (#20551) ... Co-authored-by: Orca Worker <orca-worker@localhost>	2026-09-13 23:25:51 -07:00
JinjingandJinwoo-H	d2d32691ef	perf(persistence): skip redundant whole-state flushes on terminal reattach (#20137) ... * perf(persistence): add pty-binding fast lane to skip redundant flushes Terminal pane reattachment currently clones the session and serializes the entire 9.2 MB app state even when the binding is already in place and durable. Add an early-return fast path that skips this work when all nine predicates hold: no split, binding matches in-memory and on-disk, incarnation matches, no tombstone, and generation counter proves durability. Includes one-line fix in `writeToDiskSync` to record hash-matched sync flushes as durable, so the fast path doesn't stay parked behind a stale generation. Adds `persistence.pty-binding` observability spans (local NDJSON, unsampled for mutations, budgeted for fast-lane hits) to measure eligibility rates before and after. Includes ratchet test to ensure every binding writer bumps the generation. Diagnostic tools and full investigation notes from September 7, 2026 capture that identified the 59–100 ms no-op binds and measured a real terminal keystroke queued 117 ms behind one such call. * perf(persistence): add pty-binding fast lane to skip redundant flushes Rapid rebinds of already-durable PTY bindings (e.g., remounting panes) were unnecessarily expensive because they cloned and flushed the entire document state every time. Detect when a binding hasn't changed since the last durable write and skip to return immediately, eliminating main-thread cost on that path. * perf(persistence): record binding.origin on the pty-binding span Fresh spawns always flush, so a fast-lane rate over all calls is diluted by however many terminals the user opened. Each caller knows whether it is a spawn, a reattach, a split, or a relay reattach; pass that through as metadata and record it so the reattach hit rate can be read from the trace file. Never branched on. * fix(persistence): keep the tab row on its first pane when a sibling pane binds A tab row names one PTY, but a split tab holds several panes. The renderer keeps the row on the first pane and refuses to let later split-pane spawns steal it, since a remount reattaches the tab to whatever the row says. Main overwrote it with whichever pane was binding, and the renderer's next publish put it back, so every sibling reattach was a state change and could never take the fast lane. On the real profile that is 38% of panes. Rewrite the row only when it names nothing useful: null, the PTY this leaf is replacing, or a PTY no leaf holds. The fast-lane predicate compares against the same rule. * perf(persistence): record durable pty-binding flushes per pane The global write generation is held back by any unrelated dirty state, causing bindings unchanged for minutes to appear unpersisted despite being on disk. Track per-pane durability to skip redundant flushes. * docs(persistence): describe the per-pane durability record The durability section still described the global generation check as the whole story and claimed there was no binding durability cache. Record the measurement that motivated the per-pane record, and why retiring one needs no cooperation from other binding writers. * docs(perf): consolidate every measured Orca performance issue into one register Folds the findings from all related debug sessions into the live lag investigation: the persistence/main-thread work (P1-P11), host contention (H1-H5), git and subprocess load on main (G1-G8), renderer and terminal rendering (R1-R8), the terminal daemon session leak from the deleted debug-orca-perf-issue worktree (D1-D9), and the Cmd-J palette review (C1-C6). Keeps the measurement behind each claim, records what is fixed versus open, and restates what the 117 ms keystroke delay still does not explain. * fix: address performance review findings * fix: satisfy diagnostic probe lint * chore: keep investigation artifacts out of performance PR * fix: run lag probe regression tests with Vitest * perf(persistence): replace pane receipts with global durability check * refactor(persistence): remove redundant binding review machinery * test(persistence): satisfy current assertion-free quality gate --------- Co-authored-by: Jinwoo-H <jinwoo0825@gmail.com>	2026-09-14 01:04:43 -04:00
Jinwoo Hong	c6548b98f4	test(scripts): widen the Windows shim ratchet to catch package bin spawns (#20285) ... * Widen Windows shim ratchet to detect package bin spawns Follow local program expressions into node_modules/.bin while preserving the existing literal check, roots, and allow-list. Document static-analysis limits and cover unsafe and resolver-based invocations. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(scripts): fold dot segments before matching node_modules/.bin The predicate joins call arguments textually, so a literal '..' segment hid a path that resolves into node_modules/.bin at runtime. Folds '.' and '..' (and Windows separators) first. A '..' that genuinely escapes .bin still does not match. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * style(scripts): use .at(-1) in the dot-segment fold oxlint's prefer-at rule; the repo-wide lint gate is an error, not a warning. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-13 16:08:56 -04:00
Jinwoo Hong	bc5e67606f	test(rpc): add a compile-time params catalog parity gate (#20281) ... * Add compile-time RPC params catalog parity gate Check each registered handler against its catalog params type in both directions, with explicit exceptions for the three uncatalogued schemas. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(rpc): keep the params generator off its own output The parity gate imports the generated catalog for types, and it lives under RPC_DIR, which indexableModules() scans for shared imports. That re-added OUTPUT_PATH after line 46 removed it, so the generator bundled and require()d the committed catalog. A catalog referencing a renamed or deleted shared export then crashed regeneration — in exactly the state that requires regenerating. Reproduced before and after: with a dangling reference injected into the catalog, `generate:rpc-params-catalog` threw; it now rewrites the file. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-13 16:08:53 -04:00
3 people	6c1d95b0da	perf(tooling): reuse directory entry types in source scans (#20212) ... * perf(tooling): reuse directory entry types in source scans * fix(source-scan): stat DT_UNKNOWN dirents so untyped directories are still walked `readdirSync(..., { withFileTypes: true })` can hand back a Dirent whose type the filesystem did not report. For that entry every predicate is false, so the readdir-type fast path treated a real directory as a file and silently dropped its subtree from every ratchet guard. Fall back to `statSync` whenever the entry is neither conclusively a file nor a directory, keeping the no-stat fast path for ordinary entries. Also make the two readdir-order assertions in the walk test order-independent; `scanSourceTree` returns raw readdir order, which differs on tmpfs. * test(source-scan): unit-test the stat fallback via an extracted helper The fabricated-Dirent readdir mock could not satisfy both gates at once: vi.mocked(readdirSync) resolves to Node's Dirent<NonSharedBuffer> overload, so the mock needed a type assertion, and #19462's casting gate rejects new ones on changed lines. Removing the cast then failed tsc. Extract directoryEntryNeedsStat and test it directly with a structural probe. No mock, no cast, no top-level await, and the DT_UNKNOWN case is pinned: removing the fallback fails 'stats an entry whose type readdir could not report'. --------- Co-authored-by: m4air <m4air@Mac.localdomain> Co-authored-by: Neil <neil@stably.ai>	2026-09-12 22:38:25 -07:00
Neil	e86cba888b	build: reduce native dependency installs to the host platform (#20420) ... * Reduce native dependency installs to the host platform * Remove install policy documentation * Guard cross-arch packaging and scope release installs to the runner electron-builder only logs a warning for a missing extraResources source, so a host-only install silently shipped a foreign-arch slice without its natives — `pnpm build:mac` on Apple Silicon produced an x64 DMG with no sherpa-onnx-darwin-x64 and no @parcel/watcher-darwin-x64. The previous beforePack hook covered only win32. - Add assertPackagedNativeVariantsInstalled, an arch-aware check over the target's sherpa-onnx, @parcel/watcher, and (on Windows) node-gyp addons. beforePack now runs it for every platform, with remedies split: another architecture comes from install:release, the os:win32 addons need a Windows host. - Drop --os from the release installs. Every packaging job already runs on a runner whose OS matches its target, so only the macOS lanes need extra breadth, and only on CPU for their x64+arm64 config. Windows and Linux packaging return to a plain host-only install. - Add --frozen-lockfile to install:release so a bare run cannot rewrite the lockfile. - Restore the install policy reference doc and the CONTRIBUTING note, plus the rationale comments dropped from the runtime contract test. - Gate the packaging-closure assertions on whether the Windows addons are installed rather than on the host OS, so a cross-arch install exercises them off Windows too. - Make the workflow contract test read `run:` steps as well as retry-action commands, and enforce host-only scoping on the non-macOS packaging lanes. - Remove the unreferenced install measurement script; its numbers live in the policy doc. * Track the install policy doc and index it from AGENTS.md docs/** is ignored behind a per-file allow-list, so the new reference doc was only committed via git add -f and future edits would be skipped. Add it to the allow-list and give it an AGENTS.md entry like every other tracked reference doc, so the host-only install rule is discoverable before someone packages a second architecture. * Route Windows-lane removals through the retrying helper Adding these four specs to the PR Windows lane pulled them into the windows-lane-tree-removal-boundary ratchet, which failed on 20 raw recursive removals. On Windows a bare rmSync races a handle the OS has not released, throwing EPERM after the assertions already passed and reporting a green test as a lane failure. * Adapt the packaging guard to the vendored Windows registry addon main vendored windows-native-registry as the workspace package @orca/windows-registry (#20438). A workspace link resolves on every host, so including it in the installed-Windows-addons checks proved nothing. @vscode/windows-process-tree is the only os: win32 npm addon left, so it alone decides whether the win32 resource plan resolves.	2026-09-12 21:25:03 -07:00
Neil	df375cdd8a	perf(mobile): reuse Linear issue ordering when grouping list and board (#20314)	2026-09-12 21:19:00 -07:00
3 people	81c3d188a4	build(macos): parallelize native helpers with complete cancellation (#19651) ... * build(macos): run native module builds concurrently * fix(build): terminate sibling native builds when one fails Address coderabbit review: concurrent builds kept writing native artifacts after a sibling reported failure. Track spawned children, kill remaining siblings on first nonzero exit, and forward SIGINT/ SIGTERM to all children. * fix(build): process-group teardown and prefixed output for parallel native builds Address second coderabbit round: - Detached process groups + negative-pid kill so SIGTERM reaches swift/ swiftc descendants, not just the direct pnpm child (they could keep writing artifacts after fail-fast) - Signal handlers preserve the received signal (SIGINT no longer becomes SIGTERM for children) and are removed before re-raising, so the parent actually dies instead of looping through terminateAll - runPnpmScript settles only on close, never on error alone, so Promise.all cannot exit while children are still running - Per-module output prefixes ([computer]/[keyboard-layout]/[notification- status]) match what the PR description always claimed; interleaved swiftc errors are now attributable - Windows path untouched (early return before any of this runs) execa/p-limit were considered and rejected: no new runtime deps for a build script, and detached process groups give strictly stronger cleanup than execa's direct-child kill. * fix(build): memoized handler removal and external-vs-sibling signal split Second-round coderabbit findings on 24392a0: - Registration now uses the memoized handlerFor() instances so removeListener actually removes them (inline arrows were never registered, so the parent looped through terminateAll and hung) - externalSignal is set only by the parent's own signal handlers; a sibling's fail-fast SIGTERM no longer masquerades as an external signal, so settle() resolves Promise.all with the failing module's exit code instead of leaving top-level await unsettled (exit 13) - Also fixes a TDZ crash: handlerFor() was invoked at registration time before the signalHandlers const initialized Verified: sibling fail-fast resolves failer=7 with no survivors; external SIGINT kills children then the parent exits 130; real concurrent macOS build green. * Wait for native build cancellation before exiting * Clean up native builds when output streams fail * fix: bound native build waits, forward SIGHUP, honour output backpressure - Bound the per-child close wait: two seconds after a child exits, reap its process group and destroy its pipes so a descendant that inherited stdout/stderr cannot hang `pnpm build:native` forever. - Handle SIGHUP alongside SIGINT/SIGTERM so a terminal hangup reaches the detached compiler sessions instead of orphaning them. - Pause a compiler's output stream when the launcher's stdout/stderr reports backpressure and resume on drain, so prefixed output no longer buffers without bound. - Run build-native-for-platform.test.mjs in the computer-e2e mac-native-owner-smoke PR job and trigger that workflow on launcher changes; the tests are darwin-only and no other PR job runs on macOS. - Report the first failing child's status: re-raise its signal, or use its exit code instead of Math.max over cancelled siblings. * fix(native-build): keep output when reap timer overlaps backpressure; fail on ignored re-raised signal The descendant reap timer started on every child 'exit' and fired even when 'close' was late only because the launcher paused the pipe for its own stdout backpressure, destroying pipes with compiler output still queued. Arm the countdown only while the pipes are actually draining: clear it on 'pause' and re-arm on 'resume' after exit. Write the reap notice to stderr since stdout is the stream that may be blocked. Re-raising a child's fatal signal is a no-op when Node ignores it (SIGPIPE), so set a non-zero exit code first; a failed build no longer exits 0. Tests: stall the launcher's stdout consumer past the reap timeout and assert every kernel-accepted compiler line still arrives; kill the computer build with SIGPIPE and assert the launcher exits 1. --------- Co-authored-by: m4air <m4air@Mac.localdomain> Co-authored-by: Neil <neil@stably.ai>	2026-09-12 21:18:40 -07:00
Neil	bd0f8826ea	fix(ci): match the truncated windows-process-tree virtual store dir (#20447) ... On Windows, pnpm shortens the virtual store directory to @vscode+windows-process-tre_<hash>, cutting into the package name before the @, so the @vscode+windows-process-tree@* glob matched nothing and the addon recompiled on every Windows job. node-pty escapes this because its truncation lands after node-pty@, which the glob still matches. Widening the prefix to @vscode+windows-process-tre* matches both the full name kept on macOS/Linux and the truncated Windows one.	2026-09-12 21:11:39 -07:00
Neil	411843f633	fix(ci): cache the vendored addon where node-gyp actually writes it (#20445) ... The workspace link means pnpm never creates a .pnpm/@orca+windows-registry@* entry, so all four native-cache blocks globbed a path that cannot exist and the addon was recompiled on every Windows job. Also hardens the addon itself: RegEnumValueW reports a byte count and the registry does not enforce whole WCHARs for string types, so an odd count let Napi's auto-length scan run past the value; and a value named __proto__ would reassign the result object's prototype instead of becoming an entry.	2026-09-12 20:45:42 -07:00
Jinjing	182cd4c2f7	Add code quality lint for type assertions (#19462) ... * Add casting code quality lint scan Enforce type assertion style by adding a new oxlint scan with `typescript/consistent-type-assertions` rule. Requires using `as const`, type annotations, or `satisfies` instead of raw type casts, with documented `SAFETY:` exceptions for unavoidable cases. * fix minor issue	2026-09-12 20:43:36 -07:00
Neil	5127d1eb3b	refactor(windows): vendor the registry addon as @orca/windows-registry (#20438) ... * refactor(windows): vendor the registry addon as @orca/windows-registry windows-native-registry@3.2.2 was last published in 2023 by a single maintainer. Orca called two of its exports, both read-only, so the whole dependency is replaced by a local N-API addon under native/. The vendored addon is read-only by construction: setValue, createKey and deleteKey are gone, so RegDeleteTreeW no longer ships in the app. Two upstream defects are also fixed rather than carried over — the name/data scratch buffers were file-scope statics that concurrent reads would scribble over, and createKey/deleteKey called .c_str() on a temporary. Build wiring keeps the existing shape: still an optionalDependency gated to win32, still excluded from pnpm's allowBuilds so only Orca's own Windows rebuild runs node-gyp for it, still copied into the packaged resources. The CI native caches now key on the vendored sources so an addon.cc edit cannot restore a stale .node. * test(windows): check the vendored registry addon against reg.exe The addon is vendored source, so no upstream release proves it still decodes values the way Orca's PATH readers expect. reg.exe is the only independent oracle on the box. * ci(windows): register the registry addon test on the Windows runner A Windows-gated file self-skips on ubuntu, so without both registrations it reports success while running on no machine at all. * fix(build): link the registry addon as a workspace package, not file: As a `file:` dependency pnpm re-resolved and re-linked the package on every install, including `--frozen-lockfile` (measured: "added 1" on a repeat no-op install). That virtual-store churn ran concurrently with node-gyp reading the same tree and cost @vscode/windows-process-tree its binding.gyp mid-rebuild, failing package (windows) whenever the native cache hit and only that module needed building. The linux packaging job hit the same race from the other side, as a pnpm staging move failure. A workspace link resolves once and leaves the store alone; repeat installs are now 55ms no-ops. native/windows-registry is listed explicitly so `packages:` still does not auto-discover mobile/. * fix(build): stop tracking node-gyp output for the vendored addon The build/ tree is generated per host and ABI; the committed copy was macOS-specific gyp scaffolding from a local build and would have shipped stale Makefiles to every checkout. * chore: ignore the vendored addon's node-gyp bin output too node-gyp also emits bin/<platform>-<abi>/ beside build/; both are per-host generated output that must never be committed.	2026-09-12 20:10:49 -07:00
Neil	2d1bd1eb48	perf: bound whitespace normalization for tool previews (#20332)	2026-09-12 19:40:31 -07:00
Neil	7b0701aefa	chore: remove 20.7 MiB of duplicate and unused media (#20416) ... * chore: remove duplicate and unused documentation media * chore: guard README local links and refresh tile-01 vendor metadata - Add config/scripts/check-readme-local-links.mjs: every local src/srcset/href in README.md and docs/readme/*.md must resolve to a tracked file. Runs in the ungated root_directory_guard job so docs-only diffs (which skip static_analysis) still catch a deleted docs-site or feature-wall asset the README embeds. - Refresh tile-01.recorded-at.json to what vendor-feature-wall-assets.mjs now emits for the tab-split source path. - Drop the pr-19217 evidence prose that cited the removed screenshots. * fix: accept single-quoted attributes in README local link check The parser only matched double-quoted src/srcset/href, so <img src='missing.gif'> was skipped and the guard passed a README that GitHub renders with a broken image. Regression test fails without the parser change.	2026-09-12 19:40:18 -07:00
Neil	fccc887037	perf: skip impossible inline HTML comment matches during encoding (#20293)	2026-09-12 19:40:10 -07:00
Neil	7a440b1c85	perf(mobile): skip successful duplicate connection log saves (#20252)	2026-09-12 19:39:55 -07:00
3 people	35a5259ccd	perf(android): queue fragmented scrcpy video packets (#20230) ... * perf(android): queue fragmented scrcpy video packets * fix(android): release consumed scrcpy chunk storage * fix(android): bound queued scrcpy fragment count - Coalesce pending video fragments once more than MAX_PENDING_CHUNKS (1024) are queued, so a large frame delivered in tiny socket chunks cannot retain millions of Buffer objects below the 16 MiB byte guard. - Add a regression test feeding a 256 KiB frame one byte at a time and asserting the retained fragment count stays bounded. --------- Co-authored-by: m4air <m4air@Mac.localdomain> Co-authored-by: Neil <neil@stably.ai>	2026-09-12 19:39:40 -07:00