* refactor: split pty-connection.ts under 400 lines
* rm design doc
* refactor(pty-connection): extract reattach payload handlers as factories
- Replace bindApplyReattachPayload with createReattachPayloadHandlers factory that returns handlers instead of mutating session directly, enabling better composability and testing
- Extract waitForUserInitiatedSshConnect as standalone function for reuse across deferred session attach flows
- Create ReattachPayloadSession type to document and isolate required session capabilities
- Add test coverage for overlapping reattach payload attempts
- Clean up comments to remove redundant prefixes (session.pane → pane, session.transport → transport)
* fix(pty-connection): correct sequencing and state bugs in spawn and reat
- Fix terminal tail slice to take prefix instead of suffix, preserving escape
sequence markers needed by next scan
- Clear pending pane serializer when direct SSH retry PTY is unclaimed
- Initialize interrupt status baseline to undefined so first input advances
sequence counter
- Bump reattach generation only after confirming current attempt owns the stream,
preventing superseded results from canceling in-flight prepaint
* fix(pty-connection): correct sequencing and state bugs in spawn and reat
- Fix terminal tail slice to take prefix instead of suffix, preserving escape
sequence markers needed by next scan
- Clear pending pane serializer when direct SSH retry PTY is unclaimed
- Initialize interrupt status baseline to undefined so first input advances
sequence counter
- Bump reattach generation only after confirming current attempt owns the stream,
preventing superseded results from canceling in-flight prepaint
* fix(test): increase poll iterations to prevent Node 26 test leakage
Increase event loop turns from 40 to 200 in the timer settlement loop.
Node 26's libuv poll phase can briefly starve when concurrent workers
transform tests, causing cleanup to leak into the next test. The higher
iteration count ensures async operations complete before returning.
* fix(foreground-output-budgets): use >= for budget window boundary check
At the exact window boundary, the budget should roll over. Change the
comparison from > to >= so the window resets when now equals
windowStart + FOREGROUND_BUDGET_WINDOW_MS, not just after. Add tests
to verify budget rejection and rollover behavior.
* refactor(pty-connection): add status observations and routing improvemen
- Track agent status observations with origin and transition metadata
- Separate interactive redraw input timing from general terminal input
- Restore pane authority on bind and reattach
- Refine routing trust and confirmation state handling
- Invoke queued startup callbacks when PTY is bound
- Resolve Windows shell overrides with user settings
* refactor: extract resolveLaunchAgentCandidate helper
Consolidate duplicated launch-agent resolution logic into a shared helper to prevent future divergence between paneExpectsLaunchAgent and resolveExpectedLaunchTuiAgent.
* refactor(pty-connection): use model snapshot for direct SSH reconnects
Direct SSH reconnects now restore from the full SSH model snapshot (complete scrollback) when dimensions are compatible, instead of the bounded relay tail. Falls back gracefully when incompatible or alternate-screen was exited.
* refactor(pty): retry unverifiable SSH reattaches via preserved bindings
Preserve deferred SSH session IDs longer when they serve as the only retry binding,
allowing the system to attempt recovery through direct SSH retries or PTY remounts
when reattach fails in an unverifiable way. Simplify reconnect model restoration
by removing the conditional model snapshot probe and using relay replay directly.
* test: poll terminal readiness in expectSingleOwningPty
Retry the terminal list assertion with polling to account for timing
delays in PTY state reporting from the runtime.
* fix(status-bar): remove pet menu reserved space
* test(status-bar): add pet segment layout validation tests
- Unit test guards against pr-[6.5rem] padding reintroduction
- E2E test measures trailing overhang instead of total width delta
for more accurate layout validation
- Extract enableExperimentalPet helper for test clarity
---------
Co-authored-by: m4air <m4air@m4airs-Air.localdomain>
Three bugs in one screen, reported in #15256 with a diff of the user's
orca-data.json showing defaultTuiAgent going from "claude" to null.
1. The Auto pill's handler writes null, and it was rendered as the ACTIVE
choice whenever the stored agent was merely not detected right now. So the
pill that already looked selected was destructive: one click erased the
setting, and a later successful detection did not bring it back. Auto is now
active only when null is actually stored. Detection is a transient fact; the
stored value is not, and this control reports the stored value.
2. With zero agents detected there were no agent pills at all, so the stored
choice was both invisible and unrecoverable -- nothing to click to put it
back. The stored agent is now always offered, labelled as saved but not
currently detected.
3. Refresh lived inside the Installed section, which only renders when at least
one agent was found, so the only retry control vanished in exactly the state
that needs it. An empty result now renders its own Refresh.
This matters more now than when it was filed: #16028 makes WSL detection
legitimately return an empty set on machines where the only agent was a Windows
binary reached through interop, so the empty path is about to get more traffic.
Each of the three tests was verified to fail with its own fix reverted.
Opening a remote HTML preview to the side created an empty split and made it
the active group. The next host session-tab snapshot still had the terminal
active, so the client treated that empty group as a terminal focus change.
Do not activate the empty split for unfocused remote previews, and when a
reserved preview group is still empty, keep the sibling editor as the
visible tab instead of following the host terminal.
* fix(pr-page): route remaining user avatars through GitHubUserAvatar
On a private-mode GitHub Enterprise instance the stored avatar URL 302s to
/login, and the renderer's default Electron session carries no cookie, so the
image never loads. #8784 added GitHubUserAvatar for exactly this — it degrades
to an initials placeholder via onError — but three call sites in
PullRequestPage kept a bare <img>: the reviewer picker, the comment author,
and the @ mention suggestions.
Each only guarded on avatarUrl being absent, so on GHE the URL is present, the
placeholder branch never runs, and a broken image is left on screen. The
authorAvatarUrl type comment already documents the intended contract ("falls
back to the login URL and finally an initials placeholder").
GitHubUserAvatar was already imported in this file for the PR author, so this
makes all five avatars in the page consistent. Note the three switched slots
now carry the shared border/bg styling, matching the two that already did.
Add a boundary test that fails if any avatar is rendered through a bare <img>
again.
Fixes#13976
* fix(task-page): route GitHub avatar cells through GitHubUserAvatar too
Auditing the rest of the GHE avatar path turned up the same bare <img> in
TaskPage: GitHubAssigneeAvatar, GHAssigneesCell and PRReviewCell. Fixing only
the PR page would leave half of #13976 in place.
GitHubAssigneeAvatar is the clearest case — ReviewChipAvatar directly above it
already renders through GitHubUserAvatar, so two adjacent functions disagreed
on how a GitHub user avatar is drawn. Its border also moves from
border-border/40 to /50, matching the neighbour.
Linear member avatars in this file are left alone; they use their own provider
path and are out of scope here.
Move the regression assertions into the existing repro-8784 file rather than a
new boundary test — that file already guards PullRequestPage and TaskPage
together, so it is where this belongs. The PR-page check matches the <img>
pattern instead of specific field names, so a rename or a newly added avatar
slot cannot slip past it; the TaskPage check is scoped per function to avoid
catching the Linear cells.
* test(github): scope the avatar guard per call site and cover TaskPage names
Addresses review feedback on the regression guard.
The field-name regex missed aliases and resolver expressions, and the PR-page
assertion did not require GitHubUserAvatar in each migrated slot — deleting all
three would have passed. Reject any bare <img> within the component scope
instead, which is safe now that every assertion is scoped to one function.
Drive all six slots from one table so each gets its own named case, and extend
the display-name contract to TaskPage, which previously went unchecked. The
ConversationTab entry carries displayName: null because PRComment has no
display-name field.
Reverting the fix now fails 11 cases instead of 3.
The restored-snapshot baseline permanently drops every delivery chunk at or
below the snapshot's seq, on the model's claim that those bytes are already
painted. reconcileChunkAgainstRestoredSnapshot recovers when the baseline
under-reports (a gap ahead re-restores; a rawLength mismatch re-restores) but
has no path for a baseline that over-reports: those chunks return
drop-duplicate forever, and an idle shell never re-sends them.
Gate arming on whether the snapshot painted printable cells. A snapshot
claiming seq > 0 while painting nothing cannot be the rendering of the output
it claims to cover, so the claim is disproven and the redelivery is the only
remaining copy.
Also pins disposeHeadlessTerminal's two-part write ordering, which was
previously unpinned and silently reversible.
Refs STA-5179
* fix(composer): close the Create Workspace dialog on the first Escape
The modal copied the page-level "Esc blurs the focused field, then closes"
rule from TaskPage/Automations. On a page that rule protects a focus the
user chose; this dialog auto-focuses the name input on open, so its
capture-phase handler preventDefault'd every first Escape (which also
suppressed Radix's dismissal, since DismissableLayer skips a
defaultPrevented event) and the dialog could only be closed with two
presses.
Drop the Escape branch and let the dialog's dismissable layer own it.
Radix dismisses only the topmost layer, so nested popovers, selects and
dialogs still consume their own Escape first.
* test(e2e): pin the composer's auto-focus as the reason one Escape must close it
* refactor(wsl): delete the environment-policy layer the reviews kept failing on
A design council (Opus, Grok, GPT-5.6-Sol) reviewed the merged runner after it
took eleven review rounds to land. All three reached the same conclusion: the
invocation half is sound, the environment/probe half is not, and every round had
been debugging the second one.
The finding that settled it, from Opus: `environmentResolved` had **54
references, all in tests and the runner itself. Not one production reader.** The
safety mechanism the strict default existed for was never wired to anything, so
all 19 degrading sites reported absence with full confidence anyway -- #9725
live at every one, under comments claiming it was handled. Two of those comments
say so out loud; I wrote them.
Root cause, in one line: every knob existed only because a failed probe was
fatal. So it no longer is.
- `allowDegradedEnvironment` and `WslGuestEnvironmentUnavailableError` are gone.
A missing login PATH is a fact in the result, not an exception. That deletes
23 opt-outs, six catch-and-remap blocks, the transient/rejected cooldown
split, `probedWithBudget`, and the 1.5x re-probe heuristic -- none of which
had a reason to exist once the case stopped throwing.
- `lane` + `allowDegradedEnvironment` collapse into `loginPath: 'none' |
'preferred'`. 19 of 23 sites passed the opt-out, and two said in comments that
they did not want the login PATH at all: the flag had become the `'none'` the
union was missing.
- The `interactive` lane is deleted. It had zero production callers and kept ~30
lines of fence plumbing alive for tests only.
Net -98 production lines; the runner itself sheds 86 for 38.
Also carries three fixes from the W3 orphan-PR sweep I had not done:
- `WSL_UTF8=1` in the runner. My relay migration deleted the only place setting
it, so wsl.exe's own error text arrived UTF-16LE and read as NUL-riddled.
A regression I introduced. Credit: #9010 (Chang-Jin-Lee).
- `GITLAB_HOST` is now named in WSLENV, so a ported self-hosted host actually
crosses into a distro-routed glab (#12557). Credit: #12558 (makoto-developer).
- The WSL skill-setup command pipes into `sh` instead of `eval "$(...)"`, whose
nested quoting produced `word unexpected (expecting "in")` (#14292). Credit:
#14785 (innocarpe).
* fix(wsl): restore the login PATH for the Codex availability lookup
loginPath:'none' on a PATH lookup reports an nvm-installed codex as absent,
which is #9725. A miss without a resolved environment is now 'could not
check', not 'not installed'.
Also hardens the guards that should have caught it:
- bashism ratchet is per-call, not per-file, and fails closed on lexer desync
- blankStringContents handles regex literals (an apostrophe in /'/g desynced
the lexer, so the scan silently found zero calls)
- windowsHide allowlist 85 -> 80, stale once the lexer parsed those files
Credit: Grok (P0), GPT-Sol (ratchet gaps).
* test(wsl): close the two ratchet gaps that let planted spawns pass
- variable-indirected wsl.exe (`const b = 'wsl.exe'; spawnProcess(b)`) is now
tracked, so the 5 files recorded only in a comment become real allowlist
entries. Three actually spawn that way; the other two never spawned wsl.exe
at all, so the prose record was wrong by three in the hiding direction.
- promisify(renamedAlias) is now resolved, so `const run = promisify(execFile)`
behind an `execFile as x` import can no longer skip windowsHide.
Each verified by planting the violation, watching it fail, restoring, watching
it pass. Credit: GPT-Sol.
* fix(source-scan): stop the regex-literal reader from eating block comments
At index 0 there is no preceding token, so a file opening with a banner
comment had its `/*` read as a pattern and swallowed to the next slash --
110k characters of preload/index.ts, in the direction that hides offenders.
Measured across the tree, old lexer vs new: worst-case over-blanking drops
from -110564 to -1116 characters, and files that desync drop from 51 to 22.
The remaining extra blanking is regex interiors, which is the intent.
Regression tests for both lexer bugs, each verified to fail with its fix
reverted. The first draft of the comment test did not bind -- it asserted on
text after the swallowed span.
* fix(wsl): restore the unverifiable signal on the two remaining probe sites
Round 2. Three call sites used to throw when the login-PATH probe failed;
the redesign rewired one (Codex) and left two reporting confident absence.
- skill-wsl-provider-detection: the script ends in `|| true`, so a lookup
without the login PATH exits 0 with empty stdout -- identical to 'nothing
installed'. Callers skip the ~/.codex and ~/.claude skill roots on an empty
list, losing an nvm-installed provider's skills.
- wsl-cli-installer: the dead catch is replaced by an explicit check. Its
`case ":$PATH:"` probe otherwise answers from the distro default PATH and
Settings states as fact that the CLI is not on PATH. Timeout is checked
first, since a timed-out run also leaves the environment unresolved.
Also narrows the regex-literal prev-token set. '!', '+', '-', '>' and '}' are
value terminators as often as operators, so postfix `n-- / 2` and JSX
`<A size={14} /> : <B` were read as patterns and their spans blanked -- 13
live JSX spans, and one swallowed execFile call that left no desync behind.
False negatives only risk a desync, and desync fails closed.
Plus: WSL_UTF8 on the probe spawn (#9010 reached the runner, not the probe),
and the allowlist header I shuffled by sorting comments along with entries.
Credit: Grok (both P1s), Opus (lexer false positives).
* docs(wsl): drop the lane comments the redesign made false
The interactive lane is gone, so 'both lanes' and the fenced-stdout note
described code that no longer exists. Also states plainly that
environmentResolved is always true under loginPath:'none' -- the field cannot
rescue a PATH lookup that was mislabelled, which is how #9725 came back.
Credit: Grok.
* fix(wsl): stop piping user scripts into the shell's stdin
The W3 migration moved hooks from `wsl.exe --exec bash -c <script>` to a
script piped into `bash -s`. Anything the script runs that reads stdin then
drains the rest of the script, bash hits EOF and exits 0, and the caller logs
success -- an orca.yaml hook of `ssh -T git@github.com || true` followed by
`pnpm install` silently never installs.
Scripts now travel in argv by default, which is what the pre-migration code
did and what --exec makes safe. `scriptDelivery: 'stdin'` stays for the one
caller that needs it: the hook-relay installer embeds a base64 JS bundle far
past any command-line limit, and reads no stdin.
A runner test already described this exact EOF hazard -- for the login shell,
not for the guest command it was itself creating.
Credit: code review.
* fix(skills): make the unverifiable check unconditional, and stop double-probing
Round 3.
- provider detection threw only on an EMPTY result, so a degraded partial hit
slipped through: `claude` visible on the default PATH via Windows interop
plus an nvm-only `codex` returns a plausible ['claude'], and the caller then
skips the ~/.codex skill roots for a provider that is installed. The
installer already got this right with an unconditional throw.
- three sites asked for 'preferred' without needing it. The GROK_HOME probe
runs its own `"$login_shell" -lc`, so the runner's probe was a second login
shell eating up to half an 8s budget; the two skill scans are
find/base64/head/printf/stat over $HOME.
- the indirection binder missed `private readonly x = 'wsl.exe'` (the
modifier was captured as the name), backtick literals, and
`spawnProcess(this.x)`. Commit 2bbbd99 claimed that gap closed; it now is,
verified against all three shapes.
Credit: Grok.
* fix(child-process): keep the tail of output whose failure lands last
Two console-flash bugs the ratchet was carrying on its allowlist rather than
catching: daemon-process-inspection execs powershell.exe and the gemini
extractor execs `where gemini`, both console-subsystem, both without
windowsHide (#10488). Allowlist 80 -> 78.
And a migration regression: the hook-relay install used to keep a rolling
tail of stderr (`slice(-MAX)`), while runProcess's maxOutputBytes keeps the
head. A guest install that fails after pages of apt warnings therefore
reported the warnings instead of `mv: Read-only file system`. runProcess
takes retainOutput: 'tail' for output whose meaning is at the end.
Credit: code review.
* test(wsl): close the last two indirection shapes in the binder
`this.binary = 'wsl.exe'` has no declarator keyword, and a helper that just
returns the literal is a spawn one hop away that no regex can follow. The
return case fails closed only when the file also spawns something --
local-windows-terminal-runtime.ts returns the name as terminal metadata and
never spawns, so a blanket rule flagged it wrongly.
Verified against both shapes: planted, failed, restored, passed.
Credit: Opus.
* fix(preflight): stop reporting installed WSL CLIs as absent (#9725)
The last two probe sites that turned an unresolvable login PATH into a
confident negative. The native branch of detectInstalledAgents already
consults install dirs for exactly this reason ('PATH may still be unhydrated
on a cold GUI launch'); the WSL branch had no equivalent, so a cold distro
made an nvm-installed claude/codex read as not installed and told the user to
install a CLI their own terminal runs.
Ports that fallback to the guest: agent detection checks the version-manager
bin dirs for commands the PATH lookup missed, and the preflight command runner
APPENDS them to PATH -- append, never prepend, so a resolved login PATH stays
authoritative and a stale nvm version cannot shadow the real binary.
Tested by executing the generated scripts through /bin/sh against planted
binaries, since the behaviour is shell globbing and [ -x ]. Both the
nvm-discovery and the no-shadowing tests were verified to fail when reverted.
* fix(codex-accounts): hide the console on the legacy active-home migration
execFileSync('wsl.exe') with no windowsHide flashes a conhost and steals
foreground on a GUI-launched Orca (#10488). Sibling WSL spawns got this in
earlier commits; this one only had its quoting rewritten. Allowlist 75 -> 74.
Credit: code review.
* fix(windows): close the shell:true hole that made windowsHide a no-op
I un-allowlisted the gemini extractor after adding `windowsHide: true` to an
`exec()` call. `exec` implies `shell: true`, which this repo's own chokepoint
documents as silently making windowsHide a no-op (#14543) -- so the site still
flashed a conhost while reading as guarded. Now execFile('where.exe', …),
matching the relay sibling that already did it right.
The ratchet could not see that, which is why it passed. It now treats a call
that resolves to exec/execSync, or any `shell: true`, as unguarded regardless
of windowsHide -- including through renamed imports and a renamed promisify.
Also: a script over 8000 chars now falls back to stdin. Windows caps a command
line at 32767 and a user's orca.yaml hook is the one unbounded script Orca
runs (`run-both` concatenates two; a vendored installer is ~15KB), so argv
would fail to spawn outright. Degrading beats failing.
And the binder now sees `let p: string` ... `p = 'wsl.exe'`.
Each verified by planting. Credit: Grok.
* test(wsl): an opaque payload must declare its interpreter
My per-call bashism guard REPLACED the file-wide one, and that was a strict
regression: the real payloads are built in a separate function and passed as a
bare `script,`, so the bashism is never inside the call literal and the
per-call arm cannot fire. Deleting `shell: 'bash'` from skill-discovery-wsl
-- `done < <(find ...)` and `read -r -d ''`, the #14292 signature -- passed on
this branch and failed on main.
Reading through the identifier is guesswork. Requiring the call to name its
shell when the payload is not a literal is not, so seven POSIX call sites now
say `shell: 'sh'` -- no behaviour change, sh was already the default.
Two earlier attempts at this were wrong and are worth recording: a whole-file
BASHISM test blamed codex-accounts/service.ts, which correctly pins bash on its
four inline payloads and correctly leaves printf/mkdir unpinned; and excluding
call text still caught a bash payload belonging to a non-runner execFileSync.
Also: runProcessSync now refuses retainOutput:'tail' instead of silently
keeping the head, and the union docblock no longer describes stdin delivery.
Verified against both of the plants that exposed this. Credit: Opus.
* test(wsl): judge an opaque payload by the file, not by whether shell is set
Round 5. My previous rule -- opaque payload must have `shell:` -- was the
third guard fix in a row that came out weaker than what it replaced:
`shell: 'sh'` on a bash payload satisfied it, which is #14292 with extra
steps. Flipping skill-discovery-wsl's pin from bash to sh shipped green.
Now: strip the text of every call that already names bash, and if a bashism
survives anywhere in the file while a script-carrying call is not bash-pinned,
flag it. Stripping the bash-pinned calls is what keeps codex-accounts clean.
Also closes four ways to hide a call from the collector, each verified by
planting:
- `script: \`${bashism}\`` -- a template literal read as a visible literal
- `runWslProcess({ ...spec })` -- a spread hides script AND shell
- `Object.assign({ a }, { script })` -- the collector took the first `{`, so it
now takes the whole argument list
- `import { runWslProcess as runWsl }` -- a renamed callee collected nothing,
and zero calls read as zero violations
Not fixed, recorded instead: a computed `shell:` in the console guard. Matching
any non-false value also flags `shell: spawnConfig.shell`, a pass-through that
is false in every branch, and a false positive there costs an allowlist entry
that disables the guard for a whole correct file.
Credit: Grok.
* fix(preflight): make the guest fallback match the native one it claims to mirror
Three defects in the #9725 fix from earlier today, all found by executing the
generated scripts under real dash rather than reading them.
- $HOME containing a space word-split the unquoted dir list into a relative
path, so every CLI read as absent -- the exact symptom the fix exists to
remove. Each entry is quoted now; the nvm entry quotes only its prefix so the
glob still expands.
- A directory passes `[ -x ]`, so ~/.local/bin/gemini/ was reported as an
installed CLI that then fails to launch with EISDIR. The PATH half of the
same script already guarded this, and so does the native twin.
- The header called this the "guest-side twin" of the native fallback while
omitting four of its directories: volta, asdf, fnm and mise. A WSL user on
any of those still had #9725 while the same user on native did not -- and
asdf and mise are named in the motivating comment. The claim is now true.
Credit: Opus.
* test(wsl): mask bash-pinned calls by position, not by String.replace
`rest.replace(text, '')` with a string pattern removes only the FIRST match,
so two identically-written pinned calls left one behind and its bashism then
counted against an unrelated unpinned call in the same file. A body that also
occurred earlier as a substring would blank the wrong region entirely.
The collector now returns ranges and the mask is applied by index. Verified
both directions: two identical pinned bodies plus one unpinned call flags, and
the same file with all three pinned stays clean.
* test(wsl): fail closed on call shapes a regex cannot attribute
Round 6. Rather than widen the pattern again, treat the shapes it cannot
reason about as unreadable.
A regex cannot tell which object a key belongs to, so every round produced
another way to put the pin in one place and the payload in another:
`cond ? {pinned} : {unpinned}`, `{...} as WslSpec`, `Object.assign({a},{b})`.
A call whose SPEC is chosen by a ternary or spread -- one appearing before the
first `{` -- or which carries an `as` assertion is now flagged whenever the
file has a bashism, with no `shell: 'bash'` escape, because the substring test
that would grant the escape is exactly what cannot be trusted on these shapes.
A ternary INSIDE the object is not exotic: claude-accounts/service.ts:977 uses
one to choose a script line in a call that is already pinned, and treating that
as opaque would demand a second pin it already has. Nor is a nested call --
`script: `x ${shellQuote(p)}`` is how every payload here is built, and flagging
it would demand bash on POSIX payloads that must not have it.
Also follows `const run = runWslProcess`, generics and optional chaining, and
counts collected calls against mentions so a shape that slips the pattern reads
as unreadable rather than clean.
I tried the TypeScript parser first, which would remove the class outright.
TypeScript 7 is the native port and exposes no JS compiler API; oxc-parser
works but is transitive, and declaring it surfaced an unmet peer warning.
Recorded here so the next person does not repeat the detour.
Credit: Grok.
* fix(wsl): fish is a PATH lookup, and my lint check could not fail
Two things Opus caught that I had verified wrongly.
`wsl-fish-history-cleanup` passes `program: 'fish'` -- a bare name, so a PATH
lookup by definition, the exact class the earlier rounds hunted. I mapped it to
'none' and then defended that in an audit, because I read
`allowDegradedEnvironment: true` as "does not need the login PATH". It does not
mean that: it means "do not fail when the probe fails". The old call still USED
the login PATH whenever it got one, which is 'preferred'. Under 'none' a fish
from linuxbrew or nix is invisible and the cleanup throws. The truncated
comment left behind when the flag was deleted is finished too.
And `pnpm lint` has been failing on this branch while I reported it clean: I
grepped for `error eslint|error oxlint`, but oxlint prints the rule category
(`error typescript(array-type)`, `error unicorn(prefer-ternary)`). The grep
could not match, so it never failed. Checking the exit code instead surfaced a
third violation hidden behind the first two.
Credit: Opus.
* chore(wsl): clear the round-7 P2s
- Formatting: the branch owned 22 of the tree's 26 oxfmt failures because I
never ran the formatter. Branch files now own none.
- resolveScriptDelivery was computed twice, in two places that must agree
about argv shape and stdin payload. Resolved once and threaded through.
- The allowlist header said the list only shrinks while the branch added three
entries. It grew because the scanner learned to follow a variable-bound
'wsl.exe'; those three were previously recorded in prose, so the count was
wrong by three in the direction that hides offenders. The header now says so.
- Two test comments still explained behaviour via the deleted
allowDegradedEnvironment flag; a stray triple blank line; two adjacent JSDoc
blocks where only the second attached.
Not taken: platform-guarding addWslEnvKeys. WSLENV is inert off Windows, and
the guard broke a test that asserts the key directly -- more surface than the
tidy is worth, so the reason is recorded at the call site instead.
Credit: Opus.
* test(preflight): plant a fabricated CLI name, not a real one
CI caught what my local run could not: the runner has a real /usr/bin/gh, so
`command -v gh` resolved to it and the planted nvm stub was never reached. The
fallback APPENDS, so that is the code behaving correctly -- the test was
asserting a property of my machine.
Both real-shell suites now plant `orca-fake-cli`, which exists nowhere.
Re-verified the same way as before: with the PATH fallback disabled the test
fails, with it restored it passes.
I declared this branch merge-ready without looking at CI. Local green is not
the gate.
* refactor(wsl): delete two knobs and a duplicated fallback
Elegance pass. The branch had grown from a deletion into a net addition, and
most of the growth was optional axes with one caller each.
- `retainOutput` is gone. One production caller wanted the tail of a 64KiB
buffer; head-truncation only hurt because of that cap. The caller drops the
cap, keeps the default, and slices the tail itself -- which is what the live
relay next door already does. Two mechanisms for one job became one.
- `scriptDelivery` is gone. The size rule was already the whole design:
argv unless the script is too long for a Windows command line. The option
existed so a small Orca script could opt into stdin, and no such caller ever
appeared. Both behaviours stay pinned: a huge script still goes to stdin, an
ordinary one still leaves the hook's stdin free.
- Agent detection no longer walks the fallback dirs itself. It prepends the
same PATH prelude the preflight command runner uses and lets the ordinary
lookup do the work. Its bespoke walk had duplicated the lookup script's
`! -d` guard -- and had missed it once, which is how a directory read as an
installed CLI.
All 17 detection tests still pass unchanged, including the $HOME-with-a-space,
directory-is-not-a-CLI, and volta/asdf/fnm/mise cases, so the collapse is
behaviour-preserving rather than assumed to be.
Credit: Grok.
* fix(wsl): never name a path-shaped variable in WSLENV
`buildHostEnv` forwarded every caller-supplied key into WSLENV. wsl.exe
translates path-shaped variables between Windows and Linux form, so a caller
passing PATH would have replaced the guest's own PATH with a translated
Windows one -- silently, and fatally for every lookup after it.
No caller passes PATH today. The point of a chokepoint is that it does not
depend on that staying true.
succeeded rendered as casual declarative 성공했다 ('it succeeded') next to
skipped's polite 건너뛰었습니다 and pending's noun-phrase 보류 중, in a summary
that joins all three after a count: '3 성공했다 · 1 건너뛰었습니다'. Machine
translation read succeeded as a finished sentence instead of the noun label
the other two siblings use. Switch to 성공 and pin it in the key-override
file so the catalog regen script can't revert it; #15875 fixed five other
keys in the same family but its hardcoded regression map didn't cover this
one.
* fix(ui): clear agent attention icon when Floating Workspace tab activated via keyboard
When a tab in the Floating Workspace is activated through keyboard shortcuts
while the Floating Workspace is not the active worktree, the agent completion
notifications were not being acknowledged, leaving the yellow attention icon
visible.
The issue was that `useAutoAckViewedAgent` only checked the global `activeTabId`,
which doesn't change when the Floating Workspace is not the active worktree.
Now the hook also watches the Floating Workspace's active tab
(`activeTabIdByWorktree[FLOATING_TERMINAL_WORKTREE_ID]`) and acknowledges agents
when they become visible in the Floating Workspace, regardless of whether it's
the active worktree.
Fixes#15700
* fix(ui): gate Floating Workspace auto-ack on panel visibility
The floating-workspace scan added in the previous commit had no visibility
gate. The panel stays mounted while closed (use-floating-workspace-panel:
shouldMountPanel), so its layout still resolves an active leaf and the hook
acked a floating agent completion the moment it landed — silently killing the
minimized toggle's attention dot (selectFloatingWorkspaceHasUnread), which is
the only "unseen floating activity" signal a closed panel has.
- Scan the floating tab only while the panel is actually visible
(isFloatingWorkspacePanelVisible), so a closed panel keeps its dot.
- Move the activeView filter onto the main-worktree target only: the panel is
an overlay above every view, so it must ack from the activity/tasks views too.
- Carry the owning worktree with each target instead of re-deriving it by tab
id, and keep the first entry on a tab-id collision, so a duplicate id can no
longer clear the wrong worktree's unread. Drops the `as string[]` cast.
- Re-scan on TOGGLE_FLOATING_TERMINAL_EVENT (next frame, after aria-hidden
commits) since panel open/closed is React state the store never sees —
opening onto an already-active completed tab now acks.
- Cover the new resolveAutoAckTabTargets helper, including the closed-panel
regression asserted against selectFloatingWorkspaceHasUnread.
* fix(ui): re-scan floating workspace auto-ack on every panel-open path
The visibility gate read the panel's aria-hidden and only re-scanned on
TOGGLE_FLOATING_TERMINAL_EVENT, so the two paths that open the panel without
that event — the floatingWorkspace.maximize keybinding and the default
floating-button toggle — left an already-active completed tab's attention icon
lit. Drive the gate from the committed `enabled && open` state instead: that is
what aria-hidden is derived from, it covers every open path, and it drops the
requestAnimationFrame that existed only to outrun the un-committed DOM read.
Adds a hook-level test (happy-dom) that fails both when the gate is removed and
when the open re-scan is removed.
* fix(ui): re-read store state per auto-ack target
Acking the first target writes to the store and re-enters the scan
synchronously, so the pre-write snapshot could re-ack a target the
nested pass already handled. Idempotent today; a footgun for the next
non-idempotent action.
---------
Co-authored-by: Claude <claude@anthropic.com>
Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>
* fix(automations): localize schedule weekday names and labels
The Weekly Day picker rendered a hardcoded English tuple, and shared
schedule labels built copy as `${day}s at ${time}` from an OS-locale
Intl weekday, so a non-English UI showed Sunday…Saturday (or 星期五s).
Shared now emits deterministic English (the CLI contract) plus a
locale-free AutomationScheduleDescriptor; the renderer formats that
descriptor through translate() with Intl/CLDR weekday names resolved
from getIntlLocale(). Fixes#14404.
* test(automations): assert localized weekday copy in rendered DOM
The existing coverage walked the React element tree, so nothing proved the
Day dropdown and cron status row reach the DOM localized. Mount the picker
under happy-dom with the Radix Select swapped for a native <select> (the
pattern RepositoryWorktreeDefaultsSection.test.tsx already uses, since Radix
portals its content only once opened) and read real option text.
Also key the weekday SelectItems by index rather than by translated copy, so
a runtime language switch reconciles instead of remounting all seven items.
* fix(automations): keep the weekday SelectItem key off the array index
react-doctor(no-array-index-as-key) rejects `key={index}`; the localized
weekday name is already unique per locale, so keep it as the key.
* fix(automations): match the real AutomationDraft shape in the render test
The fixture invented `repoId`/`branchMode`/`enabled` fields; runtime ignored
them but `tsc` did not. Mirror AutomationSchedulePicker.test.ts's fixture.
* fix(automations): localize the custom-cron field chips
The five cron field headers rendered one row above the status row this PR
localizes were still hardcoded English, so a Chinese UI showed
Minute/Hour/Day/Month/Weekday. Same defect shape as the deleted DAY_OPTIONS
array. Chip keys move to stable field ids so a locale that renders two fields
with the same word cannot collide, and the truncated header carries a title so
longer copy (es 'Dia de la semana') stays readable.
* fix(automations): keep weekday option keys stable
* test(automations): assert in-place language switch on mounted picker
Add a test that changes the language while the weekly picker remains mounted,
then checks that the localized labels update while the underlying values
("0"–"6") stay stable. This validates the regression-prevention that stable
index keys (added in #15884) support — without them, a locale change would
unmount and remount options, breaking the persisted dayOfWeek value.
Wrap the picker in a LanguageAwarePicker harness that calls useTranslation(),
mirroring the root-level subscription in main.tsx that drives real
re-renders on language change.
---------
Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>
* Add Artifacts and Skills pages to navigation history
- Record Artifacts and Skills visits in back/forward navigation like Automations
- Both pages properly rewind history when closed to the previous live entry
- Extract rewindHistoryIndexPastView() helper to deduplicate close-page logic across all page types
- Add test coverage for Artifacts/Skills navigation, separate entries, and shared link handling
* Add Artifacts and Skills pages to navigation history
Back/forward buttons now appear when navigating to Artifacts and
Skills pages, consistent with Terminal, Tasks, and Automations.
* fix(automations): localize schedule weekday names and labels
The Weekly Day picker rendered a hardcoded English tuple, and shared
schedule labels built copy as `${day}s at ${time}` from an OS-locale
Intl weekday, so a non-English UI showed Sunday…Saturday (or 星期五s).
Shared now emits deterministic English (the CLI contract) plus a
locale-free AutomationScheduleDescriptor; the renderer formats that
descriptor through translate() with Intl/CLDR weekday names resolved
from getIntlLocale(). Fixes#14404.
* test(automations): assert localized weekday copy in rendered DOM
The existing coverage walked the React element tree, so nothing proved the
Day dropdown and cron status row reach the DOM localized. Mount the picker
under happy-dom with the Radix Select swapped for a native <select> (the
pattern RepositoryWorktreeDefaultsSection.test.tsx already uses, since Radix
portals its content only once opened) and read real option text.
Also key the weekday SelectItems by index rather than by translated copy, so
a runtime language switch reconciles instead of remounting all seven items.
* fix(automations): keep the weekday SelectItem key off the array index
react-doctor(no-array-index-as-key) rejects `key={index}`; the localized
weekday name is already unique per locale, so keep it as the key.
* fix(automations): match the real AutomationDraft shape in the render test
The fixture invented `repoId`/`branchMode`/`enabled` fields; runtime ignored
them but `tsc` did not. Mirror AutomationSchedulePicker.test.ts's fixture.
* fix(automations): localize the custom-cron field chips
The five cron field headers rendered one row above the status row this PR
localizes were still hardcoded English, so a Chinese UI showed
Minute/Hour/Day/Month/Weekday. Same defect shape as the deleted DAY_OPTIONS
array. Chip keys move to stable field ids so a locale that renders two fields
with the same word cannot collide, and the truncated header carries a title so
longer copy (es 'Dia de la semana') stays readable.
* fix(automations): keep weekday option keys stable
---------
Co-authored-by: Jinjing <6427696+AmethystLiang@users.noreply.github.com>
* feat(automations): navigate search results with arrows
* Set overview tab for external automations on arrow selection
External automations lack a runs tab, so the detail pane must default to overview when navigating via arrow keys to keep the tab selection valid when the automation is later opened.
---------
Co-authored-by: m4air <m4air@m4airs-Air.localdomain>
* Move worktree palette search to top of sidebar nav
The Cmd+J search button is now displayed as the first item in the
sidebar navigation, improving discoverability. Styling is simplified
to match the nav item layout with flex-based display and consistent
spacing.
* Test: add guard clause for worktree palette search button
- Add explicit type annotation for the search button querySelector
- Guard against missing button with clear error message
- Use direct property access now that button existence is verified
* Remove agent map view from dashboard
Removes the view toggle and simplifies the dashboard to show only the kanban board layout.
* Assert boardProps is initialized on drawer open
Adds a configurable, unbound-by-default `dashboard.toggle` action that toggles the Agent Dashboard (in-window drawer or pop-out, per the existing mode setting).
- Wired through window-shortcut-policy, main-window dispatch, browser-guest dispatch, preload, and the renderer IPC handler.
- Opening the in-window drawer reveals the sidebar first; closing leaves it alone.
- Gated on the `experimentalAgentDashboardPopout` experiment, and the Settings shortcut row is hidden while that experiment is off.
* fix(terminal): preserve Shift+Enter during routing confirmation
Keep previously trusted CSI-u routing active only while local foreground revalidation is pending. Clear pending authority on inconclusive reads and never promote display-only identity.\n\nRefs #12541\nRefs #13597
* fix(terminal): respect global WSL routing gate
* fix(terminal): bound the retained routing capability to a live read
Two holes in the previous commit let one provider read authorize CSI-u
indefinitely.
First, routingConfirmationPending satisfied its own precondition, so a
pending entry re-published itself on every reconfirmation request. Only
one of the five callers is the Shift+Enter burst timer; the others fire
on accepted submit/interrupt bytes, focus and visibility changes, and
onAgentExited -- the last of which runs precisely when a shell title
proves the agent is gone. On cmd.exe and Git Bash there is no OSC
boundary to publish over the entry, so nothing decayed it.
Second, the flag was published even when no confirmation read was
actually scheduled -- a hidden pane, a command read already in flight, or
a null pty id -- and only the read's inconclusive settle clears it.
Require routingTrusted to grant the capability, and publish the flag only
after sampling reports a read in flight, so it cannot outlive the read
that justifies it. This also makes the canConfirmRouting gate redundant:
the tracker already refuses WSL, SSH and remote pty ids.
* review round 2: bound the retained capability to any in-flight read
onVisiblePtyBound refuses to schedule while a higher-authority command
read owns the pane, so gating the pending publish on it mistook 'a
command-finished read already owns this' for 'no read at all' and
dropped CSI-u for >=350ms — the window #13598 exists to close.
Ask the tracker whether any read is in flight instead, and settle the
visible confirmation from the command-finished branch that publishes
nothing, so the flag cannot outlive the read that justifies it.
* review round 2: tighten the reconfirmation gate and its comments
hasReadInFlight already covers the visible-pty read that
visibleForegroundSamplePending tracked, so the disjunction was
redundant. Collapse the stacked Why blocks into one.
* review round 3: release the retained capability when a read is abandoned
Every outcome path now clears routingConfirmationPending, but the two
abort guards in readForeground return without publishing or settling. A
pty rebind during the multi-second inspection RPC — a detach/remount
emits no onExit, so the store entry survives — therefore stranded the
flag permanently, leaving Shift+Enter on CSI-u with nothing left to
revalidate it. Worse, once sampling is suppressed by a live hook row the
pane routes bytes on hook evidence alone, which the resolver excludes
precisely because PTY output can forge it.
Settle an abandoned read unless a newer generation will settle for it.
* review round 4: release the retained capability at every exit without a successor
Rounds 2 and 3 closed the outcome paths and the aborted-read paths, but
two review lanes independently proved a third: cancelPendingRead bumps
the generation, so a cancel never settles, and dispose plus the three
untrackable early returns schedule no successor to settle for them. The
store entry outlives the tracker — detach/remount emits no PTY exit — so
the flag latched there with no read left alive.
A visible remount self-heals, but a hidden pane does not: the dashboard
card resolves the encoding for any pane key regardless of visibility.
Release the capability from whichever exit ends the read, and say in the
entry's own doc comment that the flag is Shift+Enter-scoped.
* test(terminal): pin the remaining capability-release exits
dispose and the command-finished exit were covered; the visible-bind and
command-start variants are the same three-line pattern and were not.
Neutering the release now kills all four.
---------
Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
* fix(terminal): mark captured shortcut input interactive
Refresh the interactive-redraw timestamp after a captured shortcut send
succeeds, so the composer redraw that follows takes the low-latency
foreground path instead of waiting out the 1s coalesce fallback.
Orca's captured shortcut path sends directly through the captured pane
transport to preserve pane, PTY and transport identity, and so bypasses
xterm's onData -- the only place that previously stamped the timestamp.
An idle pane therefore scheduled the post-Shift+Enter redraw as ordinary
throughput. Measured in a real Pi pane on Windows: ~1029ms before,
~16-20ms after, at an identical ~2.3KB redraw.
Stale pane bindings and rejected transport sends cannot refresh it.
Fixes#10203
Refs #13598
* fix(terminal): keep captured shortcuts out of the pane-teardown signal
lastTerminalInputAt has two readers, and the previous commit only meant
to change one of them. onExit reads it as "the user never typed into
this pane" to keep a newborn pane mounted when its shell dies on startup
(the failing-.envrc direnv case, pty-connection.ts onExit) so the error
stays visible and the worktree stays active.
Stamping it from a captured shortcut therefore made a single Shift+Enter
before that exit close the tab and bounce the user to Landing -- on every
platform, since captured shortcuts are not Windows-only.
Split the redraw window onto its own timestamp so captured shortcuts open
the fast path without arming the teardown, and leave onExit's behaviour
byte-identical to main.
* test(terminal): pin the captured-shortcut wiring and its staleness guard
Deleting the onAccepted block, or replacing its binding-identity check
with true, both left the whole suite green — so nothing pinned the part
of this PR that actually ships. Cover both against the existing IME
keyboard harness.
---------
Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
DialogContent used an implicit grid column, which sizes to min-content: an
unbreakable token such as a long filename in the title widened the column past
the panel, so the title overflowed and the justify-end footer buttons rendered
outside the visible surface. Pin the column to minmax(0,1fr) and let long
titles wrap.
* Increase shell readiness timeout to match daemon barrier
Slow interactive rc files can take longer than 1.5s to initialize. Raise
the startup command readiness timeout from 1.5s to 15s to match the daemon
barrier and prevent queued commands from executing mid-startup.
* fix(terminal): keep a quick command queued until its own spawn takes it (STA-4876)
Triggering a quick command opened a terminal tab titled with the command's
label, the shell started and drew its prompt, and the command never ran.
TerminalPane snapshots `pendingStartupByTabId[tabId]` in a useState lazy
initializer, and a mount effect deleted the entry immediately. The pane's key
is `${tab.id}-${tab.generation ?? 0}`, so anything that bumps generation before
the command reaches a shell — the stall-recovery remount fired from
`requestTerminalPaneRecovery`, or the allDead activation regeneration — mounted
a second pane that re-read an emptied slot and spawned with no command at all.
The loss was permanent, which is why every scope failed alike: repo, global and
agent-prompt all funnel through the same queue-then-snapshot sequence.
Spend the entry at `onPtySpawn` instead, which is the one point that proves this
pane's own fresh spawn exists. A pane retired mid-connect never reaches it, so
the command stays queued for the next mount; reattach skips `onPtySpawn`, so it
cannot spend a command it never delivers.
Three details are load-bearing:
- Ownership is reference identity (`paneOwnsQueuedStartup`). Setup and issue
splits borrow the same `deps.startup` field for their own one-shot payload, and
that payload can be structurally identical to the queued command, so a
truthiness test would let a split pane spend a command it never runs.
- The consume runs after `bindActivePanePty`. While the tab still has no ptyId,
the queued entry is the only thing holding its worktree out of the
retention-budget force-park, so dropping it first unmounts the pane mid-spawn.
- The callback is one-shot. `onPtySpawn` fires on every fresh spawn a pane makes,
including hibernation wake and the respawn ladder, and a command queued after
the first launch belongs to that later launch.
Known residual, documented at the call site: the consume tracks "a pty exists",
not "the command ran". Windows embeds short commands in the shell argv, so they
execute before the spawn resolves and a pane retired in that window re-delivers
on remount; on POSIX the write waits for shell-ready, so a pty that dies in that
window loses a command already spent. Closing either needs a delivery signal
from main rather than this callback. Both windows are narrow, and both are
strictly better than losing the command unconditionally.
* fix(terminal): guard the queued-startup wiring the review found untested
Follow-ups from the final review pass on this branch.
- Collapse the ownership + one-shot decision into `createQueuedStartupConsumer`
so the call site is a single call rather than inline logic no test could
reach. Two mutants survived the whole suite before this: relaxing ownership to
a truthiness check, and dropping the one-shot guard. Both now fail.
- Rewrite the throwing-consume test. It asserted `updateTabPtyId` had been
called, which runs *before* the callback, so it passed with the try/catch
deleted. It now asserts the throw does not escape into the connect promise,
which is the invariant the try/catch actually provides.
- Correct the `onQueuedStartupSpawned` docblock. It claimed the callback is "the
first moment the command is guaranteed to reach a shell"; the diff's own caveat
says otherwise, since Windows runs an argv-embedded command before this fires
and a POSIX shell can die before the shell-ready write. It marks a live shell,
not delivery.
No behavior change: the consumer is the same predicate and the same one-shot,
moved behind one exported seam.
* fix(terminal): roll shell wrapper isolation into a fresh daemon
* Revert "Increase shell readiness timeout to match daemon barrier"
This reverts commit 6ab273ef36.
* Condense queued startup spawn comment
Simplify the multi-paragraph explanation into a concise summary that captures the key points: spawn must wait until after the pane is bound to preserve the worktree from force-parking, and the behavior differs between POSIX and Windows for delivery timing.
* fix(terminal): prevent consuming replaced queued startup commands
When a queued startup is replaced before the pane's first spawn,
the one-shot guard alone still allows consuming the replacement.
Add isStillQueued callback to verify the slot still holds the
originally captured command (STA-4876).
---------
Co-authored-by: Jinwoo-H <jinwoo0825@gmail.com>
Neil
Jinjing
Isaque Coelhoandm4air
OrcaWinandOrcaWin
Brennan Benson
erish
Jinwoo Hong
Hwanseok Choi
Jason BrashearandClaude Opus 5
Denis Darii
Yeray
VincentandOrcaWin
ppw-stack
