mirror of
https://github.com/stablyai/orca.git
synced 2026-10-21 16:03:26 +00:00
d2000d56fbaf6fd4742101761c8cc6f6019b7c02
280
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 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.
|
||
 Brennan Benson
|
9ab0a18e82 |
refactor(agent-status): isolate legacy status ingress behind one admission point (#20716)
* refactor(agent-status): isolate legacy status ingress * fix(agent-hooks): move advertised-capability source onto the ingest envelope ingestRemote() gained a third positional argument in this PR (advertisedAgentStatusCapabilities) to satisfy a new ratchet requiring every legacy-ingress call site to name its capability source. Both production callers pass the same constant every time, so the argument carries zero runtime information — but Vitest's toHaveBeenCalledWith matches argument count exactly, so the pre-existing SSH relay integration test (which asserts a 2-argument call) started failing even though nothing about the actual admission decision changed. Capabilities are a property of the producing peer/connection, not an orthogonal call parameter, so move the field onto the envelope object instead of adding a third positional argument: ingestRemote reads envelope.advertisedAgentStatusCapabilities (defaulting to the unadvertised-legacy-peer set), and both call sites stamp the constant onto their envelope literal. Call arity stays at two arguments, so the pre-existing evidence test needs no change. The envelope never crosses the wire in either caller: SSH rebuilds it field-by-field from the RPC params, and the WSL path copies (never mutates) the wire-deserialized notification before stamping the field on, so this is purely an internal main-process shape change. Also strengthens the ingress ratchet test that required this: it previously only checked that the capability constant's name appeared somewhere in each caller's source, which a stray unused import could satisfy. It now asserts the actual `advertisedAgentStatusCapabilities: AGENT_STATUS_LEGACY_UNADVERTISED_PEER_CAPABILITIES` key:value binding is present. |
||
|
Neil
|
231e805b1e |
fix(lint): enable anti-slop/no-shape-in-symbol-names (#20785)
Flip `anti-slop/no-shape-in-symbol-names` from "off" to "error" and clear
every violation under src, config, tests and mobile.
What the rule bans
------------------
The case-insensitive substring "shape" in any JS/TS identifier: variables,
functions, parameters, types, type parameters, class members, private names,
object-literal keys and JSX identifiers. The one exemption is a statically
accessed member read owned by another value (`zodObject.shape` is fine), so
third-party APIs stay readable without a suppression.
"Shape" names a value's structure rather than its domain role. `UserShape`,
`validateArgShape` and `errorShape` all tell you the symbol is "an object
with some fields" -- which is already what a type says -- while saying
nothing about what the value is for or who owns it. The rule forces the
name to carry the domain instead.
Violations fixed
----------------
689 violations across 109 files at baseline (verified by re-running the
audit against the pre-change tree with the rule set to "error").
Fix pattern
-----------
Rename for the domain role, not the structure:
-type FieldShape = 'list' | 'map' | 'whole'
-const FIELD_SHAPES = { ... } satisfies Record<keyof Observation, FieldShape>
+type FieldEncoding = 'list' | 'map' | 'whole'
+const FIELD_ENCODINGS = { ... } satisfies Record<keyof Observation, FieldEncoding>
-function assertGitPushTargetShape(target: unknown): void
+function assertValidGitPushTarget(target: unknown): void
-function describeReadDirPathShape(p: string): ReadDirPathKind
+function classifyReadDirPath(p: string): ReadDirPathKind
Predicates became statements about the value (`isDeltaShapedProviderFrameKind`
-> `isDeltaProviderFrameKind`, `isDeleteShapedDiscardEntry` ->
`discardDeletesEntryFile`, `isSkillsCliAgentKeyShaped` ->
`isUsableSkillsCliAgentKey`). Type aliases dropped the suffix where the
remaining name was already unambiguous (`GhGraphqlErrorShape` ->
`GhGraphqlError`).
No wire-visible name was renamed: no IPC or RPC channel, stream opcode,
request/response param, persisted field, or i18n key. The `--shape=symlink|copy`
CLI flag read by .github/workflows/skill-update-roundtrip.yml is unchanged --
only the local variable holding it was renamed.
Exemptions
----------
They are file-scoped entries in config/oxlint-anti-slop.json, not inline
`oxlint-disable` comments. An inline directive naming an anti-slop rule reads
back as an UNUSED directive under the root lint scan, which does not load this
plugin -- the changed-code quality gate counts that warning, so the comment form
cannot be used for a rule that lives only in this config.
* src/renderer/src/components/browser-pane/annotate/**:
in the screenshot annotator a "shape" is the drawn geometry -- pen, arrow,
rect, ellipse, highlight. That is a genuine domain noun, and it pervades
every symbol in the module.
* repo-icon.tsx, repo-header-project-actions.tsx, mobile MobileRepoIcon.tsx:
lucide exports the icon component as `Shapes`. The name is theirs, and the
matching REPO_LUCIDE_ICONS key is the persisted icon name shared with the
desktop picker -- renaming it would orphan saved repo icons.
* src/shared/onboarding-state-types.ts, src/shared/constants.ts:
`shapedSidebar` is a persisted onboarding-checklist field and a telemetry
enum member; renaming it would orphan saved state.
* src/shared/rpc-contract/rpc-send-params.ts: matching zod's own literal `shape`
property is what selects the ZodObject branch of the conditional type.
No exemption was added merely to avoid a rename. Eight symbols initially
suppressed as "a cross-module refactor outside this change" were proven to have
zero non-TypeScript references repo-wide and renamed instead.
Zod's `ZodRawShape` needed no exemption at all: `Readonly<Record<string,
z.ZodType>>` is its definition, so repo-update-params.ts and
ui-update-value-tolerance-params.ts spell it out instead. Likewise
telemetry-event-classification.ts now reads `.shape` through an `in` narrowing,
which also retires two pre-existing type assertions; three more assertions the
rename had dragged onto changed lines (two `JSON.parse` sites, one node:sqlite
row read) became annotations and an explicit row mapping.
Verified
--------
* Audit reports zero violations; confirmed the rule genuinely fires by
planting a probe violation.
* node config/scripts/run-typecheck-projects-in-parallel.mjs exits 0.
* Vitest over src/shared, src/main/github/project-view, the annotate module,
the repo-icon components and the Chromium SameSite electron spec: all green.
* All 66 removed "shape" identifiers grepped repo-wide across every file type;
none survive.
* node config/scripts/generate-rpc-params-catalog.mjs --check exits 0.
* node --check on every changed .mjs; oxfmt clean on all changed files.
* `pnpm run check:code-quality:changed` reports 0 findings.
Not machine-verified: the 3 mobile/ files (its Vitest run cannot resolve
`expo/tsconfig.base.json` in this worktree), and the WSL- and Playwright-gated
specs. All are rename- or comment-only hunks, read in full.
|
||
  JinjingandJinwoo-H
|
d2d32691ef |
perf(persistence): skip redundant whole-state flushes on terminal reattach (#20137)
* perf(persistence): add pty-binding fast lane to skip redundant flushes Terminal pane reattachment currently clones the session and serializes the entire 9.2 MB app state even when the binding is already in place and durable. Add an early-return fast path that skips this work when all nine predicates hold: no split, binding matches in-memory and on-disk, incarnation matches, no tombstone, and generation counter proves durability. Includes one-line fix in `writeToDiskSync` to record hash-matched sync flushes as durable, so the fast path doesn't stay parked behind a stale generation. Adds `persistence.pty-binding` observability spans (local NDJSON, unsampled for mutations, budgeted for fast-lane hits) to measure eligibility rates before and after. Includes ratchet test to ensure every binding writer bumps the generation. Diagnostic tools and full investigation notes from September 7, 2026 capture that identified the 59–100 ms no-op binds and measured a real terminal keystroke queued 117 ms behind one such call. * perf(persistence): add pty-binding fast lane to skip redundant flushes Rapid rebinds of already-durable PTY bindings (e.g., remounting panes) were unnecessarily expensive because they cloned and flushed the entire document state every time. Detect when a binding hasn't changed since the last durable write and skip to return immediately, eliminating main-thread cost on that path. * perf(persistence): record binding.origin on the pty-binding span Fresh spawns always flush, so a fast-lane rate over all calls is diluted by however many terminals the user opened. Each caller knows whether it is a spawn, a reattach, a split, or a relay reattach; pass that through as metadata and record it so the reattach hit rate can be read from the trace file. Never branched on. * fix(persistence): keep the tab row on its first pane when a sibling pane binds A tab row names one PTY, but a split tab holds several panes. The renderer keeps the row on the first pane and refuses to let later split-pane spawns steal it, since a remount reattaches the tab to whatever the row says. Main overwrote it with whichever pane was binding, and the renderer's next publish put it back, so every sibling reattach was a state change and could never take the fast lane. On the real profile that is 38% of panes. Rewrite the row only when it names nothing useful: null, the PTY this leaf is replacing, or a PTY no leaf holds. The fast-lane predicate compares against the same rule. * perf(persistence): record durable pty-binding flushes per pane The global write generation is held back by any unrelated dirty state, causing bindings unchanged for minutes to appear unpersisted despite being on disk. Track per-pane durability to skip redundant flushes. * docs(persistence): describe the per-pane durability record The durability section still described the global generation check as the whole story and claimed there was no binding durability cache. Record the measurement that motivated the per-pane record, and why retiring one needs no cooperation from other binding writers. * docs(perf): consolidate every measured Orca performance issue into one register Folds the findings from all related debug sessions into the live lag investigation: the persistence/main-thread work (P1-P11), host contention (H1-H5), git and subprocess load on main (G1-G8), renderer and terminal rendering (R1-R8), the terminal daemon session leak from the deleted debug-orca-perf-issue worktree (D1-D9), and the Cmd-J palette review (C1-C6). Keeps the measurement behind each claim, records what is fixed versus open, and restates what the 117 ms keystroke delay still does not explain. * fix: address performance review findings * fix: satisfy diagnostic probe lint * chore: keep investigation artifacts out of performance PR * fix: run lag probe regression tests with Vitest * perf(persistence): replace pane receipts with global durability check * refactor(persistence): remove redundant binding review machinery * test(persistence): satisfy current assertion-free quality gate --------- Co-authored-by: Jinwoo-H <jinwoo0825@gmail.com> |
||
|
Brennan Benson
|
33149fcde5 | fix(claude): install SessionEnd for capable versions (#20530) | ||
|
Brennan Benson
|
2ce252f471 |
fix(grok): announce a completion once, when Grok is actually finished (#20523)
* fix(grok): announce a completion once, when Grok is actually finished Orca pinged on every Grok turn-end. Grok runs turns the user never asked for: when a background task finishes it wakes itself, does a little work, and ends another turn. One request produced several pings. Grok already reports, on every turn-end, whether it still has work outstanding. Read that instead of trying to classify which turns are "real": backgroundTasks absent -> silent, this is the session-end tail StopFailure / StopCancelled -> announce, a failure is never hidden stopHookActive -> silent, a Stop hook is keeping it working a shell task or subagent running -> silent, the work is not done otherwise -> announce Nothing here knows what an auto-wake turn is. A turn that ends with work outstanding stays quiet; the later turn where that work is finally done is the one that announces. That is also why this survives the case where Grok completes a user's goal inside one of those turns — prefix-based suppression would have silenced it. Monitors and scheduled entries are deliberately not counted as outstanding work. They can run indefinitely, so counting them would suppress a user's completion permanently, and a lost ping is worse than an extra one. Also registers StopCancelled, which Grok fires instead of Stop on a user interrupt, a declined permission, --max-turns, or a no-progress bail-out. Orca never subscribed to it, so those turns were reported as successes. Also removes a stale notification matcher that searched for prose the shipping binary never sends; the typed notification kind is matched instead, and neither idle_prompt nor task_complete is treated as a completion. Needs-input behaviour (permission prompts and ask_user_question waits) is unchanged and stays ungated by background work. * fix(grok): never hide a failed or cancelled turn behind the background-work gate The announce predicate checked field-absence before terminal outcome. Grok's StopFailure and StopCancelled payloads carry no background inventory at all, so the absent-field branch — added so the session-end tail stays silent — fired first and silenced every failure and every cancellation. That inverted the rule it was meant to serve. Before this series a cancelled turn at least surfaced as a (wrong) success; gated this way it surfaced as nothing. Terminal outcome is now checked first, so a failure or cancellation announces regardless of what other fields the payload happens to carry. The existing tests passed straight through the bug because they built failure payloads with a backgroundTasks field Grok never sends for those events. They now model the real payload shapes, verified against the provider's payload definitions and the captured envelopes. * fix(grok): settle completion from provider lifecycle state * fix(grok): fence stale turn ends without prompt ids |
||
  
|
96b450fae8 |
fix(ssh): bound relay incumbent lsof probe (#18304)
* fix ssh relay incumbent probe timeout
* test ssh relay unconfirmed probe termination
* fix(ssh): preserve connect evidence while bounding lsof
* fix: preserve uncertainty when relay holder enumeration fails
* fix(ssh): supervise lsof helpers and preserve partial holder evidence
* test(ssh): prevent GC racing unconfirmed probe cleanup
* refactor(process): keep POSIX lsof supervision in process owner
* fix(ssh): confirm census cleanup and handle probe startup signals
* fix(ssh): keep lsof holder evidence usable on hosts with unstat-able mounts
lsof warns to stderr about mounts it cannot stat, and any stderr byte forced
the holder enumeration to 'unavailable' — making the 'exited' verdict, and so
husk reaping, unreachable on those hosts. Pass -w to suppress the warnings.
* Revert "fix(ssh): keep lsof holder evidence usable on hosts with unstat-able mounts"
This reverts commit
|
||
 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 |
||
 Brennan BensonandMerge Sim
|
da5d555259 |
refactor(agent-status): delete the runtime's retained row store (PR 1b) (#19785)
* docs(agent-status): plan PR 1b at file level Names the five RuntimeAgentRowStore call sites and what each becomes, why terminalHandle has to be stamped before the store can go, and the one intended behavior change. * feat(agent-status): stamp the pane terminal handle on hook-server rows The runtime's retained row store carried the pty binding two readers need. Put that fact on the row that already owns the pane instead, resolved through the same lookup the renderer-facing IPC boundary runs, so the two surfaces cannot disagree about which terminal a pane is. Carried forward when a later write resolves no handle (only main's OSC parse can), and never persisted: a handle belongs to the runtime that issued it. * refactor(agent-status): route the session-tabs republish off the store `retain()` was not only a duplicate store: its boolean return was the signal that republished `session.tabs` for a status-only transition, which no title change covers (#7970). `hook-status-session-tabs-invalidation.ts` already mirrors that change set plus hook restore provenance, so route the signal off the store rather than keep a second comparator. Adds the status-drop arm a user dismissal emits, which the pane-clear fan-out deliberately skips — now load-bearing, because a dismissed row leaves the listing at once. Installed on both hosts. orcad had neither the OSC producer nor this signal, so its runtime observed agent status and published it nowhere; deleting the retained copy without wiring it would list no PTY agents there at all. * refactor(agent-status): delete the runtime's duplicate retained row store `RuntimeAgentRowStore` held the same payload the hook server already holds, so the same pane could legitimately read differently in the sidebar, in `worktree ps`, and on the phone. Both of its readers move onto the store's snapshot in `runtime-hook-agent-row-selection.ts`, and `collectRuntimeWorktreePtyAgentSources` loses the retained-versus-hook reconciliation that only existed because two stores could disagree. `ConnectedPtyEvidence` trades its flat pty-id set for `ptyIdByTerminalHandle`, which is how a row still resolves the connected PTY behind it — the working-terminal rollup's match key, and the last rescue for a row whose pane binding a controller incarnation nulled under it. The one intended behavior change: a row the user dismisses on the desktop leaves `worktree ps` and mobile at once instead of lingering until the pty exits. One store means one dismissal. The suites written against the retained store are rewired to a real AgentHookServer rather than deleted, so each still asserts the listing behavior it named. * docs(agent-status): record what PR 1b landed Past tense, plus two corrections to the plan: `terminalHandle` is not the pty id (they are different identifiers, and the explicit-status reader was already comparing against a real handle), and the legacy numeric pane key is a consequence the plan did not name. * fix(agent-status): harden single-store lifecycle * fix(agent-status): preserve mobile terminal rejoin * fix(agent-status): preserve unverifiable remote rows * fix(agent-status): own PTY row lifecycle in hook server * fix(agent-status): preserve state and renew freshness * fix(agent-status): ignore freshness for dismissed identity rows * fix(agent-status): fence orcad observed identities * fix(orcad): always release daemon adapter on cleanup * fix(agent-status): cover remint and headless lifecycle edges * fix agent status identity recovery gaps * fix(agent-status): suppress duplicate child-only row mutation * test(runtime): preserve hook store wiring in transcript harness --------- Co-authored-by: Merge Sim <sim@local> |
||
|
Brennan BensonandMerge Sim
|
bcb703fb4c |
test(ssh): isolate the MFA fixture from the developer's real ~/.ssh (#19300)
The multi-stage cases pass `resolved: null`, so `resolvePrivateKeys` falls through to `findDefaultKeyFile`, which reads `~/.ssh/id_*` via `homedir()`. On a machine with an encrypted default key ssh2 rejects with "Cannot parse privateKey" before authentication is exercised, so two cases failed locally while staying green on hosted CI, which has no key. Point home at the existing fixture directory so default-key discovery stays in the test's control. Co-authored-by: Merge Sim <sim@local> |
||
|
Jinwoo Hong
|
0c33f58e8a |
fix(ssh-relay): daemon owns the endpoint credential; a losing start never rotates it (#19052)
<!-- orca-pr-loc -->
<!-- Programmatic LoC summary. Do not edit by hand; rewritten on every commit. -->
| | Files | Added | Deleted | Net |
| :--- | ---: | ---: | ---: | ---: |
| Test | 19 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$962 | $\color{#cf222e}{\Huge{\mathbf{−}}}$136 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$826 |
| Prod | 18 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$295 | $\color{#cf222e}{\Huge{\mathbf{−}}}$116 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$179 |
<!-- /orca-pr-loc -->
## Symptom
Live 2026-09-05 (Orca 1.4.198 client, Ubuntu host): both relay processes `kill -STOP`ped for 20 s, then `-CONT`. The client redeployed while the host was frozen. Its fresh daemon lost the socket bind (`Socket path already in use`) but had **already rewritten** `relay-<id>.sock.credential`. The surviving daemon kept its in-memory credential, so every later `--connect` got `Endpoint credential mismatch; closing socket`, then `Grace started … timeoutMs=0 … ptys=1, clients=0` every ~20 s, forever. Only a manual `kill -TERM` cleared it. Receipts: `review-archive/orchestration-v3-pr16904/smoke-receipts-t012b/E16,E17,E18,E24`.
Three independent defects kept the wedge alive; each is fixed at its own seam.
## Fix
**1. The relay daemon owns credential publication (race-free under two concurrent starters).**
`relay-daemon.ts` binds the socket first, then publishes via the new `src/relay/relay-endpoint-credential-publication.ts`: adopt a valid pre-existing file (older clients still pre-write), else mint 32 random bytes and write temp+rename at 0600. A start that loses the bind exits inside `listen()` and never reaches the file. Why this option and not restore-on-loss or a client-side write: the only process that can *prove* ownership is the one whose `listen()` succeeded, and that proof is atomic with the bind. The client-side pre-write (`ssh-relay-endpoint-credential.ts`) and the launch-command `chmod 600`/`icacls` are removed on POSIX and Windows. The racing test also exposed that macOS reports a mid-bind collision as `EEXIST` rather than `EADDRINUSE`; `relay-socket-ownership.ts` now treats both as "held or stale".
**2. The client distinguishes "no daemon" from "daemon present but not answering", and never rewrites.**
A credential refusal is now typed on the wire: the daemon replies `orca-relay-handshake-credential-mismatch` (same frame type, no new opcode) and the bridge exits **43**; `waitForSentinel` maps it to `RelayCredentialMismatchError`, which the takeover treats as handshake-refusal evidence exactly like exit 42. A relay that holds the endpoint but **never refused** (the stalled-host shape: kernel backlog accepts the probe, handshake gets no answer) is now `RelayEndpointUnresponsiveError`, routed to the relay-lost backoff instead of the terminal Reset Relay path. Silence is not a decision (`docs/reference/ssh-execution-boundary.md`).
**2b. Deploy honours the verdict.** The 40 s live run exposed that the `--connect` catch block in `deployAndLaunchRelay` predates the incumbent probe and swallowed both verdicts as "probe failed, launch fresh", so a fresh daemon was still launched over the live one (it lost the bind by luck, which is exactly the collision in the incident). Held and Unresponsive now propagate; the session backs off on Unresponsive and surfaces Reset Relay on Held. Red-first in `ssh-relay-deploy-incumbent-verdict.test.ts`.
**3. The daemon cannot be wedged by a rotated file, because nothing can rotate it.**
The credential lives in the content-hashed relay dir, and after (1) the only writer is the daemon that owns the socket, so the "file changed under a live daemon" state the incident depended on is no longer reachable in-product. The credential is therefore fixed for the daemon's lifetime, as a plain secret should be. A hand-edited file is refused with the typed reply until restored (tested). Startup adoption of a pre-written file applies an owner-only + same-uid rule (review finding): anything else is replaced by a fresh mint. An earlier revision of this PR also re-read the file on mismatch and adopted it; that was removed as unreachable machinery that turned the credential into a per-handshake file-ownership check.
**3b. Fail closed between bind and publication.** A client that arrives after `listen()` resolves but before the credential is set is refused, not admitted as `unproved`. Nothing can be delivered in that window today; the guard makes the boundary structural instead of an event-loop ordering fact. Red-first in `relay-reconnect-listener-credential-gate.test.ts`.
**Wire compat.** New optional handshake reply only; an old `--connect` hits `Unknown handshake type` and exits 1 pre-sentinel, which it already treated as a generic failure. New daemon adopts an old client's pre-written file; new client still passes `--credential-file` so an old daemon reads it as before. Absence of exit 43 is never used as evidence.
**Also.** `terminal create` on a reconnecting SSH host now says what to do instead of a bare `No PTY provider for connection "<id>"` (prefix preserved; the renderer matches it).
## Tests (red first)
- `src/relay/subprocess.test.ts`: two `--detached` starts race one socket + credential file → exactly one reaches the sentinel, loser exits 1 with `Socket path already in use`, file valid + 0600, a `--connect` reading it reaches `relay.status` and reports the winner's pid. Red before (both starters died: daemon required a pre-existing file), green 6/6 after.
- `src/relay/relay-endpoint-credential-publication.test.ts`: mints after bind; adopts a pre-written 0600 file; replaces a pre-written 0644 file with a fresh mint; refuses a stale credential with exit 43 while still serving the real one, and keeps refusing a rewritten file until it is restored.
- `src/relay/relay-reconnect-listener-credential-gate.test.ts`: a client in the bind-to-publish window is refused and never attached; after publication the right credential is accepted and a wrong one refused; a daemon launched without a credential file is not gated. Red without the guard.
- `ssh-relay-deploy-incumbent-verdict.test.ts`: live-but-silent incumbent → `RelayEndpointUnresponsiveError`, refused → `RelayEndpointHeldError`, and in neither case is `--detached` launched; a failed `test -S` probe still launches fresh. Red 2/3 without the deploy change.
- `ssh-relay-deploy-helpers.test.ts` (exit 43), `ssh-relay-endpoint-takeover.test.ts` (refused → Held even with no `lsof`; silent → Unresponsive, nothing unlinked or signalled), `ssh-relay-session-terminal-error.test.ts` (Unresponsive → `onRelayLost`, not terminal). Deploy/namespace/native-deps tests updated to assert the client writes **no** credential.
## Live proof
New `tests/e2e/ssh-docker-relay-stall-credential.spec.ts` (claimed in `run-ssh-docker-e2e.mjs` and PR source routing), two cases: `kill -STOP` every relay pid in the container, send input during the freeze, hold **20 s** (the incident's duration, which races the mux liveness timeout) or **40 s** (past it for sure), `kill -CONT`; assert status back to `connected`, same pty, same daemon pid, same credential inode and content, relay.log did not shrink (a relaunch truncates it) and has zero `Endpoint credential mismatch` / `Socket path already in use` lines, in-stall input delivered at most once.
Run output (local, fixture image `orca-e2e-ssh-relay:3a864c665ba2cefd`, `ORCA_E2E_SSH_DOCKER=1 SKIP_BUILD=1 ORCA_E2E_FORWARD_APP_LOGS=1 … --project electron-headless --workers=1`, head `c2c20fd994`; re-run identically on the final head after the credential-lifetime change, 2 passed (1.7m), same annotations, and the bind-to-publish refusal never fired):
```
✓ keeps the same daemon and credential across a 20s relay freeze (38.3s)
relay-processes-stopped: 2 relay-processes-continued: 2
bridge-pids-before-after: 480 -> 480
socket-clients-accepted-before-after: 1 -> 1
in-stall-input-delivered: 1
✓ backs off and reattaches, never relaunching, across a 40s relay freeze (57.5s)
relay-processes-stopped: 2 relay-processes-continued: 4
bridge-pids-before-after: 480 -> 1202
socket-clients-accepted-before-after: 1 -> 3
in-stall-input-delivered: 1
2 passed (1.6m)
```
Client log in the 40 s case shows the new path end to end: `Relay channel lost … reconnect attempt 1/6` → `Socket probe result: "ALIVE"` → `Socket reconnect failed … Relay failed to start within 10s` → `Relay endpoint incumbent: … verdict=live evidence=accepted-connection holders=unenumerable` → `Failed to re-establish relay … A relay still owns … but did not answer the handshake … Orca will retry` → `reconnect attempt 2/6` → `Reconnected to existing relay via socket`. The 20 s case never left the frozen bridge (same bridge pid, one accept), so it exercises the "silence is not death" side of the same race. The 20 s case passed 6/6 across the session; the 40 s case was red on the prior head (`Socket path already in use` + `Startup failed: listen EADDRINUSE` in relay.log from the swallowed verdict) and is green after 2b. Before the fix the same injection produced a fresh daemon that rewrote the credential and a survivor refusing every client.
The `relay-processes-continued` count exceeds `stopped` in the 40 s case because the timed-out client's `--connect` bridge and the loser-side processes are parked behind the frozen listener when `CONT` runs; they exit on their own once it resumes.
## Gates
`pnpm test src/relay src/main/ssh` 332 files / 3884 tests pass · `pnpm typecheck:tsc:node` clean · `check:code-quality:changed` 0 findings · `check:react-doctor:changed` 0 findings · `pr-e2e-gate-contract.test.mjs` 42 pass · no lint disables or max-lines bumps added.
## Noted, not fixed here
- `terminal list` `orphaned:false` / `terminal close` `ptyKilled:true` for a pane whose relay is gone (`orca-runtime-stop-explicitly-closed-tab-ptys.ts`): different seam, `@ts-nocheck` characterization-covered file.
- On a host with no `lsof`, a stalled relay still cannot be enumerated as the holder; it is now retried rather than declared held, but a relay frozen past the backoff budget still ends in the existing "reconnect manually" banner.
|
||
|
Jinwoo Hong
|
06a607a1d7 |
feat(orchestration): make multi-agent workflows durable (#16904)
<!-- orca-pr-loc -->
<!-- Programmatic LoC summary. Do not edit by hand; rewritten on every commit. -->
| | Files | Added | Deleted | Net |
| :--- | ---: | ---: | ---: | ---: |
| Test | 225 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$21666 | $\color{#cf222e}{\Huge{\mathbf{−}}}$2820 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$18846 |
| Prod | 348 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$17107 | $\color{#cf222e}{\Huge{\mathbf{−}}}$4706 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$12401 |
<!-- /orca-pr-loc -->
## ELI5
Orca now treats orchestration like a durable control plane instead of inferring success from terminal keystrokes. Agents can tell whether a prompt was accepted or a turn started, replay an ambiguous request without sending twice, and recover coordinator mail after a crash. Completed workers can be inspected, released, or retained, and their panes no longer auto-resume as if the work were still running.
## What changed
- **Run receipts** from `run-create/use/current/show/list` are the row without routing plumbing (`home_database`, `coordinator_pane_key`) and without the duplicate `binding` object.
- **`terminal send` receipts are honest and idempotent.** `input_accepted` and `turn_started` are the only stages; `--wait-submit` observes without resending; `--retry-request <uuid>` replays the exact request against the same process incarnation. A transport timeout keeps the retry ID; only a different runtime answering strips it. Value-less or non-UUID `--retry-request` is rejected on the CLI and the SSH shim.
- **Mailbox delivery is committed before wakeup.** Pointer writes are staged in the DB before any PTY byte, replayed once after restart, and never emit a naked Enter. The watermark that parks concurrent deliveries is released with the DB reservation. Restart rescans pointer-pending and `dispatch:` mailboxes.
- **Lifecycle is a guarded transition graph** (`lifecycle-transition.ts`) with a table-driven test over every caller edge. Task reopen/overturn stays in the public contract. A PTY exit during `worker-stop` is the stop succeeding, not a failure.
- **Worker lifecycle CLI:** `worker-start` (`--spec` creates Task + attempt in one call), `worker-show`, `worker-read` (provider transcript first, bounded terminal fallback with a typed reason, local/WSL/SSH), `worker-stop`, `worker-abandon`, `worker-release`, `worker-retain`, `worker-list` (rowid-fenced pagination, fleet liveness, `attention`, literal `nextAction`).
- **Release is an explicit ownership table** (`decideWorkerTerminalRelease`): only an `owned` resource can be settled, the archive is mandatory where reachable, and an owner whose process is proven exited can always get out of `retained` via `archive_status: unavailable`. User-taken-over, external, and transferred panes stay retained.
- **Settled-worker resume fence** (folds in #17651): a settled dispatch whose pane is still open is fenced at settlement, on stop/abandon/exit, and at startup; lifted on release, retain, takeover, and pane reuse.
- **Liveness is `live` / `unverifiable` / `exited` only**, from execution-host evidence. Fleet projection reads the evidence clock, not the relay delivery clock. A host-certified exit outranks the worker's settled state. `unverifiable` never authorizes stop, abandon, retry, or release, in code or in the guide.
- **Federation:** structured reads negotiate by `method_not_found` so every shipped host keeps transcript-first output; exited remote workers are closed before being reported closed; epoch fencing holds across peer restart, downgrade, and pairing rotation; no per-second forced capability probe.
- **Schema v35:** repairs databases stamped v34 by the pre-fix branch (mailbox_handle default, index predicates), drops the write-only `lifecycle_transition_receipts` ledger and five never-read v31 identity columns.
- **Schema v36:** `dispatch:<id>` mailboxes get a real consumer generation on `dispatch_contexts` and `remote_dispatch_attachments`, bumped and fenced in the same transaction on every re-attach (manual inject, worker-start, federated attach). A stale worker whose Dispatch moved to another process now gets `consumer_fenced` instead of silently acking the new worker's Delivery. Run mailboxes already worked this way.
- **Schema v37:** `dispatch_contexts` records its creator (`creator_handle`, `creator_pane_key`), so a coordinator's context-only self-dispatch is bookkeeping rather than a nesting parent; before this, one self-dispatch made every later `worker-start` from that coordinator fail the depth cap. Pre-v37 rows keep counting (fails closed).
- **Dispatch-mailbox ownership is checked, not inferred.** A `check` from a process whose pane no longer holds the Dispatch, or whose last Attempt was abandoned/failed and moved to another terminal, gets `consumer_fenced` instead of an empty inbox that reads as "no mail yet". `--peek`/`--all` stay readable. A paneless caller still gets `stable_pane_required` with the rebind recovery.
- **Liveness certification is stricter:** a `process_exited` stage whose termination reason is `unknown` (a stop that was issued but never observed) projects `unverifiable`, not `exited`. Federated `worker-show` carries the execution host's verdict and host kind instead of a local guess. A live, ready worker with nothing pending has `nextAction: none` rather than pointing at the `worker-show` that produced it.
- **Wire:** `workerShow` keeps `dispatch.task_id` next to `taskId` for shipped CLIs. `ask --json` uses the standard `{ok, result}` envelope like every sibling verb.
- **Migration start-version detection** treats the two v32 recovery columns as versioned. Before this, every shipped database stamped below 32 resolved to the v6 floor and replayed the whole chain (the v23 backfill synthesized 68 phantom retained workers on a real v30 profile). Verified on a copy of a real 62 MB v30 profile: starts at 30, no row delta, integrity ok, 11 ms.
- **Skill guide** rewritten as a ≤200-line kernel plus seven references, to the outcome-first standard (Result / Done / Safe failure first, conditions not case lists, one done bar, references loaded at the point of use). The canonical loop uses `worker-start --spec`, names `worker-list` for completion accounting, documents `--retry-request` / `request-show` / `--wait-submit`, and requires positive evidence before any stall action. The other seven guides get the same treatment in #18724, split out so this PR stays orchestration-only.
- **`rpc/methods/orchestration-*`** (126 flat files) regrouped into `orchestration/{worker,federation,messaging,runs,gates}/`.
## Why
User reports showed the same boundary failures: false `agent_prompt_stalled` causing duplicate sends (#15180), coordinators unable to trust screen scrapes, cold-parked terminals receiving a pointer without the submit, settled workers accumulating as live tabs and auto-resuming after restart, and no way to tell a stalled worker from a working one.
## Linked issues
Fixes #15180. Fixes #17935 (orchestration skill description is 866 characters; a guard now caps every bundled skill at 1,024). Supersedes #17651 (fence folded in). Advances #16660, #16522, #14907, #13047.
## Review record
This PR was reviewed adversarially after revival: eight independent lenses (lifecycle, mailbox, send, worker, federation, transcript, complexity, live ergonomics), each required to prove findings with a failing test. That produced 16 proven blockers, all fixed with red-then-green regression tests, followed by two re-review rounds and a third fix wave that caught 3 regressions introduced by the fixes and 7 fixes that missed their target; all closed. A final pass (five lenses incl. a live built-runtime smoke, then a re-review of the fix wave) found and fixed seven more, chiefly the stale-worker mailbox steal, the self-dispatch depth wedge, and the unproven-exit certification. Three independent Codex (gpt-6-astra) passes followed: the first found nothing new, the second found and fixed 3 defects (task-status reachability, WSL-local host classification, peer-capability epoch), the third found and fixed 6 (production PTY controller never installed settled writes, ambiguous in-flight pointer failures allowed duplicate replay, SSH/relay deadlines cut off a valid `--wait-submit`, stop-vs-exit race during inspection, and two release-recovery paths for vanished or exited terminals). The full record (findings, proof tests, triage, declines with reasons) is archived outside the repo.
**Rework after the live smoke.** A first live cross-host run on the shipped adhoc build (this Mac, a paired Windows host on the same build, a paired Mac on 1.4.195, and an SSH host) found a P1: a running local worker read `unverifiable`/`missing_status` because the fleet snapshot rows lacked the terminal handle the matcher keyed on. A 59-row failure table over every bug fixed during review showed the same two classes recurring: a fact dropped in transit through optional fields, and two authorities for one fact. Two blind designs (Opus, Codex) converged on the same mechanisms, and the scoped tranches landed here with red-then-green seam tests from the real producer to the real consumer, faults injected only at the transport or hook-ingest boundary:
- **Settlement (data-loss class):** one three-valued `WriteSettlement` (`accepted | refused{reason} | unverifiable{reason, bytesHandedToTransport}`) from the SSH multiplexer through daemon client, providers, controller, to pointer staging. No boolean, no rejection-as-third-state. The two silent degrades that fabricated a handoff are deleted; a provider that cannot settle refuses before any effect. Pointer text and Enter share the contract; a partial flush is `unverifiable`, never `refused`.
- **Evidence identity (false-liveness class):** fleet agent-status evidence is a tagged union (`binding: worker | pane | unresolved{reason}`, `clock: observed | delivery`) minted once at ingest, so a hook row captured on one process incarnation can never bind to a later dispatch on the same pane. The matcher's `!worker.paneKey ||` defaults are gone. One host-scope parser replaces two.
- **Small pre-merge items:** `capability_unsupported` from an old peer is no longer relabelled `host_unavailable`; a producer census test asserts every agent-status consumer path projects a pane-only hook row as `live`.
Two ergonomics defects the second live run surfaced on a real database are fixed here too: a pre-v3 dispatch already marked `completed` projected as `outcome_unknown` / `requiresAction: true` forever (three copies of the outcome ladder disagreed on legacy rows; now one resolver, legacy `completed` reads `succeeded` with nothing to act on, legacy `failed` stays actionable on the failure), and an unscoped `worker-list` enumerated the entire database (now defaults to the Run bound to the calling terminal, `--run` overrides, and the receipt's additive `scope` field says which).
A third live round on the shipped adhoc build of `b082443e1f` (same four hosts) plus an unscripted run in the user's own prompt style (a plain Claude Code shell, `/orchestration`, three workers, zero errors, bound-Run default confirmed) found two more branch defects, fixed with red-then-green tests: a worker freshly started on a paired server projected `unverifiable`/`host_indeterminate` with `requiresAction` for ~3 minutes, including after its own `worker_done`, because the host's federation observation returned `missing_liveness_verdict` for any PTY the liveness register had not yet swept (the host now reads a connected pane it owns locally as `live`; disconnected or SSH-scoped panes stay `unverifiable`); and six pre-v3 completed rows still carried an `input` category because settling through the task-status path or `failDispatch` never closed the Dispatch's pending question threads (both paths close them now, and schema v38 closes threads already pending on settled rows). The guide's `worker-start` examples now show `--model sonnet`, since an omitted model inherits the launcher's default.
A Codex adversarial pass on the tranche diff found one real design hole (identity minted at read time instead of ingest, now closed) and two daemon settlement paths that threw instead of settling (fixed). Two `@ts-nocheck` runtime mixins on these paths were extracted into checked modules; the repo-wide `@ts-nocheck` count is unchanged at 171.
Deletions during review: ~1,900 lines (write-only ledger, unread columns, dead v1 archive path, test harnesses shipped in prod, duplicated liveness and state-machine copies, self-capability checks that were compile-time true).
## Testing
- `pnpm typecheck:tsc:node|cli|web` clean
- `pnpm run check:code-quality:changed` 0 findings; `check:react-doctor:changed` 0
- `pnpm verify:bundled-skill-guides`, `verify:skill-bundle-manifest`
- full `pnpm test` on the integrated head: 72,332 pass / 292 skipped; the only failures were three non-PR files (two zsh live-shell suites hit a node-pty spawn-helper ENOENT while a concurrent native rebuild ran, 44/44 in isolation; `release-checkout.unit.test.ts` is a known 30 s load timeout that passes in isolation on `origin/main` too).
- CI on
|
||
 
|
6494f2a4f0 |
fix(native-chat): resume a structured chat from Agent Session History (#18933)
* fix(native-chat): resume a structured chat from Agent Session History Clicking Resume on a chat-UI row could only reveal an already-open tab. If the chat had been closed, or this process had never published it, the click re-read an inventory that did not contain it and toasted "Retry in a moment" — advice that could never come true, because nothing republishes an unpublished tab. The legacy `claude --resume` fallback is deliberately refused for structured-owned rows, so the row had no way back at all. `close` already keeps the record and the journal on disk so a session can be attached again, and the hold path already resurrects one in full. What was missing was the tab: `restoreReadableSessions` is latched to run once, at startup, so nothing could ask for a single session later. Adds `agentSession.reveal`. The host looks up its own record, restores the session readable, and republishes the tab through the same call `agentSession.create` uses. Deliberately narrow: - It takes no hold. A provider child exists because a surface asked, and the chat pane asks when it binds. - A journal it cannot read is not a refusal. A chat whose journal predates the SQLite store restores to nothing here, but attach still recovers it, so the tab is published and the pane's hold finishes the job. - Workspace and provider come from the record, never the client, so a session id alone cannot aim the publication at another workspace. Claude and Codex both, by construction: eligibility is `adapterSupportsRecord`, which the router answers from the record's own provider. Gated on a new advertised capability rather than probing for method_not_found, matching agent-session.structured.hold.v1 — absence is visible during negotiation instead of by calling. * fix(native-chat): negotiate reveal against the host that owns the workspace The capability gate read the LOCAL runtime's advertised capabilities while the call went to the host that owns the workspace, which for a paired workspace is a different build. On desktop the renderer and its local host are always the same build, so the gate passed unconditionally and proved nothing about the host being called: an older paired host still received the unknown method and its method_not_found was reported to the user as 'this chat is no longer on this host'. The cache it read also starts empty and resets to empty when status.get fails, so 'not fetched yet' and 'unsupported' were the same value. Gate on the environment that will answer, the way agentSession.close already does, and skip the round trip entirely for a local host. Reveal now reports four outcomes instead of a boolean, so a host that is merely too old is not reported as a chat that is gone, and a host we could not reach keeps the retryable message. Also syncs the localization catalog: the 'gone' key shipped without an en.json entry, which reddens static analysis and verify while typecheck stays green. * fix(native-chat): tell a refused reveal apart from a missing chat The host raises two refusals here and they mean opposite things to a user: it holds no such record, or it holds one no adapter of its own can open. The client collapsed both into 'this chat is no longer on this host', which is a eulogy for a chat still sitting on disk. Read the refusal code, and fold the host-side case in with the too-old host under one honest message, since the remedy for both is the same. Adds the coverage the readiness pass found missing: the host's reveal answer itself (workspace and provider from the record, both refusals, an unreadable journal, a live session), and the activation branches for a host that cannot open the chat and for one that never answered. * fix(native-chat): read a host version block as the host's age, not a lost link The capability probe reaches assertRuntimeStatusCompatible, which throws a runtime_compat_block error. Treating that as unreachable told a user with an out-of-date host to retry, which is the one thing that cannot help. Branch on isRuntimeCompatBlockError the way remote-agent-session-launch already does for the same probe. Also adds the refusal-code case a previous commit claimed and did not deliver: nothing drove a structured_agent_session_unsupported reply through the reveal client, which is the branch that commit existed to add. Corrects a doc comment that reveal made wrong: attach is no longer the only call that builds the host. * fix(native-chat): let a dragged history row reach the same reveal as a click Dropping an Agent Session History row onto a pane activated the tab by id and, on a miss, raised the very toast this PR exists to remove — so the same row answered a click and a drop differently, and the drop kept the advice that can never come true. The structured branch never used the drop pane, so routing it through the shared activation loses nothing and gains the reveal. The helper only ever read one field, so its parameter narrows to that field and the drag payload satisfies it directly. A source ratchet holds both entry points to the reveal-capable path, since a mounted drag harness does not exist for this layer and what regresses is a call site, not a rendering. * fix(native-chat): stop an advisory refresh ending the click, and one click per row Manual QA found the reveal never ran: the inventory refresh that precedes it is an optimization, but its failure returned early with 'not available yet, retry in a moment' — reinstating the dead end this PR removes, one step earlier. A failed refresh now falls through to the reveal, which is the repair and does not need the refresh to have worked. The click can chain a refresh, a capability probe, a reveal and a second refresh, each with its own timeout, while nothing on the row says it is working. A per-session in-flight guard keeps an impatient second click from running the whole sequence again and landing its own toast. Also drops an unreachable owner scope: the snapshot apply discards any worktree whose execution host is not local before it reads one, so naming a remote scope there described a synchronisation that cannot happen. * fix(native-chat): bound the capability probe and stop naming the wrong machine The in-flight guard releases when the activation settles, so an await that never settles holds the row for the life of the process. The capability probe was the one call in the chain not raced against a deadline: on a cache hit it awaits a promise an earlier probe created, which may carry no deadline of its own. Race it like the two calls around it. A version block can name either side — evaluateRuntimeCompat reports client-too-old as well as host-too-old — so a message that blamed the host pointed half of those at the wrong machine. Name the remedy instead of the machine, which is true for every case that reaches it. * chore: remove a scratch repro file committed by mistake It was swept into the previous commit by a broad `git add` while a diagnostic ran in this worktree. It asserts the current renderer-sync defect as expected behaviour, so it would fail the moment that defect is fixed. * fix(native-chat): stop a reveal's own inventory refresh discarding its republished tab Manual QA: the host answered reveal with ok:true and republished the tab, and the chat still did not reopen — only a renderer reload brought it back. The renderer publishes under one epoch string for its whole lifetime, and a frame recorded under a different lineage retires that epoch permanently with nothing to un-retire it. The Resume click asks for an inventory first, and a worktree the host holds no entry for answers with the none/v0 sentinel; the structured path recorded it, retiring the renderer's own epoch, so the tab the reveal published a moment later was dropped. A reload minted a new epoch, which is why reloading appeared to fix it. A frame that carries no publication is not a later publication to fence against. Treat the sentinel and a removal frame as a cursor reset, the way the mainstream session-tabs path already clears its tracking — its comment names this exact hazard: recording that sentinel would retire the host epoch and reject the next live frame. Pre-existing, and it swallows an ordinary new-tab launch on an empty worktree too; the reveal is what turned a silent invisibility into a visible failure. * fix(native-chat): let a retraction prune its rows without retiring the epoch Correcting the previous commit. Skipping a retraction frame outright stopped it pruning the mirrored rows, so a worktree the host no longer publishes would have kept a chat on screen with nothing behind it. Apply the frame as before and clear its cursors instead of recording them, which is what the mainstream session-tabs path does. The unpublished sentinel keeps its cursor now too: it is skipped rather than cleared, so a stale frame arriving late is still fenced. Adds the case the earlier version would have broken. * fix(native-chat): keep the retraction's fences, and fence the reveal's refresh Correcting the retraction handling again. Clearing its cursors was more than the bug needed and cost a guard: the host mints a fresh epoch when it rebuilds a pruned entry, so a republication is never gated by the retained cursor, while dropping it left an inventory response issued before the close free to land afterwards and strand a chat row for a worktree the host no longer publishes. Skip only the recording. The mainstream path keeps its epoch history for the same reason, as a tombstone fence. The test that justified the stronger clearing asserted a host behaviour that does not exist — a rebuilt entry republishing under the renderer's epoch with a restarted counter. It now uses what publishStructuredAgentSessionTab actually mints for a pruned entry, and a new case covers the frame that would strand. Also fences the reveal's inventory refresh on the sync generation, which every other caller that applies an inventory already does: structured chat can be switched off mid-flight, and the answer would otherwise re-seed a row into a renderer that just discarded them. * fix(native-chat): drop the retraction's epoch history, keep its version cursor Third and final shape for this branch, and the only one of the three that holds. Keeping both maps re-poisons the epoch one cycle later: the consumer here is also the publisher, so the history's current is the renderer's own lifetime epoch, and recording the reveal's fresh epoch retires it. The next chat the renderer publishes is then dropped — this bug again, one close later. Deleting both loses the guard that stops a frame issued before the close landing after it and stranding a row nothing republishes. So: clear the history, keep the cursor. The mainstream path keeps its history as a tombstone because there the epochs belong to a remote publisher; that reasoning does not carry to a path that publishes under its own. Each of the three variants now fails a different test. * fix(native-chat): a retraction forgets what is current, not the tombstones The delete lost a fence the cursor cannot replace: the version cursor only compares within a lineage, so a delayed frame from an already-superseded epoch had nothing left to stop it putting a chat row back for a worktree the host no longer publishes. Keeping the record intact had the opposite fault — the renderer's own epoch is the history's current, so the next frame under any other epoch retired it. Clearing only current does neither: noteRetiredValue retires nothing when there is nothing current, and the tombstones stay. Each of the four shapes now fails a different test. * fix(native-chat): narrow the retraction frame through its own type Typecheck caught what the tests could not: `removed` is not on RuntimeMobileSessionTabsResult. The repo already names the shape — RuntimeMobileSessionTabsRemovedResult — so this reads it through a guard rather than the inline cast the mainstream path uses. --------- Co-authored-by: Orca Worker <orca-worker@localhost> Co-authored-by: Merge Sim <sim@local> |
||
|
OrcaWinandOrca Worker
|
cff202c16a |
fix(windows): drop EDR-flagged -ExecutionPolicy Bypass from encoded PowerShell (#17880)
* fix(windows): drop EDR-flagged -ExecutionPolicy Bypass from encoded PowerShell
MDE flags `-ExecutionPolicy Bypass` paired with base64 `-EncodedCommand` as a
behavioural signal. Measured on Windows 11: neither `-Command` nor
`-EncodedCommand` is execution-policy gated (both run under an explicit
`-ExecutionPolicy Restricted` and `AllSigned`; only `-File` fails), so the
switch was a pure no-op on every one of these command lines.
Removes the switch from all four sites that spelled it, and de-encodes the one
site whose payload never passes through a re-parsing shell:
- ssh-remote-powershell: one chokepoint for ~40 remote-Windows call sites.
Base64 kept — the remote sshd DefaultShell re-parses this string.
- setup-agent-sequencing / windows-cmd-runner-delayed-launch: base64 kept —
these strings are typed into a terminal pane.
- windows-interactive-login-spawn: base64 kept — `cmd.exe /c start` re-parses,
and the cmd-safe-token guard rejects the `&` and `"` in the raw relay script.
- windows-mobile-firewall local runner: `-EncodedCommand` -> `-Command`, since
execFile reaches CreateProcess with no shell in between.
The setup startup gate keeps execution-policy relief in-payload (process scope),
because it evals a user-authored startup command that may invoke a `.ps1`, and a
`.ps1` IS gated. Caught by the real-process suite; mirrors the agent-hooks
launcher's trade.
The elevated firewall child deliberately stays encoded: `Start-Process
-ArgumentList` joins its array into one ShellExecuteEx string without quoting
and PowerShell re-splits on whitespace, measured to collapse `C:\My App\...`
to `C:\My App\...` — a firewall rule for the wrong program.
* test(ssh): enforce the no-script-file invariant remote payloads rely on
Dropping `-ExecutionPolicy Bypass` from `powerShellCommand` is a no-op only
while no remote payload loads a PowerShell script file — execution policy has
never gated anything else. That invariant held by inspection and was guarded by
nothing, so a future payload that dot-sourced, used `-File`, or imported a
`.psm1` would break only on a remote host with a Restricted/AllSigned
LocalMachine policy and no GPO: a failure on someone else's machine.
States the invariant at the wrapper, and adds a ratchet that scans every module
importing it for `.ps1`/`.psm1`, `Import-Module`, `-File`, and dot-sourcing.
The scan discovers importers itself (13 today) so new ones are covered, and
asserts it found some, so an emptied list cannot pass vacuously.
Mutation-checked: injecting each construct into a real importer fails the
matching case and names the file. The first dot-source pattern passed a
`;`-prefixed sample but missed `powerShellCommand(". '$x'")` — the likelier
shape — so the pattern now accepts a string-literal start and the self-test
samples carry their surrounding quotes.
* test(ssh): close two blind spots in the remote-payload ratchet
Both found by independent mutation testing of the ratchet itself, and both let
a real violation pass while the guard reported green.
`-File` was matched case-sensitively, so `-file $scriptVar` slipped through —
PowerShell switches are case-insensitive, and with a variable path the `.ps1`
pattern does not cover for it, so that shape escaped both nets. The naive fix
is wrong: bare /-File\b/i matches `--credential-file`, `--log-file` and
`--body-file`, which occur in three of these importers. Anchoring to a token
boundary catches the lowercase, odd-spacing and argv-element forms with zero
offenders across all 14.
Comment stripping paired a `/*` appearing inside a string (a glob such as
'src/*.ts') with any later comment close and deleted everything between, hiding
violations in the gap. Anchoring the block strip to line start, as the `//`
strip already was, fixes it — verified by injecting an `Import-Module` after a
glob string: the unanchored form misses it, the anchored form catches it.
Extends the same case-insensitivity to `.ps1`/`.psm1` and `Import-Module`,
which had the identical flaw (`import-module`, `DEPLOY.PS1` are legitimate
spellings); measured to add no false positive.
Each construct now carries the fixtures it must catch AND the near-misses it
must not, so a future tightening cannot quietly trade one for the other — the
negative fixtures are what would have caught the naive `-File` fix. Non-vacuity
bound tightened to >10 against 14 importers.
* docs(ssh): state what the remote-payload ratchet cannot see
The scan matches source text, so a script file reached only through a variable
(`& $scriptPath`) never appears in source and no pattern can catch it. The
ratchet narrows the hole; the invariant note on `powerShellCommand` covers the
remainder.
Recorded because a guard that reads as complete coverage when it is not is
worse than one that states its edge: the next author trusts it further than it
deserves, and should learn this limit from the test rather than an incident.
* test(ssh): scan remote payloads with the shared source walk
The ratchet had its own tree walk and comment stripper. The walk skipped
neither node_modules/dist/.git nor dot-directories and excluded tests by
`.test.ts` alone, so its importer count -- the guard's own goalpost -- could
be wrong about what it scanned. The stripper was anchored to line start to
dodge a `/*` inside a glob string, which silently skipped trailing comments;
`stripComments` tracks quote state and handles both.
Importer set re-derived against the shared walk: 15, floor unchanged at 10.
* fix(setup): report a failed execution-policy relief instead of swallowing it
The in-payload Set-ExecutionPolicy carried -ErrorAction SilentlyContinue and
an empty catch, so any failure vanished. A Windows PowerShell 5.1 install with
duplicate extended type data fails every cmdlet in Microsoft.PowerShell.Security
-- autoload, not policy -- and the user then saw only their own .ps1 being
refused, with no trace that the relief had been attempted or why.
-ErrorAction Stop is what routes a non-terminating failure into the catch at
all; the catch reports the FullyQualifiedErrorId to stderr and deliberately
does not rethrow, so a broken policy cmdlet cannot take down the startup this
gate exists to run. Success path is unchanged and stays stderr-clean.
Verified by execution on a clean child environment: success -> policy=Bypass,
stderr empty; shadowed failing cmdlet -> diagnostic on stderr and the gate
still continues; the old empty catch -> silent.
---------
Co-authored-by: Orca Worker <orca-worker@localhost>
|
||
|
Neil
|
5cc432eead | perf(ssh): reuse streamed response idle timers (#18956) | ||
|
Neil
|
56626e7daa |
perf(ssh): reuse and release relay startup buffers (#18953)
* perf(ssh): reuse the searched relay startup prefix * perf(ssh): release startup banners after relay readiness |
||
|
Jinwoo Hong
|
54a8afc91d |
fix(orchestration): typed error codes for dispatch and worker-start refusals (#18902)
* fix(orchestration): typed error codes for dispatch and worker-start refusals orchestration dispatch (and worker-start, which composes it) surfaced task not found, task not ready, and inject rejected as the same bare runtime_error, so an agent reading the receipt could not choose between creating the task, waiting on dependencies, or picking another terminal. Add task_not_found (data.taskId), task_not_ready (data.status, data.unmetDependencies), and inject_rejected (data.terminal, data.reason), each carrying data.nextSteps so every shipped CLI already prints the recovery. worker-start's not-ready refusal moves from task_not_startable to task_not_ready with the same detail. runtime_error stays for genuinely unexpected failures. Proven red-first from RpcDispatcher through the CLI's own failure formatting, plus an SSH bridge test that the host CLI's typed refusal relays unchanged. * test(orchestration): load CLI formatter at runtime in the dispatch-code test The composite node typecheck (config/tsconfig.node.json without --composite false, as CI runs it) rejects a static import of src/cli from a main test with TS6307. Load the formatter and error class dynamically behind narrow structural types, as the CLI/runtime boundary test does. * fix(orchestration): keep task_not_startable and split the CLI-format proof Review on #18902: - Drop task_not_ready. worker-start already published task_not_startable for a not-ready Task, so renaming it would change an existing receipt value under old clients. dispatch now emits task_not_startable too (it was a bare runtime_error before, so this is purely additive), with the new data.status / data.unmetDependencies / data.nextSteps. - Move the refusal receipts (code, message, data) into src/shared/orchestration-dispatch-refusal-contract.ts so the runtime emits them and the CLI test formats the identical envelope. The RPC test under src/main asserts toEqual against the contract; the new src/cli/orchestration-dispatch-refusal-format.test.ts feeds those same receipts to formatCliError / reportCliError. Neither tsconfig widens and the composite typecheck CI runs is clean. * fix(orchestration): keep published refusal messages and type the DB claim guards Codex review of #18902: - Every call site keeps the exact message it published on main ("Task not found: <id>", "only a ready Task can start.", "cannot retry from Dispatch"); the shared contract now takes the message per site and only owns the code and data. Baseline strings are pinned as literals. - createDispatchContext's own missing/non-ready guards, including the atomic-claim loser, now emit the same typed receipt instead of a bare Error, so a dispatch that races a status change no longer flattens to runtime_error. Covered by a dispatcher-level race test. - Invalid --retry-of keeps task_not_startable but now carries status, unmetDependencies, retryOf, and a retry-specific next step. - Dependency recovery text distinguishes waiting on running deps from retrying/unblocking failed ones. - CLI test adds an unknown-code case so the old-client claim rests on an assertion, not a comment; SSH test asserts exact stdout. - Guide table narrowed to the covered preflight cases; occupancy stays runtime_error and is named as such. |
||
|
Neil
|
36a826ff48 |
fix(ssh): compile node-pty from the host's own Node headers instead of nodejs.org (STA-6674) (#18774)
* fix(ssh): compile node-pty from the host's own Node headers instead of nodejs.org
STA-6674: a Linux SSH host that cannot reach nodejs.org never came up. node-pty
ships no Linux prebuild, so npm hands it to node-gyp, and node-gyp's default is
to download node-v<ver>-headers.tar.gz before configuring. The host refused
that connection (ECONNREFUSED) and the relay deploy failed inside npm install,
which the UI showed only as "Disconnected".
Every official Node build and every version manager that unpacks one already
has those exact headers at <prefix>/include/node. Export node-gyp's nodedir to
that prefix, on every command that can compile node-pty (npm install, npm
rebuild, the cloexec patch's rebuild), when the shipped node_version.h matches
the running Node. Both npm_config_nodedir (node-gyp 10, Node 20) and
npm_package_config_node_gyp_nodedir (node-gyp >= 11.4) are set so every Node
the relay runs on reads it. A version mismatch leaves it unset, which is the
existing behaviour.
When a host is both header-less and offline, name that in the deploy error
instead of forty lines of gyp http output, with the two remedies.
Reproduced and verified with a Docker sshd whose nodejs.org resolves to
127.0.0.1, on node:24.12.0 (the user's version), node:20 and node:26:
ssh-relay-offline-node-headers.docker.test.ts.
* fix(ssh): fail loudly when node-gyp ignores the exported Node headers dir
The headers export relies on npm forwarding npm_config_nodedir /
npm_package_config_node_gyp_nodedir into lifecycle scripts. If a future npm
drops that, node-gyp would silently fall back to downloading, and an offline
host would fail with the same "install an official Node" diagnosis -- wrong,
since the host did ship headers.
The prefix now echoes ORCA-NODE-HEADERS:<dir|none> into the command's output
before the compile, and the download-failure diagnosis reads it back: an
exported dir plus a download attempt is reported as an Orca defect naming
the dir, not as a host problem. Nothing else changes when it works.
* fix(ssh): address review on the relay node-headers export
- Unset any inherited npm_config_nodedir / npm_package_config_node_gyp_nodedir
before the conditional export, so a stale header dir from the remote profile
cannot bypass the version check and build a wrong-ABI binding (CodeRabbit).
- Require `gyp ERR! configure error` and a real network errno in the
headers-download matcher; node-gyp's fetch client logs retried attempts it
recovers from, and a FetchError can be a non-2xx mirror answer (pullfrog).
- Say "no local headers matching its own version", since the probe also
rejects a version mismatch, not only absent headers (CodeRabbit).
- Log the same diagnosis from the non-fatal `npm rebuild` fallback (CodeRabbit).
- Docker test waits for the SSH banner on the mapped port before connecting
instead of trusting `docker run -d` (CodeRabbit).
* fix(ssh): read the node-headers marker from the host output, not the quoted command
execCommand rejects with `Command "<command>" failed (exit N): <output>`, and
<command> quotes the whole prefix, marker echo included. The first-match
parser hit that copy and returned `${ORCA_NODE_HEADERS_DIR:-none}"; ...` as a
"dir", so every real no-headers failure was misreported as an Orca defect
(measured by an independent Docker exercise of
|
||
|
Neil
|
766b5b153c |
fix(relay): release the ConPTY conin handle after teardown, not before it (#18601)
A Windows SSH relay leaked one Windows File handle per terminal, for the life of the relay process, across reconnects. node-pty's `kill()` flips `readable` on the conin and conout sockets and destroys neither; `_cleanUpProcess` destroys `_outSocket`, so only conin is stranded, and it wraps a real named-pipe handle from `fs.openSync(term.conin, 'w')`. The obvious fix -- and the one config/patches/node-pty@1.1.0.patch ships for the desktop -- releases it at the top of the branch, before `_getConsoleProcessList()` forks and before the native kill. Measured against a real Windows SSH host, that is three times worse than leaving the leak alone: teardown aborts partway, the forked console-list agent is never reaped, and both pipe handles stay alive. Releasing it at the end of the branch instead is flat. 20 spawn/kill cycles, handles bucketed by NT object type, identical numbers standalone and through a real relay: published node-pty File +1/terminal, Process flat desktop patch placement File +2/terminal, Process +1/terminal released last (this) File flat, Process flat `windowsTerminal.js` takes the desktop's error-listener hunks verbatim. The conin listener is not what fixes the leak -- adding it alone changed nothing -- but it is what keeps a pipe error retiring one terminal instead of the host. The desktop patch has the early placement and therefore the regression, measured against its exact installed tree. Correcting it there needs its own verification on a Windows desktop build, so the trees diverge on this one hunk deliberately and a test pins that so a future patch sync cannot copy the bug back. |
||
|
Neil
|
2ee507d744 |
fix(ssh): move Windows file writes off PowerShell 5.1 stdin onto sftp (#18596)
* fix(ssh): move Windows file writes off PowerShell 5.1 stdin onto sftp #16432 was fixed by chunking writes to 32KB, on the belief that a `DefaultShell=cmd.exe` host caps one stdin at roughly 50KB. Re-measured on Windows 11 26200.9168 / OpenSSH_for_Windows_10.0p2, that premise is wrong in both directions, and the chunking does not fix the hang. The real constraint: a read on Windows PowerShell 5.1's redirected-stdin handle over a non-pty ssh exec can die permanently when it finds the stream momentarily empty, taking both the remaining data and the EOF with it. It is probabilistic per such read — not a size threshold, and not certain on the first one. Measured by swapping the copy loop for a counting reader: a 1.5s gap before any byte -> 0 bytes received, 6 of 6 1 byte, 1.5s gap, then 32767 -> exactly 1 byte 32768, 1.5s gap, then 32768 -> exactly 32768 a continuous 2MB -> 167936 / 270336 / 372736 Those three 2MB figures are one payload run three times under the same conditions, which is what rules out a threshold. Independently reproduced by a second harness where one 1.9MB counted read completed through 39 reads and another died after 11. A payload that fits one burst usually presents only one read that can find the stream empty, which is why 32KB mostly works — and it still failed 15 times in 120 under load, and 1 in 40 on a quiet host. Neither rate survives the 62 execs a 1.9MB file needs: even 2.5% compounds to about four uploads in five failing. No chunk size helps, because the defect is per blocking read, not per byte. Three controls on the same host, same DefaultShell, rule out both a size limit and cmd.exe: `findstr` took 2,016,000 bytes through one exec's stdin, sftp moved 1.9MB 5/5, and PowerShell 7 took 2MB in one exec. Windows writes now go over the sftp subsystem, whose batch script is read by the *local* client, so no remote process reads a pipe at all. PowerShell 7 is the fallback where sftp is unavailable, and Windows PowerShell 5.1 is last, still bounded, and now reports the host limitation and its remedy instead of a bare timeout. Measured on the same host, through this code: 1.9MB x20 all succeeded, hash-verified, median 315ms, against 0/6 before. 32KB x120 zero hangs, against 15/120. Also: - Stage under a unique name per attempt. An abandoned write leaves a remote process that may still hold the staging file, and losing contact is not evidence it died (docs/reference/ssh-execution-boundary.md), so a retry must not reuse a name its predecessor may own. Sweep is best-effort and never treated as proof of anything. - Create upload directories over sftp too; the JSON mkdir batch rode the same defective read. - Cover makeWindowsWriteFileCommand and the publish command against the 8000-char budget, which F11 flagged as untested. * fix(ssh): replace the staged Windows write atomically, and translate ssh -l Three review findings, all on the failure path that the success-path measurements say nothing about. CodeRabbit, Critical: the publish deleted the destination before moving the staged file onto it, so a failed move destroyed the user's existing file and left a window where a reader saw no file at all. That is worse than the truncated partial the staging discipline exists to prevent. Now File.Replace (Win32 ReplaceFile, atomic), falling back to a plain Move only when the destination is absent — and that race is safe, because a destination appearing in between makes Move throw with the staged file preserved. The exclusive branch already had it right: Move throwing on an existing destination is the exclusive contract. Append stays non-atomic and now says why. buildSshArgs can emit '-l <username>' for a config alias no Host block claims, and the translator threw on it. isSftpUnavailableError read that throw as 'this host cannot do sftp', so those hosts fell back to the defective PowerShell 5.1 path and had the refusal cached against them for 30 minutes, silently. '-l' now maps to '-o User=', with a test for the exact argument shape buildSshArgs produces in that case. CodeRabbit, minor: two assertions passed on an absent observation — an unmatched regex yields '' and every() is true of an empty list. Both now assert the positive form first, and the same audit was applied to the three other some()/every() assertions in the file. The temp-file test now asserts mode 0600 rather than only that the file is cleaned up. * fix(ssh): keep a path sftp cannot spell from becoming a verdict about the host Audit of isSftpUnavailableError, prompted by the '-l' gap having the same shape: a per-operation condition being written into a per-host cache that holds for 30 minutes. It had a second instance, and this one was mine. UnsupportedSftpPathError was classified as 'this host cannot do sftp', but it is thrown for a UNC or relative destination and for any path sftp's batch lexer cannot quote -- including a *local* filename containing a newline, which POSIX clients allow. One such file would have routed every later Windows write to that host down the defective PowerShell 5.1 path for the rest of the cache window. The host verdict is now only the errors that really are host-scoped: a refused subsystem, a client that will not start, and an untranslatable argument list. A path refusal falls back for that one write and leaves the cache alone, in both the file-write and directory-creation paths. Revert-tested. Removing the operation-scoped catch fails all three new tests, whether or not the predicate is also widened. Widening the predicate alone does not fail them, correctly: with the catch in place the predicate no longer gates that path, so keeping it narrow is defence-in-depth rather than the live mechanism. Flag audit at the same time: -F, -o, -T, -S, -p, -i, -J, -l and -- are now the complete set buildSshArgs can emit, and all are handled. * fix(ssh): make the atomic publish actually run, and unroll the mkdir batch Two runtime defects that only a real host could surface. Both were invisible to unit tests that assert the shape of the generated command string, because both are PowerShell rejecting an argument at execution time. File.Replace was passed a bare $null for destinationBackupFileName. PowerShell coerces $null to an empty string when binding a .NET string parameter, and Replace rejects that with 'The path is not of a legal form' -- so every create-mode publish failed. The Critical fix was inert as shipped. Now [NullString]::Value, which is the construct that exists for this. Measured on awin, same staging-file lock, opposite outcomes: old publish rc=1 destination MISSING <- prior contents destroyed new publish rc=1 destination PRESENT, sha 7f06b7e0... unchanged control, destination present, no lock rc=0 replaced exactly control, destination absent, no lock rc=0 Move fallback created it End-to-end through the real uploader afterwards: 1.9MB x15 all hashes exact, median 303ms; overwrite of an existing destination exact both times. Separately, the PowerShell mkdir fallback could not create a tree of more than one directory. '@($json | ConvertFrom-Json)' wraps the parsed array in another array, so the loop variable binds to the whole thing and [string] of it is the paths joined by spaces. It only ever worked for a one-element batch, where stringifying a single-element array happens to yield the element -- which is why no existing test caught it. Pre-existing on main; fixed here because this PR puts that command on the fallback tier and claims the ladder works. Both tiers now verified live against a three-directory tree. |
||
|
Neil
|
561a94038c |
fix(ssh): stop the daemon's own services from blocking the superseded-relay reap (#18586)
`isReapableRelayHusk` required `childCount === 0`, where `childCount` came from `pgrep -P <relay> | grep -c .`. But the relay forks service children of its own, and `relay-ai-vault-service.js` never exits once spawned. Any relay that had served a single AI Vault request therefore reported a non-zero child count forever, so the sweep answered `retained-live-work` for a superseded, disconnected relay holding no user work at all — and its version directory stayed pinned against GC by its own live socket. The probe now censuses each direct child instead of counting them, and the reap gate reads the count of children it could *not* positively identify as relay infrastructure. The asymmetry is the safety argument (docs/reference/ssh-execution-boundary.md): subtracting a child we can name is positive knowledge, assuming about one we cannot is not. An unrecognised argv, an argv `ps` would not print, and a host without `pgrep` all keep the relay unreapable. `reapEmptyRelayHuskCommand` re-runs the same census on the host immediately before signalling. Fixes #13614 |
||
|
Neil
|
e42c60e8a3 |
fix(ssh): resolve a pane's binding from the target partition, not the stale local copy (#18546)
One SSH pane accumulated one extra reattachable lease per relay restart (2, 3, 4, 5, 6 across five), and every one of them costs a `pty.attach` round trip on every later connect, forever. Nothing prunes `sshRemotePtyLeases`, so the fan-out only grows. `supersedeSiblingLeasesForPane` is fenced on the PTY the pane is durably bound to, and `durablyBoundPtyIdForPane` read `state.workspaceSession` (local) before `workspaceSessionsByHostId['ssh:<target>']`. But `persistPtyBinding(binding, hostId)` updates ONLY the host partition: AFTER-PERSIST local= ssh:t@@pty2:old:1 host= ssh:t@@pty2:new:1 So for the length of a reconnect the local copy still names the predecessor, the fence resolved to it, supersession took an already-`expired` lease as its winner, and returned having marked nothing. Both partitions agree again once the renderer republishes its layout, which is why the settled store looks consistent and hid this. Read both partitions as an ordered list, target's own first, and test the fence by membership rather than by equality with whichever was read first. Pick the winner preferring a lease this client still has a route to, since the stale partition names an expired one. Never retire a lease that is both bound and live, so a partition disagreement can't strand a running remote process. Superseded predecessors stay `expired` and are never `terminated`: losing a lease is not evidence the shell died (docs/reference/ssh-execution-boundary.md). A pane with no binding is skipped rather than pruned, so a genuine orphan stays askable. Also re-runs supersession from the binding side after each spawn commit's binding write, so the lease/binding order at a call site no longer decides, and reconciles every pane for a target immediately before `reattachKnownPtys` reads the set it feeds to `pty.attach` — that repairs stores which already accumulated these rows. The guard suite could not catch this: every assertion bound the pane BEFORE upserting the lease, an order no caller uses. Rewritten to the spawn commits' real order (lease, then binding, then the binding-side trigger); it fails 8 assertions without this change. Added a suite that drives the real `persistPtyIpcSpawnCommit` rather than the store primitives, including the exact stale-partition state written by production's own binding writer. Verified on the Docker SSH lane: five `relay.js` SIGKILLs with recovery between each, reattachable leases flat at one per pane. Note: this bounds the reattach SET, not the store. `sshRemotePtyLeases` still has no cap or TTL and rows still accumulate; pruning is left alone deliberately, since an `expired` row without `supersededBy` is a genuine orphan and must not be dropped on age. |
||
|
Neil
|
04ae62202a |
fix(ssh): close the macOS relay's per-terminal pty fd leak (#18534)
The relay asset from #17920 only rewrote the forkpty `default:` call site, which sits in the `#else` arm of PtyFork's `#if defined(__APPLE__)`. macOS takes `pty_posix_spawn`, so the asset had never patched anything a Mac executes -- and `applyNodePtyMasterCloexecPatch` returned 'fixed' for any non-Linux host without running the script at all, which is what publishes a tree to the shared native-deps cache. Stock `pty_posix_spawn` opens up to three throwaway ptys to push the real master off fds 0-2 and never closes them: the cleanup loop is `for (; count > 0; count--)`, but the first `posix_openpt()` in a running process already returns >= 2, so it breaks with `count == 0` and the body never runs -- and where it does run it closes `low_fds[count]`, never `low_fds[0]`. One orphaned /dev/ptmx fd per terminal, for the life of the relay. Ports the `low_fds` fix and the Apple-branch `pty_cloexec(master)` call from the app's `config/patches/node-pty@1.1.0.patch`, byte-identical, and runs the gate on darwin. macOS needs a different build layout than Linux: it has no `build/` at all, so the fallback moved aside is `prebuilds/darwin-<arch>` -- which is also what makes node-pty's install script fall through from "prebuild found" to node-gyp -- and the compile writes a `build/Release` the loader checks first. Verification is per-platform too: Linux's leak is inheritance (/proc), macOS's is self-held (lsof). Also corrects the asset's claim that "macOS re-opens the tty through uv_tty_init's cloexec dup". Measured false: FD_CLOEXEC is not set on the master. What protects it is POSIX_SPAWN_CLOEXEC_DEFAULT, one option away from gone since uid/gid drops libuv back to fork()/exec() -- so the master is now marked there too. Measured on darwin-arm64, one PTY per open/close cycle in a relay-shaped dir running the relay's own commands: before cycle:ptmx 1:1 2:2 3:3 ... 10:10 (10 after a settle) after cycle:ptmx 1:0 2:0 3:0 ... 10:0 (0 after a settle) Linux re-verified in docker node:22: inherited before, isolated after, `already-patched` on the second run. Refs #17915 Refs #8362 |
||
|
Neil
|
f35015d0c8 |
fix(ssh): measure pane idleness in the unit the sweep's kill operates on (#18415)
The orphan-relay-PTY sweep authorizes `pty.shutdown { immediate: true }`, which
runs `forceKillPosixPtyProcessGroups`: collect every process group on the pane's
tty, then `killpg` each one. The blast radius is therefore (groups on the tty) x
(members of those groups, wherever they are). The idleness evidence measured only
the first factor, so three shapes read as idle and were SIGKILLed:
- with job control off (`set +m`) a background job keeps the SHELL's pgid, so the
tty carries exactly one process group and that group is running the user's build;
- a child that drops the controlling terminal (`ioctl(TIOCNOTTY)` without `setsid`)
keeps the pgid, reports `tpgid == -1`, and never appears in `ps -t <tty>`;
- a double-forked grandchild keeps the pgid and tty but reparents to pid 1, so the
`ppid` walk cannot reach it and the named-process backstop never fires.
`shellOwnsEveryTtyProcessGroup` now also requires the shell's own process group to
hold no other member anywhere in the table, indexed in the same single pass. A
pids-per-tty set would catch the first and third but not the second, which is why
the count is pgid-wide rather than tty-scoped. The wire field keeps its tty-shaped
name: the value only ever became stricter, so an old client skips more, never less.
Second, unrelated-in-mechanism but same file family: `foregroundSkipReason` summed
`capturedAgeMs + evidenceAgeSinceListingMs` without validating either. A non-numeric
`capturedAgeMs` makes the sum `NaN`, and `NaN > 5000` is false, so a malformed record
PASSED the freshness gate and proceeded toward the stop — the one place in the file
that defaulted toward kill. Nothing validated it on this path
(`mapSshPtyProcessList` checks the ownership fields and spreads the rest through;
`PtyProcessListAdmission` is not on the sweep path). It now runs
`isForegroundProcessEvidence` and fails closed.
Verified on real Linux, not only in mocks: a container drives `bash -i` on a real
pty, builds each construction, runs the real publisher and planner, and then calls
the real `forceKillPosixPtyProcessGroups`. Before, all three published
`shellOwnsEveryTtyProcessGroup: true`, planned SWEEP, and the planted pid was gone
after the signal. After, all three skip and survive, and an idle shell is still
reclaimed.
Residuals are written down at the predicate and in ssh-execution-boundary.md: the
capture is a snapshot (bounded by the evidence-age budget, not removed), and a
process the host's own `ps` cannot enumerate stays unobservable while `killpg`
still reaches it.
|
||
|
Neil
|
b8da193b7a | fix(ssh): route the remaining expired-lease readers through the reattach predicate (#18378) | ||
|
Neil
|
720c3299ba |
fix(ssh): require a host death certificate before recreating a pane, and unstick expired leases (#18013)
* fix(ssh): match an expired lease on where its leaf lives now, not its frozen tab A lease freezes tabId at write time, but detachTerminalPaneToTab moves a live pane, so the stored tab is the one the pane LEFT. getRecentExpiredSshLease required lease.tabId === tabId, which is wrong in both directions: a viewer on a stale mirror matched under the abandoned coordinates (and resolvePersistedStable PaneOwner then reads an empty layout for that tab, so adoptStablePane is skipped entirely and a fresh shell is spawned over a possibly-live one, binding the same leaf in two tabs), while a viewer using the pane's real coordinates matched nothing and got terminal_not_recoverable. Resolve the leaf's current tab the way restoreReattachedPtyRuntime already does and compare against that, falling back to the frozen tabId only when nothing can say where the leaf lives. Both workspace partitions are read because SSH spawns bind into ssh:<target> while reattach binds into local. * fix(ssh): let a proven reattach take an expired lease back to attached #17965 authorized reattach from `expired` but the state machine refused the transition back, so a lease that reattached and proved itself alive stayed `expired` forever. That silently exempted a demonstrably running remote shell from `ssh:reset` (skips `expired`), from the SSH_TERMINATE_RECONNECT_REQUIRED ownership fence in `ssh:terminateSessions` (marks it not-owned), and from the quit-time `detached` sweep, and made it permanently ineligible to win supersession so its own successors never retired their predecessors. Only the id-qualified caller carries per-pty proof: markSshRemotePtyLeases AttachedAsync is fed the relay's `attachedLeaseIds`, so an unqualified bulk mark over a whole target still cannot revive `expired`. `terminated` stays absorbing. Re-entering `attached` drops supersededBy/relayIdRecycled, since route retirement belongs to the shell that lost the pane and this one just proved it is not that shell — the same invariant upsertSshRemotePtyLease enforces. * fix(ssh): make the pane-recovery liveness gate refuse without positive evidence of life The gate refused only `live` and `unverifiable` and passed on `null` — but the register is an in-memory Map, so `null` is equally what a fresh app start, a never-asked host and a certified death look like. Absence of evidence was reading as authorization to spawn a shell over a possibly-live remote process: `!pty.connected` is cleared for every PTY a dropped relay owned, and `expired` only ever says the CLIENT lost its route. - `exited` is now RETAINED rather than deleted, so the register is three-valued in the map as well as in the type. Its one writer is a host-delivered exit frame — an exit with a real code, or an explicit `hostExitConfirmed` — which records the certificate instead of merely dropping the doubt. - `recoverTerminalPane` refuses on `live` and `unverifiable`, and deliberately does NOT demand a positive `exited`. The only answer that ever reaches this gate is a reachable relay reporting no such id, and that is a union: pty.attach throws not-found for an unknown id with no liveness check, and a relay restart makes every previously minted id unknown (ids carry a per-start `ptyIdMintEpoch`). No writer of `exited` co-occurs with a reattachable `expired` lease either — a host-delivered exit frame tombstones the lease `terminated` — so requiring one would close the gate permanently. - `handlePtyReattachFailure`'s not-found branch publishes `code: -1` to the renderer and does not call `runtime.onPtyExit`. The relay's not-found answer is not a death certificate, and #17963's ratchet on the same branch pins that. - The inventory's `observed === false` hunk keeps dropping doubt rather than asserting a death: `pty.listProcesses` returns the relay's CURRENT session map, so a restarted relay omits every previously minted id whether or not those shells died — the same union, one hop away. A live or unprovable pane refuses; a disowned one still recovers. No wire change. The gate's ratchets live in terminal-pane-recovery-liveness-gate.test.ts: config/vitest.config.ts — the config CI runs — matches only `*.test.ts`, so cases placed under orca-runtime-tests/*.spec.ts would never execute. * fix(ssh): gate paired-viewer pane recovery on the narrowed session-gone predicate isSshSessionGoneError landed on the IPC transport, which never calls terminal.recoverPane. The one caller that does — recoverExpiredHostPane in the paired-viewer transport — still triggered on a bare SSH_SESSION_EXPIRED substring, so the identity-mismatch reply (the relay found a LIVE PTY under that id owned by another pane, which is evidence of presence) still asked the HUB to replace the pane, putting a second agent on one transcript. Main already refuses the respawn on that same reply; this makes the two agree. A pane whose shell genuinely died is unaffected: plain SSH_SESSION_EXPIRED still matches. The mismatch reply now surfaces as an error instead of a respawn. * test(persistence): update the reattach ratchet for expired-lease reclaim markSshRemotePtyLeasesAttachedAsync is id-qualified, so a named pty that proved itself alive now returns to attached instead of staying expired. |
||
|
Neil
|
f2e95e7860 |
fix(ssh): separate a superseded lease from an orphan so reattach can tell them apart (#17966)
`expired` was one word doing two unrelated jobs — "a newer lease won this pane" and "reattach lost contact" — so `reattachKnownPtys` had to exclude all of them. That kept the 2 -> 19 -> 20 fan-out fixed at the cost of never bulk-reattaching a genuine orphan; those recovered only through the slower `adoptStablePane` path. The blocker cited in #17965 does not apply. The STA-3077 note guards `upsertSshRemotePtyLease`'s match against a RECYCLED `pty-N` after a relay restart. `supersedeSiblingLeasesForPane` is a different path and already holds `winner.ptyId` when it expires a predecessor, so recording which lease won needs no relay-start identity. `SshRemotePtyLease` gains two optional marks, each meaning exactly one thing: - `supersededBy` — the winner's stored-form ptyId, written only by supersession. - `relayIdRecycled` — written only by the pending-stop replay's `relay-id-recycled` retirement. That retirement wrote `expired` *purely* to keep the lease out of the reattach that runs one step later ("hands the user's old pane to whatever process now holds the recycled id"), and the reattach fences on paneKey/tabId, never on incarnation. Relaxing the filter without this would have silently reopened that hole. Bulk reattach now skips a lease carrying either mark and re-adopts the rest, via one shared `sshRemotePtyLeaseAllowsReattach`. `terminated` is untouched. Recycled-id safety: both marks are dropped whenever the id is re-upserted `attached`/`detached`, so a relay that renumbered onto a new shell cannot inherit its predecessor's mark. Supersession also stamps an ALREADY-expired predecessor for the same pane — same evidence, and it is what bounds the reattach set, since otherwise every past orphan for that pane would stay reattachable forever. Its `updatedAt` deliberately stays put: bumping it would make a stale lease look recent to `getRecentExpiredSshLease`. Persistence: the lease loader is a strict whitelist, so both fields are named in `normalizeSshRemotePtyLease` or they would be stripped on every launch. Absence reads as "orphan", which is the only thing an older build could have meant, and an older build ignores keys it has never heard of (remote-wire Rule 1). |
||
|
Neil
|
57681ecd09 |
fix(remote): resolve the spawn cwd, the node manager dir, the vault host and the scrollback seed (#17952)
* fix(remote): resolve workspace cwd, mise Node, host scope, and TUI scrollback honestly #15296 relay: a folder workspace id (`folder:<uuid>`) carries no path, so the worktree-id split yielded nothing and $HOME silently won. Resolve the spawn cwd through worktreeId -> ORCA_WORKSPACE_ROOT -> host default, and refuse an agent spawn outright when a folder workspace names a root this host cannot resolve. #11733 ssh: generalize the NVM dotfile scrape into `orca_dotfile_dirs` and drive mise off `MISE_DATA_DIR` / `XDG_DATA_HOME` instead of a hardcoded `$HOME/.local/share/mise`. #13713 ai-vault: an unresolvable workspace host is `unverifiable`, not local. Widen the default scope to every host rather than scanning the client's own history and reporting "No agent sessions found". #6106 terminal: hydration asked the renderer for `scrollback: 0` while an alt-screen TUI was up, which drops the normal buffer's shell history rather than the TUI bytes. Drop the flag; readers already split the two buffers apart. * fix(remote): stop the relay answering host questions for a guest execution host Three findings from review of the spawn-cwd resolver, all the same shape: a path question answered against the wrong host, or with the wrong key. - resolveRelaySpawnCwd refused an agent launch whenever a folder workspace named a root that did not stat on the relay. But relayHostDirectoryExists stats the relay's *own* filesystem, and the relay supports WSL shells, so a folder workspace on a Windows relay launching into WSL now threw where it previously spawned -- contradicting the function's own doc comment, which says an absent path for that exact host pair is a miss, not a refusal. Thread the shell's execution host in and demote the refusal to a miss when the spawn does not run on the relay's filesystem. - requireRelaySpawnCwd's doc claims both call sites route through one resolver so the fence can never be keyed on a directory the spawn won't use, but the fence key was still computed with the non-stripping splitWorktreeId while the cwd used splitWorktreeIdForFilesystem. For a `::workspace:<uuid>` id those disagree by construction, in adjacent lines: the removal fence guarded a path no spawn ever enters. Same defect in shutdownForWorktreePath and the revive path; all three now use the filesystem split. - The remote Node probe expanded `$HOME` and `~/` prefixes out of a dotfile assignment but not `$XDG_DATA_HOME`, so `MISE_DATA_DIR=$XDG_DATA_HOME/...` was used as a literal directory name. Add the case arm, defaulting to the POSIX `$HOME/.local/share` the seed value already uses -- sshd's exec channel usually has no XDG_DATA_HOME at all. |
||
|
Neil
|
64dac75d9b |
fix(ssh): stop respawning panes on client-side-only absence evidence (#17957)
* fix(ssh): stop respawning panes on client-side-only absence evidence Three respawn gates acted on evidence weaker than host-attested exit. Per docs/reference/ssh-execution-boundary.md, loss of contact, a failed reattach, an identity mismatch and absence from a client map are all `unverifiable`, never `exited`. Gate 1 (ipc-pty-connect.ts): "belongs to SSH connection" is minted by the id router from a pure client-side string compare, before any relay is asked, and still returned `sessionExpired: true` -> fresh PTY + agent resume. After an SSH target re-adoption the "other" connection is the same machine, so that puts a second `claude --resume` on the transcript the surviving PTY still owns. Now returns undefined with no error, which routes the pane to recoverUnverifiableDirectSshReattach (remount + reattach, no shell restart) and keeps #7661's no-red-toast outcome. Gate 3 (ssh-reconnect-pane-retry.ts): `!tabPtyId` read `tab.ptyId`, which is only the single-pane fallback for legacy attach. It diverges from the real records deterministically: workspace-terminal-reconnect fills ptyIdsByTabId from the leaf map but writes tab.ptyId only when a tab-level id survives, and clearTransientTerminalState nulls tab.ptyId on every hydrated row. Both leave live leaf PTYs with a null fallback field, arming a generation bump onto the fresh-spawn path. Now consults ptyIdsByTabId and the layout leaf map too; a tab with no PTY in any record still retries. Gate 2 (recoverTerminalPane): an `expired` lease plus `!pty.connected` authorized createTerminal. Every writer of `expired` records that the CLIENT lost its route, not that the shell died. Now also requires the runtime's own liveness verdict to be neither `live` nor `unverifiable`, and ssh-relay-session records markPtyLivenessLive at the persistPtyBinding refusal, which is reached only after pty.attach succeeded. See the report for why this branch is currently unreachable for SSH panes. * fix(ssh): let the respawn gate see the relay's own absence answer Gate 3 refused to respawn a pane whose records still named a PTY, which is right for a transport drop and wrong for a killed relay: after the relay is SIGKILLed and comes back, the leaf map still holds `pty2:<dead-epoch>:1` while the new relay answers that it has no such id. #18017's "replaces the pane only when the host proves the session is gone" regressed on exactly that. The gap was not the predicate, it was its inputs. `handlePtyReattachFailure` already distinguishes the three reattach outcomes and only its not-found branch publishes anything — a lost link and an identity mismatch send nothing. But it published `pty:exit { code: -1 }`, and `-1` is the sentinel every reader resolves to `stop_unverified`, so the one branch holding positive host evidence of absence arrived looking exactly like loss of contact. The renderer had no host answer at all, which the gate's own comment conceded. The exit now carries `livenessVerdict: 'exited'` beside the unchanged `-1`, so the code keeps meaning "no provable status" for every existing reader while the verdict rides its own field. A store bridge records those ids in `hostAttestedAbsentPtyIds` regardless of whether a pane is mounted to hear it — during reconnect none is — and the gate stops counting a recorded id the host has disowned. Settled when a PTY answers to that id again, because a redeployed relay renumbers from pty-1. This narrows #17963, which pinned the same exit as unverified on the grounds that not-found cannot separate "verified the pid is dead" from "my session map never had this id". Everything #17963 protects is untouched: `-1` still fails isProvenProcessExit, so the tab is not closed, the pane's leaf binding is not dropped on exit, and markUnverifiedPtyLoss still fires. Only the reconnect respawn gate reads the new field, and only for an id whose sole channel — the relay that answered — has disowned it, which no client can reach again under any verdict. That is the reading ssh-pty-relay-absence-verdict.test.ts already pins for the spawn path; the reconnect path now agrees with it. Rejected: parsing the relay's mint epoch out of `pty2:<epoch>:<n>`. It needs the current epoch on the wire (a capability-negotiated relay change), it has no answer for legacy `pty-N` ids, and a relay that comes back with zero PTYs gives the client no epoch to compare against. Rejected: clearing the leaf record outright, because the remote workspace snapshot re-hydrates those ids after the clear and the gate would refuse again. * refactor(ssh): name the relay-disowned signal for disownership, not exit |
||
|
Neil
|
510305e574 |
fix(relay): signal capacity loss instead of dropping, hanging, or truncating (#17870)
Three failures with one shape: a payload past a fixed capacity was met with silence, with a wait that never ends, or with a prefix presented as a whole. **The workspace snapshot was silently dropped.** `workspace.changed` carries the tab/session list, and a snapshot past the producer frame capacity (12288 B on a Node <=21 remote) was dropped with only a relay stderr line, so the client kept a stale list forever. The relay now publishes per client and, for a client whose sink refused the frame, sends a compact `workspace.stale` marker on the control lane; the client re-reads through `workspace.get`, whose lane is budgeted in megabytes rather than in one producer frame. A new JSON-RPC notification rather than a new field on `workspace.changed`: `normalizeSnapshot(undefined, ns)` yields revision 0 and an empty session, so a Rule-1 field would make an old client replace its tab list with nothing — worse than the drop. An old client ignores the unknown method and is exactly where it is today. The marker retention/retry machinery is extracted from the `fs.changed` overflow path and shared by both. **The Windows upload hung, and the fix for it could truncate.** `#16432` was attributed to `[Console]::In.ReadToEnd()` materializing the base64 bundle. That is not what the reporter measured: he also measured `new IO.StreamReader([Console]::OpenStandardInput())` — an incremental reader — hanging at 1 MB. The limit is in the stdin the host hands PowerShell over a non-pty ssh exec, not in the string the script builds. - `uploadFileViaSystemSsh` — the user file-import path — was piping a whole file into one Windows stdin, unchunked and untimed. That is the path large files take; it now chunks into 32 KB writes and bounds each wait. - The Windows directory upload reuses that single-file path rather than repeating a weaker copy of chunk-read + write-buffer; the `ino`/`dev` TOCTOU verification comes with it. - A Windows write needing more than one exec lands on a `.orca-partial` staging path and is published by rename, so a failed chunk cannot leave a truncated artifact under the real name. `exclusive` is enforced once at the rename, not on the first chunk, where a retry met its own leftovers. - The mkdir batch reads stdin through the stream reader the reporter measured surviving 50 KB, not `[Console]::In`, which he measured wedging at that size. - `waitForChannelClose` takes an optional bound. A wedged PowerShell stays alive at idle CPU and never closes, so without one the promise is simply never settled and the caller waits forever with no error to show. **Quick Open showed a prefix as the whole workspace.** The mechanism "a full page means there is more" only works if the caller named the cap, and the failing UI named none — it hardcoded `truncated: false`. Quick Open now names `QUICK_OPEN_LISTING_MAX_RESULTS` on both the Electron IPC hop and the runtime-RPC hop (the field #17954 added to `files.listAll`), and reads a full page as truncation. The local hop honours the cap too, which it previously ignored. Rebase note on `fs.listFiles`: an earlier revision of this work also clamped the host unconditionally, and #17934 escalated an uncapped request to an explicit error. #17954 has since landed and made an oversized reply streamable, which removes the premise — the host no longer has to choose between a prefix and a refusal, so it returns the whole listing when no limit is named and only clamps a limit it was given. Keeping either would have regressed #17954 and hard-failed three in-tree callers that deliberately pass no options (`runtime-file-commands-search-runtime-files.ts:81`, `filesystem-read-handlers.ts:125`, `runtime-file-commands-constructor.ts:41`). |
||
|
Neil
|
278f9ee876 |
fix(ssh): answer every MFA stage, stop dialling an unclaimed alias, and say where a clone failed (#17946)
* fix(ssh): answer every MFA stage, not just the first ssh2 walks one flat auth-method list exactly once, so keyboard-interactive could only ever be offered a single time. A host running `AuthenticationMethods keyboard-interactive,keyboard-interactive` (or any ladder ending in a second challenge) partial-succeeds the first stage and then finds the list exhausted, which the user sees as "All configured authentication methods failed" — the reports in #8622 and #16820. Orca's own auth handler now runs for every target instead of only multi-key ones, and rebuilds its queue on each SSH_MSG_USERAUTH_FAILURE that carries partial success, narrowed to the methods the host still offers. Narrowing also stops keys being re-offered after the host has moved past publickey, which is what exhausts MaxAuthTries before the challenge is ever shown. Covered by a real ssh2 server fixture that stages partial success. * fix(git): say where a failing clone ran and why nothing could prompt Clones go through nonInteractiveGitEnv, so `ssh` runs with BatchMode=yes and an emptied SSH_ASKPASS. On a remote or paired-runtime clone that produces `fatal: Could not read from remote repository.` while the same `git clone` typed by hand on that box succeeds — the divergence in #14533. Nothing in the message said the clone ran on the other machine, under its keys, with the prompt deliberately disabled. getGitCloneFailureMessage now appends that fact, and names the two recognisable shapes: a publickey refusal (load the key into an agent there) and a host-key failure (record the key in that machine's known_hosts). Unrecognised SSH failures still get the where-it-ran note; non-SSH failures are untouched. One builder, so the SSH-target relay path and the runtime path both get it. * fix(ssh): stop dialling a bare alias no ssh_config block claims A wildcard `Host *` block supplies ProxyCommand/ProxyJump for every alias, so shouldUseSystemSshTransport picks the system transport for an alias whose own Host block was renamed or deleted, and buildSshArgs then dials that alias verbatim: no -l, no -p, no Hostname. Orca connects as the wildcard's user to the wildcard's host and discards the endpoint it stored (#11746). The signal #11746 assumed (hostBlockMatch, from the still-open #11707) does not exist, and `ssh -G` cannot supply it — it prints the merged config and answers for unknown aliases too. The config file is the only source of truth, so: - parseSshConfigAliasClaims retains raw Host patterns and flags Match blocks, which parseSshConfig discards because it mints importable targets. - sshConfigMayClaimAlias is sound in the negative direction only: an unreadable file, any Match block, or any non-catch-all pattern that might match all answer "claimed", so absence of evidence is never read as evidence of absence. Only a proven-unclaimed alias licenses an override. - buildSshArgs then states Hostname/Port/User, and only those: the wildcard is still the route, and -o Hostname does not change block selection, so the proxy keeps applying and %h expands to the host we mean. The verdict is injected rather than read inside buildSshArgs, so an arg builder does not answer differently per machine. Default is today's behaviour. Scoped to the system-SSH transport and the connection's own command/transport path. Port-forward processes and the ssh2 transport (#11707) are unchanged. * fix(ssh): read a negated Host group as uncertainty, and gate clone SSH guidance `Host * !prod` applies to every alias but `prod`, yet skipping both the catch-all and the `!` pattern answered "unclaimed" for `stage` — which licences overriding Hostname/Port/User against a block the user wrote. Any negation now makes the whole group uncertain; the function is only sound in the negative direction. Also require an ssh(1) diagnostic beside "could not read from remote repository" before appending the SSH clone note: git prints that same line for the HTTP remote helper, where advice about keys and agents is simply wrong. * fix(i18n): restore the activity-options key the rebase dropped * fix(i18n): union en.json with main so the rebase cannot drop keys |
||
|
Neil
|
7458c39181 |
fix(ssh): declare a wedged relay link lost, and stop reading silence as a verdict (#17817)
* fix(ssh): declare a wedged relay link lost instead of suppressing the dead-link check * fix(ssh): make the Windows deps probe exit 0 on a real load failure, like its POSIX twin * fix(relay): reap a client that has stopped answering instead of holding its leases forever * test(relay): feed the primary before asserting the reaper exemption holds * fix(ssh): keep a lost link's verdict unverifiable instead of reporting absence * refactor(ssh): read the exec timeout from its typed code, not the message text * fix(relay): bound a client that clears the handshake and then never frames anything * fix(i18n): restore the activity-options key the rebase dropped * fix(i18n): union en.json with main so the rebase cannot drop keys |
||
|
Neil
|
07e1f953a5 |
fix(ssh): log an unanswered native-deps probe instead of launching silently (#18000)
* fix(ssh): log an unanswered native-deps probe instead of launching silently The wrongful rebuild used to be the only visible symptom of a dropped exec channel; #17979 removed it, so a real transport failure now leaves no trace. Matches the install-path sibling, whose callers log the same class of failure. * fix(i18n): restore the activity-options key the rebase dropped * fix(i18n): union en.json with main so the rebase cannot drop keys |
||
|
Neil
|
7c6c8ef85e |
fix(ssh): stop a late SFTP stream error crashing main, and keep the relay socket inside sun_path (#17862)
* fix(ssh): stop SFTP stream errors crashing main and bound the relay socket path
inside the protocol parser. Every transfer removed its listener on settle, so a
STATUS reply that arrived late - the normal case behind a jump host that chroots
its SFTP subsystem - threw synchronously out of Socket.emit('data') and killed
the main process. Keep one durable listener per stream, and report a sandboxed
SFTP namespace with an actionable message instead of a bare 'file does not
exist'.
104 macOS) and bind failed with a bare 'listen EINVAL'. Fall back to a per-uid
base whose length does not depend on $HOME, keeping the hashed socket name
intact.
* fix(ssh): validate the short socket dir before mutating it
* fix(ssh): keep the SFTP session guarded, scope the relocated socket, narrow the chroot verdict
Three review findings.
The CLI-launcher install ran writeStringViaSftp in a loop over a bare conn.sftp().
That helper removes its own session 'error' listener at each settle, so between
files and after the last one the emitter carried none -- and ssh2 raises a late
STATUS reply synchronously out of Protocol.parse, which is the uncaught exception
that kills main (#15479). The inline loop it replaced leaked one listener per file
and covered this by accident. Extract writeStringsViaSftp, which owns the session
latch, and share that latch with runSftpFallbackTransfer.
SSH_FX_PERMISSION_DENIED is a mode/ownership refusal on a path the subsystem can
see, not evidence of a chroot; sftp-namespace-resolution already treats only
NO_SUCH_FILE as conclusive. Narrow the predicate to code 2 so a read-only home
stops being reported as a bastion misconfiguration.
The relocated socket had no version dimension. relaySocketNameForInstanceId hashes
the target, not the build, and under $HOME the enclosing relay-<fullVersion> dir
supplied the rest -- so the short form made the path stable across updates. The
next build would bind the path the previous relay still holds, the handshake would
mismatch, and a relay holding live work would raise RelayEndpointHeldError with no
way through. Add a hashed version segment under the short base, mirroring the
relay-*/<sock> shape so one pattern serves both, and teach the superseded sweep and
force-stop about that base. The relocated tree now also gets reclaimed: nothing
else walks it.
* fix(i18n): restore the activity-options key the rebase dropped
* fix(i18n): union en.json with main so the rebase cannot drop keys
|
||
|
Neil
|
31007c0d86 |
fix(ssh): reclaim relay PTYs the client has provably lost, on host attestation only (#17831)
* fix(ssh): reclaim relay PTYs the host attests this client orphaned (#9819) Orca could lose track of terminals running on an SSH relay until the 50-slot cap refused to open any more. This reclaims them, and the whole design is built around the fact that getting it wrong destroys a user's running process on their remote machine: the failure mode is leak, never kill. A stop requires all nine of: 1. the relay published an `ownerClientInstanceId` read from the live authenticated consumer grant of the connection that requested the spawn — never from a spawn parameter, since an echoed claim is no evidence; absent means skip 2. that id equals this client's persisted consumer identity 3. this connection holds the negotiated `session-owner` grant 4. `paneBound === true`, host-published 5. no `agentSessionOwners` — the host still advertises it as adoptable 6. `hostAgeMs >= 30s`, measured on the host's clock 7. this client has no route: not reattached, no lease outside terminated/expired, no pending kill, and no `expired` lease either — an expired lease is the record of a process deliberately left running, never a licence to kill it 8. every stop is fenced on the incarnation the same listing published, and on the owner identity, both re-checked by the host 9. a pass wanting to stop more than 8 refuses entirely Absence from a client-side set is `unverifiable` by construction (docs/reference/ssh-execution-boundary.md): a second machine attaches to the same relay and displaces the session owner, and its live agents are missing from this client's store for exactly the reason a genuine orphan is. So the host has to attest ownership, and the host has to attest that nothing is running. That second attestation is measured over the pane's whole tty, not its foreground process group. `tpgid == pgid` is foreground-only: on a real `bash -i` on a real pty, a shell holding `sleep 300 &` and a shell holding a Ctrl-Z'd job both read `pgid == tpgid`, `Ss+` — byte-identical to an idle prompt, with only the job's own row differing. A foreground-only gate therefore attests `pnpm build &` and a suspended editor as idle, and the stop that follows SIGKILLs every process group on the tty. `shellOwnsEveryTtyProcessGroup` is measured over that same set of groups, so the evidence and the kill describe the same thing. No new probe: `tpgid` already identifies the terminal, because a process group belongs to one session and a session to at most one controlling terminal. The freshness field is real rather than decorative. `capturedAgeMs` is stamped from when the capture was taken, deliberately as an upper bound since the process table is TTL-shared, and the sweep refuses an observation older than its own pass budget, counting its own elapsed time since the listing arrived. Stale evidence degrades to "do not sweep", never to "sweep". The display consumer of the same measurement keeps no age budget, as a stated decision: a stale pane title costs a redraw and self-corrects. `pty.shutdown` is authorized on the host that owns the process. `pty.spawn` and `pty.attach` both take a request context and check it; the one irreversible call took none, so the rule above lived entirely on the client that decided to make the call. It gains an optional `expectedOwnerClientInstanceId` and refuses unless the connection still authenticates as that identity AND this host recorded it at spawn. Finally, a reattach refusal now says whether it observed the process. Three refusals carry the same `SSH_SESSION_EXPIRED` text and only one is absence; `restoreRequired` means the PTY is live and only its source stream is not. Testing that text with `.includes()` expired the lease and deleted ownership for a running process, erasing this client's only record of it — and a PTY with no record is one the sweep may stop. Wire compatibility: four new optional fields and one new optional param on existing methods, no new method and no new stream opcode (Rule 1, and Rule 2 does not apply). Rule 1's caveat is discharged explicitly — no reader requires any of them, each absence is a named skip reason, and an ordinary pane teardown must omit the owner fence because a revived PTY carries no attested owner at all. New client plus old relay stops zero PTYs; old client plus new relay never reads the fields. Windows relay hosts publish no evidence and therefore never sweep. Verified by joining the real publisher to the real client reader over `ps` captured verbatim from a Linux container, and by driving a real group-for-group SIGKILL against a real pty: backgrounded and suspended jobs survive by pid, and an idle shell is still reclaimed, so the narrowed predicate is not a silent no-op. Squashed deliberately. The sweep is unsafe at every intermediate commit of its own history — before the foreground gate it reaps a hand-launched `claude`, and with a foreground-only gate it reaps a backgrounded build — so this ships as one commit with no bisectable state that kills live work. Refs #9819. Folds in #17939. * fix(i18n): restore the activity-options key the rebase dropped * fix(i18n): union en.json with main so the rebase cannot drop keys |
||
|
Neil
|
aa3ae6f56e |
fix(ssh): close the pty master fd leak on relay hosts too (#17920)
* fix(ssh): close the pty master fd leak on Linux relay hosts The app gets the FD_CLOEXEC patch through pnpm patchedDependencies (#17914); the relay installs stock node-pty from npm, where no pnpm patch reaches. Linux is where that matters -- it is the only relay platform that takes forkpty()'s no-atomic-O_CLOEXEC path, and it is also the only one that already compiles node-pty at install time, so the fix costs a second compile rather than a first. Ships the patch as a relay asset applied like the existing Windows console-list one, and rebuilds only after the probe has proven node-pty loadable. The rebuild is non-fatal by construction: the working build is moved aside first and moved back on any failure, a failed attempt drops a skip marker so the compile is attempted at most once per relay directory, and the caller swallows the whole step. macOS and Windows relays never run it. Measured on node:22 with a relay-style npm install: before, the master is cloexec=false and shows up as `26 -> /dev/pts/ptmx` in both a later pty child and a later child_process child; after, cloexec=true and neither child sees it. Closes #17915. * test(ssh): feed the cloexec patch exec to the hand-rolled namespace fixtures These sequences are positional, so the new Linux-only patch exec swallowed the READY slot and every install/repair case timed out waiting for the relay. * fix(ssh): patch the pty master before publishing the shared native-deps tree * fix(ssh): refuse to publish a native-deps tree whose cloexec patch did not take |
||
|
Neil
|
7eb13c184c |
fix(ssh): keep remote PowerShell commands inside what sshd's cmd.exe accepts (#17947)
* fix(ssh): keep remote Windows commands inside cmd.exe's command-line limit Windows OpenSSH runs every exec request through sshd's DefaultShell, which is cmd.exe on a stock install, and cmd.exe refuses a line over 8191 characters with exit 1 and a localized "The command line is too long". `-EncodedCommand` spends 2.67 characters per script character, so five commands on the first-connect path were already over: the stale upload-stage recovery that opens a fresh install (23,210), promote (20,646), cleanup (19,798), the install-lock steal (11,798) and reserve (9,434). A Windows-to-Windows `ssh:connect` died on the first of them before the relay was ever uploaded (#16126). powerShellCommand now falls back to a gzip self-extracting bootstrap once the inline form passes the budget - these scripts are repetitive enough that the worst one lands at 6.5KB - and throws a message naming the limit if even that cannot fit, rather than letting cmd.exe answer in the host's locale. Commands that already fit are byte-identical. The real-binary PowerShell suite in ssh-relay-upload-stage-commands.test.ts exercises the bootstrap end to end, including `exit` and here-string semantics through Invoke-Expression. * fix(ssh): cite the real command-line budget and reuse the cmd.exe ceiling |
||
|
Neil
|
7f4a17d8eb |
perf(ssh): key relay native deps on the deps, not the bundle hash (#18033)
Relay node_modules lived inside `~/.orca-remote/relay-<version>+<hash>`, so any byte change in `src/relay/` or the `src/shared/` it pulls in minted a new directory and a fresh `npm install node-pty@1.1.0 @parcel/watcher@2.5.6`. On Linux node-pty has no prebuild, so that is a node-gyp source compile on every new bundle — eight of the fifteen deploy minutes, daily, on a dependency set that is a pinned constant (#18009). The tree now lives at `~/.orca-remote/native/<platform>-<depsHash>/node_modules` and each relay directory symlinks to it. depsHash covers RELAY_NATIVE_DEPS, an explicit epoch, and the bytes of every shipped `node-pty-*` patch artifact, so a patch change mints a new entry rather than leaving hosts on a stale tree. Three rules make one tree safe to share: - A published entry is immutable. `.deps-complete` is written last, only after a probe on that host loaded both addons. Nothing installs, rebuilds or resets into a published entry: every `npm install` is prefixed with a symlink detach, so a repair on one directory can never `rm -rf node_modules/node-pty` out from under a live relay sharing the tree. - Publication elects one winner with `mkdir`, so no client-side lock is needed and two deploys never write one tree. A loser keeps its own copy. - Every failure degrades to today's per-directory install. GC follows remote-install-gc.ts' discipline: a listing that does not end in its own OK token, an unreadable link, or a reference whose shape this client never writes aborts the whole pass. Deletion is tombstone-rename, re-read references under the rename, then remove — a deploy that linked between the listing and the rename gets its tree moved back. It only runs for a connection that could compute a key, and never removes a pinned one. Migration: a first deploy after this ships seeds its tree from a sibling relay directory whose manifest pins the same versions, so an existing host does not recompile once more. The seed is not trusted — it is a plain private install until the normal probe loads it, and only then is it published. Windows keeps the per-directory install: node-pty ships win32 prebuilts, so there is no compile to avoid, and the console-list agent patch mutates the installed tree in place, which rule 1 forbids for a shared one. The POSIX scripts are exercised against a real tree under both /bin/sh and dash, not just asserted on as strings. |
||
|
Neil
|
f9587f74f5 |
fix(ssh): repair a rebuildable node-pty failure once, instead of asking the user to reconnect (#17907)
* fix(relay): diagnose why node-pty will not load instead of hedging The relay could only say "terminals are unavailable" and then list three remedies for four different faults, none of which the user could verify (#17830). Two things were destroying the evidence: - `loadPtyUncached` caught the load error into bare `catch {}` blocks (pty-handler.ts:539, :551) and returned null. The only cause anyone had was discarded on the spot. - node-pty's own loader walks three directories and rethrows only the LAST failure, so even an uncaught error arrives as `Cannot find module '../prebuilds/...'` — the GLIBC/ABI/arch sentence is already gone. The relay now keeps the load error, recovers the real dlopen message with an out-of-process load of the file node-pty would have opened, reads what node-gyp configured the binding for (`build/config.gypi`), captures the host's Node ABI, arch and glibc, and probes the toolchain only when nothing was compiled. Each fault gets its own message naming values the user can check: toolchain_missing, dependency_missing, abi_mismatch, arch_mismatch, libc_floor, shared_library_missing, load_crashed, and load_failed which quotes the loader verbatim. A probe that did not answer stays `unverifiable` and prescribes nothing. The classification is now also structured data on the error, so a client can repair the host instead of printing a paragraph: an additive, schema-validated `data` field on an existing JSON-RPC error, with `repairable` true only for a proved fault that recompiling on the host actually fixes. Reuses orcad's loader-message parsers and out-of-process probe rather than adding a second copy; `classifyLoaderMessage` moves to a shared module and gains architecture and missing-shared-library cases, which the orcad boot precondition picks up too. * fix(ssh): repair a rebuildable node-pty failure once, instead of asking the user to reconnect |
||
|
Neil
|
4f4872c424 |
fix(ssh): treat a markerless native-deps probe answer as unverifiable (#18011)
The repair path read "the probe answered, but nothing in the answer names a dep" as "both deps are missing". The POSIX probe is fenced with `|| echo MISSING`, so the subshell always exits 0 and the unanswered-probe catch added by #17979 never ran. A node that cannot start (invalid NODE_OPTIONS, OOM kill, exit 127) therefore produced a bare `MISSING`, and every reconnect rm -rf'd node_modules/node-pty and node_modules/@parcel/watcher and burned a 240s npm install that failed the same way. `.install-complete` from the original install survives, so the relay kept launching with no PTY and no file watcher, permanently, per host. Only a marker line that actually names deps is evidence about them; anything else is `unverifiable` and launches as-is. Also drop `2>/dev/null` from the POSIX probe and carry stderr into the warning via a new execCommand `onStderr` hook, so the reason node failed survives. stderr stays its own stream — folded into stdout it would match the probe's own token strings. The Windows branch shared the same parser and is fixed with it. |
||
|
Neil
|
b8cfdb1702 |
test(ssh): ratchet the relay reattach-failure exit as unverified, not proven (#17963)
* fix(ssh): stop expiring relay-reset leases when the force-stop threw A force-stop that rejected never observed the remote shells, so bulk-expiring their leases in the finally block recorded a verdict Orca does not hold. Mirror ssh:terminateSessions: only a fulfilled stop retires a lease. Local PTY handles are still cleared, so nothing is stranded — the next connect reattaches the survivors or expires them on host evidence. * test(ssh): ratchet the relay reattach-failure exit as unverified, not proven The relay answers pty.attach not-found both when it verified the pid is dead and when its session map simply lacks the id — which is every id after a relay restart. No behavior change: -1 already routes through isProvenProcessExit to the renderer's unverified-loss path. This pins that contract and drops the comment claiming the branch holds positive proof of death. |
||
|
Neil
|
d838ca1419 |
fix(ssh): stop orphaning live relays when an endpoint is taken over (#17821)
A failed `--connect` was read as "the relay crashed": the client `rm -f`'d the socket and launched a replacement at the same path. Unlinking a unix socket does not close the listener the incumbent already holds, so an alive-but-refusing relay — the RelayVersionMismatchError case — kept running forever with its PTYs and agents (#8585). Establish the incumbent with host evidence instead, in the fixed live/unverifiable/exited vocabulary, and never unlink from the client: the daemon's own RelaySocketOwnership already performs an identity-checked takeover that is atomic with its bind. A live incumbent now raises a typed terminal RelayEndpointHeldError naming its pid rather than being abandoned. Also sweep sibling version directories for this target's socket after launch, so the relay an app update supersedes (#13614, #13852) is visible and dealt with deliberately. Only a relay proven to hold nothing — argv matched, single socket holder, zero children re-checked on the host immediately before the signal — is SIGTERMed, and `reaped` is claimed only from a post-signal `kill -0` that failed. Anything unreachable stays `unverifiable` and untouched. |
||
|
Neil
|
02ffd40edc |
fix(ssh): stop treating an unanswered native-deps probe as broken deps (#17979)
probeRequiredNativeDeps mapped any thrown error to available:false, which both triggered the repair and fed resetDeps — so one dropped exec channel rm -rf'd node_modules/node-pty on a healthy relay and forced a node-gyp source build. Verdicts are now ok / blocked / unverifiable; only an answered probe may repair, and only an answered probe may name reset deps. |
||
|
Neil
|
2222e54754 | refactor(test): organize SSH and terminal recovery fixtures (#17751) | ||
|
Brennan BensonandMerge Sim
|
aabcc57366 |
fix(runtime): publish remote control outages to host surfaces (#17531)
* fix(runtime): publish remote control diagnostics to renderer * test(runtime): account for diagnostics bridge listener * fix(i18n): add runtime connection state labels * test(runtime): clean up shared control connection * fix(runtime): fence diagnostics by shared-control capability * fix(runtime): preserve authoritative transport state * fix(runtime): preserve diagnostic overlay lifecycle * fix(runtime): avoid publishing unchanged diagnostics state --------- Co-authored-by: Merge Sim <sim@local> |
||
|
NeilandBrennan Benson
|
fbe94ceff6 |
fix: close readiness gaps found by merged-change audit (#17159)
* fix(ssh): fence stale kills and retired pane replay * fix(ssh): support cancellable interactive authentication * fix(ssh): await remote catalog before snapshot adoption * fix(pty): contain Windows ConPTY input failures * fix(power): avoid redundant macOS display blocking * perf(editor): narrow markdown override subscriptions * fix(quick-open): close directory handles after reads * refactor(linux): remove unused proc socket scanner * fix(usage): apply flat Sonnet 4.6 pricing * ci: prime Node next native test cache * docs(skills): resolve snapshot cleanup data path * fix(ssh): recover install locks after host reboot * test(ssh): recognize boot-aware install locks * test(ssh): prove previous-boot lock recovery live * test(wire): pin pre-metadata release coverage * fix(terminal): preserve remote tab ownership through recovery races * test(runtime): fence replaced terminal handles in agent guard * fix(ssh): preserve remote snapshot authority across polls * fix(pty): contain late ConPTY output EPIPE * test(pty): register Windows exit watcher before kill * fix: close SSH and tab readiness race gaps * fix(tabs): retain headless order and placeholder titles * fix(build): avoid parallel electron-vite config race * test(windows): avoid MSYS temp path rewriting * test(windows): avoid killing exited PTY * fix(pty): avoid late ConPTY input teardown race * fix(terminal): sync reconnect error ownership after commit * fix(runtime): use canonical worktree identity comparison * test(ssh): assert complete cold-hydration baseline * test(windows): invoke quoted retention fixture via PowerShell * test(windows): read ConPTY grid through mode con * fix(terminal): publish PTY replacements atomically * fix(terminal): infer stale identity on reattach * fix(terminal): fence stale pane PTY callbacks * fix(terminal): fence stale pane binds after rebind * fix(terminal): reject stale pane transport callbacks * fix(terminal): fence mirrored reattach spawn callbacks * fix(terminal): replace stale pane PTYs on remount * fix(ci): size the Windows launcher-compile test budget from measurement `native-smoke (windows-latest)` fails ~4.5% of runs on `preserves a multiline argument through the compiled remote launcher` with "Test timed out in 15000ms" — on unrelated PRs, for reasons that have nothing to do with them. Across 176 sampled attempts it is the only red that job produced, and it hit seven different PRs in two days: #16900, #16904, #16915, #16955 (twice), #16979, #17014, #17085. The test is six process creations: powershell.exe forks csc.exe, then the freshly compiled orca.exe forks node.exe, twice. Hosted Windows runners periodically slow process creation down, and this test amplifies that far harder than anything else in the job. Comparing the 80 attempts where it ran under 3s against the 12 where it ran over 12s, its own median goes 2198ms -> 15917ms (7.2x) while the same file's powershell-only test moves 556 -> 686ms (1.2x), the cmd.exe and Git Bash process tests in the neighbouring file move 1.4x, and the other 35 files put together move 1.5x. Measured across those 176 attempts: 1881ms to 35438ms, p50 4264ms, correlation +0.881 with the job's total Vitest duration. 8 of 176 (4.5%) exceeded the 15s cap; 2 of 176 (1.1%) also exceeded the shared 30s testTimeout, so deleting the override and inheriting the config is not enough on its own. 60s clears all 176 with 1.7x headroom on the worst. This is slow, not hung. Every body here is synchronous spawnSync, so Vitest cannot interrupt one — the timer fires only after the body returns and the reported duration is real elapsed time. That is why a failure reads `× ... 22464ms` under `Test timed out in 15000ms`. The work finished; the stopwatch was short. Seven reruns at one identical head measured 2053 / 4680 / 5551 / 8732 / 13506 / 14868 / 21937ms — the last of those would have been red on code that had not changed. The 15s came from #8897, which raised this test off Vitest's built-in 5s default because the job then ran bare `pnpm vitest run`. #8909 landed 3h27m later and pointed the job at config/vitest.config.ts, which is the real fix for that. The constant stayed behind and has been the binding budget ever since. * fix(terminal): fence stale remount reattach ownership * fix(terminal): reconcile mounted pane identity after replacement * fix(terminal): fence stale reattach fallback ownership * fix(terminal): fence deferred SSH reattach ownership * fix(terminal): fence stale split pane ownership callbacks * fix(terminal): keep stale spawns from consuming startup --------- Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> |
||
|
Brennan Benson
|
fd9125ea8c |
feat(native-chat): Codex structured native chat restructure (#16729)
* feat(native-chat): port structured Codex sessions from restructure-recovery Rebuilds the desktop structured native-chat implementation from brennanb2025/native-chat-restructure-recovery (tip 4e31c08db3) on top of current main as a single commit, scoped to the local Codex path. Ported: - Structured agent-session core: durable record store + single-writer lease, canonical journal, agent-session wire host/attach/eviction/subscribers, `agentSession.*` RPC surface (registered via ALL_RPC_METHODS; host-side mobile allowlist included for wire compat), pty write gate, transcript additions, and the Codex app-server adapter/launch resolution. - Renderer: NativeChatStructuredSession view/composer stack, structured launch path with the single-flight guard, local structured session tabs sync, activation gate + structured inventory (read-only `agentSession.handoffStatus` probe), agent-session tabs in the tab strip, AI-vault structured session activation, and the settings pane with the parent Experimental Chat UI toggle plus the nested "Use updated structured native chat" toggle. New sessions require both flags, agent codex, no prompt, and a local non-WSL, non-Windows-host execution host (structured-native-chat-availability). - Fixes 72c013cea6 (verified Codex launch recovery), 8ddbaf5e3d (defer native terminal view switching affordances), and 4e31c08db3 (release the launch gate after a visibility retry) with their regression tests, including the third-launch-after-retry guard case. - Cross-version agent-session wire test + CI lane, packaging entries (proper-lockfile, agent-tooling asar excludes), and the wire-compat doc section. Deliberately not ported: mobile/ changes, the Claude structured runtime (only the claude-transcript-branch-proof and claude-structured-owner-identity leaf modules remain, backing the kept TUI-recovery arms), the terminal↔chat adoption/handoff flow (`agentSession.adoptTerminal`/`requestHandoff`, the handoff request engine, TUI adoption machinery, orca-runtime adoption methods), renderer switching affordances and their dead leftovers, the hook/subagent-status refactor cluster, and unrelated branch changes. The crash-during-acquisition recovery path (restart handoff adjudication, restore/reverse re-acquire, lease schema handoff keys) is kept because every plain direct launch depends on it; a trimmed handoff coordinator exposes only status/restore/close. Branch edits that targeted files main has since split (ipc/pty.ts, worktrees.ts, rpc/methods/terminal.ts, useIpcEvents, pty-connection, store/slices/terminals.ts, runtime-types, web preload) were re-applied to the split modules, preserving main's newer logic (Windows CIM fallback, browser tab close rework, cold-restore resume flow, dispatcher threading). Known seam: the mobile clipboard image-provenance CONSUMER gate ships (agentSession.send refuses unproven mobile image refs with agent_session_image_untrusted) but the producer hunk in rpc/methods/clipboard.ts stays with the unported mobile cluster, so mobile image sends into structured chat fail closed until that side ports. * fix(native-chat): trust only authenticated local image uploads * fix(build): preserve Windows process-tree patch application * test(windows): include process creation time in addon fixture * fix(build): run windows-process-tree node-gyp from the physical package dir gyp expands the node-addon-api dependency by probing node, whose cwd resolves to the package's physical directory in the store, so the emitted target is a store-relative ../../../../node-addon-api@... hop. gyp then resolves that hop against the rebuild cwd; from the node_modules symlink/junction it escapes the store and configure fails with "node_addon_api.gyp not found" (run 32999886072). Rebuild from realpath(package dir) so both bases agree, matching how the package manager itself runs native install scripts. The regression test replays gyp's expansion+resolution against the planned cwd and fails without the fix. * fix(native-chat): keep chat tabs visible through terminal closes and empty-worktree launches Two proven blockers in the native Codex tab contract: closeTerminalTab pre-empted the canonical unified close. With one terminal left it deactivated the worktree on a terminal/editor/browser-only check, blanking a workspace that still held a renderable agent-session tab; with two or more it pre-picked a successor from terminal entities only, re-stamping the group active before closeUnifiedTab's MRU/neighbor repair could land on the chat tab. Successor choice now defers to the unified contract whenever the terminal has a unified row, and deactivation is gated on the unified renderable count (matching leaveWorktreeIfEmpty), with the legacy pre-pick kept only for terminals without a unified row. A structured session created on an empty worktree was published into the host's headless group while preserveLocalLayout froze the local layout, leaving the tab in store but permanently off screen. A preserveLocalLayout owner now always takes client-owned placement — repairing a rendered leaf whose group record is missing, or materializing a rendered group on a truly empty worktree — and applies the client-derived layout repair while still rejecting host-authored layout. Regression tests drive the real store through closeTerminalTab (git worktree and folder workspace) and the real snapshot applier for the empty-worktree adoption states; all fail without the fixes. * fix(native-chat): close stale turns and retry rejected sends * fix(native-chat): retire hosted rows on structured tab activation * fix(native-chat): preserve rpc defaults across main merge * chore: format remote wire compatibility guide * test(native-chat): cover retry after unconfirmed send * fix(native-chat): reload outbox on session switch * docs(settings): disclose structured chat platform limits * fix(native-chat): await Codex launch-home preparation * fix(codex): align child-process allowlist with async trust bridge * test(identity): update inventory for tab surface refactor * fix(windows): preserve process-tree CRLF patch sources * fix(native-chat): anchor an unmatched chat echo where it was sent (#16117) * fix(native-chat): anchor an unmatched chat echo where it was sent The reported symptom was old user messages replaying below every new turn, so the conversation read as scrambled. The cause was not that the echo failed to match a transcript row. Claude consumes a mid-turn send through a `queued_command` attachment and writes no `type:"user"` record for it, so some echoes can never match, and no amount of matching will change that. The cause was WHERE an unmatched echo rendered: buildMobileNativeChatTransientData appended every pending item after the entire transcript, so it re-read below each turn that landed afterwards. Render each echo directly after the transcript row it was sent against, using the baseline the send already captures. An unmatched echo is then at worst a duplicate in the right position rather than a scrambled one, and it stays visible. Echoes sharing an anchor keep send order; a send with no baseline, or one whose anchor folding dropped, still falls back to the tail. Deliberately NOT fixed by deleting the echo. Inferring from send ordering that an echo can never match, then removing it, loses the user's own text for a message the agent did receive, and it cannot fire in the common case anyway - measured drain groups are 1,017 of size 1 against 55 larger. It also escalates an existing gap: the count pass has no baseline-tail guard, unlike the glue pass, while `messages` is a 40-row window that head-trims, resets on reconnect and grows at the front on loadEarlier, so a false landing there would license deleting a DIFFERENT outstanding message. That count-pass gap is real and left for a separate change; anchoring makes its worst case a duplicate in place rather than a scrambled conversation. * fix(native-chat): preserve folded echo anchors * fix(native-chat): preserve forward-folded echo anchors * fix(native-chat): keep leading folded echoes in place * fix(workspace-cleanup): show git status for every row (#16690) * fix(native-chat): refuse structured chat on every Windows execution path canUseStructuredNativeChat only refused win32 when a project runtime resolved, so folder-workspace keys (and other keys with no project runtime) failed open into structured chat on Windows. Fail closed on win32 unconditionally after the host check, matching the settings copy: local macOS/Linux only; Windows/WSL/SSH stay on terminal chat. * fix(native-chat): restore runtime refusals behind the win32 gate |
||
|
Neil
|
7bbb8adc61 |
fix(ssh): replay an undelivered remote PTY stop on the next handshake (#12447 item 1) (#17011)
* fix(ssh): replay an undelivered remote PTY stop on the next handshake A pty.shutdown that dies on the transport left the remote shell running forever: kill.ts marked liveness unverifiable and nothing retried. Record the undelivered stop on the existing durable SshRemotePtyLease and replay it against the authoritative host on the next handshake to that same target, fenced by the host-minted PTY incarnation so a replay cannot kill a later PTY that reused a recycled pty-N id. Retire the record on confirmed delivery, on the host reporting the PTY absent, and on a bounded TTL. No wire change: the fence reads incarnationId, already published on pty.listProcesses. A host that does not publish it degrades to no replay. * fix(ssh): do not leave a replayable kill order behind a reversible stop Worktree sleep stops through stopAndWait and marks those stops reversible; when one does not land the pane stays live and the user keeps using it. An order recorded there would come back on a later handshake and kill that terminal. Only killPtyFromRuntimeController — where the client gives the PTY up for good — records one, and it skips any PTY a reversible stop owns. * fix(ssh): cover the renderer kill route and harden the replay's evidence pty:kill is a separate implementation from killPtyFromRuntimeController and is the one an ordinary tab close reaches, so the record was never written on the path #12447 describes. Extracted it out of inspect.ts (which was over the line budget and was not what the file is named for) and wired both branches. Also: - finishPtyShutdown no longer retires the order. It runs on paths that asked the host and on paths that never did, so retiring there was a contract every caller had to know, and the one that forgot silently dropped a kill order. Retirement is the replay's, on inventory evidence only. - A recycled relay id now expires its lease. Declining to kill was only half: reattach fences on paneKey/tabId, never incarnation, so an untouched lease bound the user's old pane to whatever now holds the id. - Dropped isPtyAlreadyGoneError from the tombstone path. It matches message text a transport failure could wear; every tombstone now traces to a listing. - TTL is owned by a durable prune that actually deletes, not by a branch that was unreachable behind the read filter and only looked tested. - The replay re-reads the inventory per wave and re-checks the fence next to each shutdown, and can never reject into the connect path. |
||
|
Jinjing
|
c4b39295c1 |
style: format codebase (#16935)
* style: format codebase * style: format codebase * refactor: extract skill install dialog footer and content Extract footer and content sections from SkillInstallDialog and SkillInstallManagementDialog into separate components for improved maintainability and clarity of component responsibilities. |
||
|
Neil
|
a7b1da9e14 |
fix(ssh): a reattach may bind a pane but must never create one (#16751)
`reattachKnownPtys` treats every non-terminated lease as live and calls `persistPtyBinding`, which had no way to say "bind only". Two of its branches then rebuild UI the user is not asking for: - `pty-binding-persistence.ts:133-143` -- `if (args.incarnationId)` unconditionally deletes the pane's close tombstone. - `:145-160` -- on a tab-not-found it mints one via `createMinimalPersistedTerminalTab`. The in-code comment names its only intended caller: "pty:spawn can beat the debounced writer." Spawn. Reattach took the same branch. This is the mechanism canceled ticket STA-4268 described: "Leases have no pane incarnation and upsert only by target/PTY. Every nonterminal lease is reattached; frozen coordinates are passed to persistPtyBinding, which creates and flushes missing tabs and layout leaves." It was fixed in #13326, reverted by #14361, re-applied by #14384, and reverted again by #14395 (opened and merged nine seconds apart, empty commit body). `grep -rn "mayCreate" src/` returns nothing on main -- the mechanism is genuinely out of the tree. Four parts: 1. `mayCreate` (default true). When false, one pre-mutation check mirrors all four creating branches and returns `false` without mutating, so a refusal leaves nothing half-written. 2. The authority gate -- the part both prior attempts lacked, and probably why both were reverted. `mayCreate: false` alone refuses in two situations: "the user closed it" AND "the renderer has not published its layout yet". The second is routine on disconnect->reconnect and is almost certainly the #14361 tab-loss mechanism. The fence was not wrong; it was UNCONDITIONED. It is now passed only when `hasHostAuthoritativeTerminalMembership()` says the persisted membership speaks for this worktree, reusing the function already guarding the same question at `orca-runtime.ts:8608`. Losing a tab is worse than keeping a duplicate, so an unauthoritative session still gets the creating write. Authority is read from `local` because that is the partition the write lands in -- it is local's absence being interpreted. But a pane the `ssh:<target>` partition still holds is not gone, so it keeps its creating write; refusing there would strand a live pane behind a binding reattach can no longer reach. (SSH spawns bind into `ssh:<target>` while this reattach binds into `local` -- GH #12721/#12723, STA-3980. This does not fix that split; it refuses to judge from one side of it.) 3. `findTerminalTabIdForLeaf` -- bind resolves the tab from the live layout instead of the lease's frozen `tabId`. Only the leaf half of a pane key is remint-stable: `detachTerminalPaneToTab` moves a live pane into a new tab, so a stored tabId names the tab the pane left. Identity vs location. 4. Pane-keyed supersession -- retires siblings on `(targetId, worktreeId, leafId)` to `expired`, guarded by the durable binding, which reads `local` then `ssh:<target>` so it is correct whichever partition the binding landed in. `upsertSshRemotePtyLease` matched `(targetId, ptyId)` alone (`ssh-pty-lease-operations.ts:34-36`), so a new relay pty id on reattach minted a SECOND lease instead of updating the first, leaving the predecessor non-terminated with nothing to retire it. On refusal the lease goes `expired`, never `terminated` -- `expired` records that this shell has no surface to reach it through; `terminated` would assert an exit nothing here observed, which `docs/reference/ssh-execution-boundary.md` forbids. The remote process is left running. Deliberately NOT done: - A collision guard for `upsertSshRemotePtyLease`. Built, tested, and REMOVED -- its own test passed with the guard disabled, i.e. vacuous. Telling "same lease" from "recycled id on a different shell" needs a relay-start identity, which would be a twelfth per-tab identity concept; the codebase already carries eleven (784 refs) that SSH-v3 Phase 3 deletes. Left as an in-code NOTE. This handles lease DIVERGENCE, not COLLISION. - A port of #13324. Its own authors deleted its load fold and reverted its local-only reader in #13326 ("a headless-owned pane still gets a vote before its lease is retired"); porting it ships a state-destroying migration they removed. - `bindPaneShell` from #13325. Its purpose is making `isSupersededPtyId` live, and that fence does not exist in main. It would have been a refactor plus a silently-ignored `mayCreate` -- TS drops excess props through spreads (verified), which is why `mayCreate` is passed as a conditional spread here. Honesty about scope: this does NOT close the daily-tab-growth report. The deterministic e2e repro (`ssh-lost-kill-tab-resurrection.spec.ts`, later in this stack) is byte-identical before and after, and instrumentation shows why -- in that scenario `restoreReattachedPtyRuntime` is never called at all (`CREATING TAB` 9 hits, `reattach gate` 0, `BYPASS` 0). The fence is on a path that bug does not take. It is a real, separately-provable defect; it is not the headline fix, and must not be claimed as one. Evidence, A/B on this tree. Disabling the authority gate (`mayCreate = true`) and the supersession call by hand: **8 of 12 fail**, including "does not resurrect a tab whose closing pty.kill failed with a transport error" and "holds the live lease count flat across ten reconnects of one pane" (`[ 'pty-0', 'pty-1', 'pty-2', …(7) ]` vs `[ 'pty-9' ]`). The 4 that pass both ways are the over-refusal tripwires, which is the point of having them. Restored: 12 passed; 2,439 passed across `src/main/ssh`, `src/main/persistence` and `src/main/runtime/workspace-session`. |