Commit Graph
5 Commits
Author SHA1 Message Date
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
OrcaWinandOrcaWin 6e8da1df8d fix(wsl): pass guest argv verbatim through --exec (#15039)
* fix(wsl): pass guest argv verbatim through --exec

`wsl.exe <...> -- <argv>` expands `$name` in every argument against the
guest environment before the guest ever runs. It does this even when no
shell is involved, so `-- /usr/bin/printf %s '$HOME'` prints /home/you.

Every WSL invocation went through that preprocessor, so scripts arrived
already rewritten: `awk '{print $2}'` lost its field reference, and a
POSIX script asking for the literal `$HOME` got the expanded path.

`escapeWslShCommandForWindows` tried to compensate by escaping `$`, but
it skipped any `$` preceded by a backslash, so a script containing `\$`
was still corrupted -- and half the call sites never applied it at all.

Route every invocation through `--exec`, which passes argv through
untouched, and delete the escaper. The direct-git path already used
`--exec`, so this is not a new compatibility dependency.

A guard test fails if the `--` form reappears anywhere in the tree.

Net -50 lines of production code.

* test(wsl): drop remaining escaped-dollar assertions

* fix(wsl): cover the --exec migration's blind spots

An audit of every wsl.exe invocation found sites the first pass missed,
including two it actively broke:

- config/scripts/wsl-git-shell-benchmark.mjs imported
  escapeWslShCommandForWindows, which no longer exists, so the script
  threw on startup. Its wslShellArgs helper also still used `--`; the
  file already had an --exec helper, so route both call sites there.
- classifySubprocessCommand unwrapped `wsl.exe <...> -- <binary>` by
  breaking on `--` alone. With every Orca spawn now on --exec it never
  found the guest binary and bucketed all WSL subprocesses as plain
  "wsl", losing the git/gh/glab breakdown. Break on either separator,
  since foreign wsl.exe processes still use `--`.

CliSkillRuntimeSetup builds its setup command as a template literal
rather than an argv array, so no array-shaped search could see it. Its
decoder accepts both separators so commands persisted before this
change still decode.

The guard now scans config/ and tests/ as well as src/, and checks the
command-string spelling alongside the argv one — the two shapes that
have each shipped a regression. It skips comment lines so prose about
the old form stays allowed, and asserts it scanned a plausible file
count so a bad root cannot make it vacuous.

* fix(wsl): restore the guard's multi-line sensitivity

The guard matched line by line, so `'--',\s*'bash'` could not span a
newline -- and every argv array in this repo is formatted one element
per line, which is exactly the shape it exists to catch. Measured
against the pre-migration tree it caught 17 files before and 9 fewer
after. It now strips comment lines and matches the rejoined text, with
a case that pins the multi-line shape so this cannot silently return.

The program list is wider than shells now, which surfaced a false
positive: tmux takes a `--` separator followed by a program too
(`split-window ... -- cat`). Matching is scoped to files that mention
WSL rather than narrowing the list back.

Also:
- Replaced the `sed` regression case, which was vacuous. A backreference
  contains no `$`, so it returned `bac` under both separators and would
  have passed without the fix. The block claimed every case proved the
  bug. Swapped in a positional argument and a shell local, both measured
  to differ -- the positional is the shape `wslUncDirectoryExists` uses,
  where `--` blanked `$1` so every existing directory probed as missing.
- windows-shell-args.test.ts derived its expected argv from
  buildWslExecArgs, the helper under test, so six assertions would still
  pass if it regressed to `--`. Spelled the expectation out.
- Dropped two comments citing the removed `--` behavior as rationale.

---------

Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
2026-08-18 02:12:21 -07:00
NeilandOrca aab112933e Revert "fix(memory): bound OOM-prone accumulators (#10179)" (#10255)
Co-authored-by: Orca <help@stably.ai>
2026-07-23 18:35:31 -07:00
Neil 8f40ddf328 fix(memory): bound OOM-prone accumulators (#10179) 2026-07-23 06:22:56 -07:00
Jinwoo HongandOrca 417723411e perf(source-control): stop gh rate-limit storms and idle git-status spawn churn (#7595)
Co-authored-by: Orca <help@stably.ai>
2026-07-06 15:12:56 -07:00