* fix(session): keep a renamed worktree's rows from matching on the id it lost
Three persisted session fields survived a worktree re-key still naming the old
identity. Two of them are suppression records, so a stale id does not read as
residue -- it silently re-admits state the user removed:
- closedTerminalTabTombstonesByTabId: the remote merge only suppresses a host
tab when the tombstone's worktree equals the tab's, and no snapshot ever
covers the old id, so the tombstone never retires either.
- clientHostedBrowserCloseIntentsByEnvironment: the replay targets the intent's
worktree, and an unresolvable selector answers selector_not_found -- which the
replay reads as definitively gone and uses to DROP the intent.
- clientHostedBrowserPagesByWorktree: keyed by worktree and re-checked against
the row's own workspaceId, so both halves have to move or the pages are never
rehydrated.
Fixed on both sides of the rename: the main-process persisted migration and the
renderer's live store, which would otherwise write the stale values straight
back. The coverage test drives off WORKSPACE_SESSION_WORKTREE_REFERENCE_KIND,
the census these three fell out of, with the shipping owner collector as its
oracle.
* docs(session): record why a re-key clobbering an existing target stays unfixed
Not a missing guard -- an unresolvable one. Keeping the target is correct when
it holds a real closed-last-terminal tombstone; keeping the source is correct
when the target row is a stub; nothing records which is newer. The recency map
is the only one that can settle it, because Math.max needs no such ordering.
* test(persistence): measure the downgrade direction for worktree identity
The stack widens migrateWorktreeIdentity to repoint worktreeId inside
session rows. That changes what lands on disk with no wire change, which
is Rule 3's shape applied to persistence, so it is measured against
v1.4.199 rather than reasoned about.
Result: new-build state does not break the old build. The old build
renames over it without throwing and loses no row; the two row kinds it
cannot repoint stay stale, which is exactly what its own renames already
produce.
The numbers are measured. A first draft asserted the old build repointed
no inner rows at all; it repoints two of four, and the probe is what
caught that.
* test(ci): run the worktree-identity downgrade lane instead of describing it
The cross-version job names its files explicitly, so a new one is inert until
it is listed; the sharded unit job excludes the whole directory and the E2E
router only takes `*.spec.ts`. Also pairs the forward-compat case against the
current build — the stack's own field-list walk is the guarantee that matters,
and only the frozen build was exercised.
* refactor(session): drop the type assertions the rename migration leaned on
`consistent-type-assertions` landed on main after this branch last built, and
three of the `as never` fixtures were hiding real contract drift: a browser
workspace row missing six required fields, a tab group naming three fields the
type does not have while omitting the two it requires, and a sleeping-agent row
whose `providerSession` had neither `key` nor `id` and whose `state` was not in
`AgentStatusState`.
Indexing the session by a computed field name is what forced the casts in the
migration, so the four row maps are now spelled out; the census test is what
keeps a fifth from joining silently. The renderer test builds its state from
the real slice instead of casting a four-field partial.
* refactor(test): name the module namespace the skew harness reads
`object` is too broad for the anti-slop gate, and the import helper already
declares what it hands back.
* docs(test): say which maps the harness actually supplies
The two under test live in slices this harness does not mount, so calling it
"the real slice's state" overclaimed.
* fix(skills): keep the disposal verdict when staging cleanup fails
`begin()` ended with `await this.removeOwnershipIfDisposed()` inside its `finally`,
so when a caller raced `dispose()` the rejection it received was whatever that
opportunistic `rmdir` threw -- not `skill-upload-service-disposed`. A caller could
not tell "the service shut down" from "the filesystem broke", and the Windows
release gate saw it as `EPERM: operation not permitted, rmdir`.
Two causes, both fixed here:
- The EPERM itself: an in-flight operation and disposal each call
`ownership.remove()`, so two `rm -rf` run concurrently against the same owner
directory. On POSIX the loser reads ENOENT and `force: true` swallows it; on
Windows the loser reads a delete-pending directory and gets EPERM.
`SkillUploadStagingOwnership.remove()` now joins one removal and forgets it on
failure so a later caller still retries.
- The masking: cleanup in a `finally` no longer replaces the outcome of the call
it is cleaning up after. Disposal retries staging removal and reports its own
failure, matching `removeUnpublished`/`retainFailedCleanup` in this class.
Both regressions are pinned platform-independently: one injects a failing
ownership removal and asserts the racing `begin` still rejects with
`skill-upload-service-disposed` while `dispose()` reports the cleanup failure; the
other models Windows delete-pending rmdir in the `node:fs/promises` mock, which
turns a second removal into EPERM on every platform.
* ci(release-cut): retry the installs that fetch node-gyp headers
`golden e2e windows` installs with lifecycle scripts enabled, so pnpm runs
node-gyp for the `native/windows-registry` workspace project, which downloads that
Node version's headers from nodejs.org. A single `read ECONNRESET` on that fetch
failed a blocking release gate, and the release build job one screen below already
wraps its install in `nick-fields/retry@v4` for exactly this class of failure.
Both remaining unretried installs in this workflow (the blocking platform golden and
the non-blocking rendering-evidence lane) now use the same wrapper, and a contract
test keeps every release-cut install retryable.
* fix: avoid retaining foreign SSH file frames before metadata
* test(ssh): exercise empty metadata through the streaming mux fixture
* fix(ssh): fail the file read when beforeResolve never runs
Moving the metadata install from .then() to beforeResolve moved it from a
mandatory callback to an optional one, and handleResponse clears the request
timer before beforeResolve runs. That left "response fulfilled, metadata never
installed" with no deadline: the read never settled, holding its notification
and dispose closures until mux disposal. Before this PR the same state failed
after the 60s inactivity deadline.
Unreachable with the concrete mux, which calls resolve on the line after
beforeResolve, but the hook is optional in the type and nothing enforces the
pairing. The guard is a no-op on every real path: empty, missing streamId,
cap-exceeded and alloc-failure all settle first, and the success path sets
metadataReady.
Found during review of #21167; raised at
https://github.com/stablyai/orca/pull/21167#issuecomment-5726058832
---------
Co-authored-by: m4air <m4air@Mac.localdomain>
Co-authored-by: Claude <noreply@anthropic.com>
The host publishes only its own store's isDirty and never learns about
client edits, so rebuilding a mirrored OpenFile from the snapshot cleared
the client's flag while editorDrafts still held the draft. The tab strip
then closed the tab with no unsaved-changes prompt and closeFile deleted
the draft; the external-change reload guards would reload over it too.
Keep the client's flag when the client's file is dirty and it holds a
draft; with no draft the host's flag still wins so a host-side save does
not strand the tab as dirty. A host-side save never clears a client draft.
Fixes#21392
On a park-reveal of a remote-runtime pty the host snapshot probe is the only
structural paint (the reattach carries no relay tail). Every non-snapshot
answer collapsed to null with no retry, so a host that stayed silent past the
request timeout, or answered 'no-serializable-buffer' ("not proof the pane is
empty"), painted the same blank pane as a host with nothing. That reads
unverifiable as exited (docs/reference/ssh-execution-boundary.md).
Classify the probe three ways: a host image paints; permanently-unavailable /
unavailable paints nothing and asks nothing; everything that proves nothing
(timeout, host declined for now, local lane gate, imageless success) paints
nothing and hands off to the hidden-output restore loop, which already budgets
retry-worthy answers (7 host declines / 30 local gates / 5 re-arm cycles),
repaints from the host on success, and ends in the explicit loss banner. The
reveal's own probe is charged to that same budget, so the bound is shared, not
doubled. No structural clear is issued on the unverifiable path, so whatever
the layout replay painted from the client's own copy stays visible.
* fix(terminal): retire explicitly closed pending split connections
* test(memory): keep pending split proof compatible with formatted source
* fix(terminal): confirm pending split retirement before stopping work
* fix(terminal): restore the pending split-close gates CI checks
Three CI gates were red on this branch and all three were this branch's own.
The hook-order parity snapshot did not count the `confirmedCloseRef` this
branch adds to `use-terminal-pane-close-actions.ts`. Dumping the flattened
order against clean `main` shows exactly one added `useRef` at position 148
and no reordering, so the count moves 211 -> 212 and the digest with it.
`pending-split-close-test-fixture.ts` is Vitest support code, but it sits
outside the `*.test` / `*.spec` / `tests` globs that already switch
`anti-slop/no-module-mocking` off, so the gate failed on all twelve of its
`vi.mock` calls. It carries a file-scoped disable with the reason, matching
`work-item-search-test-harness.ts`.
`fix.patch` still described the pre-confirmation shape of the close hook, so
`reproduce.mjs` aborted with `Source changed` and the cited ablation could not
run at this head. Regenerated against the committed sources; the harness again
reports 10 pass / 14 fail before and 24 pass / 0 fail after.
Merges `main` rather than rebasing: #21005 is stacked on this branch.
---------
Co-authored-by: m4air <m4air@Mac.localdomain>
Co-authored-by: Neil <neil@stably.ai>
A structured turn that FAILED was recorded as `completed`, identically to one
that succeeded, so nothing downstream could tell them apart. Claude mapped only
its two abort reasons to `interrupted` and let an API error fall through to
`completed`; Codex collapsed every non-`completed` status to `interrupted` and
read a missing status as a clean finish.
Add `outcome` — success / failure / cancellation — to the turn record, emitted by
both providers. The four-arm lifecycle union is deliberately untouched: it stays
a report on what the HOST observed, and its readers are unaffected by
construction.
Absent means UNKNOWN and never success. Historical rows, older hosts, and any end
the host inferred rather than heard (the child going away, a turn superseded
before its result) all carry no outcome, so a newer client cannot mistake an old
host's `completed` API error for a clean turn.
Claude's abort-reason list had a second copy in the provider-fallback reader;
both now classify through one `claudeResultOutcome`, so the durable verdict and
the visible error row cannot drift.
* fix(editor): keep an unresolvable mirrored file tab open with a truthful terminal state
A host-mirrored file whose read keeps answering `selector_not_found` used
to sit on the raw code forever (and, in the reverted #21363, was closed
outright, discarding drafts). `selector_not_found` is the host's "could
not resolve right now", not proof the workspace is gone, and the file-read
path has no definitive absence code.
Bound the retries as before, then swap in a truthful terminal message
with Retry and Close tab. The tab is never closed automatically; Close
routes through the unsaved-changes queue so a dirty draft is confirmed.
Fixes#21041
* fix(editor): classify selector_not_found by RPC code, not message text
Preserve `RuntimeRpcCallError.code` on `FileContent.loadErrorCode` and gate
the host-unresolved terminal transition with `hasRuntimeRpcErrorCode`, so a
host that sends `{ code: 'selector_not_found', message: 'Selector not found' }`
reaches the same truthful state as one that puts the bare token on the
message. Also drop the Close action on inline conflict-review rows, which
are not open tabs and would have been a dead control.
* fix(editor): localize the host-unresolved copy by sentinel, and pin the token matcher
Separate the terminal state's comparison key from its display text: the
retry hook stores `WORKTREE_HOST_UNRESOLVED_CODE` on `loadErrorCode`, and
the error view localizes by that code (`editor.fileLoad.hostUnresolved`),
so translating the message can never break the terminal check. Export the
selector_not_found matcher and cover near misses (case, suffix, prose,
wrong code) so only the defined token classifies.
* test(editor): name the it.each parameter for the host answer it labels
* fix(editor): drop the load-error Close action; closing stays with the tab strip
The Close button routed through `requestEditorFileClose`, which skips the
pinned-tab and shared-reference checks the tab strip applies, has no
listener outside the Terminal workbench (floating editor panels), and on
the conflict-review overview could target an unrelated open tab whose id
is the same absolute path as a synthesized inline row. Rather than
reimplement the tab strip's close semantics in a second place, the error
view keeps Retry and its copy points the user at closing the tab.
* fix(editor): reword the host-unresolved copy and namespace its sentinel
The copy no longer points at a Close control that is gone ("close this
tab from the tab strip") and no longer claims a scan is in progress, since
`selector_not_found` is also thrown synchronously for unregistered folder
workspaces and removed repos. The sentinel becomes
`editor_host_workspace_unresolved` so it cannot be confused with the CLI's
`worktree_host_unresolved` client error. The doc comment narrows the
"no definitive absence code" claim to git worktrees and names the two host
codes that are definitive but not yet classified.
Refs #21041
canProvideAuthoritativeBufferSnapshot is contracted as "whether this exact PTY can
return a sequence-safe provider snapshot" (pty-provider-contract.ts), and two of the
three layers already route it per id: DaemonPtyRouter forwards to adapterFor(id), and
DegradedDaemonPtyProvider forwards to the provider that owns the session. The daemon
adapter was the leaf that discarded the id and returned supportsAuthoritativeBufferSnapshots
— a negotiated protocol version, which is a fact about the connection, not about a pty.
That is reachable, not theoretical. getProviderForPty falls back to the local provider
for any id it cannot place, so a remote-runtime id (whose pty lives on another machine)
resolves to the local daemon adapter, and pty:getAuthoritativeBufferSnapshotCapabilities
answered `true` for a session this daemon has never owned. The renderer caches that as a
definitive per-pty verdict, and because the leaf discarded the id it could not tell it had
been asked about something it does not own.
Today the wrong answer is masked: allowOrdinaryParkRestore short-circuits remote and SSH
ptys before the cached verdict is read, so nothing consults it. This closes the gap before
something relies on it — a caller reaching for a per-pty answer should not be handed a
confident one that is wrong.
Not touching that short-circuit. It is deliberate: SSH bytes transit the client's own main
process into its headless mirror, so those panes have a local copy the predicate says
nothing about, and the direct-SSH lane was confirmed to repaint from a daemon-backed
restore with the park capture disabled entirely. Routing SSH around a daemon-snapshot
predicate is correct, and removing the short-circuit would disable SSH parking for no
correctness gain.
The existing protocol-compatibility test asserted `true` for a made-up session id, which
encoded the bug. It now spawns a real session, so it still proves the protocol-version
gate without depending on an unowned id reading as supported.
* fix(accounts): free the account queue when a sign-in is abandoned
Closing Settings mid sign-in left the `codex login` / `claude auth login`
child running, and every account mutation shares one FIFO queue, so the
next Add Account sat behind it for the login's whole deadline and then
inherited the abandoned call's timeout toast.
Cancel the pending login before enqueueing the next add or reauth (never
inside the queue the abandoned login owns), give Codex the cancel handle
and Cancel button Claude already had, and stop reporting a cancellation
as a failure.
Also surface the sign-in link Codex prints, with copy and open, so the
flow can be finished in a private window or another browser profile.
* test(accounts): drop the bare casts CI's changed-code gate rejects
The service doubles still need a cast; one documented helper per file
carries the SAFETY rationale instead of nine bare `as never`s.
* fix(codex): a cancel must not discard a sign-in that already succeeded
The Windows post-auth watcher gives a lingering codex login five seconds
to exit after it writes auth.json. A cancel arriving in that window
rejected the login, and the caller's rollback then deleted the managed
home that had just authenticated.
Refuse the cancel once new credential bytes exist: there is nothing left
to cancel, and the close handler already treats that state as success.
Found by review of #21372.
* fix(codex): keep a refused cancel cancellable, and require the sign-in notice
Review of the auth-aware cancel guard found two holes it opened:
- The outer handle latched `cancelled` before asking the session, so a
refusal killed cancellation for the rest of the deadline. On a host
with no post-auth watcher that reinstated the very stall this PR
removes. Latch only when the cancel is accepted.
- WSL never reads a pre-spawn baseline, so the guard read the auth.json
that was already there and refused from the first click, making a WSL
reauthentication uncancellable. Require a baseline before refusing.
Also from review: publish the sign-in link from a stdout-only buffer, so
an interleaved stderr chunk cannot truncate it; require codex's own
"navigate to this URL" notice rather than offering the first link in the
output; hide the notice in a remote account scope, where it would name a
login running on this desktop; and share the cancellation message
instead of matching a duplicated literal.
The Claude case joins the login-process suite that already owns the two
neighbouring cancel cases, and the auth-snapshot helpers move out of the
session file, which the additions pushed over the line cap.
* refactor(codex): cut the sign-in-link plumbing to its smallest form
Review found the change correct but larger than it needs to be:
- The pending-link store was a class with one permanent subscriber, a
never-called unsubscribe and a try/catch that could not fire. It is a
field and a listener set on the service, beside the cancel handle it
already owned — and the service now clears both in one place.
- The optional login-session dependencies were always supplied.
- The parser's https check could not fail; the pattern already fixed the
scheme. The renderer's unmount guard inside a synchronous IPC listener
could not fire either.
- The broadcast channel and the cancellation message are single sources
of truth in src/shared now, rather than exported next to a hardcoded
copy of themselves.
- The duplicated seven-line rationale in both services says the same
thing in three, including why only add and reauthenticate supersede.
- The codex suite reuses its own factory, and unmocks once.
Also reverts four reformat hunks the formatter pulled in around edits.
* fix(accounts): free the queue for a switch, not only for another add
Switching or removing an account shares the mutation queue an abandoned
sign-in was holding, so the commonest thing a user does after giving up
— pick a different account — still spun for the whole deadline while Add
recovered instantly. Both now supersede, as does the Claude side.
Every caller is a person: the two IPC handlers and the mobile RPC
methods. No poll, sync or CLI path reaches them, and a sign-in that
already wrote credentials refuses the cancel, so a switch cannot discard
one that succeeded.
Also from review: the Cancel button regains the gap its Claude twin has
(layout is allowed by the design-system rule; only the colour override
was not), and the URL subscription says what it is — registration for
the process's lifetime, with no teardown to hand back.
* feat(mobile-web-bundle): advertise the bundle capability where one ships
status.get pushes mobileWeb.bundle.v1 only when the install's bundle resolves and
its manifest parses, beside the other conditional capabilities. Dev trees and
`orca serve` installs may carry no out/mobile-web, and a static entry there would
promise a download that only ever answers mobile_web_bundle_unavailable.
No protocol version bump: protocol-version.ts asks for one when a method or a
required field is removed or changes meaning, not when a capability is added.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): pin that mobileWeb.bundle.v1 is inert on a released client
Derives the old desktop's reply by removing the one capability from what the new
one sends, rather than writing down what the old client had, and asserts every
released read of status.get lands identically apart from that string: the gate
hook, the three transport readers, the quick-command predicate and the
worktree-create support probe.
Proved red against three mutants: a closed enum on the capability schema (the
salvaged field drops whole, so nothing publishes), a client-side filter over the
new name, and a gate that changes floatingWorkspaceEnabled when it sees it.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* chore(mobile): name the invariant behind the fake client's cast
The changed-code casting gate wants the rationale on the line, and the reason is
narrow enough to state: every reader under test reaches the client through an rpc
operation's `request`, which uses sendRequest alone.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* refactor(repo): share the create-from picker outside automations
Move CreateFromPicker and its test from components/automations to
components/repo, next to the repo-scoped shared UI that already lives
there (RepoCombobox, RepoBadgeLabel, repo-icon). The New Workspace
composer will consume this picker instead of growing a second base-ref
combobox.
Pure move: no behavior change. The translate() keys are call-site
literals, so no locale catalog is affected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(composer): separate the branch that names a workspace from its base
baseBranch carried two meanings at once. It is the ref a worktree is created
from, and it is also what buildWorkspaceSourceSelection turns into the name
field's branch pill whenever no work item is linked. Any second control that
set a base therefore took the name field over: the pill replaced the text
input, hiding whatever the user had typed. The name survived in state, and
Advanced still exposed it, but the main field silently stopped showing it.
Add baseBranchNamesWorkspace, true only when a branch was picked to name the
workspace. The pill reads that flag; creation keeps reading baseBranch. Two
call sites set it, because those are the only paths that make baseBranch
defined with nothing linked — and an undefined base yields no pill anyway.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat(composer): let the New Workspace composer pick its base ref
The name field's tabs pick how a workspace is named; the base ref is a
separate decision the composer never exposed. Naming a workspace from a
Jira, Linear, GitHub or GitLab issue therefore pinned the project's default
base with no way to start from a release or a long-lived feature branch.
Nothing below the UI was missing. baseBranch already crosses IPC next to
linkedWorkItem and wins over every default in main, and the composer already
computed handleBaseBranchChange and startFromResetHint — the card simply
never declared those props, so its {...props} spread dropped them. Declare
them and render the shared create-from picker under the name field.
ComposerBaseRefPicker owns its own store reads, the way the sibling
ComposerParentWorktreePicker already does, so the name section stays
presentational and nothing subscribes to the worktree list while the picker
is hidden.
The picker is offered for a plain typed name and for issue-shaped sources.
It is hidden where a base already exists: PR/MR sources pin the pull
request's own head, a branch pick IS the base — and offering one there would
silently turn a checkout of that branch into a new branch off something
else, since picking a base clears reuse — and folder workspaces have no
branches. It always opens on the project default: no sticky base.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* chore(repo): drop a stale react-doctor suppression on the create-from picker
no-adjust-state-on-prop-change no longer fires on this file: removing the
directive and running the react-doctor pass over the directory — where the JS
plugin actually loads — reports nothing, at the new path and at the old one on
main alike. The suppression was already dead; the rename only put the file in
the changed set, where the quality gate reports unused directives.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat(repo): list branches as soon as the create-from picker opens
The picker only searched once two characters were typed, so opening it showed
just the project default and whatever branches already had a worktree. The
composer's Branch tab lists on an empty query through the same runtime helper;
match it, and the picker offers the repo's branches straight away.
Search stays debounced at 200ms and capped at 30 results, and it still runs on
the repo's own execution host, so a remote repo lists its own branches. The
Automations picker shares this component and gains the same listing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(composer): carry the base-ref naming intent through a saved draft
`baseBranchNamesWorkspace` lived only in component state, so restoring a
persisted draft always reset it to true. A base ref chosen in the picker
came back as a name-field source pill, hiding the name the user had typed
— the exact regression the flag exists to prevent, reappearing across a
draft round trip.
Persist it next to `baseBranch` and restore it through
`resolveDraftBaseBranchNamesWorkspace`. A draft written before the flag
existed records no intent and restores as a branch pick, which is the
behavior it had when it was saved.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(composer): preserve independent base and branch name choices
* fix(composer): pass naming-intent through the create-more reset test
IssueSourceActions now requires baseBranchNamesWorkspace. The create-more reset fixture is a source-owned base, so the flag stays true and the next create still clears it.
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Neil <neil@stably.ai>
* fix(session): scope agent resume to the host that captured the session
A provider session id names a transcript in one machine's agent state
directory. Nothing in the resume path compared that machine against the
one the resume executes on, so a record captured on host A reached a
`--resume` run on host B, which answers `No conversation found with
session ID`.
Three things make the drift reachable: `worktreeId` is `repoId::path`
with no host component, sleeping records are `'sleepingAgentKeyed'` so
boot-time host-contention parking never arbitrates them and every
partition merges into one map without retaining provenance, and both
issuers resolve their launch target from the current catalog.
Both issuers are gated. The activation sweep hands `quit`/`live` records
whose pane still exists to the pane's own cold restore, so gating the
sweep alone changed nothing in the SSH lane.
Declines rather than guesses: the record is preserved and remains
resumable by hand. A refused resume is recoverable, a forked transcript
is not. The predicate fails open on anything it cannot positively rule
out -- an unstamped record, an empty stamp, or a `runtime:` host, which a
paired client uses to relabel its host's own SSH workspaces.
The cold-restore gate consults both the pane's transport and the
catalog. The transport alone was racy: it is unresolved on an early
reattach frame, and that frame is exactly when a wrong resume escaped.
* docs(session): name the inverted fail-open direction at the resume gate
* fix(session): keep an unresolved catalog out of the resume host verdict
The worktree form of the resume gate resolved the current host through
getExecutionHostIdForWorktree, which answers 'local' for a worktree the
catalog has no row for. Read as a host, that made every SSH-stamped record
look foreign until its repo row landed, contradicting the module's own
contract that it reports only a positively-known disagreement. Add
getKnownExecutionHostIdForWorktree, which returns null in that silence
(no repo row for a git worktree, no folder-workspace row for a folder
workspace), and route the gate through it; the pair form already fails
open on a null host. The routing resolver keeps its default unchanged.
The CI red on the control case was a separate spec race: the ledger wait
returned as soon as the ledger was non-empty, and it already held the
first launch's `--version` probe, so the control read two probes and gave
up before the cold-restore had typed `--resume` (the failure screenshot
shows the command running in the pane). The spec now reads only the lines
the relaunch appended, anchors on the relaunch's PTY binding and its own
probe, and then waits for `--resume` for the control case or a bounded
grace for the refusal case.
This reverts commit 07e8c851b8.
The eviction keys on `selector_not_found`, which this repo documents twice as
UNKNOWN rather than absence:
- `remote-browser-stream-errors.ts`: "it means 'I could not resolve this right
now', which is UNKNOWN, not proof the target is gone. Its producer is a live
worktree scan behind a 1s-TTL cache ... a slow scan can surface it
transiently. Treating that as permanent would strand the pane forever, which
is the exact bug this file exists to prevent."
- `web-runtime-session-tab-lifecycle.ts`, added by #21277: "'selector_not_found'
is a transient worktree resolver state (e.g. during scans or cache warm-up)
and must not become a durable close tombstone."
Two unambiguous absence codes exist for this purpose -- `tab_not_found` and
`terminal_tab_not_found` -- and #21277 had just finished excluding
`selector_not_found` from them. This keyed on the excluded one.
Consequences, after roughly 3.75s of retries:
1. `closeFile` deletes `editorDrafts[fileId]` with no dirty check and no
confirmation, so a transient resolver blip discards unsaved edits.
2. `closeFile` calls `notifyHostOfMirroredEditorClose`, so the host closes its
copy too -- the eviction is not local and not recoverable.
The `!ownerNotReady` guard does not cover this: `ownerNotReady` means the host is
still connecting, while `selector_not_found` is emitted for a cold resolver cache
or an unhydrated catalog, which is a different state.
#21041 is still open. A correct fix keys on the two definitive absence codes,
refuses to evict a tab that has a draft, and has a test proving a dirty mirrored
tab survives `selector_not_found`.
* feat(mobile-web-bundle): serve the bundle manifest and chunks over RPC
Two paired-runtime methods on the already-authenticated connection:
`mobileWeb.bundle.manifest` returns this install's manifest plus the chunk
size it advertises, and `mobileWeb.bundle.chunk` returns one aligned range of
one asset with the whole asset's length and hash, so a single chunk describes
what it belongs to.
`path` is accepted only by exact match against a manifest member, so traversal
is unreachable rather than mitigated. Each asset's on-disk sha256 is verified
once and the verdict remembered, concurrent first readers sharing one hash.
Reads are capped at four in flight per connection, and a disconnected client
stops costing reads at the next checkpoint.
No SSH or relay proxying: a runtime answers only out of its own install.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile-web-bundle): pin the three buildId serializers against each other
The canonical serialization exists in the builder, the packaging guard, and the
shared contract, because the two packaging scripts run on bare node before any
build output exists and cannot import TypeScript. A divergence in any one would
reject every honest bundle at packaging, or ship a bundle whose id the phone
recomputes differently and re-downloads forever. Proved red by swapping the
guard's code-unit sort for localeCompare: five of six cases fail.
Exports the guard's serializer for the test; no packaging behaviour changes.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile-web-bundle): cover every error code and a multi-chunk paging round trip
Against a synthetic bundle in a temp dir, because the real builder's largest
asset is under one chunk and CI unit jobs never build out/mobile-web. The
fixture's script spans three chunks, its stylesheet is exactly one, and one
asset is empty, so paging, the eof boundary, and the zero-byte case are
exercised rather than assumed.
Reads in flight are held by latching `open`, so the four-per-connection cap and
an abort arriving mid-read are deterministic rather than a race with a
stopwatch. Both were proved red: dropping the abort check after verification
fails the abort case, and keying the cap on connectionId alone fails the
device-token case.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* fix(mobile-web-bundle): resolve the bundle root through the AppEnvironment port
check:runtime-electron-ratchet caught this: the resolver sat beside
getBundledWebClientRoot in src/main/startup and imported electron, and importing
it from an RPC method pulled the first electron edge into a runtime graph whose
baseline is zero. The runtime has to stay bootable on plain Node.
So it reads app.getAppPath() through the port every other runtime module already
uses, and moves next to its two callers under src/main/runtime. A host with no
environment installed has no install root, which is the same answer as having no
bundle. orcad answers getAppPath from its own install root, so a headless
runtime that carries the artifact serves it with no special case.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile-web-bundle): cover the resolver's two probe layouts directly
Also stops exporting the manifest filename, which nothing outside the resolver
needs.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile-web-bundle): pin both methods on the mobile allowlist
The scanner only checks mobile-used ⊆ allowlist, and no mobile source calls these
until A5, so deleting both entries left every test green.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* fix(mobile-web-bundle): keep filesystem failures inside the six error codes
An asset unlinked or truncated after its verdict was cached reached the client as
runtime_error carrying the desktop's absolute install path. Both now answer
mobile_web_bundle_asset_changed, with the cause warned host-side only. A short
positional read is the truncation case, so it throws instead of paging the client
past the end.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* refactor(mobile-web-bundle): drop the unreachable release-idempotence guard
The one caller releases exactly once in a finally; removing the flag left every
test green, so it was defensiveness against a caller that does not exist.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile-web-bundle): prove a failed verify is not cached as a verdict
The verdict cache never invalidates, so a transient read failure remembered as a
verdict would poison the asset for the life of the process. Removing the delete
left every test green until now.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* refactor(mobile-web-bundle): delete the unsatisfiable manifest params schema
The dispatcher substitutes `{}` for absent params, so `z.null()` could never
parse; the method declares `params: null` instead. A comment on the method name
records why there is no schema.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* fix(mobile-web-bundle): fill the read window instead of failing a partial read
fs.read may answer short of what it was asked for before EOF, so the previous
check turned a legitimate partial read into a spurious asset_changed. The loop
mirrors the relay's readFullStreamChunk, which is not imported because it sits
behind the relay dispatcher's module graph; only a read returning nothing is
treated as truncation.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* refactor(mobile-web-bundle): read the disconnect idiom with the shared predicate
isClientDisconnectedError already exports exactly the check the catch needed, so
the local error class goes away and the throw returns to the repo-wide idiom. The
module doc now says asContractError is a total catch.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile-web-bundle): pin the four branches no test was holding
Each one survived a mutation: the abort check before verification, the
per-process manifest cache, the buildId component of the verdict key, and
delete-at-zero in the admission map. The last two matter beyond hygiene — a
verdict keyed by path alone carries a failed verdict onto the next build of
index.html, and a map that never drops a key retains one pairing token per
socket.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(relay): measure per-connection teardown and hot-path costs by counting
Both suites replace a would-be duration with the structural fact the duration
was a proxy for, so neither depends on machine load.
The census pins that attach/publish/detach churn returns every per-connection
container to baseline, and asserts the containers actually filled first so a
green cannot come from a probe that never loaded them. It also pins the one
container with no per-client teardown: a publication-ledger entry is reclaimed
only by its own lease, never by closeClient.
The operation counts pin that notifyLegacyCapacity costs one ledger lookup per
active client, that a broadcast costs a fixed number per subscriber, and that
abortClient enumerates every controller rather than the target client's --
which is what makes a full client churn quadratic.
* perf(relay): index client request aborts instead of scanning every controller
abortClient runs on every closeClient and every setWrite. Under the flat map
keyed `${clientId}:${requestId}` it had to walk every controller in the relay to
find one client's, so a full churn of N clients each holding K in-flight requests
cost K*N*(N+1)/2 key visits: measured 50 -> 5,100, 100 -> 20,200, 200 -> 80,400,
400 -> 320,800, exactly 4x per doubling.
Do not "optimise" this back to a scan with an early break. It cannot work: the
matching keys are scattered through the map, so any correct loop still visits
every entry before it can know it is done. Only an index makes teardown
proportional to what the client owns.
`create` now returns an opaque handle carrying the owner, so a release finds its
bucket without parsing a composite string key, and no call site changes.
Also stop building the low-water key array eagerly. `belowLowWater` decides on
the aggregate ceiling first and returns without reading the keys, but the caller
had already allocated an N-element array and N template strings to pass them --
paying most in the loaded case, which is when that short-circuit fires. It takes
a thunk now.
The hot-path test becomes a guard rather than a characterisation: it asserts a
teardown visits only the target client's K controllers and never enumerates the
client index at all, since enumerating it is the old scan. Verified by mutation:
restoring the scan shape fails it with "expected 40 to be +0". It asserts the
maps really hold 160 controllers first, so it cannot pass by never filling them.
* test(relay): make the capacity-thunk guard fail when the thunk is removed
The operation-count test measured an idle dispatcher, where the aggregate ceiling
never short-circuits, so every key is read whichever call shape is used. Reverting
the thunk left all five assertions green -- it guarded nothing it claimed to.
Adds the loaded arm, where the ceiling answers first and the saving exists, and
asserts the client index is not enumerated at all. Reverting the thunk now fails
it with `expected 50 to be +0`.
Drops the ledger-retention case: it asserted a stranded entry SURVIVES close, so
it pinned a capacity leak as a contract and would have broken whoever fixed it. It
also used a key no client-keyed reclamation could match, and touched nothing this
branch changes. The churn census already proves normal closes settle every entry;
the gap is recorded there as a gap.
* test(relay): carry the SAFETY: rationale main's casting gate now requires
Not introduced here: main gained a `typescript/consistent-type-assertions` scan while
this branch sat 432 commits behind, and every `as` in the two probe files this branch
adds is new relative to main, so all 11 land as new findings. Verified by running the
gate on this branch with and without my earlier test commit — 11 either way.
Both files reach past `protected` to count containers, which is the measurement; each
cast now carries the line-specific rationale AGENTS.md mandates.
* test(relay): put the countingIterator SAFETY: directive on the line oxlint flags
The diagnostic points at the `return {` that opens the object literal, not at the
`} as IterableIterator<T>` that closes it, so disable-next-line has to sit above the
statement.
* test(relay): type countingIterator as MapIterator and drop two suppressions
The wrapper only ever receives a Map iterator, so declaring that removes the cast at
both call sites; one irreducible cast stays on the object literal, which cannot satisfy
MapIterator's full surface. Three suppressions become one.
* fix(relay): key the abort index by the id's string form so a string id can still be cancelled
The flat map's template key folded a request id of 7 and "7" onto one entry;
keying the raw value split them, so rpc.cancel (which coerces through Number)
missed a string-id request. Restore the coercion at the index.
The renderer expressed every non-answer as one nullable `status`, so a probe in
flight, a probe that failed, a host that refused us and a retired pairing all
reached readers as the same `null` -- and readers spent that `null` on decisions
of very different weight, including destructive ones.
`RuntimeHostContact` names the four. Nothing changes yet: the connection-state
derivation is rewritten on top of it and a 384-case parity table asserts the
result is identical to a frozen copy of the old one on every combination of
verification, transport, retired, answered and remote-control state.
The platform a host runs is a fact about the host, not about whether its last
probe came back. Reading `entry.status` fell through to the client's platform
the moment a probe went unverifiable, so a Windows host driven from a Mac
silently started resolving keystrokes and paths with POSIX conventions
mid-session -- and switched back on the next successful probe.
Same conversion as the four sibling reads, using the same shared reader.
Settings > Available Hosts and the repository host-setup section render the
same host from the same store entry, but this row derived its own answer from
raw `entry.status`. An unverifiable probe nulls that while the transport is
still up, so the row flipped to "error" and swapped Disconnect for Connect
while the other surface -- which already goes through
runtimeHostConnectionStateForEntry -- still showed the host as reachable.
One host, two surfaces, opposite answers. A probe that did not come back is
not a host that went away.
* fix(ssh): record the superseded-relay pass the Windows arm abandons
`sweepSupersededRelayEndpoints` returned `[]` for every Windows remote host
and for every failed listing without writing a line. Both returns are
indistinguishable from "this host had no orphans", which is the one thing
this sweep exists not to be: its own header says it makes the orphan
population "visible and deliberate rather than silent".
The Windows population is real. `relayEndpointForHost` hashes the version
directory into the pipe name, so an app update strands the incumbent exactly
as it does on POSIX, and with `--grace-time 0` that relay keeps its PTYs and
agents forever. Measured on a Windows 11 host (awin): the NPFS root lists 262
named pipes from an unprivileged shell, and the count of `orca-relay-*` names
goes 0 -> 1 the moment a relay binds, so the endpoints are enumerable; the
repo already enumerates them for GC via `relayLivenessProbeCommand`'s
`.windows-active-pipe-*` marker scan.
Reclaiming them is not this change. `probeRelayEndpointIncumbent` answers
`unverifiable` for every Windows path, so nothing here could be classified,
let alone reaped, and nothing about the kill path moves. What changes is that
an abandoned pass now leaves a trace.
* fix(ssh): keep the endpoints a half-run superseded sweep already classified
The Windows arm and the failed-listing arm now both leave a line. The loop between
them did not: socket 1 could be fully probed and classified, and an exec on socket 2
that threw took `logSupersededRelayFindings` with it — so a half-run pass and a host
with nothing to sweep produced the same silence, and socket 1's verdict was lost.
Only one failure class can leave that loop, and it is the one that matters: an exec
whose SSH channel never confirmed close, which may still be running remotely and
which `probeRelayEndpointIncumbent` rethrows by design. Every ordinary probe failure
already degrades to `unverifiable` and the pass continues — a test now pins that too,
so nobody "fixes" the loop into stopping on an absence of evidence.
Findings are logged before the rethrow, which propagates unchanged. The added line
says how far the pass got and claims nothing about the endpoints it never reached.
* fix(ssh): word the Windows sweep skip so a first install does not read as orphaned
The line fired on every Windows relay launch and asserted a population: "orphans from
earlier builds are neither listed nor reclaimed" reads as a finding on a machine that
has never had an earlier build. The skip is what is being recorded, not a census.
* fix(relay): one malformed pre-auth handshake frame closes its connection, not the daemon
parseHandshakeMessage returned whatever JSON.parse produced, and the daemon
interpolates the peer's version into a log line before any credential check.
A version that is an object with a non-callable toString throws TypeError
there, inside the frame-decoder callback. FrameDecoder.drainTurn wrapped its
synchronous dispatch in try/finally with no catch, so the throw escaped
feed(), escaped the socket data handler, and reached uncaughtException: the
relay daemon exited and every PTY and agent session it held died with it.
Two layers, because only the second closes the class:
- parseHandshakeMessage now requires the string fields each arm carries
(version; expected/got) and rejects a non-object payload. Both readers
share the parser, so neither side can interpolate a non-string again.
- FrameDecoder contains a frame owner that throws on the synchronous turn
the same way it already contained one on a continuation turn: reset the
residue and report one FrameDecoderContinuationError to onError. Every
owner's onError already closes its own connection, so any future throw
of this shape costs one connection instead of the process.
The relay CLI channel gains an explicit onError so a malformed reply still
ends that one-shot command instead of parking it.
* fix(relay): keep the diagnostic the refusal path exists to produce
Two error paths that destroy their own evidence.
`parseHandshakeMessage`'s unknown-type refusal interpolated `String(t)` on a
peer-supplied value: `{"type":{"toString":1}}` makes String() throw "Cannot
convert object to primitive value", so the refusal arrives without naming what
was refused. `describeRelayProtocolVersion` guards this exact hazard two files
away; the sibling was missed.
`runRelayOrcaCliChannel`'s new `onDecodeError` wrote to stderr and then exited
synchronously. stderr is async on a pipe transport, so the one line recording
why the command died could be dropped — the reason relay-handshake.ts already
exits inside its write callback.
* fix(relay): prove the optional handshake field too, not just the required ones
The parser refuses a non-string `version`, `expected` and `got`, then returns the
object with `endpointCredential` unproved — the most pre-auth field on the frame.
It is safe today only by accident: its one reader compares it, and a non-string
loses that comparison. Nothing holds that shape in place, and the next reader to
put it in a log line reinstates the template-literal throw this function exists
to stop.
Present-but-not-a-string is now refused at the parser. Absent stays absent: a
bridge presenting no credential is the common case, and refusing it would close
every unauthenticated-endpoint connection.
Wire-visible delta, deliberate: a peer sending a non-string credential used to get
`orca-relay-handshake-credential-mismatch` and exit 43; it now gets a bare close.
No first-party client can reach it — `runConnectHandshake` types the parameter
`string` and omits it when falsy — and a bare close is the right answer to a frame
that was malformed before any credential was checked.
* fix(relay): carry the SAFETY: rationale main's casting gate now requires
Main gained a `typescript/consistent-type-assertions` scan while this branch sat 432
commits behind, so every `as` the branch touches lands as a new finding. The parser is
the one place the handshake shape is proved, so each cast names the check that earns it,
and the hostile-frame cast in the round-trip test names the fact that it is a deliberate
lie the type system cannot describe.
* test(relay): annotate the hostile handshake frame instead of suppressing a cast
JSON.parse answers `any`, so a typed const expresses the same deliberate lie the
assertion did and the casting gate has nothing to flag. One fewer suppression.
resolveTerminalLayoutRoot discarded any known tree that did not cover the
published leaf set exactly and rebuilt the tab as a flat chain with a guessed
'horizontal' direction, restacking side-by-side panes. The guess is then
published, mirrored to every paired client, and written back over the real
tree, so the direction is gone from disk.
Prune a known tree to the leaves that survive and graft only the leaves no
tree places, which is now the sole place a direction is invented and is still
reported through onSynthesize.
* fix(mobile): mint one pairing offer per Continue on the sidebar page
Step 2 auto-minted as soon as it became visible, which is the same commit
that starts the network-interface lookup. The offer therefore advertised
whatever address was left over from the last visit (or none at all, so
main picked its own default), and when the lookup settled on a different
address the refresh handler reminted with rotate: true. Two overlapping
getPairingQR calls then raced for one pending credential: main rotates the
pending device away for the rotate mint, and orders concurrent offers by
arrival at its generation counter rather than by the order the renderer
issued them, so the request the pane is waiting on can be the one main
decided to supersede.
Defer the auto-mint until the interface lookup settles, and keep Step 2
reading as busy while it waits — the sidebar has no separate Generate step
the user is expected to reach, so it must still mint on its own, unlike
Settings which clears and waits for an explicit press.
* fix(mobile): gate the Step 2 mint on this flow visit's address lookup
The first attempt gated on a single boolean ref meaning "an address lookup
is running". That cannot describe a re-entrant operation: entering the
flow, leaving, and re-entering runs two overlapping lookups, and the first
to land clears the flag while the second is still out — so the mint went
out against the superseded lookup's address and the second lookup then
reminted with rotate: true. The same double mint the change exists to
remove, one path over.
Gate on positive evidence instead. Each flow entry bumps a visit counter;
the lookup records the visit it answered (max, so an abandoned visit
landing last cannot walk the marker backwards); the mint waits for
addressedFlowVisit === pairingFlowVisit, which is false at t=0 by
construction and makes exactly one false-to-true transition per visit. The
ref is gone and the effect's dependencies now name what it depends on.
A superseded lookup's response is also discarded outright, so it cannot
move the picker onto an address a newer lookup already replaced — that
reselection is itself a remint trigger.
The derived busy flag collapses to one clause and is renamed
awaitingPairingAddress: it was being passed down as pairLoading while
local readers used the real one. It stays separate from pairLoading
because that feeds shouldRegenerate in the invalidation hook, where
merging them would let a mode switch mint before the address settles.
* fix(mobile): put the visit-settled write behind the lookup epoch guard
setAddressedFlowVisit was the one completion side-effect outside
networkInterfacesRequestIdRef, so a superseded lookup *for the same visit*
still marked that visit addressed and released the mint while its own
replacement was still pending — the newer address then rotated the offer
away. The visit counter cannot see this case: both lookups belong to one
visit, and only the request epoch distinguishes them.
Reaching it needs a manual Refresh click to beat the commit that disables
that button, so field impact is low. The point is that the invariant is now
structural instead of resting on a button being disabled in time.
Math.max is dropped with the move. Every visit bump starts its own lookup,
so the newest request always carries the highest visit and the marker
cannot move backwards — the max could no longer be killed by any single
mutation, which made it dead code asserting a hazard the guard removes.
Also swap the test reset to _resetPairedMobileDevicesCacheForTests, matching
the sibling suites: replacePairedMobileDevices is production API that
publishes loaded:true and leaves the recovery-listener refcount untouched.
* refactor(mobile): make the unaddressed flow visit an explicit null
-1 only worked because visits start at 0 and count up; null says "no visit
has been addressed yet" without depending on that. Also record at the visit
bump why it cannot move into the stage effect: an effect runs a render after
Step 2 is visible, so the auto-mint would see the previous visit settled.
* fix(mobile): invalidate abandoned pairing mints
* fix(runtime): treat selector_not_found as definitive tab absence (#21189)
When closing a tab whose worktree selector is absent, propagate the
error through host RPC and classify it as unknown-tab on the renderer
to engage durable tombstones and prevent resurrection loops.
Pin host RPC error propagation with dedicated regression tests.
Co-authored-by: Neil Parker <neil@stably.ai>
* test(runtime): remove invalid absent-tab Docker spec
The spec dynamically imported renderer source from the browser and did not exercise a real close RPC. Keep the executable renderer and host regression coverage instead.\n\nCo-authored-by: Lesley Murfin <lesley@revivebusiness.ca>
* fix(runtime): narrow durable tab absence to tab and terminal absence (#21189)
Narrow durable close tombstones in web-runtime-session-tab-lifecycle to
tab_not_found and terminal_tab_not_found. In production, session tab close
requests pass explicit `id:` worktree selectors and take the fast path in
closeMobileSessionTab, bypassing resolveWorktreeSelector. Transient
selector_not_found errors retain normal TTL eviction.
---------
Co-authored-by: Neil Parker <neil@stably.ai>
* fix(chat): release provider children after lost resume holds
* test: load audit fixtures as modules and verify combined mobile payload
---------
Co-authored-by: m4air <m4air@Mac.localdomain>
* fix(filesystem): match allowed roots across Unicode forms
macOS returns a path in whichever Unicode form its source held: APFS gives
back what it stores (NFD), while the file picker and git
(core.precomposeunicode) give back NFC. A workspace registered in one form
never matched a file read in the other, so fs:readFile denied a path inside
the open workspace (#21172).
isDescendantOrEqual now compares byte-exactly first and retries in NFC only
when that fails and both sides carry non-ASCII, leaving ASCII containment and
the traversal guards untouched.
* fix(filesystem): prove identity before admitting a Unicode-folded root
Canonical equivalence is not identity: APFS folds both spellings onto one
directory, but a byte-exact filesystem can hold them as distinct siblings,
and admitting the unregistered one widened the allow-list.
The NFC fold now only locates the ancestor of the target that the
registered root would have to be; containment is granted only when that
ancestor and the root stat to the same dev+ino. A failed stat or an ino of
0 denies. ASCII paths and roots that do not fold onto the target never
reach the disk.
NeilandEddie Jaoude
Neil
Luke Son
Jinwoo Hong
Brennan Benson
Lesley MurfinandNeil Parker
Lucas Farias