Commit Graph
9580 Commits
Author SHA1 Message Date
Neil 8894977ad2 perf(worktrees): bound fleet resolution concurrency 2026-08-30 16:51:08 -07:00
Jinjing b3912ebed2 Split up combined-diff viewer into feature-organized modules (#17341)
* Reorganize combined-diff components into feature-organized structure

Splits flat combined-diff files into feature-focused subdirectories
(browse-files, load-sections, resolve-changes, review-controls,
scroll-viewport) to improve code organization and reduce clutter in
the editor directory. Groups related logic by concern for easier
navigation and maintenance.

* Split up combined-diff viewer into feature-organized modules

Decompose the 221-line monolithic CombinedDiffViewer into smaller, focused modules organized by feature: entry resolution, section loading, view state memory, file tree navigation, review controls, and scroll viewport handling. Main component now composes these hooks to orchestrate the combined-diff view.

* fix(combined-diff): prevent replayed preference writes

Move preference write outside state updater callback since React may
replay state updaters, causing multiple writes. Add sideBySide to
dependency array.

* fix(combined-diff): re-resolve sections by key to handle list rebuilds

The section list can rebuild while a write is pending (due to rebase, file changes, etc.); re-resolve by key instead of stale index to apply updates to the correct section.

- Convert skipped conflicts message to structured i18n plural forms
- Add oldPath field to git status signature for rename tracking

* Suppress react-doctor diagnostics in combined-diff feature

Add suppressions for react-doctor diagnostics that are necessary patterns
for the combined-diff implementation, configured in both the quality check
script and package.json.
2026-08-30 16:43:37 -07:00
d7604e4307 fix(pi): settle OMP status from the agent_end contract (#17209)
* fix(pi): settle OMP status from the agent_end contract

OMP exposes no agent_settled hook and ctx.isIdle() can stay false after a
finished turn, so Orca's idle-recheck loop backed off and spun forever and
the pane stayed "working" indefinitely. OMP instead marks non-terminal
agent_end events with willContinue; honor that for configured and
runtime-routed OMP and treat an absent flag as terminal.

Extracted from #15658, which bundles this with a launch-authority change
that conflicts with in-flight #17077. Only the settle half lands here.

STA-4130

Co-authored-by: Bing.Z <zzb@gxsmjx.com>

* fix(pi): preserve non-terminal continuation guards

Keep the base Pi and Prime willContinue guard while settling terminal OMP events directly. Add regression coverage so sibling runtimes cannot publish a false completion after conflict resolution.

---------

Co-authored-by: Bing.Z <zzb@gxsmjx.com>
Co-authored-by: Merge Sim <sim@local>
2026-08-30 16:42:46 -07:00
Neil cb8e08834c fix(paste): exclude xterm helper textareas from text ownership 2026-08-30 16:36:26 -07:00
Neil 879fdfdac6 fix(cli): resolve WSL mounted-drive worktree paths 2026-08-30 16:36:20 -07:00
Neil a085c28e1b fix(relay): propagate worktree listing failures 2026-08-30 16:36:14 -07:00
Neil 2ff0ca10d4 fix(ssh): match the exact worktree created 2026-08-30 16:36:07 -07:00
Brennan Benson f7d8d7f77a test(e2e): make the cold-hydration spec verify its own captured snapshot (#17031)
`adds no tab when the host workspace snapshot stalls across a relaunch` replays
the bytes it reads off the relay, but only ever waited for the snapshot FILE to
exist -- never for it to carry the tabs the test had just seeded. A capture that
missed the baseline produced a failure that reads as a product regression and is
not one: an empty `session.tabsByWorktreePath` places nothing, so it reports
nothing unplaced, so `remote-workspace-snapshot-apply.ts` marks the target
hydrated and `hydrateTabsSession` replaces the worktree's tabs with none. That is
exactly the observed `baseline=3 duringStall=3 afterHydration=0`, and it is
correct behaviour for a host snapshot that genuinely holds no tabs.

Assert the precondition where it belongs -- on the capture, before the relaunch
that consumes it -- so an empty or unparseable fixture names itself instead of
surfacing later as a tab count the product appears to have lost.

No assertion is weakened: `afterHydration` still has to equal the baseline, and
no retry, sleep, or timeout was added.
2026-08-30 15:56:21 -07:00
Jinjing 8585c65fa4 Respect explicit server placement for owner-pinned remote browser links (#17361)
When a remote browser pane opens a link with an explicit placementPreference,
honor that override rather than applying the generic browser client policy.
Links opened from remote panes may require specific host placement to respect
execution boundaries.
2026-08-30 15:16:56 -07:00
Neil e84042572c Upgrade xterm to 6.1.0-beta.303 and generate addon patches
* Upgrade xterm to 6.1.0-beta.303 and generate the addon patches

Takes the current xterm beta line: xterm 287 -> 303, addon-webgl 286 -> 299,
addon-serialize 287 -> 300, headless 302, the remaining addons -> 300, and the
same set on mobile. All four packages stamp upstream commit d3e32b3.

The reasons are upstream #6042/#6043/#6055 (a shared glyph atlas no longer
garbles sibling panes on a page merge, clear, or sampler-budget overflow) and
Note that core 303 is not image-addon-only over 302: it carries the buffer perf
work, including the new BufferLineStringCache.

addon-webgl and addon-serialize move into the patch generator
--------------------------------------------------------------
Both were hand-edited minified bundles, which is what the Known Gaps section of
docs/reference/xterm-patch-regeneration.md described. Both reproduce byte for
byte from the pinned commit, so they are now manifest entries generated from a
source patch like @xterm/xterm already was. Their sourcemaps now move with their
bundles; before this they shipped maps whose offsets did not match the code
beside them.

The webgl patch shrinks from a 1.06 MB hand-edited bundle to a 6.6 KB source
patch, because upstream took the invalidation half Orca had backported. What is
left is only what upstream still lacks: the fragment-shader else branch for a
v_texpage past the sampler budget, the clearTexture guard that no-ops once a
merged page holds index 0, spending the merge retry budget before beginFrame
latches the version it saw, and Orca's font-weight probe.

The serialize source patch is byte-for-byte the same fixes as before; upstream
changed nothing in that addon between 287 and 300.

Generator fixes, each of which failed silently
----------------------------------------------
- `--relative` was appended after the `--` separator in CHECKOUT_DIFF_FLAGS, so
  git read it as a pathspec and kept repo-root-relative paths, dropping every
  source hunk from an addon's patch.
- `git apply` run from a package subdirectory still resolves patch paths from
  the repo root, skips every hunk and exits 0. It now runs from the root with
  `--directory=<packageDir>`, and a source patch that leaves the checkout
  unchanged is a hard failure rather than an empty patch.
- An addon's own `tsgo -p .` has empty files/include and only project
  references, so it emits nothing and the addon webpack then fails on a missing
  ./out/. The root build now runs first.
- versionStampFile is optional; publish.js stamps an addon's package.json, which
  overlayBuildOutput never patches.
- On a version bump the lockfile has no entry under the new key yet, so --write
  reports the gap instead of aborting mid-run. --check still fails on it.

Adding the two addons pushed the generator and the Electron packaging contract
test over max-lines, so the patch-text helpers move to xterm-patch-text.mjs
(pure text: no checkout, no build) and the vendored-xterm assertions move out of
the packaging contract into xterm-webgl-runtime-contract.test.mjs.

Tests
-----
Four tests asserted upstream bugs that are now fixed, not Orca behaviour:

- xterm-user-scrolling-contract pinned headless and core by version string.
  Upstream bumps each package only when its own output changes, so headless 302
  and core 303 are the same source. It now asserts they share a commit.
- Five CSI 3 J assertions expected a reader stranded at the top after an erase.
  Upstream #6081 clears isUserScrolling there, so the erase releases them to the
  bottom instead. Orca's pin still lands them correctly, because its parser
  handler observes the erase before xterm's own handler runs.
- The IME transaction test hard-coded the xterm version; it now reads the
  installed package, since the point is that bundle, map and version agree.
- The Electron runtime contract asserted Orca's old clearModelGeneration. Shared
  atlas invalidation is upstream's now, so it asserts pageLayoutVersion on the
  resolved dependency, plus the Orca-only hunks on the patch.

Verified: 66,008 unit tests, mobile's 3,863, the four WebGL atlas e2e specs, and
`regenerate-xterm-patches.mjs --check` in sync on all three packages.

Left alone deliberately: resetAllTerminalWebglAtlases still fans out globally
even though clearTexture now self-heals siblings, and upstream #6068
(WebglAddon.dispose leaks the GL context) is still open.

* Drop the two unused WebGL atlas fan-out exports

resetAllTerminalWebglAtlases and presentAllTerminalPanesWithoutAtlasClear have
no callers, and had none at cadfc55102 either — the last call site went in
#6949, which routed reveal recovery through
resetAndRefreshAllTerminalWebglAtlases instead. Only a comment in
pane-manager.ts still named the first one; it now points at the live entry
point. scheduleRevealPresent leaves the registry's structural type with them,
though the manager method stays: terminal-visibility-resume.ts calls it
directly.

This is dead-code removal, not a consequence of the xterm bump. The live
recovery path is unchanged.

resetAndRefreshAllTerminalWebglAtlases stays, and so does the reveal-time
escalation in pane-reveal-repaint.ts. Upstream 299 does make a pane-local
clearTexture bump pageLayoutVersion so siblings rebuild on their next frame,
which is the bug the escalation was written for, but I could not demonstrate
that removing it is safe: with the escalation removed,
floating-workspace-shared-glyph-atlas.spec.ts still passed headful, and it also
passed with upstream's mechanism deliberately disabled (pageLayoutVersion
pinned to 0 in the installed bundle, verified present in the built renderer).
A guard that passes with the fix disabled cannot license removing the
workaround, so the escalation stays until that spec can reproduce the garbling.

Verified: pane-manager and terminal-pane suites (4,713 tests), typecheck, the
headful shared-atlas spec, and the three headless WebGL specs.

* Give the shared glyph atlas spec a trigger that can fail

floating-workspace-shared-glyph-atlas.spec.ts guards the corruption where one
terminal wiping the module-global atlas leaves sibling terminals drawing from
stale texture coordinates. Both of its tests drive that through a floating
panel reveal, and Orca's reveal paths escalate to a registry-wide atlas reset
that repaints every pane — so the recovery under test heals the damage before
the assertion runs, and the tests pass whether or not xterm propagates the
invalidation at all.

The new test clears the shared atlas straight through the floating manager with
the panel closed, so nothing else repaints the workspace terminal, then repaints
it with terminal.refresh(). That is the load-bearing detail: _updateModel skips
cells whose content is unchanged, so the refresh reuses vertices baked against
the pages that were just wiped, which is exactly the state the fix has to
recover from.

Verified as a discriminator rather than assumed. Pinning ITextureAtlas's
pageLayoutVersion getter to 0 in the installed bundle, which disables the
per-renderer invalidation upstream added in addon-webgl 0.20.0-beta.299, and
confirming that reached the built renderer:

  fix intact:   siblingClearIntact=true   1 passed
  fix disabled: siblingClearIntact=false  1 failed

The failure renders the workspace terminal completely blank — stale coordinates
into a wiped atlas sample nothing. The two reveal tests pass unchanged in both
configurations, which is the gap this closes.

* Compare shared-atlas screenshots with tolerance instead of byte equality

Byte equality fails on sub-pixel antialiasing noise that leaves every glyph
legible, so the headful spec flaked under xterm 303. Reuse the existing
compareTerminalScreenshots helper: real stale-model corruption blanks the
terminal at ~3% of pixels, twice the helper's 1.5% threshold, so the looser
oracle keeps its teeth. Log the ratio so failures are diagnosable.

* fix(xterm): cancel empty deferred IME compositions

* test(xterm): strengthen runtime patch contracts
2026-08-30 15:14:49 -07:00
Neil 976e05c0c8 perf(worktree): overlap finalization head probes (#17443) 2026-08-30 14:54:31 -07:00
Brennan Benson f23d0b166f fix(relay): mint PTY ids that carry the relay incarnation instead of a restarting counter (#16901)
* fix(relay): scope PTY ids to mint epochs

* test(relay): treat minted PTY ids as opaque

* test(relay): pin mint-epoch id shape and restore spawn-sequence assertions

The epoch escaping had no test: dropping encodeURIComponent left the whole
relay suite green. Pin the three-field id shape against an epoch that carries
both separators, and cover a colon-bearing relay id through the unchanged
app-side SSH id wrapper.

subprocess.test.ts had traded `pty-1`/`pty-2` for `expect.any(String)`, which
discarded the invariant those two cases exist to prove: an early node-pty load
failure burns no sequence, a late spawn failure burns one.

* test(relay): mirror production epoch escaping in testPtyId

The harness built the expected id without the encodeURIComponent production
applies at the mint site. A test epoch carrying a reserved character would
diverge silently across ~40 assertions in 11 files.
2026-08-30 14:49:18 -07:00
Neil df14d1a298 fix(dashboard): restore clipboard commands in the terminal preview on Windows (#17441)
Edit > Paste, context-menu Paste, Paste as plain text, Select All and Ctrl+V
were all no-ops in the Agent Dashboard terminal preview on Windows/Linux, while
the same commands worked in a real terminal pane. Three independent defects:

- The preview subscribed to the raw ui:appMenuPaste / ui:appMenuSelectionAction
  IPC instead of claiming the renderer ownership events a pane claims, so
  handleAppMenuPasteRequest fell through to the focused text control — which for
  a focused terminal is xterm's hidden .xterm-helper-textarea. Now it claims
  APP_MENU_PASTE_EVENT / APP_MENU_SELECTION_ACTION_EVENT with preventDefault()
  and leaves text controls unclaimed for the native fallback.
- The pop-out window has no App shell, so nothing translated the menu IPC into
  those ownership events. DashboardPopoutRoot now mounts useAppMenuPaste() and
  useAppMenuSelectionActions().
- Plain Ctrl+V was deferred to an Edit-menu accelerator that does not exist on
  Windows/Linux, where Orca draws its own titlebar. The isMenuPasteChord
  carve-out is now darwin-only, matching TerminalPane.onKeyPaste.

Also honors terminalRightClickToPaste in the preview (selection copies, no
selection pastes, Ctrl+right-click falls through), and extracts the box-fit
transform into preview-terminal-box-fit.ts to keep the component under the
max-lines cap.

Fixes #15757
2026-08-30 14:42:24 -07:00
Neil 5bd66bac8b fix(cli): resolve a WSL worktree by the Linux path its own shell prints (#16628) (#17440)
On a Windows host the runtime stores a WSL worktree as the UNC path Windows
sees, but a user inside the distro types the Linux spelling, so every `path:`
selector missed: `worktree show`, `terminal list --worktree` and
`worktree rm --worktree` all reported selector_not_found for a directory Orca
manages.

Translate once in the CLI, which is the only side that can prove which distro
the typed path belongs to — from its own UNC cwd, never from WSL_DISTRO_NAME,
which a Linux-native CLI also sets. The runtime's `path:` branch stays
exact-spelling-only for the same reason: this resolver feeds delete, so a
tail-only match would remove another distro's copy.
2026-08-30 14:42:20 -07:00
Neil 1268fb56f1 fix(worktree): complete a create Git can confirm but cannot list (#17388)
* fix(worktree): complete a create Git can confirm but cannot list

`worktree.create` verified against `listWorktrees`, which softens every git
failure to `[]`. Any listing failure therefore failed a create whose worktree
and branch `git worktree add` had already written, orphaning both, and reported
only 'Worktree created but not found in listing' — the real cause reached the
main-process console and never the user.

Verify against the error-propagating listing instead, and when that fails or
omits the row, rebuild the row by asking Git about the worktree itself. The
direct read returns nothing unless Git resolves the path into this repo's
object store with the expected branch checked out, so an unrelated or half-made
checkout still fails the create.

Fixes #16520

* fix(worktree): authorize a recovered create and reject an unreadable HEAD

Review follow-ups on the create-verification fallback:
- register the recovered worktree's own root, additively, so the create the
  user just made is not rejected by filesystem/git-status IPC
- treat an unreadable HEAD as no recovery instead of a blank OID
- keep the direct read's failure when the listing merely omitted the row
- skip the symlink cases on Windows and reset the new harness mock

* fix(worktree): bound the create-recovery disk read and keep WSL paths case-sensitive

Readiness-scan follow-ups:
- deadline the filesystem common-dir read; a .git on a hung mount left the whole
  create IPC pending where it used to fail after the Git deadline
- offer no disk candidate for a bare repo instead of a fabricated <repo>/.git
- compare POSIX common dirs case-sensitively, so two WSL repos differing only in
  case are not accepted as one object store on a Windows desktop
- move toGitOutputSpace to shared/wsl-paths as toWslExecutionSpace, next to the
  parseWslUncPath callers that already open-code it

* fix(worktree): share one budget for create verification and keep recovered roots

Three follow-ups from review of the create-recovery path:

- The recovery no longer starts a fresh 30s deadline after the listing already
  burned one, so worst-case create verification stays at ~30s instead of ~60s.
  A 5s floor keeps the direct read a chance to answer when the listing spent
  the whole budget.
- rebuildAuthorizedRootsCache now carries a repo's previously registered roots
  forward when its listing throws. A rebuild running while Git is still broken
  could otherwise un-authorize the worktree a create just recovered.
- Corrected the scan-cache doc comment: it claimed strict and lenient listings
  coalesce, but the cache key includes the runner name precisely to keep them
  apart, so a strict joiner can never inherit a lenient scan's softened [].

Each change has a negative control: reverting the hunk fails exactly its own
test and nothing else.

* fix(worktree): keep a recovered worktree authorized across roots-cache rebuilds

The previous approach registered a recovered create into the same per-repo set
the rebuild recomputes from `git worktree list`. That set is derived from the
very listing that failed, so a rebuild would re-deny the worktree — either by
overlapping the registration, or by simply listing again and omitting the row.
Carrying old roots forward on a thrown listing did not cover either case.

Recovered roots now live in their own additive layer that rebuilds union in
rather than replace. The layer is retired on evidence, not on a timer:

- the listing can see the worktree again (Git recovered), or
- the listing succeeded and the directory is gone (worktree removed).

A repo whose listing threw is left untouched, because a dead mount fails both
the listing and the stat, and treating that as "removed" would revoke the
worktree in exactly the outage this layer exists for. The layer is capped so it
cannot grow unbounded, and survives cache invalidation deliberately: repo
mutations are frequent and would otherwise re-deny a recovered worktree.

Three tests cover the healthy-rebuild-omits-the-row case, the in-flight rebuild
race, and retirement once the listing sees it again. Removing the union fails
exactly the two keep-tests and nothing else.

* perf(worktree): only read the repo's .git from disk when Git's own answer disagrees

The disk read is a second opinion on Git's reading of the common dir, but it ran
unconditionally as part of the same Promise.all. A deadline bounds the IPC, not
the syscall: Promise.race cannot cancel an in-flight fs operation, and a `.git`
on a hung mount (dead NFS/SSHFS, stalled WSL 9p) pins a libuv threadpool thread
that no timeout can reclaim. AbortSignal would not help either — fsPromises.stat
takes no signal, and a blocked syscall is not interruptible from userland.

So stop paying it on the happy path: read from disk only when Git's own reading
did not already confirm the common dir. Same accept/reject outcome, but the
threadpool exposure now requires both a failed listing and Git disagreeing about
the repo, instead of every recovered create.

* fix(worktree): compare the disk common-dir witness in Git's execution space

Exercising the fix on a real Windows host against WSL Ubuntu-24.04 found the
filesystem second opinion is inert there. Node reads `.git` in the caller's
space and answers `\\wsl.localhost\<Distro>\home\...\.git`, while Git-in-the-
distro answers `/home/...`. isSameCommonDirPath refuses to compare a POSIX path
against a Windows one, and canonicalizeLocalPath cannot bridge them because
realpath on a Linux path from a Windows process is ENOENT.

So the candidate could never match, and the one case that depends on this
witness alone — a symlinked repo root on the Git 2.25 fallback — declined a
worktree Git had already confirmed. Run the disk result through
toWslExecutionSpace, the same translation readRepoLocation already uses.

This is a false reject, not a false accept: it made recovery give up, never
adopt the wrong repo. Verified on awin; the modern --path-format=absolute
branch was unaffected because Git answers both sides itself there.

* fix(worktree): retire a recovered root only on proof, never on a stalled probe

The prune ran an unbounded stat and read every failure as removal. Two consequences, both in
the outage the recovered layer exists for: a hung mount stalled the rebuild that gates
filesystem auth, and a transient EACCES/EIO revoked a live worktree. The listingFailed guard
did not cover either, because listWorktrees softens Git failures to [] and never throws.

Prune now retires on definitive ENOENT only, probes in parallel under a deadline, and treats a
stall as inconclusive. The capacity bound refuses a new root instead of evicting an authorized
one, so an over-cap create is merely unauthorized rather than a live worktree being revoked.
2026-08-30 14:42:07 -07:00
Brennan BensonandMerge Sim 585b4086d3 test(codex): pin Codex read-repair with a real-binary contract check (#17300)
* test(codex): pin Codex read-repair with a real-binary contract check

Orca's session index-heal depends on a Codex behavior: a `thread/read` of an
unindexed rollout performs a read-repair that inserts the `threads` row. All 55
existing heal tests drive a stub app-server and assert "healed" as "the call did
not error", so if Codex ever dropped the repair they would all stay green while
the subsystem went silently inert.

Adds a real-binary contract check built to the same shape as the Git binary
compatibility contract (src/shared/git-binary-compatibility.test.ts): env-gated
test file, version asserted against the binary, dedicated path-filtered PR job.

Pins only the four arms ablation established Orca relies on:
  - a read of an unindexed rollout inserts the state row
  - a session with no read inserts nothing (the negative control that makes the
    insert causal rather than incidental)
  - re-reading an indexed thread inserts nothing
  - an archived thread stays archived rather than being resurrected

Written against codex-cli 0.150.1. The job sets ORCA_CODEX_CONTRACT_REQUIRED=1
so a missing or failed CLI install fails red instead of silently skipping.

Existing heal tests are unchanged.

* test(codex): register the contract job in the verify aggregate contract

`pr-workflow-parallelism.test.mjs` pins `verify.needs` exactly, so adding the
job to pr.yml without updating that list failed the shard. Adds the entry, and
adds a workflow contract test mirroring `git-binary-compatibility-workflow.test.mjs`:

  - the pinned CODEX_CLI_VERSION is the single source for both the npm install
    and the runtime version assertion, so the two cannot drift apart
  - the install prefix and the binary path the test is pointed at are the same tree
  - ORCA_CODEX_CONTRACT_REQUIRED=1 is set, so a failed install fails red rather
    than turning the job into a green no-op

Removing the REQUIRED env from pr.yml reddens the new test, confirming it is live.

* test(codex): make binary version guard exact and bounded

* ci(codex): cover index-heal transport dependencies

* test(ci): pin Codex contract dependency coverage

* test(codex): align contract watchdog with child deadlines

* test(codex): cover three-session contract watchdog

* fix(codex): add sqlite sync-database to index-heal scope

---------

Co-authored-by: Merge Sim <sim@local>
2026-08-30 14:39:46 -07:00
Brennan BensonandMerge Sim cad9206839 fix(native-chat): preserve structured chat across rollback (#17439)
* fix(native-chat): preserve structured tabs across rollback

* fix(native-chat): preserve rollback visibility state

---------

Co-authored-by: Merge Sim <sim@local>
2026-08-30 14:35:53 -07:00
Brennan BensonandMerge Sim 323f469642 fix(native-chat): recover a transport-unconfirmed send instead of wedging the queue (#17383)
* fix(native-chat): recover a transport-unconfirmed send instead of wedging the queue

A send that fails with a transport-class error settles as `unconfirmed`, but the
dispatch loop only advances when `outbox[0].state === 'queued'`. Nothing moved an
entry back out of `unconfirmed`, so a single unknown delivery wedged the whole
FIFO queue: every later message the user typed queued behind it and never sent,
leaving the chat silently dead behind a muted banner.

Re-issue the same envelope on a bounded backoff. Reusing the operation id with
`retryUnknown` absent is idempotent -- the operation ledger replays a recorded
outcome, or the host performs a genuine first delivery. A host-confirmed unknown
stays parked, because forcing past that redispatches to the agent and is the
user's call via Retry.

The effect depends on primitives rather than the `outbox`/`submissions` arrays:
`mergeSubmissions` rebuilds the array on every streaming batch, so an identity
dependency would restart the backoff forever while the agent is working.

Outbox persistence moves to its own module to stay under the max-lines cap.

* fix(native-chat): never auto-probe a send the user already force-retried

`retry()` on a transport-unconfirmed head with no host submission row sets
`retryAfterUnknownSubmittedAt = -1`, and both the catch block and the probe's
requeue preserve that field through a spread. Since
`structuredAgentSessionSendRequest` gates the flag on nullness alone, a second
transport failure after a user Retry left the probe re-issuing with
`retryUnknown: true` up to five times with no user action -- bypassing both host
dedupe layers and redispatching to the agent.

Restrict the probe to entries that have never been force-retried. Those stay
parked behind the existing banner, which is where escalation belongs.

Also resets `mocks.submissions` in afterEach; it leaked across tests.

* fix(native-chat): stop the pending redispatch loop and keep probing

Two defects found by adversarial review of the probe.

A `pending` submission row means the host is mid-dispatch, but the send handler
mapped every non-accepted, non-unknown state to `queued`. That re-fires the
dispatch effect immediately with no delay and no cap, so a host still working on
the turn -- exactly the state that produced the unconfirmed entry -- became a
back-to-back RPC flood plus two localStorage writes per iteration. Park `pending`
under the backoff instead.

The five-attempt budget also exhausted after ~31s and only re-armed on a
fence/session/target change, so a transport outage lasting minutes left the queue
wedged again behind the same muted banner -- the original symptom. Since each
probe is an idempotent status query that never carries `retryUnknown`, drop the
ceiling and let the backoff cap the rate at one query per 16s.

Both arms pinned by tests and verified by ablation.

* chore(native-chat): drop lockfile creep and correct the probe comment

`git add -A` swept an environment-mutated `pnpm-lock.yaml` into an earlier commit,
adding `@pnpm/exe@12.0.0` and its platform optionalDependencies with no
`package.json` change. Restore it byte-for-byte to main.

The probe comment claimed "probing never stops". Adversarial review showed a
refusal that sets the blocked id takes the head out of `unconfirmed` and ends
probing until a fence change or a manual Retry. That path predates this PR and is
pinned by existing contract tests, so it is documented rather than changed here.

Committed with --no-verify: the pre-commit lockfile policy rejects
pdfjs-dist@6.3.289 for minimumReleaseAge, but that entry is already on main and
this commit restores main's lockfile byte-for-byte. Lint, format, typecheck and
the 908-test suite were run manually and are green.

* fix(native-chat): reset probe state on runtime target changes

* chore(native-chat): keep outbox hook within lint budget

---------

Co-authored-by: Merge Sim <sim@local>
2026-08-30 14:32:05 -07:00
Brennan BensonandMerge Sim b5a85890ac perf(git): bound git subprocess execution with an atomic admission scheduler (#16874)
* perf(git): bound git subprocess execution with an atomic admission scheduler

Field traces (#16038, #11363) show Windows freeze storms driven by unbounded
concurrent git children (12+ at once, 50-65s status convoys for 25+ minutes).
Admit every main-process git child against atomic per-budget base+headroom
counters (general / network / per-route), with reserved interactive capacity,
ordering-only aging, close-bound permit release, a 120s fail-safe read timeout
that feeds scheduler backoff, tier plumbing through every option carrier, and
coalesced+jittered visibility pollers. Killswitch: ORCA_GIT_ADMISSION_DISABLED=1.

Storm harness A/B: max concurrent children 65 -> 6, interactive p95 791ms -> 88ms;
output-parity battery byte-identical with admission on vs off.

* test(git): run the admission output-parity battery on every platform

Parity needs real git, not the storm harness's PATH stub, so it must not share
that file's POSIX gate - Windows is the platform where parity evidence matters.

* fix(git): preserve interactive admission invariants

* perf(git): keep admission queue drains linear

* fix(git): close final admission gaps

* perf(git): bound eligible route selection

* fix(merge): remove unrelated stale snapshot changes

* fix(git): preserve refresh lifecycle authority

* test(git): align admission lifetime contracts

* fix(git): harden admission across runtime paths

* fix(git): restore freshness for bulk status reads

* test(git): repoint delete-dialog source pins after admission plumbing

The hydration effect now orders its targets through
orderDeleteWorktreeStatusHydrationTargets and passes includeLineStats
alongside the abort signal, so both literal anchors stopped matching.
The invariants are unchanged and still pinned: dropping the signal, the
main-worktree/folder filter, or getState-instead-of-subscribe each
still reddens this test.

* Fix git admission tier propagation and lock ordering

Decode optional Git status tiers permissively and default runtime RPC status reads to the status lane while preserving renderer caller intent.

Acquire the FETCH_HEAD mutex before atomic admission so same-repository fetch waiters hold no global or route permits.

Preserve automatic pull-request refresh reasons, keep explicit hosted-review refreshes interactive, remove the dead candidate tier, and keep relay scheduling unchanged.

Use tier-aware status lease keys because a shared lease cannot be safely promoted after its admission request is queued or granted.

* test: align expectations with admission plumbing

* refactor(child-process): move the process contract types to process-spec

run-process.ts crossed its line cap after gaining the termination observer;
the public types and defaults move out with re-exports so no caller changes.

* chore: restore pnpm-lock.yaml to main (unintended local drift)

---------

Co-authored-by: Merge Sim <sim@local>
2026-08-30 14:19:05 -07:00
Neil 81b4d88bd6 refactor(mobile): split tasks route into focused modules (#17438)
* refactor(mobile): split tasks route into focused modules

* fix(mobile): repair tasks refactor module boundaries

* chore(mobile): document intentional render resets

* fix(mobile): remove stale lint suppressions from tasks split

* test(mobile): keep parity checks stable with doctor suppressions

* test(mobile): follow tasks module split

* test(mobile): follow project routing module split
2026-08-30 14:10:36 -07:00
Brennan BensonandMerge Sim 8ce3fd8b32 fix(native-chat): keep agent responses selectable (#17437)
Co-authored-by: Merge Sim <sim@local>
2026-08-30 14:07:06 -07:00
Brennan Benson ba5f33402f fix(relay): reap owned PTYs when the daemon dies on an uncaught exception (STA-5697) (#16894)
* fix(relay): reap PTY jobs on fatal exit (STA-5697)

* test(relay): cover the POSIX fatal reap and make a failed reap observable

The fatal reap had no POSIX coverage at all -- every case forced win32 -- and
the daemon discarded the rethrown reap error in an empty catch, so a reap that
failed on a remote host left no trace in the only log a crash produces.

Collapse the job-terminated branch onto the forceKillSent flag it already sets:
the flag is what suppresses the redundant signal, so the separate "continue"
was a second expression of one intent, and the two could only be caught
together -- reverting either one alone left the suite green.
2026-08-30 12:36:23 -07:00
Brennan BensonandMerge Sim c3aceacc7b Fix PR unlink for auto-detected reviews (#16898)
* fix: make PR unlink hide auto-detected reviews

* Type the empty-content test double against the real model

The literal narrowed suppressedGitHubPR to number and typed the callback
as Mock, so neither direction was comparable and tsconfig.tc.web.json
failed on TS2352. Keeping the 'as' cast preserves checking of the fields
the double does supply.

* Add localization keys for the unlinked checks-panel state

The unlinked title, relink action, and the remote-runtime upgrade notice
introduced untranslated keys that static analysis requires in en.json.

* Advertise PR suppression capability in the transport test

The client capability list is pinned by websocket-transport.test.ts, and
adding WORKTREE_GITHUB_PR_SUPPRESSION left the expected list stale.

* Fix stale PR suppression in Checks

* fix: harden PR unlink suppression state

* refactor: extract PR unlink state handling

* fix: show PR relink recovery in source control

* fix: add unlinked PR localization

* Clarify workspace-scoped PR unlinking

---------

Co-authored-by: Merge Sim <sim@local>
2026-08-30 12:24:51 -07:00
Brennan BensonandMerge Sim c539b38856 Fix select all in native chat composer (#17294)
Co-authored-by: Merge Sim <sim@local>
2026-08-30 12:20:51 -07:00
Neil 3ab9766e38 perf(worktree): prepare checkouts while the composer is open
Squashed merge of PR #17290.
2026-08-30 12:12:04 -07:00
Neil a8183884bd perf(wsl): place worktrees inside the distro when the project runs in WSL
Fix-forward for readiness review: align retirement placement with WSL mirrors and preserve Windows-side git-common watchers.
2026-08-30 12:11:21 -07:00
Neil b81e578cff fix(updater): accept GitHub release asset redirects on Windows
Accept manual GitHub release-asset redirects on Windows, preserve non-Windows probing, and cover redirect/error/timeout paths.
2026-08-30 12:10:11 -07:00
Neil 16e6b103d6 fix(session): deduplicate editor records during restore (#17370)
Deduplicate persisted editor records and repair tab-group references during session hydration. Closes #17185.
2026-08-30 12:07:41 -07:00
Neil ac02232015 perf: overlap independent worktree create preflight (#17386)
Readiness checklist passed; required CI and review checks are green.
2026-08-30 11:50:29 -07:00
Neil fd52e942bd fix(tasks): keep the remembered GitHub scroll offset instead of clobbering it (STA-5949) (#17433) 2026-08-30 11:49:12 -07:00
Neil 5e19c35dc5 fix(automations): stop vetoing Escape for overlays the page does not own (STA-5207) (#17431) 2026-08-30 11:49:00 -07:00
Neil 6677ae4e5e test: correct 8 stale specs surfaced by the test-detected-bugs sweep (#17434) 2026-08-30 11:45:21 -07:00
Jinjing 89a4d67705 Revert waiting for setup before agent startup (#17418) 2026-08-30 09:33:09 -07:00
Neil d607a63670 fix(native-chat): replay a draft clear dropped mid-composition (#17392)
A structured send clears the draft asynchronously, on RPC acceptance. If the
user opens the next IME composition first, the clear lands while the browser
owns the field, the DOM sync skips it, and settlement adopts element.value —
which still holds the message that was already sent.

The field now records a clear dropped mid-composition and applies it at
settlement, keeping only what the IME composed on top of the value the field
held when the composition started. Browser ownership is unchanged for every
other programmatic draft; the clear stays on the acceptance path, so a
rejected send still keeps its draft.

Also advances a frame before the attachment-flush focus assertions, which
were vacuous because the focus they forbid is scheduled in rAF.

Fixes #17359
2026-08-30 03:55:37 -07:00
Neil 7612ee6c09 chore(deps): pin the pdfjs-dist release-age exception to 6.3.289 (#17389)
A bare package name exempts every future version of pdfjs-dist from the
release-age gate, including one published minutes ago. Scope it to the single
version that needs it.
2026-08-30 03:31:49 -07:00
Neil 93a080112f fix(linear): stop the filter coverage warning firing on complete selections (#17376)
* fix(linear): stop the filter coverage warning firing on complete selections

#17342 inferred transport-cap truncation from the bounded filter after the
fact, with `atLimit = selectedIds.length >= max`. A row the cap could not fit
leaves no trace in the surviving ids, so at exactly the cap a complete
selection and a trimmed one are indistinguishable from the value alone, and
the inference biased toward always warning. A workspace with 20 teams x 5
status names expands to exactly 100 ids: picking all 5 rows is provably
untruncated, yet the menu read "100 selected · partial".

Record the trim where it happens instead. `applyPickedFilter` holds both the
pre-cap expansion and the bounded result, so it stores the surviving ids as a
truncation record; the notice, section-menu summary, and pill consume that
flag. Keying the record on the ids it describes is what keeps it fresh — the
moment the facet carries anything else (row toggle, pill clear, Clear all, the
prune effect, a workspace switch) the record no longer matches and the warning
goes away, which matters because the prune effect only ever removes ids.
`intended > applied` stays as the fallback for restored filters that carry no
record, and `boundLinearIssueAttributeFilter` is still the last word on the
cap.

Also moves the section-menu partial marker out of the `max-w-[120px] truncate`
summary span, where "100 selected · partial" could clip.

Refs STA-5996

* test(linear): cover the untouched-facet truncation guard

The guard that keeps a recorded trim alive across an unrelated facet click was
untested — the first attempt sat where intended > applied, so the value-derived
shortfall answered for it and removing the guard left every test green. Move the
scenario onto the cap, where only the record can speak.

Also stop an empty record matching an empty facet: a filter carrying nothing is
never truncated.

* test(linear): pin set equality, not subset, on a truncation record

A facet that grew past its record has refetched underneath it; matching by
subset would keep warning about a trim that no longer describes the filter.
Found by mutation: the subset mutant survived the whole suite.

* test(linear): fuzz that the coverage pill and the section notice agree

They are the same claim rendered twice; a pill reading partial over a silent
section is a lie either way round. 20k random topologies, zero divergence.
2026-08-30 03:30:41 -07:00
Neil 70df6f0224 fix(terminal): mask the agent composer's dim placeholder during a preedit (#17377)
Split out of #17170, which now carries only the xterm composition-overlay work.

Codex and Claude draw an all-dim, full-row ghost placeholder. The opaque preedit
overlay reproduces the committed row tail it covers, so without this the ghost is
repeated to the right of the composing syllable instead of staying masked. The
binding keys off the `.xterm-composition-remainder` class that #17170 adds and
hides it through CSS while a composition owns a structurally verified placeholder
row — bold prompt glyph plus a dimmed model footer below a blank gap for Codex, a
frame line above the prompt for Claude. Arbitrary dim output, shell lookalikes,
and any row carrying typed text keep their tail visible.

readTerminalCursorLineContext moves from src/main/daemon to src/shared because the
renderer now needs the same reader the daemon uses; the move is import-only.

Depends on #17170.
2026-08-30 03:12:22 -07:00
Neil 3d0bd6a3ec chore(deps): restore pdfjs-dist release-age exception until it ages out (#17385)
#17372 dropped the exclusion two days early: pdfjs-dist@6.3.289 was published
2026-08-29T12:48Z and does not clear the 4320-minute gate until 2026-09-01T12:48Z,
so every pnpm install in CI fails lockfile verification.
2026-08-30 02:59:49 -07:00
Neil 7f822a73e3 fix(terminal): render the IME caret and give the candidate anchor one owner (#17170)
* fix(terminal): render IME caret without placeholder overlap

* fix(terminal): preserve dim mid-line composition tails

* fix(terminal): keep IME caret visible at row edge

* fix(terminal): harden IME overlay lifecycle and layout

* test(terminal): type final-cell layout mock

* fix(terminal): keep final-cell IME anchor on-screen

* fix(terminal): bind IME masking to composer ownership

* fix(terminal): bound IME placeholder session ownership

* fix(terminal): track latest IME placeholder session

* test(terminal): share IME session event fixture

* fix(terminal): keep both writers of the IME candidate anchor in agreement

`textarea.style.left` has two writers: xterm's patched CompositionHelper and
Orca's terminal-ime-candidate-anchor.ts. The anchor module listens on
terminal.element, so within a composition event it writes after xterm's textarea
listener and reverted the final-column clamp the patch had just applied.

Moving the clamp into the anchor module and dropping the patch hunk does not fix
it, and the rendered e2e caught that: CoreBrowserTerminal.ts:444 drives
updateCompositionElements from onRender as well, so xterm re-asserts the textarea
position on every repaint, with no composition event for that module to hear. The
anchor survived only when no render happened to follow — measured as a flake at the
final column, 1561.28px against a 1557px screen edge, the fully unclamped value.

So both writers now compute the same clamp. The patch keeps it, because it is the
writer on the render path and already holds cursorLeft, maxWidth and the preedit
bounds. The anchor module applies the same one, so its composition-event write no
longer reverts the correction in the window before the next render. Both halves are
individually necessary and both are mutation-tested.

Also restores _getRowRemainderText's expression from main: translateToString(true,
x, line.length) and translateToString(false, x, getTrimmedLength()) are the same
call, since upstream does endCol = min(endCol, getTrimmedLength()) under trimRight.

Adds the two missing tests — one installing both anchor writers in a single rig, one
driving a render under an open composition — plus disposal cleanup and clamp-bound
coverage, and moves the Codex/Claude placeholder mask to a follow-up PR.
2026-08-30 02:23:04 -07:00
Neil 040225cf94 chore(deps): remove pdfjs-dist release exception (#17372) 2026-08-30 02:19:43 -07:00
Neil fa230cee42 refactor(preflight): rename execLocalPreflightCommand to ...OrThrow (#17380)
Sibling of execCommandInWslOrThrow (#17375) with the identical throwing
contract, sitting in the same `try { ... } catch { return false }`
blocks. After that rename the pair read inconsistently — one announced
that it throws, the other did not, while both collapse to a silent
false at the call site.

Also states the contract in a doc comment: it rejects rather than
reporting "absent", so a caller that swallows the rejection makes "not
installed" and "could not run it" the same answer.
2026-08-30 02:09:05 -07:00
Jinwoo Hong d64097d109 Open linked reviews in Orca browser (#17360)
* feat(sidebar): open linked reviews in Orca browser

* test(e2e): match paired window reveal assertion

* feat(sidebar): focus linked browser tabs

* ci: retry checks after cancelled rerun
2026-08-30 04:58:56 -04:00
Neil 99b59064fd docs(ime): codify desktop composition regression checks (#17172)
* docs(ime): codify desktop composition regression checks

* docs(ime): define remote verification verdicts

* docs(ime): narrow placeholder masking invariant

* docs(ime): require final-cell caret containment

* docs(ime): cover async attachment settlement

* docs(ime): correct semantic placeholder contract

* docs(ime): record bounded ownership contracts
2026-08-30 01:58:35 -07:00
Neil 7b467bd0a6 ci: gate PRs on a real input method, and prove the lane engaged one (#17365)
* ci: gate PRs on a real input method, and prove the lane engaged one

No job on the PR gate has ever run a real input method. pr.yml and e2e.yml are
ubuntu-latest with CDP `Input.imeSetComposition`, which is a synthetic
composition; the only job that drives ibus-hangul through xdotool is
terminal-ime-e2e.yml, and it is schedule + dispatch only. A PR could turn the
real-IME path red and merge green.

Route IME source to that lane from pr.yml through the existing
pr-e2e-source-routing mechanism, so it runs on IME-touching PRs and nothing
else. The lane stays out of verify.needs — advisory, like `e2e` — because its
reliability is known only from nightly main runs. Deliberately no
continue-on-error: that reports green and hides the signal.

The harness fails open in ways that all look like success: Playwright reports a
skipped test as a pass, so an unset ORCA_E2E_NATIVE_IBUS_HANGUL, a renamed test,
or a session with no engine all exit 0 having exercised nothing. The specs now
append an engagement receipt only after observing real composition events, and
the runner requires one per expected test before the lane may report success.

Also drop the native spec from changed-e2e: it was already routed there by its
own filename, where it self-skips for want of an ibus session and reported that
skip as coverage.

* ci: let the real-IME step report even when the synthetic step failed
2026-08-30 01:58:32 -07:00
Neil 1215cc98f2 refactor(preflight): rename execCommandInWsl to execCommandInWslOrThrow (#17375)
Pure rename, no behavior change. Callers like isCommandAvailable and
isCommandOnPath wrap this in try { ... return true } catch { return false },
collapsing "distro unreachable" and "command absent" into the same value —
a recurring bug class in this subsystem (see
docs/reference/wsl-probe-failure-semantics.md). The OrThrow suffix makes
that swallow visible at the call site instead of implicit in the function
name, so a reviewer notices when a new caller does the same collapse.
2026-08-30 01:53:47 -07:00
Brennan BensonandJinwoo-H d9870c6c75 fix(browser): apply the app-wide HTTP proxy to embedded browser sessions (#15536)
* fix(browser): apply the app-wide HTTP proxy to embedded browser sessions

The proxy setting was only ever written to `session.defaultSession`, but browser
guests run on their own `persist:orca-*` partitions. Any host reachable only via
the configured proxy failed to load in an embedded tab, landing on
`chrome-error://chromewebdata/`, while the same setting worked everywhere else.

Adds a per-session applier alongside the existing defaultSession path, keyed by a
WeakMap so one session's applied config can't suppress another's, and applies it
to every browser partition through the single installer they all pass through.
Startup awaits an explicit sweep so the first guest navigation can't race the
installer's fire-and-forget write, and a settings change re-sweeps so toggling
the proxy takes effect without a restart.

Env-var fallback and the system-proxy probe mirror the defaultSession behaviour,
so a browser partition resolves the proxy the same way the rest of the app does.

Fixes STA-4779

* fix(browser): await per-session proxy readiness

* fix(proxy): preserve loopback and authenticate

* fix(proxy): settle browser partition update races

* fix(proxy): close partition policy races

* fix(proxy): order settings and release removed sessions

* test(browser): await partition proxy readiness

* fix(proxy): cancel removed partition retries

* refactor(proxy): keep OpenCode rate limits out of scope

* fix(proxy): preserve sessionless host policy

* fix(proxy): gate requests on policy readiness

* fix(proxy): retire deleted browser sessions

* fix(proxy): close retired browser guests

* fix(proxy): retain retired session guards

* fix(proxy): retain retired partition policies

* fix(browser): retry transient proxy application failures

* fix(browser): release deleted partition installer state

* fix(proxy): retry delayed transient failures

* fix(proxy): preserve route session authority after rebase

* fix(proxy): clear retired session credentials

* fix(proxy): retire failed browser profiles

* fix(proxy): harden failed session cleanup

---------

Co-authored-by: Jinwoo-H <jinwoo0825@gmail.com>
2026-08-30 04:53:20 -04:00
e741ff1318 fix(wsl): scan agent sessions only in running distros (#17072)
* fix(wsl): scan sessions only in running distros

* test(ai-vault): pin WSL discovery platform

* fix(wsl): suspend transcript watchers for stopped distros

* test(wsl): pin transcript scan gate platform

* fix(wsl): settle stopped transcript loading

* fix(wsl): add last-known-good fallback and backoff to running-distro discovery

listRunningWslDistrosAsync failed closed on any probe error (timeout, ENOENT,
wsl.exe hiccup), indistinguishable from "no distros running". A 2s poll
(wsl-transcript-running-observer.ts) calls it indefinitely while any WSL
transcript tab is open, so a persistently broken wsl.exe silently made every
WSL session vanish app-wide with no way to tell "discovery broken" from
"distro stopped", and re-spawned wsl.exe every 2s forever.

Extract a dedicated cache/backoff module (wsl-running-distro-cache.ts,
mirroring the sibling machinery already in wsl.ts for the full distro list)
so a probe failure falls back to the last-known-good running-distro list and
backs off further probes, while a genuine empty result (no distros running)
stays authoritative. Add a consumer-level test simulating a sustained wsl.exe
outage across a live transcript-watcher polling session, asserting the
observer keeps reporting "running" and that real wsl.exe spawns stay bounded.

* fix(build): list the new WSL cache module in the web typecheck project

config/tsconfig.tc.web.json enumerates its files explicitly, so a new
module imported by wsl.ts fails the full typecheck with TS6307 until it
is listed. pnpm tc:node passes without it, which is how this got missed.

  src/main/wsl.ts(13,8): error TS6307: File 'src/main/wsl-running-distro-cache.ts'
  is not listed within the file list of project 'config/tsconfig.tc.web.json'.

---------

Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>
2026-08-30 01:49:05 -07:00
Neil 58a52a8ce3 test(wsl): guard probes that report failure as a negative answer (#17352)
* test(wsl): guard probes that report failure as a negative answer

A WSL probe that cannot reach its distro returns the same value as one
that asked and got "no". Downstream nothing can tell them apart, so a
distro that was busy for a second reports no git, or no agent sessions,
until relaunch — sticky, silent, and identical to the real thing.

That has shipped three times: preflight CLI probes, the glab auth
fallback (#8941), and listRunningWslDistrosAsync failing closed with no
last-known-good while polled every 2s (PR #17072).

Scan the WSL and preflight probe modules for the shape and hold the
current set in an allowlist that only shrinks. Scoped deliberately: the
same shape appears ~850 times across src/ and is usually correct,
because for most callers a failure really does mean absent. It is only
dangerous where the answer describes a distro.

The guard cannot see the dangerous part — whether the value is later
cached or gates discovery is dataflow, not syntax. It stops a new
swallow site appearing here without someone saying why it is safe to
pin, which is the review that was missing all three times.

* test(wsl): make ratchet failures actionable

A red build must say what to do. Name the offending files, say the
allowlist is where a safe case goes, and — for a stale entry — say the
change is fine and the list just needs to shrink.

* docs(wsl): track the probe failure-semantics reference

docs/** is gitignored with an explicit allowlist, so the reference the
ratchet points contributors to was silently left out of the branch. A
guard whose error message cites a doc that is not in the repo is worse
than no doc.

* test(wsl): catch a swallow whose reason trails the return

The guard only tolerated comments before `return`, so
`return false // ...` slipped past — including the exact snippet the
doc and the test's own docstring use as the canonical example. The doc
asks authors to write down why a swallow is safe, and the natural place
for that sentence is trailing the return, so following the guidance
defeated the guard.

Verified against both shapes: trailing comment and comment on the line
after.
2026-08-30 01:30:25 -07:00
Neil d9cb020178 fix(diagnostics): count case-only worktree nesting on macOS (#17364) 2026-08-30 01:23:43 -07:00
Neil 3af2c665c0 fix(cli): name PowerShell when it strips quotes from JSON flags (#17351)
* fix(cli): name PowerShell when it strips quotes from JSON flags

Windows PowerShell 5.1 does not escape inner quotes when building a native
command line, so `--options '["a","b"]'` reaches orca.exe as `--options [a,b]`.
The value is correct when printed and damaged by the time argv is parsed, so the
resulting "invalid JSON" error blamed the user's input rather than the shell.

#16743 recovered this for `--deps`, which is safe only because generated task IDs
have a fixed 12-hex grammar. The same mangling hits `--options`, `--payload` and
`--result`, and those are NOT safely recoverable: `["1","2"]` and `[1,2]` arrive
at argv identically, so a general repair would silently turn strings into numbers.

Detect instead. `getOptionalJsonFlag` rejects the damaged shape up front with an
error that names the shell and shows the workaround. It fires only when the value
is bracketed, quote-free, fails JSON.parse, AND consists entirely of bare tokens
that quoting would rescue, so valid JSON is untouched.

Also share the generated-id contract: `task-deps-flag` hardcoded
/^task_[0-9a-f]{12}$/i, which silently diverges if `generateId`'s byte count
changes. It now calls `isGeneratedId`, with a test pinning the two together.

Verified on a Windows host. Measured argv, which the new test pins as a fixture:
  PS_VALUE=["task_b2a580db74d8","task_c3b691ec85e9"]
  ARGV=["--deps","[task_b2a580db74d8,task_c3b691ec85e9]"]

Before: Invalid --options: must be a JSON array of strings
After:  --options arrived as [a,b], which is not valid JSON.
        Windows PowerShell 5.1 strips the inner quotes ...

* fix(cli): scope JSON-flag detection to genuinely JSON flags

Review found the detector wired to two flags that are not JSON:

- `orchestration ask --options` is documented `<csv>` and the runtime splits it
  on commas, so `--options [a,b]` was a legitimate value being rejected.
- `task-update --result` is stored verbatim and reused as dispatch failure text;
  existing tests pass free text, so a bracketed `[ok]` was being rejected.

Both revert to `getOptionalStringFlag`. Only `gate-create --options`
(`<json_array>`) and `send --payload` (`<json>`) are JSON-parsed and keep it.

Three further review fixes:

- Objects now require a `key:value` pair per entry. `{a,b}` and `{a:b,c}` were
  reported as quote-stripped although quoting them cannot produce valid JSON.
- The raw value is no longer echoed. A `--payload` can carry secrets and this
  message reaches `--json` output; the flag name and guidance are enough.
- The message hedges the shell attribution. Detection inspects only the value's
  shape, so it also fires when a macOS/Linux user forgets to quote, where
  PowerShell is not involved.

Verified against a Windows host, all six cases: both JSON flags fire on the
mangled shape and pass valid JSON through to the runtime; both non-JSON flags
now reach the runtime again; and the secret in `{token:hunter2}` appears zero
times in the error output.
2026-08-30 01:23:27 -07:00