mirror of
https://github.com/stablyai/orca.git
synced 2026-10-27 16:02:42 +00:00
6be9dc43e1d8e57a60d4647bee0ab6f7b10e2fb0
8164
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Jinwoo-H
|
5359ddff97 |
Merge remote-tracking branch 'origin/main' into mobile-rearch
# Conflicts: # mobile/src/session/use-mobile-native-chat-image-attachments.ts |
||
  Brennan BensonandMerge Sim
|
c16913ab65 |
fix(native-chat): clarify active progress (#18705)
Co-authored-by: Merge Sim <sim@local> |
||
 Neil
|
d3501f7ad6 |
fix(worktrees): stop a failed worktree scan from being recorded as an authoritative empty listing (#18456)
* fix(worktrees): stop a failed worktree scan from pruning as an authoritative empty listing A `git worktree list` that could not run at all — a WSL distro that stopped resolving, a hung mount, a git binary that errored — was softened to `[]` by the lenient listing path, so the detected scan published it as `authoritative: true, worktrees: []`. That disables the #1158 retention guard, drops every persisted tab for the repo, and the pruned session is written back to disk on the next launch. The loss is permanent, not a transient glitch. Route the detected scan through a strict listing that still reports the two genuinely empty states (repo path gone, not a Git repo) as `[]`. Everything else rejects, so the existing catch answers `authoritative: false` and the destructive halves (`rememberLocalWorktreeRoots`, `pruneLineageForMissingRepoWorktrees`) never see a failed scan. Also make the failure readable: wsl.exe reports its own launch failures as exit 0xFFFFFFFF with an EMPTY stderr and the `Wsl/Service/WSL_E_*` line on stdout as UTF-16LE, which is why the field bundle carried a git error with no text. Set WSL_UTF8 for WSL-routed git (matching the wsl runner, #9010) and attach that stdout diagnostic to the error so `git.exec` spans name the cause. * fix(worktrees): surface a failed worktree scan's cause on the repo header with a retry A failed scan now travels with its reason (optional unavailableReason on DetectedWorktreeListResult), the repo header shows it with click-to-retry, and the WSL deleted-guest-directory shape measured on a real Windows host is pinned as retained-not-pruned. |
||
|
Neil
|
901c6771ff |
fix(agents): detect agent CLIs installed outside a version manager (#18336)
* fix(agents): detect agent CLIs installed outside a version manager The install-dir fallback that answers "is this agent installed?" when the login-shell PATH probe fails listed only version-manager bin dirs, so codex/opencode/cursor-agent installed by Homebrew, npm's default global prefix, snap, nix, or the CLI's own installer read as not installed. Where this is decisive, corrected from the previous message: the `orca` CLI, whose detectSkillsCliAgentKeys (src/cli/handlers/skills.ts) calls detectCommandsInInstallDirs directly and whose entry point seeds no PATH, plus unpackaged/dev runs. NOT the packaged desktop app: patchPackagedProcessPath (configure-process.ts:114) appends /opt/homebrew/bin, /usr/local/bin, ~/.opencode/bin and the Linux/nix prefixes onto process.env.PATH at main-process-preflight.ts:147, before any detection, and mergePathSegments preserves them, so the PATH scan reaches those dirs first and this fallback never fires for them. That means this does NOT explain the packaged macOS v1.4.194 report of codex/cursor-agent/opencode all undetected -- that report stays open and uninvestigated. Second correction: the fallback now carries the prefixes Homebrew actually uses on Linux (/home/linuxbrew/.linuxbrew/bin), plus /snap/bin and the two nix profile dirs, matching what patchPackagedProcessPath already seeds. The WSL guest prelude gains the same entries. Leaving them out closed the native/WSL asymmetry on darwin only, on the platforms where the fallback is decisive. Appended last so a version-manager install still wins, and kept out of getVersionManagerBinPaths, whose result is PREPENDED to PATH (#18234). Lives in its own module so node-cli-command-resolution.ts stays under max-lines. The test stages every path through `join` and asserts via detectCommandsInInstallDirs as well as resolveCliCommands, so it holds on a Windows dev machine and pins the "absolute path means installed" contract. * fix(agents): align system install-dir order across the three PATH lists Round-2 review remediation. The blocking finding was about the handoff artifact, not the code: the summary handed to review described a 2-file/+38 change with 3 new macOS dirs, while HEAD is 4 files/+227 with 6 lookup dirs plus 5 new WSL-guest prelude entries, and the quoted failing test names never existed. Restated against HEAD in the handoff; no rebuttal, the reviewer was right. Justification for the entries the summary never described: the fallback exists to close the native/WSL asymmetry for a CLI no version manager installed, and patchPackagedProcessPath already seeds Linuxbrew, /snap/bin and both nix profile dirs (configure-process.ts:141-158). Shipping only the darwin subset would have left a Linux or WSL user with a snap/nix install still reading as not installed while the packaged macOS user did not -- the asymmetry the change is for. Code changes, all from the non-blocking list: - The three lists disagreed on order while claiming to be kept in step, so a CLI in both /usr/local/bin and /snap/bin could resolve to a different binary than the seeded PATH scan or the WSL guest probe found. All three now use the seed's relative order, pinned by a new duplicate-install test and an offset assertion on the prelude. The prelude's system block also moved after the nvm glob so a version manager still wins in the guest, as it does natively. - The parity docstring asserted "the same set patchPackagedProcessPath appends, minus the sbin dirs and the generic ~/bin", which was false: it also omits ~/.vite-plus/bin (seeded by configure-process.ts:156, but no probed agent command maps to it) and /opt/homebrew off darwin. All three gaps are now named as deliberate. - win32 returns [] and stays that way, but the branch now says why: %USERPROFILE%\.opencode\bin has never had install-dir coverage in either list, and the seed's system block is POSIX-only too. Pre-existing, unchanged. - The detectCommandsInInstallDirs case read the ambient process.env.PATH, so on a box with /usr/local/bin on PATH only the opencode assertion exercised the fallback. It now stubs the GUI-launch PATH, so both do. Unchanged and restated: this does NOT explain the packaged macOS v1.4.194 report of codex/cursor-agent/opencode all undetected, and must not close G6 report #3. patchPackagedProcessPath returns early unless app.isPackaged and seeds all six dirs before any detection, and mergePathSegments never deletes them, so the packaged PATH scan reaches them first and this fallback never fires there. Decisive only for the `orca` CLI's detectSkillsCliAgentKeys and unpackaged/dev runs. Verification: the suite is red without the production hunks (5 failed/3 passed) and green with them (8 passed); 11 related suites pass (197 tests); tc:node, tc:cli, oxlint, oxfmt and the max-lines ratchet are clean. * fix(agents): correct the ordering-parity claims and widen the seed-leak guard Round-3 review found two docstring claims that are false as written and one guard that only asserted a third of its list. - system-cli-install-dirs.ts claimed a CLI in two of these dirs resolves the same here as in the packaged PATH seed. True inside the system block, false across it: `claude` in both ~/.local/bin and /opt/homebrew/bin resolves to ~/.local/bin via the fallback (getBaseVersionManagerDirectories leads) and to /opt/homebrew/bin via the seed, which appends ~/.local/bin last. Scope the claim to the block and name the gap instead of asserting it away. No behavior change: closing it would hoist a system dir over a version-manager one (#18234). - posix-version-manager-bin-dirs.ts justified moving "/usr/local/bin" after the nvm glob with "a version manager still wins in the guest, as it does natively". The glob expands lexicographically; native orders nvm dirs default-alias-first (#10932), so that is not parity. Record the move as the one behavior change in the file and bound it: entries are appended behind a resolved login PATH, and both consumers only test presence. - The #18234 seed-leak guard asserted only /opt/homebrew/bin and /usr/local/bin were absent from getVersionManagerBinPaths, leaving the four other new dirs unpinned. It now spells out all seven across darwin and linux -- spelled out rather than derived from getSystemCliInstallDirectories, which would pass vacuously against exactly the refactor it guards. Tests 8 passed (8); 5 failed / 3 passed with the production hunks reverted to origin/main. tsc node + cli clean, oxlint clean. * fix(agents): find Pi's own installer dir in the CLI install-dir fallback The fallback added `~/.opencode/bin` but skipped `~/.vite-plus/bin` on the claim that no probed agent command maps to it. False: `pi` is a probed detect command on every runtime (`tui-agent-config.ts`, no `detectUnsupportedRuntimes`) and `~/.vite-plus/bin` is the Pi installer's default — the two dirs #829 named and `patchPackagedProcessPath` seeds together. Added to both the native fallback and the WSL guest prelude, so a Pi installed by its own script is found by the `orca` CLI and in WSL, not just on a seeded packaged PATH. Also, all narrower: - `/snap/bin` + Linuxbrew now gate on `linux` like the seed does, instead of every non-darwin posix. - Docstring: `/opt/homebrew` off darwin is the one remaining seed gap and says why; the "lookup-only" paragraph names the `withCliRuntimeOnPath` exception. - New seed-order test derives the expected order from `getSystemCliInstallDirectories`, so reordering either list fails. The PR body's claim that SSH hosts benefit is dropped: they answer `preflight.detectAgents` from `src/relay/preflight-handler.ts` via `isCommandOnPathForRelay`, a separate bundle that never reaches this module. * fix(agents): build the CLI install-dir fallback order once and pin it on both resolvers resolveCliCommand (every spawn site) and resolveCliCommands (detection) each spelled the nvm -> version-manager -> system-dir order by hand, which is how the native and WSL lists drifted apart before. One getCliInstallDirectories now feeds both, and the test pins system dirs LAST on both resolvers, on darwin and linux, plus a derived check that the WSL guest prelude keeps every native version-manager dir ahead of the native system block. |
||
|
Brennan BensonandMerge Sim
|
f4c2821167 |
refactor(agent-session-journal): move the session journal onto SQLite (#18652)
* refactor(agent-session-journal): move the session journal onto SQLite The agent-session journal kept its state in three hand-rolled file formats: an append-only `log.jsonl` with torn-tail repair, a `snapshot.json` holding folded state plus a retained tail, and byte-quarantine files for anything unreadable. This replaces all of it with one SQLite database per session — `journal.db` beside the existing `blobs/` store — using the in-house adapter and the open/pragma/migrate/harden pattern the orchestration database already follows. Two tables: `journal_rows` (the append-only log, keyed by `(session_id, epoch, seq)`) and `journal_sessions` (the derived projection, upserted in the SAME transaction as every insert). Rows stay JSON in one column, so the row schema, the version upcast chain, and the reducer survive byte for byte — `journal-reducer.test.ts` and four other suites pass unchanged and are the regression proof. Deleted: `journal-log-file.ts`, `journal-compaction.ts`, `journal-corruption-quarantine.ts`, and the public `compact()` / `compactionBoundary` / `autoCompact` members, none of which had a non-test caller. Existing `log.jsonl` / `snapshot.json` journals are deliberately abandoned. No importer: a session created on the old path stops working, which is acceptable because the feature is off by default. ## The physical quota is repriced, because SQLite does not charge like a file The 256 MiB per-session bound is unchanged, but the arithmetic under it could not survive: SQLite grows the database in pages and the WAL in frames, and the checkpoint that copies the WAL forward holds the same pages in both files at once, so a transaction's peak is about twice its content. Admission now charges the candidate transaction's own measured page cost, validated against a sweep that runs as a regression test (`journal-database-space.test.ts`) rather than derived from reasoning about the allocator. Four things are load-bearing rather than tuning, each measured: - `auto_vacuum = INCREMENTAL` must be set BEFORE `journal_mode = WAL`. Set it after and it is ignored with no error, reclamation silently becomes a no-op, and the file never shrinks again. Both halves are asserted. - `wal_autocheckpoint = 0` plus an explicit `wal_checkpoint(TRUNCATE)` at the end of every write path, so the one moment the same pages live in two files is a moment the charge accounts for. - Reclamation runs in bounded chunks. A single unbounded `incremental_vacuum` took a 252 MB directory to 504 MB — the reclamation added to defend the bound would have breached it. `PRAGMA incremental_vacuum(N)` also frees exactly one page unless it is stepped to completion, which no size assertion catches, so the freed page count is asserted directly. - A blocked checkpoint leaves the WAL on disk together with the database growth it already copied, so admission charges that deferred copy explicitly. The term is zero whenever the last checkpoint succeeded, so the uncontended path admits and refuses an identical set. The epoch discard is `DELETE FROM journal_rows` with no WHERE clause, which takes SQLite's truncate optimization: measured at ~0.26% of the database in WAL bytes where the `WHERE session_id = ?` form rewrote every emptied leaf at up to 99%. One database per session is what makes the unqualified form correct. An open, empty journal costs 57,344 bytes before a single row exists, so a configured quota below `JOURNAL_MIN_SESSION_BYTES` now fails loudly at open with the existing `journal_bound_exceeded` instead of as a run of identical append failures. No production caller configures one; the affected surface is test fixtures, rescaled to the smallest value that restores what each case proves. ## One deliberate behaviour change Compaction was the only mechanism that shed bytes inside an epoch, and the write path called it precisely so an append at the bound was not refused. The SQLite-shaped replacement — a bounded prefix delete — cannot be used: with the snapshot gone the surviving rows ARE the state, so dropping the oldest of them loses the oldest transcript silently at the next reopen. So no row is ever shed inside an epoch, and a session whose row bytes alone reach the bound now refuses every append where it previously compacted and continued. A loud typed refusal beats silent data loss. What still sheds is unreferenced BLOB bytes — the dominant and unbounded byte source — on the same write-path hook. The escape from the hard stop is the fold that already exists, `replaceEpochItems`, which now actually returns bytes to the filesystem instead of leaving them on the freelist. The prune's protected set is a union of live reducer digests AND the candidate row's own digests, including those cited only by a nested lifecycle-batch mutation. Content addressing never rewrites a digest already on disk, so protecting live state alone deletes the blob the append is about to cite — a dangling reference that surfaces one reopen later as an empty expansion on an item the user can see. `journal-store-blob-budget.test.ts` pins it, and it goes red when the set is narrowed back. ## Handle ownership A file handle used to be opened and closed per append; a SQLite handle is held for the session's lifetime. Every path that can open a connection now has one owner: the open function owns its raw connection until it returns, the store owns its retained one and releases it in a new `close()`, and every other connection is closed by the call that opened it. The attach, recovery, eviction, map-overwrite and host-teardown paths close what they drop, and host teardown is failure-complete — the sink-barrier flush throws by design, so a trailing close statement would be skipped on exactly the path that leaks. `close()` has a stated contract: admission at enqueue and permanent, the close step on the same queue past that gate, one shared in-flight attempt, fulfilment terminal, and the release last and deliberately unguarded so a retry re-enters it. Guarding the release would skip it on retry, guaranteeing a permanent leak in exactly the case where it did not release. `journal_closed` joins the error union for a write after `close()`; no file outside the directory references any of these codes. * fix(agent-session-journal): make a COMMIT final, stop repairs deleting valid rows, and keep rejected closes retryable Six review findings on the SQLite journal migration. 1. A successful COMMIT is now the point of no return. The ordinary append, the epoch roll and the epoch replacement each adopt the committed row or epoch BEFORE any post-commit filesystem work; checkpoint, reclaim, blob prune and directory measurement run through `runJournalPostCommit`, which is best-effort by design and falls back to the transaction's own charge as a conservative footprint. Previously a post-COMMIT scan failure rejected a durable append and the next one reused its sequence, and a failed epoch housekeeping step left the store writing into a prefix already deleted. 2. Corruption repair preserves instead of destroying. A rejected suffix is copied into a new `journal_quarantine` table and removed from the live epoch in ONE transaction per chunk, charged against the session bound before a byte is written; a journal that cannot afford the copy refuses to open rather than falling back to deletion. The repair state is exposed as `journal.repair` and the rows are readable through `recoverQuarantinedRows()`, so Orca-owned submission, receipt and lifecycle identity survives a gap or a malformed row. 3. The physical charge covers the B-tree key payload. `session_id` and `epoch` are stored in both tables and both primary-key indexes and appear nowhere in `row_json`, so the journal boundary now bounds them and `journalTxnPhysicalCost` charges those bounds plus the projection upsert. The charge sweep runs the exact production transaction at maximum admitted key sizes. 4. A rejected `close()` no longer orphans its handle. Callers hand the journal to `agentSessionJournalCloseRetries` instead of swallowing the rejection, the attach map replacement is ABORTED when the previous journal will not close, host teardown retries what the registry holds, and a failed runtime teardown is retained so the next stop is a real retry. 5. `journalWalBytes()` returns zero only for ENOENT and propagates every other stat error, so admission and reclamation fail closed. 6. The WAL contention test closes the writer before removing its temp root and asserts the directory is removable once handles close. Regression coverage: post-commit divergence (4), corruption repair (5), key bounds (5), WAL stat (8), close retry (5), plus a runtime stop-retry case. Each fix was ablated on this head and the matching tests go red. * fix(agent-session-journal): anchor replay at sequence 1, make quarantine append-only, and charge it in bytes Three ways the corruption quarantine still lost rows it was written to keep. Replay validated contiguity from the first row that HAPPENED to remain, so an epoch missing only its sequence-1 row declared the leftovers contiguous and set no `truncateFrom`. The load was still corrupt, so recovery imported provider history and `replaceEpochItems` deleted every live row — including Orca-minted submission, receipt and lifecycle identity that no transcript can reconstruct, and that nothing had quarantined. Replay now anchors at sequence 1, so a missing epoch row rejects the whole surviving range before any replacement runs. `journal_quarantine` was keyed on `(session_id, epoch, seq)` and copied with `INSERT OR REPLACE`. A repair frees the sequences it removed and the live epoch reuses them, so a second repair in the same epoch silently deleted what the first preserved. The table is now keyed on a surrogate `quarantine_id`, the copy is a plain append, and `(epoch, seq)` is metadata; existing v1 databases are rekeyed in the migration that already bumps `user_version`. The admission charge read `length(row_json)`, which counts CHARACTERS for a TEXT value where `journalTxnPhysicalCost` expects physical UTF-8 bytes. A multibyte suffix was charged at up to a third of what it writes, which defeats the pre-write physical bound — over a megabyte on a maximum-size lifecycle batch. * fix(agent-session-journal): keep a repaired epoch anchored and stop the v1 quarantine migration doubling the file Replay validated numeric contiguity from sequence 1 but never that sequence 1 IS the epoch row. When the anchor was missing the repair set aside every surviving row, and if provider-history import then failed — a transcript that is temporarily gone is enough — the journal reopened as a clean, row-less epoch: an ordinary append took sequence 1, replay accepted it, read-restore published it as history, and automatic recovery never ran again while the user's real messages sat in quarantine. Replay now rejects an unanchored prefix, the open publishes an `unreconcilable_prefix` anchor for an epoch its repair emptied, and that anchor keeps reporting corrupt — so provider history is retried on every attach — until the timeline is rebuilt or the session writes content of its own. A repair also discloses rows it set aside when no line was unreadable at all, which is the case that removes the most. The v1 quarantine rekey copied every legacy row into the new table inside one transaction and dropped the old one. A quarantine holds whole rejected rows: a single 8 MiB row nearly doubled the database past the physical bound the open had already checked, the dropped pages only reached the freelist, and the next open refused the session it had just migrated. The v1 table is renamed and frozen instead, and reads take both generations. Table creation also moves inside the migration transaction, so a crash can no longer leave a v2-shaped database still reporting version 0 for an older build to write into. * fix(agent-session-journal): stop an empty provider transcript retiring the repair marker A transcript that exists but decodes to zero messages was imported as a success: the import published an empty `legacy_import` replacement that deleted the `unreconcilable_prefix` anchor and its disclosure, so the next probe read the session as clean and every later attach skipped provider recovery while the user's rows sat in quarantine for good. The import now leaves the epoch untouched when nothing decodes, reporting `replaced: false`, and recovery treats that like a transcript it could not read — the marker stands and a later attach with real history rebuilds the timeline. * style(agent-session-journal): merge the duplicate journal-database-space import * refactor(agent-session-journal): drop quarantine, byte bound, blob spill and rate limit Match what comparable implementations do: the journal is an unbounded append-only SQLite log with no side tables and no admission control. Corruption: the rejected suffix is DELETED rather than copied into a quarantine table. The load still reports `corrupt` and recovery still rebuilds the epoch from provider history, so the observable outcome is unchanged — only the preservation half is gone. The schema is back to one version with two tables; no v1 database exists outside unmerged commits of this branch, so the rekey migration and the two-generation read path go with it. Sequence-1 epoch anchoring and the empty-provider-transcript retry are kept: both are about the corrupt signal being correct. Size: no `maxSessionBytes`, so no page-cost arithmetic, reclaim band, incremental vacuum, lifecycle byte reservations or `journal_bound_exceeded`. `auto_vacuum` and `wal_autocheckpoint = 0` existed only to make a transaction's physical cost predictable for that charge; with the charge gone SQLite's default checkpointing is what the journal wants, and the explicit pre-close checkpoint is redundant with the one `db.close()` performs. WAL, `synchronous = FULL` and `busy_timeout` stay. Payloads: an oversized body is truncated at the existing inline cap with the existing marker and the remainder is discarded, bounded at the translation layer that already calls these helpers. The truncation point and message do not change; the content-addressed blob directory and all digest tracking do. Rate: no `maxAppendsPerWindow` and no `journal_rate_exceeded`. `JournalPayloadLimits` is now just the inline cap. * fix(agent-session-journal): mark a partial repair pending and bound multi-block tool input A repair that keeps its prefix had nothing durable to show for the suffix it deleted: a sequence gap costs no malformed row, so no disclosure is appended, and the surviving rows keep their epoch anchor. The next probe read a contiguous anchored prefix, called it clean, and the deleted stretch of timeline was never asked for again — silent loss, with the deletion already committed. The deletion now writes a `journal_repairs` marker in the SAME transaction, and replay keeps reporting corrupt while it stands. It retires under exactly the rule the emptied-epoch anchor takes: a fresh epoch carries the rebuild, or the session writes content of its own past the sequence the repair left free. The repair's own disclosure is not that content. Legacy import bounded a tool call's input only when it was the message's sole block; the multi-block path returned `tool-call` unchanged, so a mixed message from Claude, Grok or an omp execution cell persisted the whole input despite `inlineHeadBytes`. `boundBlock` now routes it through `boundToolInput`. Also drops canonical comments describing quarantine, snapshot files, blob storage and blob compaction — none of which exist any more. * fix(agent-session-wire): stop awaiting the synchronous journal probe loadJournal runs on a sync-database connection and returns JournalLoad | null, so both wire call sites were awaiting a non-Promise. The type-aware code-quality gate flags it; the native gate does not. --------- Co-authored-by: Merge Sim <sim@local> |
||
|
Neil
|
92456b1618 |
perf(ports): serve unchanged macOS listeners from remembered metadata (#18650)
* perf(ports): serve unchanged macOS listeners from remembered metadata The workspace port scan shells out three times every 30s while the window is visible. Profiling an idle app showed the cycle costs 0.67s of CPU — about 2.2% of a core sustained, roughly 65% of Orca's total idle CPU — and two of those three commands only re-derive the command line and cwd of listeners that have not changed since the last scan. Reuse the metadata already remembered for a listener when this scan's free lsof -F c process name still matches, so a recycled pid re-probes instead of inheriting a dead process's attribution. In steady state that drops the cycle to the single listening scan. * perf(ports): pin cached listener metadata to its socket and re-probe every tenth scan The metadata cache trusted an entry on pid:host:port plus the lsof -F c process name, so a recycled pid running the same command name on the same port would inherit a dead process's command line and cwd, and a process that chdir'd while listening kept its old cwd forever. Add the socket's kernel identity (lsof -F d) to the already-running listening command — measured at no spawn and no wall-time cost — and require it to match before reusing an entry: a restarted process listens through a new socket. Also re-probe every remembered entry every tenth scan (~5 min) so a changed cwd cannot go stale indefinitely. Steady state stays at one command per cycle for nine of every ten scans. * docs(ports): note the ordering contract between the reuse split and the remember step * fix(ports): never authorize a kill from remembered listener metadata requireMetadata is the SIGTERM authorization re-scan, and its own comment says it must never land on a cycle that skipped owner attribution. The metadata cache could serve it entirely from memory, so a process that had chdir'd out of a worktree could stay attributed to it for up to the re-probe ceiling and be killed on Stop. That caller now always probes; background polls still cache. * fix(ports): re-derive every port of a pid when any one of them needs metadata A process listening on several ports could have one row served from remembered metadata and another from a fresh probe, so a process that moved between scans reported two different workspace attributions until the next full re-probe. Decide per pid first, then hydrate only pids that need no probe at all. * refactor(ports): let the cache module own the requireMetadata bypass Keeps the scanner under the max-lines gate and puts the cache policy with the cache rather than at the call site. |
||
 Jinwoo Hong
|
5083b58b9c |
fix(desktop): never replay a refresh token after a timeout; jitter relay lease renewal (#18719)
Two independent desktop hardening changes that share one incident date.
1. Refresh-token replay. A refresh POST that aborts client-side leaves the
token's fate unknown: the server may already have rotated it before the
reply was lost. Every caller above this module (the relay auth coordinator
most of all) then re-entered and resent the same stored token, which the
server reads as reuse and answers by revoking the whole token family. On
2026-09-04 that turned a slow refresh endpoint into 21,605 sign-outs.
- The refresh endpoint now gets one 60s attempt instead of a 30s attempt
plus a replayed retry.
- A failure with no status line is ambiguous: it is never retried, and the
token is recorded so a re-entry within 30s is refused outright rather
than replayed. The block is bounded, not permanent -- the token is only
possibly spent, and a permanent block would sign out every desktop whose
refresh merely timed out.
- Before any retry or replay the stored session is re-read; if another
caller already rotated it, that result is adopted and the old token is
never sent again.
- Only a definitive 5xx, which proves the server rejected without
rotating, is retried, and exactly once.
- A 401 on a token whose earlier attempt never answered now logs
orca_cloud_refresh_possible_replay so support can tell a real revocation
from a sign-out we caused ourselves. clearCloudSessionIfUnchanged
semantics are unchanged and still emit the invalidation event.
2. Lease renewal jitter. Both desktop renewal timers took the mean of a
60s-wide window before expiry. A cell recreate reconnects a whole cohort
inside one second, so every host in it renewed inside the same second ~54
minutes later, re-bursting the fleet every ~54 minutes. Renewal now
carries full +/-10% jitter, spreading the same cohort over ~10 minutes.
Early renewal is free on the server side: the rebind branch resets the
full 55-minute TTL from whenever it arrives, with no minimum-age or
early-renewal restriction (cloud/apps/relay/src/host-session-registry.ts
:736-743, and :794 for a fresh session). Only lateness is fatal -- :997
drains and closes a lease that has expired. The base interval is therefore
shrunk to fit the upward jitter rather than clipping the jittered value at
the margin, which keeps the distribution unbiased and keeps every sample at
least 90s before expiry. No wire change.
|
||
|
Neil
|
b2c6f029ef |
perf(settings): commit free-text account settings on a debounce, not per keystroke (#18651)
* perf(settings): commit free-text account settings on a debounce, not per keystroke Four raw text inputs bound value straight to the store and called updateSettings in onChange, so every character was an IPC round trip that replaced the settings object identity in every other window, re-rendering everything subscribed to it. Route them through a DebouncedSettingsTextInput that keeps a local draft and commits after 700ms, on blur, and on unmount — matching the repository-hook script draft. The draft lives in the input component because the account sections are render functions the settings search calls conditionally, so hooks cannot legally live in them. * fix(settings): update the draft's latest-commit ref in an effect, not during render * fix(settings): flush a pending text-setting draft on beforeunload and drop the dirty flag A window close or app quit never unmounts the React tree, so the unmount flush could not run and a value typed within the last 700ms was lost. The close coordinator already dispatches a synthetic beforeunload while the tree is mounted, so listen for it the same way the session checkpoint does. The pending timer is now the single source of truth for "uncommitted edits"; the separate dirty flag it duplicated is gone. |
||
|
Neil
|
368a6d6ca7 |
perf(terminal): use real event-loop yields between chunked writes (#18627)
* perf(terminal): use real event-loop yields between chunked writes Both chunk-write loops yielded with setTimeout(resolve, 0), which is not a yield but a timer tick. The paste executor's awaits are nested, so Chromium clamps them to ~4ms each; a 4MB paste is ~256 chunks and up to a second of dead time. The main-process pty:write loop pays a Node timer round trip per 16KiB. Use the shared MessageChannel-based yieldToEventLoop in the renderer (already used by pty-input-write-queue one file over) and setImmediate in main (already used by the remote-runtime terminal.send path and the usage scanners). * test(terminal): pin the event-loop yield between chunked PTY writes The paste executor must default to the shared yieldToEventLoop helper and write-input must yield via setImmediate; neither was covered, so a refactor back to setTimeout(0) would have passed every suite. Both tests are deterministic: the renderer test mocks the helper module and spies on setTimeout with the operation timeout disabled, and the main-process test fakes only setTimeout so a timer-based yield stalls while a setImmediate yield races to completion on real check-phase turns. |
||
|
Neil
|
9008fb70a6 |
perf(sidebar): share one natural-worktree-id scan and drop a redundant row-key join (#18647)
The natural-worktree-id Set was spelled out four times — twice in the drag groups module, once in the drag session hook, once in the drag units module — each as rows.flatMap(row => cond ? [id] : []), which allocates a throwaway array per row. All four memoize on the same rows, so they recompute together. Share one loop-based helper. use-row-measurement also built a joined string of every row key purely as an effect dependency, alongside a Set memoized on the same input. A fresh Set always changes identity, so the string could never fire the effect on its own. |
||
|
Neil
|
2881955bae |
perf(agent-hooks): stop cloning the whole status roster on every hook event (#18642)
AgentAwakeService.setStatuses deep-copied every row of an array the hook server had just built fresh, and getEligibleRunningStatusCount allocated a filtered array only to read its length. Both run on every agent turn and tool call, and both scale with cached panes rather than with the one pane that changed. Copy the array without cloning its rows, and count in place. At 500 panes the per-event cost drops from 5.34us to 1.78us. |
||
|
Neil
|
a663a21fff |
perf(orchestration): project task columns so task reads hit the statement cache (#18641)
SyncDatabase refuses to cache any statement containing a wildcard, so every SELECT * FROM tasks recompiled on each call. getTask sits on the dispatch and lifecycle paths and listTasks runs several times per coordinator tick on the 2s poll, so those recompiles were continuous during a run. Project TASK_COLUMNS explicitly, the same fix #18420 applied to the graph publish, using the column list already imported in this file. getTask drops from 7.88us to 2.18us per call. |
||
|
Neil
|
c1850e3cc7 |
perf(claude-usage): reject non-assistant transcript lines before parsing them (#18640)
Only assistant records carry usage, but the parser ran JSON.parse on every line first and checked the type after. Claude transcripts interleave user and tool-result lines that routinely embed whole files or captured command output, so the scanner built and discarded a full object graph for each of them. A substring gate on the line rejects them first: median 50.5ms to 8.4ms over 20000 realistic lines. |
||
|
Neil
|
a7223be538 |
perf(agent-hooks): find spool newlines with Buffer.indexOf, not a per-byte loop (#18639)
readSpoolFile walked every byte of every spooled JSONL file in an interpreted loop to locate newlines. drainAgentHookSpool runs inside start() before the hook listener binds, over up to AGENT_HOOK_SPOOL_MAX_FILES files, so this sits on the startup path. Buffer.indexOf reaches the same newlines via memchr. A 5.3MB spool file drops from 32.50ms to 9.26ms per read. The torn-trailing-line contract is unchanged: a final line with no newline is still left unconsumed. |
||
|
Neil
|
1b4159a318 |
perf(sidebar): stop building host projections the row model throws away (#18638)
* perf(sidebar): stop building host projections the row model throws away getMixedHostContextLabels built a label map for every visible worktree and then returned undefined unless two distinct hosts existed — so a single-host install, the common case, paid the whole build on every buildRows rebuild. Decide first, then build only when it is mixed. getHostWorktreeCounts repeated getHostWorktreeIds' dedupe walk to compute a number that is exactly the id list's length, and both loops built the identity string twice per row. Derive the counts and hoist the identity. * fix(test): correct the worktree types import path and drop an unused parameter |
||
|
Neil
|
537a75c94d |
perf(source-control): stop reconciling selection on every panel render (#18637)
reconcileSourceControlSelectionState was called straight from the hook body, so it reran on every Source Control render — every commit-message keystroke and every status poll — and built a Set over every visible row to prune a selection that had not moved. Add an early return for the common empty-selection case and memoize the call on its three inputs. At 500 changed files the empty case drops from 13.10µs to 0.03µs per call. |
||
|
Neil
|
6093b9f058 |
perf(renderer): stop two store selectors allocating on every write (#18632)
The Claude and Codex account sync keys stringify a runtime map and join the whole managed-account roster. Both are pure functions of settings, but they ran inside useAppStore selectors, which Zustand re-runs on every store write. Memoize both on settings identity in one shared module. terminalTabLivenessMatches ran on every tab write including title frames and allocated an [index, value] tuple per worktree and per tab in each changed bucket; indexed loops visit the same entries in the same order. |
||
|
Neil
|
7ccabe5073 |
perf(right-sidebar): cache the active checks status instead of rebuilding its cache keys per store write (#18631)
* perf(right-sidebar): cache the active checks status instead of rebuilding its cache keys per store write The right sidebar is always mounted, so getActiveChecksStatus ran on every store write while it was open and rebuilt two provider cache-key strings plus a branch regex replace each time, only to return the same scalar. Memoize on the input references, matching selectFloatingVisibleTabCount. * refactor(right-sidebar): derive the active checks cache key and parameter type from one key list The cache was keyed on a hand-written field list that had to be kept in sync with every store field computeActiveChecksStatus reads; missing one would leave the sidebar checks badge silently stale. Derive both the parameter type and the cache key from ACTIVE_CHECKS_STATUS_INPUT_KEYS so TypeScript rejects reading a field the cache is not keyed on, and add a Proxy test that records every property actually read and fails on any unkeyed one. |
||
|
Neil
|
9130c6ee6c |
perf(editor): build the closing-fence pattern once per fence, not once per line (#18629)
markdownFenceRanges recompiled the closing-fence RegExp for every line inside an open code fence, though it only depends on the fence's own marker and length. A 100k-line fenced block went from 16.6ms to 5.8ms. |
||
|
Neil
|
fb99bcc3a4 |
perf(terminal): gate the Command Code banner scan before building its scan windows (#18628)
Every PTY chunk on every pane built two ~4KB scan windows and then, for panes that have not shown the banner, stripped terminal control sequences three times over them. The existing prefilter ran after that work and only required the letters C, o and d, which nearly every code agent's output contains. COMMAND_CODE_BANNER_RE requires a literal '#' and stripTerminalControl only removes characters, so raw bytes without one cannot match. Check that against the carry and the chunk before either window is built. |
||
|
Neil
|
1b692088b8 |
perf(terminal): stop copying every PTY chunk for two startup-error detectors (#18626)
Both detectors ran a case-folding copy on every PTY chunk for the pane's whole life, to answer a boolean that is read at most once. git-bash-console-capacity lowercased carry+chunk per chunk. A match that was not already found has to end inside the new chunk, and the marker's last character is a digit, so a chunk without it can skip the copy and fold only the 44-char carry. codex-backfill-error-detector allocated a lowercased copy of the full 4KB carry per chunk; a precompiled case-insensitive regex tests the carry in place. |
||
|
Neil
|
f46823a30a |
test(renderer): pin the runtime-target selector's identity stability (#18625)
#18685 landed the fix (select the host id, derive in useMemo) without a regression guard. This is that guard: it drives 50 unrelated store writes and asserts the consumer re-renders once and the returned target keeps its identity. Confirmed to fail against the pre-#18685 hook (51 renders). |
||
|
Neil
|
c5ed53a1f5 |
perf(git): overlap the three independent submodule status reads (#18623)
getSubmoduleStatus awaited the inner status, the parent gitlink oid and the submodule HEAD one after another, though none depends on another's result. The Source Control panel re-runs this on every parent status poll while a submodule row is expanded, and the same function backs the git.submoduleStatus RPC, so over SSH or WSL each read was a separate round trip. Total latency drops from the sum of the round trips to the longest one. |
||
|
Neil
|
d9680eb753 |
fix(startup): apply the HTTP/1.1 compatibility toggle for migrated profiles, and stop parsing the settings file pre-ready (#18621)
* perf(startup): read the HTTP/1.1 compatibility flag from a marker, not the settings file configureElectronNetworkCompatibility() runs before app.whenReady(), and it answered one boolean by synchronously reading and JSON.parsing the whole orca-data.json — 1.54 MB on a real profile here, measured at 6.6ms read + 3.5ms parse. The Store parses the identical file again moments later, so the first parse was pure waste on the critical path of every cold start. Cache the flag in a few-byte marker next to the other pre-ready markers (gpu-fallback, hang-detection), written at store load and whenever the setting changes. When no marker exists yet the old full read still runs, so an upgrading profile keeps the setting on its first launch. * refactor(startup): drop the unused marker clear helper |
||
|
Neil
|
cae616384f |
perf(orchestration): bound the worker terminal archive in linear time (#18622)
boundArchiveLines built its kept-lines array with kept.unshift() per line, which is O(n) per call. The 256KB char budget admits ~262k lines when they are short, so a blank-line-heavy terminal tail turned the truncation into a quadratic main-process stall: 4.3s for a 300k-line input here, versus 6ms after collecting newest-first and reversing once. Order and truncation boundary are unchanged. |
||
|
Jinwoo-H
|
6c27cc9653 |
fix(mobile-web): serialize conditional file writes per execution host
Keep revision comparison and write in one serialized RPC operation so concurrent clients cannot both consume the same file revision. Reuse the existing keyed queue and verify failed writes release the queue. External filesystem writers remain outside this RPC serialization. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
|
Jinwoo-H
|
9b86dab4bb |
test(mobile-web): use browser APIs in BOM regression coverage
Keep the regression compatible with the standalone hosted-page TypeScript project, which intentionally excludes Node globals. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
|
Jinwoo-H
|
39c727b54e |
fix(mobile-web): preserve UTF-8 BOMs through file reads and writes
Preserve leading byte-order marks when decoding hosted file content, markdown responses and drafts, and host write payloads. Verify exact disk bytes and a subsequent save using the returned revision; add bridge decoding regressions. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
|
Jinwoo-H
|
ac3c620368 |
Merge branch 'fix2-native-b-merge' into mobile-rearch
# Conflicts: # mobile/src/session/mobile-native-chat-eligibility.ts # mobile/src/tasks/mobile-tasks-refactor-parity.test.ts # mobile/src/transport/host-removal-lifecycle.ts |
||
|
Jinwoo-H
|
21c064c895 |
fix(mobile): restore native host-screen, session and task behaviour lost in the rearch
Second-pass P2 regressions against origin/main, restored through the branch's new
operation seams so both the native and hybrid builds keep one code path.
Host screen:
- HostWorkspaceOperations gains optional fetchWorkspaceCatalog and listHostContext.
The native binding implements both: worktree.ps regains its snapshot token
(WorktreeCatalogSnapshotClient) so an unchanged poll stops re-sending the whole
catalog, and the host's STA-3123 error code survives instead of collapsing into
network_error; repo metadata regains ssh.listTargetSummaries / settings.get /
host.platform so multi-host rows show their label and platform again.
- Remove Host no longer races the screen's async identity read: the shell operation
resolves the hybrid cache key itself and an unresolvable key skips the cache purge
rather than refusing the unpair.
Session:
- session.tabs.activate goes back through activateMobileSessionTab, so a
relay-to-direct cutover retries once instead of leaving the host on the old tab.
- The tabs stream degrades on {type:'end'} so the list refetches.
- routeTerminalMultiplexFrame only decodes when a terminal.multiplex stream exists,
ending a wasted payload copy per output frame on native.
- A monitoring agent no longer shows the chat busy indicator.
- The image-echo ordinal counts caption-less turns again, not images.
- Structured image sends take the per-terminal write lock and share one send budget.
UI and tasks:
- Bottom drawers ride the keyboard down again (the height-change effect no longer
resets the offset on teardown); the lift moved into its own hook.
- mailto: links open again (markdown, rich editor, task links).
- MermaidDiagram is memoized again.
- Project conversation comments address issue rows as issues, review replies carry
the server comment, and field edits / comment edits / thread toggles stop refusing
draft rows the host would accept.
@orca/expo-mobile-web-shell resolves optionally, so a build without the pod no longer
throws at module scope on the Home cold-start import chain.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
|
||
|
Jinwoo-H
|
dd4dd9a6aa | Merge branch 'fix2-wire-merge' into mobile-rearch | ||
|
Jinwoo Hong
|
ef428d879e |
feat(relay): tell the phone when its desktop is signed out (#18698)
On 2026-09-04 an auth outage signed ~21,600 desktops out of Orca Cloud and every paired phone showed the generic "Can't reach desktop" for hours. The desktop knew why, the cell watched it happen, and neither could say so. The desktop now names auth loss on its control close reason; the cell remembers that reason per (userId, relayHostId) and replays it as the close reason of the 4404 it already sends a phone whose host is absent; the phone turns it into "Desktop signed out — sign in to Orca on your desktop to reconnect". Retry cadence, close codes and every message body are untouched. The reason rides the WebSocket close reason because there is no additive JSON channel to a shipped phone: RelayPhoneHelloSchema, RelayAuthSchema and the director's ResolveResponseSchema are all zod .strict(), and /v1/connect rejects any query string outright. A new close code was also rejected — an old phone would fall out of mobileRelayRecoveryFor and off the 5-15s host-offline backoff onto the faster transport backoff. The cell keeps the reason in memory rather than Postgres: a phone reaches the cell its host's assignment row already names, which is the cell that saw the close, and losing it on a cell restart degrades to today's verdict rather than a wrong one. |
||
|
Jinwoo Hong
|
77334e7c8b |
fix(orca-profiles): tell the renderer when a cloud session is revoked (#18694)
A revoked refresh-token family makes the main process clear the stored cloud session, but the renderer cached orcaProfileAuthStatus at startup and only re-fetched when it was empty. The account card kept showing "Connected" and Mobile pairing kept showing a generic Relay failure until the user restarted the app. - Push: clearing a session on an auth failure now emits an invalidation event that broadcasts orcaProfiles:authStatusChanged to every window; the renderer re-reads auth status from it. Explicit sign-out is unchanged. - Pull: every pane that renders auth state re-reads it on mount through useOrcaProfileAuthStatusRefresh instead of only when the store is empty. - Copy: a Relay mint failure re-reads auth status, and the failure notice says the session expired and to sign in again instead of offering a retry that cannot succeed. The LAN path is untouched. |
||
|
Jinwoo-H
|
3d7ed005e4 |
fix(mobile-web): strip unknown keys on the shell to page envelope too
Follow-up to
|
||
|
Jinwoo-H
|
2ca0dc7c59 | Merge branch 'fix2-wire-merge' into mobile-rearch | ||
|
Jinwoo-H
|
4c7fd1aec6 |
fix(mobile-web): make the shell to page bridge direction forward compatible
W1 (P1): every shell-authored result and event schema was .strict(), and the page fails a schema mismatch as invalid_message with retryable:false. The shell (APK) and the page (served by the desktop) ship from different releases, so one additive field or one new session tab kind from a newer APK killed the subscription and its one-shot fallback on the same byte - Loading tabs forever, surviving force-quit. tolerantMobileWebShellPayload deep-rewrites a schema at the page's two shell-payload parse sites: strict objects strip unknown keys, an array of unions drops members it cannot classify, and an unknown value for an optional/nullable closed set reads as absent. Page to shell request schemas keep .strict() - the shell is the security authority there. A census ratchet walks every contract export the page parses and fails if a strict node survives the transform. W3 (P2): a host RPC failure collapsed into host_error, which is retryable, so method_not_found and the mobile allowlist's forbidden looked like blips. Both now map to unsupported_capability (non-retryable) through mobileWebBrokerHostRpcError, applied by codemod to the 44 regular 'if (!x.ok) throw host_error' sites. W4 (P3): BrowserScreencastResult gains the navigation member the host already emits. Decoders unchanged. W2 (P2): the file: confinement test gains a clientKind runtime case - pairedDeviceId is minted for scope 'runtime' too, so the fence also governs the web client, a remote desktop, and remote orca CLI. W5 (P3): inputFloor and queryReplyAuthority stay literals with a WHY comment. Traced: the host publishes neither over the terminal stream. isMobileTerminalQueryReplyAuthority is never sent, and opcode-17 WriteUnavailable reports one refused write with no regain signal, so it is not the floor state the field declares. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
|
Brennan BensonandMerge Sim
|
d9e44a6749 |
Keep the floating workspace above a working native chat pane (#18692)
* Keep the floating workspace above a working native chat pane While an agent is streaming, `data-native-chat-working='true'` promoted the native chat pane shell from z-10 to z-50, above the floating workspace panel's z-45. Both compete in the root stacking context, so the chat column painted over a summoned floating workspace and only the sliver past the chat's right edge stayed visible. Lower the working-state rule to z-44. It still clears the z-40 updater and onboarding chrome the rule was added for (#16729) and the 41-44 band was otherwise empty. Raising the panel instead would put it over the z-50 modal layer it deliberately sits under. The existing layering test pinned z-index 50 with no upper bound, which is what let this through; it now derives both bounds from source. * Stop the layering test reading a comment as the panel class The panel's z-index was read with a lazy match from the data attribute to the first `z-[NN]`. Nearby comments cite bare tiers (the toggle button's own note mentions z-[45]), so a reworded comment landing after the attribute would be read as the class. Strip comments and anchor to `className=`. --------- Co-authored-by: Merge Sim <sim@local> |
||
|
Jinwoo-H
|
6d01900797 | Merge branch 'fix2-desktop-merge' into mobile-rearch | ||
|
Jinwoo-H
|
b84ee23eec |
fix(desktop): drop the dead tabList override and close the hosted-bundle CI gap
- Revert CdpBridge.tabList to main's delegation: CdpBridge has no production constructor, browser.tabList is served by AgentBrowserBridgeTabs, and the override published canGoBack/canGoForward that BrowserTabInfo never declared. The hybrid shell reads navigation state from the screencast 'navigation' event, not from tabList, so nothing regresses. - pr-code-change-scope: mobile/host-web-app and mobile/app/h import broadly across mobile/src, so a mobile/src edit changes out/mobile-web-rnw, which ships in every desktop installer. Add mobile/src/ to the carve-out and keep mobile test files desktop-irrelevant so the cost stays bounded. - Move @noble/hashes to devDependencies: its only root consumers are src/mobile-web/src, which Metro inlines at packaging time, matching buffer. - files.unwatch awaits teardown again on the connection-scoped path via cleanupIfOwnedByConnectionAndWait, so a rewatch cannot hold two watchers. - readGuestNavigationState warns once instead of silently greying out Back/Forward when navigationHistory is missing. - One exported isRecord in src/shared and one byte-length encoder in src/mobile-web/src; mobile/ copies left to a later pass. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
|
Jinwoo-H
|
e51aa13180 |
fix(runtime): authorize opcode-18 query replies against the authenticated device token
Both binary query-reply paths called isAcceptableTerminalQueryReplyFrame without connectionClientId, so the guard fell back to the client-DECLARED client.id and one paired phone could author a reply as another. Thread the RpcContext clientId through TerminalSubscriptionArgs and TerminalMultiplexConnectionBase as a distinct connectionClientId, pass it at both binary sites, and make the guard field required so a new call site cannot silently degrade. Also covers browser.tabCreate file: confinement over RPC for a paired mobile device. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
|
Jinwoo Hong
|
5412276776 | Fix status bar runtime target render loop (#18685) | ||
|
Jinwoo Hong
|
3e4fd4a7af |
Shorten orchestration skill description under the Agent Skills 1024-char limit (#18683)
* Shorten orchestration skill description under the Agent Skills 1024-char limit The folded description was 1038 chars, so spec-conforming installers such as SkillStar rejected the bundled orchestration skill. Drop the two clauses already covered elsewhere in the same description: "decomposing work across agents" (implied by "structured multi-agent coordination") and "automation of the browser embedded inside Orca" (restated by the locked `orca-cli` embedded-pages sentence). Every routing trigger asserted by orchestration-skill-guidance.test.mjs, the orca-cli handoff boundary, and the Computer Use boundary are unchanged. Result: 958 chars. Add config/scripts/skill-description-length.test.mjs, which parses every skills/*/SKILL.md frontmatter with `yaml` and fails on an empty or >1024 char description, so the regression cannot return. orca-cli sits at 1015 and is left as is. Fixes #17935 * Keep the embedded browser in the orchestration description's orca-cli routing Restores the word "browser" in the orca-cli sentence ("and the Orca embedded browser") so agents scanning for it still route embedded-browser control to orca-cli. Description is 985 chars, 39 under the spec limit. |
||
 Jinjing
|
b0df874b7b |
Add tab scrollbar (#18526)
* Add interactive scroll indicator for tab strip - Support thumb dragging and track clicking to scroll - Auto-expand on hover and show when scrolling - Forward wheel events to tab container - Disable indicator during tab drag operations * Fully hide scroll indicator when disabled When disabled, the tab strip scroll indicator is hidden, unexpandable on hover, and doesn't forward wheel events. Consolidates disabled state checks across visibility, event handling, and CSS classes. * Cleanup active scroll indicator drag when disabled When the scroll indicator becomes disabled or hidden mid-drag, immediately cancel the drag operation to prevent window listeners and body styles (cursor, user-select) from remaining in a stuck state. * Refine tab scroll indicator appearance and sizing Reduce idle and expanded heights by 1px and 2px respectively, simplify color expressions from color-mix to straightforward opacity utilities, and use transparent background at rest for a more subtle, cohesive appearance. |
||
|
Jinjing
|
67999dcaae |
Keep attention glyph knockout white when row is selected (#18679)
* Simplify palette attention glyph styling Remove visual styling from the container so the glyph appears as a lightweight overlay on the row icon, not a selection bubble. * Keep attention glyph knockout white when row is selected The glyph now uses a white background (bg-popover) with a ring to create a visual knockout effect that separates it from the icon. This prevents the glyph from inheriting the row selection styling, ensuring it stays visible and distinct regardless of selection state. * Correct attention glyph knockout color description to popover-colored |
||
|
Neil
|
0d2375a7ff |
fix(remote): stop a disclosure list latching the mirror completeness gate (#18619)
* fix(remote): stop a disclosure list latching the mirror completeness gate `hostScope.omittedHostIds` was doing two jobs with opposite requirements. As disclosure it must over-name: `omitted-host-scope-selectors.ts` deliberately keeps ids for servers that are no longer paired so a caller can still see the gap, and `docs/reference/ssh-execution-boundary.md` requires a listing to name what it did not cover. As a completeness gate it must name only coverage that was owed and not delivered, or it latches. It latched. `workspaceSessionsByHostId` keeps a partition for every runtime a machine has ever paired with and nothing prunes it, and a mirrored `remote:` row names its peer too — so any client that has ever paired outward publishes a permanently non-empty `omittedHostIds`. `probeHostLiveTerminals` read that as `unverifiable`, `markHostSessionMirrorHydrated` never fired, and panes parked on `parkUntilHostSessionMirrorHydrates` never drained. `hostScopeCensusIsComplete` gives the gate its own answer and leaves the disclosure list alone. A `runtime:` host is never owed coverage by the runtime answering: a paired runtime is a peer with its own control plane reached with `--environment`, and there is no paired-runtime PTY provider for this runtime to have queried. Two other branches stay load-bearing — an absent scope is a host too old to claim one, and a listing that covered no host proves nothing. No wire change: the host publishes byte-identical content and only the client's reading moves, so this reaches the reporter by updating their client alone rather than waiting for their remote. That also avoids a new field's fallback rule, where "absent means complete" would recreate the bug with the polarity flipped. `queried-host-kinds.test.ts` pins the invariant the predicate rests on at its source, because the consolidation moving the SSH path onto orcad is the change most likely to introduce a runtime-backed PTY provider and quietly invalidate it. Fixes #18595 * test(remote): pin the orphan-recovery host-scope gate and narrow the invariant claim The readiness review found the second gate unpinned: reverting `web-session-terminal-orphan-recovery-inventory.ts` alone to the pre-PR expression left the whole renderer suite green, because every existing fixture passes `omittedHostIds: []`. The commit claimed two gates and proved one. Four cases now drive `resolveTerminalOrphanInventory` through a non-empty scope. Reverting that gate alone fails the peer-runtime case. Note the absent-scope case deletes the key rather than passing `undefined`, because `listResult` substitutes its default for `undefined` — routing through the fixture there silently tests the default instead. `queried-host-kinds.test.ts` also claimed more than it caught: a runtime-backed transport registered under an SSH connection id reports as `ssh:` and passes, which is the shape the orcad consolidation is expected to take. It pins the spelling this function emits, which is what the gate keys on, and now says so. * fix(remote): require a legible covered host before believing a census CodeRabbit found a real asymmetry: the predicate refused an omitted host id it could not parse, but accepted an unparseable *covered* id as proof of coverage. `isTerminalListResult` validates only that `hostIds` is an array, so `{hostIds: ['runtime:'], omittedHostIds: ['runtime:env-7']}` was `unverifiable` before this PR and would have become `complete` after it. Taken as "at least one legible covered host" rather than the suggested "every id parses". A host that later gains a kind this client cannot parse would otherwise report an incomplete census forever — which is this bug in a new coat, and the failure mode the predicate exists to prevent. The check exposed four tests publishing `hostIds: ['remote-runtime']`, a bare environment id that `parseExecutionHostId` rejects. No host emits that: a runtime answering `terminal.list` names the execution hosts it covered, which is `local` — verified against a live paired runtime. Those fixtures are corrected to the shape the wire actually carries, which is why the assertions move. |
||
|
Neil
|
14e4031948 |
test(pty): make the F24 patch pins catch the regressions they name (#18660)
Two of the pins added in #18635 did not discriminate. Found by review of the merged change; both are test-only defects, the fix itself is unaffected. `resolves the ConPTY DLL before it claims the close` searched the whole patch for `HANDLE hLibrary = LoadConptyDll(info, useConptyDll);`. That line occurs twice -- PtyConnect's copy comes first -- so indexOf always matched PtyConnect, and the ordering assertion held no matter where PtyKill resolved the DLL. Verified by simulation: moving PtyKill's resolve back below the claim left the suite green. `reaches hShell only under the null check` used a marker as a slice END bound without checking it existed. If that marker vanished the slice ran to the end of the patch, the stray-line filter found nothing, and the test passed silently. Both now anchor inside the PtyKill hunk only, located by its header's function context rather than line numbers. `indexIn` throws on a missing marker instead of returning -1, so a marker that moves fails the assertion that depends on it rather than making it vacuous. Adds the pin that was missing entirely: PtyKill's half of the two-sided baton free. Without it a self-exit followed by kill() -- the ordinary pane close -- leaks one baton and one entry in the vector get_pty_baton scans linearly. Mutation-tested rather than only revert-tested, because wholesale reverting the patch is what hid this: it fails every assertion for the trivial reason that nothing matches. Simulating each specific regression instead: - move PtyKill's DLL resolve below the claim -> 1 failed (was: 0) - drop PtyKill's baton free, line-count-neutral -> 2 failed (was: 0) Wholesale revert still fails all 9. Refs F24. |
||
|
Neil
|
f7e3af254a |
fix(pty): close the pseudoconsole and dispose the conout worker on Windows self-exit (F24) (#18635)
* fix(pty): close the pseudoconsole when a Windows shell exits by itself
`ClosePseudoConsole` is the only thing that reaps a ConPTY's console host.
node-pty calls it from one place, `PtyKill`, which starts by looking the baton
up by id -- and the exit watcher in `SetupExitCallback` erased that baton the
moment the shell died. So on the self-exit path (typing `exit`, how panes
usually close) the lookup missed, `PtyKill` did nothing at all, and the
pseudoconsole was never closed.
The baton now survives until BOTH the shell has exited and `kill()` has run;
whichever arrives second frees it. `PtyKill` copies `hpc` out under the lock and
closes it afterwards, guards `TerminateProcess` on a shell handle the watcher
may already have closed, and duplicates that handle rather than reordering, so
upstream's close-then-terminate sequence is unchanged.
Measured on Windows 11, 20 self-exit cycles driven exactly as Orca drives them
(`onExit -> destroy()`), handles bucketed by NT object type:
relay spawn (no useConptyDll) 225 -> 285 (+1 Process +2 File/term)
after 219 -> 219 FLAT
desktop spawn (useConptyDll) 239 -> 439 (+1 Process +2 Thread +5 File/term)
after 235 -> 395 (+2 Thread +4 File/term)
The desktop residue is a separate defect in the `useConptyDll` branch of
`WindowsPtyAgent.kill()`, which disposes the conout worker only from an
`_outSocket.on('data')` handler -- and no data arrives after the shell has gone.
Fixing that line as well takes the desktop to 222 -> 222 FLAT, but it lives in
the `kill()` hunk owned by F23, so it is left to that change.
Refs F24.
* fix(pty): dispose the conout worker when a Windows shell exits by itself
Second, independent defect on the same self-exit path, and the larger half of
the desktop's leak. The `useConptyDll` branch of `WindowsPtyAgent.kill()`
disposed the conout worker only from an `_outSocket.on('data')` handler -- and
once the shell has gone no more data ever arrives, so the worker was never
disposed. The non-DLL branch three lines above already disposed unconditionally,
which is why only the desktop (the only spawner that sets `useConptyDll`) hit it.
Measured on Windows 11, 20 cycles, handles bucketed by NT object type, totals:
self-exit, relay spawn 225 -> 285 now 219 -> 219 FLAT
self-exit, desktop spawn 239 -> 439 now 222 -> 222 FLAT
explicit kill, relay spawn 225 -> 285 now 219 -> 219 FLAT
explicit kill, desktop spawn 235 -> 395 now 219 -> 219 FLAT
Neither fix alone is enough on the desktop: the pseudoconsole close is worth
+1 Process +1 File per terminal, this dispose +2 Thread +4 File.
The relay asset (config/relay-assets/node-pty-1.1.0-windows-pty-teardown-patch.cjs)
deliberately gets no counterpart: the relay takes the non-DLL branch, where the
dispose is already unconditional. Its reconstruction table needs the new hunk
though, or un-applying the desktop hunks no longer yields published node-pty.
Taken over from F23 at win-relay-qa's request after they verified that the
desktop never executes the non-DLL branch F23 was scoped around.
Refs F24.
* fix(pty): harden PtyKill against a failed handle duplication and a missing DLL
Both from review of #18635.
DuplicateHandle's result was dropped. On the live explicit-kill path a failed
duplication left hShellDup null, which the guard below could not tell apart from
the self-exit case, so TerminateProcess was skipped and the shell kept running
after its pane closed -- a worse outcome than the handle leak this patch exists
to fix. The failure now terminates through handle->hShell under the lock, where
it is valid and where TerminateProcess does not block. The only cost is that the
rare path kills before the console closes instead of after.
LoadConptyDll is now resolved BEFORE any baton state is touched, matching what
PtyConnect already does for the same reason. It throws when conpty.dll is
missing, and a throw after consoleClosed was set would strand the pseudoconsole
permanently: the retry finds the work claimed and does nothing.
Also corrects three comments the earlier commits made stale:
- the ptyJobMutex note still said PtyKill reads the table unlocked
- PtyListJobProcessIds said the baton is gone once the shell exits; it now
outlives the shell, and the nulled hJob is what makes the answer null
- windows-pty-job.ts said node-pty drops its handle record on exit
Re-measured on Windows 11 with the rebuilt binary, 20 cycles, all four paths
still flat: self-exit relay 219->219, self-exit desktop 222->222, explicit-kill
relay 219->219, explicit-kill desktop 219->219. Both explicit-kill runs report
22/22 shells exited, so the kill still lands.
Refs F24.
|
||
|
Neil
|
886fcf083f |
fix(runtime): let a scoped worktree listing report the host it could not cover (#18645)
* fix(runtime): let a scoped worktree listing report the host it could not cover
`orca worktree list --repo <id>` passed `[]` as `knownHostIds`, so a scoped
listing could never report a gap — for any host kind, reachable or not. With zero
matched rows both scope lists are empty by construction, and the answer is
`{hostIds: [], omittedHostIds: []}`: byte-identical to a repo that genuinely has
no worktrees. docs/reference/ssh-execution-boundary.md forbids a listing from
implying exactly that.
Measured on one runtime with one refusing SSH host, in the same second:
unscoped totalCount 0 omittedHostIds ["local","ssh:<target>"] (+ --host selectors)
scoped totalCount 0 hostScope {"hostIds":[],"omittedHostIds":[]}
The same runtime reports nine omitted hosts unscoped and zero scoped against a
live profile, so this is not a subtle inconsistency: it is one runtime giving two
contradictory answers about its own coverage.
A scoped listing now names the one host the caller asked about. That costs
nothing when rows come back — the host lands in `covered`, so it is never
reported omitted — and is the whole answer when they do not. Hosts the caller
scoped out are still never named, which a test pins, because naming them all is
the obvious over-correction.
* test(runtime): pin the scoped host derivation for local and executionHostId repos
Review flagged that the host-scope cases only covered a connectionId repo.
getRepoExecutionHostId reads two spellings, and a scoped listing naming the
wrong host would be worse than naming none, so both are pinned. Both fail with
the fix reverted.
|
||
|
Neil
|
637dc30a32 |
fix(relay): observe Windows PTY child processes instead of answering false (#18591)
* fix(relay): observe Windows PTY child processes instead of answering false `processHasChildren` returned a hardcoded `false` on Windows, and a hardcoded negative is indistinguishable from a measurement. Every close guard reads it as "nothing is running in this pane", so an SSH-to-Windows tab running a build closed with no prompt. Measured on a real Windows SSH host: a live `PING.EXE` under the pane's `cmd.exe` still reported `hasChildProcesses: false`, while the identical harness on Linux reported `sleep` / `true`. Windows has no `ps`, but it does have a process table, and the pane walk over it already existed for the foreground reader. The answer now comes from `queryWindowsPaneProcessInventory`; a table it could not read reports `unverifiable` rather than a fabricated negative. `hasChildProcesses` is a boolean, which cannot hold the third answer, and it is read both as "busy, do not close" and as "the agent took the PTY, safe to type into" — so no single mapping of `unverifiable` is safe for both. The verdict moves to a new optional `childProcessEvidence` member that the close paths read; the boolean keeps its exact meaning for every client that cannot. Cost: `pty.inspectProcess` is the polled path and a relay host has no `@vscode/windows-process-tree`, so its table read falls back to the 1.36s CIM scan. Polling that would reinstate the fork storm the shared table exists to prevent, so only a caller whose answer decides something asks for the scan. * fix(runtime): forward scanChildProcesses through the environment inspection RPC `guardRunningTerminalClose` asks the host to pay for a real child-process read, but the environment path dropped the option before it reached the wire: the renderer sent only `expectedIncarnationId`, and the RPC schema — the shared `TerminalHandle` — silently stripped anything else. A host routing that pane through an SSH relay then declined to scan and answered `unverifiable`, which `inspectionReportsRunningWork` reads as running work. The result was a close confirmation on an idle pane, which is the nag this PR exists to avoid. Forwarded through all four layers: renderer payload, RPC schema, method handler, and the runtime/controller signatures. The schema is a dedicated extension rather than a field on `TerminalHandle`, so `clearBuffer`/`agentStatus`/`isRunningAgent` keep refusing an option they have no use for. The silent strip is not itself the defect — it is what makes a new optional member safe to send to an old host, per docs/reference/remote-wire-compatibility.md. The defect was the schema and its caller drifting inside one version, so the tests pin the registered method rather than the schema alone: pointing it back at `TerminalHandle` compiles, parses, and drops the option. Found by review on #18591. * fix(terminal): teach the shared running-work probe the third child-process answer Rebasing onto main landed `probePtyRunningWork`, which is a better home for this than the close guard: it already speaks `live` / `unverifiable` / `exited`, and it exists so the tab-close and window-close guards cannot drift. The child-process verdict belongs there, not in a parallel predicate beside it. So the mapping moves into the probe and `inspectionReportsRunningWork` is deleted rather than kept alongside. The probe now asks for the scan, and a host that could not observe the pane reports `unverifiable` instead of collapsing onto `exited` -- which is what `hasChildProcesses: false` meant on every Windows relay. The pane-close path is routed through the same probe for the same reason; it was the third caller asking this question through a direct inspect of its own. |
||
|
Neil
|
766b5b153c |
fix(relay): release the ConPTY conin handle after teardown, not before it (#18601)
A Windows SSH relay leaked one Windows File handle per terminal, for the life of the relay process, across reconnects. node-pty's `kill()` flips `readable` on the conin and conout sockets and destroys neither; `_cleanUpProcess` destroys `_outSocket`, so only conin is stranded, and it wraps a real named-pipe handle from `fs.openSync(term.conin, 'w')`. The obvious fix -- and the one config/patches/node-pty@1.1.0.patch ships for the desktop -- releases it at the top of the branch, before `_getConsoleProcessList()` forks and before the native kill. Measured against a real Windows SSH host, that is three times worse than leaving the leak alone: teardown aborts partway, the forked console-list agent is never reaped, and both pipe handles stay alive. Releasing it at the end of the branch instead is flat. 20 spawn/kill cycles, handles bucketed by NT object type, identical numbers standalone and through a real relay: published node-pty File +1/terminal, Process flat desktop patch placement File +2/terminal, Process +1/terminal released last (this) File flat, Process flat `windowsTerminal.js` takes the desktop's error-listener hunks verbatim. The conin listener is not what fixes the leak -- adding it alone changed nothing -- but it is what keeps a pipe error retiring one terminal instead of the host. The desktop patch has the early placement and therefore the regression, measured against its exact installed tree. Correcting it there needs its own verification on a Windows desktop build, so the trees diverge on this one hunk deliberately and a test pins that so a future patch sync cannot copy the bug back. |