mirror of
https://github.com/stablyai/orca.git
synced 2026-09-29 08:03:20 +00:00
fb56ee48e76f01acf52dedf9374ae088f8bc0f02
1035
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Jinwoo-H
|
7ff23606dd | Merge origin/main into session-search | ||
 Neil
|
ed881889c4 |
fix(terminal): fold-safe CAN/SUB and double-ESC handling in partial-escape tail (#19521)
`extractPartialEscapeTail` broke its own fold invariant
(extract(a + b) === extract(extract(a) + b)) in the oscEsc/stringEsc states,
so a PTY read that split there produced a different pending tail than the
same bytes delivered whole — the tail snapshots append after a restore.
Two causes, both in the "ESC did not terminate the string" branch:
- CAN/SUB were routed through `stateAfterEscByte`, which maps them back to
`esc` instead of aborting to ground. `extractPartialEscapeTail('\x1bPx\x1b\x18X0abc')`
returned '\x1b\x18X0abc'; the chunk-split fold returned ''.
- A second ESC opened its new sequence at `i - 1` rather than at itself.
`extractPartialEscapeTail('\x1b] \x1b\x1b^')` returned '\x1b\x1b^' whole but
'\x1b^' folded. The fold was right — xterm starts the sequence at the second ESC.
The existing fuzz only asserted the fold as `advance(extract(pending), chunk)`,
which is a tautology because every PENDINGS entry is already a tail. Replaced
with a sweep that re-splits the combined stream at every code-unit boundary,
and extended the alphabet (NUL, 0x20 intermediate, CJK) and SEQUENCES with
CAN/SUB and doubled-ESC-inside-string cases. A 1.25M-split fold fuzz over a
VT alphabet goes from 421 failures to 0.
|
||
|
Neil
|
28d936e030 |
perf(native-chat): bound journal reads during paged catch-up (#19360)
* perf(native-chat): bound journal reads during paged catch-up * perf(native-chat): reduce the journal once per catch-up run, not per page Bounding the SQL read per page left the JS side still O(total items) per page: every page re-reduced the whole timeline and rebuilt the live-item map, and the byte-shrink loop rebuilt it again on each halving. A catch-up run is a synchronous loop with no await between pages, so the reduced timeline is loop-invariant. `createAgentSessionCatchUpReader` holds one snapshot for the run and re-reduces only if the journal cursor actually moved, and the projection's live-item / alias / submission-byte indexes memoize on the snapshot arrays the reducer rebuilds on change. Per catch-up over a 8,000-message backlog: 40 timeline reductions to 1, reduce+project time 24.3ms to 3.5ms, end-to-end 134.6ms to 111.3ms. |
||
 Jinjing
|
31b84cc71f |
Fix native PTY I/O failures disabling session termination (#19523)
* Fix native PTY I/O failures disabling session termination I/O failures on write or resize were incorrectly treated as exit evidence, which disabled further termination attempts and producer flow control. Separate ioFailed state from dead state; I/O errors suppress operations but keep kill, forceKill, and signal available. Publish physical exit before notifying listeners to prevent reentrant cleanup attempts from accessing the retired native process. * Fix PTY I/O cleanup tests and config syntax error - Fixed missing closing brace and comma in reliability-gates.jsonc - Added clear() operation to PTY mock fixture and test coverage - Enhanced assertions to verify operation suppression during I/O failures - Improved kill operation error handling with better promise-chain assertions - Updated test result summaries in reliability gate documentation |
||
|
Jinwoo-H
|
0f93c6d06b |
fix(ai-vault-search): close the regressions the second review round found
Second adversarial review of the consolidation pass, six areas, and the fixes for what it turned up. Providers: a degraded OpenCode capture read no longer publishes the file cursor. The read marks the capture scope incomplete, the flag rides back across the worker hop, the writer takes the not-applied path so the store marks the candidate stale and the next scan re-parses; no red badge for a retryable read. The parse deadline no longer counts time queued behind unrelated index writes (separate worker and consumer bounds). Codex metadata refresh returns the same reference whenever nothing changed. The OpenCode worker host is generalised to LazyWorkerThreadHost and the port scanner adopts it, dropping its duplicate lifecycle. Renderer: the poll latch is gone. Main emits aiVault:searchIndexingChanged after every apply, and the store does one read, subscribes to the push, and polls only while a phase genuinely advances; a failed read is an unconfirmed reading that keeps polling with a 4/8/16/32 s backoff, cleared by success or push. The web client, which has no push channel, keeps its visibility-gated interval. Coverage is observed only from a current result and never overwrites a newer run. idle is a resting phase. Unverified sources surface in the panel status line. Service and store: a store that is transiently null during clear no longer answers "search is off"; coverage falls back to consent. A superseded transient write failure rebuilds the progress batch so a stale failure count does not linger. Consent is persisted before it is applied. The WAL sampling comment names the real second connection. messages_batch is a partial index (schema 11). The benchmarks drive the real store. Database removal uses the shared Windows retry loop. Query: operator-only pages go through the ranking owner, so forks collapse the same way for `repo:app` and `needle repo:app`. Typo repair falls through to the best visible candidate instead of abandoning the prefix. The grouped CTE's measured cost is recorded next to it. The outbound projection validates against strict enums while the received schema keeps its fallbacks, and the result type is pinned by equality rather than mutual extension. Remote and CLI: search operations share one relay lane again, so a status or configure issued during a query waits unsent and survives a sidecar fault. A query gets the scan budget rather than the title budget. A rejected backfill releases the owner lease. The configure path no longer refuses on an in-flight `applied`. The search boolean vocabulary has one owner in the spec. The method record moves into the contract module and gains searchCoverage. The CLI status formatter treats applied:false as a caveat, not as unavailable. An aggregate is not partial merely because a host reported unverifiable sources. Every fix carries a test that fails when it is reverted. |
||
|
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 |
||
  
|
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> |
||
|
Jinwoo-H
|
e0281999db |
refactor(ai-vault-search): consolidate the review fixes into one owner per rule
Second pass over the session-search delta. Keeps the staged-write, streaming capture, consent-gate and SQL-side filtering designs; removes the layers that had accumulated around them. Store and schema (schema 10): row visibility is two SQL views instead of four hand-written predicates; publish nulls messages.batch_id and drops the batch row, so the messages view is `batch_id IS NULL` and a recycled batch id can no longer hide published rows; `published` column and its index removed; WAL checkpoint guard taken off every read path and sampled on the write loop only; maintenance class split into three plain functions; cwd_key indexed and the scope filter switched to a range seek with an EXPLAIN plan assertion. Service and capture: one write shape (streamingCapture flag and the dead array branch deleted); configure applies policy to the store once; one shutdown path (dispose removed) with an ownership guard so a late close cannot unregister a replacement service's sink; unverifiable sources are returned with a caveat instead of filtered like deletions; a transient write failure no longer pins the indexing badge at error; refresh lane reuses stableInFlightKey; message channel handles concurrent checkpoints. Query: relevance sort now groups by session before the candidate limit (the must-fix was only applied to newest); one operator parser shared by panel and backend with the apostrophe bug fixed; OR within a key, AND across keys; one documented case rule; received enums tolerate unknown values; result type derived from the schema; projection applied on the desktop IPC path; tokenizer contract pinned against fts5vocab. Remote and CLI: SSH and local ids rejected at the RPC boundary for all four methods; method-name regex sniffing removed from both transports; CommandSpec gained booleanFlags/repeatableFlags so args.ts carries no command vocabulary; settings update no longer blocks or fails on scanner reconfiguration; relay owner keeps its lease through caller cancellation and answers index-status on an unreadable policy; one SESSION_SEARCH_METHODS record feeds every caller. Renderer: coverage polling stops once the index settles and re-arms on focus; search results feed the coverage store instead of re-fetching; the `updating` state and header line are deleted in favour of the list's own loading state; local-only notice only when a remote host is in scope; status-bar segment is read-only and opens settings; ownerKey folded into the args key. Providers: OpenCode capture resets (not clears) the parse deadline; capture and preview reads degrade to a scan issue instead of dropping the session; a consumer write failure is not counted as a worker death; response union discriminated on `kind`; worker host split out of the client. Every behavioural fix carries a test that fails when the fix is reverted. |
||
 BingZandNeil
|
5bd0247aaa |
fix(xterm): remove scrollback decorations by identity (#13178)
* fix(xterm): fire Marker dispose before clearing line (#10879) Scrollback trim under search highlights was O(k²) because dispose set marker.line to -1 before onDispose, collapsing SortedList keys. Fire listeners first so delete still sees the real line, then clear the line. Fixes #10879 * fix(xterm): remove scrollback decorations by identity * perf(xterm): avoid index arrays for unique decorations --------- Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com> |
||
|
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. |
||
|
Jinwoo-H
|
98c48addd7 | Add host-owned remote session search and harden indexing lifecycle | ||
|
Jinwoo-H
|
e8220c2cac | Merge main and preserve session search alongside chat resume | ||
|
Neil
|
2265fce591 | chore(mobile): remove stale max-lines exceptions (#19366) | ||
|
Jinjing
|
9fed61e5c2 |
Persist agents sidebar search visibility as pairing-local preference (#19313)
* Persist agents sidebar search field visibility as pairing-local preferen - Add `agentsShowSearch` to workspace UI state with default on - Include in pairing-local fields so preference syncs across clients - Convert search from menu action to checkbox menu item for explicit toggle - Update activity thread options menu to reflect checkbox state - Add localization strings across all supported languages - Update RPC schemas and preference persistence layer - Includes readiness validation reports confirming feature is clean * rm review * fix documentation |
||
 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. |
||
   
|
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 (
|
||
|
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. |
||
|
Neil
|
2ccf35b135 | fix: avoid quadratic trimming during fullscreen terminal redraws (#19214) | ||
|
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
|
||
|
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 |
||
|
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 |
||
|
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 |
||
|
Jinwoo-H
|
66d9db8804 | Bound session search writes with streaming capture and atomic publication | ||
|
Brennan BensonandMerge Sim
|
20eea184cc |
feat(native-chat): offer the link-action popover for chat links (#19130)
* feat(native-chat): offer the link-action popover for chat links A plain click on an http(s) link in a native chat transcript opened the system browser outright, ignoring the link-routing preference the same link honors in the terminal. Chat now shows the terminal's destination popover, with the modifier chords routing straight to a destination. The popover, its request type, the destination policy and the routed open move out of terminal-pane so both surfaces share one implementation; the catalog keys keep their original namespace because they carry shipped translations. Chat resolves its link owner from the session workspace (runtime, then SSH, unresolved stays unknown) so a remote transcript only offers Orca Browser when that host's managed browser route is eligible. The existing toggle now governs both surfaces, so it is retitled; with it off a chat link still opens on a plain click instead of going dead. * Fix native chat link popover lifecycle and keyboard anchoring * test(native-chat): use one store mock for link actions * fix: update reliability gate for shared link popover tests --------- Co-authored-by: Merge Sim <sim@local> |
||
|
Jinwoo-H
|
5d1d41a8e4 | Batch search retention cleanup and preserve runtime ownership across queries | ||
|
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. |
||
|
Jinwoo Hong
|
b8311d509a |
Revert "skills: rewrite the seven non-orchestration guides to one outcome-first standard (#18724)" (#19126)
This reverts commit
|
||
|
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>
|
||
|
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 |
||
|
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.
|
||
|
Jinwoo-H
|
0930c03cc1 |
Merge remote-tracking branch 'origin/main' into session-search
# Conflicts: # src/cli/help.ts |
||
|
Brennan BensonandMerge Sim
|
298571ad9f |
fix(codex): uncap app-server stdio records (#18590)
Co-authored-by: Merge Sim <sim@local> |
||
|
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.
|
||
|
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
|
||
|
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 |
||
|
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 |
||
|
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 |
||
|
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 |
||
|
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 |
||
|
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 |
||
|
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>
|
||
|
Jinwoo-H
|
a4c0ca4798 |
ci: run the session-search FTS5 contract on the Windows package job
Every FTS5 claim so far was measured on macOS and Linux; the Windows job is the only place the Electron-bundled SQLite runs on Windows in PR CI. |
||
|
|
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> |
||
|
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> |
||
|
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. |
||
|
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 |
||
|
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. |
||
|
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>
|
||
|
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> |
||
|
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>
|