639 Commits

Include renames

Author	SHA1	Message	Date
Jinwoo Hong	cf7408a37f	feat(ai-vault-search): session search query engine over the index (#19750) ... * feat(ai-vault-search): give the index a generation a page cursor can be fenced by A search page is a slice of one ranked list, so a cursor only means anything against the snapshot that produced it. The store now keeps a monotone generation in `meta`, bumped by every mutation that can change which rows a read returns, and starts a new one on open so a write whose bump never landed cannot leave a cursor pointing at content that is already gone. Schema version 2 adds the two tables the query layer needs: `messages_vocab` (fts5vocab over messages_fts, the typo repair's whole dictionary) and `search_log`. Both are pure additions and the index is a cache, so a version-1 file is dropped and rebuilt exactly as any other mismatch is. * feat(ai-vault-search): plan a query the way the index tokenized it The planner unfolds the tokenizer contract instead of asking SQLite: same boundaries as `unicode61 tokenchars '_.-/+'`, pinned against real fts5vocab output so a query can be planned without a round trip. It decides the route ladder's first rung (literal shape), strips stop words from prose but never from a literal, and fans an identifier out into its pieces for the OR fallback. Typo repair uses the index's own vocabulary as its dictionary, so it can never suggest a term the index does not hold. Its exact-match probe now joins `visible_messages`: `messages_vocab` is a view over the FTS b-tree and still lists staged and tombstoned terms, and the PR 2 read ratchet is right to demand the join. `splitAiVaultSearchQuery` is the index's reading of repo: / path:. It keeps operator case, which the panel folds and cwd_key must not; a census test pins the two parsers to the same answer about what is an operator until PR 7 moves the panel onto this one. * feat(ai-vault-search): narrow a search the way the sidebar keys a folder Every caller-supplied narrowing in one place, so retrieval, the operator-only page and the session load cannot drift apart: agents, an updated-at floor, the retention cutoff, scope paths, and the repo: / path: operators. Scope keying goes through PR 2's `cwdKey`, which is the sidebar's `folderGroupKey` without its prefix, rather than the original branch's second spelling. That drops the branch's WSL distro qualification, which PR 2 removed on purpose, and it makes the filesystem root a key that already ends in a separator, so the child-prefix range is built from the key rather than by appending one; `//` sorts below every real child and would scope the root to nothing. Engine types land here too, under src/main and not src/shared: nothing in this PR is a wire type, and PR 5 lifts what a caller may receive. * feat(ai-vault-search): rank, page and answer a session search `SessionSearchEngine.search()` over the PR 2 store: route ladder (phrase, AND, typo repair, OR), BM25 weights per corpus, one hit per session, fork folding, and a page. - `scope` picks the corpus and the engine never second-guesses it. `conversation` is user and assistant turns; `all` adds tool output and the identifier shadow column. Switching corpus while typing is PR 7's policy; an engine that widened on a miss would make a result impossible to reproduce from its own request. - Pagination is an offset into one ranked list, fenced by the index generation and by a hash of everything that changes the ranking. A cursor from another generation or another query is refused with a typed error rather than silently re-run. Ranking breaks every tie by session id, because a cursor indexes into that order and retrieval does not promise one. - Snippets and source presence are paid for by the page, not the list. A snippet past the per-hit ceiling is cut on a code point, never between `[[` and its `]]`, and flagged on the hit. - Source presence is read from the `files` table. No stat on the query path, and no `missing`: only a proven deletion may claim one, and this read cannot prove it. - `SESSION_SEARCH_CANDIDATE_LIMIT_DEFAULT` is an option, not a constant, and the result says when the limit was what cut the answer. - The engine is the only caller of `store.warm()`, on first search. Library only: no IPC, no settings, no Electron, nothing constructs it in production. * perf(ai-vault-search): measure what a query costs and what the candidate limit buys Two corpora, because they answer different questions. The 10.5 MB / 40-session corpus is what the scope split costs a reader: conversation is about 1.6x faster at p50 and 3.4x at p95 than the full corpus, which is the argument for the second FTS table being the one a keystroke can afford. The candidate limit needs more sessions than the limit before it costs anything, so it is swept over 2,500 one-turn transcripts with every session matching. Limits are interleaved sample by sample: run back to back, the first configuration pays for every page the OS cache had not seen and the ordering alone moved p95 further than the limit did. The doc says plainly what these numbers do not cover. They are cost, not relevance; the MRR figures quoted beside the BM25 weights and the identifier shadow column come from a shoot-out over real transcripts and cannot be reproduced from this repository. * refactor(ai-vault-search): key a page once per search, and report reachable pages honestly The page key was hashed twice per search, once to decode the incoming cursor and once to mint the outgoing one. The benchmark's reachable-page figure was clamped against a constant that could never bind; it is the limit over the page size and nothing else. * test(ai-vault-search): pin the two engine seams nothing was holding The warm wiring and the query-length cap were both written and neither was observable. A spy pins that a search is what warms the store, and the cap is pinned through the one input the planner's own term limit does not already bound: a single enormous token, where the cut is what decides whether the term matches the indexed one at all. * fix(ai-vault-search): fence the generation against writers this process cannot see The generation was cached in memory and moved only on this store's own writes, and the bump itself was a read-then-write outside any transaction. Two handles on one file is the normal case once PR 3 lands: the indexer writes in the scanner child while an engine reads elsewhere. A reader would see that writer's deletions while its own generation stood still, honour a stale cursor, and skip a session; two writers could mint one generation for two snapshots. `generation` now reads `meta` on every call, and the bump is a single `ON CONFLICT DO UPDATE ... + 1` inside the transaction that makes the change. That closes the crash hole the bump-on-open existed for, so opening a store no longer invalidates anyone's cursor. Only a change to what a read returns counts. Retiring a path the index never held hides nothing, and the row deletes that drain a tombstone take away rows that were already invisible: bumping there would refuse a cursor every 256 rows and leave pagination unusable for as long as indexing ran. Also drops redaction from the query log, following PR 2's decision to store transcript content as written; the module it called no longer exists. * fix(ai-vault-search): answer from a version-1 index, and keep a repaired literal whole Two smaller findings. An engine can be handed a connection to a version-1 file that another handle is still answering from, and this PR is what first makes that reachable. It now probes once for the two tables version 2 added and names what it cannot serve on every result, instead of throwing at the first query that reaches for a vocabulary that is not there. The route ladder simply skips its repair rung. Which process may unlink and rebuild the index is PR 3b's decision and is not solved here. Typo repair re-planned the corrected query from scratch, and a corrected spelling can read as prose even when what was typed was a literal: `parseJsonn(the, data)` has the punctuation, `parsejson the data` does not, so the re-plan dropped `the` as a stop word. The repaired query searched for less than was asked and `repairedTerms` reported a body nobody typed. The re-plan is now told what the original decided, because a repair changes spellings, not the query's character. `repairedTerms` is documented as the whole body the repaired plan ran, with the index's own case-folded spelling for the terms it corrected. * fix(ai-vault-search): make repo: and path: mean one thing in the list and the index The engine said `repo:` and `path:` a second time, in SQL, and SQL cannot say them. LIKE folds ASCII and nothing else, so `path:CAFÉ` missed `café`; the engine searched `cwd_key` while the panel searches the working directory and the transcript path, so `path:jsonl` matched every session in the panel and none in the index; and the engine compared one path segment where the panel compares the last two, so `repo:orca/session-search` missed. All four reproduce in both directions. So there is one definition now, not two that resemble each other. `matchesAiVaultQueryOperators` moves into the shared filter module beside the panel that already owned the semantics, the panel calls it, and the engine applies it over the rows it retrieved. SQL keeps only what it can express exactly: the `cwd_key` prefix range for `scopePaths`. `parseVaultQuery` now parses through `splitAiVaultSearchQuery`, so one parser decides what an operator is. Its existing tests pass unchanged; four degenerate shapes do answer differently and are pinned as decisions rather than left to be discovered. Two consequences worth stating. The operators are conjunctive now, because that is what the panel has always done, where the engine had been ORing within a key. And the operator-only page walks newest sessions in bounded pages applying the predicate, rather than taking one cut of the newest N and filtering it, which would have answered `repo:x` with nothing on a busy index. * docs(ai-vault-search): re-measure the query benchmark after the operator change repo: and path: moved out of SQL, so the operator-only row measures something different now and the note that it is a range seek was wrong. The rest of the table is re-measured on an idle machine: the previous run's p95 column was mostly contention, which is why conversation looked 3.4x faster at p95 rather than the 1.7x it actually is. Adds what this PR does not settle: which process may unlink and rebuild the index is PR 3b's, and PR 4 is only the first thing that makes reading it reachable. * fix(ai-vault-search): stop reporting a search that gave up as a search that finished The operator-only walk stops at a scan ceiling as well as at a full candidate set, and only the first of those reached the result. A query whose one match sat past the ceiling came back with no hits and truncated.candidates false, which is the engine claiming there is nothing to find when what happened is that it stopped looking. Retrieval now says why it stopped, because it is the only layer that knows, and the count it used to return could not distinguish the two cases. The cursor fence stays as it is: any published read moves the generation, so an outstanding cursor is refused, and that is what F11 asked for. What was wrong was the claim next to the row-delete skip that pagination stays usable through indexing. It does not, and the engine now says so. The rejection carries the generation the cursor was minted in and the one the index is at, so a caller can tell a moved index from a bad cursor and re-issue page one without showing anyone an error. The capability probe was nearly dead code, since every store opens through a function that rebuilds a stale file. It is not dead, because two handles can be open on one file, so the claim is corrected rather than the probe deleted. It now runs per search: a verdict cached in the constructor is wrong in both directions once another handle rebuilds the index. Also says why the row-delete loop may skip the bump: those messages keep batch_id NULL and stay in visible_messages, so what makes them unreachable is their session's tombstone, and the read ratchet is what keeps every reader joining the view that applies it. * fix(ai-vault-search): restore the panel's reading of a quote that does not end a word Unifying the two parsers changed panel behaviour on nine of twenty probed shapes, not the three previously pinned. Six of the nine were regressions, all from one rule: the shared parser refused a quoted span whose closing quote was not followed by a space, so `"a b"c` and `repo:"a"b` became single terms carrying their own quote characters, which match nothing. The rule was justified as what stops the apostrophes in `it's a repo:orca thing's` from swallowing the operator between them. It is not: a span only ever opens at a token start, and the quote in `it's` is not at one. Dropping the rule restores all six shapes to what the panel has always done and leaves that protection intact. Three changes remain and are kept because the old answer was worse in each: an operator with an empty quoted value is dropped rather than filtering on `""` and silently emptying the list, and a bare pair of quotes reads as an empty term rather than as the two characters. Each is pinned with a test that says which behaviour it is and why. * fix(ai-vault-search): trim operator values, and report a query the engine had to cut Three lows. `repo:" "` survived as a term and matched no label, silently emptying the list, which is the exact defect the empty-value drop exists to prevent wearing different clothes. Operator values are trimmed, and a whitespace-only one drops like an empty one. The substring matcher's copy of a free-text term is trimmed too, so `" "` reads as the empty term already does; the span kept for FTS is still the query verbatim. Two caps upstream of retrieval fired silently: the planner searches at most 48 terms, and the engine cuts the query at 512 characters. A 56-term query whose only match was the 56th came back with no hits and nothing truncated, which claims there is nothing to find. `truncated.query` now says when either fired, alongside the candidate and snippet flags that already did. Every cursor refusal now carries the generation the index is at, which the engine knows before it looks at the cursor, and the generation the cursor claimed wherever that survived parsing. The doc says exactly when each is present instead of leaving absence unexplained. * refactor(ai-vault-search): read the tables the simplified index writes, and own the fence PR 2 deleted the visibility views, the staging tables and the store's generation, so this reads `sessions` and `messages` directly and carries the three schema objects only a query needs — the vocabulary, the query log, and the triggers that move the generation — as its own extension over the store's schema. The fence is now three triggers on `files`, because every transaction the store opens that can change what a search returns writes that table and nothing else does; retention's orphan drain is the one write path that touches neither, and it is the one that must not bump. The triggers live in the file, so a writer in another process moves the generation without knowing a reader exists. A message row can now outlive its session row until the drain reaches it, so the snippet read joins `sessions` and the typo repair asks for a live posting instead of trusting the vocabulary's document count. The engine takes a connection rather than a store: PR 2's store keeps its connection private, and which process may open or rebuild the index file is PR 3b's decision, not a query engine's. * perf(ai-vault-search): price the second FTS table, and re-measure without warmup Open decision 3. A column filter over `messages_fts` returns the identical rowid set as `conversation_fts` — checked here per query rather than assumed — so the table exists for latency alone. On a 105 MB corpus at both ends of the tool-output band, the column-filtered form costs 1.16-1.42x at p95, against a bar of 2x, so the recommendation is to delete it. The shoot-out writes its own corpus because the answer turns on the one property the shared generator fixes: how much of a transcript is tool output. Half the tokens in that output are words the conversation also uses, which is deliberately generous to the table under question. The doc records the number that argues the other way. PR 2 priced the table at about a quarter of the index on a corpus whose tool output is 56% of its message text; on a tool-heavy one it is 6.7-11%, because `messages_fts` grows with the tool text and the second table does not. Page warmup is not re-added. The measurement behind it was on a 4 GB index, removing it moves this corpus by less than the run-to-run spread, and a cancellable background pass needs a lifecycle a query library does not have. * test(ai-vault-search): pin that an append moves the generation a cursor is fenced by * refactor(ai-vault-search): answer the conversation scope with a column filter PR 2 deleted `conversation_fts` on the strength of this PR's shoot-out, so the scope is a column filter over the one FTS table now. `ftsTableFor` is gone; a scope is a pair of `scopedExpression` and `scopedWeights`, and the table name no longer travels through the engine, the snippet builder or a hit. The filter is parenthesised, and that is the whole of it: `{cols}: (a AND b)` binds both terms, while `{cols}: a AND b` binds only the first and searches tool output for the rest. A test drives an AND whose second term lives only in tool output through both scopes. The snippet keeps one guard, not two. Its column list and its expression were each hiding the other's mistakes — a tool-only row was unreachable through either — so the list is the same four columns for every scope and the scoped expression is what makes a conversation snippet impossible to draw out of tool output. Dropping it now leaks that row, which a test catches. One behaviour the deleted table did not have, pinned rather than wished away: bm25 normalises by the whole row's length and has no per-column length, so two rows with identical prose score differently when one also holds tool output. The rowid set is unchanged; the order within it can move. Re-measured on the shipping schema. The conversation scope is 1.2-1.4x faster than `all` at every rung, and the index is 57 MB rather than about 150 MB at 93% tool output, because a tool row is now capped at 3,072 characters. * fix(ai-vault-search): repair a spelling inside the scope that will answer it Typo repair read `messages_vocab` and probed `messages_fts` with no column filter, so tool output decided whether a conversation-scoped query was repaired, in both directions. A tool row carrying the misspelling made the query look correctly spelled and suppressed the repair; a tool row carrying a rare word became the suggestion, naming in `repairedTerms` a string from a column the scope will never show. Both reproduced against a control index that differs by exactly that one row. The vocabulary proposes and a scoped count disposes. fts5vocab is per table and cannot be column-filtered, so every decision that reaches the plan — already spelled right, eligible, and which of two equally close candidates wins — now comes from a `messages_fts MATCH` under the same filter retrieval uses, joined to `sessions`. That also takes the vocabulary's `doc` out of the ranking, which is the half of the drain defect that belongs here: `doc` counts rows whose session a purge has already cut loose, so reclaiming them changed which word a query was repaired to. Candidates are ordered by term now, because the ordering decides which of them survive the scan limit, and ties on similarity go to the more common word counted live rather than to the vocabulary's number. The cost is one bounded count per candidate examined, at most eight per prefix, and only for a term the scope has no posting for at all. * fix(ai-vault-search): fence the rows a purge reclaims after it cuts a session loose Retention's second half deletes from `messages` alone and touched neither `files` nor `sessions`, so it moved no generation. The argument was that those rows answer nothing, which was true of retrieval and not of the engine: the typo repair's dictionary is a view over the FTS b-tree and listed them, so a drain running between two pages swapped the repair under a cursor that was still honoured, and a search that had answered stopped answering. The commit before this one fixes that at its source by counting live rows. It does not make the drain provably inert — the vocabulary still decides which candidates survive its scan limit, and reclaiming a term's last row moves where that limit cuts — so the fence is what covers the rest. A fourth trigger, on `messages`, with a `WHEN` clause that is the whole reason it is affordable: a replace and a `removeFile` delete a session's rows while its `sessions` row still stands, so neither fires, and both already bump through `files`. Only the drain deletes a row whose session is gone. The price is named rather than avoided: a cursor outstanding while a purge runs is now refused once per batch, which `SessionSearchCursorError` reports as `stale-generation` so a caller re-issues page one. The test that pinned the old contract is replaced by one for the new one, and by one proving a replace still does not fire it. * fix(ai-vault-search): tell a highlight from a transcript that contains brackets The snippet builder asked each of a row's four columns for a marked snippet and showed the first whose text contained `[[`. Transcripts contain `[[`: a bash `if [[ -f … ]]`, numpy's `[[1, 2], [3, 4]]`. A row matching only in tool output was shown its user turn instead, with nothing highlighted in it, and the any-column fallback an identifier-only match depends on was unreachable behind the same collision. Whether a column matched is now the difference between two renderings of the same text: `snippet(…, MARK, MARK, …)` beside `snippet(…, '', '', …)`. Content cannot forge a difference between those two, because it is the same content either way. The marks FTS5 inserts are private-use code points, rewritten to the public `[[` and `]]` once, at the end. That is for the other decision that has to tell a mark from content: the truncation refuses to cut between an open mark and its close, and a transcript's own bracket used to move that cut. * fix(ai-vault-search): cut a query on a code point and bind ids in batches Two small ones from the review's not-routed list. `query.slice(0, 512)` can land between the halves of a surrogate pair, leaving a lone half that matches nothing and that a caller cannot echo back. The reader already has `sliceAtCodeUnitLimit` for exactly this. `loadSessions` bound one parameter per candidate id in a single statement. The list is as long as the candidate limit, the tuning doc invites a host to raise that limit, and SQLite's `SQLITE_MAX_VARIABLE_NUMBER` is 999 on builds older than 3.32 — so one settings change away from `too many SQL variables`. Read in batches of 500, leaving room for the filter's own bound values. * docs(ai-vault-search): price the repair rung, and record what is left open Typo repair is the one rung whose cost tracks the vocabulary rather than the result, and it only runs for a term the scope has no posting for. Measured over 1.6 M distinct terms: 10 ms for one unknown term, 387 ms for a 480-character query of thirty-nine of them. The scoped-count fix made that cheaper rather than dearer, from 737 ms, because ordering the vocabulary scan by term drops the sort `doc DESC` needed and the counts it adds are at most eight bounded probes per prefix. A cap on unknown terms per query is a follow-up in the split plan, with the five other items the final review raised and did not route. * test(ai-vault-search): make each snippet mark mechanism answer for itself Two mechanisms landed together and hid each other: choosing a column by comparing a marked rendering against an unmarked one, and marking with private-use code points instead of `[[`. Either alone fixed the bracket repro, so neither had a mutation against it — the same masking the snippet's two column guards had a round ago. They do different jobs, so both stay and each gets the test that needs it. A transcript holding a private-use code point of its own is what the comparison is for; agent output carries Nerd Font glyphs from that block. A snippet past the character ceiling with a bracket after its last real mark is what the private-use marks are for, because the truncation has to find that mark by searching the text. The two one-line fixes get honest framing rather than a mutation neither can have. A lone surrogate is not a token character, so the planner drops it either way and the safe cut is hygiene. And no SQLite this stack runs refuses 1,100 bound ids — 32,766 has been the floor since 3.32 — so the batch is about owning the ceiling here rather than rescuing a reachable failure. * refactor(ai-vault-search): keep the scope's expression with the other expressions Making the typo repair ask its questions in the search's own scope put an import from retrieval into it, and retrieval already owns the repair — a cycle the native audit catches. `scopedExpression` belongs beside `phraseExpression`, `andExpression` and `orExpression` anyway: it builds a MATCH expression, and two callers now need it. The bm25 weights stay in retrieval, where the SQL that uses them is. * docs(ai-vault-search): say which delete paths fire the orphan-reclaim trigger after a replace cuts loose	2026-09-11 01:11:37 -04:00
Jinwoo Hong	c84007c541	feat(rpc): generate a shared params catalog from the host registry, gated on parse parity (#19961)	2026-09-10 21:18:39 -07:00
Jinwoo Hong	2c984bcdaa	feat(ai-vault-search): session search index as a transcript reader consumer (#19687) ... * feat(ai-vault-search): add the session search index schema and row modules The FTS5 index that PR 2 folds the transcript reader's message stream into: two FTS tables behind visibility views, the identifier shadow column, the resumable fork digest, redaction at the row-insert choke point, and the bounded compaction, warm-up and retention lanes. Schema version starts at 1: this branch drops the query log and the fts5vocab table, which the query engine reintroduces with its own bump. * feat(ai-vault-search): stage index writes and publish them atomically The writer stages a read's rows against an unpublished session row and a batch id, then flips the whole read visible in one transaction: both FTS tables are written together per row, and publish nulls the batch pointer before dropping the batch so a recycled rowid can never name a later in-flight write. Buffering replaces the branch's streamed producer. The transcript reader pushes messages synchronously, so a staged write cannot make it wait; rows are held to 128 of them or 256 KB and then flushed in one transaction, which bounds both the retained bytes and one stall. The store owns the database and the set of files the index is behind on. It carries no query, coverage or scheduling: draining that set is the indexer's. * feat(ai-vault-search): register the index as a transcript reader consumer The index keeps its own per-file cursor in the `files` table and never consults the parse cache; the branch's AsyncLocalStorage capture scope is gone. Three refusals, each leaving the cursor where it was and recording the file for a later whole re-read: an append whose predecessor offset is not this index's own cursor (or whose dev/ino changed) is declined in `beginRead`, a staging failure ends the read's rows without throwing back at the reader, and an `incomplete` outcome never publishes. A null session drops the file's rows and still advances the cursor, because the file was read through. Nothing in production registers it: the indexer decides when the index is live. * test(ai-vault-search): measure the index's disk and latency cost The write benchmark indexes a synthetic corpus through the real reader and reports rows/s, per-table bytes per transcript MB, write amplification and rebuild time; the retention benchmark compares the batched purge against a whole-file delete. The corpus generator ships with them, so the numbers are reproducible on any host and no real transcript is ever read. * fix(ai-vault-search): decline a source the message channel cannot reach An OpenCode SQLite candidate decodes inside a worker, so the reader reports every read of it as incomplete. Staging a batch first meant one staging session and one tombstone per scan of every such session, forever. Declining in `beginRead` also keeps it out of the re-read set, because no re-read helps: the index cannot cover that source until its capture path lands. * fix(ai-vault-search): redact a message before it is cut into rows A credential is a shape, and a shape split across two chunks matches neither half: an 8000-char boundary landing inside a PEM block or a JWT indexed the key material in full, identifier shadow terms included. Redaction moves ahead of chunking, and the row type is branded so only `searchMessageRows` can mint a row an insert site accepts. `upsertFile` stops erasing dev/ino when a candidate carries none. A host that cannot stat identity (SSH, WSL, a degraded Windows stat) was overwriting a proven identity with NULL, which made the next rename-replace of that path undetectable. * test(ai-vault-search): pin each index guard on its own Three guards were each shadowed by another, so mutating any one of them left every test green. Driving the store directly separates the writer's own predecessor-offset check from the consumer's cursor check, and a stubbed store proves `beginRead` refuses before the writer is ever asked. `publishable` gets the case it never had: a second read of the same path publishes first, and the stale stage is tombstoned rather than resurrected. It covers the overlap the parse file lane normally prevents, so the invalidation slot being per-path is not load-bearing on its own. Two ratchets: every provider fingerprint needs a fixture that reaches past `SECRET_ANCHOR`, and every FTS read in the index modules must subtract staged rows through a visibility view. * fix(ai-vault-search): rebuild an index that cannot be trusted, and index retention Opening self-heals. A file too torn to open, or one whose recovery fails, is unlinked with its sidecars and reopened once; a second failure throws. Before this a torn index broke open permanently, even though the index is a cache over the transcripts and throwing it away costs only a re-scan. A `meta` table with no readable version row is now `stale`, not `fresh`. Seeding the current version over it would have kept whatever rows the old schema left. `fresh` means no `meta` table at all, and no longer triggers a rebuild: the first open of a new profile was doing create-remove-create. Recovery moves inside open, because retiring a batch that outlived its writer is part of opening the index rather than of using it. Its append case gets the test it never had: 200 rows staged onto a live session and abandoned leave the published generation intact and nothing else. Also an index on files(mtime_ms), so the retention pass seeks the expiring end of the list instead of scanning and sorting it, a pragma read-back test, and a sessions.file_path comment that says what is actually true of OpenCode. * fix(ai-vault-search): key a session's cwd the way the sidebar already does `cwd_key` had its own normalizer, which qualified a WSL cwd with its distro: `/home/me/repo` was stored as `//wsl/ubuntu/home/me/repo` while the sidebar's `folderGroupKey` stored `/home/me/repo`. Any later join between an indexed hit and a sidebar group would have returned nothing for exactly the WSL users the qualification was meant to help. The key is now the shared normalizer verbatim, pinned against `folderGroupKey` for POSIX, Windows drive, /mnt/c, both WSL UNC aliases, a Linux path, and the root. The collision it guarded against is real: two distros both spell `/home/me/repo`. It is also a collision every SSH host has, neither key qualifies for SSH, and the honest fix is a column naming the execution host rather than a path key that only some hosts spell differently. `/` now keys as `/` instead of the empty string. An empty key cannot be told apart from a session with no cwd, and the scope filter builds its child prefix as `key + '/'`, which would have been `//`. * test(ai-vault-search): widen the FTS visibility ratchet past whole literals Concatenating or interpolating a table name hid it from the per-literal scan, so `'SELECT rowid FROM ' + table` passed. The unit is now the file, the scan covers src/main and src/relay rather than one directory, and both bypass shapes are checker unit tests. Exemptions carry a reason and the census fails on one that has stopped being needed, which is how the schema module lost its. * fix(ai-vault-search): keep a failed rebuild's real cause, and read per statement Closing the stale handle before the unlink left `db` dangling: if the unlink or the reopen threw, the failure path closed it a second time and node:sqlite's ERR_INVALID_STATE replaced the real cause. Nothing classifies that as worth retrying, so an index a virus scanner or a second Orca was holding would never rebuild. The handle is nulled while none is open. The visibility ratchet moves from the file to the statement. A file is far too coarse for what it exists to guard: the query module will name both views somewhere, and that whitelisted every raw read in it. Statements come from literals and their concatenation chains, split on `;`, and a table name assembled at runtime counts as a read in any file that names an FTS table. The roots now cover cli, shared and preload as well. Half a recorded identity is now stated to be no identity. `remote-session-file-stat` spreads dev and ino independently and the COALESCE preserves whichever half a host could prove, so one number cannot tell a rename-replace from a same-file re-read; comparing it would decline healthy resumes on a coincidence. `discard` clearing the staging slot gets a test: without it the map grew one entry per path for the store's life. * fix(ai-vault-search): give a behind consumer a way to get the read it needs `markStale` promised a whole re-read that nothing could deliver. The reader picks append or replace from the session list's resume point, so with an empty index and a warm parse cache every read arrives as `append`, the consumer declines every one, and nothing is ever indexed. That is the state on first enablement inside a running app. `requestWholeTranscriptRead` in the reader drops that path's resume point, which is the one lever that changes the next read's mode, and it lives with the cache that owns it rather than with the consumer that wants it. `takeStale` documents that its paths need it before a scan is re-dispatched. Pausing no longer empties the re-read set. `acceptsCandidate` answers whether a write may start now and was being used for both jobs, so reconfiguring retention while paused dropped every entry and a declined read during a pause was never recorded at all. Retention alone prunes; a paused decline is recorded, bounded, with the oldest dropped and the drops counted, because a set that silently forgets is worse than one that says it is incomplete. A read that decodes no session now tombstones its staged rows before the store schedules cleanup. The cleanup lane reads the tombstone table the moment it is scheduled, so the old order left that batch on disk until some later write, and for the last read before a shutdown that is never. * fix(ai-vault-search): create the directory the index lives in Nothing made `<userData>/ai-vault-search/`, and SQLite's failure for a missing parent is `unable to open database file`, which is correctly not classified as corruption — so the retry never fired and the feature stranded on any profile that had never held an index. * refactor(ai-vault-search): store transcript content as written Decision: the index does not redact. A secret in a transcript is already plaintext under the user's home directory and is treated as compromised, so the index is a second copy of content the user already holds, not a new exposure. The reference agent-session-search products do not redact either. What a snippet may carry once it leaves this machine is a transport policy and belongs where the wire is, not in the write path. Removes the redaction module and its census, the branded row type that existed to prove redaction had run before an insert, and the two chunk-boundary tests. `insertSearchMessage` takes a plain chunk again and identifier shadow terms come off the raw text. `src/main/observability/redactor.ts` goes back to what main has, so this PR no longer touches it at all. Chunking and the fork digest are unchanged; the digest always folded raw message text, ahead of chunking, so its tests hold as written. Cost: redaction was 14 ms per 10.5 MB of transcript, about 3% of a rebuild and below the benchmark's run-to-run spread, so the write numbers are unchanged at roughly 22-25k rows/s and 22-27 MB/s. * fix(ai-vault-search): hide a tombstoned session's messages, not only its row `visible_sessions` already subtracted session-keyed tombstones; the message half filtered on the batch pointer alone. A published row outlives its session row until the cleanup lane reaches it, so between a `replace` publish and that drain both generations answered, and a removed file kept answering after its session was gone. Both views now subtract the same set, over a partial index so neither read scans the tombstone table. * fix(ai-vault-search): drain the rows a read that never published staged Only `writePublished` and `removeFile` scheduled the cleanup lane. A read that staged rows and then declined to publish them tombstoned its batch and told nobody, so 256 rows of a 300-message incomplete read sat in `messages` and both FTS tables until some unrelated write happened to schedule a pass. Open-time recovery had the same hole: it wrote the tombstones and drained none. The abandoned branch now schedules the drain the published branch already got, and opening schedules one pass for what recovery just tombstoned. Recovery no longer adds a batch tombstone when the session-keyed one already covers those rows, which it did once more on every reopen. * fix(ai-vault-search): continue the cursor of a file that decoded no session A read that decodes no session — an excluded Codex worker transcript — advances the cursor and leaves `files.session_row_id` null. Every later append was then declined, so a file that only ever grows would be re-read whole on every pass for the rest of its life. An append now only has to continue this index's own cursor; it creates the session row when there is none to resume. That also collapses the `append` flag, which meant both "resume this session" and "do not own it", into the resumed row id itself. * refactor(ai-vault-search): drop the index path module nothing calls No caller in this PR or the two that follow it: the composition root that would capture the userData path is PR 3b's. * refactor(ai-vault-search): read the schema version as stale or not Only the stale answer was ever acted on; the fresh/current split named two ways of being fine. * fix(ai-vault-search): read the resumed session id off an optional row * refactor(ai-vault-search): leave page warmup and the freshness check to PR 4 `warm()` and `session-search-page-warmup.ts` have their first caller in the query engine, which is what knows which pages are worth warming; the module itself went with the previous commit. `isSessionSearchFileCurrent` has its first caller in the reconciler. `session-search-file-cursor.ts` stays, because `fileIdentity` and both its types are load-bearing here. * refactor(ai-vault-search): stop pruning the re-read set on a retention change The prune walked the whole set to drop what the next `beginRead` would refuse anyway: `acceptsCandidate` applies the window when the re-read is dispatched, and `markStale` applies it again before recording anything. The bound stays. * fix(ai-vault-search): keep a batch tombstone from outliving its batch row Draining a session-keyed tombstone deletes the session's batch rows, but left any batch-keyed tombstone naming them in place. `search_write_batches` empties on every publish, so ids restart low and the next read takes the freed rowid; the stale tombstone then deleted that live batch's staged rows and the session published missing messages, with no re-read to fill the gap. The session drain now clears those tombstones in the same transaction. * refactor(ai-vault-search): commit a file's rows and its cursor in one transaction The staged-publish model is replaced by one SQLite transaction per file in WAL mode. A read buffers its decoded rows and writes them, its session and its cursor together; a reader on another handle sees the last committed state, which is the "never a torn session" guarantee the staging machinery was built to provide. A crash rolls the whole file back and it is re-read. Gone with it: `search_write_batches`, `search_pending_deletes`, `messages.batch_id`, `sessions.index_ready`, both `visible_*` views, the open-time recovery pass, the cleanup lane and `settled()`, and the ratchet test that made every query site read through a view. Rounds 2 through 6 were all seams between those pieces. A file whose rows exceed `SESSION_SEARCH_COMMIT_CHARS` is cut into chunks. The reader only hands out a byte offset when a read finishes, so a chunk records one no append can continue from: its rows answer searches as a coherent prefix of the session, and the next whole read replaces them. Retention keeps no record of unfinished work. It cuts a session loose from its file in one small transaction, which is what stops it answering, then reclaims its rows in bounded batches and hands the freed pages back as it goes. Rows whose session row is gone are the record of what an interrupted purge left. Deleted for want of a caller in PR 2, 3 or 4: the WAL budget (a buffered write has no staging window to grow one across), the compaction module (folded into the drain), the `sessions_content_hash` index (fork folding runs in JS over rows PR 4 already holds), `lastWriteAt`, `failures`, `SessionSearchStoreOptions`, `openStageCount` and `chunkMessageText`. * test(ai-vault-search): price a per-file commit and the ceiling that bounds it The write benchmark now times every transaction, because with one per file that is the whole stall a file costs. A second phase indexes a single synthetic 100 MB transcript, which is what the commit ceiling is chosen against. * fix(ai-vault-search): stop a fenced write reopening a transaction per message A chunk write that finds the file record moved under it — a `removeFile`, or an overlapping read of the same path — can never land anything afterwards. It kept buffering, so every remaining message re-entered `BEGIN IMMEDIATE` only to roll back, once per message for the rest of a file that may be a hundred megabytes. It now stops at the first refusal and drops what it holds. * fix(ai-vault-search): never hand a live session the id of a purged one `sessions.id` was a plain rowid alias, so SQLite reissued it as max+1. That id names rows in `messages` for far longer than the row itself lives: retention cuts a session loose in one transaction and reclaims its messages over many, and a session created inside that window was handed a freed id and adopted whatever of the purged conversation the drain had not reached. The drain then skipped those rows for good, because their session row exists again, and a search answered for a purged transcript under a live session's name. Reachable with no crash at all: an append of a growing transcript the parser decoded no session from creates its session row mid-purge. AUTOINCREMENT is the class fix; the schema version goes to 3 so a file written by an earlier commit of this branch rebuilds rather than keeping a plain-rowid table under a `CREATE TABLE IF NOT EXISTS`. `messages.id` was checked and is not exposed to the same window: a row and its two FTS entries always go in one transaction. Two smaller ones in the same pass. `removeFile` did not fence a read of a path this index had never written: `current()` compared a cursor, and on an unknown path the absent row and the absent expectation are both undefined, so the write recreated the source after its owner had proven it gone. The writer now counts removals per path and a write compares that count, which is a positive fence rather than an inferred one. And `drainOrphanedMessages`, `CONTENT_HASH_MESSAGE_LIMIT` and `CONTENT_HASH_MIN_MESSAGES` are no longer exported: nothing outside their own modules reads them. * fix(ai-vault-search): report a half-written file as held, not as unknown `indexedFile` returned null for a file a chunked read left partway through, the same answer it gives for a path the index has never seen. A caller asking "do you hold this file" therefore read a half-written one as new, asked for whatever read the parse cache offered, and the reader picked append — which the consumer then declined. Only the stale set healed it, a cycle later. It now reports the record with a null `byteOffset`, and `requiresWholeRead(indexed)` names that state. Null rather than an added flag because the mtime and size on that record are the file's real ones: a freshness check comparing only those would call a half-written file current, and a boolean is easy to not read, while every site that does arithmetic on the offset has to say what null means at compile time. The test also found the writer accepting an append that passed the partial sentinel back as its predecessor offset. Nothing in the reader produces a negative offset, but the sentinel is not a byte offset and no caller should be able to continue it; `beginWrite` now refuses it outright. * fix(ai-vault-search): cut a chunk at whitespace, never mid-token The 8,000-char chunker backed up only to a newline, and only when one sat in the second half of the window. A wrapped paragraph, a CJK transcript or a minified log has no newline there, so the cut landed inside whatever word straddled the target: the user's term was filed as two halves and matched neither. It now backs up to the last Unicode whitespace in the second half, and falls through to the target when there is none, because 4,000 characters without a space is not a word. A phrase that straddles a chunk boundary is still not matched. Chunks are separate FTS rows and FTS5 cannot span them; that is stated at the chunker. * feat(ai-vault-search): cap an indexed tool row at 3,072 characters Tool output is 80-97 % of a transcript's bytes and a single message may be a quarter of a megabyte, so without a cap the index, the buffer a read holds and the transaction it commits are all sized by how much a tool printed rather than by how much is worth searching. A `tool` message now becomes one row of at most 3,072 characters, kept from the head, where the command and the first lines of its output are. User and assistant text is never capped. Measured on the write benchmark, 40 sessions with tool output at 95 % of message text (the real band), per transcript MB: index 1,334,126 -> 353,973 bytes, write amplification 1.27x -> 0.34x, rows 18,804 -> 9,600, rebuild 1,676 -> 379 ms, largest transaction 49.7 -> 12.1 ms. On the default corpus, whose tool results are 1.4 KB and so under the cap, nothing changes at all: 2,167,887 bytes per transcript MB on both arms. The corpus generator takes `toolResultWords` and the benchmark reads ORCA_SEARCH_BENCH_TOOL_WORDS so both arms are the same harness. * fix(ai-vault-search): check the commit ceiling per row, not per message The buffered-chars check ran after a whole message had been folded into rows, so a single message could carry a transaction as far past the ceiling as it was large. A conversation turn is one message and can be megabytes; the ceiling exists to bound how long one commit holds the process and how large a WAL it produces, and neither bound survived a message that overshot it. * style(ai-vault-search): undo a stray reformat of untouched assertions Three assertions in the file-write test were expanded by a formatter run that was not the repo's, and the expansion survived because a multi-line object literal is preserved once it exists. Restored to what they were. * feat(ai-vault-search): write a session's identity with its first commit A chunked read created its session row with an empty session id, an empty title, an empty resume command and null cwd and timestamps, and only filled them in on the final commit. Those chunks answer searches the moment they land, so until the read ended every hit they produced named a session nothing could identify — and a crash between chunks left it that way for good, since the re-read that heals it is the same read starting over. The reader already knows the id, cwd and timestamps from a transcript's opening lines; it just had nowhere to put them. `TranscriptReadStart` now carries an optional `identity()` the consumer calls during the read, backed by an optional `identity()` on `ResumableSessionParseState`: one line in the shared accumulator fold (which covers cursor, copilot, droid, gemini, antigravity and the graph parsers) and one each in the Claude and Codex folds, which keep their own state. A chunk commit writes what it returns; the final commit overwrites it from the decoded session, so the mid-read title stays provisional. Two `cwd` guards in the Codex fold collapse into the `?? ` form the `branch` line beside one of them already used. `extractString` never returns an empty string, so it is the same assignment in one line instead of four, and it is what keeps the file under the 300-line cap with the identity accessor added. * refactor(ai-vault-search): drop conversation_fts for a column filter The second FTS table existed for query latency alone: a column-filtered `messages_fts MATCH '{user_text assistant_text}: q'` returns the identical rowid set, which PR 2's round 5 review proved and PR 4's benchmark re-checked per query. PR 4 then measured the filter at 1.16-1.36x the p95 of the second table on a 105 MB corpus across two points in the tool-output band, under the 2x bar the decision was set at, and closes the stack's open decision 3. It cost a quarter of the index on a corpus whose tool output is half the message text. With the tool-row cap it is a smaller saving than the decision was framed around, but it is still a table, a write per conversational row and a delete per reclaimed one. Schema version 4: `CREATE TABLE IF NOT EXISTS` is a no-op over an existing index, so only the bump makes a file that still carries the table go. * feat(ai-vault-search): expose the index handle a composed reader queries PR 4's engine reads the index this store owns. One getter lets it compose over the store's handle instead of opening a second connection to the same file, and it carries the two rules this PR measured: never hold a read transaction across an await, and no `.iterate()` outliving its statement. Either pins a read snapshot, and a checkpoint cannot pass one, so the WAL grows without bound for as long as it is held (10 MB to 266 MB on the write benchmark). * fix(ai-vault-search): cut a chunk at punctuation, not only whitespace The 8,000-character chunker backed up to the last whitespace in the second half of the window and fell through to the target when it found none. A minified tool result has none to find: valid minified JSON runs past 8,000 characters without a space, so the cut landed inside whatever word straddled the target, and a search for `pericardium` matched neither `perica` nor `rdium`. That is the shape most likely to be chunked in the first place, because tool output is 80-97 % of a transcript's bytes. The backoff now takes any character that cannot be inside a token — anything outside a letter, digit or underscore — and still falls through to the target when the window holds none, because 4,000 characters without one is not a word. Surrogates are excluded from the class so a cut never lands between the halves of an astral character. A few of the tokenizer's own `tokenchars` (`. - / +`) are cut on even though unicode61 keeps them inside a token. That costs the joined form of a path, and only in a window with no whitespace anywhere, which is a far smaller loss than the torn word this exists to prevent. * fix(ai-vault-search): never chunk a read that cannot name its session `updateProvisionalSession` is fed by the resumable readers alone. Claude and Codex carry an `identity()` off their fold; `readWholeTranscript` supplies none, because a format rewritten in place has no resumable state to ask. So a Grok, Cursor, Gemini or OpenCode file large enough to pass the commit ceiling published its chunks under a session with an empty id, an empty title and a null cwd — rows that answer searches at once and that an interrupted read leaves behind for good, since the re-read that heals them is the same read starting over. `add` now commits a chunk only while the read can name what it is writing. A read with no identity, or one whose parser has decoded no id yet, keeps buffering and commits whole at `finish`. The whole-file formats are the ones with nothing to give and they are small — the largest on this machine is 5 MB — so buffering one to the end costs nothing, and chunking stays reserved for the readers that can say which session a prefix belongs to. `updateProvisionalSession` takes a non-null identity now, so the invariant is the type rather than a guard that silently wrote nothing. * perf(ai-vault-search): cut a replaced generation loose instead of deleting it The first transaction of a replace deleted every row of the old session before inserting its own bounded chunk, so the transaction was sized by the history being replaced rather than by the rows being written. On the synthetic 100 MB transcript the longest replace transaction was 1,255-1,283 ms against 668-672 ms for the same file read fresh, and the gap grows with the session. A replace now mints a new session row, points the `files` row at it and deletes the one old `sessions` row, all in the same transaction. Every retrieval joins `sessions`, so the old generation stops answering the moment that commits — the same thing retention's first transaction does — and its messages are reclaimed afterwards by `drainOrphanedMessages`, the bounded batch loop that already exists for exactly this set. `sessions.id` is AUTOINCREMENT, so the freed id is never handed to another session while those rows still name it. The longest replace transaction is now 746-848 ms, the same band as a fresh read. The trade is stated plainly: the pass does more total work, 3.7 s against 3.1 s, because the reclaim is 360 bounded transactions with an incremental_vacuum step each instead of one large delete. It yields between them, so none of it holds the process, and returning the pages per batch also takes the index from 173.6 MB to 162.0 MB. The store schedules the drain off the committing stack: an async function runs synchronously to its first `await`, so calling it inline would put the first batch back inside the transaction's own call. One drain at a time, with a repeat flag for a replace that commits while one is running. * fix(ai-vault-search): preserve search tokens and index Codex tools * fix(ai-vault-search): match SQLite marks and Codex file-change records * fix(ai-vault-search): preserve stat pairs and validate benchmark results	2026-09-10 23:28:39 -04:00
3 people	254a07bb05	fix(release): port three release-gate fixes to main so cuts stop re-inheriting them (#19945) ... * fix(release): prune optional natives before the linux arch floor check The linux-arm64 release build failed packaging, and retried three times: [verify-linux-glibc-floor] 1 bundled native binary is built for the wrong architecture (target arm64): resources/node_modules/@parcel/watcher-linux-x64-glibc/watcher.node is x64, expected arm64 (from its own path) `afterPack` ran `verifyLinuxGlibcFloor` at its top, before `prunePackagedRuntimeNodeModules`. A cross-build intentionally installs every optional native variant, so at that point the arm64 slice still carries the x64 `@parcel/watcher` package that the prune exists to drop. The check was reading a file that was never going to ship. Move the floor check below the prune so it inspects the binaries actually packed. Same ordering as 35f1b0ebb9 on the v1.4.197 release branch, which shipped green and was never merged back to main. (cherry picked from commit 6400598212) (cherry picked from commit a883e17116) * fix(release): restore the minified telemetry constant fallback The macOS, Windows, and both Linux release builds all failed on "Verify telemetry constants present in app.asar", blocking publish-release: ::error::BUILD_IDENTITY constant missing or unexpected value in dist/mac-arm64/Orca.app/Contents/Resources/app.asar The verifier printed no context sample, because the string `BUILD_IDENTITY` does not occur anywhere in the shipped bundle at all. #17527 added `minify: 'oxc'` to the main bundle, so oxc renames the module-local `const BUILD_IDENTITY` to a short identifier. `BUILD_IDENTITY_RE` keys off the literal name and therefore cannot match a production bundle. `MINIFIED_TELEMETRY_RE` matches the adjacent injected identity/key pair instead, which survives renaming. #11019 created these patterns without that fallback, and its own comment predicted this exact failure if minification were ever enabled on the main bundle. The fallback was written on the v1.4.197 release branch in 35f1b0ebb9 and never merged back to main, so main has never been able to verify a minified bundle. Verified against a real local bundle built with the release env vars (ORCA_BUILD_IDENTITY=stable, ORCA_POSTHOG_WRITE_KEY=phc_...): the bundle does not contain "BUILD_IDENTITY"; both the narrowed and the widened BUILD_IDENTITY_RE fail; MINIFIED_TELEMETRY_RE matches and recovers identity=stable plus the write key. (cherry picked from commit 39c3e44cd1) (cherry picked from commit d0bbe4475d) * fix(skills): stop asserting readdir order in the skill root walk test The Windows skill-sharing release gate — which blocks publish-release — failed on this test, wedging the 1.4.198 cut. The implementation is correct; the assertion was not. `findSkillFiles` pushes results in `readdir` order and dedupes visited directories by realpath, so of the 32 junctions pointing at one target exactly one survives. The assertion hardcoded both the array order and which link won. `readdir` order is filesystem-dependent: APFS and ext4 return SKILL.md first, while NTFS enumerates its filename index alphabetically on the uppercased name, where "LINK00" sorts before "SKILL.MD" (L=0x4C < S=0x53). Windows therefore returned the same two paths in the opposite order. Assert the contract instead: the real file is present, exactly one link-routed path survives dedup, and nothing else does. (cherry picked from commit 9a832e9bda) (cherry picked from commit 505b85cd0d) * test(release): pin ported release gate fixes * style(skills): apply pinned test formatting --------- Co-authored-by: Neil <neil@stably.ai> Co-authored-by: Merge Sim <sim@local>	2026-09-10 19:30:47 -07:00
Jinwoo Hong	4b1b7178ad	fix(orchestration): scope @ group addresses to the sender's Run (#19783) ... * fix(orchestration): scope @ group addresses to the sender's Run `@all`, `@idle`, and the agent-name groups (`@claude`, `@codex`, ...) resolved against every terminal on the host. A coordinator meaning "my three reviewers" reached 126 agents across every open project, twice in one day, and every unrelated agent burned a turn discarding mail that was never for it. Every group except `@worktree:<id>` now means the live Dispatches of the sender's own Run, each addressed as `dispatch:<id>` so delivery is durable even when the worker terminal is not attached yet. A sender bound to no Run is refused with `invalid_argument` naming `run:<id>` / `dispatch:<id>`; there is no host-wide fallback and the host's terminals are never enumerated for it. `@idle` and the agent-name groups filter within that set by the same terminal status and host-resolved identity as before. `ask --to @group` returns the same code and points at the owning Run mailbox. Federated Dispatches read relayed control mail rather than a local mailbox, so a Run-scoped fan-out skips them with a `recipient_unreachable` warning naming the direct `dispatch:<id>` address. Group addresses are resolved host-side, so no RPC or stream shape changes; an older CLI sending `@all` to a new host gets the Run-scoped meaning. Claude-Session: run-scoped-group-addresses * fix(orchestration): revalidate legacy takeover before the recipient verdict A legacy coordinator taken over while `listTerminals` was in flight reported `runtime_error` instead of `legacy_read_only`: Run scoping made "no live Dispatch in this Run" the first thing the group send could fail on, and that threw before the takeover check ran. Takeover is a precondition, not a commit-time detail — the sender must be told it is read-only whatever else is wrong with its recipient set. Revalidation moves to immediately after the only `await` in the path. Everything below it is synchronous, so the commit-time window it used to guard is unchanged; only the error paths now see it. The legacy partition test gave `term_current_worker` no Dispatch, so under Run scoping it is correctly not a recipient. It now holds a real current-contract Dispatch in the same adopted Run, which is what the test is named for: one `legacy_direct` and one `current_delivery` recipient in one fan-out. Claude-Session: run-scoped-group-addresses * fix(orchestration): address the Run a nested coordinator created, not its parent A nested coordinator is both a worker of its parent Run and the coordinator of the Run it created. `resolveMessageRun` answers with the parent, correctly, because that is where its own `worker_done` belongs — but audience is a different question. Scoping `@all` to that Run sent a nested coordinator's "shared context" to the siblings it was started beside instead of the workers it started, and reported success, so it never learned its sub-workers heard nothing. Before Run scoping the host-wide fan-out reached the sub-workers by accident; this turned an over-broad delivery into a wrong-audience one, the exact failure class the change exists to remove. Group audience now resolves off the Run the sender coordinates, falling back to its Dispatch's Run. A leaf worker coordinates nothing and is unaffected. This is a separate question from `routing.run`, not a second answer to the same one, so `resolveMessageRun` keeps its meaning for point-to-point mail. Also: when every live Dispatch in a Run is federated, the fan-out skipped them all and threw a bare `Error` that discarded the warnings naming those remote workers and how to address each one. The sender was told "no recipients" while three remote workers existed. That throw now carries a code and the skip explanations. Claude-Session: run-scoped-group-addresses * docs(orchestration): say that no group address reaches a coordinator A coordinator is not a Dispatch, so Run-scoped groups never include one. That follows from the rule, but nothing said it, and the old host-wide meaning did include the coordinator — a worker sending `@all` to raise a blocker would be heard by its siblings and by nobody who can act. The guide, the CLI note, and the docs page now say to use `run:<id>` for that, and that a worker which created its own Run addresses that Run's workers. Also restores the `@cursor` case dropped when the group tests moved: a Claude pane titled "Fix the text cursor blink" must not receive Cursor's mail. That hazard was recorded from real titles and `@droid` alone did not cover it. Claude-Session: run-scoped-group-addresses * fix(orchestration): preserve group audience and mailbox identity * fix(orchestration): validate group scope before dispatch routing * fix(orchestration): preserve pane identity and exclude coordinator dispatches	2026-09-10 15:06:03 -04:00
Brennan BensonandMerge Sim	7197593e31	fix(lint): preserve deliberate collator benchmark baselines (#19686) ... Co-authored-by: Merge Sim <sim@local>	2026-09-08 23:53:17 -07:00
Brennan BensonandMerge Sim	72befaf360	feat(native-chat): show Claude subagent activity on the shared carrier (#18806) ... * feat(native-chat): show Claude subagent activity on the shared carrier Claude's `message:system:task_*` frames are classified `status-chrome` and reach the transcript as nothing at all, so a turn that spawns subagents renders as an idle turn. The journal translator now reads them into the shared subagent-group carrier — no new UI, and the frames stay `status-chrome` so nothing prints a raw opcode row. `local_agent`, `local_workflow` and `local_bash` tasks share that channel and all carry a `tool_use_id`, so `task_type` is the discriminator and a backgrounded `sleep 20` stays out of the roster; `subagent_type` covers releases that predate `task_type`. `skip_transcript` tasks never render, `is_backgrounded` children survive the turn-end sweep, and a resumed task re-announced under a fresh tool id is aliased onto its `task_id` rather than duplicated. A child still reported as working when the turn — or the session — ends becomes `unverifiable`: contact was lost, which is not evidence it exited. * fix(native-chat): stop the Claude subagent roster dropping its own rows The roster published under the same coalescing key it appends the row with, and the sink queue replaces any queued operation sharing a key regardless of kind: once a write was in flight, each new append evicted the pending publish and the next publish evicted that append, so the body never reached the journal and `lastSerialized` had already moved past it. Publish now takes the sink's own slot, as the Codex streams do. A tombstoned row could never come back: the non-batch item-row builder derived its revision from `items` alone, so a re-add was built at revision 1 against a tombstone at 2 and the reducer discarded it forever. It now takes the same `max(items, tombstones)` the batch builder already used — reachable here because an announcement that reveals a `local_bash` task empties and tombstones the group row that a genuine subagent later in the turn reuses. `settleTurn` swept whatever group the key named at the time it ran, so children rostered before any turn key existed were never swept, and a turn whose result never arrives was left working forever. The ending turn's key is now an argument, a superseding turn start settles the turn it replaces, and every turn end also sweeps the outside-turn group. Teardown without an `ended` event, and eviction past the group bound, both lose contact instead of stranding a row at `working`. Label ordinals are a high-water mark now: releasing one on a re-label handed the next child an ordinal that was already on screen. * fix(native-chat): bound subagent-group blocks on every wire that carries one Adding a fifth arm to `NativeChatBlock` made every consumer that assumed four wrong. Two of them ended in `return block`, so they compiled while handing a roster straight through: the mobile RPC sanitizer shipped it unclipped past both mobile char caps, and the legacy transcript import stored an untrusted roster unbounded. Both now clip each label and cap the entry count the way they bound their other blocks. The remaining three sites did not compile at all. The worker transcript payload and the live-session benchmark get real arms rather than casts — a cast would have turned the transcript one into a third silent passthrough inside the wire byte budget — and the CLI worker output renders a roster with its shared summary instead of `[image omitted]`. The mobile sanitizer moves to a sibling module beside the image-block one: the file sat exactly on the max-lines bound, and the block bounds are a self-contained concern with their own caps. Also caps the roster's `subagent_type` label fallback, which reached the journal uncapped, and covers the new block type in the schema audit. * test(native-chat): cover the capped subagent_type label The roster stores the frame's label verbatim, so the cap on the `subagent_type` fallback is the only thing bounding it. * fix(native-chat): type the roster fixture so the suite typechecks The mobile-cap test built its entries with an inferred `state: string`, which is not a `NativeChatSubagentState` — the only typecheck failure on the branch. * fix(native-chat): stop child traffic rostering an id Claude never announced `observeChildActivity` minted a provisional row for any `parent_tool_use_id` outside the excluded set. An id that was never announced is never excluded, so a nested Task, a workflow child, or a grandchild parented to a tool id inside the sidechain each produced a permanently unlabelled `subagent` row that could only ever end `unverifiable`. The bounded exclusion set cannot cover an id no frame ever declared, and in a long session it can forget a genuine exclusion. Track instead whether this CLI announces tasks at all — set by ANY `task_started`, including one the subagent filter rejects. Once it has, an undeclared child is provably not a new subagent, so no row is created. The provisional path now serves only releases that announce no task frames, which is what its comment already said it was for. The label-ordinal test moves to an announcement-driven removal, the scenario that path now actually reaches; it still fails if `remove` releases the ordinal. * fix(native-chat): outrank the tombstone when building one too `buildJournalTombstoneRow` still built its revision from `items` alone, leaving it asymmetric with the item builder. It is correct today only because `upsertItem` clears the tombstone whenever a re-add wins — an invariant that lives in the reducer and was not pinned. Apply the same `Math.max`, and pin the invariant so the reducer cannot drop it silently. * fix(native-chat): stop an unrelated turn end settling an outside-turn child `settleTurn` swept the `outside-turn` group on every turn end, so a child Claude announced while no turn was live — a frame trailing the previous turn's result, or one that arrives before the first turn starts — was marked `unverifiable` by the next, unrelated turn ending. That state is terminal and latches, so the `task_updated: completed` that followed was discarded: loss of contact was recorded as the child's outcome on evidence that was never about that child. A turn end now sweeps exactly the group its key names. `outside-turn` belongs to no turn, so only an end with no key of its own reaches it, and what no turn end reaches `settleSession` does — reliably, since teardown without an `ended` event also routes through it. The cost is a child outside every turn showing `working` a little longer; the alternative prints a wrong outcome that nothing can revise. Also pins that a subagent announced after a task the filter rejected still rosters: the announcement path was never what the child-traffic gate closes. * fix(native-chat): bound a subagent entry's id, not just its label Every site that bounds a `subagent-group` block clipped the label and handed the id through whole. From the Claude producer the id is bounded upstream, but the legacy transcript import reads an untrusted file, so an oversized id survived into the journal and then out to every wire that replays it — 64 entries of it, since only the entry count was capped. Each site now clips the id with the helper it already uses for its other bounded fields: the journal's inline-text bound on import, the transcript payload's metadata clip, and the mobile char cap (renamed, since it is no longer a label-only cap). * fix(native-chat): surface an adverse subagent outcome in the fallback sentence The roster row's plain-text stand-in counted only `working`, so a fan-out whose children all latched `unverifiable` (or `failed`, or `stopped`) rendered as "Ran 3 subagents" — a completion claim. Mobile and paired web have no roster renderer, so that write-time-frozen sentence is the entire row there, and collapsing `unverifiable` into something that reads like success is exactly what the SSH execution boundary forbids. It now appends the worst adverse count, worst-first across failed/stopped/ unverifiable, and shows it even while siblings still work — matching the Codex lane's shared `subagentGroupFallbackText` verbatim so collapsing the two copies later is a deletion, not a behaviour change. Also bounds the provisional entry id. `observeChildActivity` wrote the `parent_tool_use_id` straight into the entry's durable id with no length cap, while the announced path already rejects an over-long id via `claudeTaskId`. Both now share `isBoundedClaudeTaskId`, and the provisional path rejects rather than truncates, as the announced one does. * fix(native-chat): stop a subagent label ordinal and a clipped roster key colliding - claimLabel probes the labels the group actually rendered instead of a per-base counter, so a generated `Audit 2` cannot duplicate a provider's own `Audit 2`. - Bound `NativeChatSubagentEntry.id` with a head plus a digest of the whole id at every site that bounds it. The id is the roster key: a prefix clip merged two distinct children onto one entry. - Correct a stale journal-reducer test comment: tombstone cleanup is a map-state invariant, no longer load-bearing for revision ordering. * fix(claude): merge duplicate unhandled-provider-frame imports * fix(claude): preserve subagent lifecycle and bounded invocation identity --------- Co-authored-by: Merge Sim <sim@local>	2026-09-08 21:44:10 -07:00
Jinwoo Hong	98fdbc4ade	fix(orchestration): file federated worker mail under the coordinator Run (#19542)	2026-09-08 04:03:07 -04:00
Neil	e182930670	test: cover input in five simultaneously flooding SSH panes (#19071) ... * test: cover keyboard input in five simultaneously flooding SSH panes * test: capture pane focus and buffers on flood input failure * test: capture pane focus and buffers on flood input failure * test: capture pane focus and buffers on flood input failure * test: record replay input loss and application fix dependency * test: record merged replay-input fix in the five-pane flood gate	2026-09-07 19:51:44 -07:00
3 people	aeddfa463d	perf(renderer): avoid per-second spinner animation events (#19407) ... * perf(renderer): avoid per-second spinner animation events * fix(bench): ensure the Electron runtime before bench:spinners The script launches Electron via Playwright but skipped ensure:electron-runtime, which every other Electron-launching bench script runs first. * docs(renderer): scope spinner pixel-tolerance claim to paused-animation checks --------- Co-authored-by: m4air <m4air@m4airs-MacBook-Air.local> Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com>	2026-09-07 19:29:53 -07:00
Neil	cbc7bb418c	fix(ci): read the changed-path list past the first pipe buffer (#19409) ... `pr-code-change-scope.mjs` read stdin with `readFileSync(0, 'utf8')`, a single read of fd 0. Once the writer outgrows the 64 KB pipe buffer that read returns early or throws EAGAIN, the script exits 0 having emitted no `name=value` pairs, and `tee -a "$GITHUB_OUTPUT"` records nothing -- so every lane the classifier gates is silently skipped rather than failing loudly. A PR opened long ago carries a stale `pull_request.base.sha`, so the gate's merge-base diff spans the whole base branch. PR #13178 diffed 13,294 files (773 KB) against a base 1,592 commits behind main and lost typecheck, test, static analysis, xterm patch sync, package and e2e to this. Stream stdin instead, matching how the sibling `pr-e2e-source-routing.mjs` already reads the same list in the same workflow.	2026-09-07 17:57:05 -07:00
Jinwoo Hong	c37413271e	perf(mobile): open a session with parallel startup RPCs and a pre-warmed terminal engine (#19260) ... Startup RPCs now fan out in parallel and the xterm engine pre-warms inside the real terminal frame while they are in flight, so the first pane inherits a warm WebView and an already-measured viewport instead of paying a round trip for it. The pre-warm opens its engine before measuring: web-ready only reports that the bundle loaded, and the WebView answers a measure with null until a terminal exists. It also pre-warms at the user's saved text size, because cell size is what the frame height gets divided by. Host writes such as worktree.activate wait for an evaluated status.get reply. Navigation still fails open when a host cannot answer one, but that fallback no longer reads as a passing compatibility verdict.	2026-09-07 13:16:44 -04:00
4 people	a899f92402	feat(windows): enable structured Codex chat on native Windows (#18519) ... * feat(native-chat): enable Windows structured sessions * fix(codex): prove native Windows process identity * style(codex): format Windows session seam * fix Windows structured Codex admission * fix(windows): reprobe missing process identity capability * fix(windows): decide folder-workspace WSL routing before the click Review found pathUsesWslUnc exported but unused, and the folder composer hardcoding worktreeUsesWslPath:false. Together those meant a folder picked under a \\wsl.localhost\ parent routed to structured chat, then got refused by the host and fell back AFTER the click -- which defeats the lane's own design goal that create cannot fail after the click. The group's parentPath is in scope at submit and the workspace is created under it, so the parent decides WSL-ness pre-click. Wires pathUsesWslUnc there and adds tests for the helper, including the unhydrated-store case that previously threw. * fix(windows): collapse the gate derivation to one call, restoring max-lines CI static analysis failed: launch-agent-in-new-tab.ts crossed the 300-line oxlint ceiling. Adding a max-lines disable is forbidden, so the two gate derivations collapse into one readWindowsStructuredGateInputs() call -- a store-backed site now adds one line and one import name instead of two. Better shape anyway: one derivation entry point rather than two reads a call site must remember to pair. * fix(windows): engage the legacy fallback when the host THROWS a refusal Review found a P1 this merge composes: neither parent could reach it. At the lane head the only structured entry was launch-agent-in-new-tab (full store-backed WSL check); on main all win32 was refused. The merge enables win32 in creation flows that pass no projectRuntime, so a WSL folder workspace, a WSL-configured repo, or a repair-required runtime now routes structured -- and the host refuses correctly, but by THROWING rather than returning {ok:false, refusal}. Callers engage their legacy-terminal fallback on the refusal CLASS, so an unmapped throw arrives as a generic RPC rejection: no fallback, empty workspace, error toast, prompt stranded in the launch outbox. Pre-merge the same action opened a legacy terminal agent. Map the host's thrown definitive refusals onto the refusal class at the launch boundary, so every creation flow -- present and future -- degrades to the legacy terminal instead of stranding. Narrow predicate: unrelated failures (ECONNRESET, empty message, non-Error) still propagate untouched. Ablation-proven: removing the mapping reddens the fallback test. * fix(windows): teach the mobile RPC double the status probe the lane added CI's first-ever run on this lane caught a pre-existing lane defect. The lane changed status.get to resolve through runtime.getStatusAfterWindowsProcessStartTimeProbe(), but never taught the mobile-surface runtime double about it, so status.get failed for mobile clients with "not a function". The lane's own test list did not include this file and the lane had zero CI, so nothing ever ran it. The real runtime always implements the method; the double omitted it. * chore: merge current main and regenerate the localization runtime catalog CI static analysis failed on a stale en-runtime-required.json: main added onboarding integration-capability keys, and the generated catalog is checked against the PR MERGE result, not the branch alone -- so it read clean locally while failing in CI. Merging current main (90780acb85) and regenerating. Gates after the merge: pnpm tc 0, oxlint 0, changed-code quality 0/56, 7 gate/lane test files 69 tests green. * fix: route structured launches by execution host platform * fix: recover paired structured session mirror on host swap * Revert "fix: recover paired structured session mirror on host swap" This reverts commit 81bfca0007. * Revert "fix: route structured launches by execution host platform" This reverts commit 47abbd354a. * fix(windows): refuse structured chat in a paired web client Reverts the two review-loop commits (restoring a tree byte-identical to the validated head) and closes the hole they were aiming at, without their cost. A paired web client's `platform` describes the browser's machine, not the host that will run the agent, so the Windows gate cannot be evaluated there. Before this, a browser on macOS driving a Windows runtime read "not win32", skipped the creation-time proof entirely and allowed structured chat — fail-OPEN, the dangerous direction, bypassing the guarantee this lane is built on. `isWebClient` is a required input like the other gate fields, so the compiler enumerated all seven call sites. Refusal is synchronous and fail-closed: no async round-trip, no null window, no cache to invalidate — unlike keying on an asynchronously-fetched host platform, which would have made every desktop launch wait on a round-trip to fix a paired-web-only hole. Paired web therefore gets the legacy chat until the host publishes eligibility itself; that is the proper fix and belongs in its own PR. Ablation-proven: removing the guard reddens both refusal tests; the desktop-unaffected test is a preservation check and passes either way. Gates: tc 0, oxlint 0. Known open: repos-onboarding-folder-startup.test.ts fails on this branch and passes on plain main — under investigation, NOT caused by this commit. * test(onboarding): mock the web-client check the store path now reaches The web-client refusal added `isWebClientLocation()` to the launch-route inputs, which this suite's store path reaches while adding the FIRST folder. The suite stubs `window` as `{ api }` with no `location`, so the function cleared its `typeof window === 'undefined'` guard and then threw on `window.location.pathname`. That threw inside addNonGitFolder's own catch, so folder-1 never activated; folder-2 then returned early (a project already existed) before reaching the call at all, leaving exactly one activation with no startup seed. Test artifact, not a product defect: a real renderer always has `window.location`, so the seeding path is intact for users. Mocking the module is the convention 7 other suites already use, and keeps product code free of defensive branches that only exist to satisfy a stub. Ablation-proven: removing the mock reproduces the original failure exactly. * fix(renderer): make the web-client check total over a partial window isWebClientLocation() guarded `typeof window === 'undefined'` and then assumed `window.location` existed. A window stubbed without a location cleared the guard and threw on `.pathname`. That matters because this branch put the call on the launch-routing path, where the throw is swallowed by the caller's catch and silently becomes a FAILED LAUNCH rather than a visible error. CI caught it as 9 failures in launch-work-item-direct.test.ts. I previously "fixed" this by mocking the module in the one suite I knew about. That was whack-a-mole against an unbounded set, and it missed this one. The defect is the partial-window assumption, so fix it there: the mock is removed from the onboarding suite and both suites now pass on the hardening alone. Ablation-proven: reverting to the unguarded form reddens 11 tests across the new unit suite and launch-work-item-direct. Gates: tc 0, oxlint 0, changed-code quality 0/58. * Move Codex's Windows structured-chat eligibility onto the host createSupport probe The renderer no longer decides Codex win32 eligibility: launchStructuredAgentSession probes agentSession.createSupport for both providers, the host answers via supportsCodexStructuredLocation (process start-time proof + WSL refusal), and the create path re-checks live. Deletes the client-side windows gate module and its routing inputs (windowsProcessStartTime, worktreeUsesWslPath, isWebClient, platform) from six call sites. Splits killCodexAppServerProcessTree out of codex-app-server-session to hold the max-lines ceiling without a disable. * fix(ci): keep pnpm lockfile stable * test(windows): align foreground snapshot flags * Restore main's pane-snapshot flag contract Main asks for CreationTime on both projections; this branch's hot-path isolation went away with the async probe it served. --------- Co-authored-by: Orca Worker <orca-worker@localhost> Co-authored-by: Merge Sim <sim@local> Co-authored-by: Merge Sim <merge@localhost>	2026-09-07 09:18:38 -07:00
Neil	314506003a	fix: retain MSYS shell descendants in their terminal job (#19068) ... * fix: retain MSYS shell descendants in their terminal job * test: complete MSYS regression CI registration and teardown contract * fix(windows): deny job breakaway for the whole Cygwin/MSYS shell family The per-PTY job probed only msys-2.0.dll, and only for bash.exe/sh.exe. Cygwin ships the same spawn.cc breakaway logic under cygwin1.dll, and an MSYS2 zsh escapes exactly like its bash does, so both kept the orphan bug. Probe the runtime DLL on the shell's own search path instead of matching shell names: that is the property that decides whether the runtime will ask for CREATE_BREAKAWAY_FROM_JOB, and it drops the name special-casing. * chore(patch): restore the conpty.cc index line The earlier hand-edit dropped it while every sibling section kept one. Recomputed against the real blobs: applying this patch to 7b286d3d yields exactly 4b06d185, so git apply -3 has its fallback back.	2026-09-07 00:35:55 -07:00
Jinwoo Hong	fb322046e8	skills: rewrite and trim the seven non-orchestration guides (#19128) ... * skills: rewrite the seven non-orchestration guides to one outcome-first standard Every guide leads with Result / Done / Safe failure, states conditions instead of case lists, keeps one done bar and one autonomy envelope, and loads references at the point of use via `skills get <topic> --full`. orca-cli drops from 424 to 260 always-loaded lines with three references; orca-per-workspace-env from 794 to 397 with five. Defects fixed in shipped guides: `emulator camera` (no such command), iOS `permissions` (backend refuses it), Android pane described as in development, `relayGracePeriodSeconds: 0` documented as immediate teardown (it is unbounded), doctor `ok: true` hiding `warn`, an SSH exemplar setting both `jumpHost` and `proxyCommand`, a provisioned-root fetch from `origin`, and the Linear unconfirmed-write rule keyed on four verbs when ten emit it. The resolver ladder, placeholder rule, and older-binary fallback shared by every installable SKILL.md now come from one skill-stubs/_shared/cli-resolution.md fragment composed by the generator, which also bundles per-guide references into --full. New guards: every ORCA invocation and flag resolves against COMMAND_SPECS, descriptions carry no angle-bracket tokens, reference routing is checked both ways, and an always-loaded size ratchet (300 lines) that guides may leave but never join. * skills: address review on the SSH recipe and the parity guard - ssh-host create script: route the bootstrap ssh through the chosen jump host or proxy command, refuse both at once, use StrictHostKeyChecking=accept-new instead of a blind ssh-keyscan append, and pass gh_token/project_root/repo_url/repo_ref to the remote bash via printf %q so a quote in a value cannot break out of the command. - per-workspace-env envelope: the step-10 workspace test the user asked for is no longer forbidden by the same paragraph. - linear guides: name the full verb, ORCA linear list-issues. - parity guard: a prefix reference such as ORCA linear --help or ORCA emulator --webcam now has its flags checked against every command under that prefix; only an exact path or an explicit ... was checked before. * skills: tighten prose in the seven rewritten guides Shorter outcome spines, one idea per sentence, no restated rationale after a rule. No rule, command, or pinned phrase changes; 47 net lines fewer across the guides and references. * skills: route orca-cli and per-workspace-env gates through --reference Both guides told agents to load --full at a gate because the per-reference selector did not exist when they were written. Now that main serves `skills get <topic> --reference references/<file>.md`, load only the named file and keep --full as the fallback for an older CLI, matching the orchestration kernel. * skills: drop outcome-spine boilerplate from the CLI-wrapper guides The Result/Done/Safe-failure preambles and Next Action closers restated rules the body already carries. Agents stop fine without them, and for a CLI wrapper the command surface is the guide. Keeps the one substantive rule computer-use's Done block added (never report unverified as success) inside Action Rules. orchestration and per-workspace-env keep theirs: those are multi-step workflows where the done bar is load-bearing. (cherry picked from commit 44a74baf73) * skills: trim the guides and stubs to what agents actually need - Drop the Result/Done/Safe-failure preambles and Next Action closers from the six CLI-wrapper guides; the one substantive rule (never report an unverified computer-use action as success) moves into Action Rules. - Drop the 'guide may be stale, trust --help' lines: the guide is served by the binary that runs the commands, so it cannot be stale relative to it. - Drop the status --json / open --json preflight from every guide; the stub no-guessing paragraph now says to start Orca only when a command reports it is not running. - Cut the ORCA placeholder paragraph in each guide to one line that points back at the stub's resolution. - Trim the orchestration, orca-cli, and computer-use descriptions to trigger phrases plus one line of scope. - Remove the older-binary fallback section from every stub (and its two shared blocks); a binary without skills get gets one sentence. - Remove the guide size ratchet test. * skills: apply independent review cleanup * skills: clarify guide loading and Linear command discovery * skills: harden environment recipe examples * test: complete branch rename journal doubles * skills: clarify custom Codex launch and refresh model example * test: deduplicate journal fix now present on main	2026-09-07 00:03:48 -04:00
Neil	1848855515	test: enable software WebGL for Linux CI headful specs (#19001) ... * test: enable CI WebGL and route GPU-dependent regressions * test: retain headful atlas cases in terminal rendering goldens * test: reuse golden command in project coverage assertions	2026-09-06 19:27:12 -07:00
Neil	1e301ab1df	test: cover native Wayland Hangul in isolated CI (#19174) ... * test: exercise native Wayland Hangul in isolated CI session * test: wait for nested compositor socket before selecting IBus * test: align Wayland IBus discovery with GNOME environment filtering * test: assert Wayland launch and register native Hangul evidence	2026-09-06 19:02:54 -07:00
Neil	2e8fa3fe9b	test: exercise packaged browser compatibility in scheduled CI (#19157) ... * test: exercise packaged browser compatibility in scheduled CI * test: record final packaged workflow participation evidence * test: expose manual packaged revision and simplify executable check * test: reject missing package checksum assertion	2026-09-06 17:42:49 -07:00
Neil	4120501979	perf(store): detect Zustand rerender churn the current audit cannot see (#19059) ... * perf(store): detect Zustand rerender churn the current audit cannot see The app-store-performance audit only understood inline selectors passed to a hook imported literally as `useAppStore`, so three shapes went unlinted: - a selector referenced by name (`useAppStore(selectRows)`), including one hoisted below its call site — resolved now via a Program:exit pass - the sibling store hooks (`usePluginPanelsStore` and friends), matched by the use<Name>Store convention on local imports; React's `useSyncExternalStore` matches that shape and is excluded - a fresh reference nested inside a `useShallow` projection, which is the worst case of the three: the comparator runs on every write and can never match, so the memo silently buys nothing `no-nested-fresh-under-shallow` covers the last one. `src` is clean against all four rules today, so this is a ratchet rather than a cleanup. The write side stays undecidable statically — whether a `set()` reallocated for nothing depends on the payload — so it gets a runtime probe instead. withStoreIdentityChurnProbe counts writes that replace a field's reference while its value stays equal, and can name the calling site. Cost when disarmed is one boolean load per write, matching react-commit-cascade-write-probe. * perf(store): scope the churn probe's scan to the write's own keys recordWrite iterated Object.keys of the full post-write state, so the armed cost scaled with the store's top-level field count (hundreds) rather than the size of the write. `set(partial)` merges, so no field outside the partial can have changed. The wrapper now resolves a functional updater itself and iterates the resolved partial's keys. Same function, same argument, called once — there is a test pinning that, since calling it twice would double any work a slice does inside its own updater. A replace write drops absent fields, so that path still scans every field. Disarmed cost is unchanged: one boolean load. * perf(store): follow a selector one hop into its helper Review feedback: both the lint rule and the manual sweep it was checked against only looked at the inline selector body, so neither could see a fresh allocation made inside a helper the selector calls — and delegating to a module-scope helper is the idiomatic shape here. Two methods sharing a blind spot is not corroboration. The two fresh-reference rules now resolve a single hop into a module-scope helper. The predicate used across that hop is deliberately stricter than the inline one: it requires EVERY returned expression to allocate unconditionally, so the common `cache.get(k) ?? buildFresh(state)` identity-caching shape is not flagged. An unresolvable helper is left alone rather than guessed at. Still zero hits across 20,330 files, so this stays a ratchet. * perf(store): keep the churn probe off the shipped write path Review hardening for the churn probe and the widened lint rules. Probe: it no longer resolves a functional updater itself. Zustand keeps sole ownership of when and with what argument an updater runs, so the middleware cannot double-invoke it or hand it a stale state. Object partials still scope the scan to the write's own keys; updater and replace writes fall back to the full field list, which costs one Object.is per untouched field and nothing more, since the deep compare only runs on replaced references. store/index.ts installs the probe only when import.meta.env.DEV or e2eConfig.exposeStore is set, the same gate as __store exposure. Nothing in the app arms it, so a shipped build was paying a wrapper frame per write for a diagnostic it could never read. The cascade probe stays unconditional because crash telemetry arms it in the field. Site capture now skips any *-probe.ts frame; under the real composition the first non-node_modules frame was the cascade probe's wrapper, so every churn was attributed to react-commit-cascade-write-probe.ts:32 instead of the caller. Plugin: named-selector recording is restricted to module scope. A component-local `const selectRows = ...` used to overwrite the entry for a same-named imported selector and flag an unrelated useAppStore(selectRows). The any-branch and every-branch allocation predicates are one function with a flag, the Object.* static list is a Set, and import recording is a single pass. Tests: updater called once with live state, identical-state writes ignored, disarmed path forwards exact arguments without calling get(), full composition with the cascade probe (no drop, no double, correct site), and the module-scope shadowing case for the plugin.	2026-09-06 15:12:19 -07:00
Jinwoo Hong	b8311d509a	Revert "skills: rewrite the seven non-orchestration guides to one outcome-first standard (#18724)" (#19126) ... This reverts commit 15d0f8aedf.	2026-09-06 17:01:49 -04:00
Brennan BensonandMerge Sim	1478101342	fix(windows): unblock structured native chat by exposing process creation time (#18986) ... * fix(windows): guard process creation times * fix(windows): ask the relay's bare addon for creation times too The relay addon build now emits creationTimeMs, but the runtime binding for the bare addon still declared only CommandLine, so a Windows relay host requested flag 2 and every row came back without a creation time. That leaves captureWindowsDescendantSnapshot returning null and verifyWindowsProcessIdentity false forever on those hosts -- the relay half of the patch was unreachable. Naming CreationTime in the adapter is safe because the bare addon is a content-hashed relay artifact: it ships in the same immutable relay directory as the bundle reading it, so it can never be older than the code asking for the bit. Also bound the win32 guard test on our own row, which the addon can never fail to answer, so an unconverted FILETIME or a 1601-epoch stamp fails instead of satisfying a bare count. * fix(windows): make the compiled addon prove its own CreationTime support CI caught the real defect: the win32 guard test read isWindowsProcessStartTimeAvailable() as true and then found 0 rows carrying creationTimeMs. Unlike node-pty, this package publishes a prebuilt .node at the same build/Release path node-gyp writes to, so pnpm patches the source tree and leaves that binary alone. A host then holds a patched lib/index.js -- ProcessDataFlag.CreationTime and all -- over a binary that ignores flag 4, and neither a load check nor a path check can see the difference. So the binary now says so itself: addon.cc exports supportedProcessDataFlags, lib/index.js re-exports it, and - windows-process-tree-creation-time.cjs asserts it during install, which is what forces a from-source rebuild. It is shared by the Node probe in ensure-native-runtime.mjs and the Electron probe in rebuild-native-deps.mjs, exactly as node-pty-job-ownership.cjs is -- the Electron half matters because that probe decides onlyModules, so without it the packaged app would ship the stale prebuilt. - isWindowsProcessStartTimeAvailable() gates on the reported bit, not the enum. Believing the enum is worse than reporting false: the descendant snapshot returns null forever and the exit proof latches unverifiable while structured chat believes it has a reaper. rebuildNodeRuntimeModules could not actually have rebuilt this package: the patched binding.gyp includes deps/node-addon-api, which the tarball does not ship, and node-gyp must run from the physical dir. Also closes the relay repair path's divergence: repairCreationTimeSources wrote the C++ but not the buildNode splat or the tree-node typing, and assertPatchApplied checked neither, so a repaired tree passed as patched with buildProcessTree silently dropping the field. The guard test is unchanged. * fix(windows): keep the process-tree patch LF-only windows-process-tree-patch-contract.test.mjs requires the patch file to carry no CR bytes. Regenerating through pnpm patch-commit emitted 199 of them, because the creation-time change is the first to touch files the package ships as CRLF (src/process.h, src/process_worker.cc, src/addon.cc, lib/index.js, lib/index.ts, the typings) -- and #17886's own hunks over binding.gyp and src/process_commandline.cc carry the rest. Stripping them is safe and changes nothing the lockfile records: pnpm hashes patches CRLF-normalized, so the digest stays e66202cc623996d02040c93449eb9ae353fddadf426cb53202a59ee710ee6fe7 and now equals the file's plain sha256 too. It also still applies -- verified against a deleted store entry, not a warm one -- and the precedent was already there: the previous patch was LF-only and had been patching those same CRLF files all along. ensure-native-runtime.test.mjs stages the siblings the script loads at module scope into its temp project. The import walk added by #17886 sees `from './x.mjs'` only, so the createRequire'd .cjs siblings still have to be named, and this PR adds a second one. --------- Co-authored-by: Merge Sim <sim@local>	2026-09-06 13:59:59 -07:00
Jinwoo Hong	59fe8266bd	fix(orchestration): keep worker lineage across app restart (STA-6366) (#19121) ... * fix(orchestration): keep worker lineage across app restart (STA-6366) Terminal handles are minted per process, so after a restart the projected parent (coordinator or creator) named a handle no live row carried and every worker rendered as a top-level row. The projection now resolves the parent from the durable pane keys (runs.coordinator_pane_key, tasks.created_by_pane_key) whenever the stored handle is not one this process minted, re-resolves it to the live handle for that pane, and omits stale handles so they cannot mismatch a row. The creator-pane incarnation gate is untouched: it still decides mutation authority, and display lineage no longer depends on it. Dispatch lookup also passes the pane identity so a worker's own dispatch resolves once its handle is reminted. * test(orchestration): compare lineage without the merged attention field	2026-09-06 16:53:35 -04:00
Jinwoo Hong	15d0f8aedf	skills: rewrite the seven non-orchestration guides to one outcome-first standard (#18724) ... <!-- orca-pr-loc --> <!-- Programmatic LoC summary. Do not edit by hand; rewritten on every commit. --> | | Files | Added | Deleted | Net | | :--- | ---: | ---: | ---: | ---: | | Test | 6 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$​544 | $\color{#cf222e}{\Huge{\mathbf{−}}}$​49 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$​495 | | Prod | 36 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1719 | $\color{#cf222e}{\Huge{\mathbf{−}}}$​1703 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$​16 | <!-- /orca-pr-loc --> ## ELI5 Orca ships eight skill guides that agents read before running the CLI. Seven of them (everything except `orchestration`, which #16904 rewrites) were command catalogs that had drifted from the binary. This PR rewrites them so an agent reads the outcome, the done bar, and the safe-failure rule first, loads reference material only at the step that needs it, and never sees a command or flag the installed CLI does not define. ## What changed - **Seven guides rewritten** to one standard: outcome spine first (Result / Done / Safe failure), conditions instead of case lists, one done bar, one autonomy envelope, references loaded at the point of use via `skills get <topic> --full`, every runnable invocation spelled `ORCA`. `orca-cli` is 424→260 always-loaded lines with three references (browser, automations, publishing); `orca-per-workspace-env` is 794→397 with five (provider-vercel, ssh-host, docker-ssh, windows-scripts, failure-modes). - **Defects fixed in shipped guides:** `emulator camera` (no such command), iOS `permissions` (backend refuses it), Android pane described as "in development" (shipped in June), `relayGracePeriodSeconds: 0` documented as immediate teardown (it is unbounded), doctor `ok: true` hiding `warn`, an SSH exemplar setting both `jumpHost` and `proxyCommand`, a provisioned-root fetch from `origin`, the Linear unconfirmed-write rule keyed on four verbs when ten emit it. Linear and emulator descriptions dropped embedded commands and angle-bracket placeholders (651→329, 732→404 chars). - **Generator bundles references.** `skill-guides/<name>/references/*.md` is appended to `--full`; `skills get` help says compact by default, full with references. - **Stubs single-authored.** The resolver ladder, placeholder rule, and older-binary fallback shared by all eight installable `SKILL.md` files come from one `skill-stubs/_shared/cli-resolution.md` fragment composed by the generator. Projections were byte-identical before the content fixes. - **Guards:** every `ORCA <cmd>` and flag in every guide and reference resolves against `COMMAND_SPECS` (this found the camera defect); descriptions ≤1024 chars with no angle-bracket tokens; reference routing checked both directions; an always-loaded size ratchet (300 lines) that guides may leave but never join. `orchestration` (440 lines on main) is recorded as an exception until #16904 lands its kernel. ## Relationship to #16904 Split out of #16904 so that PR carries only the orchestration guide. On main, `terminal send` has no `--wait-submit` / `--retry-request` and the orchestration kernel still carries the resolver ladder and worktree-selector rule, so this branch pins `accepted: true` for handoff receipts and leaves the orchestration pins where main has them. The merge in either direction is mechanical: #16904 rebased on this becomes a one-file `orchestration.md` change plus dropping the two exceptions. ## Standard Compound Engineering's portable skill-authoring guidance (outcome spine, conditions not cases, pinned fragile commands with an ordered hatch, references at point of use). NVIDIA SkillEvaluator Tier 1 (`schema,pii,license,quality,unicode,lint`) was run on every guide; its deterministic checks pass, its template nudges (Instructions/Examples sections, 50–150 char descriptions) do not apply to Orca's stub architecture and were not applied. ## Testing - `pnpm typecheck:tsc:cli` clean; `check:code-quality:changed` and `check:react-doctor:changed` 0 findings - `pnpm verify:bundled-skill-guides` and skill-bundle manifest verify clean - vitest over `config/scripts`, `src/cli/skill-guide-cli-parity.test.ts`, `src/cli/skills.test.ts`, `src/cli/specs/skills.test.ts`, `src/cli/help.test.ts`, `src/main/skills`: 240 files / 2,019 pass - Live smoke on the built CLI of every `skills get <topic>` and `--full`, every emulator, linear, and vm verb named in the guides, and every projection's resolver, GNOME warning, and bounded fallback (done on the #16904 branch before the split; the guide bodies are identical here except the send-receipt vocabulary noted above) ## Deferred product decisions Merging `orca-emulator` and `orca-emulator-android` into one skill with a platform branch; collapsing `linear-tickets` to a guide alias; a `skills get --reference <name>` selector so a gate table can load one file; a fresh-agent routing eval before trimming the `orca-cli` (1,015 chars) and `orchestration` descriptions, whose quoted triggers each fixed a routing misroute.	2026-09-06 16:10:16 -04:00
Jinwoo Hong	0c33f58e8a	fix(ssh-relay): daemon owns the endpoint credential; a losing start never rotates it (#19052) ... <!-- orca-pr-loc --> <!-- Programmatic LoC summary. Do not edit by hand; rewritten on every commit. --> | | Files | Added | Deleted | Net | | :--- | ---: | ---: | ---: | ---: | | Test | 19 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$​962 | $\color{#cf222e}{\Huge{\mathbf{−}}}$​136 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$​826 | | Prod | 18 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$​295 | $\color{#cf222e}{\Huge{\mathbf{−}}}$​116 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$​179 | <!-- /orca-pr-loc --> ## Symptom Live 2026-09-05 (Orca 1.4.198 client, Ubuntu host): both relay processes `kill -STOP`ped for 20 s, then `-CONT`. The client redeployed while the host was frozen. Its fresh daemon lost the socket bind (`Socket path already in use`) but had **already rewritten** `relay-<id>.sock.credential`. The surviving daemon kept its in-memory credential, so every later `--connect` got `Endpoint credential mismatch; closing socket`, then `Grace started … timeoutMs=0 … ptys=1, clients=0` every ~20 s, forever. Only a manual `kill -TERM` cleared it. Receipts: `review-archive/orchestration-v3-pr16904/smoke-receipts-t012b/E16,E17,E18,E24`. Three independent defects kept the wedge alive; each is fixed at its own seam. ## Fix **1. The relay daemon owns credential publication (race-free under two concurrent starters).** `relay-daemon.ts` binds the socket first, then publishes via the new `src/relay/relay-endpoint-credential-publication.ts`: adopt a valid pre-existing file (older clients still pre-write), else mint 32 random bytes and write temp+rename at 0600. A start that loses the bind exits inside `listen()` and never reaches the file. Why this option and not restore-on-loss or a client-side write: the only process that can *prove* ownership is the one whose `listen()` succeeded, and that proof is atomic with the bind. The client-side pre-write (`ssh-relay-endpoint-credential.ts`) and the launch-command `chmod 600`/`icacls` are removed on POSIX and Windows. The racing test also exposed that macOS reports a mid-bind collision as `EEXIST` rather than `EADDRINUSE`; `relay-socket-ownership.ts` now treats both as "held or stale". **2. The client distinguishes "no daemon" from "daemon present but not answering", and never rewrites.** A credential refusal is now typed on the wire: the daemon replies `orca-relay-handshake-credential-mismatch` (same frame type, no new opcode) and the bridge exits **43**; `waitForSentinel` maps it to `RelayCredentialMismatchError`, which the takeover treats as handshake-refusal evidence exactly like exit 42. A relay that holds the endpoint but **never refused** (the stalled-host shape: kernel backlog accepts the probe, handshake gets no answer) is now `RelayEndpointUnresponsiveError`, routed to the relay-lost backoff instead of the terminal Reset Relay path. Silence is not a decision (`docs/reference/ssh-execution-boundary.md`). **2b. Deploy honours the verdict.** The 40 s live run exposed that the `--connect` catch block in `deployAndLaunchRelay` predates the incumbent probe and swallowed both verdicts as "probe failed, launch fresh", so a fresh daemon was still launched over the live one (it lost the bind by luck, which is exactly the collision in the incident). Held and Unresponsive now propagate; the session backs off on Unresponsive and surfaces Reset Relay on Held. Red-first in `ssh-relay-deploy-incumbent-verdict.test.ts`. **3. The daemon cannot be wedged by a rotated file, because nothing can rotate it.** The credential lives in the content-hashed relay dir, and after (1) the only writer is the daemon that owns the socket, so the "file changed under a live daemon" state the incident depended on is no longer reachable in-product. The credential is therefore fixed for the daemon's lifetime, as a plain secret should be. A hand-edited file is refused with the typed reply until restored (tested). Startup adoption of a pre-written file applies an owner-only + same-uid rule (review finding): anything else is replaced by a fresh mint. An earlier revision of this PR also re-read the file on mismatch and adopted it; that was removed as unreachable machinery that turned the credential into a per-handshake file-ownership check. **3b. Fail closed between bind and publication.** A client that arrives after `listen()` resolves but before the credential is set is refused, not admitted as `unproved`. Nothing can be delivered in that window today; the guard makes the boundary structural instead of an event-loop ordering fact. Red-first in `relay-reconnect-listener-credential-gate.test.ts`. **Wire compat.** New optional handshake reply only; an old `--connect` hits `Unknown handshake type` and exits 1 pre-sentinel, which it already treated as a generic failure. New daemon adopts an old client's pre-written file; new client still passes `--credential-file` so an old daemon reads it as before. Absence of exit 43 is never used as evidence. **Also.** `terminal create` on a reconnecting SSH host now says what to do instead of a bare `No PTY provider for connection "<id>"` (prefix preserved; the renderer matches it). ## Tests (red first) - `src/relay/subprocess.test.ts`: two `--detached` starts race one socket + credential file → exactly one reaches the sentinel, loser exits 1 with `Socket path already in use`, file valid + 0600, a `--connect` reading it reaches `relay.status` and reports the winner's pid. Red before (both starters died: daemon required a pre-existing file), green 6/6 after. - `src/relay/relay-endpoint-credential-publication.test.ts`: mints after bind; adopts a pre-written 0600 file; replaces a pre-written 0644 file with a fresh mint; refuses a stale credential with exit 43 while still serving the real one, and keeps refusing a rewritten file until it is restored. - `src/relay/relay-reconnect-listener-credential-gate.test.ts`: a client in the bind-to-publish window is refused and never attached; after publication the right credential is accepted and a wrong one refused; a daemon launched without a credential file is not gated. Red without the guard. - `ssh-relay-deploy-incumbent-verdict.test.ts`: live-but-silent incumbent → `RelayEndpointUnresponsiveError`, refused → `RelayEndpointHeldError`, and in neither case is `--detached` launched; a failed `test -S` probe still launches fresh. Red 2/3 without the deploy change. - `ssh-relay-deploy-helpers.test.ts` (exit 43), `ssh-relay-endpoint-takeover.test.ts` (refused → Held even with no `lsof`; silent → Unresponsive, nothing unlinked or signalled), `ssh-relay-session-terminal-error.test.ts` (Unresponsive → `onRelayLost`, not terminal). Deploy/namespace/native-deps tests updated to assert the client writes **no** credential. ## Live proof New `tests/e2e/ssh-docker-relay-stall-credential.spec.ts` (claimed in `run-ssh-docker-e2e.mjs` and PR source routing), two cases: `kill -STOP` every relay pid in the container, send input during the freeze, hold **20 s** (the incident's duration, which races the mux liveness timeout) or **40 s** (past it for sure), `kill -CONT`; assert status back to `connected`, same pty, same daemon pid, same credential inode and content, relay.log did not shrink (a relaunch truncates it) and has zero `Endpoint credential mismatch` / `Socket path already in use` lines, in-stall input delivered at most once. Run output (local, fixture image `orca-e2e-ssh-relay:3a864c665ba2cefd`, `ORCA_E2E_SSH_DOCKER=1 SKIP_BUILD=1 ORCA_E2E_FORWARD_APP_LOGS=1 … --project electron-headless --workers=1`, head `c2c20fd994`; re-run identically on the final head after the credential-lifetime change, 2 passed (1.7m), same annotations, and the bind-to-publish refusal never fired): ``` ✓ keeps the same daemon and credential across a 20s relay freeze (38.3s) relay-processes-stopped: 2 relay-processes-continued: 2 bridge-pids-before-after: 480 -> 480 socket-clients-accepted-before-after: 1 -> 1 in-stall-input-delivered: 1 ✓ backs off and reattaches, never relaunching, across a 40s relay freeze (57.5s) relay-processes-stopped: 2 relay-processes-continued: 4 bridge-pids-before-after: 480 -> 1202 socket-clients-accepted-before-after: 1 -> 3 in-stall-input-delivered: 1 2 passed (1.6m) ``` Client log in the 40 s case shows the new path end to end: `Relay channel lost … reconnect attempt 1/6` → `Socket probe result: "ALIVE"` → `Socket reconnect failed … Relay failed to start within 10s` → `Relay endpoint incumbent: … verdict=live evidence=accepted-connection holders=unenumerable` → `Failed to re-establish relay … A relay still owns … but did not answer the handshake … Orca will retry` → `reconnect attempt 2/6` → `Reconnected to existing relay via socket`. The 20 s case never left the frozen bridge (same bridge pid, one accept), so it exercises the "silence is not death" side of the same race. The 20 s case passed 6/6 across the session; the 40 s case was red on the prior head (`Socket path already in use` + `Startup failed: listen EADDRINUSE` in relay.log from the swallowed verdict) and is green after 2b. Before the fix the same injection produced a fresh daemon that rewrote the credential and a survivor refusing every client. The `relay-processes-continued` count exceeds `stopped` in the 40 s case because the timed-out client's `--connect` bridge and the loser-side processes are parked behind the frozen listener when `CONT` runs; they exit on their own once it resumes. ## Gates `pnpm test src/relay src/main/ssh` 332 files / 3884 tests pass · `pnpm typecheck:tsc:node` clean · `check:code-quality:changed` 0 findings · `check:react-doctor:changed` 0 findings · `pr-e2e-gate-contract.test.mjs` 42 pass · no lint disables or max-lines bumps added. ## Noted, not fixed here - `terminal list` `orphaned:false` / `terminal close` `ptyKilled:true` for a pane whose relay is gone (`orca-runtime-stop-explicitly-closed-tab-ptys.ts`): different seam, `@ts-nocheck` characterization-covered file. - On a host with no `lsof`, a stalled relay still cannot be enumerated as the holder; it is now retried rather than declared held, but a relay frozen past the backoff budget still ends in the existing "reconnect manually" banner.	2026-09-06 14:39:25 -04:00
Jinwoo Hong	06a607a1d7	feat(orchestration): make multi-agent workflows durable (#16904) ... <!-- orca-pr-loc --> <!-- Programmatic LoC summary. Do not edit by hand; rewritten on every commit. --> | | Files | Added | Deleted | Net | | :--- | ---: | ---: | ---: | ---: | | Test | 225 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$​21666 | $\color{#cf222e}{\Huge{\mathbf{−}}}$​2820 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$​18846 | | Prod | 348 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$​17107 | $\color{#cf222e}{\Huge{\mathbf{−}}}$​4706 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$​12401 | <!-- /orca-pr-loc --> ## ELI5 Orca now treats orchestration like a durable control plane instead of inferring success from terminal keystrokes. Agents can tell whether a prompt was accepted or a turn started, replay an ambiguous request without sending twice, and recover coordinator mail after a crash. Completed workers can be inspected, released, or retained, and their panes no longer auto-resume as if the work were still running. ## What changed - **Run receipts** from `run-create/use/current/show/list` are the row without routing plumbing (`home_database`, `coordinator_pane_key`) and without the duplicate `binding` object. - **`terminal send` receipts are honest and idempotent.** `input_accepted` and `turn_started` are the only stages; `--wait-submit` observes without resending; `--retry-request <uuid>` replays the exact request against the same process incarnation. A transport timeout keeps the retry ID; only a different runtime answering strips it. Value-less or non-UUID `--retry-request` is rejected on the CLI and the SSH shim. - **Mailbox delivery is committed before wakeup.** Pointer writes are staged in the DB before any PTY byte, replayed once after restart, and never emit a naked Enter. The watermark that parks concurrent deliveries is released with the DB reservation. Restart rescans pointer-pending and `dispatch:` mailboxes. - **Lifecycle is a guarded transition graph** (`lifecycle-transition.ts`) with a table-driven test over every caller edge. Task reopen/overturn stays in the public contract. A PTY exit during `worker-stop` is the stop succeeding, not a failure. - **Worker lifecycle CLI:** `worker-start` (`--spec` creates Task + attempt in one call), `worker-show`, `worker-read` (provider transcript first, bounded terminal fallback with a typed reason, local/WSL/SSH), `worker-stop`, `worker-abandon`, `worker-release`, `worker-retain`, `worker-list` (rowid-fenced pagination, fleet liveness, `attention`, literal `nextAction`). - **Release is an explicit ownership table** (`decideWorkerTerminalRelease`): only an `owned` resource can be settled, the archive is mandatory where reachable, and an owner whose process is proven exited can always get out of `retained` via `archive_status: unavailable`. User-taken-over, external, and transferred panes stay retained. - **Settled-worker resume fence** (folds in #17651): a settled dispatch whose pane is still open is fenced at settlement, on stop/abandon/exit, and at startup; lifted on release, retain, takeover, and pane reuse. - **Liveness is `live` / `unverifiable` / `exited` only**, from execution-host evidence. Fleet projection reads the evidence clock, not the relay delivery clock. A host-certified exit outranks the worker's settled state. `unverifiable` never authorizes stop, abandon, retry, or release, in code or in the guide. - **Federation:** structured reads negotiate by `method_not_found` so every shipped host keeps transcript-first output; exited remote workers are closed before being reported closed; epoch fencing holds across peer restart, downgrade, and pairing rotation; no per-second forced capability probe. - **Schema v35:** repairs databases stamped v34 by the pre-fix branch (mailbox_handle default, index predicates), drops the write-only `lifecycle_transition_receipts` ledger and five never-read v31 identity columns. - **Schema v36:** `dispatch:<id>` mailboxes get a real consumer generation on `dispatch_contexts` and `remote_dispatch_attachments`, bumped and fenced in the same transaction on every re-attach (manual inject, worker-start, federated attach). A stale worker whose Dispatch moved to another process now gets `consumer_fenced` instead of silently acking the new worker's Delivery. Run mailboxes already worked this way. - **Schema v37:** `dispatch_contexts` records its creator (`creator_handle`, `creator_pane_key`), so a coordinator's context-only self-dispatch is bookkeeping rather than a nesting parent; before this, one self-dispatch made every later `worker-start` from that coordinator fail the depth cap. Pre-v37 rows keep counting (fails closed). - **Dispatch-mailbox ownership is checked, not inferred.** A `check` from a process whose pane no longer holds the Dispatch, or whose last Attempt was abandoned/failed and moved to another terminal, gets `consumer_fenced` instead of an empty inbox that reads as "no mail yet". `--peek`/`--all` stay readable. A paneless caller still gets `stable_pane_required` with the rebind recovery. - **Liveness certification is stricter:** a `process_exited` stage whose termination reason is `unknown` (a stop that was issued but never observed) projects `unverifiable`, not `exited`. Federated `worker-show` carries the execution host's verdict and host kind instead of a local guess. A live, ready worker with nothing pending has `nextAction: none` rather than pointing at the `worker-show` that produced it. - **Wire:** `workerShow` keeps `dispatch.task_id` next to `taskId` for shipped CLIs. `ask --json` uses the standard `{ok, result}` envelope like every sibling verb. - **Migration start-version detection** treats the two v32 recovery columns as versioned. Before this, every shipped database stamped below 32 resolved to the v6 floor and replayed the whole chain (the v23 backfill synthesized 68 phantom retained workers on a real v30 profile). Verified on a copy of a real 62 MB v30 profile: starts at 30, no row delta, integrity ok, 11 ms. - **Skill guide** rewritten as a ≤200-line kernel plus seven references, to the outcome-first standard (Result / Done / Safe failure first, conditions not case lists, one done bar, references loaded at the point of use). The canonical loop uses `worker-start --spec`, names `worker-list` for completion accounting, documents `--retry-request` / `request-show` / `--wait-submit`, and requires positive evidence before any stall action. The other seven guides get the same treatment in #18724, split out so this PR stays orchestration-only. - **`rpc/methods/orchestration-*`** (126 flat files) regrouped into `orchestration/{worker,federation,messaging,runs,gates}/`. ## Why User reports showed the same boundary failures: false `agent_prompt_stalled` causing duplicate sends (#15180), coordinators unable to trust screen scrapes, cold-parked terminals receiving a pointer without the submit, settled workers accumulating as live tabs and auto-resuming after restart, and no way to tell a stalled worker from a working one. ## Linked issues Fixes #15180. Fixes #17935 (orchestration skill description is 866 characters; a guard now caps every bundled skill at 1,024). Supersedes #17651 (fence folded in). Advances #16660, #16522, #14907, #13047. ## Review record This PR was reviewed adversarially after revival: eight independent lenses (lifecycle, mailbox, send, worker, federation, transcript, complexity, live ergonomics), each required to prove findings with a failing test. That produced 16 proven blockers, all fixed with red-then-green regression tests, followed by two re-review rounds and a third fix wave that caught 3 regressions introduced by the fixes and 7 fixes that missed their target; all closed. A final pass (five lenses incl. a live built-runtime smoke, then a re-review of the fix wave) found and fixed seven more, chiefly the stale-worker mailbox steal, the self-dispatch depth wedge, and the unproven-exit certification. Three independent Codex (gpt-6-astra) passes followed: the first found nothing new, the second found and fixed 3 defects (task-status reachability, WSL-local host classification, peer-capability epoch), the third found and fixed 6 (production PTY controller never installed settled writes, ambiguous in-flight pointer failures allowed duplicate replay, SSH/relay deadlines cut off a valid `--wait-submit`, stop-vs-exit race during inspection, and two release-recovery paths for vanished or exited terminals). The full record (findings, proof tests, triage, declines with reasons) is archived outside the repo. **Rework after the live smoke.** A first live cross-host run on the shipped adhoc build (this Mac, a paired Windows host on the same build, a paired Mac on 1.4.195, and an SSH host) found a P1: a running local worker read `unverifiable`/`missing_status` because the fleet snapshot rows lacked the terminal handle the matcher keyed on. A 59-row failure table over every bug fixed during review showed the same two classes recurring: a fact dropped in transit through optional fields, and two authorities for one fact. Two blind designs (Opus, Codex) converged on the same mechanisms, and the scoped tranches landed here with red-then-green seam tests from the real producer to the real consumer, faults injected only at the transport or hook-ingest boundary: - **Settlement (data-loss class):** one three-valued `WriteSettlement` (`accepted | refused{reason} | unverifiable{reason, bytesHandedToTransport}`) from the SSH multiplexer through daemon client, providers, controller, to pointer staging. No boolean, no rejection-as-third-state. The two silent degrades that fabricated a handoff are deleted; a provider that cannot settle refuses before any effect. Pointer text and Enter share the contract; a partial flush is `unverifiable`, never `refused`. - **Evidence identity (false-liveness class):** fleet agent-status evidence is a tagged union (`binding: worker | pane | unresolved{reason}`, `clock: observed | delivery`) minted once at ingest, so a hook row captured on one process incarnation can never bind to a later dispatch on the same pane. The matcher's `!worker.paneKey ||` defaults are gone. One host-scope parser replaces two. - **Small pre-merge items:** `capability_unsupported` from an old peer is no longer relabelled `host_unavailable`; a producer census test asserts every agent-status consumer path projects a pane-only hook row as `live`. Two ergonomics defects the second live run surfaced on a real database are fixed here too: a pre-v3 dispatch already marked `completed` projected as `outcome_unknown` / `requiresAction: true` forever (three copies of the outcome ladder disagreed on legacy rows; now one resolver, legacy `completed` reads `succeeded` with nothing to act on, legacy `failed` stays actionable on the failure), and an unscoped `worker-list` enumerated the entire database (now defaults to the Run bound to the calling terminal, `--run` overrides, and the receipt's additive `scope` field says which). A third live round on the shipped adhoc build of `b082443e1f` (same four hosts) plus an unscripted run in the user's own prompt style (a plain Claude Code shell, `/orchestration`, three workers, zero errors, bound-Run default confirmed) found two more branch defects, fixed with red-then-green tests: a worker freshly started on a paired server projected `unverifiable`/`host_indeterminate` with `requiresAction` for ~3 minutes, including after its own `worker_done`, because the host's federation observation returned `missing_liveness_verdict` for any PTY the liveness register had not yet swept (the host now reads a connected pane it owns locally as `live`; disconnected or SSH-scoped panes stay `unverifiable`); and six pre-v3 completed rows still carried an `input` category because settling through the task-status path or `failDispatch` never closed the Dispatch's pending question threads (both paths close them now, and schema v38 closes threads already pending on settled rows). The guide's `worker-start` examples now show `--model sonnet`, since an omitted model inherits the launcher's default. A Codex adversarial pass on the tranche diff found one real design hole (identity minted at read time instead of ingest, now closed) and two daemon settlement paths that threw instead of settling (fixed). Two `@ts-nocheck` runtime mixins on these paths were extracted into checked modules; the repo-wide `@ts-nocheck` count is unchanged at 171. Deletions during review: ~1,900 lines (write-only ledger, unread columns, dead v1 archive path, test harnesses shipped in prod, duplicated liveness and state-machine copies, self-capability checks that were compile-time true). ## Testing - `pnpm typecheck:tsc:node|cli|web` clean - `pnpm run check:code-quality:changed` 0 findings; `check:react-doctor:changed` 0 - `pnpm verify:bundled-skill-guides`, `verify:skill-bundle-manifest` - full `pnpm test` on the integrated head: 72,332 pass / 292 skipped; the only failures were three non-PR files (two zsh live-shell suites hit a node-pty spawn-helper ENOENT while a concurrent native rebuild ran, 44/44 in isolation; `release-checkout.unit.test.ts` is a known 30 s load timeout that passes in isolation on `origin/main` too). - CI on 70b4811267 (rerun, pre-Codex): the only reds are five SSH e2e specs plus `terminal-send-agent-prompt-submit:198`, each shown failing identically on main (main's E2E workflow is red on its last 40 runs). The terminal-send spec is root-caused and fixed separately in #18707. The Windows hook-service flake (#17721) and the federation load flake did not recur. - Skills: `pnpm exec vitest run` over the skill gate files plus `src/cli`, `config/scripts`, `src/main/skills` pass; live smoke on the built CLI of `skills get orchestration` and `--full` (7 references). - live headless runtime (`orca-dev serve`, isolated profile): canonical loop, stop, release, archive read, retry rejection, stale-handle check, SIGKILL-and-replay all verified with receipts - Live cross-host smoke on the shipped adhoc build of `0d465e7931` (this Mac and a paired Windows host on the build, a paired Mac left on 1.4.195, an SSH host): local, paired-new, paired-old and SSH loops all settle; running workers read `live` on every host and `exited` after release; the old peer reads `capability_unsupported` and refuses release honestly. Injected 10 s relay stall with a send in flight: delivered exactly once after recovery, zero duplicates. Every liveness field across 104 receipts is only `live` / `unverifiable` / `exited`. - Final live cross-host smoke on the shipped adhoc build of `b082443e1f` (same hosts): every loop settles; 942 of 948 legacy completed rows read settled with `requiresAction: false` before the question-thread fix and all of them after; `worker-list` scope reads `bound` / `flag` / `all` correctly; 122 JSON receipts carry only `live` / `unverifiable` / `exited`. Unscripted prompt-style run: clean. - Confirmation smoke on the shipped adhoc build of `2da076d4e9` (this Mac and the paired Windows host, both updated): a freshly started Windows worker reads `live` on the first fleet poll and on all 20 that follow, with no `host_indeterminate` at any point, and `exited` after release; all 948 legacy completed rows read `requiresAction: false` with `nextAction: none` after schema v38; every verdict across 60 receipts is `live` / `unverifiable` / `exited`. - Not physically exercised: WSL hosts, the renderer notification bell (headless has no renderer), same-session fence via a real pane close (renderer-only state), restart mid-delivery on a real app (covered by e2e only). ## Notes - Remote-wire additions are optional fields or `method_not_found`-negotiated methods; one new Electron-only IPC channel (`agentStatus:legacyWorkerTerminalResumeFence`) never crosses the wire. - SSH contact loss remains `unverifiable`; the execution host stays authoritative. - Intentional wire projection change: an SSH host scope with an empty `targetId` now projects host id `ssh` instead of an empty string (remote-wire-compatibility rule 3, old clients decode the same field). A fleet pane key without a terminal handle is now `unidentifiable` rather than matched by pane key alone. - Found live but pre-existing on main, filed separately: a relay daemon-start collision during transport loss rewrites the endpoint credential and wedges the surviving relay (host needs a manual kill); `terminal create` on a reconnecting SSH host reports an opaque `No PTY provider for connection`; `terminal list` reports `orphaned:false` and `terminal close` reports `ptyKilled:true` for a pane whose relay is gone (orchestration's own projection reads `unverifiable` correctly at the same moment). - Downgrade after this PR is not a supported path: main opens a v37 database and early-returns (its inserts still work against the v36/v37 defaulted columns), but its one-outstanding-Delivery-per-Run index is a no-op against the branch's mailbox-scoped index of the same name. - Known follow-ups (not blockers): `worker-list` materializes every dispatch row per call; a positive "agent absent" signal distinct from PTY liveness is a product decision left open (a headless fake agent never reaches `live`, so its `nextAction` stays `inspect`); a context-only self-dispatch still lists as `role: worker` in `worker-list`; `dispatch` task-not-found / task-not-ready / inject-rejected still surface as `runtime_error`; task and inbox receipts still expose raw row columns. Deferred skill product decisions live on #18724.	2026-09-06 14:34:03 -04:00
Neil	3be526c5e6	test: cover SSH reattach replay and enable deterministic Codex CI (#19106) ... * test: cover SSH replay replies and run deterministic Codex restore scenarios * test: register replay probe unit command in reliability gate	2026-09-06 11:25:58 -07:00
Neil	4d9e963ffd	test: enable localhost SSH terminal and hook journey in CI (#19097) ... * test: run localhost SSH terminal and hooks in CI * test: isolate localhost SSH session fixtures across repetitions * test: route remote agent hook source changes to localhost journey * test: record localhost SSH reliability evidence and remaining gaps * test: route the real SSH session hook authority	2026-09-06 10:05:26 -07:00
Neil	f5960cec00	test: enable Docker SSH browser network route coverage in CI (#19095) ... * test: enable Docker SSH browser network route journeys in CI * test: register Docker browser job in token permissions contract * test: declare SSH client dependency and narrow browser fixture routing	2026-09-06 09:29:56 -07:00
Neil	3631f886a7	test: enable direct and client-hosted SSH browser coverage (#19090) ... * test: enable direct and client-hosted SSH browser coverage * test: record twelve passing SSH browser journey repetitions * test: distinguish SSH journey evidence from unit runtime budget	2026-09-06 08:25:13 -07:00
Neil	1d2e00819f	test: restore SSH bulk-open freeze coverage in headed CI (#19081) ... * test: restore SSH bulk-open freeze coverage in headed CI * test: record ten passing headed SSH freeze repetitions * test: record ten passing headed SSH freeze repetitions * test: route changed SSH freeze spec only to its dedicated lane	2026-09-06 07:42:50 -07:00
Neil	f952f1ac96	test: run real WSL terminal launch and paste in PR CI (#19072) ... * test: continuously exercise real WSL terminal launch and paste * test: establish live WSL reader before changing default shell * ci: pin WSL kernel installer and participation selectors * ci: route deleted WSL paths and record immutable run evidence * test: require exactly three WSL repetitions in lane contract	2026-09-06 05:24:54 -07:00
Brennan BensonandMerge Sim	6933fd70d7	fix(packaging): ship Claude agent SDK with desktop builds (#19042) ... * fix(packaging): include Claude agent SDK at runtime * test(packaging): cover spaced runtime imports * fix(packaging): verify every emitted main file for bare runtime imports The packaged-main verifier read two fixed entry files, but rolldown hoists modules shared by two entries into out/main/chunks. jsonc-parser is reached only from a chunk today, so nothing verified it, and the agent-hooks entry the list names contributes no coverage at all. An import that migrates into a chunk would silently stop being checked -- the same blindness that let the missing Claude agent SDK ship. Scan every out/main/**/*.js entry in the asar instead, keeping the two required-file assertions as a build-integrity check. Measured against the shipped 1.4.198 app: 93 entries in 72ms, reporting the absent SDK and nothing else. Also tighten the specifier match with a (?<![.\w]) lookbehind. Orca has three registry methods of its own named require(), two taking a string key, so a minified registry.require('public-a') otherwise reads as a bare module specifier and fails packaging with a confusing error -- a risk the wider file set would have multiplied. The lookbehind drops nothing real: detection over the shipped bundle is identical with and without it. * test(packaging): cover the missing packaged main entry assertion The required-file check had no test, so the refactor that split it out of the scanning loop could have dropped it silently. Removing the assertion now fails this case. * docs(packaging): name the embedded-source-string limit of the main scan ssh-relay-deploy builds a probe script for the REMOTE host as a string, and its require("node-pty") / require("@parcel/watcher") survive into out/main/index.js, where this scan counts them as desktop-main imports. Both are packaged, so it is benign today, but a remote-only dependency added to that script would fail desktop packaging with a false message -- and the two obvious fixes (ship the remote dep, or weaken the guard) are both wrong. Separating an embedded string from real code needs a parser. * test(packaging): pin the exact import shape oxc emits for the SDK The fixture only carried the spaced `import (` variant, so nothing pinned the form a shipped build actually contains. Use the real emitted shape -- `p??=import(`@anthropic-ai/claude-agent-sdk`)`, no space, backticks, and the `??=` that precedes it -- and keep the spaced variant on the second entry so both stay covered. * fix(packaging): keep the main scan able to see a spread require The `(?<![.\w])` lookbehind also rejected `[...require("pkg")]`, because the third dot of a spread satisfies it. That trade is not symmetric: excluding a member call costs a loud release-build failure if it ever misfires, but excluding a real specifier is this guard going blind -- the failure mode the whole verifier exists to prevent. Readmit a dot that ends a spread. Zero occurrences in the shipped bundle today, so this was latent. The chunk test's asar mock now also emits directory nodes, because real listPackage does and extractFile throws on them -- that makes the `.js` anchor's load-bearing role something the tests can actually catch. --------- Co-authored-by: Merge Sim <sim@local>	2026-09-06 02:27:57 -07:00
3 people	6494f2a4f0	fix(native-chat): resume a structured chat from Agent Session History (#18933) ... * fix(native-chat): resume a structured chat from Agent Session History Clicking Resume on a chat-UI row could only reveal an already-open tab. If the chat had been closed, or this process had never published it, the click re-read an inventory that did not contain it and toasted "Retry in a moment" — advice that could never come true, because nothing republishes an unpublished tab. The legacy `claude --resume` fallback is deliberately refused for structured-owned rows, so the row had no way back at all. `close` already keeps the record and the journal on disk so a session can be attached again, and the hold path already resurrects one in full. What was missing was the tab: `restoreReadableSessions` is latched to run once, at startup, so nothing could ask for a single session later. Adds `agentSession.reveal`. The host looks up its own record, restores the session readable, and republishes the tab through the same call `agentSession.create` uses. Deliberately narrow: - It takes no hold. A provider child exists because a surface asked, and the chat pane asks when it binds. - A journal it cannot read is not a refusal. A chat whose journal predates the SQLite store restores to nothing here, but attach still recovers it, so the tab is published and the pane's hold finishes the job. - Workspace and provider come from the record, never the client, so a session id alone cannot aim the publication at another workspace. Claude and Codex both, by construction: eligibility is `adapterSupportsRecord`, which the router answers from the record's own provider. Gated on a new advertised capability rather than probing for method_not_found, matching agent-session.structured.hold.v1 — absence is visible during negotiation instead of by calling. * fix(native-chat): negotiate reveal against the host that owns the workspace The capability gate read the LOCAL runtime's advertised capabilities while the call went to the host that owns the workspace, which for a paired workspace is a different build. On desktop the renderer and its local host are always the same build, so the gate passed unconditionally and proved nothing about the host being called: an older paired host still received the unknown method and its method_not_found was reported to the user as 'this chat is no longer on this host'. The cache it read also starts empty and resets to empty when status.get fails, so 'not fetched yet' and 'unsupported' were the same value. Gate on the environment that will answer, the way agentSession.close already does, and skip the round trip entirely for a local host. Reveal now reports four outcomes instead of a boolean, so a host that is merely too old is not reported as a chat that is gone, and a host we could not reach keeps the retryable message. Also syncs the localization catalog: the 'gone' key shipped without an en.json entry, which reddens static analysis and verify while typecheck stays green. * fix(native-chat): tell a refused reveal apart from a missing chat The host raises two refusals here and they mean opposite things to a user: it holds no such record, or it holds one no adapter of its own can open. The client collapsed both into 'this chat is no longer on this host', which is a eulogy for a chat still sitting on disk. Read the refusal code, and fold the host-side case in with the too-old host under one honest message, since the remedy for both is the same. Adds the coverage the readiness pass found missing: the host's reveal answer itself (workspace and provider from the record, both refusals, an unreadable journal, a live session), and the activation branches for a host that cannot open the chat and for one that never answered. * fix(native-chat): read a host version block as the host's age, not a lost link The capability probe reaches assertRuntimeStatusCompatible, which throws a runtime_compat_block error. Treating that as unreachable told a user with an out-of-date host to retry, which is the one thing that cannot help. Branch on isRuntimeCompatBlockError the way remote-agent-session-launch already does for the same probe. Also adds the refusal-code case a previous commit claimed and did not deliver: nothing drove a structured_agent_session_unsupported reply through the reveal client, which is the branch that commit existed to add. Corrects a doc comment that reveal made wrong: attach is no longer the only call that builds the host. * fix(native-chat): let a dragged history row reach the same reveal as a click Dropping an Agent Session History row onto a pane activated the tab by id and, on a miss, raised the very toast this PR exists to remove — so the same row answered a click and a drop differently, and the drop kept the advice that can never come true. The structured branch never used the drop pane, so routing it through the shared activation loses nothing and gains the reveal. The helper only ever read one field, so its parameter narrows to that field and the drag payload satisfies it directly. A source ratchet holds both entry points to the reveal-capable path, since a mounted drag harness does not exist for this layer and what regresses is a call site, not a rendering. * fix(native-chat): stop an advisory refresh ending the click, and one click per row Manual QA found the reveal never ran: the inventory refresh that precedes it is an optimization, but its failure returned early with 'not available yet, retry in a moment' — reinstating the dead end this PR removes, one step earlier. A failed refresh now falls through to the reveal, which is the repair and does not need the refresh to have worked. The click can chain a refresh, a capability probe, a reveal and a second refresh, each with its own timeout, while nothing on the row says it is working. A per-session in-flight guard keeps an impatient second click from running the whole sequence again and landing its own toast. Also drops an unreachable owner scope: the snapshot apply discards any worktree whose execution host is not local before it reads one, so naming a remote scope there described a synchronisation that cannot happen. * fix(native-chat): bound the capability probe and stop naming the wrong machine The in-flight guard releases when the activation settles, so an await that never settles holds the row for the life of the process. The capability probe was the one call in the chain not raced against a deadline: on a cache hit it awaits a promise an earlier probe created, which may carry no deadline of its own. Race it like the two calls around it. A version block can name either side — evaluateRuntimeCompat reports client-too-old as well as host-too-old — so a message that blamed the host pointed half of those at the wrong machine. Name the remedy instead of the machine, which is true for every case that reaches it. * chore: remove a scratch repro file committed by mistake It was swept into the previous commit by a broad `git add` while a diagnostic ran in this worktree. It asserts the current renderer-sync defect as expected behaviour, so it would fail the moment that defect is fixed. * fix(native-chat): stop a reveal's own inventory refresh discarding its republished tab Manual QA: the host answered reveal with ok:true and republished the tab, and the chat still did not reopen — only a renderer reload brought it back. The renderer publishes under one epoch string for its whole lifetime, and a frame recorded under a different lineage retires that epoch permanently with nothing to un-retire it. The Resume click asks for an inventory first, and a worktree the host holds no entry for answers with the none/v0 sentinel; the structured path recorded it, retiring the renderer's own epoch, so the tab the reveal published a moment later was dropped. A reload minted a new epoch, which is why reloading appeared to fix it. A frame that carries no publication is not a later publication to fence against. Treat the sentinel and a removal frame as a cursor reset, the way the mainstream session-tabs path already clears its tracking — its comment names this exact hazard: recording that sentinel would retire the host epoch and reject the next live frame. Pre-existing, and it swallows an ordinary new-tab launch on an empty worktree too; the reveal is what turned a silent invisibility into a visible failure. * fix(native-chat): let a retraction prune its rows without retiring the epoch Correcting the previous commit. Skipping a retraction frame outright stopped it pruning the mirrored rows, so a worktree the host no longer publishes would have kept a chat on screen with nothing behind it. Apply the frame as before and clear its cursors instead of recording them, which is what the mainstream session-tabs path does. The unpublished sentinel keeps its cursor now too: it is skipped rather than cleared, so a stale frame arriving late is still fenced. Adds the case the earlier version would have broken. * fix(native-chat): keep the retraction's fences, and fence the reveal's refresh Correcting the retraction handling again. Clearing its cursors was more than the bug needed and cost a guard: the host mints a fresh epoch when it rebuilds a pruned entry, so a republication is never gated by the retained cursor, while dropping it left an inventory response issued before the close free to land afterwards and strand a chat row for a worktree the host no longer publishes. Skip only the recording. The mainstream path keeps its epoch history for the same reason, as a tombstone fence. The test that justified the stronger clearing asserted a host behaviour that does not exist — a rebuilt entry republishing under the renderer's epoch with a restarted counter. It now uses what publishStructuredAgentSessionTab actually mints for a pruned entry, and a new case covers the frame that would strand. Also fences the reveal's inventory refresh on the sync generation, which every other caller that applies an inventory already does: structured chat can be switched off mid-flight, and the answer would otherwise re-seed a row into a renderer that just discarded them. * fix(native-chat): drop the retraction's epoch history, keep its version cursor Third and final shape for this branch, and the only one of the three that holds. Keeping both maps re-poisons the epoch one cycle later: the consumer here is also the publisher, so the history's current is the renderer's own lifetime epoch, and recording the reveal's fresh epoch retires it. The next chat the renderer publishes is then dropped — this bug again, one close later. Deleting both loses the guard that stops a frame issued before the close landing after it and stranding a row nothing republishes. So: clear the history, keep the cursor. The mainstream path keeps its history as a tombstone because there the epochs belong to a remote publisher; that reasoning does not carry to a path that publishes under its own. Each of the three variants now fails a different test. * fix(native-chat): a retraction forgets what is current, not the tombstones The delete lost a fence the cursor cannot replace: the version cursor only compares within a lineage, so a delayed frame from an already-superseded epoch had nothing left to stop it putting a chat row back for a worktree the host no longer publishes. Keeping the record intact had the opposite fault — the renderer's own epoch is the history's current, so the next frame under any other epoch retired it. Clearing only current does neither: noteRetiredValue retires nothing when there is nothing current, and the tombstones stay. Each of the four shapes now fails a different test. * fix(native-chat): narrow the retraction frame through its own type Typecheck caught what the tests could not: `removed` is not on RuntimeMobileSessionTabsResult. The repo already names the shape — RuntimeMobileSessionTabsRemovedResult — so this reads it through a guard rather than the inline cast the mainstream path uses. --------- Co-authored-by: Orca Worker <orca-worker@localhost> Co-authored-by: Merge Sim <sim@local>	2026-09-06 00:50:20 -07:00
OrcaWinandOrca Worker	0f27445789	fix(build): pin config/relay-assets LF so one release is one relay hash (#19024) ... * fix(build): pin config/relay-assets LF so one release is one relay hash * test(build): correct why the negative fixtures exist Review measured it: the first assertion checks the eol attribute via check-attr, not file content, so it fails first without the pin. The fixtures add over-broadness coverage, they do not carry the test. * test(build): key the relay line-ending pin off the manifest, not a directory A path glob proves the directory is non-empty, not that it is still the directory build-relay reads from. Relocating an asset into config/scripts (where only **/*.mjs is pinned) reintroduced the CRLF bug with the suite fully green. RELAY_ARTIFACTS is the right anchor: build-relay refuses to emit an artifact absent from it, so a relocated or new asset cannot slip past. Bundles have no tracked source and drop out with zero hits. --------- Co-authored-by: Orca Worker <orca-worker@localhost>	2026-09-06 00:02:05 -07:00
Jinjing	891ae62df5	fix(release): revalidate draft state before patching generated notes (#19019) ... * fix(release): revalidate draft state before patching generated notes The release listing is a snapshot taken before generate-notes runs. If the draft is published in that window, the PATCH overwrote a live release body. Re-read the release by id immediately before the update and skip it when the release is no longer a draft. * Handle publication race during draft release notes patch Between the draft status check and the PATCH request, a release can be published. The PATCH succeeds but now modifies published content. Check the PATCH response—if draft=false, publication won; restore the published body and leave generated notes unapplied. * fix(release): only roll back the draft body we actually wrote Re-read the release before the compensating PATCH and skip the rollback when the body no longer matches the notes we patched in, so a body written after our PATCH is not clobbered.	2026-09-05 22:34:56 -07:00
Neil	bf5f3c2ec4	test: cover native X11 Hangul-plus-digit PTY bytes in CI (#19013) ... * test: run the native Hangul terminating-digit regression in CI * test: distinguish X11 byte coverage from the manual Wayland repro * test: require native IME engagement proof for the digit case	2026-09-05 22:12:21 -07:00
Jinjing	bdad20b4c1	Support updating existing draft releases when regenerating notes (#19014) ... Move release existence check into create-draft-release.mjs. Draft releases are updated via PATCH, published releases are skipped, making the release-cut workflow idempotent.	2026-09-05 21:52:54 -07:00
OrcaWinandOrca Worker	ec030f1d35	fix(windows): sign the NSIS uninstaller via SignPath (#17868) ... * fix(windows): sign the NSIS uninstaller via SignPath `Uninstall Orca.exe` ships NotSigned, and MDE's whole update cluster is that one file: electron-builder copies it to `old-uninstaller.exe` and runs it silently during every update. The cause is narrower than "NSIS generates the uninstaller at install time". app-builder-lib already builds the uninstaller in its own makensis pass and calls `packager.signIf(uninstallerPath)` on it before embedding it (NsisTarget.computeScriptAndSignUninstaller). Orca signs nothing during electron-builder — SignPath signs afterwards, behind a human approval — so that hook is a no-op and the file is deleted before CI can reach it. Use the hook as a relay instead of a signer: the first Windows build exports the uninstaller, it rides the existing inner-binaries SignPath request (no third approval wait), and the rebuild-from-signed-tree pass swaps the signed bytes back in before makensis embeds them. Every added step is fail-open. A missing export, a SignPath artifact configuration that does not cover `uninstaller/`, or a relay error costs only the uninstaller signature — the inner-binary chain and the shipped installer are unchanged. * fix(windows): keep the uninstaller relay out of the packed checkout Review fixes on the uninstaller signing chain. The export path lived at `${{ github.workspace }}\uninstaller-signing\`. `files` in the electron-builder config is all-negation, so app-builder prepends `**/*` and packs whatever is left in the checkout root, and the build step retries up to three times — attempt 1 wrote the file after packing, attempts 2 and 3 would have packed an unsigned `.exe` into app.asar. All seven relay sites move to `runner.temp`, and a contract test now fails if any of them points back into the checkout. The uninstaller staging block guarded with `Test-Path` but left `New-Item` and `Copy-Item` able to throw. That step's outcome gates the upload of every inner binary, so a locked file there would have cost all of them their signatures — worse than before the chain existed. It is wrapped in try/catch, asserted. Also: test `signWindowsUninstallerViaSignPath` itself (it runs in a step with no continue-on-error, so its no-throw property is load-bearing) and the sha1+sha256 double invocation; make the rehearsal verify the uninstaller the installer actually writes to disk rather than only the relay receipt, whose digest comparison is equal by construction; correct the staged-name comment, which asserted a collision that does not reproduce; count what was reported rather than what was extracted; and note two traps — a custom sign hook replaces signtool outright, and the single-env-var relay would race if a second NSIS target or arch is added. * fix(windows): stop the signing rehearsal failing on its own artefact The rehearsal is the merge gate for this chain, so it must not be able to fail on something that is not the thing under test. It trusted whatever 7-Zip's NSIS handler emitted. That handler produces partial or garbled output on some NSIS builds, and a truncated extract would score NotSigned and be reported as "the shipped uninstaller is unsigned" when nothing was wrong. It now has to reproduce the digest the sign hook recorded before its output is trusted; otherwise it falls through to the silent-install route, which is ground truth. A name miss falls through the same way. The install route only checked the signature. Comparing the on-disk file against the receipt is what actually proves the shipped installer embedded the SignPath-signed bytes — the release job's own comparison is equal by construction, so this is the only place the claim is really tested. Also: bound the silent install (a bare `-Wait` on an installer that ever prompts hangs to the 360-minute job cap) and poll before stopping Orca, since the oneClick installer launches the app as it finishes and the process can appear after the installer has already exited. Two smaller ones: `-ErrorAction Stop` on the staging New-Item/Copy-Item so the catch above them does not depend on GitHub's $ErrorActionPreference default; and the relay-path test now counts every occurrence rather than the first, so a step carrying two paths cannot root one in RUNNER_TEMP and leave the other bare-relative — the exact shape of the bug it guards. * test(windows): stop a pre-existing elevate.exe defect masking the gate The first real rehearsal (run 33484703381) proved the uninstaller relay works end to end — the 7-Zip route read the embedded uninstaller, the digest guard did not trip, SignPath accepted the new uninstaller/ zip entry, and the shipped `Uninstall Orca.exe` came back signed. It also failed, on `resources\elevate.exe`, for a reason that predates this PR. app-builder-lib re-copies the pristine cached elevate.exe over `resources\elevate.exe` on every nsis pack — `AppPackageHelper.packArch` calls `elevateHelper.copy()` before `buildAppPackage`, and `CopyElevateHelper.copy` does `copyFile(elevatePath, outFile, false)` then `signIf(outFile)`, which signs nothing because this build configures no certificate. The signed copy restored into win-unpacked is clobbered by the rebuild. That is not the sign hook displacing a signtool call: with no `sign` hook, `signFile` already returned false at "no signing info identified", so nothing was signing elevate.exe before either. release-cut.yml mitigates it separately by pre-seeding the electron-builder cache; this workflow has no such step, which is why the clobber is visible here and not there. Downgrade elevate.exe alone to advisory so it cannot mask the uninstaller result, and record it in the evidence artifact so downgrading stays distinguishable from deleting the check. Both uninstaller verdicts stay fatal, pinned by a contract test that also holds the escape hatch to exactly one file. The underlying defect gets its own PR — it is a UAC elevation helper and deserves more scrutiny than a footnote here. * docs(windows): warn against relaxing the elevate.exe cache guard The tempting edit, for anyone who finds the rehearsal red on resources\elevate.exe, is to relax release-cut's `Valid` + `CN=SignPath Foundation` guard so the cache swap runs under test-signing and the rehearsal goes green. That guard is the only thing stopping a test certificate from being seeded into a cache a real release restores from — both workflows share the key `electron-builder-win-<lockfile hash>`. Shipping users a binary signed by "Test certificate for 'Orca agent ide [OSS]'" is worse than shipping it unsigned, so say so at the place someone would make that edit. --------- Co-authored-by: Orca Worker <orca-worker@localhost>	2026-09-05 21:44:28 -07:00
OrcaWinandOrca Worker	8415d53a05	fix(release): stop shipping an unsigned elevate.exe on Windows (#18044) ... * fix(release): stop shipping an unsigned elevate.exe on Windows The release cut swaps the SignPath-signed elevate.exe into the electron-builder toolset cache so the NSIS rebuild's CopyElevateHelper re-copy becomes a no-op. It searched `<cache>\nsis`, a directory no app-builder-lib layout creates, and `-ErrorAction SilentlyContinue` plus `exit 0` turned that miss into a green step — v1.4.193 and v1.4.194 shipped an unsigned UAC elevation helper. Move the lookup into a script that covers the real layouts (`nsis-3.0.4.1/…`, `nsis@<toolset>/…`, `ELECTRON_BUILDER_NSIS_DIR`), asks app-builder-lib for the authoritative path, and exits non-zero with an ::error:: annotation when it finds nothing. The step stays continue-on-error so the inner-signing chain remains fail-open. * fix(release): make the elevate.exe swap prove it replaced the packed copy Success was "some cached copy was replaced", which a stale release directory carried in by the `electron-builder-win-` prefix restore can satisfy on its own while the bundle the rebuild packs stays unsigned. The app-builder-lib probe returns the exact path CopyElevateHelper will pack, so make that the check and the directory scan the fallback: exit non-zero when the probed copy was not replaced, and annotate a warning when the probe could not run at all, so a green step never quietly means the authoritative check was skipped. Also pin both shebang scripts to LF: `core.autocrlf=true` gives a Windows checkout CRLF, and CRLF plus a shebang breaks vite's transform, so resolve-7za-path.test.mjs currently runs zero tests there. --------- Co-authored-by: Orca Worker <orca-worker@localhost>	2026-09-05 21:44:20 -07:00
OrcaWinandOrca Worker	c252d855ac	fix(windows): resolve npm/pnpm .cmd shims past cmd.exe (#17869) ... * fix(windows): resolve npm/pnpm .cmd shims past cmd.exe A `.cmd` target forces every spawn through `cmd.exe /c` with each argument caret-escaped, and Microsoft Defender for Endpoint scores a long `cmd.exe /c` line carrying caret-escaped natural language as obfuscation. `codex.cmd` is named in the spawn cluster of the MDE incident this addresses. npm's `cmd-shim` and pnpm's `@zkochan/cmd-shim` generate files whose whole body is "find node, run this script". Read one, and the spawn can go straight to `node.exe <script> <args>` — no cmd.exe, no caret escaping. Anything the parser does not recognise exactly, or whose target cannot be confirmed on disk, keeps the existing cmd.exe path. Incidentally fixes a real bug: cmd ends its command at a raw CR/LF whatever the quote state, so a multi-line agent prompt through a `.cmd` shim had to be rejected. Resolved shims have no such limit. * fix(windows): refuse drive-relative shim paths and run the win32 tests in CI Two blocking findings from review. A drive-relative path defeated the absolute-path guard: `win32.isAbsolute('D:evil.js')` is false, but `win32.resolve` reads the drive letter and lands on `D:\evil.js`, outside the shim directory. cmd would have built `C:\shim\D:evil.js` and failed; we would have executed the wrong file. Adding `:` to the unsafe-character set closes it, and the alternate-data-stream spelling `a.js:zone` with it. It costs no coverage: 84 of the 91 real shims on this box still resolve, the same seven fall back. Neither `windows-cmd-shim-resolution.test.ts` nor its `.win32` sibling was in the Windows package job's file list, so the whole filesystem/resolution half and the real-spawn equivalence suite ran nowhere. Both are now in `WINDOWS_PACKAGE_TESTS` and in the pr.yml step. Also from review: clear `windowsVerbatimArguments` explicitly on the resolved branch rather than inheriting it, since there is no caller-built command line there; document the kill switch and the PTY/hook-wrapper scope limits in docs/reference; and cover drive-relative, BOM, line-ending, casing and `%*` tampering in the platform-independent half of the tests. * docs(windows): justify the shim-path colon guard from the filesystem rule The guard was argued empirically ("none of the 91 shims on this box has one"), which invites a future reader to relax it for a shim we have not seen. Windows reserves `:` within a path segment, so a relative path cannot carry one at all: the only spellings that can are drive-qualified, an alternate data stream, or a `\?\` device path, and the last is already refused as absolute. That makes a false refusal impossible rather than unobserved. * refactor(child-process): move resolveSpawn into its own module The merge with main pushed run-process.ts one line past the 300-line cap: both sides grew it. The spawn-argv decision is already a pure, separately tested unit, so it moves out rather than the cap moving up. run-process.ts re-exports it, so no caller changes. * perf(child-process): cache the shim interpreter lookup The parse cache spared the shim read but not the PATH walk, so a second resolution of the same .cmd did 0 reads and one statSync per PATH entry -- 30 on a 30-entry PATH, synchronous on resolveSpawn, where one dead network mount blocks the calling thread on every spawn. Keyed by shim directory AND PATH, since the shim's own rule is %~dp0\node.exe first then PATH, and a PATH edit between spawns must miss. Corrects the stat comment, which accounted only for the shim itself. * fix(child-process): revalidate a cached shim interpreter before using it The node cache was held for process life and never rechecked, so a cached node.exe that was later uninstalled -- or dropped from PATH by a version manager -- was still handed to resolveSpawn, failing the spawn with ENOENT. An uncached process in the same state returns null and falls back to cmd.exe successfully, so the cache was strictly worse than no cache. One statSync on a non-null hit, not one per PATH entry, so the walk this cache exists to skip is still skipped. The stale-null direction stays uncorrected on purpose: it only keeps the working cmd.exe fallback. Both directions are now stated in the comment, along with the known miss for callers that vary PATH per spawn. * fix(child-process): honour PATHEXT when resolving the shim interpreter The doc claimed a node.com/.bat/.cmd on PATH returned null and fell back to cmd.exe. The scan actually skipped those entries and kept looking for a node.exe, so PATH=C:\A;C:\B with C:\A\node.com and C:\B\node.exe resolved to B's node.exe while the shim runs A's node.com -- a different binary, chosen silently, on the one axis this module must not get wrong. The scan now follows cmd's rule: first PATH directory holding any PATHEXT spelling wins, PATHEXT order decides within it, and only an .exe winner is returned. Anything else gives up and keeps the cmd.exe path, which restores the strict-subset-of-cmd property everywhere except the documented cwd case. PATHEXT is read from the child's env and joined into the cache key, since it now changes the answer. Costs one stat per PATHEXT entry per node-less directory, paid once per process behind the cache. --------- Co-authored-by: Orca Worker <orca-worker@localhost>	2026-09-05 21:33:16 -07:00
Neil	ba86e1c83c	perf: avoid repeated whitespace scans in diagnostic redaction (#18908) ... * perf: scan diagnostic environment lines without repeated whitespace searches * perf(observability): skip redundant terminator scans in environment-line redaction After a successful match ENV_LINE.lastIndex already sits at end of input or a line terminator, and a failed start's whitespace skip plus terminator scan collapse into one LINE_CONTENT exec. Byte-identical output; verified against the pre-change regex over 280k generated inputs. * perf(observability): find environment-line ends without entering the regex engine	2026-09-05 21:17:46 -07:00
3 people	0cbb01ef4b	fix(security): apply the Windows path-hardening ACL that never ran (#17884) ... * fix(security): apply the Windows path-hardening ACL that never ran `buildWindowsRestrictAclArgs` invoked the hardening script as `powershell.exe -Command <script> <path> <sid> <isDir>`. `-Command` does not populate `$args`; it appends the trailing tokens to the command text. The script therefore read `$args[1]` as `$null`, threw `NullArrayIndex` at `$allowedSids[$sidText] = $true` under `$ErrorActionPreference = 'Stop'`, and exited 1. Both callers swallowed that: the async callback was empty and `applySecurePathRestriction` returned `true` regardless, while the sync `catch` returned `false` and nobody logged. Every Windows secure path has been left on its inherited ACL since the ACL was introduced (#5006), and nothing said so. Replace PowerShell with `icacls.exe`, which takes plain argv. That removes the quoting surface entirely rather than escaping it: interpolating a path into the command text would have turned a dead no-op into arbitrary PowerShell on a filesystem path, since `-Command` executes what it appends. It also drops the execution-policy dependency and the `powershell.exe` spawn an EDR flags, and runs ~25x faster than the PowerShell cold start. Hardening is now three passes: `/reset` to purge explicit ACEs that `/inheritance:r` leaves behind, `/inheritance:r` plus a `/grant:r` per allowed SID, then a read-back that checks the DACL is protected and grants only the intended rights. The predecessor's verification block was equally dead, and an apply that is never read back is only half a control. Failures stay non-fatal — non-NTFS volumes, network paths and restricted tokens fail legitimately and must not break startup — but they are no longer invisible: every failure is logged, and a failed async apply now evicts its cache entry so the next call retries instead of trusting a success that never happened. Routing through `runProcess`/`runProcessSync` also retires this file's `node:child_process` allowlist entry. * fix(security): verify the hardened ACL by identity, not by shape Review found the bug class this PR fixes surviving inside the fix. The verify pass checked rule count, absence of the inherited marker, and exact rights — never *who* the rules named. Granting Everyone full control satisfies all three, so hardening reported success on a DACL that handed the credential to every local account, and most of the real-filesystem tests still passed. Verification now reads the descriptor back with `icacls /save`, which emits SDDL with raw SIDs, and compares the principal set exactly. That is also locale-independent by construction: the previous parse read localized account names out of icacls' OEM-codepage stdout, where a non-ASCII path survived by accident rather than by the documented mechanism. SDDL parsing moves to `windows-security-descriptor.ts`. Two further self-inflicted problems, both measured: The post-rename re-harden led with `/reset`, which re-widened a DACL that was already correct — the staged file's protected DACL survives the rename, so the pass had nothing to do but open a window. Polling an external process during a write into a relocated root caught it: the e2ee keypair dropped to `BUILTIN\Users:(RX)` plus `Authenticated Users:(M)` — read *and* write — before tightening again. Hardening now verifies first and returns early when the DACL already reads back correct, which closes the window and cuts the steady state from three spawns to one. Re-measured: 158 samples, one DACL state, zero broad. Evicting the cache on every failed async apply reintroduced #4901. The env store re-hardens on the read path at ~2/s, so on a host where hardening cannot work (FAT32, network path, restricted token) that was two icacls spawns and two warnings a second, forever. Async retries now take a retry floor and a hard per-path attempt cap. The write path keeps retrying unthrottled — it is user-driven, and a failed credential ACL must still be retried on the next write. Also: failures route through a reporter hook that the main process points at the diagnostic tracer, because `console.warn` reaches nothing in a packaged GUI-subsystem build; `writeSecureFile` returns whether hardening took, and the async branch reports `pending` rather than claiming `applied`; a transient `whoami` failure no longer disables hardening for the process lifetime, and the SID is shape-validated; the `/c` guard now covers the synchronous runner too. * fix(security): re-probe hardening instead of latching a transient failure The per-process attempt cap added for the read-path storm was a permanent latch: one AV scan, momentary lock or %TEMP% blip and every later credential write in that session went unhardened, silently, on a host where hardening would now succeed. Same defect class as #17858's computer-use host, and worse here because what stops happening is security hardening on credential files and nothing said so. The retry budget now bounds the *rate*, not the lifetime: at most three attempts per path per minute, re-probing in every later window, forever. The transition is announced in both directions — `throttled` once per window on entry, `recovered` when a rate-limited path hardens again — so a host stuck in the degraded state is diagnosable rather than merely quiet. The reporter type covers both, and the main process ends the `recovered` span successfully rather than failing it. Extracted to secure-path-hardening-retry-budget.ts, which keeps secure-file.ts under its line cap without a max-lines disable. Also confirms the second flagged risk rather than assuming it: a real unwritable %TEMP% is now covered by a test proving verification fails closed, reports at the `verify` stage, and still leaves the ACL applied — so that path loses proof, not protection, and with the lifetime cap gone it can no longer combine into a permanent-off state. * fix(security): verify a directory's whole inheritance flag set The flag check tested only that `OI` was present — never that `CI` was, nor that nothing else was. That was harmless while `/reset` + `/grant` ran on every pass and repaired whatever was there. The verify-first short-circuit made it load-bearing: what verification accepts is now left alone, so a latent under-check went live because a different fix started depending on it. Two directory DACLs passed while being wrong — both protected, three non-inherited full-control rules, correct SIDs, differing from correct only in their flags: (OI)(F) - no CI, so subdirectories are left unprotected (OI)(CI)(IO) - inherit-only, so the directory object itself grants nobody anything; the next writeFileSync into it fails with EPERM, on a directory just cached as hardened Verification now compares the whole flag set, which also rejects IO and NP, and names the offending flags in the failure. Both shapes are planted in real-filesystem regression tests, including an assertion that a write into the repaired directory succeeds and its child inherits. Confirmed both tests fail against the old check and pass against this one. * fix(security): back the hardening retry off exponentially The fixed one-minute window bounded the retry rate but left a standing floor of three attempts per path per minute on a host where hardening can never succeed — FAT32/exFAT, a network path, a redirected profile. That budget is per path and there are several secure files, so the floor multiplied into tens of thousands of icacls spawns a day for work guaranteed to fail. The delay now doubles after each consecutive failure, from a one-minute floor to a thirty-minute ceiling, and the attempt cap is gone entirely: once the backoff elapses the path is re-probed however long it has been failing. A permanently incapable host settles at ~2 attempts/hour. Slowing the backstop costs almost nothing, because it is not the recovery mechanism: the synchronous write path is deliberately unthrottled, so a host that recovers hardens on its very next credential write regardless of what the read-path budget says. The `throttled`/`recovered` reports are unchanged and matter more here, since the quiet periods between probes are now much longer. The curve is pinned in a new unit test against the exported delay function rather than a copy of its constants, covering the doubling, the ceiling holding at 5000 consecutive failures, a 30-day failing path still re-probing, one announcement per degraded episode, and per-path isolation. The integration tests keep only what they uniquely prove: that the read path is wired to the budget, and that a day of failures still re-probes. Confirmed four of these fail against a reinstated lifetime cap. * ci(windows): run the real-icacls DACL suite in CI The win32 suite only self-skips off Windows, so it passed vacuously in every lane. Register it the way the cmd-shim suite is registered. * fix(security): describe the cache's real cost, which is icacls now Both cache comments still justified themselves with PowerShell -- "~1-1.5s" and "a PowerShell spawn every read" -- in the same file whose PR removed PowerShell from this path. The caches are still right, but for different numbers, and the old ones are the kind an engineer would reasonably delete a cache over. The real shape: hardening verifies first and returns early, so an already-correct DACL costs one synchronous icacls spawn and a rewrite costs four (verify, reset, grant, verify). Still worth caching on the read path, which polls at ~2/s. * test(security): make the DACL suite safe to schedule Registering this spec in the Windows lane put it under two rules it had never been measured against. Teardown now goes through `removeTreeSync`, which the lane's boundary test requires, and repairs the DACLs the suite plants on purpose first: those retries only cover transient locks, so a regressed `(OI)(CI)(IO)` repair leaves the root un-removable and `afterAll` throws EPERM. And the no-permission case decides by elevation before it writes anything. `windows-2022` runs elevated, where hardening succeeds: the old branch asserted nothing about denial and instead replaced the `hosts` DACL, then `icacls /reset` -- which is not a restore, it drops the explicit `SYSTEM:(F)` that file ships with. Ephemeral in CI; permanent for a developer running the lane from an elevated shell. Now it asserts or it skips. The probe reads the token integrity SID rather than `icacls /save`, which succeeds unelevated (`BUILTIN\Users:(RX)` carries READ_CONTROL) and would have skipped the case on every machine. * fix(security): measure the hardening latches on a clock that cannot go backwards `mayAttemptHardening` compared wall-clock times, so any backwards step -- an NTP correction, a VM snapshot restore, a user changing the clock -- made the elapsed time negative and held every failing path below its delay until the clock caught up. Measured at the 30-minute ceiling with the clock stepped back a year, the path was refused at +0d, +1d, +30d, +180d and +364d, and re-probed only at +366d. That is the permanent latch the exponential backoff was added to remove, and it contradicts the module's own "bounds the rate without ever bounding the lifetime". The SID lookup's own one-minute window had the identical shape and is worse: a failed lookup makes `planFor` return null, which disables the synchronous *write* path too, so the write-path exemption that recovers the read-path budget cannot recover it. Both now measure elapsed monotonic time, following the repo's existing `monotonicNowMs` spelling. Two things the write path was not doing, both found in the same pass: - A successful synchronous apply now records the outcome. It is exempt from the budget, but it was also invisible to it, so a host that had demonstrably recovered kept the read path backing off for up to 30 minutes and no `recovered` transition ever came from that lane. Only success is recorded; recording failure would put the exempt lane back under the budget. - `writeSecureFile`'s JSDoc now says its boolean covers the file only. The directory harden is fire-and-forget and answers `pending` on Windows regardless, so a `true` says nothing about the directory's ACL. * fix(security): stop the hardening test doubles from faking a no-op Three CI failures on this branch, one failure shape: hardening silently does nothing and the check that should have caught it agrees. The auth critical-path test hand-rolled a `node:child_process` factory with `execFileSync`/`execFile`. The rewritten ACL path goes through `runProcessSync`, i.e. `spawnSync`, which the factory never returned — so every spawn threw into the SID lookup's bare catch, `planFor` returned null, and hardening no-opped. It mocks `child-process/run-process` now, the boundary production code actually calls and the one sibling ACL tests already mock: an export missing there fails loudly by name instead of returning undefined. Its fake icacls writes a real UTF-16LE SDDL file, so the pinned spawn count per write is a property of the ACL path rather than of the double. The test forces `platform='win32'`, so this failed on every platform, Linux CI included. `windowsSystem32Binary` is a production bug, not a test bug: it builds a Windows path with the host `join`, which off-platform yields the mixed `C:\Windows/System32/whoami.exe`. On Windows the two joins agree, which is why it survived; on Linux the SID lookup's whoami match missed and 27 of secure-file's 32 tests exercised a lane that never ran. These are always Windows paths, so `path.win32.join` is what it should have been. The import-boundary pin still read 160 after this branch migrated secure-path-windows-acl.ts off `node:child_process`; the ratchet correctly refuses a pin left above reality. * fix(security): resolve the machine-relative SDDL alias, and stop a denied read destroying the file Path hardening verified the DACL it wrote by comparing the SIDs `icacls /save` reports. SDDL substitutes two-letter aliases for well-known SIDs, and the resolution table could only hold constants -- but `LA` and `LG` name an account by RID inside the *machine's own* SID, so on a box whose user is the built-in Administrator (a CI runner, an Administrator-only install) the current user read back as `LA`, matched nothing, and hardening reported failure for every path. Resolve those two against the machine authority derived from the user SID; without one they stay unresolved and the comparison still fails closed. Three secret stores treated any read failure as "malformed -- regenerate" and overwrote. A hardened file granting a SID this process does not hold reads as EPERM while its directory stays writable, so the overwrite succeeds: renaming over an unreadable file needs FILE_DELETE_CHILD on the parent, not DELETE on the file. That destroyed the E2EE secret key, every paired device's bearer token, and the plugin vault. Distinguish EPERM/EACCES from a parse failure and refuse. Also close the async lane's unhandled rejection: `void p.then(onSettled)` turned a throw from `onSettled` into a dead main process, and the retry budget it calls threw whenever nothing had configured it -- a contract held only by import order. The budget now defaults its own bounds. * test(windows): say which ACEs icacls listed when a planted DACL fails `toHaveLength` reports only a count and vitest elides the array, so three preconditions failing on the CI runner said "expected 3, got 6" and nothing about what the sixth entry was. Name the entries in the failure. * fix(security): stop three more stores overwriting what they were denied Same swallow-default-overwrite shape as the readers already fixed, found by sweeping every store that reads under a hardened root. - plugin-storage-store.ts returned `{}` on any read failure and set()/delete() wrote it back, losing the plugin KV store. It is the secrets store's shape line for line, so the two now behave identically. - relay-revoke-outbox.ts returned [] and save() wrote it, dropping revocations that never reached the relay -- a revoked device stays live. - profile-cloud-session-store.ts mapped an EPERM read onto `decrypt-failed`, which fails the `status === 'found'` guard in clearCloudSessionIfUnchanged and falls through to an rmSync of the account session. A denied read now reports `unreadable`, which licenses nothing; the refresh path bails on it and the auth status surfaces it rather than reporting a bare reconnect. All reuse isPermissionDeniedError. The predicate stays an EPERM/EACCES allow list rather than "ENOENT defaults, everything else throws": these stores are meant to self-heal a truncated or malformed file, and inverting it would turn a corrupt keypair into an app that cannot start. The distinction that matters is "could not read it" versus "read it and it was garbage". * test(windows): plant fixture DACLs that cannot inherit what they did not plant %TEMP% grants [SYSTEM, Administrators, <user>] (OI)(CI)(F) by default, and those propagate into every fixture. Three preconditions read back 4 and 6 ACEs where 3 were planted, and the extras looked like Orca's own hardening because the shape is identical -- on a runner whose user is the built-in Administrator, the inherited trio IS the trio production grants. Combining /inheritance:r with /grant:r leaves the argument order to icacls, and that combined form drops the inherited ACEs on Windows 11 but keeps them as explicit ones on the Windows Server runner. Removing inheritance in its own invocation makes the grant the whole DACL on either host, and the fixture root is de-inherited once up front so nothing propagates in. Rooting the fixtures outside %TEMP% would not have fixed this: any directory inherits from wherever it lives. The fix is to stop inheriting, not to move. No assertion is relaxed -- the counts stay exact. * test(windows): pick a foreign SID that stays foreign on an elevated runner `S-1-5-32-544` is only foreign to a token that is not an administrator. The CI runner is elevated AND logged in as the built-in Administrator, so granting Administrators granted the reader full control: the file stayed readable, and all six preservation assertions went vacuous rather than proving anything. BUILTIN\Guests is resolvable everywhere and no interactive token is a member, so the read is denied on an unelevated developer box and on the runner alike. An unresolvable SID would have been the stronger choice but icacls rejects one with ERROR_NONE_MAPPED (1332). The premise guard is what caught this -- it asserted the file was actually unreadable instead of trusting the grant, and named elevation as the suspect. * fix(security): refuse on any read that never reached the contents, not just a denied one isPermissionDeniedError becomes isUnreadableError, because "permission denied" was never the concept -- "could not read it", as opposed to "read it and it was garbage", is. EBUSY, EMFILE, ENFILE and EIO say exactly as little about a file's contents as EACCES does, and they fell into the branch that regenerates and overwrites. On Windows EBUSY is the likelier of the two: antivirus holding a credential open at the moment of a startup read produces it, which makes it a commoner path to the same permanent loss than the ACL case that motivated the original fix. Still an allow list, deliberately: ENOENT keeps licensing a create, and a parse failure keeps self-healing. The stores are built to recover from a truncated write, and turning that into a refusal would trade a recoverable state for an unrecoverable one on the startup path. Also fixes the regression suite's own premise on an elevated runner: makeUnreadable combined /inheritance:r with /grant:r, and that form keeps %TEMP%'s inherited [SYSTEM, Administrators, user] as explicit ACEs on Windows Server -- so the file stayed readable and all six assertions were vacuous. Same split-the-invocation fix as the ACL suite's planter. * test(windows): skip the preservation suite where a read cannot be denied An elevated token logged in as the built-in Administrator reads straight through a DACL that grants it nothing -- confirmed on the CI runner against both BUILTIN\Administrators and BUILTIN\Guests, and with the grant split into its own icacls invocation so the DACL really was the planted one. On such a host the premise these tests rest on does not hold, and every assertion would pass while proving nothing. So probe once at module scope and skip rather than assert vacuously -- the same trade the ACL suite already makes for its unelevated-only case. The gate stays in the compound `<win32 check> && <flag>` form the win32 lane ratchet detects, so the file stays registered in both lane lists. Coverage is not lost where it counts: isUnreadableError has unit tests that run on every platform and every host, and the stores' refusal is exercised in full on any machine where a denial is reproducible -- which is every developer box. --------- Co-authored-by: Orca Worker <orca-worker@localhost> Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>	2026-09-05 21:13:06 -07:00
OrcaWinandOrca Worker	975bbdedcc	fix(windows): scan ports natively instead of encoded PowerShell (#17861) ... * fix(windows): scan ports natively instead of encoded PowerShell Microsoft Defender for Endpoint scored the relay's Windows port scan as suspicious PowerShell plus network discovery (T1049). The command line was `-ExecutionPolicy Bypass -EncodedCommand <base64>` around a Get-NetTCPConnection/Get-Process join -- base64 next to a policy override is the highest-weighted token pair on a PowerShell command line, and netstat only ever ran as its fallback. Invert the chain. `netstat.exe -ano` is now the primary reader and the owning process name comes from the shared native process table, which exists to keep PID lookups off PowerShell. The payload survives only as a last resort, and without the override: execution policy gates script files, never `-Command`, so nothing needed it (verified: `-ExecutionPolicy Restricted -Command` runs). Drop `-p tcp` while inverting: on Windows that protocol name means IPv4 only, so as a primary reader it would have hidden every `[::]` listener the payload used to report. Names arrive as `sshd.exe` from the table and are published as `sshd`, keeping the sshd filter and old clients' rendering intact. Routes both spawns through runProcess, removing the file from the child_process and windowsHide ratchets. * fix(windows): read netstat state by shape and refuse a truncated table Review of the port-scan inversion found two ways the new primary path could be silently wrong, both of which would have kept the flagged PowerShell payload running on exactly the hosts this change targets. `LISTENING` is not in netstat.exe. It lives in System32\<locale>\netstat.exe.mui and MUI selection follows the UI language, so the pinned-locale env in relay-command-env.ts cannot reach it -- a German host prints `ABHOEREN` and the word test parsed zero rows. The zero-listeners guard then read that as a blocked reader and ran `Get-NetTCPConnection` every 12-30s forever, or returned nothing at all where PowerShell is also restricted. Keep the word as the fast path and, when it finds nothing over output that did contain TCP rows, re-read by shape: only a listening socket has no peer. Measured on this host across all four states present (LISTENING 47, ESTABLISHED 49, CLOSE_WAIT 29, TIME_WAIT 213): zero non-listening rows with a zero peer, zero listening rows without one, and the same 47 rows parse after substituting the German state words. Shape stays the fallback because `BOUND` also prints a zero peer. Truncation was invisible: createOutputSink discards overflow, ProcessResult carries no flag, so a capped read still exits 0 and its head still parses. netstat orders IPv4 TCP, then IPv6 TCP, then UDP, so a host with tens of thousands of TIME_WAIT rows would have lost every `[::]` listener -- the exact loss dropping `-p tcp` exists to prevent, and one the zero-listeners guard cannot see. Refuse the read instead. A `truncated` flag on the shared sink would be cleaner and is left as a follow-up rather than widened into this PR. Also: decline to wait on the shared process table once the request is aborted (it takes no signal and must not be cancelled for other callers); note the name lookup as best-effort, since a TTL-cached snapshot can hand a recycled PID its previous owner name; log once on either fall-through, because both are permanent and invisible when wrong; and drop a stderr assertion that any PowerShell autoload banner would redden. Correcting the cost claim in the previous commit: the aggregate win holds with the native addon (netstat 21ms vs the retired payload 860ms at 532 processes), not without it. The addon is optional, the snapshot TTL is 500ms and the scan cadence is 12-30s, so a relay with no active agent pane never warms its own cache and pays ~1.4s cold on the CIM path -- slower than what it replaced. * fix(windows): log the port-scan fall-through on the relay diagnostic stream Checked where this code actually runs before trusting the log. `console.warn` did reach a file, but relayLogLine is the right call and the reasoning is worth recording. `scanWindowsListeningPorts` runs only in the detached relay daemon: relay.ts returns early for --connect and --orca-cli, so PortScanHandler is reached only through runRelayDaemon, and both launchers start it detached with a log file (POSIX `> relay.log 2>&1`, Windows `1>relay.log 2>relay.err.log` via Win32_Process.Create). installRelayLogRotation then wraps both streams into relay.log, which is the file the documented diagnostics tail reads. Verified by installing the real rotation over a temp path and reading the file back. So the line surfaced -- but untimestamped, in a log whose format exists so reconnect flaps can be correlated with the events around them (#7773). relayLogLine is that format and the relay idiom in 41 other places, and "since when has this host been stuck on PowerShell" is most of what this line is for. The test spies on process.stderr to pin the stream and the ISO stamp rather than just asserting something was called, since a fall-through logged somewhere unread is the failure being guarded against. Also fixes a comment that ended its own block early: `relay-*/relay.log` in a doc comment contains `*/`. * fix(windows): keep the dominant zero-peer state when reading a localized netstat Shape alone promoted any zero-peer TCP row, not just listeners. `BOUND` and `CLOSED` print a zero peer too, and on a localized host their state words are exactly as unreadable as the listening one -- so a German host with listeners plus one BOUND socket published a phantom listener. Reachable on an English host too: with zero listeners a lone BOUND row is promoted AND, because the result is then non-empty, it suppresses the blocked-reader fall-through. Group the zero-peer rows by state word and keep only the largest group. A transient BOUND or CLOSED socket cannot outnumber the listeners (51 against 0 on this host), so this removes the class rather than special-casing the words, which would just be the localization bug again. An exact tie keeps every tied group rather than guessing -- no worse than reading shape alone. Verified against real netstat output: injecting a BOUND row into the localized capture leaves the result identical to the English answer (47 rows, no phantom 65001). The new test has teeth -- reverting the grouping fails it and nothing else. Corrects two claims that were slightly wrong: the docblock said shape was the fallback because BOUND prints a zero peer, which described the hazard without saying it was unhandled; and a test comment said an English host "never sees a bound socket", true only when it has at least one readable LISTENING row. Also gates the fall-through log per reason instead of per module, so a host that parses nothing today and truncates tomorrow reports both faults. Same one-shot cost, and the vocabulary is two fixed strings so the set cannot grow. That guard matters more than it looks: --log-file rotates stdout only, so the file stderr can land in is unrotated. * docs(windows): note the direction the zero-peer majority rule can fail in The docblock described the tie case and stopped there, which reads as a complete account of the limits when it is not: a majority rule inverts if the majority is wrong, and enough transient zero-peer sockets would publish the phantoms and drop the real listeners. Someone would reasonably have concluded the rule was safe in both directions. Trigger numbers and the repro stay in the PR discussion; the code only needs the reader to know the rule has a direction, and the hatch (defer to the PowerShell reader, which reads the state word instead of inferring it) since that is the part a future editor would otherwise re-derive. * ci(windows): run the real-netstat port scan suite in CI The win32 suite only self-skips off Windows, so it passed vacuously in every lane. Register it the way the cmd-shim suite is registered. * test(windows): lower both child-process ratchets to the ground this PR took Migrating the port scan off `node:child_process` onto `runProcess` drops `src/relay/windows-port-scan.ts` from both allowlists, so both offender counts fall by one. Each ratchet pins the count from below as well as above, so a pin left above reality fails and re-opens room for the next direct import to land for free. * docs(windows): qualify the no-PowerShell claim on the netstat scan The scan starts no PowerShell of its own, but no released relay carries the optional `windows-process-tree.node` addon (only dev-channel-win-build.yml builds it), so the shared process-table read falls back to a CIM scan that forks one `powershell.exe`. The EDR win is the removal of the `-EncodedCommand` / `-ExecutionPolicy Bypass` shape, not the elimination of PowerShell. Comment-only. * docs(windows): record the identity-reader follow-up and the perf table's addon attachWindowsProcessNames reads only `name`, so it should move to `readWindowsProcessIdentityTable` once #17866 lands -- on that PR's detailed reader it would open per-process handles for a field it discards. The reader does not exist on this branch, so the call stays as-is with the follow-up recorded rather than pulling #17866 in. The process-table perf table's two Toolhelp32 rows assume the optional `windows-process-tree.node` addon. The desktop bundles it; no released relay does, so on an SSH host the CIM row is the operative number. Comment-only. * docs(windows): state the CIM scan as the relay's normal path, not a fallback No released relay carries the optional `windows-process-tree.node` addon -- release-cut.yml has zero references to it and only dev-channel-win-build.yml builds it -- so the PowerShell CIM scan is what every SSH host runs. The call-site docstring read as a conditional fallback standalone. Comment-only. --------- Co-authored-by: Orca Worker <orca-worker@localhost>	2026-09-05 21:12:59 -07:00
3 people	bfc6a262a7	fix(windows): read command lines from the kernel, not each process's PEB (#17886) ... * fix(windows): read command lines from the kernel, not each process's PEB MDE incident D scored Orca for suspicious memory activity: the vendored `@vscode/windows-process-tree` recovered every process's command line by opening it with `PROCESS_QUERY_INFORMATION | PROCESS_VM_READ` and chaining three `ReadProcessMemory` calls through the PEB and `RTL_USER_PROCESS_PARAMETERS`. On a 750ms/2s cadence over the whole table that is the credential-dumping primitive, whatever the intent. Windows 8.1 added `NtQueryInformationProcess`'s `ProcessCommandLineInformation` class (60), which returns the same string as a kernel-built `UNICODE_STRING` under `PROCESS_QUERY_LIMITED_INFORMATION` alone. Electron's floor is Windows 10, so every supported OS has it. The PEB reader stays behind a process-wide latch that only `STATUS_INVALID_INFO_CLASS`/`NOT_SUPPORTED`/`NOT_IMPLEMENTED` can set; a pid that merely denied a handle does not re-arm it, because `PROCESS_QUERY_INFORMATION` implicitly grants the limited right and so cannot be obtained where the weaker open already failed. The same hunk drops `PROCESS_VM_READ` from `GetProcessMemoryUsage` and `GetCpuUsage`, which acquired it and never read an address space. Measured on Windows 11 (514 processes), counted in-process by swapping the addon's import table entries for counting stubs, per CommandLine scan: `ReadProcessMemory` 1128 -> 0, desired access 0x0410 -> 0x1000, p50 12.7ms -> 9.3ms. Command lines were byte-identical on every process both readers recovered (376/376, 379/379 across runs), including a 24,068-character argv with quotes, non-ASCII and trailing whitespace, and a WOW64 target. Three processes that refused the old rights granted the new one; none went the other way. * chore(deps): refresh the windows-process-tree patch hash in the lockfile * fix(windows): drop the PEB fallback and detect the unpatched prebuilt Review of #17886 found three ways the reader could still perform, or silently resume, the primitive it exists to remove. The class-missing latch was a permanent, process-wide, one-way downgrade back to the PEB read, and any single target returning STATUS_INVALID_INFO_CLASS / NOT_SUPPORTED / NOT_IMPLEMENTED could trip it. On an EDR-hooked ntdll -- the entire premise of this change -- a hook that does not recognise class 60 would have restored PROCESS_VM_READ plus three ReadProcessMemory per pid per scan for the life of the process, unobservably, on precisely the machines this was written for. The fallback is deleted rather than guarded: GetProcessCommandLine now returns false and leaves the command line empty, which callers already handle, so the addon imports no ReadProcessMemory at all. That absence is what makes the property checkable on the artifact. The published 0.8.0 tarball ships a loadable prebuilt built from unpatched source; it is node-addon-api, so a bare require() accepts it, allowBuilds is false and CI installs with --ignore-scripts, and a rebuild that soft-exits on a Windows file lock leaves it in place. Source-text guards could never see it. windowsProcessTreeAddonReadsProcessMemory() checks the compiled binary instead, and is wired into the install check, the rebuild, and the relay build. The repair itself never worked: `git apply` run inside a work tree prefixes patch paths with the cwd-relative prefix, skips what does not match, and exits 0, so the branch always fell through to its own post-check throw. The package dir is always under the project root, while the fixture that covered it was in %TEMP%, outside any repo. Blinding git with GIT_DIR fixes it, and the test now runs inside a real work tree. Also from review: bounds-check the returned UNICODE_STRING against the allocation (not the size the second query clobbers) and cap the probe so a bogus length cannot bad_alloc a whole scan; test NT_SUCCESS explicitly; value- initialize ProcessInfo, which left `memory` as stack garbage -- measured, 82 processes reported the same bogus working set; and correct a comment in windows-process-table.ts that still described the command line as a PEB read. Re-measured on Windows 11 (543 processes): ReadProcessMemory 1128 -> 0, with the symbol absent from the import table so the IAT hook finds no slot to count; desired access 0x0410 -> 0x1000 on all 543 opens; p50 13.5 -> 12.3ms; 405/405 command lines byte-identical including a 24,087-character quoted non-ASCII argv and a WOW64 target; 3 processes recovered only by the new path, 0 only by the old. * chore(deps): refresh the windows-process-tree patch hash in the lockfile * test(scripts): stage a script's local imports into the native-runtime fixture ensure-native-runtime.mjs gained an import of windows-process-tree-gyp-rebuild.mjs, but the fixture copied only the script itself, so every case in the suite died with ERR_MODULE_NOT_FOUND before reaching its own assertions. copyScriptWithLocalModules already walks a script's co-located imports for exactly this reason -- its own doc comment names this failure -- so use it rather than listing files by hand. The two Windows cases still fail here, on a missing node-pty ConPTY runtime that also fails on main; this only stops a resolution error from standing in front of whatever they were meant to catch. * fix(windows): route a locked stale addon to the Windows file-lock message `pnpm install` with Orca running aborted with a raw EPERM stack. The stale-binary guard -- which deletes an addon that still imports ReadProcessMemory so a skipped rebuild cannot use it -- ran outside the try whose catch classifies Windows file locks, and whose message is literally "Close running Orca/Electron/dev processes for this worktree": exactly this situation. Measured rather than assumed: rmSync against a loaded (memory-mapped) addon throws EPERM, and `force: true` does not help, since it only swallows ENOENT. Cold copies of the same file delete fine. So the delete threw a page before the handler that knows what it means. Moving the guard inside the try is the whole fix; the classifier already matches the EPERM text. The new case runs the real script against a temp project whose stale addon is held open by a live child process, and fails against the old placement with the raw `syscall: 'rm'` stack the report described. * feat(windows): warn once when command-line recovery is refused host-wide Removing the PEB fallback removed a total-defeat vector, but it left a cliff: if NtQueryInformationProcess(ProcessCommandLineInformation) is refused -- a hooked ntdll that does not know class 60 -- every command line comes back empty and agent identity matching silently degrades to image names. The addon still loads and still enumerates, so every health check the app has stays green. A cliff nobody can see is the failure mode this area keeps producing. The querying process is the unambiguous probe. A process can always open itself with PROCESS_QUERY_LIMITED_INFORMATION, so its own command line coming back empty means the query is refused for every process -- not that some target denied a handle, which is normal for roughly a quarter of the table. Keying on our own row rather than a fraction means no threshold to tune and no false positive on a hardened box where most processes deny. One warning per session, gated on the CommandLine flag actually being requested so a future identity-only reader cannot trip it. The suite's own SELF fixture gains a command line for the same reason: a self row without one is the alarm, not a detail. * fix(windows): check the relay's staged addon at load, and answer tri-state Two gaps in the ReadProcessMemory check, both about what it does not see. It only ever looked at node_modules/@vscode/windows-process-tree. A relay host has no node_modules of ours: it loads ./windows-process-tree.node staged beside the bundle. The relay build asserts the symbol on the artifact it produces, but a bundle and the addon beside it redeploy independently, so a host that has not taken a new bundle keeps whatever binary is already there -- and the published prebuilt is node-addon-api, so it binds cleanly and then walks every process's address space. loadWindowsProcessTree now checks that file too and refuses it, falling back to the CIM scan: slower, but not the thing an EDR quarantines a host for. The predicate is duplicated rather than imported, because the config-script copy is install-time tooling that drags in node-gyp and child_process, and this module is bundled into the app and the relay. And it returned false for a binary that is not there. All three callers happened to be safe, but the name read as a safety predicate, so a future caller would take a missing binary as verified. inspectWindowsProcessTreeAddon() now answers clean/unpatched/missing over an explicit binary path -- which is also what lets the relay's staged addon be checked at all -- and each caller states which state it acts on. Both are covered by cases that fail against the old code: without the load-time check the unpatched staged addon is bound and the CIM fallback never runs, and with 'missing' folded back into 'clean' the absence case fails outright. * test(windows): load the addon in beforeAll, not at collection time loadAddon() ran while the file was being collected, so on a Windows checkout with no built addon the require threw before any case existed and took the seven patch-text cases down with it -- cases that read only the patch file and need no binary at all. Verified both ways against a deliberately unresolvable addon path: at collection time vitest reports "no tests" for the file; from beforeAll the seven text cases pass and only the three addon cases go. * fix(deps): normalize the windows-process-tree patch to LF and let pnpm own its hash `pnpm install --frozen-lockfile` failed on this branch on every platform with ERR_PNPM_LOCKFILE_CONFIG_MISMATCH, which breaks CI and the release build. Two coupled defects. The patch file was committed with CRLF -- 174 CR bytes, against zero on main -- and `.gitattributes` pins `/config/patches/*.patch -text` precisely so checkout cannot convert it, so those bytes reached every runner. And pnpm hashes a patch **LF-normalized**, so the raw sha256 of a CRLF file is a value pnpm never computes: raw sha256 322965470c05f63d8527f7d8e892ee26ee444136b66b57fd64c362a9f2ff05d1 LF-normalized f8ea245391c94da5770045aeea01fa6de466c2199c6ef46b5b769b398aa9823e The lockfile carried the raw one, at all three sites. It is the only one of the seven patches where the two digests differ, which is why the other six passed. Normalized the patch to LF and took pnpm's own value from `pnpm install --no-frozen-lockfile`; nothing here is hand-computed. With the file LF-only the two interpretations coincide, so the lockfile, the contract test's no-CR assertion and its hash assertion all agree at one number -- and `config/scripts/windows-process-tree-patch-contract.test.mjs`, which was red on this branch for the same reason, is green again. The lockfile diff is exactly the three hash lines. The regression check is the installer, not a digest. Two separate reviews "verified" the shipped hash by recomputing sha256(patchBytes) and matching the lockfile; both were wrong, because both repeated the same wrong assumption about which bytes pnpm hashes. A check that reproduces the original mistake is not independent. So the new case runs `pnpm install --frozen-lockfile --lockfile-only --ignore-scripts` against a copy of the manifest, lockfile and patches, and asserts exit 0 -- verified by deletion: restoring the shipped hash fails it with the exact ERR_PNPM_LOCKFILE_CONFIG_MISMATCH from the branch's package (windows) job. Also corrected the `.gitattributes` comment claiming pnpm hashes patches byte-for-byte. The `-text` setting is right -- `git apply` needs the exact bytes -- but that sentence is the claim that produced the wrong hash twice. * ci(windows): run the process-tree patch suites in CI Both suites only self-skip off Windows, so the binary-level check that the addon carries no ReadProcessMemory passed vacuously in every lane. * fix(windows): force core.autocrlf=input for the patch repair My LF normalization of the windows-process-tree patch broke the `git apply` repair path introduced in this PR. The two are coupled and I checked only one. Those 174 CR bytes were not editor noise. They sat on exactly the pre-image lines and nowhere else -- 107/107 in src/process.cc, 67/67 in src/process_commandline.cc, 0 on every added or context line -- because @vscode/windows-process-tree@0.8.0 ships those two sources as CRLF. Normalizing the patch made its pre-image stop matching the file it is applied against. Measured, reconstructing the true CRLF pre-image from the pre-normalization blob and applying the current LF patch: core.autocrlf plain -c core.autocrlf=input true exit 0 exit 0 input exit 0 exit 0 false exit 1 exit 0 `false` is Git's own built-in default and what "checkout as-is" selects in the Git for Windows installer -- on this box the `true` that hides it comes from the installer's system gitconfig, not from anything in the repo. There the repair throws, ensureWindowsProcessTreeCommandLinePatch reports "still reads the PEB, and repairing it ... failed", isWindowsNativeLockError does not match that text, and `pnpm install` dies with no path forward. Forcing the mode rather than `--ignore-whitespace`: both fix every cell and both leave the applied file fully LF, but `input` relaxes line endings only, so a hunk whose real content drifted is still rejected. The repair rewrites a security-relevant source file; it should stay strict about everything except the thing that is legitimately ambiguous. Not reverting the patch to CRLF: windows-process-tree-patch-contract.test.mjs (pre-existing on main) forbids CR bytes in it, and pnpm computes the same hash either way. LF plus the forced mode is the end state. The suite could not have caught this. The fixture built its pre-image from the patch itself and joined with '\n', so fixture and patch agreed by construction on any encoding -- once again a test that passes without its fix. It now emits the CRLF the real package ships, and the case runs under both autocrlf modes pinned through a temp HOME gitconfig, because the repair blinds git to the repo and so reads global config. Verified by deletion in both directions: with the flag removed the autocrlf=false case fails with the exact "still reads the PEB" dead end while autocrlf=true still passes, and with the fixture back on LF all eight cases pass with no fix present at all. Also corrected the .gitattributes comment I added last commit. It said `git apply` needs the bytes the patch was written against, which is now false -- the pinned bytes are LF and the bytes it was written against are CRLF. That is the same class of confident-and-wrong claim that produced the bad hash twice. * fix(windows): assert the rebuilt addon, and install the patch for real in tests Three follow-ups from review. **The packaged binary had no check.** The relay build asserts its own artifact and ensure-native-runtime asserts what it loads, but nothing looked at the addon copied into the packaged app -- so a rebuild that silently produced the upstream reader shipped. `rebuild-native-deps.mjs` now asserts `clean` on it after `rebuild()`. This is also the caller D4's tri-state was missing: every existing site branches on `=== 'unpatched'`, so `missing` still behaved exactly like `clean` everywhere, which was the thing making it a state rather than a boolean. Here both non-clean states fail, and they fail differently: after a rebuild that reported success, an absent binary is a broken build, not an absence to shrug at. The fake `rebuild()` had to start producing a binary for that to mean anything, so it now emits stand-in bytes and takes `addon: 'clean' | 'unpatched' | 'none'`. Verified by deletion: with the assertion removed both new cases pass. **The frozen-install case could not see a patch at all.** `--lockfile-only` resolves and never applies one, so its coverage stops at hash consistency. Added a case that installs `@vscode/windows-process-tree@0.8.0` for real with the patch and asserts the materialized `src/process_commandline.cc` carries the marker and no longer carries `ReadProcessMemory` -- about 1.5s for the pair. Correcting the brief on that one: it does **not** catch the `git apply` breakage from the previous commit. Measured -- with `-c core.autocrlf=input` removed it passes cleanly, because `pnpm install` uses pnpm's own patch applier and never runs our repair script. What it does catch is a patch pnpm can no longer apply: corrupting one pre-image line fails both cases. The repair path stays covered by the CRLF fixture in rebuild-native-deps-node-pty.test.mjs. Worth recording, since it decides whether the LF normalization was safe at all: pnpm applies the LF patch to the CRLF tarball sources without complaint, and materializes them as LF with the marker present and `ReadProcessMemory` absent. The primary install path was never affected -- only the `git apply` fallback was. **Dead timeout.** The frozen-install case passed `timeoutMs: 300_000` to the spawn while vitest capped the case itself at 30s, so on a cold runner vitest would have killed it first. Both cases now declare the budget they use. * test(windows): route the frozen-install check through the pnpm invocation owner The new patched-dependencies check hand-rolled a PATH walk naming 'pnpm.cmd', which the windows batch shim spawn boundary ratchet rejects: pnpm-cli-invocation already owns that decision for every other script, and its allowlist only shrinks. Reuse resolvePnpmCliInvocation for the command and prefixArgs, and the shared resolveCliCommand for the presence check, so no shim name is spelled here. Its `shell` flag is dropped because runProcessSync refuses it and already drives a shim through the interpreter itself. --------- Co-authored-by: Orca Worker <orca-worker@localhost> Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>	2026-09-05 21:12:47 -07:00
OrcaWinandOrca Worker	687a22e1ee	fix(computer-use): run the Windows runtime as one persistent helper (#17858) ... * fix(computer-use): run the Windows runtime as one persistent helper Microsoft Defender for Endpoint raised multi-stage Execution + Collection incidents against Orca on Windows ("Screenshots were taken unexpectedly on this device... Screen capture code was found in a script launched by powershell.exe", factor "Executes suspicious MSIL code"). The desktop script provider spawned a fresh powershell.exe per operation, so a single computer-use session produced a burst of short-lived PIDs and re-emitted runtime.ps1's inline Add-Type P/Invoke assembly on every click. runtime.ps1 gains a -Serve mode that loads its assemblies once and then reads NDJSON requests from stdin, and a new DesktopScriptRuntimeHost owns one long-lived child: lazy spawn, strict serialization, a 30s per-request timeout, restart on crash, a 120s idle shutdown, and dispose() on provider teardown. The one-shot -OperationPath path stays as the fallback, and Linux keeps its python3 bridge unchanged. Both Windows spawn sites now use -ExecutionPolicy RemoteSigned instead of Bypass, falling back once to Bypass (and logging) when a Restricted host refuses the unsigned script. * fix(computer-use): recover the runtime host instead of latching it off Review follow-up on the persistent Windows computer-use helper. A helper that died before producing a line set an unavailable flag nothing ever cleared, and the client then dropped the host for the life of the session. One transient bad spawn — a Defender scan, a locked CSC temp directory — silently restored the per-click powershell.exe burst and per-operation MSIL emission this work exists to remove, with computer use still working so nothing looked wrong. Start failures are now retried, then cool down for 60s, then re-probed; the client keeps the host so it can come back. Repeated post-answer crashes cool down too, and a single reply no longer clears the failure count. The one-shot bridge decided its execution-policy retry from a message that fell back to stdout, so a window title containing "SecurityError" could replay a non-idempotent operation — a double click, keystroke or paste — and stick the session on Bypass. The retry now requires empty stdout and a matching stderr. Serve-mode replies carry an echoed request id. Without one a single stray stdout line would make every later response answer the previous request, acting on stale element indexes with no error raised; a mismatch now kills the child. Non-JSON noise is ignored rather than counted as the helper having answered. Also: warnings reach the main process over the sidecar's IPC channel rather than its piped, unread stdio; the child is watched on close rather than exit; dispose latches so a queued request cannot respawn during teardown; and the host is split into a serve channel and an availability policy to stay under max-lines. * fix(computer-use): prove a helper never started before replaying its request The retry that replaced the permanent-latch bug could deliver unrequested input. send() re-sent the same request whenever the helper died without replying, but "no reply came back" is not "the operation did not run": runtime.ps1 synthesizes the click and only then builds the snapshot, which allocates a full-window bitmap and walks the UIA tree — a native GDI+/UIA fault there is uncatchable, and leaves the click already delivered. A deterministic fault meant three clicks from the host plus a fourth from the one-shot bridge, surfaced as a single failed operation. -Serve now writes one {"ready":true} line after its Add-Type work and before its first read, so "never started" is a fact rather than an inference. A request is replayed only when the helper died before announcing. A runtime.ps1 that predates the announcement — reachable through the provider path override — is covered by an observation-tool allowlist until a ready line proves otherwise. Host-detected aborts (timeout, desynchronised reply, oversized line) suppress the exit handler, so they were bypassing failure accounting entirely and a helper failing that way was respawned once per operation forever. They now count and are logged. Also stop charging twice for one outage: entering the cooldown resets the failure count, so the first death after recovery no longer re-enters a full cooldown and an interleaved workload cannot be stranded on the one-shot bridge. * fix(computer-use): ignore a stdin write callback from a torn-down helper stop() destroys stdin, so a write still queued at teardown calls back with ERR_STREAM_DESTROYED. The callback carried no channel or request identity and write() had no closed guard, so it ran abortChannel a second time: stopChannel no-opped but recordFailure and the warning did not, charging two failures for one operation and reaching the 3-strike cooldown at half the intended rate. That feeds the same accounting that keeps a persistently broken helper from respawning once per operation. The same root also allowed a late callback landing after a replacement channel existed to stop that channel and reject a different request with the previous one's error. Node fires the destroyed-stream callback on the next tick, well before a new request arrives, so the double-count is the reachable effect; binding the callback closes both. write() now drops payloads and error reports once closed, and the host ignores any report whose channel or request id is no longer current. * test(computer-use): pin each stale-write guard independently The channel's closed guard and the host's request-identity check are redundant by design, and the existing tests only failed when both were absent. Someone deleting one, believing the other was the covered one, would have got a green suite and a live regression — the same shape as a test that passes without the fix it was written for. Each is now pinned on its own. The channel's half is tested against the channel directly: after stop() it takes no writes and reports no error from one already queued, which the host cannot observe because it drops the channel at the same moment. The host's half is pinned by the case the channel cannot see — a live channel whose request was already answered, where backpressure delivers a write callback for a request that is no longer pending. Removing either guard alone now fails a test. Both carry a comment saying they are deliberately redundant and separately pinned, so the next reader does not have to rediscover this from the diff. * ci(windows): run the computer-use runtime host suite in CI The win32 suite only self-skips off Windows, so it passed vacuously in every lane. Register it the way the cmd-shim suite is registered. * fix(computer-use): time the runtime host cooldown on a monotonic clock The start-failure cooldown was a wall-clock deadline, so a backwards step — an NTP correction, a VM snapshot restore, a user changing the clock — left `remainingCooldown()` returning the cooldown plus the whole step. A one-hour step measured 3,660,000ms, and ten real minutes later still 3,060,000ms. Nothing shortens it from there. Only `recordSuccess()` clears the cooldown on a non-dispose path, and no request can reach a helper to succeed while it holds, so every `send()` throws `runtime_host_unavailable` first. The host is built with no `now` override and its lifecycle is a module-level singleton that shuts down at process exit, so the latch held for the sidecar's life — computer use kept working via the one-shot bridge while the per-click powershell.exe burst this host exists to remove came back silently. Store the instant the cooldown began and compare elapsed monotonic time, following the two fixes in #17884. The field is `number | null` rather than sentinel 0 because `performance.now()` legitimately returns 0. Both new tests leave `now` unset, because the bug was in the default the host picks and a test that injects a clock cannot see it. * fix(computer-use): give a queued request its own deadline The 30s request timeout was armed only in `sendOnce`, once a request reached a helper. A request behind N timing-out ones therefore waited roughly N times that with no deadline of its own: bounded, but the caller sees an `await` that looks hung for minutes and gets no error to act on. Move the serialization tail into its own class and arm a deadline at enqueue time. Only the wait is bounded — a request that reaches a helper still gets its full execution budget, so nothing that used to succeed now fails. An expired request is dropped rather than sent late: the caller has already been told it failed, and a click delivered after that is worse than no click. The tail keeps its never-rejecting shape and chains on the turn rather than on the raced promise, so a caller giving up early cannot release the next request while its predecessor is still in flight. * fix(computer-use): stop reading a locked file as an execution policy block `UnauthorizedAccess` is the FullyQualifiedErrorId PowerShell reports for a policy block, and it is also a strict prefix of `UnauthorizedAccessException`, which .NET raises for any ordinary locked or ACL-denied file. The predicate matched the token unanchored, so an AV scan holding runtime.ps1 or a locked CSC temp directory was read as a policy block. Two consequences, both bad. `escalateExecutionPolicy()` has no path back, so one false match spent the rest of the session on `-ExecutionPolicy Bypass` — the exact command line token this stack exists to stop emitting. And on the one-shot path `isPolicyBlockedStart` re-runs the operation: one-shot mode writes stdout only after the operation returns, so a crash partway through an action is indistinguishable from a helper that never started, and the click lands twice. Measured on Windows against all three records, which the test carries verbatim as fixtures: policy/Restricted FullyQualifiedErrorId: UnauthorizedAccess policy/RemoteSigned FullyQualifiedErrorId: UnauthorizedAccess genuine access denied FullyQualifiedErrorId: UnauthorizedAccessException `\b` is the whole discriminator: between `s` and `E` both sides are word characters, so no boundary exists there and the exception cannot match. Dropped two alternatives that measurement showed were wrong. `PSSecurityException` never appears — the record surfaces through a native-command wrapper and reports `ParentContainsErrorRecordException`. The prose is wrong three times over: it differs by policy, it is localized, and PowerShell hard-wraps it mid-sentence. Anchoring on the `FullyQualifiedErrorId:`/`CategoryInfo:` labels would be more precise again, but those labels are localized where the values are not, so it would lose a real block on a non-English host and strand it with no fallback. Matching the values with word boundaries keeps both directions; a fixture with translated labels pins it. The escalation stays sticky. With the predicate correct, it only fires on a machine that really does block, where re-probing the preferred policy would buy a guaranteed failed spawn per operation. * fix(computer-use): route a malformed request back to the request that caused it `ConvertFrom-Json` throws before `$requestId` is read, so the serve loop answered an unparseable request with an untagged error. On the client that is not an error at all: `deliver()` sees no matching id, calls `abortChannel`, kills the helper and charges a failure — and the helper's own message is discarded. A parse failure was reported as a stream desync with no trace of the real cause, and three of them walked into the 60s cooldown behind three misleading "did not match" messages. Recover the id from the raw line when the parse fails. No wire change: the response shape is untouched and `BridgeResponse.requestId` already documents this echo. It is the same shape the helper already returns for `not_a_tool`, where the id survives because it is read before the operation runs. Both mixed pairings degrade safely — a new script with an old client resolves the error normally, and an old script with a new client still aborts, but now reports what the helper said. When the line is mangled past recovering an id, the desync abort is the honest outcome, so keep it and carry the helper's text into it rather than replacing it. A line the helper could not tag is usually the only account of the cause. Proven against the real `runtime.ps1 -Serve`: the host can only write well-formed JSON, so the parse-failure branch is unreachable through it and the test drives the channel directly. * fix(computer-use): keep the Bypass escalation only when Bypass actually works AppLocker and WDAC constrained language mode raise PSSecurityException under the same SecurityError category a real execution-policy block uses, so the predicate matches them - correctly, on the evidence available. But those block the script at parse time, which `-ExecutionPolicy Bypass` cannot lift. The escalation was sticky unconditionally, so on a WDAC host we misdiagnosed, retried, failed again, and then latched: every later command line carried the most heavily weighted MDE token there is, on exactly the hardened, monitored enterprise machine that is watching for it. Treat the escalation as the diagnosis it is. A fallback that cannot start a helper either disproves it - the policy was not what stopped the first attempt - so revert to RemoteSigned instead of latching. When Bypass does start a helper the diagnosis is confirmed and it stays sticky exactly as before, so a genuinely Restricted machine still never pays a re-probe per operation. The revert lands inside the outage rather than only at its end, so a misdiagnosis costs one Bypass command line instead of one per attempt, and an escalation that never proved itself does not outlive the cooldown that ends the outage. Deliberately not a permanent "fallback is useless" flag: a Bypass attempt that failed for a transient reason would then disable the fallback for the session, which is the same latch in the other direction. Only `runtime_host_unavailable` proves no helper started, so only that reverts; a helper that started and then died proves Bypass works. That also makes the policy branch reachable on a final attempt for the first time, so it now rejects as unavailable rather than a generic error - that code is what routes the operation to the one-shot bridge, which carries its own policy fallback, and without it an all-blocked host would fail operations outright instead of degrading. The pre-existing "reports itself unavailable when Bypass is also refused" test pins that. --------- Co-authored-by: Orca Worker <orca-worker@localhost>	2026-09-05 21:12:33 -07:00
OrcaWinandOrca Worker	fba90e017c	fix(windows): copy the daemon host exe verbatim instead of renaming it (MDE T1036) (#17865) ... * docs(windows): document the EDR signal surface Six Microsoft Defender for Endpoint incidents fired against Orca 1.4.192 in eight days on one enterprise Windows 11 / Intune tenant. All six were behavioural process-tree scoring, not signature hits; two escalated to multi-stage incidents mapped to ATT&CK Execution and Collection. Add a reference doc mapping each attack-technique-shaped behaviour to the code that produces it and to why it exists: the renamed daemon image (T1036), the per-process PEB read, encoded policy-bypassed PowerShell (T1049), caret-escaped cmd.exe lines, and computer-use screen capture plus runtime-compiled MSIL (T1113). Records that signing is not the gate -- reputation is signer plus hash-keyed prevalence -- and carries the two evidence gaps the report noted. Adds an engineer checklist, deployment guidance for admins (AV path exclusions do not suppress EDR behavioural alerts; an MDE alert suppression rule does), and an explicit pre-deployment warning about computer use. * docs(windows): correct the PowerShell flag inventory and admin paths Review corrections to the EDR posture doc. The "encoded, policy-bypassing PowerShell" list conflated three different shapes and was incomplete. Split it into the three tiers an EDR actually scores differently -- bypass plus encoding, encoding alone, and bypass alone -- and add the sites it missed, including windows-mobile-firewall.ts, which encodes a script and launches it elevated through Start-Process -Verb RunAs. system-fonts.ts (-Command) and desktop-script-provider-bridge.ts (-File) were listed as encoded and are not. Notes that a raw grep under-reports, because the hook sites reach -EncodedCommand through wrapWindowsPowerShellEncodedCommand. Attribute the in-payload Set-ExecutionPolicy move to #16576 rather than to #16003's measurement, which keyed on -WindowStyle Hidden + -EncodedCommand, and record that the launcher's own tradeoff is unverified on a real box. Admin guidance was missing two ways a suppression rule pinned to one full path misses real activity: the .staging-<hex> sibling that exists mid-update, which is when the update-cluster incidents fire, and the userData fallback when LOCALAPPDATA is unset. Also: state the measurement conditions on the process-table timings, note that Hermes has surface even though we have no telemetry for it, note that the uninstaller names are electron-builder-generated and in no repo file, drop a volatile line count, and mark the per-operation computer-use shape as being addressed by an unmerged change. Drops the duplicated AGENTS.md section, keeping the indexed bullet. * docs(windows): reconcile the EDR posture doc with the shipped remediation Three claims in this doc became false once the rest of the Windows EDR set landed, and two told engineers the opposite of what the release does. The process-table section still described one shared snapshot taken with `Memory | CommandLine | CreationTime`, argued that splitting the cache per field set "would restore exactly the fan-out it exists to prevent", and concluded the shape was unfixable because "the information is only in the PEB". The split shipped (identity opens no handle at all), `Memory` is retired, and the command line now comes from the kernel through `ProcessCommandLineInformation` -- `ReadProcessMemory` is absent from the compiled addon and a ratchet asserts it against the import table. An engineer reading the old text would have concluded both fixes were dead ends. The PowerShell site inventories were stale in three of four lists: the port scan went native, every `-ExecutionPolicy Bypass` + `-EncodedCommand` pair was dropped as a measured no-op, and of the unencoded-bypass list only `wsl-cli-scripts.ts` survives. Regenerated against the merged tree, including the sites that reach the flag through `wrapWindowsPowerShellEncodedCommand` and never spell it, which a raw `rg` misses. Incident-evidence sections are left alone: they record what the tenant observed on 1.4.192, not what the code does now. * fix(windows): copy the daemon host exe verbatim instead of renaming it Microsoft Defender for Endpoint flagged `orca-terminal-daemon.exe` as MITRE T1036 (Masquerading): Orca copied its own `Orca.exe` into %LOCALAPPDATA% under a different name, specifically so the NSIS updater's `taskkill /IM Orca.exe` could not match, then ran it detached. Because that process is what every other flagged action was attributed to, the name mismatch acted as a reputation multiplier on unrelated findings. The rename was never what made the daemon survive. In app-builder-lib 26.15.3 the installer's FIND_PROCESS/KILL_PROCESS select processes whose image path is under $INSTDIR; `taskkill /IM` is only the fallback for hosts where PowerShell is missing or blocked. Survival is a property of the path, and %LOCALAPPDATA%\Orca\daemon-host is outside $INSTDIR whatever the file is called. Derive the host exe name from process.execPath so the copy is byte-for-byte, name included — it keeps its Authenticode signature and carries no renamed-image signal. On the no-PowerShell fallback the daemon is now killed with the app and terminals cold-restore, which is the documented pre-relocation outcome the update harness already asserts, not a regression. The uninstall macro no longer needs a distinct name to find the daemon; it kills the app's own image name (plus the legacy name, for hosts left by older builds). Adds docs/reference/windows-daemon-host-relocation.md with the survival contract, the rejected alternatives and their measured costs, and the invariants to keep. * fix(windows): apply daemon-host relocation review corrections Scope the uninstall taskkill to the current user with `/FI "USERNAME eq %USERNAME%"` via cmd.exe, matching upstream's per-user KILL_PROCESS — without it an elevated machine-wide uninstall reaches another logged-on user's session, so the "no collateral" claim in the comment was overstated. Comment the rmSync-before-publish: Windows refuses to delete a running image, so a live daemon already hosted in this version's dir (same-version reinstall, or a dev channel reusing a version) throws and materialization fails open. Doc corrections: - The fallback selector is the full per-user `taskkill /F /IM "<app>.exe" /FI "PID ne $pid" /FI "USERNAME eq %USERNAME%"`, not a bare `taskkill /IM`. - The probe reads `Get-ExecutionPolicy -Scope Process`, not the effective policy, and GPO writes MachinePolicy/UserPolicy — so GPO-managed hosts take the primary path-scoped branch. Narrow the fallback triggers accordingly. - Drop the Authenticode sentence: the old name was equally byte-identical and equally signed, so a filename has no bearing on signature validity. - Name the new update-abort path: the daemon now matches FIND_PROCESS, so on the fallback branch an unkillable host reaches the retry loop's MessageBox /SD IDCANCEL and Quits, aborting a silent update. - Correct the customCheckAppRunning rejection. It is ~6 lines, not a rewrite; it is wrong because forcing the PowerShell branch where PowerShell is absent makes FIND/KILL silently no-op and leaves the real app running with files in use. - Bound the win honestly: OriginalFilename is empty on the shipped binary, so the strongest T1036 indicator never fired, and the residual copy-and-run-detached shape still maps to T1036.005. Reconcile docs/reference/windows-edr-posture.md, which documents the rename as a live finding and would otherwise contradict this change. Content-only edit: markdown under docs/reference/ is not oxfmt-formatted as a matter of practice and nothing in CI gates it, so the file is left consistent with its neighbours. * fix(windows): expand USERNAME in NSIS instead of spawning cmd.exe The uninstall macro routed both taskkills through `"$SYSDIR\cmd.exe" /C` purely so `%USERNAME%` would expand — two extra interpreter spawns on the uninstall path, in a change whose whole point is not adding scored behaviour, and the exact `cmd.exe /c` shape the new AGENTS.md EDR bullet warns about. NSIS reads the variable itself with ReadEnvStr, so the spawns buy nothing. Verified on Windows 11 that the generated command line does what the filter is there for: a copy of cmd.exe running as orca-nonexistent-probe.exe (pid 34244) was terminated by `taskkill /F /IM "orca-nonexistent-probe.exe" /FI "USERNAME eq <user>"` — SUCCESS, exit 0, process gone. Guarded on an empty USERNAME because the degenerate case is silent: taskkill rejects an empty filter value outright ("The search filter cannot be recognized") and kills nothing, which would leave exactly the orphaned daemon this macro exists to reap. `*` is rejected as a filter value too, so there is no branchless spelling. With no USERNAME to scope by it kills unfiltered, as the macro did before the filter was added. Stack stays balanced: three pushes, two nsExec pops, three restores. Also strike the last stale row in windows-edr-posture.md's remediation table. "Copying our own image under a different name" read as outstanding work; it is done by this change, so the row now points at the relocation doc. Same class of staleness as the section reconciled in the previous commit, and git would not have flagged it either. * fix(windows): port the daemon-host uninstall sweep into the live NSIS include The uninstall macro this branch rewrote lived in config/nsis/daemon-host-uninstall.nsh, which main no longer includes: #17906 consolidated every Windows installer hook into config/nsis/orca-installer-hooks.nsh because electron-builder accepts exactly one `nsis.include`. Merged as-is, the rewritten macro would have been dead code while the shipped uninstaller kept running main's stale sweep — `taskkill /F /IM orca-terminal-daemon.exe`, which matches nothing now that the relocated host is a verbatim Orca.exe copy. The RMDir that follows then cannot delete the running image, so a live orphaned daemon and its ~224 MB tree would survive every uninstall. Ported into the live include: the ${APP_EXECUTABLE_FILENAME} kill, the USERNAME filter that keeps an elevated machine-wide uninstall out of another logged-on user's session, and the register save/restore around both. The legacy orca-terminal-daemon.exe kill stays so hosts left by older builds are still reaped. The ratchet that was meant to catch exactly this pinned only the legacy image name, which main's stale macro already satisfied, so it passed both ways. It now asserts the app-exe kill and the USERNAME filter, against comment-stripped script — the prose above the macro names both image names, so a toContain over the raw file proves nothing. --------- Co-authored-by: Orca Worker <orca-worker@localhost>	2026-09-05 21:11:28 -07:00
Neil	8ed81ceb8d	perf(tabs): index saved tab order during hydration repair (#18964)	2026-09-05 20:04:11 -07:00
Neil	56626e7daa	perf(ssh): reuse and release relay startup buffers (#18953) ... * perf(ssh): reuse the searched relay startup prefix * perf(ssh): release startup banners after relay readiness	2026-09-05 20:03:58 -07:00
Neil	bf073b833e	perf(skills): skip symlink probes beyond discovery depth (#18937)	2026-09-05 20:03:37 -07:00
Neil	6f28e019b5	perf(hooks): use native reverse search for transcript lines (#18936)	2026-09-05 20:03:33 -07:00