Commit Graph
928 Commits
Author SHA1 Message Date
Jinwoo Hong 012e9f410c fix(runtime): recover adopted tui-idle and pin worker fixtures (#15569) 2026-08-20 15:51:48 -07:00
Neil ef096d539d fix(terminal): refuse a cursor on a screen read, and correct the source docs (#15563)
Review follow-up on #15380.

The RPC accepted `cursor` and `screen` together. The CLI refuses the pair, but
terminal.read is reachable without it, and honoring both answered with rendered
lines carrying the stream's pagination metadata — two frames of reference in one
payload, which is the confusion `source` exists to remove. The guard beside it,
withVisibleSnapshotFallback, already declines to substitute rendered lines when
a cursor is present; the screen path now agrees, at the RPC boundary where every
remote caller passes. Nothing could previously send both, since `screen` did not
exist, so rejecting breaks no existing caller.

The command notes and the runtime comment both still described the fallback as
`source: stream`, left over from renaming that value to `screen-unavailable`
during implementation. The spec text is surfaced through `orca help` and the
agent-context schema, so a caller following it would test for a value the code
never emits. Both now describe all four states, including that an absent source
means the host predates the field.
2026-08-19 22:42:39 -07:00
Brennan Benson a76a95d111 fix(terminal): make remote-host take-back release the phone-fit lock (#15473)
* fix(terminal): make remote-host take-back release the phone-fit lock

The remote-desktop branch of reclaimTerminalForDesktop was the one path that
rolled the presence lock back when its reclaim resize did not converge. On a
remote/SSH host that left the phone-fit banner stranded and made every
subsequent "Take back all terminals" click a no-op. Its sibling (active mobile
subscriber) released the lock but still reported the layout's `ok`, so the
desktop renderer skipped its post-take-back refit and focus.

Both now follow the guarantee the method already documents: an explicit desktop
take-back always drops the lock, and the trailing remote layout is best-effort.

* review: pin the driver flip and correct the applyMobileDisplayMode contract

- Tighten the held-branch driver assertion to the exact post-release state so
  it pins releaseDesktopTakeBack's flip rather than merely "not mobile".
- applyMobileDisplayMode's doc claimed reclaimTerminalForDesktop gates its
  transitions on the returned convergence flag. No branch does after this
  change; say so, so the gate is not reinstated.

Hooks skipped (machine load); oxfmt --check and oxlint verified clean manually.

* test(terminal): pin the converging remote take-back resize

The two take-back tests both force the reclaim resize to fail, so nothing
covered a take-back that converges. Deleting the `idle` driver flip left
every suite green while applyRemoteDesktopLayout no-opped on a still-mobile
driver — lock dropped, `true` returned, PTY stranded at the phone grid.
2026-08-19 18:13:57 -07:00
Brennan Benson 59c5624e14 fix(terminal): require a proven exit before retiring a subscription's lease (#15470)
isPtyKnownExited read a PTY record as exited whenever `connected` was false:

  if (pty) { return !pty.connected }

Its own leaf fallback, one line below, demands getTerminalState(leaf) === 'exited',
which is only true once lastExitCode is set. So the same function proves absence on
one path and infers it on the other, and the inferring path is the one that runs
whenever a record exists.

onPtyExit is the only writer of lastExitCode. The liveness sweep clears `connected`
with no exit code for every PTY behind a dropped relay, so that state is a lost
connection to a process that may still be running on the host — not an exit. Reading
it as one makes subscribeToPtyExit fire its listener synchronously at subscribe time,
which retires the lease and emits `end` for a live terminal. Mobile reads `end` as
"PTY gone" and rearms; after three attempts it stops and leaves the composer on
"Waiting for terminal…", which is where the 0.0.44 permanent lock comes from.

Use the runtime's existing three-valued discriminator so both paths demand the same
proof. 'unknown' now keeps watching, and the later real exit still fires the listener;
a subscription that outlives its PTY is still bounded by the connection abort.

Both callers are in subscribeToPtyExit — the subscribe-time fast path and the
registration-race re-check — and both want proven-exited, so neither changes shape.
2026-08-19 18:13:26 -07:00
Brennan Benson 30bf2647fc fix(mobile): replay a delivery-ambiguous worktree.create instead of failing it (#15472)
* fix(mobile): replay a delivery-ambiguous worktree.create instead of failing it

A socket close or response timeout rejects an in-flight worktree.create as
delivery-unknown: the frame reached the wire, so the host may already have
built the worktree. The client only replayed connection-migration cutovers,
so every other ambiguity surfaced as a create failure for a create that may
well have succeeded. Replay on the same clientMutationId — which the host
already dedupes — after waiting for the transport to come back.

* fix(mobile): bound the ambiguous worktree.create replay by the host's dedupe window

The replay was bounded only by a retry count, but what makes a replay reconcile
instead of building a second worktree is wall clock: the host drops a settled
create's dedupe record 60s after it resolves, and past that the replay is just a
fresh create that the host's suffix loop happily duplicates — for a folder
workspace, into a second workspace with the very same name and no collision
check at all.

Two paths ran past that window:

- The request-timeout path. A silently dropped response frame leaves the socket
  alive, so nothing rejects until WORKTREE_CREATE_TIMEOUT_MS — ten minutes, with
  no bound at all on when the host actually resolved. This was previously the
  path that replayed *soonest*, short-circuiting the reconnect wait because the
  transport still looked healthy. Invert it: every path that reports a real drop
  has already left 'connected' by the time the rejection surfaces, so still being
  'connected' identifies the timeout and is now refused.
- The reported-drop path. Worst-case detection is a full liveness idle period
  plus the missed-probe budget before the client even learns the socket is dead,
  and the old 20s wait on top of that overran the record. Derive the wait from
  the watchdog constants and the TTL instead of hardcoding it, and anchor a
  single deadline at the first ambiguity so a second wait gets the remainder
  rather than restarting.

The TTL now has one definition shared by both processes, so the client asserts
its budget against the host's real window instead of a copied literal.

* fix(mobile): end the reconnect wait on a revoked pairing, and pin the wait's behavior

waitForRpcClientReconnected resolves only on 'connected' or the timeout, but an
'auth-failed' client never reaches 'connected' — so a create interrupted by a
revoked pairing sat out the full wait before surfacing the error it already had.
Treat auth-failed as a terminal answer on both the fast path and the listener.

The helper also shipped with no tests of its own: its already-connected fast path,
its timeout path, and the synchronous-notification-during-subscribe teardown were
only ever exercised indirectly through the retry suite, and neither RpcClient
implementation notifies synchronously, so that branch had no coverage at all. Add
a direct suite covering all of them, asserting listener and timer teardown rather
than just the resolved value.

Also give the fake-timer tests an explicit timeout. advanceTimersByTimeAsync
yields through real macrotasks between ticks while vitest's own budget runs on
real time, so on a loaded runner the default 5s is reachable — observed once as a
spurious timeout in this suite.

* fix(mobile): bound the ambiguous replay in wall clock, not timer time

The replay window was derived from the liveness watchdog's own budget
(idle + missed probes x probe timeout). That is a bound on how long the
watchdog takes to *fire*, not on how much wall clock passed. iOS and
Android suspend JS timers while the app is backgrounded, so across a
background cycle the socket dies silently and the pending create rejects
delivery-unknown minutes later with the timer-derived ceiling still
reading ~44s. The replay then lands well past the host's 60s dedupe
record and the suffix loop builds a SECOND worktree - for a folder
workspace, one with the very same name and no collision check at all.

Anchor the deadline on the watchdog's lastInboundAt instead: a wall-clock
stamp of a frame that really arrived, so it stays honest across a
suspension. Fall back to the send time when the transport can't vouch for
one (relay sessions run with idleProbeMs: null), which errs toward
refusing the replay.

Also restore the delivery-unknown discrimination test that the
still-connected guard had made vacuous, pin the still-connected guard
itself against a live inbound stamp, and pin the deadline against being
re-read from a fresher replacement session.
2026-08-19 18:10:39 -07:00
Jinwoo Hong 0e5b348414 fix(repos): keep the desktop-owned manual project order authoritative across paired clients and SSH catalog publishes (STA-4850) (#15538) 2026-08-19 17:26:16 -07:00
Neil a61b39a9a6 fix(runtime): stamp a runtime's own project setups as local, and report remote status about the remote (STA-4792) (#15376)
* fix(runtime): stamp a runtime's own project setups as local, and report remote status about the remote (STA-4792)

Two independent frame-of-reference bugs, both from code describing one machine
while labelled as another.

#15366 — projectHostSetup.* persisted the caller's host id verbatim. Those
`runtime:<environment-id>` ids are minted by the calling client's own pairing
store, so they name a machine only relative to that client. A client sending
one is addressing this runtime, and runtimes do not proxy these calls onward,
so the host it names is us. Storing the client's spelling made one machine look
like a different host to every other client, hid its rows from them, and
defeated the (projectId, hostId) duplicate check — two laptops paired to one
server each created their own setup for the same checkout. Re-spell it as
`local` at the RPC boundary. Rows written earlier keep their old stamp; readers
already project `local` back to `runtime:<their-id>`, so the client-visible
model is unchanged and no ids are rewritten.

STA-4792 defect 4 — `status --environment <name>` hardcoded app.running:false
to mean "no desktop on THIS machine" while every other field in the same object
described the target, including a desktopWindowStatus echoed straight from it.
The result contradicted itself and read as "that run was headless" when the
remote GUI was up. `app` now describes the target, keyed off the one window
status that requires a live renderer, and the result names its own subject so
the frame can't be misread again. The remote pid is not knowable, so it stays
null.

STA-4792 defect 2 gets a regression test rather than a fix: routing already
made the client remote, which is what stops a Windows destination being joined
to the local cwd. The test pins the exact reported invocation.

* fix(status): share the remote app projection with the SSH host passthrough, and name the version gap on project host setup

Two review follow-ups.

The SSH host passthrough answered `app.running: true` unconditionally for the
Orca host a caller reached over SSH, claiming a desktop app even for a headless
`serve`. That is the same defect as the paired-server path, one transport over,
so the projection moved to shared and both now answer the question the same way.

`--host runtime:<id>` routes project commands to a paired server, which means a
client can reach a server that predates project host setup without meaning to.
That answered a raw `method_not_found`, which reads as an Orca bug rather than a
version gap; the CLI now names it the way the desktop already does.

Reverted a third change: making the persistence duplicate check treat `local`
and `runtime:*` as one machine. That assumption holds at the RPC boundary, where
a `runtime:` host means the runtime being addressed, but not in the store, which
also records independent provisioning metadata for machines that are not itself.
An existing test covers exactly that, and it was right. The duplicate
convergence therefore stays bounded to rows written after the normalization.
2026-08-19 17:12:17 -07:00
Brennan Benson 7fa23438f4 fix(mobile): keep a desktop take-back from being undone by a passive viewport report (#15539)
A phone that stays subscribed after 'Take back this/all terminals' re-phone-fits
the PTY and re-takes the presence lock on its next terminal.updateViewport, which
iOS forces on app resume and on every reconnect. updateMobileViewport consulted
only mobileDisplayModes, which reclaimTerminalForDesktop resets to 'auto' before
returning, so the take-back had no suppression left. Treat an in-force desktop
take-back the same as the existing desktop display mode: record the viewport,
apply nothing.
2026-08-19 16:52:58 -07:00
Jinwoo Hong 7675da363e fix(remote): stop paired-tab resurrection, ghost agent rows, and frozen visible panes (STA-4593) (#15459) 2026-08-19 15:31:26 -07:00
Brennan Benson 9b8dbd8930 fix(mobile): keep a terminal lease alive when the handle lookup fails (#15463)
* fix(mobile): keep a terminal lease alive when the handle lookup fails

`waitForTerminal(..., 'exit')` rejects with `terminal_handle_stale` whenever the
handle cannot be resolved right now. The loudest source is
`record.rendererGraphEpoch !== this.rendererGraphEpoch` — every renderer graph
reload invalidates handles issued before it, on panes whose PTY is untouched.

All three terminal.subscribe branches wired that rejection straight to teardown
(`.catch(() => registration.releaseIfCurrent())`), so the host retired a live
lease and emitted `end`. Mobile reads `end` as "PTY gone" and rearms; after
MAX_REARM_ATTEMPTS = 3 it stops and leaves the composer on "Waiting for
terminal…", recovering only when a replacement handle arrives. A live PTY keeps
its handle, so no replacement ever comes: the pane is dead until the app is
backgrounded and reopened.

Demand proof instead. Retire on abort (this socket is going away) or when
isLeafPtyProvenAbsent says the process is gone; a real exit still resolves and
retires through the .then leg. Absence must be proven, never inferred from a
lookup that failed.

Reproduced live: a paired 0.0.44 client over LAN, fault injected on the handle
lookup, composer locked on "Waiting for terminal…" and STILL locked 97s after
the fault was lifted with the PTY alive and the handle unchanged.

Narrows one behavior #14992 pinned: a stale handle alone no longer retires the
owner. The ownership property is unchanged — once absence is proven, the owning
registration is still what retires. That test is updated, not deleted.

* fix(mobile): observe PTY exit independent of stale handles

* fix(mobile): stop subscribe setup when an exited pty releases synchronously

The runtime doubles for terminal.subscribe never stubbed subscribeToPtyExit,
so the new lifetime watcher threw and killed every subscription under test.

Also return early when the watcher releases synchronously: cleanup had already
run, so the stream handlers, view subscribers, and mobile presence registered
afterwards were never torn down.
2026-08-19 14:14:00 -07:00
Neil 3ffab9a6b3 feat(terminal): read the rendered screen with terminal read --screen (STA-4792) (#15380)
* feat(terminal): read the rendered screen with `terminal read --screen` (STA-4792)

`terminal read` returns accumulated pty output with escape sequences stripped.
That is the right answer for "what happened over time" and the wrong one for
"what is on screen": any program that repaints a line comes back as stacked
fragments, so one `clear` typed key by key reads as `cclclecleaclear`, and a
prompt that draws a space by moving the cursor loses it. Nothing in the output
said which question had been answered, so it was used as rendering evidence and
produced false conclusions.

The runtime already knew how to render — it replays the byte stream through a
headless emulator — but only as a fallback for blank reads, alternate screen,
and never-attached ptys. A normal attached terminal never reached it. `--screen`
asks for it directly.

Every read now reports its source, which also surfaces the pre-existing
snapshot fallback that until now swapped rendered lines into an ordinary read
with no indication. `screen-unavailable` distinguishes "asked for a screen,
none could be rendered, here is the stream" from a stream the caller asked for,
and an absent source means the host predates the field. Because an older host
strips the unknown param and answers with its ordinary read, `--screen` against
one fails with that explanation rather than passing the stream off as a screen.

`--screen` and `--cursor` are mutually exclusive: a screen is the current frame
and has nothing behind it to page.

* refactor(terminal): stamp the screen source where rendered lines enter the read

Inferring it from tail array identity worked but made a load-bearing contract
out of reference equality; any later path spreading the read would silently
mislabel. Rendered lines only enter through one builder, so it stamps there and
anything still unlabelled is the stream.
2026-08-18 22:51:37 -07:00
Jinwoo Hong 9b5538d786 fix(runtime): scope create-with-activate navigation to the requesting client (STA-2802) (#15407) 2026-08-18 21:37:35 -07:00
Neil ccb2305c8d refactor(runtime): name every mobile-session close outcome so the tombstone decision is explicit (STA-4718) (#15346) 2026-08-18 15:17:33 -07:00
Brennan Benson e5a1744e27 fix(agent-status): stop an idle title retiring a pane pending a human answer (#15351)
A Claude pane parked on a permission prompt published `done` to mobile and
paired clients — the state that retires the card — while the user was still
being asked.

The title layer cannot express permission for Claude or OpenCode: a title reads
`permission` only from a vendor glyph or a synthesized `<Agent> - action
required` label, and SYNTHETIC_AGENT_TITLE_PROFILES has no entry for Claude
(OpenCode sets synthesizeTerminalTitle: false). So `titleConfirmsState` is
unreachable for them and the hook's only protection was a timestamp comparison.

That comparison loses because the two title clocks advance on different events:
lastAgentStatusRichInvalidatedAtEpochMs moves only when the title's status class
changes, while lastOscTitleEpochMs moves on every write. Measured against a real
claude 2.1.234 (two runs, identical structure): the title settles working->idle
~12ms after PreToolUse, PermissionRequest fires ~39ms later, then one more
same-class repaint lands ~123ms after the hook. That last repaint pushes title
evidence past a hook that is still current. The title then goes silent for the
whole prompt (31s and 85s in the two runs), so the wrong verdict is frozen for
as long as the user takes to answer.

Treat a fresh `waiting`/`blocked` under an idle title as non-renewable. Scoped
to idle on purpose: idle is the absence of activity evidence, whereas a
`working` title or a null/shell/identity-only one contradicts the hook and must
still retire the row and its stale question (#11761). Bounded by the stale
window so an agent killed while parked still decays.

Both state names matter: Claude's PermissionRequest normalizes to `waiting`, not
`blocked`, so a guard written against `blocked` alone passes its own tests and
misses every Claude permission prompt.
2026-08-18 15:10:36 -07:00
Jinwoo Hong 79be5b7fde feat(orchestration): report a worker blocked on a human prompt (STA-4513, STA-3714) (#15261)
* feat(orchestration): report a worker blocked on a human prompt (STA-4513, STA-3714)

A lane parked on an approval, trust, or permission prompt looked exactly like a
lane that was thinking or inside a long tool call. On origin/main, driving a real
cursor-agent through Orca:

  surface                        running `sleep 60`   awaiting approval
  worktree ps agents[].state     working              working
  terminal show / list           no such field        no such field
  terminal wait --for tui-idle   satisfied: true      satisfied: true
  worker-show                    no agent state       no agent state

The runtime already fuses hook state, OSC title, and matched prompt text into a
`permission` verdict inside getTerminalAgentStatus — it was reachable only from the
renderer, and it was blind to cursor-agent approvals. Two gaps, one boundary.

Exposure: getTerminalInteractiveWait publishes that same fusion, minus the async
foreground probe, as `agentWait` on `terminal show` and on `worker-show`'s
observation. It carries the evidence that proved the wait (hook, prompt-text, or
title) so a coordinator can weigh it. Null means no proof; a missing field means
the host predates it — absence is never read as "not waiting".

Detection: cursor-agent's hook set has no approval event and beforeShellExecution
fires identically for auto-allowed commands, so its rendered menu is the only
authority. Matched on the key-bound choices rather than the prose, requiring two,
and self-clearing when the follow-up input line returns. Its live spinner title is
exempted from the staleness rule that clears startup modals, because cursor keeps
spinning while it waits.

Falls out of routing it through the shared verdict: `dispatch --inject` into a
cursor pane on an approval now refuses with agent_prompt_blocked instead of typing
the preamble into the dialog.

Fixtures are captured verbatim from cursor-agent 2026.08.11-e8db854 driven through
Orca; the same case matrix was replayed live against a built runtime.

terminal list stays untouched: its rows would each need a full tail scan, and
STA-4694 owns the one-call-per-run aggregate.

* fix(orchestration): only call a Cursor approval live while it owns the screen

Independent review found the approval detector trusted one dismissal string, so
any later output that did not contain cursor's follow-up line left the menu
reading as a live wait. Reproduced: a tail of the real menu followed by two lines
of ordinary output returned agent-approval-prompt, which fails tui-idle and
refuses prompt injection on a healthy lane.

Replaced with the structural property the string was standing in for: a live
dialog owns the bottom of the screen, so the last choice may sit at most one line
above the end of the retained tail. That tolerates a status footer or a partial
line mid-redraw without admitting scrollback, and it drops the vendor prose.

Being bottom-of-screen is also the dating this reason needed, so it no longer
requires waitBlockedAt. A tail restored from terminal history carries none, and a
lane parked on a prompt emits no bytes — so before this, an Orca restart made
exactly the lane both issues are about go quiet for good. The startup modals keep
the timestamp rule: their text lingers in scrollback with nothing to say whether
it was answered.

Also from review:
- worker-show and federationShow reuse the verdict showTerminal already computed
  rather than rescanning the tail, so the two can no longer disagree.
- The worker-show test now drives a real runtime, real PTY tail, and the real
  detector; it previously mocked getTerminalInteractiveWait, so it would have
  passed with detection permanently returning null.
- The guard claim is now asserted against the guard: a blocked pane rejects both
  assertTerminalAgentSendable and sendTerminalAgentPrompt, and a working pane
  still passes.
- Added a non-local (connectionId) pane case, since the verdict is derived from
  retained tail and title state on every host.

* fix(agent-status): stop a hook wait from outliving its agent

A third reviewer caught that the hook branch proved agent ownership from the pane
title alone, while the shared verdict it claimed to reuse also probes the
foreground process. A shell that takes a pane back usually sets something like
`user@host: ~/repo`, which no title rule recognizes, and a hook row stays fresh
for AGENT_STATUS_STALE_AFTER_MS — so a dead agent could be reported as waiting on
a human for half an hour.

Hook evidence now goes through getTerminalAgentStatus, which is the only thing
that can answer whether an agent still owns this PTY. The two prompt branches skip
it: a matched prompt is on the pane's screen now, so it proves itself. That makes
the probe cost fall exactly where correctness needs it, and getTerminalInteractiveWait
async, which only showTerminal had to absorb.

Also trims the comments the same reviewer flagged as longer than the repo's rule.

* test(agent-status): pin that a dead pane stops reporting a human wait

A fourth reviewer noted the approval menu sits at the bottom of a dead pane's tail
forever, and that no test covered process exit with no trailing output. The
snapshot already refuses an exited pane, and worker-show gates agentWait on proven
identity — this pins both so neither can drift into reporting a worker that needs
intervention as one that needs an answer.

* fix(orchestration): never report an unchecked worker as not waiting

Automated review caught that the three worker paths which return before the wait
is ever evaluated — unattached, missing, and identity_changed — then had their
undefined coerced to null by the emitters. A worker whose process was replaced was
reported as `agentWait: null`, which reads as "Orca looked and nobody is waiting"
when Orca never looked. That is the false negative this field exists to remove.

The field is now emitted only when it was evaluated, so a present null is a claim
about the pane and an absent one means nobody looked — because the host predates
the field, or the worker's identity could not be verified. The CLI and the
worker-show note say that rather than blaming an old host.

Covered on the context-only path, where the regression test fails against the
previous behavior; the supervised and federated emitters take the identical
one-line change.

Also trims the two test-file headers to one statement of purpose.

* fix(agent-status): tighten the Cursor menu match and stop guessing on unknowns

Fourth review round, three findings, each reproduced before acting.

Matching each choice marker with an independent lastIndexOf let text outside the
menu carry the anchor. An agent narrating "next time I'll suggest Run Everything"
after the menu was answered pulled the match down to the bottom of the screen and
revived it. The match is now confined to the last lines of the tail, and a choice
is a line that ends in the key that picks it — prose writes the same words but not
the same shape.

The one line of slack under the dialog went with it. It was a guess; every capture
of a live dialog ends on its last choice, and one line is exactly enough room for
that narration. A redraw caught mid-flight now reads as no wait until the next
poll, which is the safe way to be wrong.

The hook branch awaited a foreground probe that reaches a PTY controller which may
be a remote host, so a wedged probe stalled every caller of showTerminal — a path
that never probed before. It is bounded now, and a timeout leaves the wait
unevaluated rather than claiming there is none.

Which is the same distinction the previous commit only fixed one level up:
getTerminalInteractiveWait itself turned an unreadable pane into `null`, so
showTerminal published "looked, nobody waiting" for a pane it could not read. It
returns undefined there, showTerminal omits the key, and worker-show's text output
prints unknown rather than rendering it the same as none.

* fix(agent-status): bound the wedged probe's cost and stop matching prose keys

Fifth review round. No correctness defects in the shipped behaviour this time; two
robustness holes and the documentation of the contract.

The bounded probe abandoned the wait but not the request, so a coordinator watching
a wedged remote host added one live probe on every poll. It is single-flighted per
PTY now, the way the leaf-absence probe already is.

The trailing-key rule that separates a menu row from the agent narrating a choice
was written as a character class, and any lowercase run up to twelve characters
satisfied it — "…suggest Run Everything (as before)" passed. Spelled out as key
names instead, which also lets the glyph forms of those keys through.

The contract wording said an absent agentWait meant an old host or an unverifiable
identity. It also covers an unreadable pane and a probe that did not answer, and a
reader diagnosing an old peer from that would be wrong. Corrected on the type, the
worker-show note, and in docs/reference/remote-wire-compatibility.md, which had no
entry for a field whose absent and null states mean different things.

Also strengthens the worker-show agreement test, which compared the terminal and
observation payloads without asserting either held the expected wait, so it passed
when both were absent.
2026-08-18 14:19:20 -07:00
Jinwoo Hong a77a2f93f7 fix(remote): search Quick Open paths on the host (#15158) 2026-08-18 13:32:48 -07:00
Jinwoo Hong bef76953d7 fix(orchestration): record why a terminal's process is gone (STA-4603, STA-4536) (#15244) 2026-08-18 13:17:42 -07:00
Jinwoo Hong a27527ae06 fix(orchestration): deliver worker-exit escalations to lightweight Run coordinators (STA-4604) (#15235) 2026-08-18 12:35:24 -07:00
Brennan Benson 8e13485c9b fix(stats): count agent sessions from hook transitions, not OSC titles (STA-2445) (#14657)
* test(stats): dual-record the OSC-title detector against canonical hook transitions

Adds an AgentSessionTransitionRecorder that derives agent-session start/stop
boundaries from agent-hook status transitions, and a side-by-side comparison
that feeds both pipelines into a real StatsCollector.

Nothing is rewired yet — this commit only measures the delta:

                                 title detector   canonical
  hook-only agent                       0             1
  braille-spinner non-agent TUI         1             0
  one agent, one reconnect              2             1
  totals                                3             2

Refs #10201, STA-2445.

* fix(stats): count agent sessions from hook transitions and delete the title detector

Switches StatsCollector off AgentDetector and onto the canonical agent-hook
status stream, then removes the detector and its raw-PTY invocation.

- main/index.ts subscribes the recorder to subscribeEnrichedStatus and
  subscribePaneStatusClear, next to where StatsCollector is constructed.
- orca-runtime.ts no longer feeds raw PTY bytes to a stats detector.
- StatsCollector keys sessions on a stable pane key, not a per-spawn ptyId.

Fixes #10201, refs STA-2445.
2026-08-18 00:35:50 -07:00
Brennan Benson eb0ec39242 fix(runtime): stop one unreachable relay from freezing every workspace as active (STA-517) (#14649)
* fix(runtime): stop one unreachable relay from freezing every workspace as active

The worktree.ps liveness refresh is the only thing that retires an exited PTY,
and mobile renders "active" straight off the summary it produces. Its aggregate
inventory ran every provider through Promise.all with no per-provider deadline,
so a single SSH relay that rejected — or simply did not answer inside the 3s
budget, since a relay list runs to the mux's own 30s default — cost the runtime
the whole inventory. Nothing was ever proven dead, so every retained pane kept
reporting hasHostSidebarActivity/liveTerminalCount, and the SSH workspaces stayed
"active" on mobile for as long as the connection stayed unreachable.

Settle each SSH provider independently and forward the caller's deadline, so
local and healthy relays are still reconciled. A provider that does not answer is
unknown, not empty: the runtime's existing hasPty rescue keeps its panes. A local
failure still fails the aggregate, matching pty:listSessions.

The restored-orchestration-authority sweep now runs after that rescue, so a pane
the controller still vouches for keeps its handle instead of losing it to a
listing that merely omitted it.

STA-517

* test(runtime): assert the provider scope, not the exact arity, of inventory calls

These assertions exist to prove which provider scope the inventory asked for.
Forwarding the caller's deadline added a second argument, which broke them on
arity alone. Match the scope argument and require a numeric deadline beside it,
so the intent is preserved and the budget is covered too.

* test(runtime): type the inventory mock's scope parameter

A bare `async () =>` mock types mock.calls as an empty tuple, so reading the
scope argument off it fails typecheck. Declare the parameter the runtime
actually passes.
2026-08-18 00:35:16 -07:00
Neil e39def3825 fix(repo-identity): bound git remote-identity probes and retire them with their repo (#15196)
* fix(repo-identity): bound git remote-identity probes and retire them with their repo

The local `git remote -v` probe ran with no timeout and no signal, and the
runner only arms its kill timer when a timeout is passed, so a hung NFS/SMB
cwd or a wedged `wsl.exe -d <distro>` left the promise unsettled and the child
alive. Because the sweep is sequential and dedupes per location, that one
wedged location stalled enrichment for every other repo.

- probeGitRemoteIdentity/detectGitRemoteIdentity take `{ signal, timeoutMs }`;
  local reads get the 5s background local-git-read budget, SSH gets a budget
  under the relay's 30s request timeout. Timeouts/aborts still map to
  `unavailable`, never `no-remote`, so they cannot clear a resolved identity.
- In-flight probes are tracked with an AbortController and retired (aborted +
  dropped) when their location is no longer backed by a repo, so a re-added
  repo is not poisoned by the dead entry and a retired probe cannot re-seed a
  retry deadline.
- Added the missing sweep-level guard so repos:list / projects:list /
  projectHostSetups:list coalesce into one pass instead of stacking one
  sequential sweep per list IPC.

STA-4452

* refactor(repo-identity): bound the enrichment listener set to stable caller references

Every call site allocated a fresh onChanged closure, so the Set that notifies
coalesced sweeps deduped nothing: during a chain that never quiesces it grew one
entry per list IPC and multiplied the repos:changed broadcast. Hoist the closures
to stable references in ipc/repos.ts and OrcaRuntimeService, and state the
contract on the set.

Also: guard the synchronous retirement call so the fire-and-forget entry point
keeps its no-throw contract, drop the placeholder promise in favour of building
the in-flight entry in one shot, and make the coalescing test use a shared list
reference plus a distinct runtime reference so it detects both stacked passes and
a dropped caller.
2026-08-17 20:43:06 -07:00
Neil 0e96b82e44 fix(mobile): keep phone tab selection across host snapshots
* fix(mobile): keep phone tab selection across host snapshots

Preserve device-owned tab focus across ordinary host republications while explicit follow navigation remains authoritative. Retire closed selections across clients so stale snapshots cannot resurrect tabs.

* fix(mobile): acknowledge session tab closes

* fix(mobile): avoid tombstones for uncommitted closes

* fix(web): implement session close IPC stubs

* refactor: simplify mobile tab close flow

* fix: bound session tab close confirmation
2026-08-17 18:26:52 -07:00
Brennan Benson 8cd338357e fix(runtime): tear down terminal subscriptions only through their owning registration (#14992)
* fix(runtime): tear down terminal subscriptions only through their owning registration

A terminal.subscribe teardown was keyed on `${terminal}:${clientId}`, which is
stable across reconnects. cleanupSubscription invokes whichever cleanup currently
owns that key, so after a mobile reconnect rebound the id, a late teardown from the
dead connection killed the replacement stream and the terminal froze (STA-4510).

Add registerOwnedSubscriptionCleanup, returning a registration handle whose
releaseIfCurrent no-ops once the id has been rebound, and route all 12 teardown
call sites in the three terminal.subscribe branches through it. Register-time
eviction now also targets the owner it captured rather than re-resolving the key.

terminal.unsubscribe gains the same ownership rule via connectionId, matching the
runtime.clientEvents.unsubscribe precedent: make-before-break migration sends the
unsubscribe over the old session after the new one has already rebound the id.

The teardown-by-key pattern predates the bug; #7490 made it reachable by binding
the exit-waiter to the per-socket abort signal, so every socket close now runs it.

Tests use a faithful subscription-registry double; the ad-hoc Map stubs they
replace never evicted the prior generation, which is why no test caught this.

* test(runtime): migrate the remaining subscription stubs to the faithful registry

streaming.test.ts and terminal-provider-snapshot-sequence.test.ts still stubbed
registerSubscriptionCleanup only, so terminal.subscribe's owned registration was
undefined and teardown never fired. Assert the registration is retired rather than
spying on cleanupSubscription, which the owned path now calls internally.

* fix(runtime): guard the lease-only presence release and use the shared registry double

Review findings on this PR:

- The lease-only branch's compensating handleMobileUnsubscribe ran unconditionally
  after a rebind. Presence is keyed (ptyId, clientId) with no refcount and `closed`
  is exactly the post-rebind state, so a superseded handler deleted the replacement
  subscriber's presence. Gate it on the registration still being current. This also
  gives SubscriptionRegistration.isCurrent its production caller.

- The STA-4510 regression test hand-rolled a second registry copy that diverged from
  the shared double (no try/catch around cleanup, no in-flight join). Import the
  shared double instead, so the test proving the bug uses the same fidelity as the
  rest of the suite.

- cleanupSubscriptionIfOwnedByConnection treats an absent connectionId as authority.
  That is the connection-less local unix-socket path, not an oversight; say so.

* fix(runtime): drop the lease-only presence guard; it disabled a real compensation

Review pass 2 showed the guard added in the previous commit was wrong.

registration.isCurrent() is always false whenever `closed` is true: either our own
cleanup ran, in which case cleanupSubscriptionAndWait already deleted the map entry,
or a rebind replaced it. So the guard did not distinguish the two cases — it made the
compensating handleMobileUnsubscribe unreachable, which is precisely the
resurrect-after-cleanup case that line exists to handle.

The scenario the guard was meant to fix is also not reachable: the lease-only call
passes no viewport, and both !viewport paths in handleMobileSubscribeInternal return
with no await, so the subscribe resolves in microtasks and a socket close cannot win it.

Revert the guard, and drop SubscriptionRegistration.isCurrent with it — it had no
remaining production caller and shipping unused runtime API invites exactly this.

Also from review:
- cleanupSubscriptionIfOwnedByConnection reported false for an id with no registration,
  conflating 'refused, another connection owns it' with 'already gone'. Report true.
- Note on the registry double that it mirrors production and can drift; the runtime
  tests pin real behavior, the doubles only pin routing.

* fix(runtime): make the unsubscribe refusal observable and restore test-double parity

Review pass 3:

- The registry test double omitted production's 'unregistered id is already gone'
  early-out, so it returned refused where production returns gone. The legacy
  terminal.unsubscribe path reaches that branch with a never-registered bare id, so a
  routing test would have locked in the inverse of production.
- terminal.unsubscribe ORed the bare-id and composite results. Registrations always use
  the composite, so the bare id reported 'already gone' and masked a real ownership
  refusal: a stale connection was correctly refused but told unsubscribed: true. The
  composite answer is authoritative when we try it.
- Pin why the lease-only compensating handleMobileUnsubscribe is deliberately unguarded:
  it is safe only because that call passes no viewport and returns with no await.
- Cover the no-registration branch, which had no test.

* fix(runtime): report an unsubscribe refusal without masking a real teardown

Review pass 4 disproved the previous commit's rationale. A clientless legacy-JSON
stream registers under the bare terminal id, not the composite, so either call in
terminal.unsubscribe can be the real teardown. Overwriting reported false after a
destructive success; the earlier OR reported true after a refusal. AND over the calls
that actually ran is the honest aggregate: false needs a genuine refusal.

Tests:
- cover the bare-id path, which the ownership tests never exercised
- pin the microtask invariant the unguarded lease-only compensation depends on. The
  first version of that test was vacuous: it raced two setTimeout(0) timers, and the
  earlier-registered one always won, so it passed with an await injected. It now drains
  microtasks only and goes red under that mutation.
2026-08-17 16:13:45 -07:00
Brennan BensonandQA 64de8dd637 fix(workspaces): delete on the confirmed host, and make both hosts' rows selectable (STA-4343) (#15013)
* fix(workspaces): host-qualified workspace deletion (STA-4343, STA-4448)

Squashed integration of PR #14606 + the codex review-loop output, replayed
onto current main. Granular history preserved on brennanb2025/sta-4343-review-full.

Fixes the regression from #13413: a workspace id is repoId::path with no host
component, so the same repo at the same path on two hosts published one id for
two workspaces, and deletion routed by that id landed on whichever host routing
preferred - usually the ACTIVE one, not the row the user confirmed.

- removeWorktree takes a REQUIRED host-qualified WorktreeRemovalTarget; omitting
  the host is a type error. All destructive callers migrated.
- Projections dedup on (host, id), so two hosts render as two selectable rows
  while the createWorktree/fetchWorktrees race duplicate still collapses.
- Ephemeral VM cleanup is host-scoped. It matched on bare workspaceId, so the
  host-scoped delete path destroyed the SURVIVING host's VM and its unpushed
  filesystem - a leak fix that had become data destruction.
- Selection, keyboard routing, lineage grouping and Space rows carry host
  identity end to end; fixing the executor dedupe alone would have turned
  one-row intent into deleting both hosts.

Files split to stay under max-lines rather than raising any cap.

* refactor: split files that crossed max-lines

The review-loop commits used --no-verify, so the pre-commit hook never
enforced the caps. Extracted cohesive units rather than raising any limit:
renderer teardown, delete-with-toast, pinned-group rows, host-scope helpers,
workspace-kind predicates, filter actions, kanban drag selection, the
renderer removal result type, and the native-chat persistence tests.

* refactor(workspaces): extract cleanup deletion-phase selector

Clears the last max-lines violation and the import-type side effect the
changed-code gate flagged.

* refactor(sidebar): track the delete-dialog extraction modules

* fix(workspaces): preserve host identity across remaining surfaces

* fix(sidebar): re-carry host through the rewritten palette result model

#15170 replaced PaletteSearchResult while this PR was open. Re-applied the
host qualification on top of the new model instead of taking either side:
results carry worktreeHostId again, and the board filter keys its matched
set on host identity rather than the bare id.

Known gap, documented in the board test rather than deleted: searchWorktrees
resolves evidence through a `documents` map keyed by BARE worktree id, so two
same-id host rows collapse before this code sees them. Closing that belongs
with the palette work.

* test(cmd-j): pin the palette collision gap instead of asserting the old model

The palette collision test asserted two host-qualified rows, which #15170's
rewrite made unreachable: item ids are bare again and worktreeMap is id-keyed.

Rewritten to assert what holds — activation always names a host — and to pin
the defect it exposes: two same-id rows render on ONE command value, so React
sees duplicate keys and a click on the first row activates the second row's
host. That reproduces on main, so it is pre-existing, not from this PR. Pinned
rather than deleted so fixing it must update this test.

---------

Co-authored-by: QA <qa@local>
2026-08-17 15:57:07 -07:00
Jinwoo Hong 0bedeea642 fix(orchestration): expose unsupervised dispatch lanes (#15105) 2026-08-17 13:53:26 -07:00
Brennan Benson 32ee3b0536 reland(browser): route every cookie-import write through CDP identities, and never clear what it will not write back (#15030)
* reland(browser): restore CDP-identity cookie-import writes (#14729)

Reverts the revert 3c8410d927 (#14942) to restore the reviewed bf6dc6fcba
tree. The defect that caused that revert is NOT fixed by this commit — it is
fixed in the commits that follow, so the delta a reviewer must scrutinise
stays small instead of hiding inside a 2000-line re-add.

Conflict resolutions, all keep-both:
- browser-cookie-import.ts: two hunks around the zero-import early return.
  #14683's undecryptable warning and decryptedCookies.length === 0 condition
  are kept alongside the reland's old-client gate and partitionSkippedCookies.
  The old-client gate stays above the early return, and so above any mutation.
- en.json: both string sets (#14683's undecryptable copy and partitionSkipped).
- crashpad-capture.test.ts: took HEAD wholesale; unrelated to this ticket.

getStoragePath, and its positive-write assertion moves from cookies.set to the
CDP identity store — adapted, not weakened, and now strictly stronger because
it also asserts cookies.set carries no imported user data. Every decryption
assertion is untouched.

* fix(browser): add registrableFamily, one definition of a cookie family

STA-4300, change 1 of the reland fixes. No behaviour change yet — this only
adds the helper the later commits derive every skip scope from.

Deriving "family" inline in several places is what let the removal scope and
the write set disagree in STA-4090 and STA-4170, so there is exactly one.

The IP check runs on normalizeCookieDomain's OUTPUT, never the raw string.
psl treats an IPv4 literal as a dotted DNS name — psl.parse('127.0.0.1').domain
is '0.1' — and Chromium accepts many spellings of one address. new URL() inside
normalizeCookieDomain canonicalises 127.1 / 2130706433 / 0x7f.1 / 010.0.0.1 and
a trailing dot to a dotted quad first, so isIP() then catches all of them.
Mutation-proved: moving the check ahead of normalisation reddens the five
alternate-spelling cases and nothing else.

Bracketed IPv6 needs its own branch because isIP('[::1]') is 0; without it the
value falls through to psl, which throws, which returns the host — the right
answer for the wrong reason.

Returns null for a bare public suffix: naming `com` as a family would preserve
an entire TLD from removal and silently turn an import into a no-op.

* fix(browser): plan every cookie's fate before any jar mutation

STA-4300, change 2. Adds planImportWrites — pure, no I/O — so the write set and
every removal scope can derive from one value instead of being computed twice.
Not yet wired into the import paths; that is the next commits.

TWO passes, and the second is not optional. Family-atomic skip is a property of
the whole input: with a readable mixed.example row BEFORE an unreadable
sub.mixed.example row, a per-row guard emits the readable one before anything
knows the family will be skipped. Pass 1 classifies and collects the skipped
families; pass 2 re-filters the provisional writes.

Mutation-proved with a faithful one-pass guard (consult skippedFamilies as you
go): the readable-before-unreadable case goes red and the unreadable-before-
readable case still passes. That asymmetry is why both orders are tested and
why only the first is the named detector.

Family closure is at the registrable boundary because that is the boundary the
removal scope actually expands to: importedDomainScopes turns an imported
mixed.example into descendant roots, so a readable apex cookie drags a skipped
subdomain's live session into the removal scope (STA-4300 §2b).

hasUnrepresentableSkip surfaces a skip whose family cannot be named. A family we
cannot name is one we cannot exclude from the removal plan, and clearing a
family we cannot protect is the P0 this ticket exists to stop — so the caller
refuses before mutating rather than proceeding and hoping.

* fix(browser): derive path A's removal scope from the write plan (STA-4300 §2b)

Change 3. importValidatedCookies now plans before it opens the jar, and the
replacement scope and the write set are the SAME array.

bf6dc6fcba filtered replacementDomains per exact cookie
(partition.status !== 'unreadable'). That is not sufficient:
replaceCookiesForImportedDomains expands each imported domain into its
descendant roots, so a readable cookie on mixed.example pulls sub.mixed.example
into the removal scope — and a skipped cookie living there has its session
removed with nothing written back. Same erasure as the P0, different path.

plan.writes is family-closed, and because path A passes the very same array to
replaceCookiesForImportedDomains and to writeImportedCookies, the two sets
cannot drift apart. That is stronger than keeping them in sync: there is only
one set.

Also here, both before any mutation:
- an unrepresentable skip (registrableFamily → null) refuses the import, because
  a family we cannot name is one we cannot exclude from the removal scope;
- the old-client gate now keys off plan.skips rather than re-deriving the
  condition.

Counters: partitionSkippedCookies is a BREAKDOWN of skippedCookies — the
unreadable rows plus their family-suppressed siblings — added in exactly once,
so totalCookies === importedCookies + skippedCookies keeps holding. Counting it
separately is how a summary silently stops adding up.

Full src/main/browser suite: 69 files, 729 tests, green.

* fix(browser): split path B into scan and emit, and never stage a skip (STA-4300)

Change 4 — this is the commit that fixes the shipped P0.

bf6dc6fcba did:
    decryptedCookies.push({ ..., partition })   // write set built HERE
    if (partition.status === 'unreadable') { ... continue }   // guard AFTER

so an unreadable row discovered late could not retract a sibling already
emitted, while removeTransplantableCookies cleared the ENTIRE jar. The mutation
set was the whole jar and the write set a strict subset of it, which is how a
mixed readable/unreadable source emptied a populated jar and repopulated only
part of it.

Now the row loop SCANS only. Nothing is emitted inside it — not decryptedCookies,
not domainSet, not a staging row, not the imported count. Between scan and emit,
planImportWrites closes the skip over whole registrable families, and emit walks
the plan. Each scanned candidate carries its raw source row because
buildChromiumCookieInsertParams needs it; a record holding only the derived
fields compiles and then silently cannot stage.

Also between scan and emit, both before any mutation:
- an unrepresentable skip refuses the import;
- any skipped family calls disableStaging. A staged image is a whole-database
  replacement on the next start, so it cannot express "preserve this family";
  main's existing memoryFailed arm then reports restart-fallback-unavailable.

Test contract change, deliberately stronger: "never stages a partitioned row
whose ancestor bit is unreadable" asserted that an image WAS registered and
merely omitted the row. That image would still have erased the preserved family
on replay. It now asserts no image is registered at all — plus a new paired case
proving a no-skip import still stages, so "disable on skip" cannot be
implemented as "disable always" unnoticed.

Honest note on detection: reverting the emit filter currently reddens only the
staging assertion, because every end-to-end fixture in this module still starts
with an EMPTY jar — where clear-then-write-all and clear-then-write-some are
indistinguishable. That is the blind spot that let this ship. The populated-jar
suite in the next commit is what actually detects the erasure.

Full src/main/browser: 69 files, 730 tests, green.

* fix(browser): never remove a family the import declined to write (STA-4300 I2)

Change 5, and the one that makes preservation observable.

removeTransplantableCookies takes preserveFamilies. removableCookieEntries
filters those families out, which keeps them out of the removal plan AND — since
the CDP snapshot is taken from that same list — out of the restore set, so a
preserved coordinate is never submitted to any mutation at all.

When anything is preserved, the bulk clearData path is not used. clearData
removes everything outside excludeOrigins, and its own comment concedes a
rejection may already have emptied part of the jar; a preserved family is
deliberately absent from the snapshot, so a partial delete followed by a
rejection would destroy it with no identity able to restore it. Handing a
dynamically derived preserve list to that primitive cannot be made safe, so a
skip-bearing clear runs the frozen per-coordinate plan — which already excludes
those families — as its primary path instead. Nothing is preserved on an
ordinary import, so that path keeps today's single clearData call unchanged.

New populated-jar suite. Every end-to-end fixture in this module starts with
cookies.get returning [], and against an empty jar "clear then write all" and
"clear then write some" are indistinguishable — which is why four independent
gates missed the P0. These fixtures start populated.

Mutation-proved, and this is the first detector in the change set that catches
the actual erasure rather than a proxy:
- drop the preserved-family filter          → 6 of 8 red
- use bulk clearData while preserving       → 6 of 8 red, including
  "leaves a preserved family untouched", which is the erasure itself

IPv4-literal and single-label host families are covered explicitly: psl reads
127.0.0.1 as the dotted DNS name '0.1', so a wrong family here would fail to
match the preserve set and erase the live loopback session.

Full src/main/browser: 70 files, 738 tests, green.

* test(browser): pin STA-4300 end to end in real Electron, both write paths

Ports the pre-implementation reproduction into the PR. Verified RED against
main's product code and GREEN with the fix, on both paths — I re-ran it both
ways rather than assuming.

Two assertions are INVERTED rather than deleted, and both are now stronger. The
repro proved the defect by showing cookies.set was called WITHOUT a partitionKey;
after the reland the import write path does not touch cookies.set for user data
at all, so seeing the imported cookie there is itself the regression. The
fixture's internal guard is inverted the same way.

Non-vacuity, asserted in-run rather than in a report: ok:true with a nonzero
importedCookies, the importer reaching its terminal step, the source really
carrying its partition fields, and a CONTROL cookie written via CDP *with* a
partitionKey and read back with it — so "partitionKey absent" cannot be confused
with "the oracle cannot see partitions". electronVersion proves Electron ran.

Fixture correction worth recording, because the original reproduction was
invalid on this path and I did not catch it on first read: readJsonCookiePartition
reads a NESTED partitionKey object, but the fixture wrote topLevelSite and
hasCrossSiteAncestor at the entry's top level, where the importer never looks.
The file/paste case therefore failed on main for a fixture-shape reason rather
than for the defect — and its own non-vacuity check validated what the fixture
wrote instead of what the parser reads, which is exactly how a check that looks
rigorous proves nothing. Only the native half was ever a real reproduction.

Partition keys use a schemeful SITE, not an origin: Chromium canonicalises
https://app.example.com to https://example.com, measured via CDP.

* fix(browser): fail closed on lossy cookie identity recovery

* fix(browser): preserve unreadable families before decrypt

* fix(browser): reject malformed cookie partition sites

* fix(browser): validate recovered cookie partition sites

* fix(browser): reject empty JSON partition identities

* fix(browser): fail closed and serialize cookie imports

* fix(browser): reject malformed CDP opaque flags

* fix(browser): roll back outcome-unknown cookie writes

* revert(browser): move import serialization out to STA-4601

Reverts only the concurrency-serialization half of f7c27b71ab so #15030 stays
scoped to the STA-4300 reland. That commit mixed two concerns; this removes one
of them and keeps the other.

REMOVED (moves to STA-4601, a separate pre-existing P1):
- acquireCookieMutationLock / withCookieMutationLock and the mutationLocks map;
  clear.ts goes back to withCookieClearLock
- the import-wide lock in path A (mutationOwner on the target, acquire/release
  around replace + writes + rollback)
- the widened lock around path B's clear + memory writes
- browser-cookie-import-concurrency.test.ts

KEPT (genuinely STA-4300 scope, not concurrency):
- partitionKeyOpaque handling in readJsonCookiePartition and the CDP snapshot.
  An opaque partition key cannot be represented faithfully, so it reads as
  unreadable and its family is preserved — exactly the rule this ticket adds.
  194e57a628's refinement of that shape stays too.

The concurrent-import interleaving is real and reachable (nothing serialises
imports per partition, and the clear lock is released before the memory writes),
but it predates this change and is not caused or worsened by it. It gets its own
PR and its own review rather than riding a P0 reland whose history is that every
additional fix introduced a new defect.

src/main/browser: 71 files, 772 tests, green. Electron oracle still passes on
both write paths and still goes red against main's product code.
2026-08-17 12:44:39 -07:00
Brennan Benson 60805f5c45 fix(agent-status): preserve restored child provenance (#15082)
* fix(agent-status): preserve restored child provenance

* fix(agent-status): preserve restored completion context

* fix(agent-status): retain child boundary across OSC
2026-08-17 11:10:38 -07:00
Jinwoo Hong 646e9b692b fix(mobile): render pairing QR at scanner-safe scale (#15058) 2026-08-17 10:29:18 -07:00
Jinwoo Hong be07b43a2b fix(orchestration): enforce honest recipient routing (#14964) 2026-08-17 01:50:17 -07:00
Jinwoo Hong 7c798907c5 fix(skills): harden cross-host bundle installs (#15000) 2026-08-17 00:55:04 -07:00
Jinwoo Hong 30b76e6edb fix(orchestration): enforce task dispatch state invariant (#14961) 2026-08-17 00:52:10 -07:00
Brennan Benson 7afce2ea41 fix(ssh): stop reporting a confirmed kill when the SSH provider is gone (#14977)
* fix(ssh): stop reporting a confirmed kill when the SSH provider is gone

A detached relay PTY is designed to outlive the provider that addressed it
(it ignores SIGHUP and ships with an unlimited grace), so "the SSH provider
is no longer registered" is lost contact, never evidence the remote process
stopped. Both stop primitives in the PTY controller returned `true` from
that branch, and every caller downstream reported the fabricated success:
the CLI printed "PTY killed.", worker-stop settled the dispatch as stopped,
and — because the stop "succeeded" — the unstopped-PTY gate never ran, so
worktree removal walked straight past a live remote agent.

`kill`/`stopAndWait` now still tombstone the local lease but report an
unconfirmed stop and record why, using the three-verdict vocabulary the
worktree teardown gate already spoke (`live` / `unverifiable` / `exited`),
promoted out of that module into `src/shared/pty-liveness-verdict.ts`.
The close receipt, the CLI wording, worker-stop and the removal gate all
read that verdict instead of inferring an exit from silence.

The same rule fixes the mirror-image defect: the aggregate inventory only
enumerates registered providers, so a dropped relay clears `connected` for
every remote PTY at once. The sweep now separates the provider answering
"absent" (an exit) from no provider being able to answer (lost contact), so
worker-stop stops claiming `exited` from a disconnect.

The `connected` wire field is unchanged in meaning and shape.

* fix(orchestration): apply the same honesty to the federation stop path

The federation host runs its own copy of the worker observation and stop
logic, with the same two defects: `inspectRemoteAttachment` read a dropped
relay's `connected: false` as `exited`, and `federationStop` settled the
dispatch as stopped from a close it never confirmed — relaying a fabricated
success all the way home to the coordinator.

Two guards also had to move so the honest verdict does not become a new
refusal. `federationRead` gated on `status !== 'running'`, which would have
rejected a connected terminal the moment a stop lost contact with it; it now
gates on `status === 'exited'`, which is equivalent for every pre-existing
status given the two guards beside it. Local `workerStop` likewise still
attempts the close when the verdict is `unverifiable` — losing contact is a
reason to report the outcome honestly, never a reason to stop trying.

The show observations now carry the reason alongside the status, so a bare
`unverifiable` is actionable. Both are new optional fields.

* fix(ssh): preserve unconfirmed stop verdicts across consumers

* fix(ssh): use canonical live verdict wording

* fix(ssh): refuse wrong-host teardown verification

* test(orchestration): confirm worker release teardown

* fix(orchestration): negotiate honest worker stop receipts

* fix(agent-teams): fence uncertain teammate respawns

* fix(ssh): avoid duplicate missing-provider teardown

* fix(orchestration): preserve archives across release retries

* fix(ssh): preserve verdicts across synthetic kill exits

* fix(ssh): preserve liveness evidence across teardown

* fix(agent-teams): replace panes only after confirmed stop

* fix(ssh): distinguish host exits from relay loss

* fix(ssh): narrow concurrent inventory verdicts

* fix(orchestration): serve archives after uncertain release

* fix(orchestration): expose unverifiable read liveness

* test(ssh): align liveness assertions with verdicts

* fix(ssh): preserve host scope across inventory failures
2026-08-17 00:11:19 -07:00
Neil 3bb87ff93b reland(shell): one portable Unix startup dialect, with both revert causes fixed (#15018)
* reland: portable startup-shell dialect, with the two revert causes fixed

Relands #14863 (reverted by #14975) with fixes for both regressions the
revert cited.

1. History GC deleted folder-workspace shell history. The live set was built
   from `getAllWorktreeMeta()` alone, but a folder workspace's PTY carries
   `folder:<id>` as its worktree id, so every live folder workspace looked
   orphaned. `getKnownWorktreeIdsForHistoryGc` now unions in
   `getFolderWorkspaces()`. Both consumers — the history-directory prune and
   the fish-history sweep — read that one set, so the fix covers bash, zsh and
   fish history alike. The directory prune had this gap since #1524; #14863
   only widened its blast radius to fish files.

2. A copied Codex resume command aborted under `set -u`. Its leading clear
   statement has to test `$fish_pid`, and that unbound expansion takes the
   whole line — including the agent launch — down with it. Copied text runs in
   a shell Orca never spawned, so nothing can seed that variable first. The
   removal now rides on the agent itself as `env -u`, which needs no shell
   syntax and no expansion. Verified byte-identical under `set -u` in sh,
   bash, zsh, dash, ksh and fish.

   `env` cannot run the `cd` builtin, and a child `cd` would not move the
   agent, so the prefix is placed on the agent rather than on the whole
   `cd … && agent` chain. cmd and PowerShell have no nounset hazard and keep
   their clear ahead of the `cd`, which preserves `cd … && agent` — a failed
   `cd` still cannot launch the agent in the wrong directory.

* fix(history-gc): stop three more paths from deleting live shell history

Found by adversarial review of the reland. All three are the same class as
the bug that caused the revert: a live set that is missing a category of
real workspace, so the GC reads it as orphaned.

1. Profiles. The history root is `userData/terminal-history`, which has no
   profile segment, but the Store the GC consults is per-profile. So after a
   profile switch the live set condemned every other profile's history — and
   fish history, which lands in the user's own fish data dir, is shared by
   every profile on the machine. The live set now unions in the inactive
   profiles' worktrees and folder workspaces, read from their data files. A
   profile whose ids cannot be read reports the empty set rather than one
   that condemns real history.

2. No empty-set guard on the tree scan. `sweepOrphanedFishHistoryFiles`
   refuses an empty live set because it cannot be told apart from a store
   that failed to hydrate; the directory scan, which deletes more, had no
   such guard. A store that fell back to default state would have taken
   every worktree's bash and zsh history with it, across all roots including
   WSL. Four existing tests passed `new Set()` and relied on "empty means
   everything is orphaned" — exactly the behavior being removed — so they
   now pass a real live set.

3. Relay fish history. The relay isolates its history tree under its own
   root but wrote fish history into the shared fish data dir under the
   desktop naming, keyed by the CLIENT's worktree ids. On a machine running
   both Orca and a relay host, the desktop sweep deleted remote sessions'
   history once it went stale. Relay files are now `orca_relay_<hash>`,
   which the sweep's pattern deliberately does not match; the relay still
   deletes them by exact name when the worktree goes away.

* fix(resume): enforce the env-removal invariants instead of documenting them

Both found by adversarial review; both were unreachable from today's callers
and silent if reached, which is exactly how they would survive to a caller
that does reach them.

- A pinned CODEX_HOME and the removal named the same variable, and `env -u`
  strips what the assignment just set — so the agent would have resumed
  against the real home and not found the session. The removal list now
  excludes any name the prefix pins, keeping the assignment authoritative as
  the old `clear…; CODEX_HOME=x agent` ordering did. Same fix in the git-bash
  twin. The PowerShell branch already clears before it assigns, so it was
  never affected.

- Placement was keyed on the platform while the grammar it selects is keyed
  on the shell, so `platform: 'linux'` with `shell: 'powershell'` emitted
  POSIX `env -u` into a PowerShell line. PowerShell now routes to the
  PowerShell builder whatever the host, and the POSIX/cmd split below asks
  the shell rather than the platform.
2026-08-16 23:51:53 -07:00
Jinwoo Hong 77ef6bb9ee fix(terminal): verify agent prompt submission (#14962) 2026-08-16 23:34:18 -07:00
Jinjing 81d7f9b24e refactor: split db.ts under 400 lines (#14979)
* refactor: split db.ts under 400 lines

* rm plan

* fix(orchestration-db): add safety guards to database operations

Add status guards to UPDATE statements to prevent late operations from
overwriting changes made by concurrent requests. Validate mutation results
to surface silent no-ops. Extract circuit-break threshold, add transaction
wrapping, and sanitize untrusted input. Bump schema version to v28.

* Add transactional safety to dispatch and message operations

Wrap dispatch failures and batched message updates with SAVEPOINTs to
ensure atomicity and idempotency:
- Dispatch failures now check status guards and roll back if the
  related task update fails, preventing partial state corruption
- Message batches (across multiple 500-id chunks) roll back entirely
  if any batch fails, avoiding partial mutations
- Add tests verifying idempotency and atomicity under failure conditions

* Handle concurrent writes and improve transactional safety

- Remote question answering: add classification check before and after UPDATE to safely detect concurrent modifications. Prevents false success when the UPDATE loses a race.
- Transaction rollback: wrap in try-catch to prevent errors from masking the original failure.
- Question thread reset: use status update instead of deletion to preserve message references.

* test: add answer replay and race condition edge case coverage

Add test cases for answer replay idempotency, conflict detection, and a race condition between concurrent answer updates in federation relay. Also verify local question state transitions during orchestration reset.
2026-08-16 22:44:43 -07:00
Brennan Benson 8ca4ed945e feat(terminal): report execution host and listing scope in terminal list (#14973)
* feat(terminal): report execution host and listing scope in terminal list

`orca terminal list` returned rows with no host identity and no statement
of what the listing covered, so a scoped listing that saw nothing read as
"nothing exists anywhere" — an agent reported a live remote worker dead.

Each row now carries an optional `executionHostId` derived from the PTY id
(SSH and paired-runtime ids embed their owner), and the result carries an
optional `hostScope` naming the hosts covered and the known hosts skipped.
Both are surfaced in `--json` and in the human-readable CLI output, where
an absent field renders as `unknown` rather than `local`.

Both row builders route through one resolver, so the rule lives in one place.

* fix(terminal): preserve unverifiable host scope

* fix(terminal): fail closed on unverifiable hosts

* test(terminal): name unverifiable scope explicitly

* perf(terminal): keep graph hydration host scans narrow

* fix(terminal): reject blank foreign host owners

* fix(terminal): validate inferred inventory hosts

* fix(terminal): preserve paired folder host scope

* fix(terminal): keep inventory host inference typed

* fix(terminal): disclose paired folder hosts
2026-08-16 22:13:03 -07:00
Brennan Benson 5e9e38fa75 fix(agent-status): announce Claude turn complete while background work runs (#14580)
* fix(agent-status): announce Claude turn complete while background work runs

Lead Stop/StopFailure already ends the turn, but resolveClaudePaneState keeps
the pane working for subagents, background shells, and session crons. That
erases the working→done edge that mints the completion banner: subagent turns
notify late with a stale body, and shells/crons never notify at all.

Stamp turnCompletedAt on the gated lead Stop, announce immediately from that
row, and pair the later all-clear done to the same end time so it cannot
double-fire or collapse consecutive turns onto the pinned stateStartedAt.

Fixes #13245

* fix(agent-status): suppress stamped turn replays

* fix(agent-status): notify paired clients at turn end

* Fix late-paired completion notification arming

* test(agent-status): make the notification-id test fail on the pre-fix ordering

The stored row inherited the helper's default codex agentType while the event
named claude, so agentSnapshotMatchesExplicitTitle dropped it and
freshStoredAgentStatus was undefined — the assertion held under either side of
the `??`. Name the stored row's agent so the pinned working row survives and
the snapshot-first precedence is what the test actually pins.

* Suppress stamped completion tail replays

* Prevent cross-coordinator title replays

* Preserve stamped tails across fallback signals

* Keep remount replay state while sibling lives

* Scope OSC turn stamp preservation

* Forward paired host completion stamps

* Deduplicate paired completion tails

* Bind paired completion tails to their turn

* Preserve paired completion tail ownership

* Keep paired tail replay state across remounts

* Seed paired recovery without replaying completions

* Seed startup replay and release stale fallback dedupe

* fix(notifications): preserve stamped OSC repaints

* fix(notifications): retain paired client turn boundary

* fix notifications module import safety

* chore: keep main integration focused

* fix: preserve completion re-enable boundary
2026-08-16 20:47:02 -07:00
Jinjing f070033156 Revert "refactor(shell): one portable Unix startup dialect instead of shell d…" (#14975)
This reverts commit b6ea3f17a9.
2026-08-16 16:48:55 -07:00
Neil b6ea3f17a9 refactor(shell): one portable Unix startup dialect instead of shell detection (#14863)
Orca had to guess which shell would parse a queued command line, then emit
syntax for it. Guessing is unreliable for a remote or WSL host, and every
dialect-dependent function is a place to get it wrong.

Replace the guess. Everything emitted for a Unix shell is now built to be
correct in sh, bash, zsh, dash, ksh and fish alike, so no detection is needed:

- quoteStartupArg emits backslashes as "\\" and apostrophes as "'" between
  single-quoted runs. Both families read that identically, unlike the sh '\''
  idiom, which fish silently halves and which makes a trailing backslash a
  hard syntax error.
- clearEnvCommand emits a self-contained fish/sh branch. It deliberately does
  NOT call a helper defined by Orca's shell wrappers: Orca wraps only zsh, bash
  and fish, so an `sh`/`dash`/`ksh` login shell launches unwrapped — and the
  same text is copied to the clipboard and pasted into shells Orca never
  spawned. In both, a helper would be `command not found`, which is the exact
  failure this exists to avoid. Two guarded statements rather than `A && B ||
  C`, because fish's `set -e` returns non-zero for an already-unset variable
  and would fall through to the sh branch; a trailing `true` pins the status,
  since this is the last statement of a launch line and the prompt renders it.
- One tokenizer for Unix. The input is a settings string the shell never
  parses, so parsing it per-shell only made the same setting mean different
  things in different workspaces.

AgentStartupShell loses its 'fish' and 'unix' members, and the three
login-shell resolvers, the fish tokenizer and the agentEnv.SHELL probe go with
them.

Per-worktree shell history now actually works:

- zsh on macOS was a no-op. /etc/zshrc assigns HISTFILE unconditionally before
  any wrapper Orca controls, so the injected value was already gone — and with
  ZDOTDIR still pointing at Orca's wrapper dir, history landed inside it. The
  intended path rides ORCA_HISTFILE and is restored after user config.
  Fixes #11044.
- fish keeps history in its own data dir keyed by session name, since it
  ignores HISTFILE and has no custom-directory knob. Files are deleted rather
  than truncated, a symlinked ~/.local/share no longer disables cleanup, and a
  GC sweep reclaims orphans whose meta.json is gone. The sweep refuses an empty
  live-worktree set (indistinguishable from a store that failed to hydrate) and
  skips files younger than GC_MIN_AGE_MS, mirroring the tree GC's guard against
  the live-set snapshot race.

Verified against real shells rather than asserted as strings:
startup-shell-portability.live-shell.test.ts runs 194 assertions across
sh/bash/zsh/dash/ksh/fish, and zsh-scoped-histfile.live-shell.test.ts drives a
real login zsh through /etc/zshrc. Both are vacuity-checked. The same quoting
corpus was replayed byte-exact on Linux, where /bin/sh is dash.
2026-08-16 15:28:50 -07:00
Jinwoo Hong 9e3e583a83 feat(relay): prefer the closest available region (#14366) 2026-08-16 13:51:04 -07:00
Jinwoo Hong fa9b20cb41 feat(skills): reland private bundle sharing safely (#14934) 2026-08-16 13:45:54 -07:00
Jinjing 3c8410d927 Revert "fix(browser): route every cookie-import write through CDP identities …" (#14942)
This reverts commit bf6dc6fcba.
2026-08-16 12:33:45 -07:00
Jinjing ef1224c4f7 Revert "Preserve OpenCode session across command completion, control SessionS…" (#14943)
This reverts commit 1da1bdc01c.
2026-08-16 12:33:28 -07:00
Brennan Benson bf6dc6fcba fix(browser): route every cookie-import write through CDP identities (STA-4300) (#14729)
* test(browser): repro STA-4300 CHIPS partition downgrade on native cookie import success path

* fix(browser): route every cookie-import write through CDP identities (STA-4300)

* test(browser): cover partition fidelity on both import write paths (STA-4300)

* fix(browser): never stage unreadable cookie partitions (STA-4300)

* fix(browser): gate partition skips on client support (STA-4300)

* test(browser): anchor the client partition-skip capability assertion (STA-4300)

* fix(browser): skip Firefox CHIPS cookies whose Chromium identity cannot be rebuilt (STA-4300)

* fix(browser): tolerate a Firefox schema without originAttributes (STA-4300)

* docs(browser): correct a comment that still named the removed cookies.set write

* fix(browser): block lossy Firefox cookie import on old clients

* fix(browser): read Firefox CHIPS from schema flag
2026-08-16 11:46:45 -07:00
Jinjing 763b1febeb Revert "feat(skills): add private bundle sharing (#14401)" (#14913)
This reverts commit 757fae28d7.
2026-08-16 10:39:57 -07:00
Jinjing 1da1bdc01c Preserve OpenCode session across command completion, control SessionStart emission (#14866)
* Preserve OpenCode session across command completion

- Add session start events and launch token tracking to establish session boundaries
- Defer retiring launch authority until OpenCode process actually exits, not just when a command finishes
- Fence previous tokens after restarts to prevent status updates from stale sessions
- Maps SessionStart as a session boundary for proper turn/state management

* Emit SessionStart only from OpenCode, not mimo-code

Restrict SessionStart lifecycle events to OpenCode exclusively. Mimo-code no longer emits SessionStart, as it should rely on OpenCode for session boundary signals. This prevents duplicate lifecycle events that could interfere with pane authority tracking and session state management. Also tighten foreground process result validation to reject stale results after title observation changes, fixing a race where a delayed foreground read from a previous cycle would incorrectly retire authority.
2026-08-16 10:01:27 -07:00
Jinwoo HongandE2E Test 757fae28d7 feat(skills): add private bundle sharing (#14401)
Co-authored-by: E2E Test <e2e@test.local>
2026-08-16 02:36:18 -07:00
Jinjing 8b04e060fa refactor(persistence): extract modules to half persistence.ts (#14252)
* refactor(persistence): extract modules to half persistence.ts

* refactor(persistence): tighten the extracted operations seam

Review follow-ups on the module extraction, all behavior-neutral.

The extracted operations read and mutate the Store's state object in place, but
every seam typed it as a bare PersistedState, so nothing at the boundary said a
caller must pass the live reference — a future caller handing over a clone would
have its writes silently dropped. Name that contract: StoreOwnedPersistedState
carries it to every operations interface and every mutating free function.
normalizePersistedPaneIdentityState and backfillFolderScopeConnectionIds stay on
PersistedState; they build a fresh state rather than mutating the Store's.

The six *PersistenceOperations wrappers were constructed per delegate call. They
are stateless today, so this was inert, but any future instance state would be
lost between calls. Memoize them, and mark state and gitUsernameCache readonly
so the compiler enforces the single-assignment invariant memoizing them relies
on.

Also: restore flushSshPtyConsumerRecovery, whose inlining left its rationale
duplicated at both call sites; document that migrateWorktreeIdentity's boolean
gates the caller's save, since the extracted function kept no docs of its own;
and merge a duplicate shared/types import that was failing lint under
--deny-warnings.

* delete plan doc

* refactor(persistence): add error recovery and improve field cleanup

- Rollback failed migrations to prevent corrupted state that blocks retry
- Gracefully skip malformed entries in normalization instead of aborting
- Strip retired fields to prevent orphaned state and sync issues

* refactor(persistence): drop the redundant persistence- filename prefix

The extracted modules already live in src/main/persistence/, so name
them after the domain they own. Point leftover shared/types imports
at the real type modules while touching those files.

* refactor(persistence): optimize lookups and fix unsanitized updates

- Use Maps instead of repeated array searches for O(1) lookups
- Apply sanitized updates instead of raw input in ui-state-update
- Compare fields directly rather than JSON strings to avoid false dirty states from persisted key ordering differences

* refactor(persistence): group modules into lifecycle folders

Move the 42 flat persistence modules into six folders named for what the
module does, and lift the Store class out of the barrel so persistence.ts
becomes an 8-line public surface.

Bodies are unchanged: every moved file diffs clean against HEAD once import
blocks are excluded. Only import specifiers were rewritten, by resolving each
one to an absolute path and mapping it through the move map.

Store keeps its existing max-lines suppression; its baseline entry is repathed
rather than re-added. Its 119-method public API sets a ~525-line floor, so it
cannot meet the 400-line cap without breaking the API for 153 importers.

* Sanitize worktree visibility sources and preferences on hydration

Ensure invalid or corrupted data from disk (untracked whitespace,
relative paths, bogus preference values) is cleaned during load
rather than corrupting the in-memory store.
2026-08-15 21:41:01 -07:00
Neil 73aa5d0ca7 refactor(daemon,runtime): split daemon, pty and rpc modules under the max-lines budget (#14834)
Splits the nine oversized modules in the daemon/provider/runtime domain into
focused per-concern files and drops their max-lines baseline entries.

- daemon: `Session` decomposes into an output plane (emulator, pending-output
  buffer, client fan-out), a producer-pause controller, a shell-ready barrier and
  a termination controller; `DaemonClient` into socket connect, hello handshake,
  ndjson readers, pending-request settlement, listener registry and notify
  settlement; `daemon-health` into pid-file parsing, process identity,
  stale-kill, TCC attribution and bundle staleness; `shell-ready` into the marker
  constant and the bash/zsh rcfile generators.
- providers: local-pty shell-ready wrapper generation, wrapper root, startup
  command and bash rcfile split out of local-pty-shell-ready.
- runtime: `Coordinator` sheds DAG convergence, decision gates, escalation
  triage, the runtime contract, the stale-base flag and task dispatch; the
  files/git/github rpc modules split into per-domain method groups.

Behavior-preserving: the extracted units keep their original construction order,
guards and timer lifetimes, and every RPC method name is still registered.
Test `vi.mock` surfaces were re-partitioned to follow the moved symbols.
2026-08-15 19:36:03 -07:00
Neil bc28107864 refactor(hooks,relay): split agent hook services and relay under the max-lines budget (#14725)
The four agent hook services, the main hooks module, and the two relay modules
each carried a file-level `eslint-disable max-lines` and ran 365-628 counted
lines against a 300-line budget. AGENTS.md calls for splitting rather than
suppressing, and config/max-lines-baseline.txt is a shrink-only ratchet, so this
removes all seven suppressions and prunes their entries (341 -> 334).

Pure move, no behavior change. Each hook service splits into its managed script
source, its config/bundle serialization, and its remote-install path, keeping the
per-agent integrations independent: copilot, amp, antigravity and hermes each
retain their own getManagedScript rather than sharing one, because each emits a
different script body for a different agent. Merging them by name would have
been a behavior change, not a refactor.

For antigravity the suppression's stated rationale -- that local install, Windows
wrapper generation, status cleanup, and SSH remote install must share one event
list and managed-command matcher so stale-hook cleanup cannot drift by platform
-- is now enforced structurally instead: both install paths call
buildInstalledConfig + createAntigravityManagedCommandMatcher over the single
ANTIGRAVITY_EVENTS catalog, with the graph a strict DAG.

Also registers the six new antigravity/ and copilot/ modules in
config/tsconfig.cli.json. That project uses a curated `include` list rather than
a glob, so an unlisted module fails `tsc -p config/tsconfig.tc.cli.json` with
TS6307 even though the entire unit suite passes.

Verified: oxlint clean, ratchet passes, typecheck clean, full unit suite green
(remaining failures are pre-existing load flakes in untouched files, green when
re-run serially), no new runtime import cycles, and no lint suppression added.
2026-08-15 18:25:37 -07:00