createSupport gates the create path, but a session's account state can change
while it lives. A reacquire after an unexpected child exit re-resolves the
launch and re-derives auth, with nothing re-checking the gate — so a session
created while supported could come back up in the refused shape. With the strip
predicate keyed on there being an active non-WSL account, the WSL-only user's
normalized steady state (accounts exist, none active) does not strip, and that
reacquire reaches the child with ambient auth while the UI names the account.
Gate at resolveLaunch, the one choke point every acquisition passes through,
refusing with the pre-spawn error the caller already handles. Same predicate as
create-time, now sharing one settings reader so the two cannot drift.
Claude only; Codex resolves its account on a different path and is untouched.
The runtime class that wires this does not typecheck its own `this` calls — a
missing hookup compiles clean — so the wiring is pinned behaviourally rather
than trusted to the compiler.
N-1: my presence-based conflict predicate refused a terminal launch that works
today. 'ANTHROPIC_API_KEY=' is how a user blanks a variable and the settings
pipeline preserves that empty value; an empty override cannot beat the pinned
account and the strip removes the name anyway. Back to truthiness for the value,
keeping the win32 case folding.
N-2: enter the live-auth gate only after the exit/close handlers that release it,
so no throw in between can leave an entry nothing reconciles.
N-4: the Claude transcript resolver searches config-dir-then-default and de-dupes,
matching the Codex sibling in the same file, so adopting CLAUDE_CONFIG_DIR no
longer hides history written before it.
The gate resolved the active account from the account-service snapshot's
runtime map; the auth policy resolves it with
getSelectedClaudeAccountIdForTarget(settings, { runtime: 'host' }). Those are
two sources and two resolution rules, and they disagree on a legacy settings
blob that carries the selection only in the flat activeClaudeManagedAccountId:
the accessor falls through to it, a direct read of the runtime map does not. The
gate would then refuse a launch the policy would have run under host-1 — and in
the mirror case a session could be admitted under a policy computed from a
different account than the gate approved.
Read the same settings through the same accessor so agreement is structural
rather than coincidental, and drop the controller accessor that existed only to
reach the snapshot.
No behaviour change for any state both already agreed on; Codex is untouched.
Structured Claude launches against the ambient Claude config, which the account
service keeps in sync with the selected HOST account. A WSL-bound managed
account lives inside the distro and is never synced there, so on Windows a
structured session would authenticate as whatever the ambient identity happens
to be while the UI names the WSL account — the user is told one identity and
given another.
That was unreachable only because nothing offered structured Claude on win32.
Enabling it makes it reachable, so gate it here rather than patching the auth
layer: refuse the structured path when the active managed Claude account is
WSL-bound, and let the terminal-backed path — which resolves the account per
runtime — handle that account shape.
The answer rides the agentSession.createSupport seam the renderer already
consumes, so no new capability and no renderer knowledge of account internals.
A create the host declines becomes the definitive refusal the launch fallback
already turns into a legacy native chat tab, with no error toast.
Unknown answers refuse. An install with no managed accounts claims no identity
and is fine, but an active selection that cannot be resolved — or account state
that cannot be read at all — is not evidence that the ambient identity is right.
Claude only. Codex resolves its account through a different path and its
createSupport answer is untouched, as is every Codex routing decision.
P2-1: a switch beginning inside the acquire teardown left a dead chat and no
replacement. Past that point the launch waits the swap out and refuses only if it
never settles; the entry guard still refuses outright, because nothing is torn down
there yet.
P2-2: structured children now hold the same OAuth-refresh gate a Claude PTY does,
so a managed refresh cannot rotate the token out from under a live turn.
P3: the refusal now matches the strip it guards (case-folded on win32, presence not
truthiness), and the dead structured-to-TUI builder states its auth policy instead
of silently signing a system-auth user out.
session-file-resolver's default ignored the variable the pinned account home
follows, so a CLAUDE_CONFIG_DIR launch wrote one tree and mobile read another. The
Task-4 test now resolves with no root override (mobile's own call) and checks the
answer against the root the CLI itself reports, instead of mirroring the code under
test's own expression.
The optional dep plus a {stripAuthEnv:false} fallback meant a dropped wiring
under-stripped silently. Required at all three hops, asserted at install time for
the @ts-nocheck caller, and the settings-to-policy mapping is now a named tested
function.
The main process has had a complete, correctly gated Claude Agent SDK lane for
a while, but no renderer ever asked for it: the launch route accepted only
`codex`, and the create path was typed `agent: 'codex'` end to end.
Widen both to the structured provider union that already exists
(`AgentSessionHandleProvider`), and generalize the codex-named create path
instead of adding a Claude twin beside it. The pending-launch registry is now
keyed by agent as well as workspace — a shared key handed a second caller the
first agent's intent, so a Claude and a Codex launch in one worktree collided.
Windows, per agent. Codex's client-side win32 refusal is deliberate and settled
elsewhere, so it stays exactly as it was. Claude's answer is no longer guessed
from the client's platform: a structured session fences its provider child on
that child's process start time, and only the executing host knows whether it
can read one. `agentSession.createSupport` already answers precisely that, per
agent, and had no renderer caller — so the Claude create path asks it before
creating and turns a "no", or a probe it cannot get answered, into the
definitive refusal the launch fallback already handles. Fail closed either way.
That refusal mapping also closes a real gap: the host reports an unsupported
location by throwing `structured_agent_session_unsupported`, which reaches the
client as a transport rejection rather than a refusal envelope, so
`StructuredAgentSessionCreateRefusalError` never fired. The launch would retry
the create, strand itself in `visibilityUnknown`, run no legacy fallback, and
show an error toast.
Close a fail-open hole while Claude and win32 become reachable: `create` with a
client-supplied location, and `ensure`, both skip the worktree-resolving support
check. They now ask the executing host the same question directly, so a host
that cannot fence a provider child no longer creates one on a client's say-so.
Also deletes `structured-agent-session-provider-routing.ts`, a duplicate of
`structured-agent-session-provider-support.ts` with no importers.
WSL, SSH and paired hosts, floating workspaces, draft prompt delivery, explicit
TUI customization and initial session options all keep refusing; folder
workspaces keep working.
The SDK path stripped ambient Anthropic auth unconditionally, let an explicit
agentDefaultEnv override beat a pinned managed account, and had no account-switch
guard. Reuse the terminal preflight's own predicate and messages so both transports
strip, refuse, and report identically, and cover the CLI transcript location that
mobile native chat depends on.
The direct root kill goes through the handle Node owns, not through a pid:
libuv drops that handle in the same turn it reaps, so the signal either
reaches the process Orca spawned or reaches nothing at all. Gating it on a
process-table probe therefore bought no safety and cost the tree its only
fallback whenever the probe declined -- a first capture landing in the fork's
own second, a recycled descendant pid voiding the snapshot, or a process table
that could not be read on either platform.
Identity verification stays where a bare pid is genuinely addressed: Windows
`taskkill /T /F`, and the descendant sweep's own revalidation before it signals.
Also stops a declined root probe from collapsing an observed `live` or `exited`
descendant verdict into `unverifiable`, and stops a successful taskkill from
reporting `unverifiable` because a later probe found the root correctly dead.
Replace the translucent bg-muted/25 with an opaque color-mix blend
(40% muted on background) to ensure scrolled rows don't show through
the sticky header. Add test coverage for header styling and layout.
* perf(hot-paths): delete allocation-only work in sort, explorer, monaco, rpc, snapshots
* fix(perf): revert snapshot revision fast-path — same revision can carry a new session
* perf(hot-paths): drop the unproven rpc buffer rewrite, dedupe the equality helpers
- Revert the unix-socket chunk-carry change. Its comment claimed it avoided
O(n^2) rescans, but chunks is reset to [remainder] every data event, so the
join plus the tail byteLength is two passes where the old code did one;
benchmarks showed no win. It also moved consumed-frame bookkeeping out of the
closure, so a synchronous throw from the handler would re-dispatch frames.
- project-host-compatibility: fold the two byte-identical array comparators
into one generic arraysEqualByJson.
- smart-attention: drop the leftover byTab.size === 0 branch that returned the
same value as the line after it.
* perf(renderer): stop five timers from ticking behind a hidden window
* perf(renderer): park hibernation and panel-watchdog work behind a hidden window
Narrowed from five timers to two, and made both correct:
- Gate on getWindowParkVisible(), not raw document.visibilityState. macOS can
wedge visibilityState at 'hidden' with no further visibilitychange, which
would park these for the rest of the session.
- Add a real becoming-visible pass via subscribeWindowParkVisibility, so resume
does not wait out the remaining interval. Unsubscribed on stop.
Dropped the other three gates:
- terminal-delivery-watchdog: it is the recovery lane for the byte-drop bug the
stale-visibility latch exists for; parking it costs a frozen terminal.
- crash-diagnostics: the dashboard-popout surface is normally occluded, so it
would sample once at startup and never again.
- use-contextual-tour: attempts only increments past the gate, so a hidden
window turned a self-clearing 20-attempt interval into a permanent one.
Tests stub visibilityState and the stale latch; both watchdog cases are RED
against the previous raw-visibilityState implementation.
* perf(renderer): narrow vault and editor subscriptions off the every-write path
* fix(perf): revert EditorPanel narrowing — downstream hooks need the full openFiles list
* perf(renderer): make the vault session-id cache resettable between tests
Every production writer replaces agentStatusByPaneKey, but test fixtures
commonly mutate it in place, which would keep serving the key cached for that
identity. Fold the WeakMap into the existing reset hook.
* fix(windows): refuse tree-kills of Orca's own Chromium pids and record the rest
G2 is 20 field reports that share only a symptom. It is at least four
fingerprints: ~15 Windows `reason=killed exitCode=1`, 3 POSIX SIGKILL under
memory pressure (G4-oom), 2 duplicate reports of one macOS V8 Proxy Resolver
SIGKILL, and 1 `0x80000003` install-dir ACL crash (G1; #17740 ships in
v1.4.196 only, not 1.4.195). Nothing here claims to fix all of them.
Two changes:
1. Behaviour. `classifyWindowsTreeKillTarget` returns `own` for any direct
child of the main process — which our renderer, GPU and network-service
utility all are — so PTY teardown could `taskkill /T /F` Orca's own UI
(#10680). Both that classifier and `terminateWindowsProcessTree` now refuse
any pid Electron is currently accounting for in `getAppMetrics()`.
2. Diagnosis. An Orca-issued kill and an external one are byte-identical in
every field the crash report records today, so the cluster is undecidable.
Every main-process force-kill choke point now records a durable
`self_tree_kill` breadcrumb, and `process_gone` reports carry
`selfInitiatedTreeKills` naming the pid and its offset from the death.
A refused kill records `self_tree_kill_refused_own_chromium`, which is
falsifiable: if it ever shows up in the field, we were the killer.
* fix(crash-reporting): coalesce self-kill breadcrumbs and scope the discriminator
Round-1 review remediation. Three blocking findings, all accepted.
1. Breadcrumb flood (accepted). recordSelfInitiatedTreeKill wrote an
uncoalesced durable crumb from two routine teardown paths, and the
reviewer reproduced 12 terminal closes x 3 process groups completely
evicting the 30-slot ring — including this PR's own refusal crumb — plus a
forced writeSync per killed group. It now uses the existing
recordCoalescedDurableCrashBreadcrumb (5s window for pid-addressed
taskkills, 60s for routine group/job teardown), so a burst costs one ring
slot and one flush. The refusal crumb is coalesced per victim pid, so a
retry loop cannot flood while a distinct pid always gets its own crumb.
Regression test replays the reviewer's exact 12x3 reproduction and asserts
the refusal crumb and a pre-existing gpu_process_crashed both survive.
2. Undifferentiated count (accepted). posix-process-group and win-pty-job are
structurally incapable of reaching a Chromium process, and scope was absent
from the persisted string. Scope is now in every entry
(`<scope>/<site>/pid<N> +Nms`), and the count is split:
selfInitiatedTreeKillCount now counts only pid-addressed taskkills — the
kills that can land on a recycled pid that is now our renderer — with
pty-scoped sweeps in selfInitiatedGroupKillCount. The list is renamed
selfInitiatedKills because it carries both, and sorts pid-addressed kills
first so truncation never drops the discriminating ones for teardown noise.
The reviewer's repro (routine macOS terminal close + unrelated exit-133
crash) now yields selfInitiatedTreeKillCount undefined.
3. Recording gaps and a false comment (accepted). New
admitSelfInitiatedTreeKill gate: it refuses own-Chromium pids and records
the rest, and all three main-process taskkill families now go through it —
terminateWindowsProcessTree plus codex-accounts/service.ts and
claude-accounts (which keep their own spawn lifetimes). The false "single
taskkill choke point" comment is gone. The runProcess choke point the
investigation asked for is instrumented via a
setProcessTreeKillObserver seam in src/shared/child-process — shared code
runs in the CLI and relay so it cannot import the main breadcrumb store —
registered in main preflight. The codex app-server POSIX group teardowns
and the claude POSIX branch record too. The module doc no longer claims
absence is discriminating: it enumerates what is instrumented and names the
direct process.kill(-pid) sites that are not.
Non-blocking, also fixed:
- Breadcrumb calls moved out of the try blocks whose catch is the ESRCH
contract (posix-pty-process-groups, codex teardown, claude POSIX), so a
throw from the diagnostic path can never be reported as a failed kill.
- Detail truncation now bounds the first entry too, matching its comment.
- own-chromium-tree-kill-refusal.test.ts renamed to
own-chromium-tree-kill-guard.test.ts, colocated with the module it tests.
Not changed, with reasons:
- Date.now() vs performance.now(): kept. Offsets are computed against
goneAt = Date.now() in process-gone-recorder; a monotonic clock here would
make the offsets meaningless. The reviewer verified this and agreed it is
not a defect.
- app.getAppMetrics() per force-kill remains unbenchmarked. It reads
in-process browser state rather than enumerating the OS process table, and a
TTL cache would let a recycled pid slip past the refusal, so it stays
uncached.
- The ~15 remaining direct process.kill(-pid) sites (browser routes,
notebooks, automation prechecks, ephemeral VM recipes) are not instrumented.
Rather than claim coverage this PR does not have, the module doc names them.
claude-command-process.ts crossed the 300-line cap, so terminateClaudeProcess
moved to claude-login-process-termination.ts. No max-lines suppression added.
* fix(crash-reporting): scope the self-kill guard to its real host topology
Round-2 review findings on the own-Chromium tree-kill guard.
BLOCKING 1 — "the own-Chromium refusal is a no-op in the process that issues
the pty-descendant-sweep taskkill". Correct on the mechanism, wrong on the
consequence; REBUTTED in part and documented in full.
Confirmed: the only non-test `setAppEnvironment` installs are
main-process-preflight.ts:177 (Electron) and orcad-entry.ts:84 (Node, whose
`getAppMetrics()` is `[]`); daemon-init-fresh-import.ts is a test harness. So
in the standalone daemon `readOrcaChromiumProcessPids()` is empty and
`admitSelfInitiatedTreeKill` always admits.
But that is not a live hazard. `killWithDescendantSweep` reaches
`terminateWindowsProcessTree` only when `verifyWindowsTreeKillTarget` returns
`own`, and that walks ancestry back to `deps.ownerPid ?? process.pid` — the
KILLING process's pid. In the daemon that is the daemon's pid. Orca's Chromium
processes are children of Electron main, a sibling of the daemon, so their
chain never reaches it: hop 0 lands on main, and within MAX_ANCESTOR_HOPS the
walk dead-ends and returns `foreign`. The reviewer's probe passes
`ownerPid: 1000` with the renderer as a direct child of 1000 — that is the
Electron-main topology, where the AppEnvironment IS installed and the guard DOES
fire, not the daemon's. On an orcad/SSH host there is no Chromium on the box at
all, so `[]` is accurate rather than degraded.
Locked in as tests rather than prose (own-chromium-tree-kill-guard.test.ts):
a renderer classifies `foreign` from a daemon ownerPid with an empty pid set,
and `own` from main's ownerPid with an empty set — the falsifiable pair showing
the pid set is load-bearing in main and nowhere else. Documented the host
coverage in orca-chromium-process-pids.ts and own-chromium-tree-kill-guard.ts.
One genuine hole the finding exposes: `signalProcessTree`'s `taskkillTree` is a
fourth pid-addressed taskkill family (non-blocking item 2), it runs in the
daemon/relay/CLI where the guard cannot run, and it guarded only on
`!child.pid`. Reusing the predicate the codex login teardown already uses, the
win32 branch now refuses a reaped child and falls back to `killRoot` — the same
shape as the existing `!child.pid` branch. That closes the reaped-then-recycled
pid path in every host.
BLOCKING 2 — module doc overstates coverage. Rewritten: the ring is per-process
and its only reader lives in Electron main, so a count on a `render-process-gone`
covers main-issued kills only. Sites are now split into main-only, main-and-
other-hosts (runProcess choke point, POSIX PTY group sweep, Windows Job Object —
which record into a ring nothing reads when they run in the daemon or relay),
and never-instrumented, with the note that a daemon/relay omission is a
diagnostics gap, not a missed suspect, per the topology argument above.
BLOCKING 3 — the three out-of-main instrumentation sites were untested. Added
regression coverage: the runProcess seam on both branches plus the reaped-child
refusal (process-tree-termination.test.ts), the group sweep recording only
groups it actually signalled and skipping an ESRCH group
(posix-pty-process-groups.test.ts), and the Job Object recording the shell pid
only on `terminated` (windows-pty-job.test.ts). Verified red: reverting the
three production files to origin/main fails 7 of the new tests.
BLOCKING 4 — the Windows evidence validates a single-process model. Accepted.
The main2.js arms exercise `pty-descendant-sweep` inside one Electron process;
that models the in-process/degraded daemon and the local PTY provider, not the
standalone daemon. Arm C's "the 449351d6 shape is not producible with the guard"
holds for main-issued kills only. In the daemon the shape is blocked one layer
earlier, by the ancestry check, which the arms do not exercise.
NON-BLOCKING taken: `recordSelfInitiatedTreeKill` moved outside the native
`terminateJob` try in windows-pty-job.ts, so a diagnostics throw can no longer
downgrade a real termination to `unavailable` and escalate callers to a broader
kill; covered by a test. The "all three families" parenthetical is gone with the
doc rewrite. `pnpm build:relay` run: exit 0, all seven targets built.
NON-BLOCKING declined: codex-accounts/service.ts records before the spawn
because a refusal must prevent the spawn — the crumb means "we were about to
kill this pid", which is the artifact worth having; the existing comment already
says so. `app.getAppMetrics()` perf is unbenchmarked and unchanged by this round.
Verification: pnpm tc clean; oxlint clean on touched paths;
check:code-quality:changed 0 new findings; oxfmt applied. 730 tests pass across
shared/child-process, main/crash-reporting, main/pty, main/windows and the guard
and descendant-sweep suites. The 4 failures in providers/git/codex-integration
reproduce on HEAD without these changes.
* fix(crash-reporting): keep the reaped-pid skip from flipping the termination barrier
The win32 hasExited short-circuit correctly avoids taskkill on a pid Windows
may have reissued, but it resolved `true` — verified tree termination. A
taskkill against a reaped pid already resolved `false`, and run-process turns
`true` into barrierTerminationVerified + terminationReporter.report(), which
releases the git admission grant on root exit instead of on `close`. That
admits the next git command while a descendant holding the inherited pipes is
still writing the repo. Resolve `false` so the skip changes only which process
we refuse to signal, not what the barrier claims.
* perf(renderer): index diff comments, skip no-op hydration, drop duplicate normalizes
* fix(perf): keep tree-path stability hook render-pure for react-doctor
* fix(perf): publish the returned array from the tree-path stability hook
The ref was written with the raw input but read in render to pick the return
value, so it trailed one commit and a wave of content-equal arrays flipped
identity every render — re-firing the uncancellable full-tree git check-ignore
it exists to prevent. Publish `stable` instead, keyed on `[stable]`.
Also drops the hydrateOverrides no-op skip: notifyChange is not a bare wakeup
(it drives getPanesNeedingOverrideFit -> safeFit and the remote viewport
re-claim), and the branch never fires in production anyway.
The relay carried its own copies of the worktree-list and unmerged-entry
porcelain parsers, and both had drifted from the desktop originals: the
relay copy had no `sparse` branch, so SSH sparse checkouts were never
marked, and it never C-quote-decoded a conflict path, so a conflicted file
with a space or non-ASCII byte was published under its raw quoted name and
probed as missing.
Move both parsers into src/shared and delete the relay copies, so there is
one implementation each. Type the relay's worktree-list plumbing on
GitWorktreeInfo instead of Record<string, unknown> so a field-copying step
can no longer silently drop a newly parsed field.
`isSparse` is a new optional field on the git.listWorktrees result
(remote-wire-compatibility Rule 1); Git <2.28 omits the porcelain line and
the field stays absent. No new git subcommand or option.
Closes#18280
`ForgeProvider.createReview(repoPath, input, connectionId, options)` and the
`connectionId` on `ForgeProviderRepositoryContext` carried the same collapse the
five prior migrations closed: `string | null` spells "genuinely local", "runtime
host" and "could not resolve" with one value. Because it was decided two layers
up -- `repo.connectionId ?? null` at the `hostedReview:*` IPC handlers and in
`RuntimeHostedReviewCommands` -- a row naming its owner only as
`executionHostId: ssh:<target>` ran the whole review path against this machine's
copy of a remote path (#11163): `git rev-parse`, `git status`, the base-on-remote
ref probe, the upstream divergence read, and `gh`/`glab` with no host flags.
Replace it with a required `ExecutionHostId` threaded from the decision point
through the contract, routed by #18296's `resolveGitRouteForHost`. The parameter
is removed rather than added beside, so all five implementations -- GitLab,
GitHub, Bitbucket, Azure DevOps, Gitea -- and every caller became a compile
error. None of these families carries `@ts-nocheck`, so unlike #18325 that
guarantee is real here; `orca-runtime-file-commands.ts` does, but it only
constructs `RuntimeHostedReviewCommands` with unchanged deps.
Also fixed at the sites:
- The branch cache scoped entries on `connectionId ?? ''`, so two rows at one
path on different hosts shared one cached review, one backoff deadline and one
invalidation. Keyed on the resolved host now, as #18377 did for its probe key.
- `hostedReview:create` resolved shared symlink paths and normalized worktree
paths off the raw field, so an `executionHostId`-only SSH row read `orca.yaml`
and `resolve()`d a remote POSIX path on the client. Those ask the file-holder
question -- `getRepoSshConnectionId` -- not the dialable one.
- An SSH host with no provider now refuses inside the git-state layer instead of
reaching the local branch, keeping "remote and unreachable" distinct from
"local" (docs/reference/ssh-execution-boundary.md).
`runtime:` is a routing mistake inside `hostedReviewSshConnectionId` -- that
environment's server runs its own git, and the SSH target on its repo row is
nested in that server's namespace, so dialing it here reaches a same-named box of
ours. But store-backed callers ask `getRepoHostedReviewExecutionHostId` first,
which is "what may this client dial" and answers `local` for a `runtime:` row.
That is deliberate and matches #18377: the runtime registration controller only
adopts a `runtime:` stamp onto a row with no `connectionId`
(`runtimeRepoMatchesExecutionHost` refuses to match an SSH row), so the checkout
really is in this process and refusing would regress a runtime server creating
reviews for its own rows.
No wire change. `connectionId` on `CreateHostedReviewArgs`,
`CreateStackedHostedReviewArgs` and `HostedReviewCreationEligibilityArgs` in
src/shared/hosted-review.ts is untouched -- every host already ignores it in
favor of the repo row, and removing it from the request types would only churn
the schema older clients still populate. The main-side eligibility input `Omit`s
it so nothing on this side can read the ambiguous field again.
* fix(runtime): recover stale session owners and await retirement
* fix(runtime): preserve session hydration and smoke compatibility
* test(runtime): cover empty and unindexed session owners
* feat(cli): make terminal close the canonical workspace teardown
* fix(preload): align ssh termination result type
* test(runtime): assert folder hydration owner
* fix(runtime): fence legacy terminal stop by worktree host
* fix(preload): reconcile ssh result import with main
* fix(runtime): keep same-id sibling hosts out of workspace close
The stale-owner fallback in the session controller re-routed any worktree whose
catalog partition had no tabs to whichever other partition held tabs. Only
`runtime:` environment ids rotate across relay restarts; `repoId::path` legitimately
repeats across hosts, so an SSH workspace close could retire the local copy's
tabs and resume records, or flip owners mid-close and strand the SSH PTY.
Restrict the fallback to runtime hosts, and pin the session partition once per
workspace close so record clearing targets the partition that owned the tabs.
* test(runtime): give the cross-host close fixture a real resume record
* fix(preload): take main's ssh-bridge import order so the merge stays duplicate-free
`detectRepoIcon`, `detectRepoIconAndUpstream`, `detectGitHubAvatarIcon`,
`detectRepoFileIcon` and `probeGitRemoteIdentity` took a `connectionId`-shaped
parameter threaded down from their callers. That shape spells "runtime host",
"unresolved" and "genuinely local" all as one falsy value, and because it is a
*parameter* each caller decided independently what to pass — a wrong answer was
invisible at the boundary.
Replace it with a required `ExecutionHostId` and route through #18296's
`resolveGitRouteForHost` / `resolveFilesystemRouteForHost`. The parameter is
removed rather than added beside, so every caller became a compile error. No new
resolver, no wire change: nothing these modules return carries a host id.
Fixed at the call sites:
- `repo-git-remote-identity-enrichment` read `repo.connectionId` raw, so a row
minted with only `executionHostId: ssh:<t>` ran `git remote -v` against this
machine's copy of the path (#11163), and a `runtime:` row handed its *nested*
SSH target to this client's dispatch table — a same-named box of ours.
- Its location key had the same collapse, so two rows at one path on different
hosts shared a probe, an abort controller and a backoff deadline.
- `runtime-repository-fork-backfill` guarded on `repo.connectionId`, so an
`executionHostId`-only SSH row had its upstream read off the client.
`runtime:` is refused inside the modules (this process does not execute another
environment's git or filesystem), but store-backed callers ask
`getSshTargetIdForExecutionHost` — "what may this client dial" — so a `runtime:`
row keeps the probe this process has always run for it. Registering and cloning
stay `local` on purpose: those controllers do the filesystem work here, whatever
host id is stamped on the row (see `assertCloneHostIsSupported`).
* perf(renderer): take the English catalog, xterm WebGL addon and emoji data off the boot graph
The renderer's boot graph — the entry chunk plus its 331 modulepreload links,
all fetched and evaluated before first paint — carried three payloads nothing
needs at that moment.
`en.json` (644 KB) was an eager i18next resource, but every renderer string
goes through `translate(key, fallback)` and `en` resolves that inline default,
so most of the catalog was dead weight. The renderer now bundles a generated
`en-runtime-required.json` holding only the 2,583 of 13,828 entries a default
cannot reproduce: plural-suffixed keys, keys whose catalog value differs from a
call site's default, and keys no call site references with a literal default.
`en.json` stays the translator source and the input to the four lazy catalogs.
`@xterm/addon-webgl` (243.6 KB) and `emojibase-data` (170 KB) are now primed
right after the React root renders instead of statically imported. The load
stays eager and `attachWebgl` stays synchronous — it reads the resolved
constructor — so no terminal ever falls back to the DOM renderer for a frame.
`isPluginPanelTabKey`/`isQualifiedPluginKey` move to schema-free sibling
modules, re-exported from `plugin-manifest.ts`. This evicts the plugin manifest
schema graph from the boot chunk but measures ~0 KB, because six other shared
modules still put zod on the boot path.
Boot graph: 332 chunks / 5107.2 KB -> 336 chunks / 4161.5 KB (-945.7 KB, -18.5%).
A new ratchet parses the built index.html and fails if `en.json`,
`@xterm/addon-webgl` or `emojibase-data` is preloaded again; it runs at the end
of every `build:electron-vite`.
* chore(i18n): pin the generated English subset to LF and mark it generated
* fix(i18n): make the runtime-catalog gate merge-robust and prime emoji data in tests
CI builds the merge of a PR with main, so a byte-for-byte comparison against a
committed generated file fails the moment any unrelated PR adds a translate()
call — which is what happened here. The check now asserts the property that
actually matters instead of byte equality: every runtime-required entry is
shipped, and nothing shipped disagrees with en.json. Entries that stopped being
required are dead weight, never a wrong string, so they are reported and
tolerated. Failures now name the offending keys rather than saying "stale".
The generator itself was already deterministic (plain code-unit sort, no
locale collation, order-independent set construction); a test now pins that a
reversed call-site walk produces byte-identical output.
Test fixes for the catalog prune and the deferred emoji load:
- browser-search / NativeChatSupportedAgents asserted key presence on the
renderer's runtime resource. The durable contract is en.json — the renderer
deliberately no longer bundles entries a call site default reproduces — so
they assert against the translator catalog.
- Four emoji tests typed a shortcode in the same tick as mount, before the
catalog the hook primes on mount resolves. Not reachable by a human; the
tests now await the prime.
* revert(renderer): keep the emoji shortcode catalog statically imported
Deferring emojibase-data introduced a window that did not exist before: until
the dynamic import settled, getPrimedEmojiShortcodeEntries returned [], so
exactShortcodeIndex built an empty map and replaceCompletedWorkspaceEmojiShortcode
returned null — leaving a typed `:wink:` in the field literally, and persisting
it as the workspace display name.
Pre-change the shared catalog was statically imported, so the first call at any
tick returned full data. The window is reachable by anything that dispatches
input in the same task as the field's mount effect — Playwright/CDP in the e2e
suite and agent automation both do, and the WorktreeMetaDialog test failure was
exactly that, producing 'Feature 😉' instead of 'Feature 😉'.
Nothing that resolves a shortcode can be async without that race, and a wrong
persisted name is not an acceptable trade for 166.7 KB, so the deferral is
reverted rather than papered over in the tests. The boot-graph ratchet drops
its emojibase-data probe and records why.
Boot graph: 5108.9 KB -> 4329.9 KB (-779.0 KB, -15.2%), down from -945.7 KB.
* fix(terminal): make the deferred WebGL addon load recoverable and refit on late attach
Two defects the deferral introduced, neither possible with a static import.
A failed load latched the DOM renderer for the whole session. `.then(onOk,
onError)` settles fulfilled, so the memoized promise was cached forever with a
null constructor: attachWebgl's re-prime got the cached promise back, and
resetTerminalWebglSuggestion — the documented "GPU setting changed, retry" path
— could not clear it either. The rejection path now clears the memo, latches the
queued panes the way a failed construction does so they retry at a recovery
boundary rather than every frame, and caps attempts so a genuinely missing chunk
is not re-fetched forever. The recovery boundary re-arms it.
The queued-attach drain skipped the refit. Every other late-attach path pairs
attach with a refit because the grid was measured under DOM cell metrics and
WebGL floors the device cell width. Post-deferral, openTerminal's attachWebgl
queued and returned, the initial fit rAF then measured DOM metrics and sized the
PTY from them, and the addon attached with no refit — a persistently narrow PTY
and an unpainted right gutter, not a one-frame flicker. Both paths now go
through one attachWebglAndRefit pairing so they cannot diverge again.
Regression tests cover both, and each was verified to fail without its fix.
The addon-load state machine moves to terminal-webgl-addon-loader.ts and the
viewport presentation helpers to pane-viewport-present.ts, keeping
pane-webgl-renderer.ts under the 300-line budget without a suppression.
`getRecentExpiredSshLease` selected the first `expired` lease matching the pane
coordinates and left eligibility to the caller. Only `recoverTerminalPane` asked,
and it asks id-qualified, where lease identity `(targetId, ptyId)` already makes
the match unique -- so that check could never fire on a lease a different one
shadowed. The two unqualified callers never asked at all:
`workspaceSessionWorktreeHasRuntimeOwnedPtyCandidate` and
`hasRecentExpiredSshLeasePane` both take a bare `!== null`.
`(worktreeId, tabId, leafId)` is not unique. `supersedeSiblingLeasesForPane`
exists because a pane accumulates leases as it re-leases under new relay ids, and
it stamps `supersededBy` on an already-expired predecessor precisely so the
predecessor stops counting. Inside the 30s SSH_PANE_RECOVERY_GRACE_MS window
those two readers still counted it: a pane whose only recent lease is a
superseded or relay-id-recycled corpse was reported as runtime-owned and
preserved for recovery, and `recoverTerminalPane` then refuses it. Where an
eligible successor also exists, the predecessor is stored first and shadowed it.
Apply the existing `sshRemotePtyLeaseAllowsReattach` inside the selection, so the
reader answers with the first ELIGIBLE orphan or nothing, and all three callers
agree on what `expired` authorizes. `recoverTerminalPane`'s own check becomes
unreachable and is folded into the comment on the branch that now covers it.
Scope: an over-report in headless/mobile reconciliation, not a wrong-route
readoption -- the id-qualified recovery path already refused these leases. No
wire change and no host-semantics change: `expired` still says only that the
client lost its route, and nothing here asserts a remote shell died.
Coverage lives in a `*.test.ts`: config/vitest.config.ts, the config CI runs,
includes only `*.test.ts`, so the orca-runtime-tests/*.spec.ts neighbours would
never execute.
* docs: add SSH agent identity implementation plan
* feat(ssh): host-stamped remote foreground identity
* fix(runtime): preserve unfenced inspect call shape
* perf(ssh): traverse foreground descendants linearly
* fix(ssh): bound retired PTY evidence records
* test(ssh): cover retired incarnation retention
* fix(ssh): make remote process inspection total
* Split SSH identity build hot spots
* Fix process table snapshot module split
* test(ssh): update process inspection expectations
* docs: drop the SSH identity plan from the PR
The design doc does not belong in the product repo; it stays out of the
shipped tree while the implementation carries its own comments.
---------
Co-authored-by: Merge Sim <sim@local>
* perf(editor): stop reclassifying the whole markdown document on every render
EditorPanel re-renders from ~18 store subscriptions, and the rich-mode
classifier ran unmemoized in its render body — so an idle git-status poll
rescanned (and TipTap round-tripped) every open markdown tab.
- Memoize `getMarkdownRichModeEligibility` on (content, sizeOverridden).
Idle 60 s with one markdown tab: 31 -> 1 classifier calls, 31 -> 1
round-trip calls. Render-model self time for a 100 KB .md with HTML:
6.485 ms -> 0.004 ms per render.
- Drop the React effect that mirrored `content` into the doc-link decoration
refresh; the controller's own `onDidChangeModelContent` listener already
covers it (and catches programmatic edits). 800 -> 400 debounce timer ops
per 200 keystrokes, same decorations.
- Scan doc-link decorations by line offsets instead of per-line substrings,
reusing the allocation-free `forEachLine` from the conflict decorations.
600 KB / 46k lines: 50,853 -> 4,623 string allocations per scan,
34.1 ms -> 20.3 ms per scan.
- Replace the per-keystroke double `trimEnd()` dirty check with a
trimmed-length probe plus one native prefix compare: 400 -> 0
full-document copies (40 MB -> 0 bytes) per 200 keystrokes. Move
fileContents/diffContents behind refs so the change callback identity
stops churning on every content load.
No behavior change: rich-vs-source selection, decorations, and the dirty
dot are all covered by equivalence tests against the previous code.
* fix(editor): move the dirty-check content refs out of the render body
React Doctor's `no-ref-current-in-render` flagged the `fileContents` /
`diffContents` ref writes added for the stabilized change callback, and it
was right on substance: a render React discards would still have moved the
dirty-check baseline, so a later keystroke could be compared against content
from a render that never committed.
Assign both refs in a `useLayoutEffect` instead — the same latest-value
pattern `useIpynbDocumentEditing` already uses. Layout effects only run for
committed renders and land before any input event can reach the handler, so
the baseline is always the committed one.
The handler and its refs move into `use-editor-content-change-handler.ts`;
that keeps `EditorPanel.tsx` under the 400-line cap (no `max-lines` bump) and
puts the draft write, the dirty comparison and their inputs in one place.
Callback identity stays stable (1 distinct identity across 30 idle renders)
and every measured number is unchanged: 1 classifier call and 1 round-trip
call per 60 s idle, ~0.005 ms render-model self time for a 100 KB .md with
HTML. Also asserts the reverse direction of the reload case — the stable
handler marks the file dirty when handed the pre-reload content.
* fix(editor): keep the rich-mode fallback banner localized behind the memo
The memo keyed on `(content, sizeOverridden)`, but the value it cached was
not a pure function of those two: `unsupportedMessage` comes from a matcher
`get message()` accessor that calls `translate()` at access time, so the
active UI language is a third, ambient input. Caching the resolved string
froze the banner in whichever language was active at first classification —
visible when switching Settings → UI language, and at startup for non-English
users because `I18nProvider` applies the persisted language from an effect,
after the settings-driven render has already classified.
Adding the language to the cache key would only work until the next ambient
input. Instead, split the classifier: `getMarkdownRichModeEligibilityDecision`
returns the genuinely pure part (`exceedsSizeLimit` plus which matcher fired)
and is what the cache stores, while `resolveMarkdownRichModeUnsupportedMessage`
reads the matcher's getter per read. `getMarkdownRichModeEligibility` keeps its
old signature as a thin composition of the two.
Cost of re-resolving per read is one `i18n.t` on documents that show a banner
and nothing at all on documents that do not (a null reason short-circuits).
Render-model self time for a 100 KB .md with HTML moves 0.005 ms -> 0.011 ms,
against a 6.485 ms pre-PR baseline; classification still runs once per content
change (1 decision and 1 round-trip across 30 idle git-status ticks).
Two regression tests, both confirmed to fail against a string-caching cache:
a unit test asserting a cache hit follows `changeLanguage('ja')`, and an
EditorPanel test that renders the banner from a reference-link document,
switches the language, forces an idle store write, and asserts the Japanese
text while the decision stays cached.
The workspace-cleanup scan threaded `provider: IGitProvider | null`, derived from a
raw `repo.connectionId` read, through listing, activity and git evidence. That `null`
spelled "this is local", "the host is remote but unreachable" and "the host is a
runtime environment" with one value, so a row naming its owner only as
`executionHostId: 'ssh:<target>'` listed worktrees, statted paths and ran `git status`
for a *remote* checkout on this client (#11163).
The three sites had to move together: the `provider!` assertions in
workspace-cleanup-git-evidence.ts were sound only because they re-read the same field
that workspace-cleanup-worktree-listing.ts used to decide whether `provider` was
populated. Migrating one alone turns them into crashes.
Routing now goes through the shared resolution layer -- `getRepoExecutionHostId` for
the repo that produces the listing, `getWorktreeExecutionHostId` for the workspace's
own host -- into `resolveGitRouteForHost` from #18296's host-keyed dispatch. The
ambiguous carrier is removed rather than supplemented, so every reader became a
compile error; unlike #18325's family, workspace-cleanup carries no `@ts-nocheck`, so
that guarantee is real here.
`runtime:<env>` is not a route variant. Its Git runs on that environment's own server
and the SSH target on its repo row is that server's nested one, addressable only as
(environmentId, targetId); handing it to this client's SSH table dials a same-named
target in the wrong namespace. It throws, matching workspace-space-repo-scan and
repos:listForExecutionHost.
No wire change: `WorkspaceCleanupCandidate` (including `connectionId` and
`executionHostId`) and the workspaceCleanup RPC UI-state schema are untouched.
* perf(browser-pane): share one visibility-gated rAF loop across client-hosted page overlays
Each shown client-hosted browser page started its own permanent, ungated
requestAnimationFrame loop whose callback forced a layout flush via
getBoundingClientRect, so N shown hosts cost N loops forever, including
while the document was hidden.
Registers every host with one shared driver instead: one rAF callback per
frame syncs all registered hosts, the loop starts on the first registration
and stops on the last, and it pauses while the document is hidden with an
immediate resync of every host before it resumes.
* fix(browser-pane): drop the hidden-document gate from the shared overlay position loop
Measured on Electron 43 / macOS: a hidden, minimized or fully occluded window
reports visibilityState 'hidden' AND already runs 0 rAF callbacks/s, so the gate
saved nothing. Its only live effect would be in the wedged-occlusion state the
renderer already works around, where it would freeze every overlay for good.
Also release a retained page's position sync when the registry tears the page
down, instead of waiting for the pane's own detach that may never run.
* fix(browser-pane): isolate a throwing host from the shared overlay position loop
One loop now serves every shown client-hosted overlay, so an exception from any
single host's viewport sync escaped runFrame before it rescheduled and stopped
every other overlay tracking its pane, with nothing left to restart it: a pane
that merely moves fires no resize or scroll event.
Each sync now runs isolated and the reschedule is unconditional, so a failing
host is skipped and reported once instead of sixty times a second.
Merge Sim
Merge Sim
Jinjing
Neil
Jinwoo Hong
Brennan BensonandMerge Sim