mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
debug-github-rate-limit
93
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  
|
921882619e |
fix: retire closed editor models from the app shell (#21178)
* fix: retire closed editor models from the app shell * test(editor): use checked Monaco attachment calls * Preserve bounded editor view caches when retiring closed models * docs(editor): describe batched model retirement * fix(editor): preserve cleanup work across registry replacement * fix(editor): build editor model URIs with the file scheme Monaco keys its model registry by `uri.toString()`, and both `@monaco-editor/react` (via the `path` prop) and the closed-tab disposal path built that key with `Uri.parse`. On Windows a raw path such as `C:\repo\a.ts` parses as scheme `c`, which fails the scheme gate in `modelService._schemaShouldMaintainUndoRedoElements`, so closed-file undo history was dropped for every file at any size — not only the large files the tradeoff note covers. Add `toEditorModelUri`, the one filesystem-path -> model-key function, built on `Uri.file` so the result always carries the `file:` scheme and re-parses to itself. Route model creation, disposal lookup and the still-open ownership comparison through it so all three agree; a divergence there would dispose a model an open editor is still editing. --------- Co-authored-by: m4air <m4air@Mac.localdomain> Co-authored-by: Neil <neil@stably.ai> |
||
|
|
89acf1e1fa |
fix(plugins): release diagnostic logs after successful uninstall (#21185)
* fix(plugins): retire log owners after successful uninstall * fix: address memory PR review regressions and withdraw false positives * fix(plugins): fence stale activation after uninstall * chore: allow durable plugin uninstall audit evidence --------- Co-authored-by: m4air <m4air@Mac.localdomain> Co-authored-by: Neil <neil@stably.ai> |
||
|
OrcaWin
|
7063c2cbdd |
fix: read crash diagnostics without loading whole dumps into memory
Read crash diagnostics incrementally to avoid whole-dump memory spikes. |
||
 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. |
||
|
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.
|
||
|
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. |
||
 Jinwoo Hong
|
1f7655f3e3 |
feat(ai-vault-search): public session search contract and transports (#20277)
* feat(ai-vault-search): define public contract and service seam * feat(ai-vault-search): add IPC runtime relay and web transports * fix(ai-vault-search): register search IPC at the core handler site ai-vault.ts was two lines over the 300-line max-lines limit; the search handlers belong with the other register*Handlers calls anyway. * fix(ai-vault-search): withhold degraded-root paths from relay status Status carried local filesystem paths over the relay while hits redact theirs. redactStatusForTransport applies the same policy at the same boundary: relay callers keep each root's reason and the array length as the count, so the type only makes root optional. * fix(ai-vault-search): close diagnostic path leak and remove test casts * feat(ai-vault-search): carry an execution host id and per-host outcomes on hits * feat(ai-vault-search): route desktop search by execution host scope, including runtimes * feat(preload): accept an execution host scope on session search * feat(web): answer only for the paired runtime on session search * docs(ai-vault-search): describe execution-host routing and the all-hosts merge * test(ai-vault-search): cover every host scope, the all-hosts merge and wire compat * fix(ai-vault-search): resume every host mid-page so a merged page never drops a hit * fix(ai-vault-search): decode the merged cursor with a schema instead of casts CI's type-aware audit refuses type assertions; a zod record validates the per-host entries and yields the typed map without one. * refactor(ai-vault-search): defer cross-host merged search |
||
|
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. |
||
|
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. |
||
|
Jinwoo Hong
|
fa3d29fa11 |
feat(ai-vault-search): session search query engine over the index (#20029)
* feat(ai-vault-search): give the index a generation a page cursor can be fenced by
A search page is a slice of one ranked list, so a cursor only means anything
against the snapshot that produced it. The store now keeps a monotone
generation in `meta`, bumped by every mutation that can change which rows a
read returns, and starts a new one on open so a write whose bump never landed
cannot leave a cursor pointing at content that is already gone.
Schema version 2 adds the two tables the query layer needs: `messages_vocab`
(fts5vocab over messages_fts, the typo repair's whole dictionary) and
`search_log`. Both are pure additions and the index is a cache, so a version-1
file is dropped and rebuilt exactly as any other mismatch is.
* feat(ai-vault-search): plan a query the way the index tokenized it
The planner unfolds the tokenizer contract instead of asking SQLite: same
boundaries as `unicode61 tokenchars '_.-/+'`, pinned against real fts5vocab
output so a query can be planned without a round trip. It decides the route
ladder's first rung (literal shape), strips stop words from prose but never
from a literal, and fans an identifier out into its pieces for the OR fallback.
Typo repair uses the index's own vocabulary as its dictionary, so it can never
suggest a term the index does not hold. Its exact-match probe now joins
`visible_messages`: `messages_vocab` is a view over the FTS b-tree and still
lists staged and tombstoned terms, and the PR 2 read ratchet is right to
demand the join.
`splitAiVaultSearchQuery` is the index's reading of repo: / path:. It keeps
operator case, which the panel folds and cwd_key must not; a census test pins
the two parsers to the same answer about what is an operator until PR 7 moves
the panel onto this one.
* feat(ai-vault-search): narrow a search the way the sidebar keys a folder
Every caller-supplied narrowing in one place, so retrieval, the operator-only
page and the session load cannot drift apart: agents, an updated-at floor, the
retention cutoff, scope paths, and the repo: / path: operators.
Scope keying goes through PR 2's `cwdKey`, which is the sidebar's
`folderGroupKey` without its prefix, rather than the original branch's second
spelling. That drops the branch's WSL distro qualification, which PR 2 removed
on purpose, and it makes the filesystem root a key that already ends in a
separator, so the child-prefix range is built from the key rather than by
appending one; `//` sorts below every real child and would scope the root to
nothing.
Engine types land here too, under src/main and not src/shared: nothing in this
PR is a wire type, and PR 5 lifts what a caller may receive.
* feat(ai-vault-search): rank, page and answer a session search
`SessionSearchEngine.search()` over the PR 2 store: route ladder (phrase, AND,
typo repair, OR), BM25 weights per corpus, one hit per session, fork folding,
and a page.
- `scope` picks the corpus and the engine never second-guesses it.
`conversation` is user and assistant turns; `all` adds tool output and the
identifier shadow column. Switching corpus while typing is PR 7's policy; an
engine that widened on a miss would make a result impossible to reproduce
from its own request.
- Pagination is an offset into one ranked list, fenced by the index generation
and by a hash of everything that changes the ranking. A cursor from another
generation or another query is refused with a typed error rather than
silently re-run. Ranking breaks every tie by session id, because a cursor
indexes into that order and retrieval does not promise one.
- Snippets and source presence are paid for by the page, not the list. A
snippet past the per-hit ceiling is cut on a code point, never between `[[`
and its `]]`, and flagged on the hit.
- Source presence is read from the `files` table. No stat on the query path,
and no `missing`: only a proven deletion may claim one, and this read cannot
prove it.
- `SESSION_SEARCH_CANDIDATE_LIMIT_DEFAULT` is an option, not a constant, and
the result says when the limit was what cut the answer.
- The engine is the only caller of `store.warm()`, on first search.
Library only: no IPC, no settings, no Electron, nothing constructs it in
production.
* perf(ai-vault-search): measure what a query costs and what the candidate limit buys
Two corpora, because they answer different questions. The 10.5 MB / 40-session
corpus is what the scope split costs a reader: conversation is about 1.6x
faster at p50 and 3.4x at p95 than the full corpus, which is the argument for
the second FTS table being the one a keystroke can afford.
The candidate limit needs more sessions than the limit before it costs
anything, so it is swept over 2,500 one-turn transcripts with every session
matching. Limits are interleaved sample by sample: run back to back, the first
configuration pays for every page the OS cache had not seen and the ordering
alone moved p95 further than the limit did.
The doc says plainly what these numbers do not cover. They are cost, not
relevance; the MRR figures quoted beside the BM25 weights and the identifier
shadow column come from a shoot-out over real transcripts and cannot be
reproduced from this repository.
* refactor(ai-vault-search): key a page once per search, and report reachable pages honestly
The page key was hashed twice per search, once to decode the incoming cursor
and once to mint the outgoing one. The benchmark's reachable-page figure was
clamped against a constant that could never bind; it is the limit over the page
size and nothing else.
* test(ai-vault-search): pin the two engine seams nothing was holding
The warm wiring and the query-length cap were both written and neither was
observable. A spy pins that a search is what warms the store, and the cap is
pinned through the one input the planner's own term limit does not already
bound: a single enormous token, where the cut is what decides whether the term
matches the indexed one at all.
* fix(ai-vault-search): fence the generation against writers this process cannot see
The generation was cached in memory and moved only on this store's own writes,
and the bump itself was a read-then-write outside any transaction. Two handles
on one file is the normal case once PR 3 lands: the indexer writes in the
scanner child while an engine reads elsewhere. A reader would see that writer's
deletions while its own generation stood still, honour a stale cursor, and skip
a session; two writers could mint one generation for two snapshots.
`generation` now reads `meta` on every call, and the bump is a single
`ON CONFLICT DO UPDATE ... + 1` inside the transaction that makes the change.
That closes the crash hole the bump-on-open existed for, so opening a store no
longer invalidates anyone's cursor.
Only a change to what a read returns counts. Retiring a path the index never
held hides nothing, and the row deletes that drain a tombstone take away rows
that were already invisible: bumping there would refuse a cursor every 256 rows
and leave pagination unusable for as long as indexing ran.
Also drops redaction from the query log, following PR 2's decision to store
transcript content as written; the module it called no longer exists.
* fix(ai-vault-search): answer from a version-1 index, and keep a repaired literal whole
Two smaller findings.
An engine can be handed a connection to a version-1 file that another handle is
still answering from, and this PR is what first makes that reachable. It now
probes once for the two tables version 2 added and names what it cannot serve
on every result, instead of throwing at the first query that reaches for a
vocabulary that is not there. The route ladder simply skips its repair rung.
Which process may unlink and rebuild the index is PR 3b's decision and is not
solved here.
Typo repair re-planned the corrected query from scratch, and a corrected
spelling can read as prose even when what was typed was a literal:
`parseJsonn(the, data)` has the punctuation, `parsejson the data` does not, so
the re-plan dropped `the` as a stop word. The repaired query searched for less
than was asked and `repairedTerms` reported a body nobody typed. The re-plan is
now told what the original decided, because a repair changes spellings, not the
query's character.
`repairedTerms` is documented as the whole body the repaired plan ran, with the
index's own case-folded spelling for the terms it corrected.
* fix(ai-vault-search): make repo: and path: mean one thing in the list and the index
The engine said `repo:` and `path:` a second time, in SQL, and SQL cannot say
them. LIKE folds ASCII and nothing else, so `path:CAFÉ` missed `café`; the
engine searched `cwd_key` while the panel searches the working directory and
the transcript path, so `path:jsonl` matched every session in the panel and
none in the index; and the engine compared one path segment where the panel
compares the last two, so `repo:orca/session-search` missed. All four
reproduce in both directions.
So there is one definition now, not two that resemble each other.
`matchesAiVaultQueryOperators` moves into the shared filter module beside the
panel that already owned the semantics, the panel calls it, and the engine
applies it over the rows it retrieved. SQL keeps only what it can express
exactly: the `cwd_key` prefix range for `scopePaths`.
`parseVaultQuery` now parses through `splitAiVaultSearchQuery`, so one parser
decides what an operator is. Its existing tests pass unchanged; four degenerate
shapes do answer differently and are pinned as decisions rather than left to be
discovered.
Two consequences worth stating. The operators are conjunctive now, because that
is what the panel has always done, where the engine had been ORing within a
key. And the operator-only page walks newest sessions in bounded pages applying
the predicate, rather than taking one cut of the newest N and filtering it,
which would have answered `repo:x` with nothing on a busy index.
* docs(ai-vault-search): re-measure the query benchmark after the operator change
repo: and path: moved out of SQL, so the operator-only row measures something
different now and the note that it is a range seek was wrong. The rest of the
table is re-measured on an idle machine: the previous run's p95 column was
mostly contention, which is why conversation looked 3.4x faster at p95 rather
than the 1.7x it actually is.
Adds what this PR does not settle: which process may unlink and rebuild the
index is PR 3b's, and PR 4 is only the first thing that makes reading it
reachable.
* fix(ai-vault-search): stop reporting a search that gave up as a search that finished
The operator-only walk stops at a scan ceiling as well as at a full candidate
set, and only the first of those reached the result. A query whose one match
sat past the ceiling came back with no hits and truncated.candidates false,
which is the engine claiming there is nothing to find when what happened is
that it stopped looking. Retrieval now says why it stopped, because it is the
only layer that knows, and the count it used to return could not distinguish
the two cases.
The cursor fence stays as it is: any published read moves the generation, so an
outstanding cursor is refused, and that is what F11 asked for. What was wrong
was the claim next to the row-delete skip that pagination stays usable through
indexing. It does not, and the engine now says so. The rejection carries the
generation the cursor was minted in and the one the index is at, so a caller
can tell a moved index from a bad cursor and re-issue page one without showing
anyone an error.
The capability probe was nearly dead code, since every store opens through a
function that rebuilds a stale file. It is not dead, because two handles can be
open on one file, so the claim is corrected rather than the probe deleted. It
now runs per search: a verdict cached in the constructor is wrong in both
directions once another handle rebuilds the index.
Also says why the row-delete loop may skip the bump: those messages keep
batch_id NULL and stay in visible_messages, so what makes them unreachable is
their session's tombstone, and the read ratchet is what keeps every reader
joining the view that applies it.
* fix(ai-vault-search): restore the panel's reading of a quote that does not end a word
Unifying the two parsers changed panel behaviour on nine of twenty probed
shapes, not the three previously pinned. Six of the nine were regressions, all
from one rule: the shared parser refused a quoted span whose closing quote was
not followed by a space, so `"a b"c` and `repo:"a"b` became single terms
carrying their own quote characters, which match nothing.
The rule was justified as what stops the apostrophes in `it's a repo:orca
thing's` from swallowing the operator between them. It is not: a span only ever
opens at a token start, and the quote in `it's` is not at one. Dropping the
rule restores all six shapes to what the panel has always done and leaves that
protection intact.
Three changes remain and are kept because the old answer was worse in each: an
operator with an empty quoted value is dropped rather than filtering on `""`
and silently emptying the list, and a bare pair of quotes reads as an empty
term rather than as the two characters. Each is pinned with a test that says
which behaviour it is and why.
* fix(ai-vault-search): trim operator values, and report a query the engine had to cut
Three lows.
`repo:" "` survived as a term and matched no label, silently emptying the
list, which is the exact defect the empty-value drop exists to prevent wearing
different clothes. Operator values are trimmed, and a whitespace-only one drops
like an empty one. The substring matcher's copy of a free-text term is trimmed
too, so `" "` reads as the empty term already does; the span kept for FTS is
still the query verbatim.
Two caps upstream of retrieval fired silently: the planner searches at most 48
terms, and the engine cuts the query at 512 characters. A 56-term query whose
only match was the 56th came back with no hits and nothing truncated, which
claims there is nothing to find. `truncated.query` now says when either fired,
alongside the candidate and snippet flags that already did.
Every cursor refusal now carries the generation the index is at, which the
engine knows before it looks at the cursor, and the generation the cursor
claimed wherever that survived parsing. The doc says exactly when each is
present instead of leaving absence unexplained.
* refactor(ai-vault-search): read the tables the simplified index writes, and own the fence
PR 2 deleted the visibility views, the staging tables and the store's
generation, so this reads `sessions` and `messages` directly and carries the
three schema objects only a query needs — the vocabulary, the query log, and
the triggers that move the generation — as its own extension over the store's
schema.
The fence is now three triggers on `files`, because every transaction the store
opens that can change what a search returns writes that table and nothing else
does; retention's orphan drain is the one write path that touches neither, and
it is the one that must not bump. The triggers live in the file, so a writer in
another process moves the generation without knowing a reader exists.
A message row can now outlive its session row until the drain reaches it, so
the snippet read joins `sessions` and the typo repair asks for a live posting
instead of trusting the vocabulary's document count.
The engine takes a connection rather than a store: PR 2's store keeps its
connection private, and which process may open or rebuild the index file is
PR 3b's decision, not a query engine's.
* perf(ai-vault-search): price the second FTS table, and re-measure without warmup
Open decision 3. A column filter over `messages_fts` returns the identical
rowid set as `conversation_fts` — checked here per query rather than assumed —
so the table exists for latency alone. On a 105 MB corpus at both ends of the
tool-output band, the column-filtered form costs 1.16-1.42x at p95, against a
bar of 2x, so the recommendation is to delete it.
The shoot-out writes its own corpus because the answer turns on the one
property the shared generator fixes: how much of a transcript is tool output.
Half the tokens in that output are words the conversation also uses, which is
deliberately generous to the table under question.
The doc records the number that argues the other way. PR 2 priced the table at
about a quarter of the index on a corpus whose tool output is 56% of its
message text; on a tool-heavy one it is 6.7-11%, because `messages_fts` grows
with the tool text and the second table does not.
Page warmup is not re-added. The measurement behind it was on a 4 GB index,
removing it moves this corpus by less than the run-to-run spread, and a
cancellable background pass needs a lifecycle a query library does not have.
* test(ai-vault-search): pin that an append moves the generation a cursor is fenced by
* refactor(ai-vault-search): answer the conversation scope with a column filter
PR 2 deleted `conversation_fts` on the strength of this PR's shoot-out, so the
scope is a column filter over the one FTS table now. `ftsTableFor` is gone; a
scope is a pair of `scopedExpression` and `scopedWeights`, and the table name
no longer travels through the engine, the snippet builder or a hit.
The filter is parenthesised, and that is the whole of it: `{cols}: (a AND b)`
binds both terms, while `{cols}: a AND b` binds only the first and searches
tool output for the rest. A test drives an AND whose second term lives only in
tool output through both scopes.
The snippet keeps one guard, not two. Its column list and its expression were
each hiding the other's mistakes — a tool-only row was unreachable through
either — so the list is the same four columns for every scope and the scoped
expression is what makes a conversation snippet impossible to draw out of tool
output. Dropping it now leaks that row, which a test catches.
One behaviour the deleted table did not have, pinned rather than wished away:
bm25 normalises by the whole row's length and has no per-column length, so two
rows with identical prose score differently when one also holds tool output.
The rowid set is unchanged; the order within it can move.
Re-measured on the shipping schema. The conversation scope is 1.2-1.4x faster
than `all` at every rung, and the index is 57 MB rather than about 150 MB at
93% tool output, because a tool row is now capped at 3,072 characters.
* fix(ai-vault-search): repair a spelling inside the scope that will answer it
Typo repair read `messages_vocab` and probed `messages_fts` with no column
filter, so tool output decided whether a conversation-scoped query was
repaired, in both directions. A tool row carrying the misspelling made the
query look correctly spelled and suppressed the repair; a tool row carrying a
rare word became the suggestion, naming in `repairedTerms` a string from a
column the scope will never show. Both reproduced against a control index that
differs by exactly that one row.
The vocabulary proposes and a scoped count disposes. fts5vocab is per table and
cannot be column-filtered, so every decision that reaches the plan — already
spelled right, eligible, and which of two equally close candidates wins — now
comes from a `messages_fts MATCH` under the same filter retrieval uses, joined
to `sessions`.
That also takes the vocabulary's `doc` out of the ranking, which is the half of
the drain defect that belongs here: `doc` counts rows whose session a purge has
already cut loose, so reclaiming them changed which word a query was repaired
to. Candidates are ordered by term now, because the ordering decides which of
them survive the scan limit, and ties on similarity go to the more common word
counted live rather than to the vocabulary's number.
The cost is one bounded count per candidate examined, at most eight per prefix,
and only for a term the scope has no posting for at all.
* fix(ai-vault-search): fence the rows a purge reclaims after it cuts a session loose
Retention's second half deletes from `messages` alone and touched neither
`files` nor `sessions`, so it moved no generation. The argument was that those
rows answer nothing, which was true of retrieval and not of the engine: the
typo repair's dictionary is a view over the FTS b-tree and listed them, so a
drain running between two pages swapped the repair under a cursor that was
still honoured, and a search that had answered stopped answering.
The commit before this one fixes that at its source by counting live rows. It
does not make the drain provably inert — the vocabulary still decides which
candidates survive its scan limit, and reclaiming a term's last row moves where
that limit cuts — so the fence is what covers the rest.
A fourth trigger, on `messages`, with a `WHEN` clause that is the whole reason
it is affordable: a replace and a `removeFile` delete a session's rows while
its `sessions` row still stands, so neither fires, and both already bump
through `files`. Only the drain deletes a row whose session is gone.
The price is named rather than avoided: a cursor outstanding while a purge runs
is now refused once per batch, which `SessionSearchCursorError` reports as
`stale-generation` so a caller re-issues page one. The test that pinned the old
contract is replaced by one for the new one, and by one proving a replace still
does not fire it.
* fix(ai-vault-search): tell a highlight from a transcript that contains brackets
The snippet builder asked each of a row's four columns for a marked snippet and
showed the first whose text contained `[[`. Transcripts contain `[[`: a bash
`if [[ -f … ]]`, numpy's `[[1, 2], [3, 4]]`. A row matching only in tool output
was shown its user turn instead, with nothing highlighted in it, and the
any-column fallback an identifier-only match depends on was unreachable behind
the same collision.
Whether a column matched is now the difference between two renderings of the
same text: `snippet(…, MARK, MARK, …)` beside `snippet(…, '', '', …)`. Content
cannot forge a difference between those two, because it is the same content
either way.
The marks FTS5 inserts are private-use code points, rewritten to the public
`[[` and `]]` once, at the end. That is for the other decision that has to tell
a mark from content: the truncation refuses to cut between an open mark and its
close, and a transcript's own bracket used to move that cut.
* fix(ai-vault-search): cut a query on a code point and bind ids in batches
Two small ones from the review's not-routed list.
`query.slice(0, 512)` can land between the halves of a surrogate pair, leaving
a lone half that matches nothing and that a caller cannot echo back. The reader
already has `sliceAtCodeUnitLimit` for exactly this.
`loadSessions` bound one parameter per candidate id in a single statement. The
list is as long as the candidate limit, the tuning doc invites a host to raise
that limit, and SQLite's `SQLITE_MAX_VARIABLE_NUMBER` is 999 on builds older
than 3.32 — so one settings change away from `too many SQL variables`. Read in
batches of 500, leaving room for the filter's own bound values.
* docs(ai-vault-search): price the repair rung, and record what is left open
Typo repair is the one rung whose cost tracks the vocabulary rather than the
result, and it only runs for a term the scope has no posting for. Measured over
1.6 M distinct terms: 10 ms for one unknown term, 387 ms for a 480-character
query of thirty-nine of them.
The scoped-count fix made that cheaper rather than dearer, from 737 ms, because
ordering the vocabulary scan by term drops the sort `doc DESC` needed and the
counts it adds are at most eight bounded probes per prefix. A cap on unknown
terms per query is a follow-up in the split plan, with the five other items the
final review raised and did not route.
* test(ai-vault-search): make each snippet mark mechanism answer for itself
Two mechanisms landed together and hid each other: choosing a column by
comparing a marked rendering against an unmarked one, and marking with
private-use code points instead of `[[`. Either alone fixed the bracket repro,
so neither had a mutation against it — the same masking the snippet's two
column guards had a round ago.
They do different jobs, so both stay and each gets the test that needs it. A
transcript holding a private-use code point of its own is what the comparison
is for; agent output carries Nerd Font glyphs from that block. A snippet past
the character ceiling with a bracket after its last real mark is what the
private-use marks are for, because the truncation has to find that mark by
searching the text.
The two one-line fixes get honest framing rather than a mutation neither can
have. A lone surrogate is not a token character, so the planner drops it either
way and the safe cut is hygiene. And no SQLite this stack runs refuses 1,100
bound ids — 32,766 has been the floor since 3.32 — so the batch is about
owning the ceiling here rather than rescuing a reachable failure.
* refactor(ai-vault-search): keep the scope's expression with the other expressions
Making the typo repair ask its questions in the search's own scope put an
import from retrieval into it, and retrieval already owns the repair — a cycle
the native audit catches. `scopedExpression` belongs beside `phraseExpression`,
`andExpression` and `orExpression` anyway: it builds a MATCH expression, and
two callers now need it. The bm25 weights stay in retrieval, where the SQL that
uses them is.
* docs(ai-vault-search): say which delete paths fire the orphan-reclaim trigger after a replace cuts loose
* test(ai-vault-search): make the trigger-restore test exercise the trigger it drops
* fix(ai-vault-search): keep a scope nothing could key from widening the search
* fix(ai-vault-search): read a fractional or negative cursor generation as malformed
* fix(ai-vault-search): highlight only the marks FTS5 inserted, not the text's own
* test(ai-vault-search): compare the whole query, so a quoted operator value survives
* fix(ai-vault-search): filter routes and fence page reads; simplify query engine
* refactor(ai-vault-search): narrow retrieval API and clarify page rejection
|
||
|
Neil
|
6bb2b0c6d7 |
test(runtime): capture real Antigravity transcripts — the detector is inverted on live output (#19983)
* test(runtime): capture real agent PTY transcripts before rewriting Antigravity readiness
Antigravity readiness has been written five times against a five-line screen
typed from memory. There is no Antigravity transcript in this repository, so
every attempt was a guess tested against another guess. This adds the recorder,
the protocol and the fixture-driven suite so the sixth attempt can be written
against evidence, and changes no detector logic.
- config/scripts/capture-agent-pty-transcript.mjs records a live agent session
through a real PTY, escapes and wrapping intact. Ctrl-] is consumed by the
recorder and never forwarded, which is the only way to end a capture while a
dialog still owns the screen.
- config/scripts/pty-transcript-secret-scan.mjs finds account identifiers and
credentials, redacts them with same-length placeholders so wrapping survives,
and recognises its own placeholders so a scrubbed file verifies clean.
- src/main/runtime/antigravity-readiness-transcripts.test.ts asserts a verdict
per transcript and skips by name until the transcripts land, with a
doc-coverage ratchet and a guard that a fixture contains escape bytes.
The escape-byte guard exists because the three cursor-agent fixtures carry a
comment claiming they were captured verbatim through Orca, yet contain zero ESC
bytes and zero carriage returns. That comment is corrected here to say what
those files are; the fixtures and the rules built on them are untouched.
* test(runtime): capture real Antigravity transcripts, and pin what they prove
`agy` 1.1.25 turned out to be installed, so the transcripts this scaffold was
built for now exist. Six are recorded from live sessions and committed; the
rest are named as skipped, because reaching them would mean signing the
operator out or deleting their config.
The captures invert the story. On real output the shipped detector refuses a
genuinely ready screen and accepts a live `/model` picker:
- Antigravity paints a block-glyph logo down the left, so the model row never
starts a line. `startsWith('gemini', trimmedStart)` cannot match a real ready
screen, on any account or model. Stripping the logo flips the same screen to
ready, which means a decorative glyph decides readiness today.
- The `/model` picker prints `Gemini 3.x Flash` one per line, at line start, and
a bare `>` composer sits earlier in the tail. Both halves of the rule are
satisfied while a dialog owns the screen.
- For an API-key user the identity row reads `Gemini API key` — no `@`, no
domain — and `AGY_CLI_HIDE_ACCOUNT_INFO=1` removes the row entirely. The
account-row requirement of attempts 4 and 5 can never pass for those users.
- The banner is printed once and never reprinted after a dialog is dismissed, so
`headerIndex` cannot be the ordering anchor.
Four suite cases are pinned as KNOWN DEFECT: they assert what the detector does
so CI stays honest instead of permanently red, and flip to failing the moment
someone fixes it. No detector logic changed.
The recorder gains `--send "<ms>:<text>"` because a dialog capture has to be
driven and an unattended run has no TTY, and the scrub scanner gains a UUID rule
because agy prints a resumable conversation id on exit.
* test(runtime): capture agy mid-turn, and make the scan file reviewable
Answers the busy-frame question a P1 review raised against attempt six, with
two new captures from a live turn.
At the frame level the review is right: a busy frame parks the caret with the
same bytes as an idle one, `CR ESC[2A ESC[2C`, and the only differing row —
`esc to cancel` versus `? for shortcuts` — is erased by that park.
At the retained-tail level it does not reproduce. Each spinner tick is its own
repaint with its own `CR ESC[2A`, two rows higher than the frame's, which
splices the composer away: a live turn's tail ends on `⣟ Generating...`, with
no bare caret to match. A constructed input that keeps the park and edits only
the status text is not faithful, because a live turn has a spinner row
repainting below the composer.
The residual is the gap between a frame park and the next tick, where the tail
does end on the bare caret. Quiescence-gated paths are safe there because ticks
keep arriving; text-only paths are not, and for those the capture supports one
clause: a braille glyph on the last visible line means working. That predicate
already exists here for cursor-agent and should be reused, scoped to the last
line — a first-run transcript prints `⠾ Signing in...` during startup.
Also in this commit, from the same review:
- pty-transcript-secret-scan.mjs held raw 0x00-0x1f bytes in a character class,
so the one file gating real PTY data into history was binary to git and
unreviewable in a diff. It now tests codepoints, which the formatter cannot
fold back into control bytes.
- Pin `src/main/runtime/__fixtures__/*.txt` as -text. A Windows checkout would
otherwise normalise line endings and rewrite the CR bytes that make these
files evidence.
The recorder now stops appending at the stop moment rather than through
shutdown: an agent repaints an idle frame on its way out, which was overwriting
the mid-turn state the capture existed to record.
* test(tooling): allowlist the transcript scan test in the batch-shim ratchet
pty-transcript-secret-scan.test.mjs asserts that the capture recorder routes
an 'agy.cmd' shim through cmd.exe, so the shim literal it names is the
assertion, not a spawn. Fits the existing assert-on-shim-files category.
|
||
|
Jinwoo Hong
|
f31bfa8fb7 |
Revert "feat(ai-vault-search): session search query engine over the index (#19750)" (#20023)
This reverts commit
|
||
|
Jinwoo Hong
|
cf7408a37f |
feat(ai-vault-search): session search query engine over the index (#19750)
* feat(ai-vault-search): give the index a generation a page cursor can be fenced by
A search page is a slice of one ranked list, so a cursor only means anything
against the snapshot that produced it. The store now keeps a monotone
generation in `meta`, bumped by every mutation that can change which rows a
read returns, and starts a new one on open so a write whose bump never landed
cannot leave a cursor pointing at content that is already gone.
Schema version 2 adds the two tables the query layer needs: `messages_vocab`
(fts5vocab over messages_fts, the typo repair's whole dictionary) and
`search_log`. Both are pure additions and the index is a cache, so a version-1
file is dropped and rebuilt exactly as any other mismatch is.
* feat(ai-vault-search): plan a query the way the index tokenized it
The planner unfolds the tokenizer contract instead of asking SQLite: same
boundaries as `unicode61 tokenchars '_.-/+'`, pinned against real fts5vocab
output so a query can be planned without a round trip. It decides the route
ladder's first rung (literal shape), strips stop words from prose but never
from a literal, and fans an identifier out into its pieces for the OR fallback.
Typo repair uses the index's own vocabulary as its dictionary, so it can never
suggest a term the index does not hold. Its exact-match probe now joins
`visible_messages`: `messages_vocab` is a view over the FTS b-tree and still
lists staged and tombstoned terms, and the PR 2 read ratchet is right to
demand the join.
`splitAiVaultSearchQuery` is the index's reading of repo: / path:. It keeps
operator case, which the panel folds and cwd_key must not; a census test pins
the two parsers to the same answer about what is an operator until PR 7 moves
the panel onto this one.
* feat(ai-vault-search): narrow a search the way the sidebar keys a folder
Every caller-supplied narrowing in one place, so retrieval, the operator-only
page and the session load cannot drift apart: agents, an updated-at floor, the
retention cutoff, scope paths, and the repo: / path: operators.
Scope keying goes through PR 2's `cwdKey`, which is the sidebar's
`folderGroupKey` without its prefix, rather than the original branch's second
spelling. That drops the branch's WSL distro qualification, which PR 2 removed
on purpose, and it makes the filesystem root a key that already ends in a
separator, so the child-prefix range is built from the key rather than by
appending one; `//` sorts below every real child and would scope the root to
nothing.
Engine types land here too, under src/main and not src/shared: nothing in this
PR is a wire type, and PR 5 lifts what a caller may receive.
* feat(ai-vault-search): rank, page and answer a session search
`SessionSearchEngine.search()` over the PR 2 store: route ladder (phrase, AND,
typo repair, OR), BM25 weights per corpus, one hit per session, fork folding,
and a page.
- `scope` picks the corpus and the engine never second-guesses it.
`conversation` is user and assistant turns; `all` adds tool output and the
identifier shadow column. Switching corpus while typing is PR 7's policy; an
engine that widened on a miss would make a result impossible to reproduce
from its own request.
- Pagination is an offset into one ranked list, fenced by the index generation
and by a hash of everything that changes the ranking. A cursor from another
generation or another query is refused with a typed error rather than
silently re-run. Ranking breaks every tie by session id, because a cursor
indexes into that order and retrieval does not promise one.
- Snippets and source presence are paid for by the page, not the list. A
snippet past the per-hit ceiling is cut on a code point, never between `[[`
and its `]]`, and flagged on the hit.
- Source presence is read from the `files` table. No stat on the query path,
and no `missing`: only a proven deletion may claim one, and this read cannot
prove it.
- `SESSION_SEARCH_CANDIDATE_LIMIT_DEFAULT` is an option, not a constant, and
the result says when the limit was what cut the answer.
- The engine is the only caller of `store.warm()`, on first search.
Library only: no IPC, no settings, no Electron, nothing constructs it in
production.
* perf(ai-vault-search): measure what a query costs and what the candidate limit buys
Two corpora, because they answer different questions. The 10.5 MB / 40-session
corpus is what the scope split costs a reader: conversation is about 1.6x
faster at p50 and 3.4x at p95 than the full corpus, which is the argument for
the second FTS table being the one a keystroke can afford.
The candidate limit needs more sessions than the limit before it costs
anything, so it is swept over 2,500 one-turn transcripts with every session
matching. Limits are interleaved sample by sample: run back to back, the first
configuration pays for every page the OS cache had not seen and the ordering
alone moved p95 further than the limit did.
The doc says plainly what these numbers do not cover. They are cost, not
relevance; the MRR figures quoted beside the BM25 weights and the identifier
shadow column come from a shoot-out over real transcripts and cannot be
reproduced from this repository.
* refactor(ai-vault-search): key a page once per search, and report reachable pages honestly
The page key was hashed twice per search, once to decode the incoming cursor
and once to mint the outgoing one. The benchmark's reachable-page figure was
clamped against a constant that could never bind; it is the limit over the page
size and nothing else.
* test(ai-vault-search): pin the two engine seams nothing was holding
The warm wiring and the query-length cap were both written and neither was
observable. A spy pins that a search is what warms the store, and the cap is
pinned through the one input the planner's own term limit does not already
bound: a single enormous token, where the cut is what decides whether the term
matches the indexed one at all.
* fix(ai-vault-search): fence the generation against writers this process cannot see
The generation was cached in memory and moved only on this store's own writes,
and the bump itself was a read-then-write outside any transaction. Two handles
on one file is the normal case once PR 3 lands: the indexer writes in the
scanner child while an engine reads elsewhere. A reader would see that writer's
deletions while its own generation stood still, honour a stale cursor, and skip
a session; two writers could mint one generation for two snapshots.
`generation` now reads `meta` on every call, and the bump is a single
`ON CONFLICT DO UPDATE ... + 1` inside the transaction that makes the change.
That closes the crash hole the bump-on-open existed for, so opening a store no
longer invalidates anyone's cursor.
Only a change to what a read returns counts. Retiring a path the index never
held hides nothing, and the row deletes that drain a tombstone take away rows
that were already invisible: bumping there would refuse a cursor every 256 rows
and leave pagination unusable for as long as indexing ran.
Also drops redaction from the query log, following PR 2's decision to store
transcript content as written; the module it called no longer exists.
* fix(ai-vault-search): answer from a version-1 index, and keep a repaired literal whole
Two smaller findings.
An engine can be handed a connection to a version-1 file that another handle is
still answering from, and this PR is what first makes that reachable. It now
probes once for the two tables version 2 added and names what it cannot serve
on every result, instead of throwing at the first query that reaches for a
vocabulary that is not there. The route ladder simply skips its repair rung.
Which process may unlink and rebuild the index is PR 3b's decision and is not
solved here.
Typo repair re-planned the corrected query from scratch, and a corrected
spelling can read as prose even when what was typed was a literal:
`parseJsonn(the, data)` has the punctuation, `parsejson the data` does not, so
the re-plan dropped `the` as a stop word. The repaired query searched for less
than was asked and `repairedTerms` reported a body nobody typed. The re-plan is
now told what the original decided, because a repair changes spellings, not the
query's character.
`repairedTerms` is documented as the whole body the repaired plan ran, with the
index's own case-folded spelling for the terms it corrected.
* fix(ai-vault-search): make repo: and path: mean one thing in the list and the index
The engine said `repo:` and `path:` a second time, in SQL, and SQL cannot say
them. LIKE folds ASCII and nothing else, so `path:CAFÉ` missed `café`; the
engine searched `cwd_key` while the panel searches the working directory and
the transcript path, so `path:jsonl` matched every session in the panel and
none in the index; and the engine compared one path segment where the panel
compares the last two, so `repo:orca/session-search` missed. All four
reproduce in both directions.
So there is one definition now, not two that resemble each other.
`matchesAiVaultQueryOperators` moves into the shared filter module beside the
panel that already owned the semantics, the panel calls it, and the engine
applies it over the rows it retrieved. SQL keeps only what it can express
exactly: the `cwd_key` prefix range for `scopePaths`.
`parseVaultQuery` now parses through `splitAiVaultSearchQuery`, so one parser
decides what an operator is. Its existing tests pass unchanged; four degenerate
shapes do answer differently and are pinned as decisions rather than left to be
discovered.
Two consequences worth stating. The operators are conjunctive now, because that
is what the panel has always done, where the engine had been ORing within a
key. And the operator-only page walks newest sessions in bounded pages applying
the predicate, rather than taking one cut of the newest N and filtering it,
which would have answered `repo:x` with nothing on a busy index.
* docs(ai-vault-search): re-measure the query benchmark after the operator change
repo: and path: moved out of SQL, so the operator-only row measures something
different now and the note that it is a range seek was wrong. The rest of the
table is re-measured on an idle machine: the previous run's p95 column was
mostly contention, which is why conversation looked 3.4x faster at p95 rather
than the 1.7x it actually is.
Adds what this PR does not settle: which process may unlink and rebuild the
index is PR 3b's, and PR 4 is only the first thing that makes reading it
reachable.
* fix(ai-vault-search): stop reporting a search that gave up as a search that finished
The operator-only walk stops at a scan ceiling as well as at a full candidate
set, and only the first of those reached the result. A query whose one match
sat past the ceiling came back with no hits and truncated.candidates false,
which is the engine claiming there is nothing to find when what happened is
that it stopped looking. Retrieval now says why it stopped, because it is the
only layer that knows, and the count it used to return could not distinguish
the two cases.
The cursor fence stays as it is: any published read moves the generation, so an
outstanding cursor is refused, and that is what F11 asked for. What was wrong
was the claim next to the row-delete skip that pagination stays usable through
indexing. It does not, and the engine now says so. The rejection carries the
generation the cursor was minted in and the one the index is at, so a caller
can tell a moved index from a bad cursor and re-issue page one without showing
anyone an error.
The capability probe was nearly dead code, since every store opens through a
function that rebuilds a stale file. It is not dead, because two handles can be
open on one file, so the claim is corrected rather than the probe deleted. It
now runs per search: a verdict cached in the constructor is wrong in both
directions once another handle rebuilds the index.
Also says why the row-delete loop may skip the bump: those messages keep
batch_id NULL and stay in visible_messages, so what makes them unreachable is
their session's tombstone, and the read ratchet is what keeps every reader
joining the view that applies it.
* fix(ai-vault-search): restore the panel's reading of a quote that does not end a word
Unifying the two parsers changed panel behaviour on nine of twenty probed
shapes, not the three previously pinned. Six of the nine were regressions, all
from one rule: the shared parser refused a quoted span whose closing quote was
not followed by a space, so `"a b"c` and `repo:"a"b` became single terms
carrying their own quote characters, which match nothing.
The rule was justified as what stops the apostrophes in `it's a repo:orca
thing's` from swallowing the operator between them. It is not: a span only ever
opens at a token start, and the quote in `it's` is not at one. Dropping the
rule restores all six shapes to what the panel has always done and leaves that
protection intact.
Three changes remain and are kept because the old answer was worse in each: an
operator with an empty quoted value is dropped rather than filtering on `""`
and silently emptying the list, and a bare pair of quotes reads as an empty
term rather than as the two characters. Each is pinned with a test that says
which behaviour it is and why.
* fix(ai-vault-search): trim operator values, and report a query the engine had to cut
Three lows.
`repo:" "` survived as a term and matched no label, silently emptying the
list, which is the exact defect the empty-value drop exists to prevent wearing
different clothes. Operator values are trimmed, and a whitespace-only one drops
like an empty one. The substring matcher's copy of a free-text term is trimmed
too, so `" "` reads as the empty term already does; the span kept for FTS is
still the query verbatim.
Two caps upstream of retrieval fired silently: the planner searches at most 48
terms, and the engine cuts the query at 512 characters. A 56-term query whose
only match was the 56th came back with no hits and nothing truncated, which
claims there is nothing to find. `truncated.query` now says when either fired,
alongside the candidate and snippet flags that already did.
Every cursor refusal now carries the generation the index is at, which the
engine knows before it looks at the cursor, and the generation the cursor
claimed wherever that survived parsing. The doc says exactly when each is
present instead of leaving absence unexplained.
* refactor(ai-vault-search): read the tables the simplified index writes, and own the fence
PR 2 deleted the visibility views, the staging tables and the store's
generation, so this reads `sessions` and `messages` directly and carries the
three schema objects only a query needs — the vocabulary, the query log, and
the triggers that move the generation — as its own extension over the store's
schema.
The fence is now three triggers on `files`, because every transaction the store
opens that can change what a search returns writes that table and nothing else
does; retention's orphan drain is the one write path that touches neither, and
it is the one that must not bump. The triggers live in the file, so a writer in
another process moves the generation without knowing a reader exists.
A message row can now outlive its session row until the drain reaches it, so
the snippet read joins `sessions` and the typo repair asks for a live posting
instead of trusting the vocabulary's document count.
The engine takes a connection rather than a store: PR 2's store keeps its
connection private, and which process may open or rebuild the index file is
PR 3b's decision, not a query engine's.
* perf(ai-vault-search): price the second FTS table, and re-measure without warmup
Open decision 3. A column filter over `messages_fts` returns the identical
rowid set as `conversation_fts` — checked here per query rather than assumed —
so the table exists for latency alone. On a 105 MB corpus at both ends of the
tool-output band, the column-filtered form costs 1.16-1.42x at p95, against a
bar of 2x, so the recommendation is to delete it.
The shoot-out writes its own corpus because the answer turns on the one
property the shared generator fixes: how much of a transcript is tool output.
Half the tokens in that output are words the conversation also uses, which is
deliberately generous to the table under question.
The doc records the number that argues the other way. PR 2 priced the table at
about a quarter of the index on a corpus whose tool output is 56% of its
message text; on a tool-heavy one it is 6.7-11%, because `messages_fts` grows
with the tool text and the second table does not.
Page warmup is not re-added. The measurement behind it was on a 4 GB index,
removing it moves this corpus by less than the run-to-run spread, and a
cancellable background pass needs a lifecycle a query library does not have.
* test(ai-vault-search): pin that an append moves the generation a cursor is fenced by
* refactor(ai-vault-search): answer the conversation scope with a column filter
PR 2 deleted `conversation_fts` on the strength of this PR's shoot-out, so the
scope is a column filter over the one FTS table now. `ftsTableFor` is gone; a
scope is a pair of `scopedExpression` and `scopedWeights`, and the table name
no longer travels through the engine, the snippet builder or a hit.
The filter is parenthesised, and that is the whole of it: `{cols}: (a AND b)`
binds both terms, while `{cols}: a AND b` binds only the first and searches
tool output for the rest. A test drives an AND whose second term lives only in
tool output through both scopes.
The snippet keeps one guard, not two. Its column list and its expression were
each hiding the other's mistakes — a tool-only row was unreachable through
either — so the list is the same four columns for every scope and the scoped
expression is what makes a conversation snippet impossible to draw out of tool
output. Dropping it now leaks that row, which a test catches.
One behaviour the deleted table did not have, pinned rather than wished away:
bm25 normalises by the whole row's length and has no per-column length, so two
rows with identical prose score differently when one also holds tool output.
The rowid set is unchanged; the order within it can move.
Re-measured on the shipping schema. The conversation scope is 1.2-1.4x faster
than `all` at every rung, and the index is 57 MB rather than about 150 MB at
93% tool output, because a tool row is now capped at 3,072 characters.
* fix(ai-vault-search): repair a spelling inside the scope that will answer it
Typo repair read `messages_vocab` and probed `messages_fts` with no column
filter, so tool output decided whether a conversation-scoped query was
repaired, in both directions. A tool row carrying the misspelling made the
query look correctly spelled and suppressed the repair; a tool row carrying a
rare word became the suggestion, naming in `repairedTerms` a string from a
column the scope will never show. Both reproduced against a control index that
differs by exactly that one row.
The vocabulary proposes and a scoped count disposes. fts5vocab is per table and
cannot be column-filtered, so every decision that reaches the plan — already
spelled right, eligible, and which of two equally close candidates wins — now
comes from a `messages_fts MATCH` under the same filter retrieval uses, joined
to `sessions`.
That also takes the vocabulary's `doc` out of the ranking, which is the half of
the drain defect that belongs here: `doc` counts rows whose session a purge has
already cut loose, so reclaiming them changed which word a query was repaired
to. Candidates are ordered by term now, because the ordering decides which of
them survive the scan limit, and ties on similarity go to the more common word
counted live rather than to the vocabulary's number.
The cost is one bounded count per candidate examined, at most eight per prefix,
and only for a term the scope has no posting for at all.
* fix(ai-vault-search): fence the rows a purge reclaims after it cuts a session loose
Retention's second half deletes from `messages` alone and touched neither
`files` nor `sessions`, so it moved no generation. The argument was that those
rows answer nothing, which was true of retrieval and not of the engine: the
typo repair's dictionary is a view over the FTS b-tree and listed them, so a
drain running between two pages swapped the repair under a cursor that was
still honoured, and a search that had answered stopped answering.
The commit before this one fixes that at its source by counting live rows. It
does not make the drain provably inert — the vocabulary still decides which
candidates survive its scan limit, and reclaiming a term's last row moves where
that limit cuts — so the fence is what covers the rest.
A fourth trigger, on `messages`, with a `WHEN` clause that is the whole reason
it is affordable: a replace and a `removeFile` delete a session's rows while
its `sessions` row still stands, so neither fires, and both already bump
through `files`. Only the drain deletes a row whose session is gone.
The price is named rather than avoided: a cursor outstanding while a purge runs
is now refused once per batch, which `SessionSearchCursorError` reports as
`stale-generation` so a caller re-issues page one. The test that pinned the old
contract is replaced by one for the new one, and by one proving a replace still
does not fire it.
* fix(ai-vault-search): tell a highlight from a transcript that contains brackets
The snippet builder asked each of a row's four columns for a marked snippet and
showed the first whose text contained `[[`. Transcripts contain `[[`: a bash
`if [[ -f … ]]`, numpy's `[[1, 2], [3, 4]]`. A row matching only in tool output
was shown its user turn instead, with nothing highlighted in it, and the
any-column fallback an identifier-only match depends on was unreachable behind
the same collision.
Whether a column matched is now the difference between two renderings of the
same text: `snippet(…, MARK, MARK, …)` beside `snippet(…, '', '', …)`. Content
cannot forge a difference between those two, because it is the same content
either way.
The marks FTS5 inserts are private-use code points, rewritten to the public
`[[` and `]]` once, at the end. That is for the other decision that has to tell
a mark from content: the truncation refuses to cut between an open mark and its
close, and a transcript's own bracket used to move that cut.
* fix(ai-vault-search): cut a query on a code point and bind ids in batches
Two small ones from the review's not-routed list.
`query.slice(0, 512)` can land between the halves of a surrogate pair, leaving
a lone half that matches nothing and that a caller cannot echo back. The reader
already has `sliceAtCodeUnitLimit` for exactly this.
`loadSessions` bound one parameter per candidate id in a single statement. The
list is as long as the candidate limit, the tuning doc invites a host to raise
that limit, and SQLite's `SQLITE_MAX_VARIABLE_NUMBER` is 999 on builds older
than 3.32 — so one settings change away from `too many SQL variables`. Read in
batches of 500, leaving room for the filter's own bound values.
* docs(ai-vault-search): price the repair rung, and record what is left open
Typo repair is the one rung whose cost tracks the vocabulary rather than the
result, and it only runs for a term the scope has no posting for. Measured over
1.6 M distinct terms: 10 ms for one unknown term, 387 ms for a 480-character
query of thirty-nine of them.
The scoped-count fix made that cheaper rather than dearer, from 737 ms, because
ordering the vocabulary scan by term drops the sort `doc DESC` needed and the
counts it adds are at most eight bounded probes per prefix. A cap on unknown
terms per query is a follow-up in the split plan, with the five other items the
final review raised and did not route.
* test(ai-vault-search): make each snippet mark mechanism answer for itself
Two mechanisms landed together and hid each other: choosing a column by
comparing a marked rendering against an unmarked one, and marking with
private-use code points instead of `[[`. Either alone fixed the bracket repro,
so neither had a mutation against it — the same masking the snippet's two
column guards had a round ago.
They do different jobs, so both stay and each gets the test that needs it. A
transcript holding a private-use code point of its own is what the comparison
is for; agent output carries Nerd Font glyphs from that block. A snippet past
the character ceiling with a bracket after its last real mark is what the
private-use marks are for, because the truncation has to find that mark by
searching the text.
The two one-line fixes get honest framing rather than a mutation neither can
have. A lone surrogate is not a token character, so the planner drops it either
way and the safe cut is hygiene. And no SQLite this stack runs refuses 1,100
bound ids — 32,766 has been the floor since 3.32 — so the batch is about
owning the ceiling here rather than rescuing a reachable failure.
* refactor(ai-vault-search): keep the scope's expression with the other expressions
Making the typo repair ask its questions in the search's own scope put an
import from retrieval into it, and retrieval already owns the repair — a cycle
the native audit catches. `scopedExpression` belongs beside `phraseExpression`,
`andExpression` and `orExpression` anyway: it builds a MATCH expression, and
two callers now need it. The bm25 weights stay in retrieval, where the SQL that
uses them is.
* docs(ai-vault-search): say which delete paths fire the orphan-reclaim trigger after a replace cuts loose
|
||
  Brennan BensonandMerge Sim
|
ebb1acfa37 |
refactor(agent-status): publish structured sessions into the hook server store (#19683)
* refactor(agent-status): publish structured sessions into the hook server store Structured (native chat) sessions have no PTY and no hook script, so their status never reached the hook server's store; #19217 gave `worktree ps` its own adapter over the structured feed instead. The feed now writes every projection into that store through a status sink the runtime wires, drops the row when the host closes the session, and `worktree ps` reads the one snapshot like every other agent. Rows carry a `structuredHost` marker and the journal clock; they are never persisted to last-status.json, and the main process does not forward them to the renderer yet, whose feed bridge still owns them until it is retired. Design and the two follow-ups: docs/reference/agent-status-store.md. * chore: drop stray @pnpm/exe lockfile entry An unrelated local pnpm run added @pnpm/exe as a packageManagerDependency with no package.json change, so CI's --frozen-lockfile install failed before any job ran. * docs(agent-status): describe the step that actually landed The design record claimed PR 1 deletes RuntimeAgentRowStore, drops the retained-versus-hook reconciliation, stamps terminalHandle on OSC rows, and tags rows with a source field of 'structured-host'. None of that is true of the shipped code: the retained store and its reconciliation are still in place, and the row field is structuredHost: 'held' | 'owned'. AGENTS.md points every future contributor here before they touch agent status, so split the roadmap into the 1a that landed and the 1b that has not, and name the fields the code actually writes. * fix(agent-status): pair session removal with the status-row forget A session dropped from the host's map without an explicit forget left its row in the store forever: `structuredHostOwned` bypasses the staleness check, so a failed re-attach (the Claude rewind path reaches one) stranded a permanently working agent in `worktree ps` and on mobile with no UI able to clear it. Deletion and forget are now one operation both callers route through. * fix(agent-status): give orcad the store worktree ps reads from `orcad` constructed its runtime with neither `getAgentStatusSnapshot` nor `structuredAgentStatusSink`, so once `worktree ps` sourced rows only from that snapshot the headless host published nowhere and listed nothing. The hook server's store is a module singleton whose import tree never reaches Electron, and its file paths come from `start()`, which orcad never calls. * fix(agent-status): drop a structured row without a renderer clear `dropStructuredStatus` went through `clearPaneState`, which fans a pane clear out to the renderer for a pane key the renderer's own feed bridge still writes - so 'exactly one writer per pane key' held for writes and not for deletes. `dropStatusEntry` routes through the status-drop tap instead, and skips the resume-identity remnant: a structured session has no pane to resume into, and every null-status publish would otherwise re-mint one. * test(agent-status): pin both half-migration structured-row filters Neither the `agentStatus:getSnapshot` filter nor the main-window listener's had a single assertion, so deleting either — the first step of PR 2 — was green everywhere. Also covers the perf skip and the drop's lack of a renderer clear. * docs(agent-status): correct three statements this PR made false The sink JSDoc claimed only tests construct a host without one; `orcad` did. The doc argued a structured row needs no tab mirror 'because headless serve has no renderer', reasoning about exactly the topology the wiring had not reached. The deleted runtime adapter's warning that the pane key must be the DERIVED one - never a bearer handle or minted worker key - was lost with it. * test(agent-status): declare orcad in the hook-row producer census Wiring the hook store into the orcad runtime added a production site that hands hook rows to a consumer, which the census ratchet pins deliberately. --------- Co-authored-by: Merge Sim <sim@local> |
||
|
Jinwoo Hong
|
d53cbed43f |
revert: hold mobile push feature for user testing (#19203)
Reverts
|
||
|
Jinwoo Hong
|
3160b54c69 |
feat: real background push notifications for the mobile app (#8129) (#18554)
* feat(cloud): add the mobile push gateway and its contract package (#8129) A small open-source service that holds the APNs key and FCM credentials and sends background push to paired phones on the desktop's behalf. Hosts authenticate with a box challenge and HMAC proof on their pairing key, the same shape the relay uses, so signed-in and accountless desktops share one path. Tokens are stored; alert text is held only for the coalescing window. The contract doc in docs/reference is the source of truth for every wire shape. The interop test runs the real desktop answerer against a real gateway-issued challenge so transcript drift fails in CI. * feat(push): register phones and send background push from the desktop (#8129) Adds the notifications.remote-push.v1 capability, the registerPush and unregisterPush RPCs on the mobile allowlist, a gateway client with a cached session and 401 re-auth, a durable unregister outbox, and a dispatcher that offers every mobile notification to the gateway after the socket fan-out. The dispatcher is fire-and-forget with one retry and drops registrations the gateway reports dead. Puts agentState on the mobile frame and fixes the #4375 wording so a working agent is never announced as finished. The relay host-proof code moves onto a shared envelope module with no behaviour change. * feat(mobile): background push registration, receive, and settings (#8129) Fetches the native APNs or FCM token, registers it with every paired host that advertises the capability, and re-registers on token change. Foreground pushes are suppressed inside handleNotification against the same seen set the socket path uses, so nothing shows twice. Taps route by host fingerprint. One Background notifications switch, off by default, with the disclaimer and needs-input / finished sub-switches; hidden until a paired desktop is new enough. Adds google-services.json and the expo-notifications plugin. * chore(cloud): Terraform and deploy workflow for the push gateway (#8129) Declares the Cloud Run service, runtime account, secrets, and orca_push database behind push_gateway_enabled, true only in production. The deploy workflow is gated like the relay's, deploys with no traffic, probes /ready and a validate-only FCM send, then shifts traffic. It runs as the shared production deploy account because the Cloud SQL rollout lease grant is foundation-owned; its extra authority is three bindings on the push service. docs/push-gateway.md carries the import commands for the resources created by hand and the APNs key rotation procedure. * docs: describe background notifications on the phone (#8129) * docs: check in the mobile push contract (#8129) Seven committed files cite it as the source of truth for every wire shape; docs/reference is allowlisted per file, so add the entry. * test(push): replay one checked-in host-proof vector on both sides (#8129) Cloud Verify installs only the cloud workspace, so the gateway suite cannot import the desktop answerer. Replace the cross-workspace import with a fixed challenge vector generated from the contract package; the gateway fixture and the desktop answerer each replay it and must produce the same HMAC. A transcript drift on either side now fails in that side's own suite. * fix(cloud): open the push gateway with invoker_iam_disabled, not an allUsers binding (#8129) The production domain-restricted-sharing policy rejects an allUsers run.invoker member, which the runbook anticipated. Opt the service out of invoker IAM the way the relay director already does; the host proof is the authentication either way. * docs(cloud): the push.onorca.dev record exists and is hand-managed (#8129) * fix(push): close review findings in the gateway (#8129) - Quota reservation takes a per-host advisory lock; READ COMMITTED admitted a whole burst past the cap (80/80 without, 60/80 with, against Postgres 16). - Challenge issuance no longer writes push_hosts; the row lands on proof verification. Stale hosts prune after 30 days. Per-IP token bucket on the two unauthenticated routes. - Streaming body limit via hono bodyLimit; a chunked body bypassed the Content-Length check. - registrationIds deduped in the schema; per-host device cap of 64; list bounded to its schema. - Gateway-side challenge TTL is the specified 10 s, not 40 s. - APNs stream settles on close as well as end/error. * fix(push): close review findings in the desktop client (#8129) - A gateway registration the registry cannot persist is enqueued for delete instead of leaking a live token. - Unregister outbox re-reads pending per pass, honours enqueues during a drain, and retries with backoff instead of waiting for the next launch. - Dispatcher batches registrations by 20 rather than starving the rest. - 401 compare-and-clear; a 401 after re-auth is unreachable; refused handshakes and 429s are cached briefly instead of re-handshaking per event. - Service is stopped on quit. * fix(mobile): close review findings in push registration and receive (#8129) - Consent generation guards a register that finishes after the switch went off; the host is re-queued for unregister instead of recorded live. - Foreground pushes seed the watermark before adopting the epoch, so a push on a never-connected session cannot wipe a valid watermark. - aps-environment follows the build via app.config.js; the iOS release workflow sets it to production. A bare plugin entry wrote development. - Pushes the OS showed while closed are marked seen before catch-up replay. - Token null result is not cached; failed capability probes are retried and never block an unregister; coalesced summaries are shown but not marked. - Unresolvable fingerprint routes nowhere and is suppressed in foreground. - Android channel ensured at boot; capability hook diffs clients by identity. * fix(cloud): harden the push deploy workflow and size the gateway to the budget (#8129) - Roll traffic back on a failed post-shift check; delete a candidate that never took traffic; retry the origin probe and the FCM probe. - Assert Terraform-owned scaling instead of mutating it from the workflow. - Build before taking the Cloud SQL rollout lease. - Declare the database pool in Terraform (2 per instance, max 2 instances) and add the gateway to the connection budget; the previous default put the shared instance 65 connections over its ceiling. - State plainly that the shared deploy identity's relay authority is inherited. * fix(push): read the runtime from shared state at push startup (#8129) Threading the runtime through launchDesktopMode put the launch module one line over the 300-line lint budget after the rebase. * fix(push): key the unauthenticated rate limit on the hop Cloud Run wrote (#8129) Cloud Run appends the connecting peer to x-forwarded-for; the limiter read the left-most value, which the caller controls, so a forged first hop earned a fresh bucket per request. * fix(push): close the final security review findings in the gateway and infra (#8129) - app.onError logs only the error name and answers a bare 500; hono's default handler printed the whole error, and a pg error carries the row in detail - a second per-IP bucket (240/min) runs ahead of the bearer lookup on every authenticated route, so forged bearers cannot spend the two-connection pool - one live session per host: minting deletes the host's earlier row - device-less hosts are pruned after 1 h, not 30 d; any keypair mints one free - notificationId is printable ASCII, since it becomes the APNs collapse header - the impersonated FCM probe token is masked in the workflow log - prevent_destroy on the Apple secrets and the orca_push database * fix(push): close the final security review findings in the desktop client (#8129) - fetch never follows a redirect: a 307 would replay the host proof and the phone's token to whatever origin the redirect named - registerPush params are strict and the paired identity is spread last - a per-device bucket (10/min) bounds a phone looping registerPush, which costs a gateway write and a synchronous registry write each time * fix(mobile): close the final security review findings in push receive (#8129) - a push with no epoch can no longer claim a seq-derived dedup key, in the foreground or from the tray; a forged seq:N could otherwise swallow the real bell at that seq - a provider-delivered push with no host catalog, or no fingerprint at all, stays unrouted instead of falling back to the hostId its raw data carries * docs(push): record the ip buckets, session and host retention, and the token-ownership limit (#8129) * fix(push): apply the schema on an untimed pool and retry statement-timeout aborts (#8129) Ports the relay's #18722 pattern to the gateway: DDL runs on a one-connection pool with statement_timeout 0 that is closed before the serving pool opens, and SQLSTATE 57014 joins the bounded transaction retry path. * fix: harden mobile push delivery and deployment recovery * feat: align mobile notification preferences with desktop delivery * fix: accept variable-length APNs device tokens * fix: deduplicate native APNs and background socket notifications |
||
 OrcaWinandOrca Worker
|
c252d855ac |
fix(windows): resolve npm/pnpm .cmd shims past cmd.exe (#17869)
* fix(windows): resolve npm/pnpm .cmd shims past cmd.exe
A `.cmd` target forces every spawn through `cmd.exe /c` with each argument
caret-escaped, and Microsoft Defender for Endpoint scores a long `cmd.exe /c`
line carrying caret-escaped natural language as obfuscation. `codex.cmd` is
named in the spawn cluster of the MDE incident this addresses.
npm's `cmd-shim` and pnpm's `@zkochan/cmd-shim` generate files whose whole body
is "find node, run this script". Read one, and the spawn can go straight to
`node.exe <script> <args>` — no cmd.exe, no caret escaping. Anything the parser
does not recognise exactly, or whose target cannot be confirmed on disk, keeps
the existing cmd.exe path.
Incidentally fixes a real bug: cmd ends its command at a raw CR/LF whatever the
quote state, so a multi-line agent prompt through a `.cmd` shim had to be
rejected. Resolved shims have no such limit.
* fix(windows): refuse drive-relative shim paths and run the win32 tests in CI
Two blocking findings from review.
A drive-relative path defeated the absolute-path guard:
`win32.isAbsolute('D:evil.js')` is false, but `win32.resolve` reads the drive
letter and lands on `D:\evil.js`, outside the shim directory. cmd would have
built `C:\shim\D:evil.js` and failed; we would have executed the wrong file.
Adding `:` to the unsafe-character set closes it, and the alternate-data-stream
spelling `a.js:zone` with it. It costs no coverage: 84 of the 91 real shims on
this box still resolve, the same seven fall back.
Neither `windows-cmd-shim-resolution.test.ts` nor its `.win32` sibling was in
the Windows package job's file list, so the whole filesystem/resolution half and
the real-spawn equivalence suite ran nowhere. Both are now in
`WINDOWS_PACKAGE_TESTS` and in the pr.yml step.
Also from review: clear `windowsVerbatimArguments` explicitly on the resolved
branch rather than inheriting it, since there is no caller-built command line
there; document the kill switch and the PTY/hook-wrapper scope limits in
docs/reference; and cover drive-relative, BOM, line-ending, casing and `%*`
tampering in the platform-independent half of the tests.
* docs(windows): justify the shim-path colon guard from the filesystem rule
The guard was argued empirically ("none of the 91 shims on this box has one"),
which invites a future reader to relax it for a shim we have not seen. Windows
reserves `:` within a path segment, so a relative path cannot carry one at all:
the only spellings that can are drive-qualified, an alternate data stream, or a
`\?\` device path, and the last is already refused as absolute. That makes a
false refusal impossible rather than unobserved.
* refactor(child-process): move resolveSpawn into its own module
The merge with main pushed run-process.ts one line past the 300-line cap:
both sides grew it. The spawn-argv decision is already a pure, separately
tested unit, so it moves out rather than the cap moving up. run-process.ts
re-exports it, so no caller changes.
* perf(child-process): cache the shim interpreter lookup
The parse cache spared the shim read but not the PATH walk, so a second
resolution of the same .cmd did 0 reads and one statSync per PATH entry --
30 on a 30-entry PATH, synchronous on resolveSpawn, where one dead network
mount blocks the calling thread on every spawn.
Keyed by shim directory AND PATH, since the shim's own rule is
%~dp0\node.exe first then PATH, and a PATH edit between spawns must miss.
Corrects the stat comment, which accounted only for the shim itself.
* fix(child-process): revalidate a cached shim interpreter before using it
The node cache was held for process life and never rechecked, so a cached
node.exe that was later uninstalled -- or dropped from PATH by a version
manager -- was still handed to resolveSpawn, failing the spawn with ENOENT.
An uncached process in the same state returns null and falls back to
cmd.exe successfully, so the cache was strictly worse than no cache.
One statSync on a non-null hit, not one per PATH entry, so the walk this
cache exists to skip is still skipped. The stale-null direction stays
uncorrected on purpose: it only keeps the working cmd.exe fallback. Both
directions are now stated in the comment, along with the known miss for
callers that vary PATH per spawn.
* fix(child-process): honour PATHEXT when resolving the shim interpreter
The doc claimed a node.com/.bat/.cmd on PATH returned null and fell back to
cmd.exe. The scan actually skipped those entries and kept looking for a
node.exe, so PATH=C:\A;C:\B with C:\A\node.com and C:\B\node.exe resolved to
B's node.exe while the shim runs A's node.com -- a different binary, chosen
silently, on the one axis this module must not get wrong.
The scan now follows cmd's rule: first PATH directory holding any PATHEXT
spelling wins, PATHEXT order decides within it, and only an .exe winner is
returned. Anything else gives up and keeps the cmd.exe path, which restores
the strict-subset-of-cmd property everywhere except the documented cwd case.
PATHEXT is read from the child's env and joined into the cache key, since it
now changes the answer. Costs one stat per PATHEXT entry per node-less
directory, paid once per process behind the cache.
---------
Co-authored-by: Orca Worker <orca-worker@localhost>
|
||
|
OrcaWinandOrca Worker
|
fba90e017c |
fix(windows): copy the daemon host exe verbatim instead of renaming it (MDE T1036) (#17865)
* docs(windows): document the EDR signal surface Six Microsoft Defender for Endpoint incidents fired against Orca 1.4.192 in eight days on one enterprise Windows 11 / Intune tenant. All six were behavioural process-tree scoring, not signature hits; two escalated to multi-stage incidents mapped to ATT&CK Execution and Collection. Add a reference doc mapping each attack-technique-shaped behaviour to the code that produces it and to why it exists: the renamed daemon image (T1036), the per-process PEB read, encoded policy-bypassed PowerShell (T1049), caret-escaped cmd.exe lines, and computer-use screen capture plus runtime-compiled MSIL (T1113). Records that signing is not the gate -- reputation is signer plus hash-keyed prevalence -- and carries the two evidence gaps the report noted. Adds an engineer checklist, deployment guidance for admins (AV path exclusions do not suppress EDR behavioural alerts; an MDE alert suppression rule does), and an explicit pre-deployment warning about computer use. * docs(windows): correct the PowerShell flag inventory and admin paths Review corrections to the EDR posture doc. The "encoded, policy-bypassing PowerShell" list conflated three different shapes and was incomplete. Split it into the three tiers an EDR actually scores differently -- bypass plus encoding, encoding alone, and bypass alone -- and add the sites it missed, including windows-mobile-firewall.ts, which encodes a script and launches it elevated through Start-Process -Verb RunAs. system-fonts.ts (-Command) and desktop-script-provider-bridge.ts (-File) were listed as encoded and are not. Notes that a raw grep under-reports, because the hook sites reach -EncodedCommand through wrapWindowsPowerShellEncodedCommand. Attribute the in-payload Set-ExecutionPolicy move to #16576 rather than to #16003's measurement, which keyed on -WindowStyle Hidden + -EncodedCommand, and record that the launcher's own tradeoff is unverified on a real box. Admin guidance was missing two ways a suppression rule pinned to one full path misses real activity: the .staging-<hex> sibling that exists mid-update, which is when the update-cluster incidents fire, and the userData fallback when LOCALAPPDATA is unset. Also: state the measurement conditions on the process-table timings, note that Hermes has surface even though we have no telemetry for it, note that the uninstaller names are electron-builder-generated and in no repo file, drop a volatile line count, and mark the per-operation computer-use shape as being addressed by an unmerged change. Drops the duplicated AGENTS.md section, keeping the indexed bullet. * docs(windows): reconcile the EDR posture doc with the shipped remediation Three claims in this doc became false once the rest of the Windows EDR set landed, and two told engineers the opposite of what the release does. The process-table section still described one shared snapshot taken with `Memory | CommandLine | CreationTime`, argued that splitting the cache per field set "would restore exactly the fan-out it exists to prevent", and concluded the shape was unfixable because "the information is only in the PEB". The split shipped (identity opens no handle at all), `Memory` is retired, and the command line now comes from the kernel through `ProcessCommandLineInformation` -- `ReadProcessMemory` is absent from the compiled addon and a ratchet asserts it against the import table. An engineer reading the old text would have concluded both fixes were dead ends. The PowerShell site inventories were stale in three of four lists: the port scan went native, every `-ExecutionPolicy Bypass` + `-EncodedCommand` pair was dropped as a measured no-op, and of the unencoded-bypass list only `wsl-cli-scripts.ts` survives. Regenerated against the merged tree, including the sites that reach the flag through `wrapWindowsPowerShellEncodedCommand` and never spell it, which a raw `rg` misses. Incident-evidence sections are left alone: they record what the tenant observed on 1.4.192, not what the code does now. * fix(windows): copy the daemon host exe verbatim instead of renaming it Microsoft Defender for Endpoint flagged `orca-terminal-daemon.exe` as MITRE T1036 (Masquerading): Orca copied its own `Orca.exe` into %LOCALAPPDATA% under a different name, specifically so the NSIS updater's `taskkill /IM Orca.exe` could not match, then ran it detached. Because that process is what every other flagged action was attributed to, the name mismatch acted as a reputation multiplier on unrelated findings. The rename was never what made the daemon survive. In app-builder-lib 26.15.3 the installer's FIND_PROCESS/KILL_PROCESS select processes whose image path is under $INSTDIR; `taskkill /IM` is only the fallback for hosts where PowerShell is missing or blocked. Survival is a property of the path, and %LOCALAPPDATA%\Orca\daemon-host is outside $INSTDIR whatever the file is called. Derive the host exe name from process.execPath so the copy is byte-for-byte, name included — it keeps its Authenticode signature and carries no renamed-image signal. On the no-PowerShell fallback the daemon is now killed with the app and terminals cold-restore, which is the documented pre-relocation outcome the update harness already asserts, not a regression. The uninstall macro no longer needs a distinct name to find the daemon; it kills the app's own image name (plus the legacy name, for hosts left by older builds). Adds docs/reference/windows-daemon-host-relocation.md with the survival contract, the rejected alternatives and their measured costs, and the invariants to keep. * fix(windows): apply daemon-host relocation review corrections Scope the uninstall taskkill to the current user with `/FI "USERNAME eq %USERNAME%"` via cmd.exe, matching upstream's per-user KILL_PROCESS — without it an elevated machine-wide uninstall reaches another logged-on user's session, so the "no collateral" claim in the comment was overstated. Comment the rmSync-before-publish: Windows refuses to delete a running image, so a live daemon already hosted in this version's dir (same-version reinstall, or a dev channel reusing a version) throws and materialization fails open. Doc corrections: - The fallback selector is the full per-user `taskkill /F /IM "<app>.exe" /FI "PID ne $pid" /FI "USERNAME eq %USERNAME%"`, not a bare `taskkill /IM`. - The probe reads `Get-ExecutionPolicy -Scope Process`, not the effective policy, and GPO writes MachinePolicy/UserPolicy — so GPO-managed hosts take the primary path-scoped branch. Narrow the fallback triggers accordingly. - Drop the Authenticode sentence: the old name was equally byte-identical and equally signed, so a filename has no bearing on signature validity. - Name the new update-abort path: the daemon now matches FIND_PROCESS, so on the fallback branch an unkillable host reaches the retry loop's MessageBox /SD IDCANCEL and Quits, aborting a silent update. - Correct the customCheckAppRunning rejection. It is ~6 lines, not a rewrite; it is wrong because forcing the PowerShell branch where PowerShell is absent makes FIND/KILL silently no-op and leaves the real app running with files in use. - Bound the win honestly: OriginalFilename is empty on the shipped binary, so the strongest T1036 indicator never fired, and the residual copy-and-run-detached shape still maps to T1036.005. Reconcile docs/reference/windows-edr-posture.md, which documents the rename as a live finding and would otherwise contradict this change. Content-only edit: markdown under docs/reference/ is not oxfmt-formatted as a matter of practice and nothing in CI gates it, so the file is left consistent with its neighbours. * fix(windows): expand USERNAME in NSIS instead of spawning cmd.exe The uninstall macro routed both taskkills through `"$SYSDIR\cmd.exe" /C` purely so `%USERNAME%` would expand — two extra interpreter spawns on the uninstall path, in a change whose whole point is not adding scored behaviour, and the exact `cmd.exe /c` shape the new AGENTS.md EDR bullet warns about. NSIS reads the variable itself with ReadEnvStr, so the spawns buy nothing. Verified on Windows 11 that the generated command line does what the filter is there for: a copy of cmd.exe running as orca-nonexistent-probe.exe (pid 34244) was terminated by `taskkill /F /IM "orca-nonexistent-probe.exe" /FI "USERNAME eq <user>"` — SUCCESS, exit 0, process gone. Guarded on an empty USERNAME because the degenerate case is silent: taskkill rejects an empty filter value outright ("The search filter cannot be recognized") and kills nothing, which would leave exactly the orphaned daemon this macro exists to reap. `*` is rejected as a filter value too, so there is no branchless spelling. With no USERNAME to scope by it kills unfiltered, as the macro did before the filter was added. Stack stays balanced: three pushes, two nsExec pops, three restores. Also strike the last stale row in windows-edr-posture.md's remediation table. "Copying our own image under a different name" read as outstanding work; it is done by this change, so the row now points at the relocation doc. Same class of staleness as the section reconciled in the previous commit, and git would not have flagged it either. * fix(windows): port the daemon-host uninstall sweep into the live NSIS include The uninstall macro this branch rewrote lived in config/nsis/daemon-host-uninstall.nsh, which main no longer includes: #17906 consolidated every Windows installer hook into config/nsis/orca-installer-hooks.nsh because electron-builder accepts exactly one `nsis.include`. Merged as-is, the rewritten macro would have been dead code while the shipped uninstaller kept running main's stale sweep — `taskkill /F /IM orca-terminal-daemon.exe`, which matches nothing now that the relocated host is a verbatim Orca.exe copy. The RMDir that follows then cannot delete the running image, so a live orphaned daemon and its ~224 MB tree would survive every uninstall. Ported into the live include: the ${APP_EXECUTABLE_FILENAME} kill, the USERNAME filter that keeps an elevated machine-wide uninstall out of another logged-on user's session, and the register save/restore around both. The legacy orca-terminal-daemon.exe kill stays so hosts left by older builds are still reaped. The ratchet that was meant to catch exactly this pinned only the legacy image name, which main's stale macro already satisfied, so it passed both ways. It now asserts the app-exe kill and the USERNAME filter, against comment-stripped script — the prose above the macro names both image names, so a toContain over the raw file proves nothing. --------- Co-authored-by: Orca Worker <orca-worker@localhost> |
||
 foXaCe
|
49d6d35b16 |
feat(i18n): add French UI locale
foXaCe <290678+foXaCe@users.noreply.github.com> |
||
|
OrcaWinandOrca Worker
|
2c4989ea94 |
docs(windows): document the EDR signal surface (#17856)
* docs(windows): document the EDR signal surface Six Microsoft Defender for Endpoint incidents fired against Orca 1.4.192 in eight days on one enterprise Windows 11 / Intune tenant. All six were behavioural process-tree scoring, not signature hits; two escalated to multi-stage incidents mapped to ATT&CK Execution and Collection. Add a reference doc mapping each attack-technique-shaped behaviour to the code that produces it and to why it exists: the renamed daemon image (T1036), the per-process PEB read, encoded policy-bypassed PowerShell (T1049), caret-escaped cmd.exe lines, and computer-use screen capture plus runtime-compiled MSIL (T1113). Records that signing is not the gate -- reputation is signer plus hash-keyed prevalence -- and carries the two evidence gaps the report noted. Adds an engineer checklist, deployment guidance for admins (AV path exclusions do not suppress EDR behavioural alerts; an MDE alert suppression rule does), and an explicit pre-deployment warning about computer use. * docs(windows): correct the PowerShell flag inventory and admin paths Review corrections to the EDR posture doc. The "encoded, policy-bypassing PowerShell" list conflated three different shapes and was incomplete. Split it into the three tiers an EDR actually scores differently -- bypass plus encoding, encoding alone, and bypass alone -- and add the sites it missed, including windows-mobile-firewall.ts, which encodes a script and launches it elevated through Start-Process -Verb RunAs. system-fonts.ts (-Command) and desktop-script-provider-bridge.ts (-File) were listed as encoded and are not. Notes that a raw grep under-reports, because the hook sites reach -EncodedCommand through wrapWindowsPowerShellEncodedCommand. Attribute the in-payload Set-ExecutionPolicy move to #16576 rather than to #16003's measurement, which keyed on -WindowStyle Hidden + -EncodedCommand, and record that the launcher's own tradeoff is unverified on a real box. Admin guidance was missing two ways a suppression rule pinned to one full path misses real activity: the .staging-<hex> sibling that exists mid-update, which is when the update-cluster incidents fire, and the userData fallback when LOCALAPPDATA is unset. Also: state the measurement conditions on the process-table timings, note that Hermes has surface even though we have no telemetry for it, note that the uninstaller names are electron-builder-generated and in no repo file, drop a volatile line count, and mark the per-operation computer-use shape as being addressed by an unmerged change. Drops the duplicated AGENTS.md section, keeping the indexed bullet. * docs(windows): reconcile the EDR posture doc with the shipped remediation Three claims in this doc became false once the rest of the Windows EDR set landed, and two told engineers the opposite of what the release does. The process-table section still described one shared snapshot taken with `Memory | CommandLine | CreationTime`, argued that splitting the cache per field set "would restore exactly the fan-out it exists to prevent", and concluded the shape was unfixable because "the information is only in the PEB". The split shipped (identity opens no handle at all), `Memory` is retired, and the command line now comes from the kernel through `ProcessCommandLineInformation` -- `ReadProcessMemory` is absent from the compiled addon and a ratchet asserts it against the import table. An engineer reading the old text would have concluded both fixes were dead ends. The PowerShell site inventories were stale in three of four lists: the port scan went native, every `-ExecutionPolicy Bypass` + `-EncodedCommand` pair was dropped as a measured no-op, and of the unencoded-bypass list only `wsl-cli-scripts.ts` survives. Regenerated against the merged tree, including the sites that reach the flag through `wrapWindowsPowerShellEncodedCommand` and never spell it, which a raw `rg` misses. Incident-evidence sections are left alone: they record what the tenant observed on 1.4.192, not what the code does now. --------- Co-authored-by: Orca Worker <orca-worker@localhost> |
||
|
Neil
|
6aba202d5e |
feat(docs): publish OSS docs with stable releases
Publish the standalone docs site under docs/site and deploy it on stable desktop releases. |
||
|
Neil
|
99b59064fd |
docs(ime): codify desktop composition regression checks (#17172)
* docs(ime): codify desktop composition regression checks * docs(ime): define remote verification verdicts * docs(ime): narrow placeholder masking invariant * docs(ime): require final-cell caret containment * docs(ime): cover async attachment settlement * docs(ime): correct semantic placeholder contract * docs(ime): record bounded ownership contracts |
||
|
Neil
|
58a52a8ce3 |
test(wsl): guard probes that report failure as a negative answer (#17352)
* test(wsl): guard probes that report failure as a negative answer A WSL probe that cannot reach its distro returns the same value as one that asked and got "no". Downstream nothing can tell them apart, so a distro that was busy for a second reports no git, or no agent sessions, until relaunch — sticky, silent, and identical to the real thing. That has shipped three times: preflight CLI probes, the glab auth fallback (#8941), and listRunningWslDistrosAsync failing closed with no last-known-good while polled every 2s (PR #17072). Scan the WSL and preflight probe modules for the shape and hold the current set in an allowlist that only shrinks. Scoped deliberately: the same shape appears ~850 times across src/ and is usually correct, because for most callers a failure really does mean absent. It is only dangerous where the answer describes a distro. The guard cannot see the dangerous part — whether the value is later cached or gates discovery is dataflow, not syntax. It stops a new swallow site appearing here without someone saying why it is safe to pin, which is the review that was missing all three times. * test(wsl): make ratchet failures actionable A red build must say what to do. Name the offending files, say the allowlist is where a safe case goes, and — for a stale entry — say the change is fine and the list just needs to shrink. * docs(wsl): track the probe failure-semantics reference docs/** is gitignored with an explicit allowlist, so the reference the ratchet points contributors to was silently left out of the branch. A guard whose error message cites a doc that is not in the repo is worse than no doc. * test(wsl): catch a swallow whose reason trails the return The guard only tolerated comments before `return`, so `return false // ...` slipped past — including the exact snippet the doc and the test's own docstring use as the canonical example. The doc asks authors to write down why a swallow is safe, and the natural place for that sentence is trailing the return, so following the guidance defeated the guard. Verified against both shapes: trailing comment and comment on the line after. |
||
 Neilandinnocarpe
|
f400f8fd5f |
fix(macos): opt out of press-and-hold so held keys repeat (#14746) (#15589)
* fix(macos): opt out of press-and-hold so held keys repeat (#14746)
macOS routes press-and-hold to the accent picker unless an app sets
ApplePressAndHoldEnabled=false for its own bundle, so holding j in vim
inserted one character instead of repeating. Orca never set it.
Written at most once, and never over an explicit value: `defaults read`
is domain-scoped and exits 1 when the key is absent, which is the only
way to tell "unset" from a deliberate false — Electron's
systemPreferences.getUserDefault reports false for both. A recorded
decision in userData keeps a later launch from re-clobbering a user who
deletes the key to get the accent picker back.
* docs(macos): record the revert hazard and CI's macOS test gap
Two things a reader of this module cannot otherwise know.
A revert leaves the key written in every user's domain forever. AppKit reads
the plist, not this file, so removing the code alone keeps press-and-hold
disabled for everyone who ran an affected build. The sibling period-substitution
module carries the same warning because that fix was already lost once this way.
And the real-binary test file that pins the defaults(1) exit-code semantics this
design rests on never runs in CI: the e2e workflow and both unit-test jobs are
ubuntu and windows, and the only macOS runners in the repo are build and
packaging jobs that run no tests. Those six tests plus the real-bundle e2e case
pass on a developer Mac and execute zero times in a green PR, so the comment
should not imply enforcement that is not there.
Refs #14746
* feat(macos): let users turn the accent menu back on (#14746)
Orca disables press-and-hold for its own preferences domain so held keys
repeat. That is the right default, but the way back was a `defaults write`
buried in a source comment: nothing in docs/ or the README mentioned it, and
the preference is per-application, so it silently takes the accent picker
away from the Markdown editor and every other text field too.
Terminal -> Advanced now carries a "Character Accent Menu" switch, macOS and
desktop only. A web client cannot write a macOS preference for the machine the
user is looking at, so the control and its search-index entry are both gated on
that, not on the client's platform alone.
Precedence, which is the part that is easy to get wrong: the setting is
`undefined` until the user touches it, which is what keeps a hand-run `defaults
write` in charge for everyone who never opens the toggle. Once used, Orca owns
the key and writes exactly what the switch asks for -- `ApplePressAndHoldEnabled`
*is* the accent-menu switch, so it maps straight through with no inversion. The
choice is compared against `appliedSetting` in the existing decision record
rather than against the domain, so a `defaults write` made *after* using the
toggle is still the newer choice and survives the next launch. Re-asserting the
value every launch would have reintroduced the clobbering the record exists to
prevent.
The write lands for the next launch, since AppKit reads the preference as the
process starts, so the toggle shows the same restart banner the window-blur
setting uses. That banner is now a shared component, keeping its original
translation keys.
docs/reference/macos-press-and-hold.md records the precedence rules, the
`defaults read` rationale, the revert hazard, and the fact that none of this
executes in CI: every macOS job builds or packages and runs no tests, so the
real-binary and e2e coverage here passes only on a developer Mac.
* docs(macos): stop asserting when AppKit re-reads the press-and-hold key
Five places stated "AppKit reads the preference as the process starts" as
fact. That is the reason given for requiring a relaunch, and it is not
something this change ever measured.
Evidence points the other way: terminal emulators that register this key
after their process has started get key repeat in that same launch, which a
read-once-at-startup model cannot explain.
The relaunch requirement itself still looks right, but for a different and
verifiable reason: the write goes out through a separate `defaults` process,
so this app's own cached copy need not observe it. That is what the comments
now say, with the AppKit question left open rather than answered.
Refs #14746
* docs(macos): correct the startup comment's launch-timing claim
The comment said this call site is "the last point that can still matter for
this launch", which contradicts the rest of the module: the write is assumed
to land for the next launch because it goes out through a separate `defaults`
process. Reported on the PR by @innocarpe, who also supplied the replacement
wording.
Co-authored-by: innocarpe <innocarpe@users.noreply.github.com>
* refactor(macos): probe press-and-hold through the shared spawn chokepoint
`src/shared/child-process/child-process-import-boundary.test.ts` forbids a
direct `node:child_process` import outside its allowlist, and the allowlist only
shrinks — so this module moves to `runProcessSync`, which exists for callers
that genuinely cannot await. This one runs before `app.whenReady()`.
`runProcessSync` returns a non-zero exit instead of throwing it, so the
three-way read decision is re-expressed against `ProcessResult`: exit 0 is an
explicit value, exit 1 is a missing key, and a timeout, a signal kill, any other
exit, or a child that never started all stay 'unknown'. The throw path is now
inside `interpretDefaultsRead` so a spawn failure is reachable from a test
rather than hidden in an untested catch, and the write checks the exit code —
a refused `defaults write` no longer looks like success.
Both boundary-test failures were the same import: with it gone the offender
count returns to 155, so no ratchet baseline is bumped.
* Revert "feat(macos): let users turn the accent menu back on (#14746)"
This reverts commit
|
||
|
Neil
|
5631aa00dd |
feat(orcad): items 2–7 — degradation, natives, daemon, ops, deploy (#16398)
* fix(ports): stop joining an undefined resourcesPath on a non-Electron host `resolveWorkerEntryPath` branched on `isPackaged` alone and joined `process.resourcesPath`. orcad reports `isPackaged` true — correctly, it is a production build, and ~15 consumers read it that way to gate HTTPS-only skill downloads and the real CLI name — but `process.resourcesPath` is Electron-only and `undefined` under plain Node. So the packaged branch threw `TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string` where a clean "worker unavailable" was the honest outcome. The type said `resourcesPath: string`, which is how it went unnoticed; it is now `string | undefined`, so the compiler carries the fact. A host with no Electron resources tree has no asar to look in, so it falls back to the module directory and lets the caller report a missing worker. Found by the item 1 agent while auditing the same `isPackaged` defect class in the watcher. Verified in both directions: reverting the guard reproduces the TypeError. * feat(orcad): prove node-pty loads before anything requires it Of the two ways node-pty fails, only one is catchable. A missing module throws MODULE_NOT_FOUND. A module built against the wrong libc or Node ABI is refused by the dynamic loader, and in the worst case takes the process down before any handler exists — that is #9902, which crashed the desktop app on Ubuntu 20.04 before a window appeared. There was no libc or ABI precondition anywhere in the tree. So orcad now proves the load in a CHILD process, from main.ts, before anything requires node-pty. Whatever the child does — throw, abort, die on a signal — is data rather than our own death, and the operator gets a sentence naming the host's libc, Node ABI and prebuild slot plus the command to run. Proven-unloadable exits 78 (EX_CONFIG), so a supervisor does not restart an unequippable host forever. A probe that never answered is unverifiable, not blocked: refusing to boot on an inconclusive signal would take down hosts that work. The child dlopens the file node-pty would have chosen, before requiring the package. node-pty's loader walks several directories and rethrows only the LAST error, so a refused binary reads as "Cannot find module ./prebuilds/..." — which sends the operator to install a module that is already there. It also reports through stdout: node echoes the whole -e source above a stack trace, and matching tokens against stderr made the probe's own source text answer for the verdict. Verdicts reach clients as a terminal_unavailable degradation alongside the existing browser_unavailable one, through the same cause-registry shape. degradations[].code is now an open vocabulary; clients already render only `message`. Prebuilds are compiled from PATCHED sources — the patch IS the glibc-floor fix, so an upstream tarball reproduces #9902 — into linux-{x64,arm64}-{glibc,musl} and darwin-{x64,arm64} slots. libc is in the slot name because node-pty's loader falls back to prebuilds/<platform>-<arch> and cannot tell glibc from musl. orcad installs the matching slot at boot, so a host with no compiler serves terminals. The relay's five pure toolchain-diagnosis functions moved to a transport-free module so the Node bundle can reuse them without dragging ssh2 in behind them; the relay keeps its API by re-export. macOS gets `xcode-select --install` rather than the cross-distro apt/dnf/pacman/apk menu, every line of which is wrong there. * test(orcad): pin the node-pty precondition to ground truth, not a prepared host CI's test shard runs `vitest` directly, so `ensure-native-runtime --runtime=node` never prepares node-pty for the Node ABI — `degraded` is the correct verdict there, and asserting 'ok' encoded an environment the shard does not have. Asserting whatever it returned would be vacuous, so the expectation is now derived from an independent require() of node-pty. Verified it still bites: forcing the precondition to always report 'ok' fails the suite. * feat(orcad): run the terminal daemon, and the ops contract around it orcad declared `canRecoverPersistentLocalPtys: () => false` because it did not run the terminal daemon, so every restart, update and rollback SIGKILLed every running terminal — on the host whose selling point is that work survives the client going away. That is the one property `ssh-execution-boundary.md` recommends the peer model for. Item 4 — the daemon: - Port the launch path off electron: `daemon-init.ts`, `daemon-host-relocation.ts` and `observability/logs-directory.ts` now read the `AppEnvironment` port. Relocation additionally asks whether the app root is an asar archive rather than whether the build is packaged, so a Node host answering `isPackaged() === true` no longer walks into an Electron-only NSIS-escape path (same precedent as `parcel-watcher-entry-path.ts`). - `build-orcad.mjs` emits `daemon-entry.js` beside `orcad.js`, scans the forked children's metafiles for electron/node:sqlite, and load-checks the child under plain Node. - orcad spawns and adopts the daemon; shutdown disconnects and never kills it. `canRecoverPersistentLocalPtys` now reads the live provider and is false under degraded routing, where fresh terminals would die with the process. Item 3 — the ops contract (docs/reference/orcad-operations.md): - Bind policy: `--bind`, default loopback, pinned so neither `orca serve`'s wide default nor the connected-device widen can override it, and so a paired client cannot rebind the listener from outside. - Instance lock on the data root before profile load, scoped to the runtime role so it never refuses a restart that a live daemon makes worthwhile. - Supervision: exit codes a supervisor can act on (78 = do not retry), second-signal escalation, a shutdown deadline, and crash-loop containment on daemon respawn. - Health in the readiness payload: build hash, Node ABI, and a PTY self-test that spans both processes — the daemon spawns a real PTY in its own process and the verdict crosses its socket. Both bundle load-checks now assert on exit codes: these bundles are minified onto one line, so Node's uncaught-exception report echoes every string literal in the bundle and the previous message match passed against a bundle that never loaded. * feat(orcad): deploy, activate and roll back a versioned orcad install Plan items 6 and 7 from docs/design/shipping-orcad.html. Install reuses the relay's transaction verbatim — per-version lock, staged SFTP write, .install-complete sentinel, stale-lock recovery — under a parameterized namespace, so orcad-<v>/ sits beside relay-<v>/ permanently (§06). Parameterizing GC is the trap that creates: each model now collects only its own directories, enforced twice (prefix-scoped remote listing plus a local ownership re-check), and a client picks its model from how the host is registered, never from what it finds on disk. Activation is separate from installation, because a versioned directory selects nothing. A candidate is launched, publishes orca_server_ready, and only becomes active if its cross-process health payload passes: right build hash, listening, daemon live, PTY self-test green. A rejected candidate is stopped and the incumbent restarted, so a careful deploy cannot cause the outage it was being careful about. Update and rollback are shaped by the daemon. An update restarts orcad, the daemon outlives it, and the surviving daemon was forked from the outgoing bundle — so live terminals defer the update rather than proceed, and GC pins the active version, the rollback target and the live daemon's bundle. Orca's persisted state carries no schema version, so rollback restores a pre-activation snapshot rather than trusting backward-readability; the point past which it is unsafe is the first terminal created after activation, which the snapshot cannot describe and the surviving daemon still owns. Running the generated shell for real found two bugs the text assertions missed: tar members re-quoted inside a shell variable captured nothing, and kill -0 reports a zombie as alive. * test(orcad): assert the precondition is self-consistent, not environment-shaped The real-host case cannot predict a status: CI's shard runs vitest directly, so node-pty is never built for the Node ABI and 'degraded' is correct there, while a prepared checkout gives 'ok'. The previous attempt used require('node-pty') as ground truth, which resolves the JS wrapper while the native binding loads lazily — it proved strictly less than the precondition checks, and failed CI for exactly that reason. What is invariant on a host with node-pty installed: never 'blocked', and never a degraded verdict carrying an unestablished reason. The injected-input tests keep the logic coverage. * fix(orcad): drop an eslint-disable the rule no longer needs * test(orcad): separate slot placement from the load verdict Both remaining CI failures were the same shape: tests reaching into node_modules for a pty.node that only exists after `ensure-native-runtime --runtime=node`, which CI's shard never runs because it invokes vitest directly. Slot *placement* is the logic worth checking on every host, so it now uses a synthetic payload and asserts the verdict stays honest about not loading. The three assertions that genuinely need a Node-ABI binding are gated on it existing. Verified: breaking slot installation fails both placement tests; with the real pty.node hidden the file is 17 passed / 3 skipped instead of ENOENT. * test(orcad): gate the load-dependent cases on a real load, not on the file existing CI ships a pty.node built for Electron's ABI, so existsSync was true while require still failed — the gate ran exactly the tests that host can never satisfy. It now probes the binding in a child process, so a bad one cannot take the runner down. The self-consistency assertion also allowed too little: 'blocked' is the honest verdict for a corrupt binding, alongside 'ok' on a prepared host and 'degraded' on an unprepared one. What stays invariant is that anything other than 'ok' names an established cause, so a terminal is never declined for a reason nobody worked out. Verified against all three host states: prepared (19 passed), unprepared, and a corrupt binding (17 passed / 3 skipped, no failures). * test(orcad): gate on the whole premise — binding AND spawn-helper CI has a loadable pty.node but no spawn-helper, and a slot without the helper is legitimately 'degraded'. So the previous gate let a test run whose premise ('a complete slot yields ok') that host cannot satisfy. Verified in both states: with the helper present 19 pass; with it removed the load-dependent cases skip (17 passed / 3 skipped) instead of failing. * fix(orcad): preserve degradation types after rebase |
||
 Jinjing
|
3fbed6612e | docs: remove tracked design plans (#16663) | ||
|
Jinjing
|
cda2280d63 |
Show all automations (#16532)
* Add all-host automations with scoped ownership and multi-authority suppo
Enable automations to run on multiple hosts (SSH targets and local) with
owner-fenced mutations, scoped list queries per host, and conflict
resolution. Introduces desktop and runtime authorities as distinct
automation storage owners, with per-host caching, invalidation, and
retry scheduling on the renderer. Captures registration generations for
SSH hosts to survive re-adoption. Adds CLI support for destination
selection and conflict recovery.
* Filter automation create projects by destination host
Only offer projects available on the selected destination, preventing
the mismatches that would fail at submit time. Auto-adjust the project
selection if it becomes unavailable when the destination changes.
* Add runtime storage authority support for automations
- Support both runtime and desktop as automation storage authorities
- Make owner preconditions optional for legacy-client compatibility
- Cache automation list projections to improve performance
- Add per-row repo/worktree resolution for cross-authority collisions
- Extend automation.list RPC to always include owner metadata
* Replace child_process.execFile with runProcess for external automations
- Migrate external-manager to use cross-platform runProcess wrapper per child-process safety policy
- Abstract electron app/ipcMain APIs in orca-runtime via environment accessors
- Install fake app environment in automation tests for consistent setup
- Reorganize imports to use specific module paths (ssh-target-registry, agent-detection, browser-error)
- Remove external-manager from child-process import allowlists (no longer violates direct import)
* Unify desktop automation CRUD onto the local runtime RPC surface
The desktop authority now speaks the same automation.* RPC contract as
remote runtimes, via callRuntimeRpc({kind:'local'}) -> runtime:call ->
the shared RpcDispatcher. The automations:list/listRuns/create/update/
delete/runNow IPC arms, their preload members, and every renderer
desktop-vs-runtime transport fork are retired; the runtime methods are
the single implementation of scoped lists, owner fencing, and change
publication for both transports (mobile clients already exercised them).
The desktop probe scheduler's priority lease survives the move as an
AutomationService hook the IPC registration installs and the runtime
methods take, so Orca's own automation traffic still parks queued
external-manager probes.
External-manager scope arms and dispatch-loop plumbing stay on IPC by
design; automation change events keep their existing channels (renderer
ingestion already converges them by authority).
* Remove automation ghost SSH tombstone scanning
This functionality for synthesizing tombstones for automation-referenced SSH
targets is no longer needed as part of the automation system refactoring.
* Refuse orphan automations at dispatch time, not migration time
Remove migration-time disabling of orphan automations and the `enabledDecidedBy` field. Dispatch now refuses orphans at runtime instead, simplifying state management and UI. Orphans are left unstamped and enabled; dispatch refuses to run them via `resolveAutomationRunTarget`.
* Show all automations in flat table with unified filter menu
- Replace host picker component with comprehensive Filters menu supporting status, last run, agent, and host filters
- Flatten automation list layout to single table instead of host-grouped sections
- Add Host column to display execution host for each automation
- Display active filters as removable pills below toolbar
- Delete unused AutomationHostPicker* components
* Add automation owner fencing and destination validation
- New AUTOMATION_OWNER_FENCING_RUNTIME_CAPABILITY for owner preconditions; legacy clients get owner metadata snapshotted at RPC boundary for compatibility
- Editor captures and revalidates automation destination before save, preventing silent retargeting if SSH infrastructure changes mid-edit
- SSH target types now isolate renderer-authored fields; generation is server-owned and stripped by IPC handlers
* Route automation recovery actions to the origin host
When an automation action fails due to owner fencing, recovery verbs
("Update server", "Reconnect") must run on the host where the refusal
originated: the row's captured owner for row operations, or the
destination the create dialog captured, not the list's filtered host.
* Remove external manager scope limitation notices
Consolidate create destination eligibility checks with a unified predicate
and fix the bug where desktop repo IDs could be sent to runtime hosts where
they cannot resolve.
* Persist only store-derived automation contexts, not client-perspective o
Store contexts must never be based on client-provided runContext or sourceContext
values—clients speak a different perspective (e.g., 'runtime:<id>' for host IDs
they assign), and persisting those makes the store projection orphan automations
it actually owns. Derived contexts now take precedence in create and update paths,
with explicit null still honored to clear a value. Tests verify this by simulating
drift after storage and confirming that moves re-derive while toggles preserve.
|
||
 Jinwoo HongandJinwoo-H
|
a9781a4118 |
STA-4150: client-hosted remote browser (consolidated) (#15448)
Co-authored-by: Jinwoo-H <jinwoo@stably.ai> |
||
|
Neil
|
5651662494 |
fix(wsl): migrate 21 call sites onto the WSL runner (#15923)
* fix(wsl): migrate 21 call sites onto the runner, after five review rounds Rebased onto main now that the runner (#15903) has landed. 21 sites across 15 files move off ad-hoc `execFile('wsl.exe', ...)`. Allowlist 23 -> 16 on the WSL guard; 163 -> 152 on the W1 child_process guard, which moved as a consequence. Five review rounds, each finding real defects -- several introduced by the previous round's fixes: 1. Hooks ran user orca.yaml scripts under dash; probe failure fell back to the login shell, reintroducing the ~/.profile stall the runner exists to remove. 2. An unparseable probe was cached permanently, disabling every WSL feature on the distro; hooks regressed from "runs degraded" to "fails". 3. Exit 127 had no expiry; a starved 5s probe hard-failed the 10s scan behind it; a joiner burned its budget on someone else's probe. 4. The comment stripper blanked live code, so the windowsHide guard walked past a real unguarded spawn and reported the file clean; an ownership-probe timeout silently deselected the user's Claude account. 5. Verification of the guards themselves. The recurring finding -- a call answering "is this installed?" on a degraded PATH -- was eventually fixed structurally rather than per-caller: the runner refuses an unresolved guest PATH unless the caller opts in. Per-site vigilance was demonstrably not holding; 3 of 8 sites had already forgotten the analogous exit-code check. Remaining 16 files need a runner mode that does not exist: a long-lived streaming child (OAuth logins, hook relay), a synchronous caller, or a host-level flag like --status that the guest-command API cannot express. * fix(wsl): close round 5's P1s -- degrade where PATH was never needed Round 5 measured the guards by re-executing their algorithms standalone rather than reading them, and found four things. P1 -- four skill/plugin paths gained a hard dependency on the login-shell probe that they never had. They ran under a plain non-login `sh -c` on main, so a probe failure now breaks WSL skill discovery and install on exactly the distro the runner was built for: one with a slow `~/.profile`. Worse, the throw escapes before each site's own error mapping, so the UI gets a raw internal string. They degrade now, per the rule this branch already wrote down in `wsl-fish-history-cleanup.ts`. P1 -- Codex and Claude were asymmetric. Claude's five credential sites degrade; Codex's were strict, so adding a WSL Codex account failed where adding a Claude one succeeded. Three of the four are byte-equivalent to Claude sites, and their scripts read `$HOME`/`$WSL_DISTRO_NAME`, which wsl.exe supplies without a login shell. `assertWslCodexCliAvailable` stays strict on purpose -- that one really does answer "is this installed?" (#9725). P1 -- the ownership-probe timeout fix did not survive the rebase onto main. A timeout still returned "not owned", which the caller *persists*, clearing the user's account selection. P1 -- `blankStringContents` desynced on a nested template literal (`` `${`x`}` ``), leaving 116 lines of a child_process importer outside the ratchet, with 27 importers structurally at risk. Now tracks template depth. Regenerating against the fixed blanker: 70 -> 68 offenders. Also: the windowsHide vacuity check could not fail while the allowlist alone exceeded its bound -- the exact defect the sibling guard documents avoiding. It now names a file that definitely offends. * fix(wsl): close round 6 -- my blanker fix had traded a false positive for a miss Round 6 re-derived the guard's answer from a TypeScript AST instead of trusting the regex, and caught two things. P1 -- the nested-template fix I shipped in round 5 introduced a worse bug than the one it closed. Switching to "code mode" inside `${...}` without also resetting the quote at a newline meant an apostrophe in a regex literal -- `` `'${value.replace(/'/g, "'\\''")}'` `` , which is exactly the shellQuote shape all over this codebase -- inverted the lexer for the rest of the file. `claude-accounts/service.ts` went blind from line 96, hiding a REAL unguarded `spawn` at :1097: the WSL Claude managed-login path, which opens a console and steals foreground on Windows. Round 5 traded one false positive for one false negative and I did not notice, because the offender count went down. The blanker now resets non-backtick quotes at a newline (the rule stripComments already had) and tracks brace depth per interpolation. The spawn is fixed rather than allowlisted, and the count is 69 -- the number the AST predicted. P1 -- the ownership-timeout guard was dead code: it threw into its own `catch` three lines below, which returned null, which the caller persists as "not owned" and clears the user's account selection. Now a typed sentinel the catch rethrows. P2 -- `WslGuestEnvironmentUnavailableError` reached the UI verbatim from the CLI installer and the Codex availability check. Both mapped. Method note: I had been regenerating the allowlist with a Python transcription of the scanner, and the two drifted -- the same two-implementations problem this workstream keeps finding. The allowlist is now generated by running the shipped test with an empty list and taking what it reports. * fix(guards): stop patching the lexer -- make the scanner fail closed instead Round 7 proved my round-6 fix also did not work, by planting a plainly-named unguarded `spawn` in `claude-accounts/service.ts` and watching the guard pass 3/3. That is three consecutive attempts at an exact lexer, each shipping a desync that hid real calls, and each time the offender count went DOWN, which I read as progress. Round 6's diagnosis was wrong too: the culprit is the `templates` brace-depth stack, which nothing resets, not quote state. So stop trying to be exact. `blankStringContentsDesynced` reports when the lexer lost its bearings, and the guard treats that as an offender. Over-reporting is a nuisance; under-reporting is a false clean, and a false clean is what let a real console-flash spawn out of the ratchet twice. The allowlist goes 69 -> 82: the 13 extra are files whose scan cannot be trusted, now named rather than assumed fine. The planted violation is now caught. Also from round 7: - `SPAWN_CALL` missed promisified and renamed bindings, so `exec('where gemini')` (a real Windows cmd.exe spawn) and a detached `shell: true` in `cli/runtime/launch.ts` were invisible. Added execAsync/execFileAsync/ execFileCb/spawnDetached. - `BASHISM` matched `set -o pipefail` but not `set -euo pipefail`, which is the only spelling this tree uses -- so the check could not have caught the #14292 signature it exists for. Fixed, and it immediately flagged a file; that one turned out to be a comment, so the bashism scan now strips comments too. - The CLI installer error mapping my round-6 commit claimed was "both mapped" was never applied -- only the Codex side had been. Now actually mapped. * fix(guards): close the four holes round 8 found by planting violations Round 8 stopped reasoning about the guard and planted spawns into it. Four holes, none of which reading had found: - `windowsHide: false` **passed**. The check was `args.includes('windowsHide')`, a substring test. Now matches `windowsHide: true`. - A ternary first argument was silently skipped: the method-declaration filter `/^\(\s*\w+\s*[:?]/` also matches `exec(useAlt ? 'a' : 'b', …)`. Now requires a type after the colon. - Renamed bindings were not covered, despite the comment I wrote saying they were -- I had hardcoded three names. Aliases are now resolved from the import. Each is verified closed by planting it and watching the guard fail. `fork` is deliberately still unscanned. Round 8 is right that Node forwards the option, but `ForkOptions` does not declare it, so the two live sites cannot be fixed without a cast. Recorded in the verification doc rather than left as a silent gap, along with two others worth knowing: the allowlist is file-granular, so its ~18 false-positive entries carry a standing pre-approval for real regressions in those files and cannot be retired by fixing code; and `stripComments` has no desync report, so the fail-closed check is only half applied. The doc now also says how to verify a guard change: plant a violation. Every guard fix here that was verified by reading was wrong. * fix(wsl): stop preflight reporting installed CLIs as absent on a slow distro Round 9's merge blocker, and the sharpest finding of the whole workstream: the branch built to close #9725 had reopened it from the other side. `preflight-wsl-command.ts` was one of five sites without `allowDegradedEnvironment`, so a guest-PATH probe failure threw. Every consumer collapses a throw into a verdict: `isCommandAvailable` and `isCommandOnPath` catch to `false` ("not installed"), `isGhAuthenticated` and `isGlabAuthenticated` read an empty payload as "not authenticated". So a slow distro made WSL git, gh and glab read as missing. Two things made it likely rather than theoretical. The probe took two thirds of a 5s budget, leaving the command ~1667ms where main gave it the full 5s inside its own login shell -- a cold WSL VM start routinely lands in that band. And a probe timeout is cached for 30s with a re-probe threshold of 1.5x the failed budget, which a 5s caller can never clear, so every preflight command short-circuited without spawning wsl.exe at all -- and Re-check does not invalidate the cache. Fixes: preflight degrades instead of refusing, and the probe is capped at half the caller's budget and at 4s, so no caller ends up with less time than it had before the runner existed. Also fixes a real console flash found on the way: `preflight-command-exec.ts` spawns git/gh/node through `promisify(execFile)` with no `windowsHide`. Round 9 also confirmed the credential paths are now *safer* than main: all 11 account sites degrade, every destructive guest operation is still marker-gated, and main's `getOwnedManagedAuthPath` could disown an account on a 5s timeout -- which this branch turns into a failed launch instead of a destroyed selection. * fix(wsl): make "Try again" able to succeed, and test the round-9 fix Round 10 returned MERGE with one residual worth closing first. A transient probe failure left the null-resolving promise in `inFlight`, so the only way back was `retryAfter` -- and the 4s probe cap made the 1.5x budget escape unreachable, because no caller can pass more than 4s. For the full 30s window the four non-degrading sites returned their error *without spawning wsl.exe at all*, and each of those errors says "Try again". The advice was guaranteed to fail. The entry is now dropped on a transient outcome and an explicit cooldown gate replaces it, so the window alone decides. The window drops 30s -> 5s: long enough to stop a stampede, short enough that the user's next click reaches a distro that has since warmed up. Round 10 also noted the round-9 fix shipped untested, which was fair. Added: the probe-budget floor for 5s/8s/10s callers, and preflight's degrade opt-in plus its stdout/stderr-carrying rejection, which isGhAuthenticated reads off the caught error as an auth-success fallback. * test(wsl): make the probe-budget guard actually guard Round 11 caught that the regression test I added for the probe cap did not bind: it seeded the guest environment, so the probe resolved in ~0ms and the assertion read the command leg's timeout instead. Reverting the cap to the old 2/3 split left all three cases green. Dropping the seed and asserting on the probe leg fixes it -- verified by reverting the cap and watching all three fail. A regression guard that cannot fail is the shape that has cost the most in this workstream: the windowsHide guard silently passed a real unguarded spawn twice for the same reason. |
||
|
Neil
|
057fbfcffc |
perf(windows): read the process table natively instead of forking PowerShell (#15749)
* perf(windows): read the process table natively instead of forking PowerShell Seven independent readers each forked powershell.exe to run Get-CimInstance Win32_Process, with a wmic fallback that Windows 11 24H2 has removed. On a domain-joined host with PowerShell Transcription enabled by policy, one of them running every ~2s recorded ~289GB across 1.4 million files (#15209). The same scan cost ~700ms and ran per pane (#15036), and a Group Policy or AV block turned it into 'unavailable', which callers read as 'no evidence' -- which is how a PTY tree survives its own teardown (#9045, #10475). A Toolhelp32 snapshot answers the same question with no child process. Measured on Windows 11 with 1050 processes, p50/p95: pid+ppid+name 15.9 / 17.5 ms +memory +command line 30.6 / 33.7 ms Get-CimInstance 706 / 723 ms Two upstream defects needed patching, both found by running it on real hardware. The binding requires Spectre-mitigated libraries our agents do not carry (node-pty is patched the same way). And enumeration stopped after 1024 processes: on a host with 1051 the module returned exactly 1024, and the querying process was itself among the 27 missing -- a truncated snapshot silently hides the descendants teardown is looking for, which is the failure this whole change exists to remove. Migrated: the foreground/descendant reader (the #15209 scraper and the teardown identity gate) and the port scanner's PID attribution. NOT migrated: the memory collector and three identity probes, which need Win32_Process.CreationDate and have no native equivalent. Start time is a proxy for identity anyway; an inherited job handle is the real answer, so those belong with the job-object work rather than here. Packaging follows the windows-native-registry contract exactly: optional, absent from onlyBuiltDependencies so macOS/Linux never run node-gyp, win32-only in the packaged runtime. Asserted by the existing contract test, which also stops pinning a whole source literal that only tested its own formatting. * chore(process): ratchet the child_process allowlist down windows-foreground-process-rows.ts no longer spawns anything, so its allowlist line is stale. The guard fails on a stale entry as well as a new one, precisely so a migrated file cannot keep a slot open and hide the next regression in the same path. * fix(ports): import the process-table reader the scanner uses Missing import: the migration replaced the PowerShell call but the new symbol was never imported, so tsc failed. Vitest transpiles without typechecking, which is why the port-scanner suite stayed green. * fix(deps): sync this branch's lockfile with its patch set Same class as the fix on the tip branch: pnpm records a hash per patched dependency, and this branch introduces the windows-process-tree patch without its lockfile entry matching. Every job here failed at install with ERR_PNPM_LOCKFILE_CONFIG_MISMATCH. Verified with --frozen-lockfile, which is what CI runs and what my local runs were not. * test(relay): drive the relay's Windows fixtures from the native snapshot Two relay cases fed a PowerShell CIM payload through a mocked execFile. That reader is gone, so both failed -- deterministically, on every PR run for this branch and the one above it. I did not catch it because my own verification sweep was 'src/main src/shared config/scripts' and never included src/relay. The relay is a first-class consumer of the process table; leaving it out of the sweep is how a deterministic failure survived six review rounds. |
||
|
Brennan Benson
|
5ca747dad0 |
docs(ssh): state the SSH execution boundary and pin the liveness vocabulary (#14971)
* docs(ssh): state the SSH execution boundary and pin the liveness vocabulary Nothing under docs/ described how work splits between the client and an SSH host, so agents and humans inferred it from error strings and got it wrong: loss of contact was repeatedly reported as process death, which orphaned live remote agents and cold-started duplicates over the same worktree. Pins the vocabulary to the incumbent live/unverifiable/exited verdict from unstopped-pty-verification so no synonym is introduced, records the one real discriminator (all of a host's terminals drop together on link loss; one alone means process exit), and lists the outstanding gaps with citations. Tracked via the docs allow-list and linked from AGENTS.md, per the convention in .gitignore. * docs(ssh): cite the live restoreRequired site after it moved The throw now lives in reattachSshPtySessionForSpawn; ssh-pty-provider.ts no longer contains it. Caught by the worker fixing it, against a newer main than the audit ran on. * docs(ssh): require host evidence for liveness verdicts * docs(ssh): keep boundary references stable * docs(ssh): fence liveness evidence to its host identity * docs(ssh): state replay and environment boundaries precisely * docs(ssh): correct replay and platform boundary claims * docs(ssh): describe headless runtime continuity accurately * docs(ssh): distinguish authority from client metadata * docs(ssh): describe pending fixes accurately * docs(ssh): date the gap list and name the PR that closes each entry The Known gaps section was accurate when written and becomes actively misleading as its fixes land: it told a reader to go fix restoreRequired, the missing unverifiable verdict, and the absent terminal-list host field, three things now addressed by #14974, #14977 and #14973. Mark the section as dated, require verification against current code before acting on any entry, name the PR per entry, and move landed items out. Also correct the two body claims that the landed fixes invalidated. The rules above are durable; only this section rots. * docs(ssh): make the boundary doc a durable ruleset, not an incident record The Known gaps section was 18 of 93 lines enumerating specific defects from one investigation, several already fixed by sibling PRs in the same batch. A reference doc that needs a 'this section rots' warning is telling you the section belongs somewhere else; those entries belong in issues. Replace the six-row table of currently-lying signals with the method that outlands any particular bug: ask whether the owning host produced the signal, whether every PTY on the target went quiet together, whether the termination event matches the current incarnation and generation, and whether a returned status is actually a claim. Same for artifacts - state what ls-remote and a PR head each do and do not prove, rather than listing which command is currently wrong. Nothing here goes stale when the open fixes land. |
||
 OrcaWinandOrcaWin
|
3a9f40ed70 |
fix(wsl): read machine output from a fenced login shell (#15290)
* fix(wsl): read machine output from a fenced login shell Orca runs WSL reads through the distro's *interactive* login shell so PATH matches the user's own terminal (nvm, mise and asdf only install into rc files interactive shells read). An interactive shell also runs the distro's rc/motd, and stock Ubuntu 24.04 writes its "run a command as administrator" hint to stdout -- no user customization required. Every caller parsing that stream was reading the banner as data: statPath -> "To run a command as administrator...\n\ndirectory" readPath -> banner prepended to the contents of every file read preflight -> banner prepended to `gh --version` / auth output `.trim()` cannot recover any of these, so a WSL worktree's file explorer sees no valid entry types and file reads return junk. Three call sites had independently grown their own marker to survive this (`__ORCA_AGENT_PATH__`, `ORCA_WSL_GIT_READ_ENV_V1`, and a `>/dev/null` fd dance), which is the tell that it belongs in one place. Fence the payload once, in the shared builder, and hand callers a reader that returns just their bytes. The fence carries a per-call nonce so `cat`-ing a file that happens to quote a marker is not truncated. Exit status is preserved, so the ENOENT mapping still works. wsl-git-read-environment drops its bespoke marker and parsing. * test(wsl): fence the login-shell path-lookup boundary test It asserted a raw interactive login-shell read matched an absolute path, so the distro rc banner made it fail on any stock Ubuntu. It is part of the shell-contracts CI gate, where it skips on Linux and hid the break. * docs(wsl): record the guest command-execution contract Both failure modes are silent - the command runs, exits 0, and returns the wrong bytes - so the rules need to live somewhere a reader will find them before writing the next wsl.exe call site. * fix(codex): fence the WSL Codex identity probe buildWslCodexBinaryStamp reads the login shell's stdout positionally -- path before the first newline, version after -- through an interactive login shell. On a stock Ubuntu the rc banner lands ahead of the payload, so the first newline falls inside the banner and the stamp becomes path="To run a command as administrator..." with the rest as version. Both halves are non-empty, so nothing throws: the stamp is silently wrong, and an unstable stamp reads as "the Codex binary changed" and reissues the trust grant. The identity script ends in `exec`, so it never writes a closing fence; the reader returns everything after the opening one, which is exactly this case. buildWslCodexIdentityArgs becomes buildWslCodexIdentityProbe and returns the reader with the argv so the two cannot drift apart. The other three WSL Codex commands are deliberately left unfenced: availability is exit-code only, and app-server/login hand stdout to a long-running program. * fix(wsl): harden the capture fence after review - readStdout now takes the LAST opening fence, matching the lastIndexOf the wsl-git-read-environment marker used deliberately: a login shell can echo the command text before running it, repeating the fence. - local-worktree-filesystem throws instead of falling back to raw stdout when the fence is missing. The fallback silently reinstated the bug being fixed -- statPath would return the banner as a file type and readPath would return banner+contents, with no signal. Preflight keeps its fallback; its matchers scan the whole blob and tolerate a prefix. - The exit-status test asserted only that the script CONTAINS `exit $?`, which is true for any input and never executed those lines. It now runs a real distro and asserts status 2 reaches the caller, which is what statPath's ENOENT mapping depends on. - Corrected the doc: a sed backreference has no `$`, so `--` never rewrote it. Replaced with the positional and shell-local cases that were measured to differ. * fix(wsl): stop running a login shell for filesystem reads statPath/readPath/rm run coreutils at standard paths and shell builtins. They need nothing from the user's PATH, so there was never a reason to start a login shell -- and starting one is what put the distro's rc/motd on the stdout these callers parse. Fencing that output treated the symptom. Using a plain `sh -c` removes the cause: no profile, no rc, no banner, by construction. The fence and its missing-fence error go away with it. The fence stays where it is actually needed: the three places that must run the user's shell to resolve their PATH (the preflight CLI probe, the WSL git environment probe, and the Codex identity probe). Net -12 lines. --------- Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> |
||
|
Neil
|
49752477a6 |
build(xterm): restore the patch regeneration harness and gate it in CI (#15223)
* build(xterm): restore the patch regeneration harness and gate it in CI docs/reference/ime-architecture.md says "Never hand-edit the bundles in the patch" and links to docs/reference/xterm-patch-regeneration.md. That doc does not exist, and neither does the harness it describes. Both landed in |
||
|
Neil
|
24e662adc1 |
feat(ssh): verify host keys, and restore panes correctly across a reconnect (#14844)
* docs(ssh): design for real host key verification (STA-4319)
Today's ssh2 verifier records a fingerprint and returns true — every host key is
accepted, with no known_hosts consult and no change detection anywhere in
src/main/ssh/. Scope is per-connection, so exec, SFTP, port forwarding, the
watcher and relay deploy all ride that one unverified handshake, and the
ProxyJump path puts the final hop — the topology most likely to cross untrusted
network — on ssh2 specifically.
Decisions worth calling out:
- Read the user's known_hosts as a trust source but NEVER write to it. That file
is shared with every other SSH tool on the machine; appending means line
endings, permissions, concurrent writers and a corruption blast radius well
beyond us. Accepted keys go to our own per-target store. Reading theirs is also
the entire migration story: most developers already have their hosts there.
- Mismatch is scoped to the SAME key type. A host with only an RSA entry that
presents ed25519 is unknown, not changed. ssh2 negotiates ed25519 first, so
without this we would fire a change-of-key alarm at nearly every existing user
on their first upgraded connect — training them to dismiss the one warning that
is supposed to mean something. Flagged in review as the decision I am least
sure of; a downgrade-vector argument against it is being tested.
- Changed key hard-fails with no override button; recovery is a separate explicit
action, offered only when OUR store is what disagreed, because forgetting our
record cannot unblock a known_hosts conflict.
- Background reconnects deny rather than prompt. A dialog the user cannot place
in context only teaches click-through.
Two traps are documented because either would make the fix silently do nothing:
an async verifier returns a Promise, which ssh2 reads as truthy and accepts
immediately; and the existing test mock invokes hostVerifier with one argument
and ignores the return, so it would pass against a verifier that never decides.
Design only — no behaviour change. The doc is added to the tracked-reference
allowlist in .gitignore alongside the other docs/reference entries.
* docs(ssh): revise the host key design after security and migration review
Three things the reviews changed, kept visible rather than quietly edited out.
THREAT MODEL WAS WRONG IN THREE PLACES. Jump hosts are not the worst case — they
are already safe: shouldUseSystemSshTransport branches on exactly the inputs
resolveEffectiveProxy does, and attemptConnect returns after the system probe, so
ProxyJump goes through OpenSSH and is verified. Agent forwarding was overstated
(gated on the user's ForwardAgent). Credential theft was understated: any auth
error counts as agent fallback, so a MITM walks the user to the password AND
private-key passphrase prompts, and cachedPassword replays without prompting. The
relay claim was backwards — the attacker owns their own machine; the real impact
is the return direction, where they become the host our workspace trusts.
TYPE SCOPING IS A DOWNGRADE VECTOR WITHOUT ALGORITHM ORDERING. This was the
decision I flagged as least certain and asked to have argued both ways. OpenSSH
is safe only because order_hostkeyalgs() puts known types first and RFC 4253
gives the client's order priority. ssh2 negotiates ed25519 first regardless, so
an attacker who cannot forge the RSA key on file just presents ed25519 and gets a
friendly first-contact prompt instead of a hard failure. Keep scoping, but set
algorithms.serverHostKey to lead with the types on file — and add a sixth
outcome for 'unknown type, known host', which must never read as first contact.
SHIP THE DEFENCE BEFORE THE DIALOG. Startup restore fires eager connects for all
targets in parallel with a 15s timeout while a prompt would live 120s; ephemeral
VM targets present a new key every launch; paired-web connects run on the host
desktop, so the dialog opens on someone else's screen. Phase 1 is therefore no
modal at all: consult known_hosts and our store, match connects, unknown persists
with accept-new semantics, mismatch and revoked hard-fail. That is the whole MITM
defence with none of the migration risk.
Also folded in, verified live against OpenSSH 10.2p1: the without-port fallback
(bracketed lookup first, then bare, where the second pass can only yield match or
unknown — otherwise a bare line plus a non-default port produces a spurious
prompt); hashed entries hash the candidate form; multiple files union; a
cert-authority line does not match a plain key. IPv6 and bracket parsing moved
INTO scope — that is a parser requirement, not a scope call, and getting it wrong
produces the prompt-training harm the design exists to avoid.
* feat(ssh): parse and match OpenSSH known_hosts
The matcher half of STA-4319. No behaviour change yet — nothing calls this.
Hand-rolled because no maintained JS implementation exists, and written against
behaviour observed from OpenSSH 10.2p1 rather than inferred from the man page.
Three of those behaviours a reasonable reading gets wrong:
- A non-default port is TWO ordered lookups, not one candidate set: '[host]:port'
first, then bare host ('checking without port identifier' in ssh -v). The
fallback pass can only yield match or unknown — OpenSSH downgrades a wrong key
there rather than reporting a change. Collapse them and anyone holding a bare
line who connects off-port gets a spurious first-contact result; treat the
fallback as authoritative and they get a false change-of-key alarm.
- Revocation resolves in its own pass so the verdict cannot depend on line order.
Verified both orderings.
- A cert-authority line never matches a plain host key; it only validates
certificates. A normal line alongside it still decides.
Mismatch is scoped to the same key type, and a host known by a DIFFERENT type
returns unknown-type-known-host rather than plain unknown — an attacker who
cannot forge the key on file must not get a friendly first-contact result by
presenting another type. That outcome is only half the defence; the other half
(leading serverHostKey with known types) lands with the wiring.
47 tests from vectors executed against real sshd, including ssh-keygen -H hashed
entries. Each of six mutations reddens it: collapsing the passes, letting the
fallback report mismatch, dropping type scoping, resolving revocation in line
order, honouring an unrecognised marker, and skipping the blob/type agreement
check.
* feat(ssh): decide what to do with a presented host key
The policy half of STA-4319, kept separate from the ssh2 wiring so it is testable
without a handshake and injected rather than importing its sources, so a test
states its own trust state instead of writing files.
Phase 1 ships no dialog — a test asserts the decision is never 'prompt'. Startup
restore opens every previously-active target at once, ephemeral VM targets would
ask every launch, and paired-web connects run on the host desktop where the
dialog would appear on someone else's screen.
Ordering that matters: revocation outranks everything including
StrictHostKeyChecking=no, because a revoked key is a statement that this key is
known-bad rather than merely unrecognised. known_hosts is named before our own
store on a change, because its remedy (ssh-keygen -R) is the one that also
unblocks ssh and git — pointing at a remedy that cannot work is worse than none.
Two carve-outs with reasons: an ephemeral runtime target accepts WITHOUT
recording, since a fresh VM presents a new key every launch and a stored record
would accumulate per launch and eventually read as a spurious change; and when
ssh -G ran on the HOME-divergent path that suppresses /etc/ssh/ssh_config, an
unknown host is denied, because a site-wide policy may forbid it and being laxer
than ssh is the one outcome that is never acceptable.
Rejection text deliberately avoids 'authentication failed' and 'permission
denied': the reconnect ladder classifies on those substrings, so a denial phrased
that way is retried forever against a decision that will never change. Pinned by
a test.
* feat(ssh): build the host key verifier and the algorithm order that makes it safe
Still not wired into the handshake — that lands next. This is the piece that
turns a decision into an ssh2 callback, plus the half of the design that is easy
to forget because it lives in a different config field.
The verifier MUST be a plain function returning undefined. ssh2 does
'const ret = verifier(key, verify); if (ret !== undefined) verify(ret)', so an
async function returns a Promise — neither undefined nor falsy — and ssh2 accepts
the key immediately while ignoring whatever the callback later decides. Making
this async would silently restore exactly the accept-everything behaviour the
module exists to remove, so a test asserts the return value is undefined.
orderServerHostKeyAlgorithms is what makes type-scoped matching safe rather than
a downgrade. RFC 4253 gives the client's algorithm order priority, so leading
with the types we already hold for a host denies a server the choice of
presenting some other type to convert a hard failure into first contact. Without
it, an attacker who cannot forge the key on file just offers a different
algorithm. Revoked entries never contribute to that order.
Also fails closed on two paths that would otherwise hang or over-trust: a key
whose own length-prefixed header cannot be read is refused rather than reasoned
about, and a throw from any dependency denies, because ssh2 may not catch an
exception raised inside the verifier and the handshake would hang instead of
failing.
18 tests. Includes the two negative cases that matter — first-contact keys are
recorded, but keys we already know, rejected keys, ephemeral runtime targets and
a lax StrictHostKeyChecking are not.
* fix(ssh): promote every RSA signature algorithm for a known ssh-rsa key
A known_hosts entry names the KEY type, which is not the negotiated ALGORITHM
name. One ssh-rsa key is offered as rsa-sha2-512, rsa-sha2-256 or ssh-rsa
depending on the signature algorithm, so matching the literal name only would
leave a host we know by RSA ordered behind ed25519 — precisely the ordering this
function exists to prevent, and precisely the population (RSA-era known_hosts
entries) it was written for.
Verified from ssh2's own negotiation while wiring this: kex.js iterates the
CLIENT list and takes the first entry the server also offers, so client order
does decide, as RFC 4253 says. ssh2's default order leads with ed25519 and places
the RSA algorithms fifth through seventh.
* fix(ssh): verify host keys instead of accepting every one (STA-4319)
The actual fix. ssh-connection's verifier recorded a fingerprint and returned
true, so every ssh2 connection accepted every host key — no known_hosts consult,
no change detection. It now consults the user's known_hosts plus our own store
and refuses a changed, revoked or unverifiable key.
Phase 1 by design: no dialog. Unknown hosts are accepted and recorded
(accept-new semantics), because startup restore opens every previously-active
target at once, ephemeral VM targets present a new key each launch, and
paired-web connects run on the host desktop where a prompt would appear on
someone else's screen. The MITM defence lands now; the prompt is Phase 2.
Also sets algorithms.serverHostKey to lead with the types already known for the
host. Without it the type-scoped matching is a downgrade — an attacker who cannot
forge the key on file just presents another type and turns a hard failure into
first contact. Verified from ssh2's kex.js that the client list decides.
Denial replaces ssh2's generic handshake error with the specific reason, because
the reconnect ladder cannot distinguish a generic failure from a transient fault
and would retry forever against a decision that will never change.
An unreadable trust store degrades to known_hosts only rather than failing the
connect: a changed key is still refused, and a host trusted only by us falls back
to first contact and is re-recorded, reaching the same decision.
The ssh2 mock now uses the callback form and aborts the handshake on denial. As
written it called hostVerifier(key) with one argument and ignored the result, so
it would have passed against a verifier that never decides — flagged in the
design as a mock that had to change, not a test to quietly rewrite. Two new tests
pin the wiring rather than the module: an unidentifiable blob is refused, and a
well-formed key is accepted.
Note for review: commit
|
||
|
Brennan Benson
|
7b4e10b104 |
fix(mobile-ios): pin fastlane and gate the Fastfile in CI (#15092)
* fix(mobile-ios): pin fastlane and gate the Fastfile in CI The ios-distribute job failed on every run from 2026-08-10 to 2026-08-13 because distribute_testflight passed distribute_only without app_platform, so pilot fell through to an interactive platform prompt on ubuntu. No CI check loads the Fastfile, so external testers got nothing for six days. - Pin fastlane 2.238.0 and commit mobile/Gemfile.lock so ios-build (macos) and ios-distribute (ubuntu) cannot resolve different versions ~25 minutes apart. Fixes the Gemfile comment's dead mobile-build.yml reference. - Add a Fastfile smoke check (bundle exec fastlane lanes) plus a static contract test for the TestFlight lane arguments to Mobile Checks. - Set reject_build_waiting_for_review so a superseded same-train build in beta review stops blocking the submission. * fix(mobile-ios): install the pinned Gemfile.lock in frozen mode Without frozen, a lockfile that drifts from the Gemfile is silently re-resolved per job, which is the version split the pin exists to prevent. * test(mobile-ios): anchor the TestFlight argument contract against an empty selection * chore(mobile-ios): canonicalize the lockfile platforms Bundler's own normalization drops arm64-darwin-25 as redundant with the versionless arm64-darwin, and the ubuntu runners resolve x86_64-linux-gnu. |
||
|
Jinwoo Hong
|
fa9b20cb41 | feat(skills): reland private bundle sharing safely (#14934) | ||
|
Jinjing
|
763b1febeb |
Revert "feat(skills): add private bundle sharing (#14401)" (#14913)
This reverts commit
|
||
 Jinwoo HongandE2E Test
|
757fae28d7 |
feat(skills): add private bundle sharing (#14401)
Co-authored-by: E2E Test <e2e@test.local> |
||
|
Neil
|
eb22e497bb | Revert "fix(ssh): reapply the reattach-identity work and stop the fallback fence stranding moved panes" (#14395) | ||
|
Neil
|
6a0c8fa541 |
fix(ssh): reapply the reattach-identity work and stop the fallback fence stranding moved panes (#14384)
* Reapply #13326 and #13928 (un-revert #14361) Restores the SSH reattach-identity and daemon-occupancy fixes. Reverting them reintroduced their P0s, filed as STA-4224, STA-4225, STA-4227, STA-4230, STA-4232, STA-4233 and STA-4234 against #14361. The tab loss that motivated the revert is fixed in the commits that follow, so this reapplication is not a straight redo. * fix(relay): stop the fallback attach fence refusing a pane that moved tabs The primary fence was moved to the shell's own incarnation precisely because paneKey/tabId froze the pane's LOCATION at spawn and refused panes that had merely moved. The fallback that older clients fall into kept the old rule, so the correction never reached it — the same 'the rule exists, but this path does not ask it' leak this work has hit repeatedly. A refusal here is not recoverable: an identity mismatch never grounds a respawn, so the pane keeps a live shell it can no longer reach and renders blank. Narrowed to paneKey, which is the identity; the tab is a location. Restoring the tabId comparison reddens the new test. |
||
|
Neil
|
11cd2b4310 |
revert(ssh): back out #13326 and #13928 — reconnect loses every tab (#14361)
* Revert "fix(daemon): stop killing live coding agents when the daemon can't report its sessions (#13928)" This reverts commit |
||
 NeilandOrca
|
3ab8b6a117 |
fix(ssh): stop SSH reconnect from multiplying terminals and resuming agents twice (STA-3077) (#13326)
* fix(ssh): stop reconnect from grafting panes and stacking remote leases Reconnecting an SSH-backed workspace added terminal panes the user never opened, and the remote host accumulated shells nobody was using — one report went from 2 to 19 to 20 relay PTYs across three reconnects (STA-3077). Two root causes, both in the store. Reattach could create UI. `persistPtyBinding` has four creating branches — mint a tab, mint a root leaf, split the root and graft a leaf, mint a layout. All four are load-bearing for `pty:spawn`, which can beat the renderer's debounced layout writer, but none of them is appropriate on reattach, where the pane either already exists or is gone for good. Add `mayCreate`, defaulting true so the spawn path is untouched; every creating branch already sets `terminalMembershipChanged`, so refusing is a check rather than a new code path. Lease identity had no pane key. `upsertSshRemotePtyLease` matched on `(targetId, ptyId)` alone, so a pane that re-leased under a new relay id left its predecessor live with nothing to retire it, and the next reattach fanned out over both. One pane now keeps at most one live lease. Superseded leases are marked `expired` rather than terminated: losing a lease is not proof the shell died, so the remote process is deliberately left running. Tests assert observable behavior rather than mechanism, so they stay valid under any implementation that fixes this. Co-authored-by: Orca <help@stably.ai> * docs(terminal): record the terminal session behavior contract Properties stated as observable behavior rather than mechanism, so an oracle written against them survives a change of implementation. Records the weaker, correct form of the timer rule — a timer may never be the sole cause of a destructive action — because recovery budgets and scratch-file age gates are correct code that an absolute ban would condemn. Also notes which mechanisms are deliberately not required, so each has to earn its place rather than arrive with an architecture. Co-authored-by: Orca <help@stably.ai> * fix(ssh): heal duplicate pane leases that predate pane-keyed supersession Pane-keyed supersession stops new duplicates, but it does nothing for installs that already carry the ones STA-3077 accumulated — the report behind this reached 20 live leases across a handful of panes, and every reconnect fanned out over all of them. Retire the stale duplicates once per reattach pass, keeping the newest lease for each pane under a total order so two hosts resolve a tie the same way. As with supersession, retired leases are marked `expired` rather than terminated: their remote shells are deliberately left running, because a lease we chose not to revive is not evidence the shell died. The relay-session store stubs gain the new method. Note the gap this leaves open: those shells keep running and are no longer reachable from the app, so the "accumulates unused shells" half of the report needs a visible recovery surface rather than a silent kill. Co-authored-by: Orca <help@stably.ai> * fix(terminal): stop respawning a shell that is still running A pane that failed to reattach spawned a fresh shell. Because the restored session id came along, the replacement resumed the same agent session, and two processes appended to one transcript — reported repeatedly, up to five concurrent resumes of a single session. Two defects fed it. The relay reported a source that merely needed re-establishing as `SSH_SESSION_EXPIRED`. The shell was still running; only its output source was gone. Give that outcome its own error so it stops reading as "the session no longer exists". The reattach failure handler then treated every error as proof of death. It checked for expiry and, in the else branch, took the identical action — so the check bought nothing and a transport fault, a timed-out call, or a wedged relay all respawned. Respawn now requires proof: an explicit host expiry or a not-found PTY. Anything else, including an error we have never seen before, is unresolved, leaves the shell running, and keeps the binding for a later reattach. Two existing tests asserted the old behavior. One threw a bare error as scaffolding to reach the spawn-adoption door; it now throws proof, which is what it meant. The other pinned the expiry mapping itself, and now asserts the outcome fails closed *without* being reported as expiry. Co-authored-by: Orca <help@stably.ai> * docs(terminal): record what makes a retention bound safe Shortening a grace period is the wrong lever. Measuring process time and gating reclamation on an independent observation are what make one safe, and they are what deployed systems actually do. Also records that lifecycle belongs in the attach reply rather than a delivered event — that is what removes the need for a durable per-consumer cursor to guarantee an exit is never lost. Co-authored-by: Orca <help@stably.ai> * test(terminal): assert the empty-failure case without an empty Error A thrown empty value exercises the same property — a failure carrying no usable message is not proof the session is gone — and does not trip the empty-error-message lint. Co-authored-by: Orca <help@stably.ai> * fix(ssh): let the durable pane binding outrank recency when retiring leases Choosing the newest lease for a pane is wrong whenever a newer lease exists that no pane is bound to: it retires the lease the pane is actually attached to, detaching a live terminal instead of healing it. Two changes. Arbitration now prefers the lease matching the pane's durable binding, across both the SSH-target and local partitions, falling back to recency only when no binding names either candidate. And supersession at upsert time now defers rather than expiring a bound predecessor. When a lease arrives for a pane that is still bound to a different PTY, the binding has not caught up yet, so both stay live and reattach arbitrates once the binding is available. Co-authored-by: Orca <help@stably.ai> * fix(ssh): roll back a lease retirement whose durable write fails `flush()` logs and swallows write errors, so a failed write left these leases retired in memory while disk still called them attached — and the pane bindings scrubbed alongside them stayed scrubbed. Use `flushOrThrow` and restore both the lease states and the affected session partitions when it throws, reporting nothing retired. Co-authored-by: Orca <help@stably.ai> * test(ssh): prove pane and remote PTY cardinality across reconnects Counts the shells the relay actually hosts, on the container, rather than inferring them from app state — that is the census the report was based on. Asserts the PIDs are unchanged, not merely the count, so a kill-and-respawn cannot pass. Every pane streams before the transport is severed: an idle pane sends no recovery checkpoint, so only a live source comes back needing re-establishment, which is the outcome that used to read as expiry. Co-authored-by: Orca <help@stably.ai> * fix(ssh): actually pass mayCreate:false from the reattach binding write The `mayCreate` guard was correct and had no production caller, so the reattach path still went through the creating branches and grafted panes back. `restoreReattachedPtyRuntime` is that call site — RC3 in the original diagnosis — and it now refuses to create. Binding moves ahead of runtime registration, because registering first would surface a pane the user never opened before the refusal landed. A refusal leaves the remote shell running and reattachable; a *thrown* write stays unknown and still registers, so a failed disk write cannot detach a live pane. Adds an oracle over the call site itself. The store-level tests all passed while the fix was inert, because they called the store directly — only pinning the wiring catches that. Co-authored-by: Orca <help@stably.ai> * fix(terminal): apply the respawn-requires-proof rule to both reattach paths connectPanePty has two near-verbatim reattach blocks — one keyed on the deferred SSH session, one on the restored session — and only the second was fixed. The first still checked for expiry and then respawned unconditionally anyway, so a transport fault there resumed the same agent session a second time. Also keep the wire token out of the pane. The main-process bridge only special-cases expiry, so a source-restore failure crossed IPC as raw `SSH_SOURCE_RESTORE_REQUIRED: <id>` text and surfaced to the user. It correctly does not respawn; it just should not read like that. Co-authored-by: Orca <help@stably.ai> * test(ssh): state plainly that the reconnect spec is a forward guard It was run against an unfixed tree and passed, so it does not prove the STA-3077 fixes and should not be read as if it does. A clean severed transport does not reproduce the field conditions — accumulated duplicate leases, or a source returning needing re-establishment. It keeps its place as a forward guard: it counts the shells the relay actually hosts and pins their PIDs, so a later change that grafts a pane or respawns a shell fails here. Co-authored-by: Orca <help@stably.ai> * docs(terminal): record that a guard must be pinned at its call site A refusal that exists and is never passed is indistinguishable from no refusal, and store-level tests cannot tell the difference — they call the store directly. Learned from `mayCreate`, which was correct and had no production caller for several commits. Co-authored-by: Orca <help@stably.ai> * fix(ssh): park one PTY's exhausted delivery recovery instead of dropping the channel A per-PTY recovery budget running out disposed the whole relay channel, so one PTY that could not re-prove its delivery aborted every in-flight filesystem and git request on that host and stalled every sibling pane. A retry count is not proof of anything, and it certainly is not proof about the other sessions sharing the channel. Exhaustion now parks that PTY's delivery. The remote shell keeps running, its lease stands, and the next relay open reattaches it with a fresh delivery generation — the parked state is cleared on teardown and the generation changes on reconnect, so a reconnect recovers it. The consecutive-attempt ceiling goes away entirely; the per-generation one is what bounds the retry cost, and the second ceiling only existed to reach the channel drop sooner. Tradeoff worth stating: the failing pane used to self-heal within seconds because the forced reconnect wiped all rejection state, and it now stays frozen until the next relay open. That is a worse outcome for that one pane and a much better one for every other session on the host, and reconnecting is user-reachable. Co-authored-by: Orca <help@stably.ai> * fix(pty): let liveness say unknown instead of forcing it to say dead `IPtyProvider.hasPty` returned a boolean, so a provider whose inventory was empty for reasons that have nothing to do with the session — socket down, cache never hydrated, provider generation just constructed — had no way to say so and answered "absent". Its own siblings already knew better: `probePtyLiveness` and the runtime's `PtyController.hasPty` were both already `boolean | null`, with consumers branching on null correctly. The lie was injected at exactly one interface. Now three-valued, and each provider answers unknown where it cannot prove absence: the daemon adapter off-socket, the SSH provider before a completed listing, the router when any adapter cannot answer, and the degraded provider rather than fabricating a verdict. `terminal_gone` requires unanimous proven absence. Also fixes a real cold-start bug this surfaced: `pty:hasPty` never awaited the daemon-swap startup promise, though the sibling `probePtyLiveness` bridge already did, so before the swap the local provider answered an authoritative false for every daemon-owned id. Net +27 production lines. The plan behind this predicted -92 on the strength of deleting the renderer's dead-session reconcile path; that code is live (`pty-connection.ts` imports it), so nothing was deleted. Expressing a third value where there were two costs lines, and a deletion that is not real is not worth manufacturing. Co-authored-by: Orca <help@stably.ai> * docs(terminal): track the terminal-session correctness handoff package The package was untracked under a gitignored `docs/**`, with the un-ignore rules living only in an uncommitted .gitignore edit — a single `git clean -xdf` would have destroyed the authoritative plan. The 814-path construction snapshot is now pushed as `nwparker/react185-authority-snapshot` too; it had no remote ref. Co-authored-by: Orca <help@stably.ai> * test(ssh): make the reconnect settle window actually wait The settle poll reused a matcher the assertion 15 lines above had already satisfied, and Playwright's poll engine probes immediately and returns as soon as the matcher passes — so it observed the same state twice and elapsed 0ms. A shell grafted a second or two after reattach reported ready slipped through into the next cycle. Reviewer was right on #13111. Test-only; no production change. Co-authored-by: Orca <help@stably.ai> * test(ssh): census both durable session partitions on reconnect Adds a second reconnect scenario and a helper that reads pane records from the local partition as well as the ssh host partition. That split matters: the reattach binding call passes no hostId, so a grafted pane lands in the LOCAL partition and an oracle reading only the host partition passes whether or not the guard is present. Both tests remain forward guards. The second one was reported as discriminating and did not reproduce: with `mayCreate: false` removed from the call site and the app rebuilt, both still passed. Its induction races `pty:kill` against a severed transport, so when the kill lands the lease is cleaned up and there is nothing left to graft. The handoff README is corrected to say so rather than claim a journey. Co-authored-by: Orca <help@stably.ai> * docs(terminal): record the user decision relaxing G6 G6 becomes minimise-and-justify rather than strictly net-negative. The deletion budget the plan assumed does not exist: an entrypoint-rooted import graph found 51 of 53 candidate files reachable and instantiated on live paths, leaving 263 deletable LOC against roughly +1,021 to offset. Correctness may still not be traded for line count. Co-authored-by: Orca <help@stably.ai> * test(terminal): add discriminating oracles for restart, daemon, skew and namespaces Six parallel streams, each required to fail with its guard removed rather than merely pass. Local restart proves the OS process itself survives, by reading `ps -o lstart=` for the shell's own pid. That matters: with the quit path made destructive, the tab, leaf and pty ids all came back byte-identical while the shell underneath was a new process — every existing restart spec would have stayed green. Two separate guards were removed to redden it, and the second reddens only the stale-operation case. Daemon restart discriminates by reverting three-valued `hasPty`; version skew now covers publication semantics and confirms the new `SSH_SOURCE_RESTORE_REQUIRED` token mutates nothing on an old client; two-host isolation censuses both containers. Deletes `src/relay/pty-source-replay-index.ts` — 201 production lines with no importer outside its own test, verified against an entrypoint-rooted import graph rather than a name grep. Five namespace tests are skipped, not passing: they reproduce a defect still live on main where folder-workspace ids compare equal with the instance suffix stripped. PR #12474 fixes it; they are its oracle. Co-authored-by: Orca <help@stably.ai> * test(ssh): induce the reattach graft deterministically instead of racing a kill The previous induction closed a pane while the transport was severed and relied on `pty:kill` FAILING so the lease outlived the pane record. It does not fail: with the provider already torn down, `pty:kill` takes its tombstone branch and marks the lease terminated, and `reattachKnownPtys` filters terminated leases out of the fan-out — so the reconnect never visited the PTY the test was about. It passed on both trees. Seed the precondition instead. Spawn a real remote PTY on a leaf that never becomes a pane, then roll the host partition back to its pre-spawn snapshot, leaving a live lease and a live remote shell that no durable pane owns. No failure races a success. Adds a vacuity guard that is independent of the tree under test: the lease's own `lastAttachedAt` must advance, proving the fan-out actually visited this lease before the pane census is trusted. Verified on this machine under an isolated TMPDIR, since the e2e harness keys its seeded-repo pointer on a machine-global tmpdir path: guard present passes, guard removed fails with the phantom leaf grafted into the local partition, guard restored passes. Co-authored-by: Orca <help@stably.ai> * docs(terminal): propose one authoritative binding identity Every defect this program has touched is the same defect: identity compared with the wrong key, or not compared at all. Lease keyed without the pane, reattach using a creating write, folder-workspace ids compared with the instance suffix stripped, local mutating IPC carrying only an id, a live shell classified as expired, liveness unable to say unknown. Proposal: one branded binding type built from fields that already exist and are already persisted, constructible only from an authoritative source, carried by mutating operations, compared by one shared function. Makes a wrong-key comparison a type error rather than the next incident. Under adversarial review, including against the open issue corpus. Not accepted. Co-authored-by: Orca <help@stably.ai> * fix(pty): refuse mutating operations aimed at a superseded PTY `pty:write`, `pty:writeAccepted` and `pty:resize` accepted any id. The renderer queues input, so a keystroke buffered before a reattach landed on whatever PTY had since taken the pane — and a resize reshaped the successor's shell. Main already tracks `ptyPaneKey` and `paneKeyPtyId` in lock-step, so their disagreement is proof the caller's id was superseded. No wire change, no renderer change, nothing added to the input payload. An id with no recorded pane stays permitted: unowned and orphaned PTYs are unknown, not stale, and unknown never authorizes refusing an explicit operation. That is also what keeps orphan cleanup working — those ids have no pane by construction. The tests pin the CALL SITES, not the predicate. A capability that exists and is never called is indistinguishable from no capability, which is exactly how `mayCreate` sat inert here for several commits with every test green. Co-authored-by: Orca <help@stably.ai> * fix(pty): fence signals at a superseded PTY, and pin why kill is exempt A signal means "interrupt my pane", so delivering one to a PTY the pane has already replaced is a misdirected interrupt. Fence it with the same lock-step proof used for write and resize. `pty:kill` stays deliberately unfenced and a test now pins that: a superseded PTY is orphaned, and reclaiming it is exactly what the orphan-cleanup callers ask for. Refusing there would break the operation that reclaims leaked shells — the opposite of the intent. The fence sits at the IPC boundary, above `tryGetProviderForPty`, so it covers local, daemon and SSH rather than the local path alone. Co-authored-by: Orca <help@stably.ai> * test(terminal): poll the pane binding read so a slower host cannot flake it `readPaneBinding` took a single unpolled read of a DOM dataset attribute immediately after a renderer reload, while its sibling helper polls the same data for 15s. On a native Linux host both tests failed every run with 'No bound terminal pane is mounted' while the app was demonstrably healthy — the screenshot showed the terminal restored with a live prompt and the boot PID echoed. The assertion is unchanged; it is only awaited. Nothing is weakened. Found by running this spec on native Linux rather than assuming macOS behaviour generalises. Co-authored-by: Orca <help@stably.ai> * test(terminal): make the restart identity spec run on Windows too Both probes were POSIX-only and unconditional: `echo ...=\$\$` for the shell's own pid, and `ps -o lstart=` for its start time. Running the spec on a real Windows host proved it dies before reaching either guard, so Journey 1's Windows half was unprovable rather than merely unproven. PowerShell exposes the same two facts as `$PID` and `Get-Process` StartTime. The start time still matters on both platforms for the same reason: a PID alone cannot separate a survivor from a reused number. Still green on macOS. The Windows path is written from the host probe and has not itself been executed end to end — that is the next thing to run there, not a claim being made here. Co-authored-by: Orca <help@stably.ai> * docs(terminal): record the fence's real gap and what peer designs taught Marks the client-constructed binding proposal as rejected with the three false claims that sank it, and records what shipped instead. States the shipped fence's actual limitation rather than leaving it implied: it compares a binding, not an incarnation, so a respawn under a reused ptyId passes. The obvious remedy is wrong here — the agent-create id is deterministic by design so a replayed create stays idempotent, and randomising it would trade this narrow gap for a duplicate-spawn bug. Also records the ranked lessons from four comparable agent IDEs, chiefly that a typed end-reason at end time is what stops a user quit from looking like a resume candidate. Co-authored-by: Orca <help@stably.ai> * docs(terminal): promote Journey 1 to proven on all three platforms The oracle now runs natively on macOS, Linux and Windows, and its discrimination was watched on each: a mutation reddens it, a restore greens it. On Linux and Windows both mutations were run, and the second reddens only the stale-operation test — so the journey's two clauses are proved independently rather than jointly. Windows is the new evidence. The PowerShell branches added blind at ebffb85a848 executed correctly on their first run: `$PID` expanded to real integers, which also proves the pane shell there is PowerShell-family rather than Git Bash, and `Get-Process StartTime` returned kernel start times 5.4s apart — so a recycled pid could not have passed as a survivor. First journey promoted in this program. The other twelve are unchanged, and the residual limit on "every stale exact operation" is recorded rather than glossed. Co-authored-by: Orca <help@stably.ai> * test(terminal): add discriminating oracles for the daemon, skew and multi-host journeys Daemon: replaces a spec that modelled only a client restart and never crossed the daemon boundary, whose successor generation owned nothing so "the live successor is neither killed nor replaced" was vacuous. The PTY leader is now a real login shell reporting `$$` back through the production write path, resolved to a kernel start time. Two mutations each redden exactly one of the three clauses, on macOS and Linux: reverting three-valued `hasPty` reddens only the unknown-not-dead clause; widening the sole-provider fallback reddens only the stale generation clause. Skew: reverting the restore-required publication to expiry reddens 4 of 5 new tests while the legacy control stays green — the regression this branch fixed is now caught if reintroduced. Multi-host: restoring `mux.dispose('connection_lost')` reddens sibling isolation on one host. It does NOT redden across hosts, and that is recorded rather than glossed: a mux belongs to one relay session per target, so its dispose cannot cross a host boundary. Journey 4's cross-host clause rests on isolation-by-construction, not on a mutation. No production code changes. Co-authored-by: Orca <help@stably.ai> * docs(terminal): record journey evidence that falls short of promotion Four journeys now have discriminating oracles but none meets its full stated scope, and each shortfall is named rather than rounded up. Journey 2 is one WSL run from promotion. Journey 12's tests are in-process, so they do not close the live-skew gap the original ledger named. Journey 4's cross-host clause cannot be proven by mutation at all — a mux is per target, so its dispose cannot cross hosts, and the cross-host test stayed green under the mutation that reddens siblings. Journey 13 measured one dimension of ten, on lifted predicates rather than through real IPC. Co-authored-by: Orca <help@stably.ai> * docs(terminal): promote Journey 2 to proven on macOS, Linux and physical WSL The oracle runs on every environment the journey names, and is clause-selective on all three: reverting three-valued `hasPty` reddens only the unknown-not-dead clause, and widening the sole-provider fallback reddens only the stale-generation clause. Selectivity in WSL was established rather than assumed. The spec runs serially, so a red first test reports the others as "did not run" — they were re-run alone under the same mutation and stayed green. Also records that an Orca WSL-mode terminal now starts on that host at all, which it could not before: the distro had no provisioned default Unix user, so every interactive launch blocked on first-run setup. One diagnosis from the WSL run is corrected here rather than repeated: the unrelated `local-pty-shell-ready` failure was attributed to bash 5.3.9, but macOS runs the same bash version and passes 67/67. The trigger is environmental to that distro, and the underlying defect is that the spec pins an absolute count of OSC markers it does not own. Co-authored-by: Orca <help@stably.ai> * docs(terminal): correct the WSL provider-suite diagnosis The WSL run blamed bash 5.3.9 for the unrelated `local-pty-shell-ready` failure. macOS runs the same bash version and passes 67/67, so the version is not the cause — the trigger is environmental to that distro, and the underlying defect is that the spec asserts an absolute count of OSC markers it does not own. Co-authored-by: Orca <help@stably.ai> * test(runtime): unskip the workspace-namespace oracles now their fix has merged These five reproduced a defect that was live on main: folder-workspace ids were compared with the instance suffix stripped, so two workspaces sharing a directory read as the same namespace. They were committed skipped, pointing at the PR that fixes it. That PR is merged, and they pass. Verified they still bite: restoring the suffix-stripping comparison reddens exactly these five and leaves the other four green. An oracle written before its fix, held skipped, and confirmed against the fix after the merge — rather than deleted and rewritten from the answer. Co-authored-by: Orca <help@stably.ai> * test(ssh): add MaxSessions, lazy-discovery and paired-skew oracles Three journeys attempted; none promoted, and the reasons are recorded in the ledger rather than rounded up. MaxSessions=1 against real OpenSSH, with the cap read back from `sshd -T` rather than assumed, and remote pids read on the container two independent ways that must agree, each carrying its kernel start time. Two disjoint mutations discriminate — one reddens only the reconnect clause, the other only the two restart clauses. But the disconnect clause is a forward guard: four separate guard removals left it green, so nothing shipped is load-bearing for it. Lazy discovery samples sshd's own accept log and live session census across a 22s window with the in-use host as a positive control. No mutation reddens its third clause alone — the real cross-host lease scoping is load-bearing, but removing it breaks the sibling host during setup, so the failure carries no clause information. The paired-runtime skew spec pairs two real processes at different versions and refuses to run rather than degrade into a same-version pairing that would look green and prove nothing. No production code changes. Co-authored-by: Orca <help@stably.ai> * docs(terminal): record why the duplicate-resume fix was not built I recommended adding a typed end-reason so a user quit stops looking like a resume candidate, then went to implement it and stopped. `SleepingAgentSessionRecord` already carries three fields that each exist to stop something resuming that should not have — `origin`, `restoreOnTabOpenOnly`, and `automaticResumeBlockedBy` — each traceable to its own incident, consulted at 22 non-test sites. A fourth predicate, however well typed, is the fifth containment cycle. The designs without this bug do not have a better flag; they resume only on an explicit action, into a new terminal id, and make two agents in one terminal unrepresentable in the schema. The first of those is a product decision about whether automatic resume stays a feature, so it is the user's call rather than mine. Co-authored-by: Orca <help@stably.ai> * docs(terminal): reconcile G6 with the recorded decision and assess its clauses G6's body still demanded strictly-negative production LOC after the user relaxed it to minimise-and-justify, so the gate had two conflicting pass conditions and no single truth value. Its body now points at that decision. Assessed the remaining clauses against the branch rather than assuming. Two fail structurally: more than one identity comparison and mutation admission path still exist, and `terminal-input-quarantine.ts` is still reachable from two production files. Records why the quarantine is not subsumed by the superseded-PTY fence, which I had assumed and checked. The fence refuses writes aimed at a stale ptyId; the quarantine guards the user's next keystrokes landing on the successor under its current, correct id — a case the fence never sees. Removing it needs the recovery path to surface a different shell as unresolved, not a deletion. Co-authored-by: Orca <help@stably.ai> * docs(terminal): the input quarantine is load-bearing, not superseded G6 lists "no superseded quarantine remains reachable" and this module was assumed to be one. Disabling its single call site reproduces the hazard it exists for — `cho hi; rm -rf x` reaching the shell — so deleting it without a replacement re-opens command execution. The replacement was costed by building it rather than estimated: +26 production LOC to thread the incarnation, ~+33 complete, and the cross-remount state it needs outlives the destroyed pane so it becomes a module about the size of the one deleted. Floor is roughly +140 to delete 88, and it would add a second identity comparison to a gate already failing for having more than one. The decisive part is that the route is not uniformly available: remote runtime results carry no incarnation, old hosts cannot be made to publish one, and mixed versions are the normal state. A paired client reads unknown, which this program's own rule says is not proof — so either every remote reattach surfaces unresolved, or a fallback is needed and the only correct fallback is this module. Whether to amend the clause or accept something weaker on remote hosts is a user decision, so the clause verdict is left as failing rather than quietly reclassified. Co-authored-by: Orca <help@stably.ai> * refactor(runtime): collapse duplicate identity comparisons G6 requires one identity comparison; five implementations existed across two concepts. Worktree-namespace identity had two: `runtimeWorktreeIdsEqual` and `runtimeWorktreeIdentityKey` independently re-derived repoId plus normalized path. Equality now derives from the key, so the comparison and the sleep / mutation-queue keying cannot drift into two different rules — which is exactly how the suffix-stripping bug reached production once. Pane identity had three byte-identical leaf-UUID comparisons, in orchestration `db.ts`, `lifecycle-reconciliation.ts`, and `orchestration-legacy-process-identity.ts`. One copy moved to `stable-pane-id.ts`, which already owns `PaneKey`, `parsePaneKey` and `makePaneKey` and which all three already imported. No new module, no branded type, no parallel comparison. Net -14 production lines. The namespace oracle still bites: restoring the filesystem parser inside the identity key reddens exactly its five cases. The raw counts are not the actionable set, and the classification is worth recording: of 409 non-test `worktreeId` comparisons, 71 are typeof guards and 81 are sentinel tag checks. Most of the remainder are renderer predicates over store rows where both operands are the same main-minted id, so normalizing there would widen equality rather than correct it. Co-authored-by: Orca <help@stably.ai> * refactor(terminal): finish a half-done fixture move and audit the rest `xterm-bypass-event-fixture.ts` and `__fixtures__/xterm-bypass-event.ts` were byte-identical apart from an import path. The `__fixtures__` copy had zero importers and the live copy compiled as production — someone started the move and left both. Dead copy deleted, live one moved, its three test importers updated. Audited the wider G6 clause by importer rather than filename: 32 test-only files, roughly 3,300 LOC, currently compile as production; 4 of the 36 candidates have real production importers and are correctly placed. The list is recorded in the goalposts. Those 32 are almost all older than this program and outside the terminal surface, so sweeping them belongs in its own change rather than inside a terminal PR. The clause stays failing, with the remaining files named. Co-authored-by: Orca <help@stably.ai> * docs(terminal): the fixture clause already holds where it matters Checked what the build emits rather than reasoning from file paths. None of the 32 test-only fixtures appears in `out/` — Rollup drops them because no production entrypoint reaches them. On "compiles into the shipped product", this clause holds today. On the other reading it cannot be closed by moving files at all: both production tsconfigs use bare `include` globs with no `exclude`, so a `__tests__/` directory matches exactly like any other path, as does every `*.test.ts` in the repo. Relocating 32 fixtures would remove nothing from typecheck scope. A sweep was started and stopped once this was verified, rather than landing 32 moves across areas this program does not own for no gain. If the intent is that typecheck scope should exclude test code, that is a repo-wide tsconfig change with a different owner. Co-authored-by: Orca <help@stably.ai> * docs(terminal): add plain-language design and test overviews Two reviewable documents with diagrams, written so someone with no prior context can follow what breaks, why, and what changed. The design overview explains the five things stacked behind one terminal rectangle, the 2 -> 19 -> 20 report, the three root causes, and the rule underneath all of them: unknown is not dead. The test overview explains why a green test proves nothing on its own, the four-step mutation proof we adopted, and — the part worth reviewing hardest — an honest account of what could not be proven and why, including the properties that are true by construction and therefore have no guard to remove. Co-authored-by: Orca <help@stably.ai> * docs(terminal): add a self-contained visual report of the design and its evidence Pre-renders every diagram to inline SVG in both themes so the report opens offline and stays sharp when zoomed. States the gate/journey score and the retractions alongside the fixes, so the unproven half is as visible as the proven half. Co-authored-by: Orca <help@stably.ai> * docs(terminal): record the finalized two-plane architecture decision Adopts the data-plane proposal and adds the control-plane track it does not cover: re-key ownership by pane, split orphan inventory out, then delete the compensating code. Records that the host-authority alternative was refuted and that the shipped keystroke fence is inert on the reattach path. Co-authored-by: Orca <help@stably.ai> * docs(terminal): add the design brief the review counsel works from Separates verified code facts from unverified leads so reviewers attack the design rather than a reconstruction of it, and records which simpler alternatives were already refuted and why. Co-authored-by: Orca <help@stably.ai> * docs(terminal): report the design counsel's outcome and the live respawn bug it found Three review rounds across two models replaced the two-record split with one leaf-keyed record, deleted attach-time pane identity, and made orphans a connect-time projection. Records that a shipped gesture still turns a healthy remote shell into a duplicate agent resume, and that the renderer classifier in that chain treats an error-message shape as proof of death. Co-authored-by: Orca <help@stably.ai> * docs(terminal): correct the report — the respawn proof gate guards a minority path A final review traced every auto-respawn route. The primary one converts the reattach failure into a boolean before any classifier sees it, so the shipped proof gate never runs there. Records that two of the six shipped changes are narrower than claimed, and why their tests could not have caught it. Co-authored-by: Orca <help@stably.ai> * docs(terminal): explain the landed design on its own terms One leaf-keyed ownership record, orphans computed at connect, and replacement shells only on positive proof — with the shipping order and the one product trade the design asks the owner to accept. Co-authored-by: Orca <help@stably.ai> * docs(terminal): rewrite the design explainer in plain English The first version assumed the reader knew the codebase. Reframed around two bugs, two fixes and one decision, with the jargon replaced by pane / program / note / helper and a five-word glossary for what could not be avoided. Co-authored-by: Orca <help@stably.ai> * fix(ssh): stop reading an identity mismatch as a dead shell The relay reports a pane-identity mismatch by saying the pty was not found, but it found it — comparing identity is how it noticed. Publishing that as expiry made the renderer clear the binding and cold-restore with agent resume, so a live shell gained a second agent on one transcript. Reachable today by detaching a pane into a new tab, which changes the tab the relay froze at spawn. Mismatch now carries its own token and the classifier refuses it as proof. Genuine absence still expires, so a shell that really went away is not stranded. The three failure tokens move to src/shared: main published them and the renderer decided respawn on them, from two copies that had drifted apart. Co-authored-by: Orca <help@stably.ai> * fix(ssh): stop sending pane identity on reattach The relay froze pane identity at spawn, so moving a pane to another tab made it refuse a live shell — and refuse by saying 'not found'. The comparison is presence-guarded, so not sending the fields disarms it on every relay version including ones already installed on hosts: no wire change, no redeploy. Nothing is lost. It existed to catch a relay restart recycling pty-N for a new shell, and in exactly that case pane and tab both still match, so it accepted the wrong shell anyway. The incarnation the attach returns is what distinguishes those, and it already crosses the wire. Removes the whole client-side apparatus: the expected-identity type, its per-lease derivation, its map, and the parameter threaded through four layers. Co-authored-by: Orca <help@stably.ai> * docs(terminal): add tracked goalposts for the new design Each goalpost is a behaviour with an oracle and the mutation that must redden it, so 'proven' cannot be claimed from a green test. Records the anti-inert rule as a first-class goalpost, since three guards in this program passed their tests while sitting off the route production takes. Co-authored-by: Orca <help@stably.ai> * docs(terminal): record that the recovery grant is dead code, deleting a design step The lease stores a relay-native pty id and the caller passes the app form, with a raw equality comparison between them, so the 30s grant cannot fire for a real SSH pane. The death rule that existed to referee it is deleted rather than built, and the dead path itself becomes a removal. Co-authored-by: Orca <help@stably.ai> * docs(terminal): keep the full design detail in the repo It only existed in an ephemeral job directory, so the plain-English explainer had no durable source for its specifics — record shape, death rule, reattach algorithm, migration order and the 25 oracles. Co-authored-by: Orca <help@stably.ai> * docs(terminal): add a resume prompt for a clean session Points at the goalposts as the contract, names the three goalposts whose oracles are already written and red, and carries the process rules that were learned the expensive way — prove guards reachable, verify mutations land, commit per step, and never let a subagent write production files in a shared worktree. Co-authored-by: Orca <help@stably.ai> * test(ssh): add the failing oracles for goalposts S3, S4 and S5 Intentionally RED: 14 clauses that fail against current behaviour and go green under the changes named in new-design-goalposts.md. The branch is held unmerged, so red here means unimplemented, not broken. Each was verified to fail for the right reason and to flip green under the identified fix, which was then reverted. Each pins the producer as well as the consumer, so no clause can pass vacuously if its route is ever severed — the failure mode that let three earlier guards ship inert. Co-authored-by: Orca <help@stably.ai> * fix(ssh): stop fabricating an exit when a reattach fails A failed attach never proves the shell exited. The relay answers not-found for a pane-identity mismatch and for any id it merely cannot hand back, so treating it as death sent the pane a synthetic `pty:exit { code: -1 }`, cleared provider state, deleted ownership and expired the lease — four claims about a process we know nothing about, on a shell that is usually still running. Collapse every failure into the non-destructive branch that already existed a few lines above (`restoreRequired = 'reattachAttemptsExhausted'` + wakeRecovery). A branch collapse, not a new mechanism: goalpost S3. Two tests pinned the deleted premise and are INVERTED rather than patched, so the new intent stays covered: - ssh-relay-orphan-abandon-paths: "retires the lease without a kill when the relay proves the PTY is gone" -> "leaves the shell running when the relay only reports the PTY as not found". Its comment claimed attach verifies liveness before answering not-found; it does not. - ssh-relay-session: "invalidates and broadcasts remote PTYs that cannot reattach" -> "leaves an unreattachable remote PTY alone while its sibling reattaches". Also repairs two clauses left red by |
||
|
Neil
|
501337454c |
perf(runtime): stop rescanning every repo every 30s with a Git-admin fingerprint (#14207)
The main-process worktree resolution cache expired on wall-clock time: the whole-fleet snapshot has a 1s TTL, so any poller faster than 1Hz recomputed it, and every 30s the per-repo scan cache expired and shelled out `git worktree list` for every registered repo. A production trace recorded 4,272 `git worktree` invocations over 3h27m across 10 repos. In-Orca mutations are already event-driven, so the 30s TTL existed only to discover changes made outside Orca. Before re-running an expired scan for a local, non-WSL repo, read a cheap subprocess-free Git-admin fingerprint (admin dir entries, per-checkout HEAD and its ref tip, gitdir/locked/existence per entry, packed-refs and reftable stamps). If it matches the fingerprint captured at the cached scan's start, extend the cache without spawning Git. A real scan still runs every 5 minutes so anything the probe cannot see still reconciles. Measured: 600 -> 60 `git worktree list` spawns on the reported workload (10 idle repos, 1Hz polling, 30 simulated minutes), and main-thread event-loop stall of 2.69ms -> 0.01ms per refresh. External worktree add/remove/move/lock/checkout/commit discovery stays bounded at 30s. SSH repos, WSL-routed repos, folder workspaces, agent-scratch repos, and any repo whose layout the probe cannot read keep today's behaviour exactly. Design: docs/reference/worktree-scan-fingerprint.md |
||
|
Neil
|
95ff19f35f | chore(repo): ignore generated Clawpatch state (#14144) | ||
|
Neil
|
9c091cf77e |
chore(perf): add renderer agent-status benchmark harness (#13905)
Splits run-idle-cpu-benchmark.mjs into a scale fixture, an in-page timing probe, and process sampling, and records a measured origin/main baseline so the agent-status batching slice has an auditable before. The agent-status write workload is not included: it needs setAgentStatuses, so it lands with the store slice. |
||
|
Brennan Benson
|
d50adec2d2 |
feat(ai-vault): isolate scanning from terminal workloads (#13411)
* feat(ai-vault): isolate scanning in service processes * fix(ai-vault): retire idle service processes * fix(ai-vault): discard unverified cache processes * fix(ai-vault): clear relay sidecar cancel watchdog on acknowledgement A cancelled relay call is settled before its 2s cancel watchdog is armed, so the acknowledgement path bailed out of settle() before clearing the timer. The watchdog then faulted a healthy sidecar two seconds after every aborted scan, killing whatever request had since become active. * fix(ai-vault): clear the pending restart before scheduling another recordFault overwrote this.timer, stranding a restart that dispose() could no longer cancel. * refactor(ai-vault): drop the orphaned first-prompt IPC wrapper session-first-user-prompt-handler.ts now owns this entry point and routes through the service; the copy left in the read module had no callers. * fix(ai-vault): retry a faulted cold start before surfacing it A slow first start surfaced a raw 'did not become ready' error to the caller even though the supervisor was already respawning. Requeue an unsent call once onto the scheduled respawn instead. Also stop arming the cancellation watchdog for a call the child never received: no acknowledgement is coming, so it killed a healthy service and stalled the lane. Invalidation bookkeeping and ready-waiter construction move to the state module to stay under the max-lines cap. * fix(ai-vault): give relay title reads their own lane Before this branch the relay read title files directly, concurrently with scans. Routing both through one sidecar lane put title resolution behind a list scan that may run up to 130s, so SSH tab titles could lag minutes behind. Split cache and interactive lanes in both the relay client and the sidecar entry, mirroring the desktop service. Also: clear the ready deadline on fault, so a sidecar that dies before ready cannot fault its healthy replacement five seconds later; retry an unsent call once across a respawn; and skip the cancellation watchdog for a call the sidecar never received. Restart/circuit bookkeeping moves to its own module, mirroring the desktop policy, to stay under the max-lines cap. * fix(ai-vault): degrade relay title resolution on sidecar failure listSessions already returns a host issue when the sidecar is unavailable; titles propagated the raw RPC error instead. Return no titles so callers fall back to preview text, and keep cancellation propagating. * fix(ai-vault): scrub the service child environment The children are forked with a 384 MiB heap cap and no loader, but both spawn sites handed them the full parent environment, so an exported NODE_OPTIONS silently raised the cap or --require'd code into them. Allowlist both, following the plugin worker. The desktop child keeps the eleven agent-root overrides it resolves its own roots from; the relay sidecar takes remoteHome and hostPlatform from its init message and so needs none of them. Both children share one priority module while they share this one. * fix(ai-vault): soft-disable relay vault when the service is missing A missing service threw out of the constructor, so a Vault wiring bug would abort relay startup and take every PTY on the host with it. The unsupported-platform branch three lines above already treats a Vault failure as a soft disable; do the same here. Threading the service through the two handlers instead of a field also retires the definite-assignment assertion the throw was propping up. * fix(ai-vault): drain consumed cache invalidations invalidatedPaths was re-applied in every request's finally and never drained, so once N paths had been invalidated every later request paid N evictions for the life of the process; the 4096 cap only bounded how bad that got. The re-apply exists to cover a read that overlapped the invalidation, so drain once nothing is executing. Clearing unconditionally would drop the re-apply for a request still running on the other lane. * fix(ai-vault): keep a busy child through slow invalidation acks invalidate() reused the 5s ready budget as its acknowledgement deadline and killed the child on expiry, so a delete issued during a large scan could kill a healthy process mid-scan and burn a slot toward the restart circuit. Fault only when nothing is executing. Fork IPC ordering already puts the invalidation ahead of any later request, so a busy child owes no ack here, and the 130s/15s request deadlines still catch a wedged one. The start-retry predicate moves to the state module to stay under the line cap, matching the shape the relay client already uses. * fix(ai-vault): report a failed local scan as a host issue A local-scope scan let its error escape to the renderer, which paints it over the session list. Service supervision now produces those errors, so "AI Vault service restart circuit is open." replaced the list. Route local scope through the degradation the all-hosts leg and every SSH leg already use, so it lands as a retryable host issue row instead. Same result shape either way, so no IPC or wire contract changes. * test(ai-vault): cover the relay restart circuit transitions The relay policy shipped without tests. Pin both circuit edges, the aging-out case, the forced-refresh reopen the relay has and the desktop does not, and the backoff schedule. * fix(ai-vault): keep the OpenCode roots in the service child env The scrubbed allowlist dropped XDG_DATA_HOME and OPENCODE_DB, which the child reads to locate the OpenCode store and database. The pre-PR worker thread inherited them, so a user who sets either lost every OpenCode session. * test(ai-vault): anchor the service spawn env assertion |
||
|
NeilandOrca
|
17cfc968cf |
Revert the terminal IME composition-ownership change (#13282)
* Revert "test(ime): restore coverage the composition-ownership change removed (#13168)" This reverts commit |
||
|
NeilandOrca
|
17b3dff3c4 |
refactor(terminal): return IME composition ownership to xterm (#13128)
* fix(terminal): return IME composition ownership to xterm * fix(mobile): derive terminal input from native replacement ranges * test(mobile): record iOS Japanese IME traces * fix(mobile): preserve native IME replacement ranges * fix(xterm): flush queued application input after IME commit * test(terminal): pin Korean intermediate commit * test: pin Windows IME shortcut ownership * test: replay IBus number candidate commit * fix: preserve native macOS input-method punctuation * refactor(terminal): remove stale mac focus override * fix(mobile): preserve soft keyboard deletion ranges * fix: keep IME-owned palette chords in renderer * fix: stop carried IME shortcuts at renderer owner * fix: preserve carried IME shortcut dispatch * fix: narrow main-owned shortcut actions * test(mobile): pin Japanese IME replacement traces * test(terminal): retain paired native IME trace * fix(chat): preserve browser IME composition ownership * fix(chat): retain macOS IME confirm gesture * fix(chat): expire unmatched IME confirm carry * fix(chat): isolate IME confirmation expiry * fix(chat): retain active IME confirmation * refactor(terminal): remove dead composition handler * feat(ime): add shared Enter-ownership seams for CJK composition The confirming Enter of a CJK composition arrives as two keydowns and the orderings differ by platform: Windows/Linux redispatch the unmarked Enter/13 before keyup, macOS delivers keyup first. A guard reading only isComposing or keyCode 229 misses the redispatch, so surfaces submitted on a confirm. Adds useImeEnterGestureOwnership (carry token, next-frame expiry), a shared ImeEnterGuardedForm for native implicit submission, and the cmdk seam covering 18 CommandInput surfaces at one site. A chorded Enter arms the carry but is never swallowed — the reverse would eat a user's deliberate Cmd/Ctrl+Enter. Both failure modes are pinned by ime-enter-gesture-ownership-contract.test.ts. Co-authored-by: Orca <help@stably.ai> * refactor(terminal): consolidate native input listeners and parked-screen owner Extracts the shared native-input listener installer and renames the parked-screen detector for what it actually does, replacing per-call-site duplication. The listener installer keeps a forgetOptionKeyLocationOnBlur flag so per-window semantics are preserved rather than flattened. Net deletion; no behaviour change intended. Co-authored-by: Orca <help@stably.ai> * test(terminal): pin recorded IME shapes as regression tests Nine regression tests built from hashed affected-platform captures, each with a paired ordinary negative and a discriminating mutation verified to take the file from all-passing to exactly one failure. Covers the Windows MS-Korean Shift family (#12179, #11878, #12151, #11946, #12152) and the Korean TUI line-break rows (STA-3237, STA-3222, STA-3129). STA-3237 pins the empirical 3-Shift / 2-active-composition / 2-newline ratio the device run established — the third Shift produces nothing because Space has already committed. That ratio is not derivable from a static capture. Co-authored-by: Orca <help@stably.ai> * fix(ime): guard Enter-commit surfaces against CJK confirm Applies the Enter-ownership guards across the surfaces whose Enter commits something: publishes, clones, pairs, installs, posts, or persists. Tiered deliberately rather than uniformly. Irreversible and remote-effect sites take the carry token, which also blocks the unmarked redispatch. Locally reversible sites take the oracle check with a one-line comment naming the residual, because a spurious commit there costs one undo. Three numeric fields are left unguarded with the reason in-code: Chromium blanks number inputs at compositionstart, so a confirm-Enter only ever reaches an empty-draft reset. Measured with a CDP probe rather than assumed — a guard that cannot fire is noise. Co-authored-by: Orca <help@stably.ai> * test(ime): teeth-check the Enter guards on every guarded surface One suite per guarded surface, each verified by deleting the guard and confirming the test fails. A green guard test without that check is unverified, not verified. Two shapes pass vacuously in happy-dom and are avoided here: native implicit form submission never fires, and blur() is inert on an unfocused element. Both made "the commit did not happen" assertions pass with the guard removed, so the suites assert the guard's contract directly instead. Co-authored-by: Orca <help@stably.ai> * fix(mobile): keep iOS Korean commits whole through the live-input path iOS Korean reports isComposing: false on every event, so it bypasses the composition guard entirely. The strict owner rejected UIKit's transformed post-change field and sent only the leading jamo — the reported symptom. Prefers the authoritative same-event field text over the predicted text when the supplied operation cannot produce it. Generic: no Korean special-case, no locale classifier, no normalization. Adds the RN-target-keyed submit carry alongside it. Co-authored-by: Orca <help@stably.ai> * test(e2e): make IME capture harnesses fail loudly instead of silently Four instruments recorded silence as success, so a void run scored as a clean one: - readTerminalImeBoundaryTrace returned an empty trace when the probe never installed, making every "nothing leaked" negative pass vacuously - summarizeLatencies([]) returned a perfect zero distribution that passed all three latency thresholds - the macOS Vietnamese spec pinned an input-source ID that does not exist, and failed as though the operator had chosen the wrong source - the expectedLineCount=1 prefix property was undocumented and one edit from silently downgrading a PTY assertion Input sources now resolve by enumeration and name the near-matches on failure. Co-authored-by: Orca <help@stably.ai> * test(terminal): cover Cangjie cancellation and fix a cross-namespace assertion Adds #11951's recorded Cangjie cancel shape to the existing cancellation suite, which covered Pinyin and Sogou but not Cangjie. One keystroke then Backspace arriving as deleteContentBackward with data: null, so the stale preedit is the only thing a fallback could replay. Verified against the historical pre-6cd944c62b3 bundle: the positive fails with ['尸'] where [] is expected, while the ordinary negative stays green. Also fixes the Vietnamese spec, which asserted a TIS-space input-source ID against getKeyboardInputSourceId(). Those two Orca APIs report the same source in different namespaces — TIS nests it under VietnameseIM, the app API does not. The resolver stays as an installation precondition; the assertion matches the leaf. Co-authored-by: Orca <help@stably.ai> * test(e2e): add a real-IME macOS arm for the Korean chord commit The existing korean-ime-terminal-shift-enter-commit spec synthesizes composition over CDP: Input.imeSetComposition sets the preedit directly and Input.insertText performs the commit. Asserting the IME produced events you injected yourself is circular, so that spec cannot certify real-IME behaviour. This arm selects 2-Set Korean via TIS, reads it back live, and injects through System Events key codes, so the OS owns the preedit, the commit instant, and isComposing. PTY byte expectations are preserved verbatim. Covers 2 of the original 4 cases by design. The other two are the Windows/Linux redispatch-before-keyup ordering, which macOS cannot produce and which cannot be selected -- the OS decides it. Reintroducing synthesis to "restore coverage" would reintroduce the circularity. Co-authored-by: Orca <help@stably.ai> * test(e2e): assert the macOS chord arm at the PTY boundary, not the renderer The byte expectations were transcribed from korean-ime-terminal-shift-enter-commit :364/:383, which assert against onData -- a renderer boundary where the terminator is CR. This spec reads the PTY child, where the tty has already converted CR to LF. Names both forms per row rather than swapping the constant, so the conversion reads as evidence that the capture reached past the renderer, as #11936 and #11951 record. Ctrl+Enter's CSI-u sequence is unaffected and is identical at both boundaries. Co-authored-by: Orca <help@stably.ai> * test(e2e): measure composer-to-onData latency and stop dropping IME keystrokes Two defects in the echo latency probe. It hooked onWriteParsed and onRender but never onData, so it measured key->parse->render echo rather than the composer-vs-onData delta the latency rows need. Adds a third hook feeding its own sample set. And `event.key.length !== 1` silently dropped IME keystrokes: Pinyin and Cangjie keydowns arrive as key:'Process' (length 7). Replayed over the captured corpus, the old filter accepted 580 of 4137 Chinese IME keydowns -- it was discarding 80% of them. The new filter matches the shape the owner itself branches on. Attribution charges each onData to the latest keydown rather than a FIFO head, because composing jamo emit no onData at all and a queue would credit a whole composition to its first keystroke. The consumer now asserts sample count before any percentile, so a zero-sample run cannot render as a flawless distribution. Co-authored-by: Orca <help@stably.ai> * test(terminal): pin the WSL shifted-jamo newline shape for #11919 In Korean 2-set, Shift types ordinary letters -- the double consonants and the compound vowels. Each such keystroke reaches Chromium as key='Process', keyCode=229, shiftKey=true. The v1.4.163 classifier matched exactly that pattern with no code guard, so it called those keystrokes Enter, rewrote them to a synthetic Shift+Enter, and injected a newline into the middle of the word -- with no Enter key pressed. That is why the reporters said "no modifier key pressed": they had not chorded Shift+Enter, but they had pressed Shift, to type the double consonant. Asserts the row's own recorded capture: 40 immediate keydowns, exactly 3 of them Shift-carrying inside a single syllable, and an onData stream with one newline per Enter press and none mid-word. Two ordinary negatives keep it from being a blanket mute -- the same session's non-IME keydowns still reach shortcut policy, and an ordinary Shift+Enter still resolves through the real policy. Co-authored-by: Orca <help@stably.ai> * test(terminal): pin the composition commit lag that made Korean type one behind macOS Korean 2-Set commits syllable N only when the first jamo of N+1 arrives, so compositionend and compositionstart land in the same task. A composition-start handler cancelled the pending finalizer that was the only path to triggerDataEvent and ended the session without emitting bytes, so every committed syllable reached onData exactly one syllable late and the backlog cleared only at a Space or Enter. Types continuously with no Enter and no Space -- either would flush the backlog and hide it -- and samples onData at every syllable boundary. Paired with a length-matched ASCII arm that stays green throughout, so the positive is a fact about composition rather than about timing in general. Bisected to a single call site across five builds: pristine, 1.4.155 and 1.4.162 pass, 1.4.163 fails, removing the one call repairs it, restoring it fails identically. That window is exactly the reporter's "started immediately after updating". Co-authored-by: Orca <help@stably.ai> * test(mobile): cover the send-queue abort that silently drops queued keystrokes One failed send in use-terminal-live-input-commit aborts every keystroke queued behind it, with the error swallowed by .catch(() => false). The existing test resolves(true) on every send, so the failure branch was uncovered. Four arms: the abort itself, an ordinary negative on the healthy path, a throwing sender, and a liveness control proving the queue recovers once the chain settles. Deleting the abort takes 4 passed to 3 failed, with the ordinary negative correctly surviving. Scope is stated in the docblock: this is a transport send-queue abort, reachable only via a real disconnect or RPC error. REQUEST_TIMEOUT_MS is 30s, so latency alone cannot reach the branch — consistent with #7094's symptom class, not proven to be its cause. * test(terminal): pin that daemon snapshot/restore cannot disturb a composition Two independent reporters attributed broken Korean composition to the always-on PTY daemon repainting terminal state over the preedit. The attribution is wrong on ancestry — the daemon shipped three months before the version both call good — but the boundary was never actually tested. Runs the real applyMainBufferSnapshot choreography against a live composition, including the full 2J/3J/H wipe plus the resize and alt-screen branches. textarea.value, selectionStart/End, compositionView.textContent and .active all survive byte-identical, and interleaving a restore between every jamo of 문제 still commits 문제 at onData. Also pins that the uncommitted preedit is absent from the captured snapshot: it lives in the textarea, never the buffer, so a restore has nothing stale to echo back. Injecting one textarea.value = '' into the restore fails exactly the three restore-boundary tests. * test(terminal): pin that Cmd tears down a composition where Ctrl and Shift do not xterm's composition keydown exempts only keyCode 16/17/18 (Shift/Ctrl/Alt) plus 20/229. macOS Meta — 91/93/224 — is absent, so a Cmd press mid-composition takes _finalizeComposition(false): the overlay goes dark and never recovers, because compositionstart is not re-fired. The user composes the rest of the word blind. Linux and Windows users press Ctrl and are exempt. xterm already has a Meta-aware modifier predicate in wasModifierKeyOnlyEvent, so this is an internal inconsistency rather than a deliberate choice. Owns no reported row and is version-neutral: 5/5 on both 1.4.162 and 1.4.163. The branch is unexercised in all 328 recorded traces, so this is a hazard pin, not a regression guard. Only the teardown is asserted; the likely duplicated commit needs a compositionend the IME kept alive across the Cmd, which no capture contains. Deleting the exemption fails exactly the three paired negatives; adding Meta to it fails exactly the two Cmd arms. * test(native-chat): characterize preedit loss when a question card replaces the composer An AskUserQuestion card fully replaces the composer by design, but the in-flight composition goes with it: the composer unmounts before compositionend reaches it, so the preedit is never committed to the draft. The committed text survives only because the draft is cached and restored via defaultValue. Node identity changes, value 'abc' is preserved, the 가 is gone. Drives the real NativeChatView -> SessionGate -> InteractiveCard -> questionActive swap -> Composer -> ComposerField, flipped by writing the same store field an AskUserQuestion hook event writes. Flipping questionActive to false fails exactly this test and nothing else across 639 native-chat tests, so the path was entirely unguarded. CHARACTERIZATION TEST: it asserts the loss. Fixing the defect — committing the preedit before the swap, or keeping the composer mounted — will make this file fail. Update the expectations to the new contract rather than working around them. Owns no reported row. #12118/STA-3219 flicker is keyed to token counters, which provably do not remount, and a question card arrives once per question. * test(terminal): pin the duplicated commit when Meta interrupts a composition _finalizeComposition(false) sends textarea.value.substring(start, end) but cannot clear the IME-owned textarea, so a later compositionend re-sends the same range. Meta reaches that path because CompositionHelper exempts only Shift/Ctrl/Alt; xterm's own wasModifierKeyOnlyEvent covers Meta four ways, so the omission is an internal inconsistency rather than a choice. Companion to the modifier-exemption guard, which deliberately pins only the overlay teardown. This pins the data consequence. HAZARD PIN: owns no reported row. The trigger is unverified on hardware — no capture in the corpus contains a Meta-during-composition gesture, and whether macOS keeps the composition alive across it is unmeasured. The duplication follows from the code given that sequence; whether users reach the sequence is the open half. An earlier premise that Space (keyCode 32) reaches this path was refuted by a corpus scan: 0 of 731 evidence files carry a keyCode-32 Space while composing, against 171 at 229, and 229 returns early. * test(terminal): characterize the syllable lost when the textarea blurs mid-composition CoreBrowserTerminal._handleTextAreaBlur clears the helper textarea unconditionally — "Text can safely be removed on blur" — while CompositionHelper._finalizeComposition reads the committed text back out of that same value from a deferred timeout. By the time it runs the value is empty, the substring is '', and triggerDataEvent never sees the syllable. xterm checks composition state in _syncTextArea and omits the same check here. Six cases. Blurring mid-composition loses the syllable in every ordering, including compositionend-before-blur, which is Chromium's real order — so it is not an ordering artifact. A bare textarea.blur() with no Orca code loses it too, which places the owner upstream: Orca's unguarded release on outside pointerdown is one trigger, not the cause. Committing 한 then blurring mid-가 yields ['한'] where ['한','가'] is correct: one syllable gone, surrounding text intact. Teeth checked by inverting — adding an Orca-side composition guard flips exactly the three cases that route through the release path and leaves the bare-blur and no-blur cases green, which is the scope split: a fix in regular-terminal-focus-ownership alone would not close this. HAZARD PIN, but unlike the others this one has a real production injector — clicking outside the terminal mid-composition. Owns no reported row. The shape matches #9738's report; the injector does not, and a shape match with a mismatched injector is not an owner. * test(terminal): say which arm the STA-3237 fixture came from The recorded keydowns are wave 4's A-shift-unmarked-only — the arm that emits no PTY bytes. Nothing in the file said so, so two readers concluded the row's events fail the owner's predicate and that STA-3237 and STA-3222 were different defects. They share an owner; the arm that fires is Process/229+Shift, absent from this bubble-phase trace because the owner claims it in the capture phase. Also corrects "code-blind": the v1.4.163 policy emits \x1b\r only for a shift-only key:'Enter', and a jamo keydown reaches that branch solely via the isTerminalImeProcessEnter rewrite. The mock is deliberately wider so the ownership guard stays under test if that rewrite moves. Comments only — no assertion, fixture value, or mock behaviour changed. * test(e2e): track the input-source selector the macOS specs shell out to Five tracked macOS IME specs ran `swift .tmp/select-input-source.swift`, a file that is gitignored and existed only on one machine. Anyone else checking out the repo — or the same machine after .tmp is cleaned — could not run them, and they are the capture drivers for the macOS rows that are blocked waiting for exactly those runs. Moves it to tests/e2e/ beside its callers. The chord spec now resolves it from __dirname rather than reaching two levels up into .tmp. * test(terminal): pin the CJK repaint decision against the reporter's own output #12164 comment 1 and #5921 report agent output with double-width glyphs rendering duplicated character-by-character while ASCII in the same line stays clean. No IME, no composition, no keystroke — the user never types the CJK. Segmenting all three verbatim samples into maximal same-risk-class runs gives 33 runs and zero violations of "this run is corrupted iff the production detector flags it": 17 wide runs all corrupted, 16 narrow runs all byte-identical. The paired negative is co-located in the same line rather than in a separate run — the reporter supplied it without knowing. Doubling is asserted as present, not uniform: 자바스크립트 and 시스템 each leave a jamo undoubled, which is a repaint-region boundary artifact rather than a per-character transform. The discriminating arm is in the test rather than a source mutation: |
||
|
Jinwoo Hong
|
06780260c0 |
test(remote-runtime): run an old client and an old server against current code (#12682)
Mixed versions are the normal state of the remote-server feature: users update clients and servers independently. Until now nothing tested that. Every cross-version claim was made by code reading plus unit tests with hand-written old/new shapes — enough to catch design problems, not enough to catch a real skew regression. This runs the REAL protocol implementations from two builds against each other in one process: the actual host methods and RPC dispatcher on one side, the actual renderer multiplexer on the other, with a transport that reproduces the production asymmetry — each side decodes with its OWN codec and drops frames whose opcode it does not know. A frame survives only if the RECEIVING build understands it, which is what makes this level sufficient without launching two apps. The old side is a genuine checkout extracted from the release tag; the extracted client was confirmed to lack a symbol that exists only on main. Journey: subscribe, first snapshot, input reaching the process, live output, hide/reveal snapshot, transport drop, resubscribe, input landing again — across old->new, new->old, and a current/current control. Every step ends on an observed-state barrier; no sleeps. The oracle asserts the recorded step list, the exact 16-frame named sequence, negotiated capabilities, the exact input the host wrote to the PTY, rendered content, and zero decoder-rejected frames. A host method the stub lacks is recorded by name and asserted empty, so a harness gap cannot masquerade as a wire break. Detection is proven per violation shape, and it attributes each to the correct side: an unnegotiated opcode goes red only where a decoder would reject it, a removed published field goes red only where an old client consumes it, and a legal additive field stays green in all three pairings so the harness will not cry wolf on safe changes. It also documents the three compatibility rules in docs/reference/remote-wire-compatibility.md, linked from AGENTS.md, since they previously existed only as folklore — notably that "decoders reject unknown opcodes" is true for the desktop decoder but NOT for mobile, which silently drops them. Deliberately scoped: terminal stream only. The session-tab sync channel is not covered, nor agent-session publications, file/Git RPCs, mobile E2EE framing, or the relay transport. Two version points, so a regression introduced and reverted between them is invisible. CI selection was verified rather than assumed — `vitest list` confirms 0 matches under the shard's exclude and 4 under the dedicated job — because a lane silently running zero tests is precisely how a host-side defect escaped CI earlier in this series. Closes STA-3469. |
||
|
OrcaWinandOrcaWin
|
ed7849eb7b |
fix(worktrees): stop silently switching existing Windows setup scripts to Git Bash (#12406)
* fix(worktrees): stop silently switching existing Windows setup scripts to Git Bash #6967 derived the Windows setup-runner shell from `terminalWindowsShell`. On upgrade, any Windows user whose terminal preference resolved to Git Bash had their existing `orca.yaml` setup script (and issue command) handed to bash instead of cmd.exe. Scripts authored against the cmd runner — `copy`, `xcopy`, `set VAR=value`, `if errorlevel 1`, `%VAR%`, backslash paths — broke with no migration and no warning, and the failure looked like Orca broke the project. The conflation is also wrong in the steady state: a terminal preference is per-user, so two people on the same repo got different interpreters for the same orca.yaml and no project could write a setup script that worked for all of its Windows contributors. The interpreter is now a property of the script, declared the standard way: a leading `#!` line. Native Windows keeps the historical `.cmd` runner unless the script declares a POSIX shell, so no existing script changes behavior. `resolveSetupRunnerShell` keeps its role as the feasibility gate — a bash runner still requires the terminal to resolve to Git Bash, since the launch command is typed into that shell and uses MSYS `/c/...` paths. `buildWindowsRunnerScript` now drops a leading `#!` line rather than `call`ing it, so a declared-bash script that falls back to cmd (Git Bash missing) fails on a real setup line instead of aborting on errorlevel at line one. WSL worktrees, POSIX platforms, and SSH hosts are untouched. * fix(worktrees): keep the cmd setup runner launchable from a Git Bash pane Adversarial review of this PR found that pinning the runner format per script reopened issue #6896 one layer down. - `WorktreeSetupLaunch.shell` had been redefined to mean "the format the runner file was written in". `resolveSetupRunnerCommand` consumes it as "the shell that types the launch command", so a Git Bash terminal with a batch setup script produced `cmd.exe /c "C:\...\setup-runner.cmd"` typed into a bash pane, where MSYS rewrites the `/c` switch into a drive path: cmd opens interactively and setup never runs. `shell` is the terminal's family again; the runner file's .cmd/.sh extension carries the format, and a batch runner launched from a POSIX pane reuses the existing PowerShell ProcessStartInfo launcher. - The cmd runner dropped a leading `#!` line and ran the rest as batch, so a bash script reaching cmd (PowerShell/cmd terminal, or any SSH-to-Windows host) got its interpreter-agnostic prefix executed before failing mid-way. It now prints why and exits 1 without running anything. - A `#!` line's option flags were discarded: `#!/usr/bin/env -S bash -euo pipefail` lost pipefail because the runner is launched as `bash <path>`. The generated posix runner now replays declared flags via `set` and drops the duplicate interpreter line. - Docs cover the per-user setup command in repository hook settings, which goes through the same `#!` rule, and describe what the `#!` line does and does not select. Tests: composed launch command for a POSIX pane + cmd runner (hooks, shared runner command, setup sequencing gate, observed-setup signal), the cmd runner's shebang refusal, and shebang flag replay. Each fails with the source reverted. * fix(worktrees): replay only real `set` flags and keep the gate in the pane's shell Two round-2 review findings: - `#!/bin/bash -l` replayed `set -l`, which exits 2 and aborted the runner under its own `set -e` before a single setup line ran (all platforms). Only the flags `set` documents are replayed now; a bare `-o` with no option name is dropped instead of dumping the shell-option table. - The wait-for-setup gate picked its language from the runner file, so a batch runner launched from a Git Bash pane got the PowerShell gate while the agent startup command was already POSIX-quoted — `Invoke-Expression` cannot parse `'\''`. The gate now follows the pane; the runner still launches through the ProcessStartInfo launcher, never through bash. --------- Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> |