Commit Graph
14 Commits
Author SHA1 Message Date
Neil ee1a0a4e2d fix(git): avoid Windows tree kills after the command has exited (#20606)
Validated and independently reviewed OMP integration fix.
2026-09-14 13:56:25 -07:00
Jinwoo Hong c84007c541 feat(rpc): generate a shared params catalog from the host registry, gated on parse parity (#19961) 2026-09-10 21:18:39 -07:00
Neil 75c1f32f81 fix(gh): log when gh/glab is killed at its deadline (#18555) 2026-09-06 16:17:54 -07:00
Neil 0a821e5bc8 fix(crash-reporting): make the own-Chromium gate a real choke point, and stop a refusal leaking the root (#18459)
* fix(crash-reporting): make the own-Chromium gate a real choke point

Round-3 review found the guard was not the choke point its own comments
claimed: six pid-addressed `taskkill /pid <pid> /t /f` families in main were
ungated and uninstrumented, so the stale-pid shape stayed producible and a
`selfInitiatedTreeKillCount: 0` could read as exculpatory when it was not.

- Gate the remaining main-process families: the git command-runner abort, the
  notebook-cell and automation-precheck timeouts.
- Turn the `src/shared` seam into the gate itself (`process-tree-kill-gate`), so
  the runProcess choke point, the codex app-server deadline kill and the
  ephemeral-VM recipe kill ask the same decision. Those three are compiled into
  the CLI/relay too and cannot import main; main installs the guard at preflight.
- Ratchet (`main-process-tree-kill-gate.test.ts`): a new pid-addressed taskkill
  in main that skips the gate fails, and the allowlist entries must still exist.
- Give pid-addressed kills eviction priority in the 32-entry ring: 32 routine
  `win-pty-job` teardowns from a window-close burst no longer evict the one
  entry that discriminates a self-kill from an external one.
- Correct the coverage doc, which described the uninstrumented Windows sites as
  POSIX `process.kill(-pid)` group kills and omitted the git and codex paths.

* fix(crash-reporting): keep a refused tree-kill from leaking the root it owns

A refusal must block the pid-addressed tree walk, not the termination. Five of
the six gated sites returned on refusal with no fallback, so a refused
`taskkill /pid /t /f` left git.exe, a timed-out notebook cell, an automation
precheck or an ephemeral-VM recipe running while the caller reported it stopped.
The root kill is addressed by the child handle, which cannot reach the recycled
pid the refusal is about, so it stays correct and required on that path.

Also fixes the ring eviction the scope preference introduced: with the ring
saturated by pid-addressed kills, the only non-pid-addressed entry is the one
just pushed, so the splice evicted itself and the detail came back `{}` --
byte-identical to the external-kill arm, in the window-close case the guard
exists for. Eviction now excludes the newest entry and falls back to FIFO.

Tests: refusal now asserts the root kill at all six sites, and the ring covers
the saturated-pid ordering as well as round 3's group-burst ordering.

* fix(crash-reporting): stop a refused tree-kill leaking the commit-message agent, and count call sites

Two round-5 blocking findings, both open on main and on both branches.

`killSourceControlAgentProcess` had no root-kill fallback on its win32 arm: the
taskkill was the only termination, so once the own-Chromium gate could refuse it
the promise resolved having killed nothing. Both callers do
`terminationComplete ??= killSourceControlAgentProcess(child)` and then release
the managed-home lock on that promise, so a refusal left the local Codex/Claude
commit-message agent running while the caller reported it stopped -- the
lock-contention failure the taskkill was added for. Same fix as the six sibling
sites: the handle-addressed root kill cannot reach the recycled pid the refusal
is about, so it stays correct and required on that path.

The ratchet was file-granular, not call-site granular: one gate mention anywhere
in a file exempted every taskkill in it, which left the six files that now ask
the gate ratchet-blind -- the inverse of what it is for. It now counts `/pid`
call sites against gate admissions per file, so a second ungated kill inside an
existing family fails. Keying on the `/pid` argument rather than a quoted
`taskkill` also catches a kill whose program name comes from a constant. The
three comments that claimed more than the old scan enforced now state the rule
and its two remaining blind spots.

Also: the recording in `admitSelfInitiatedTreeKill` is now wrapped the way the
`admitProcessTreeKill` seam already wraps it, with the refusal decision taken
before anything that can throw so a diagnostics failure cannot flip it; and
`orca-chromium-process-pids` documents the false-positive direction (a stale
`getAppMetrics()` entry plus pid reuse refuses a live unrelated child), which is
the mechanism the root-kill fallback exists to bound.

Tests: refusal now asserts the root kill at all seven sites; the ratchet asserts
call-site counting and the constant-program form.

* test(crash-reporting): run the own-Chromium gate against real Windows trees

Nothing on this branch had ever executed on Windows. The unit tests pin the
gate's decision against a mocked taskkill, which cannot show that the decision
does anything to a real process: that `/T /F` reaps a detached grandchild, that
a refusal leaves that tree standing, or that the handle-addressed root kill the
refusal path falls back to reaps the root while orphaning descendants.

Adds a win32-gated live test covering all four, registered in both the
`package_windows` CI lane and `WINDOWS_PACKAGE_TESTS` as
`win32-test-lane-registration` requires.

Also completes the coverage doc's "never instrumented" list, which omitted the
macOS keyboard-input-source probe's POSIX group kill in `ipc/app.ts`.

* fix(crash-reporting): pin the commit-message root kill on the Windows arm

The first Windows run of this branch found nine failures the macOS suite
cannot see: `commit-message-text-generation-test-harness` asserts
`expect(child.kill).not.toHaveBeenCalled()` on `process.platform === 'win32'`,
which is the contract the previous commit deliberately replaced — and it
branches on the real platform, so it is dead code everywhere CI runs today.

The harness now asserts the handle-addressed root kill on every platform. On
win32 it lands after the tree walk, so the expectation waits rather than reading
one tick early, and its ten call sites await it. Red against the pre-fix arm at
all seven sites; the production code is unchanged.

* test(crash-reporting): remove the Windows lane marker tree through the retrying helper

The new win32 spec teardown used a raw rmSync, which the windows-lane-tree-removal
boundary ratchet rejects — and which is exactly the EPERM the ratchet exists to
prevent, since this spec's marker directory is written by processes it has just
force-killed.

* fix(crash-reporting): only refuse pid-addressed tree walks, disclose the handle-less codex site

The own-Chromium gate refused the POSIX process-group arm of
signalProcessTree as well, which was new macOS/Linux behaviour: a stale
getAppMetrics() entry plus pid reuse would orphan a group that main reaps
today. A POSIX group only holds what Orca put in it, so the refusal is now
scoped to win-taskkill-tree and the POSIX arm is recorded and admitted like
the other group kills in main. That also drops the synchronous
getAppMetrics() read from every POSIX termination.

codex-turn-added-roots kills roots found by a table walk, so a refusal has
no handle to fall back to. Pin that the refusal is visible - crumb written,
turn reported as not cancelled - rather than fixing what cannot be fixed.

* test(crash-reporting): detach the Windows survival fixture and observe real spawns
2026-09-04 16:42:42 -07:00
Neil d3501f7ad6 fix(worktrees): stop a failed worktree scan from being recorded as an authoritative empty listing (#18456)
* fix(worktrees): stop a failed worktree scan from pruning as an authoritative empty listing

A `git worktree list` that could not run at all — a WSL distro that stopped
resolving, a hung mount, a git binary that errored — was softened to `[]` by the
lenient listing path, so the detected scan published it as
`authoritative: true, worktrees: []`. That disables the #1158 retention guard,
drops every persisted tab for the repo, and the pruned session is written back to
disk on the next launch. The loss is permanent, not a transient glitch.

Route the detected scan through a strict listing that still reports the two
genuinely empty states (repo path gone, not a Git repo) as `[]`. Everything else
rejects, so the existing catch answers `authoritative: false` and the destructive
halves (`rememberLocalWorktreeRoots`, `pruneLineageForMissingRepoWorktrees`)
never see a failed scan.

Also make the failure readable: wsl.exe reports its own launch failures as exit
0xFFFFFFFF with an EMPTY stderr and the `Wsl/Service/WSL_E_*` line on stdout as
UTF-16LE, which is why the field bundle carried a git error with no text. Set
WSL_UTF8 for WSL-routed git (matching the wsl runner, #9010) and attach that
stdout diagnostic to the error so `git.exec` spans name the cause.

* fix(worktrees): surface a failed worktree scan's cause on the repo header with a retry

A failed scan now travels with its reason (optional unavailableReason on
DetectedWorktreeListResult), the repo header shows it with click-to-retry,
and the WSL deleted-guest-directory shape measured on a real Windows host is
pinned as retained-not-pruned.
2026-09-04 15:23:09 -07:00
Neil fb48a9771b fix(gh): reap the whole gh/glab process tree at the deadline on POSIX (#18258)
`gh` and `glab` on PATH are routinely shims — mise, asdf, volta, or a
hand-written wrapper — so a timed-out invocation has a chain to stop, not
one process. `execFileCapture`'s POSIX kill path signals only the direct
child; the descendants are orphaned to init and keep running. #18234 is
exactly that shape: `bash ~/.local/bin/gh` -> `mise x gh` -> `gh`, where
the reporter found the tail reparented to `systemd --user` and still at
100% CPU nearly two hours later. The 15s deadline #18239 added bounds
Orca's semaphore slot and its promise; it does not bound the CPU burn.

Route both CLIs through `execFileCaptureToTermination`, the primitive
git's barrier path already uses: POSIX children spawn `detached`, the
deadline signals `-pgid` and escalates to SIGKILL, and the promise waits
for verified termination. Windows behaviour is unchanged (`taskkill /t`
either way).

Switching primitives also swapped execFile's hard maxBuffer failure for
`runProcess`'s silent clipping, which would have turned an oversized gh
response into a shorter valid-looking one. `ProcessResult` now reports
truncation and the capture rejects on it, restoring the old contract and
closing the same latent gap on git's barrier path.
2026-09-02 15:12:21 -07:00
Neil 1910ab9c9c fix(github): bound and coalesce the Orca star check so gh children cannot pile up (#18239)
`checkOrcaStarred`, `starOrca` and `getAuthenticatedViewer` were the only gh
call sites that reached for the legacy `execFileAsync` instead of
`ghExecFileAsync`, so they ran with no deadline, no process-tree kill and no
coalescing. A `gh` that never exits therefore ran forever and never released
its slot in the 4-wide GitHub semaphore in gh-utils.

Route all three through `ghExecFileAsync`, coalesce concurrent star checks onto
one child, and hoist the Landing star-state effect out of the conditionally
rendered footer so a repo-catalog rewrite no longer remounts it and re-forks gh.

Adds a ratchet test asserting no file outside the command runner names `gh` as
a spawned program.

Fixes #18234
2026-09-02 13:29:12 -07:00
Neil 4bc20cb842 fix(wsl): name an explicit Windows cwd for wsl.exe spawns (#17834)
* fix(wsl): name an explicit Windows cwd for wsl.exe spawns

Removing the worktree Orca was launched from broke every wsl.exe spawn for
the rest of the session. The WSL command builders passed `cwd: undefined`
meaning "the directory is inside the command" -- but CreateProcessW reads
NULL as "inherit the parent's", and the parent's was a \\wsl.localhost path
Linux had just deleted.

Fixes #16463

* fix(wsl): name the spawn directory at the six remaining wsl.exe sites

The first commit fixed the WSL command builders. Six spawn sites were left
inheriting the process cwd, which is the same deletable `\\wsl.localhost`
worktree: `wsl-availability` (both probes), the WSL filesystem watcher, the
agent-hook relay launch, the UNC delete, and the local worktree filesystem.

`wsl-availability` is the one that matters most, and it turns the bug into a
latching false negative. `isRetryableWslProbeFailure` returns false for ENOENT,
so a spawn that failed only because the inherited cwd was gone is cached as
"WSL is not installed" on the 10-minute definitive TTL with exponential
backoff up to 30 minutes. Git keeps working and Orca reports WSL unavailable --
worse than the bug being fixed.

ENOENT stays non-retryable. It is answer-shaped for the reason it is meant to
be -- wsl.exe is not on PATH -- and naming the directory is what removes the
one cause that was not. Making it retryable would instead re-probe every
non-WSL Windows machine on the short window, and would leave the false ENOENT
in place for the other five sites, which have no cache to correct.

Three of these are also on the `runWslProcess` W3 migration allowlist; this is
the interim until they move, and matches what #17837 does inside the runner.
2026-09-02 01:39:48 -07:00
Neil 8ac1c6e2ac perf(git): bound ref and worktree scans (#17655)
* perf(git): bound ref and worktree scans

* fix(repo-search): clamp oversized ref limits

* fix(worktree): keep strict worktree listing unshared

The shared-scan re-export flipped every `listWorktreesStrict` caller from an
isolated subprocess to the coalesced scan. `git worktree prune` in the removal
recovery path does not bump the scan generation, so a post-prune verification
could join a pre-prune scan, see the stale row, and report a successful removal
as a stale registration. The same gap defeats the post-archive-hook rechecks
that exist to catch an external Git client locking the row.

Restore the unshared export and make coalescing opt-in via
`listWorktreesSharedStrict`, which existing callers already use deliberately.

* fix(git): separate a proven absent ref from a failed probe

`show-ref --verify --quiet` exits 1 for a missing ref, but so does `wsl.exe`
when its own launch fails, so reading any exit 1 as absence collapsed
`unverifiable` into `exited`. A genuine miss prints nothing while a wrapper
failure always explains itself, so require empty stderr alongside the exit
code; a runner that reports no stderr at all keeps its exit-code contract.

That same signal removes a spawn regression: `show-ref` is a direct-git read
under WSL, and the runner retried any numeric exit through the user's
interactive login shell. The replaced `for-each-ref` exited 0 on a miss, so
absence never retried; every absent probe now would. Treat a quiet exit 1 as
Git control flow and skip the fallback.

Also narrow the hosted-review suffix fallback: the replaced
`refs/remotes/*/<base>` could not cross a slash, but `show-ref -- <base>`
matches at any depth, so `origin/feature/main` answered a query for `main`
and submitted a review against a base the provider rejects.

Refresh the real-binary compatibility contract to the shipped excludes, and
assert exact probe concurrency rather than an upper bound so a regression to
serial probing fails.
2026-08-31 16:27:28 -07:00
Neil ca516a4306 test(git): pin FETCH_HEAD lock order in the admission lifetime test (#17641)
`serializes FETCH_HEAD callers before they enter admission` assumed that two
same-repo fetches join the FETCH_HEAD lock lane in call order. They do not.

`runWithGitFetchHeadLock` first `await`s `fetchLockPath`, which walks the
filesystem (`realpath`, `stat` per parent directory, `readFile` of `commondir`,
`realpath` again) before it calls `runWithGitOperationLock`, and the lane is
registered only after that walk resolves. For a non-existent `/repo` that is
five libuv threadpool round-trips per caller. Two callers issued back to back
run their chains concurrently, so lane order is threadpool completion order,
not call order.

When the `interactive` fetch won that race it entered the lane ahead of the
`background` fetch. On the first caller's release it reached admission
immediately and, being interactive, took the free network headroom slot instead
of queueing, while the background fetch stayed parked on the lock. `queued`
therefore settled at 0 and never reached the asserted 1. Measured inversion
rate for the bare lock-path walk was 54/500 on an idle machine; the test itself
failed 5/20 locally, always at the same assertion, matching the two CI failures
on unrelated PRs (#17530, #17630) at the same line.

Fix the premise rather than the symptom: stub only the key derivation, keeping
the real FIFO `runWithGitOperationLock` that the test actually exercises, so the
lane is registered synchronously with the call. Key derivation keeps its own
coverage in `src/shared/git-fetch-head-lock.test.ts`. This also stops the fetch
tests in this file from sharing one global `/.git/FETCH_HEAD` lane with each
other and from touching the real filesystem.

Verified deterministic: 40/40, then 30/30 clean runs, plus 25/25 with twelve CPU
hogs and a concurrent `src/main/git/command-runner/` run saturating the box.
2026-08-31 00:09:07 -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 96565fe370 perf(source-control): stop re-running every git read on each file selection (#15036) (#16600)
* perf(source-control): stop blocking main on four sync git-dir probes per status poll

detectConflictOperation ran four existsSync calls against the git dir on every
status poll. On a `\\wsl.localhost\...` worktree each one is a 9p round trip, and
being synchronous they landed on the Electron main thread back to back.

Replace them with concurrent fs/promises access probes: same "any failure reads
as absent" semantics existsSync had, one wave instead of four serialized blocking
calls. The outer try/catch went with them -- neither resolveGitDir nor the probes
can throw now, so it was unreachable.

Part of #15036 (source-control latency).

* perf(wsl): let git reads take the shell-free route from a cwd-derived distro

shouldAttemptWslDirectGit required options.wslDistro, so a `\\wsl.localhost\...`
worktree without a resolved WSL project runtime never qualified -- even though the
distro is right there in the cwd and wslDistroForCommand already knew how to read
it. Every `git show` behind a diff therefore ran through the user's login shell,
executing their rc once per blob read.

Three changes:

- Derive the distro from the cwd when no override was supplied. This is the fix;
  the routing decision now depends on where the repo actually lives.
- Wait, bounded, for a cold read-environment probe instead of resolving without it.
  The probe is one wsl.exe call shared per distro, so the wait is paid at most once,
  and past WSL_GIT_READ_ENVIRONMENT_WAIT_MS the shell route runs exactly as before.
  It returns null rather than a settled promise when there is nothing to wait for,
  so a non-WSL git call is not pushed into a later microtask.
- Opt the blob reads into preferWslDirectGit via gitReadOptionsForWorktree (renamed
  from gitStatusReadOptionsForWorktree; it was never status-specific). Belt-and-
  braces only: `show`, `config --get-regexp`, `ls-files` and `rev-parse` were all
  already matched by isWslDirectGitReadCommand, so this changes no routing today --
  it just stops the diff path depending on a heuristic it knows the answer to.

git-blob-read also gains a `failed` flag distinguishing "git ran and reported the
path absent" (exit 128) from "the read never got an answer"; nothing consumes it
yet, the settled diff cache does.

Part of #15036 (source-control latency).

* perf(source-control): give diff reads a settled cache keyed on stamped git state

gitDiffReadDedupe coalesces only while a read is in flight, so every file
selection re-ran the whole read: a `git config --file .gitmodules` spawn, one or
two `git show` spawns, and a working-tree stat+read. On a WSL/UNC worktree each
git spawn is a wsl.exe invocation, which is the ">3s Loading diff..." in #15036.

Correctness first -- a stale diff is worse than a slow one. The cache never
expires on a clock and there is no TTL to tune. Instead:

- worktree-diff-stamp.ts takes a subprocess-free stamp of exactly the inputs a
  file diff is built from: HEAD (by resolved tip *content*, so a commit is
  visible even though HEAD's own bytes never move), `.git/index` (mtime+size),
  `.gitmodules` (submodule routing), and the working-tree file. A linked
  worktree's commondir and the packed-refs/reftable fallback are handled; an
  unborn branch is caught by recording "no loose ref" rather than only the
  packed stamps.
- The stamp is captured BEFORE the read and stored with the result. Anything
  that moves during or after the read leaves the stored stamp behind, so the
  next lookup misses. That, not a freshness window, is why a stale diff cannot
  be served.
- A store is refused unless the stamp was taken a full mtime bucket (2s, FAT's
  granularity) after its newest component. Below that, a second write inside the
  same bucket would be invisible -- git's own racy-index rule.
- `null` stamp means "cannot prove" and never caches: a folder workspace, a repo
  whose layout cannot be read, or a filesystem reporting no usable mtime.
- Submodule routes and reads that failed rather than proved absence are not
  reusable. A wsl.exe hiccup produces the same empty left side a new file does,
  and pinning that would persist a wrong diff.
- invalidateGitReadCaches clears it and bumps a generation, so a read that
  started pre-mutation cannot store its result post-mutation.

`ino` is deliberately optional in the working-tree component: Windows reports 0
for it on the redirector behind `\\wsl.localhost`, and requiring an unstable 0 to
match would make the cache silently never hit on the exact host it exists for.
Cache counters are exposed for the same reason -- a miss storm and a cold start
otherwise look identical.

Also drops gitDiffReadDedupe.clear() from getStatus. A status poll is a read; all
it did was destroy a live coalescing entry so a concurrent identical request
started duplicate git work. Mutations still invalidate through the shared point.

Memory is bounded by retained characters, not entry count -- one diff result can
legitimately hold megabytes.

Fixes the source-control half of #15036.

* perf(source-control): reuse BoundedMap and stop the WSL probe wait from outliving its answer

Review follow-ups on the settled-diff-cache work:

- SettledDiffCache now sits on the shared BoundedMap instead of hand-rolling the
  same Map + character ledger + evict-oldest loop.
- pendingWslDirectGitReadEnvironment returns null once the probe has settled
  either way, so a distro whose direct route was disabled no longer pays for a
  1.5s timer and two microtask hops on every git read.
- That wait now honours the read's abort signal and goes through withTimeout, so
  an aborted read is not held for the full bound and a probe rejection can never
  surface as a read failure.
- The settled-cache generation fence is taken before the stamp read, so a
  mutation that lands entirely inside the stamp's stats can no longer store an
  entry whose stamp is torn across it.
- The cache counters are folded into the main-thread churn probe report, which is
  what tells a permanently-cold cache apart from a cold start in the field.

* fix(source-control): tell WSL clock skew apart from a genuinely fresh write

The racy-write margin compares two clocks: capturedAtMs is this host's, while
the component mtimes come from whatever wrote the files. On a \\wsl.localhost
worktree the guest sets them, so a guest running ahead pushes every
recently-touched file past the margin and the cache refuses to store — for as
long as the skew lasts, on exactly the platform this cache exists for.

Nothing was wrong with the refusal; it was invisible. racyWrites alone cannot
distinguish "the repo was just edited" from "the clocks disagree and this will
never resolve on its own", so a permanently cold cache looked like a cold start.

isDiffStampClockSkewed flags the one thing no local write can produce — an mtime
in this host's future — and the cache counts those separately as
clockSkewedWrites. A nonzero count is the signal that the cache is off for a
reason idling will not fix.

Found by review of #16600; behavior is unchanged, only observability.
2026-08-26 15:44:11 -07:00
Jinjing 5479bd9159 refactor(task-page): split task page into focused modules (#15163)
* rm unused files

* rm unused files

* fix(task-page): clean readiness lint findings

* Add GitLab IPC timeout wrapper and improve error handling

- Extract GitLab timeout logic into reusable `withGitLabIpcTimeout` wrapper to protect all GitLab API calls from hanging indefinitely
- Apply timeout protection to all GitLab list and fetch operations
- Add error handling for GitHub and Linear issue creation operations
- Fix event bubbling in GitHub work item row to prevent nested button clicks from opening detail page
- Remove unused `usePRReviewCellState` hook
- Consolidate redundant imports

* refactor(task-page): extract components and improve provider handling

- Add glab timeout handling (30s) to prevent IPC thread blocking
- Extract GitHub assignee/review components to dedicated files
- Improve GitLab work item row keying (repoId:id) and keyboard event handling
- Add context-aware error handling for Jira creation failures
- Refactor GitHubAssigneeAvatar to use shared GitHubUserAvatar component

* Add timeout support and error handling for GitLab operations

- Admission control times out queued work after 30s to prevent
  indefinite queueing behind saturated operations
- Mutation errors now display to users via toast instead of failing
  silently

* Consolidate workspace attachment labeling into unified utility

Extract common label-generation logic from GitHub and Linear
work-item components into a single getWorktreeAttachmentLabel
function, removing duplication across attachment types.

* Improve TaskPage accessibility, i18n coverage, and error handling

- Add missing aria-labels, roles, and semantic attributes for improved screen reader support
- Extract hardcoded UI strings into i18n system with translate() calls
- Add error handling and proper abort signal support for async operations
- Use locale-aware date formatting throughout
- Fix pagination disabled state and reviewer suggestion merging logic
- Improve async state management with proper refs and effects
- Add Textarea component import for Jira dialog

* Improve TaskPage accessibility and i18n key naming

- Add DialogTitle/Description with i18n to Linear issue dialog
- Use useId to improve aria-labelledby in GitHub selectors
- Replace hash-based i18n keys with semantic names
- Use Object.hasOwn instead of `in` for safer filter checks
- Fix PR review cell to clear input only on success

* Add missing dependencies to TaskPage hooks and useCallback/useEffect arr

Fixes exhaustive-deps warnings by adding missing setters, refs, and computed
values to dependency arrays. Refactors GitHub and Linear issue state handling
to compute values from pageData where available, with fallback to local state.
Moves imperative ref updates into useEffect to properly track dependencies.

* Fix TaskPage ref timing and null repo selection state

Treat null newIssueRepoId as a valid selection, and use useLayoutEffect to synchronize the provider context ref before paint rather than after.

* Extract Linear issue dialog components and fix popover scroll styling

- Consolidate scroll styling: apply popover-scroll-content and scrollbar-sleek classes to PopoverContent wrappers
- Remove redundant max-h-60 overflow-y-auto styles from inner picker divs
- Fix GitHub new issue repo selection to explicitly target first selected repo on fresh mount
- Correct CacheEntry import paths from store/slices/github to store/github/cache-model
- Update tests to reference extracted dialog components instead of TaskPage.tsx

* Improve GitHub task page i18n and fix issue creation edge cases

- Add i18n support to GitHub work item aria-labels (draft PR, PR, issue)
- Optimize work item row by extracting repeated source context call
- Add safety check to prevent opening detail page when issue URL is missing
- Fix dependency reference in detail opener hook
- Extend GitLab job trace timeouts (60s backend, 65s frontend) for slow logs

* Increase GitLab job trace fetch timeouts

Job traces can outlive the runner's 30-second default timeout.
Extend fetch operations to allow 60–65 seconds to complete.

* Verify sourceContext variable extraction in github row test

Update expectations to check that sourceContext is assigned to a
variable rather than called inline, matching the refactored component
implementation.
2026-08-25 21:28:21 -07:00
NeilandNeil 822087c8ec refactor(git): split runner.ts into focused command-runner modules (#16395)
* refactor(git): split runner.ts into focused command-runner modules

* chore(ratchets): repoint child_process and wsl.exe allowlists at the split modules

---------

Co-authored-by: Neil <n@example.com>
2026-08-25 02:34:11 -07:00