diff --git a/.t3-turbo/customizations.json b/.t3-turbo/customizations.json index 1d6eab2e2918..631f4572ef34 100644 --- a/.t3-turbo/customizations.json +++ b/.t3-turbo/customizations.json @@ -1407,6 +1407,91 @@ ] } ] + }, + { + "id": "startup-load-shedding", + "status": "implemented", + "summary": "Connection setup no longer competes with background repository work. Measured on 0.0.49 with 28 projects / 125 threads / 72 worktrees, trivial local HTTP GETs took 3-8.5s and the client's 15s CONNECTION_ESTABLISHMENT_TIMEOUT failed repeatedly. Four fork-owned changes: VcsStatusBroadcaster bounds automatic remote refreshes with a REMOTE_STATUS_REFRESH_CONCURRENCY=3 semaphore and holds them for a REMOTE_STATUS_STARTUP_GRACE of 90s after build (local status and user-triggered refresh unaffected, failure backoff unchanged); ThreadSettlementReactor drops its sweep fan-out from 8 to SETTLEMENT_SWEEP_CONCURRENCY=2 and gates the periodic sweep behind a 5-minute boot delay plus a 10-minute floor, while a settings change still sweeps at once; @t3tools/shared/shell caches explicit-path resolutions (hits only) in the shared CommandResolutionCache, memoizes the synchronous Windows spawn resolver, and drops the per-probe shell.isExecutableFile span; and ws.loadServerConfig reads editors through ExternalLauncher.availableEditorsSnapshot, which answers from the 60s cache or returns [] and warms on a detached fiber instead of blocking the snapshot on a PATH walk over every known editor.", + "checks": [ + { + "path": "apps/server/src/vcs/VcsStatusBroadcaster.ts", + "markers": [ + "REMOTE_STATUS_REFRESH_CONCURRENCY", + "REMOTE_STATUS_STARTUP_GRACE", + "RemoteStatusStartupGrace", + "remoteRefreshPermits", + "remainingStartupGrace" + ] + }, + { + "path": "apps/server/src/vcs/VcsStatusBroadcaster.test.ts", + "markers": [ + "holds automatic remote refresh for the startup grace", + "caps how many automatic remote refreshes run at once" + ] + }, + { + "path": "apps/server/src/orchestration/ThreadSettlementReactor.ts", + "markers": [ + "SETTLEMENT_SWEEP_CONCURRENCY", + "SETTLEMENT_SWEEP_MIN_INTERVAL", + "SETTLEMENT_SWEEP_BOOT_DELAY", + "claimAutomaticSweep", + "SweepTrigger" + ] + }, + { + "path": "apps/server/src/orchestration/ThreadSettlementReactor.test.ts", + "markers": [ + "holds automatic sweeps until the boot delay elapses", + "throttles automatic sweeps to the minimum interval", + "sweeps on a settings change without waiting out the boot delay" + ] + }, + { + "path": "packages/shared/src/shell.ts", + "markers": [ + "COMMAND_RESOLUTION_EXPLICIT_PATH_KEY", + "SPAWN_EXECUTABLE_CACHE_TTL_NANOS", + "spawnExecutableCache", + "scanSpawnExecutableWithNode" + ] + }, + { + "path": "packages/shared/src/shell.test.ts", + "markers": [ + "serves a repeat explicit-path resolve without stating again", + "does not cache an explicit path that is missing", + "keys cached explicit paths per command", + "re-probes an explicit path once the 30s TTL expires" + ] + }, + { + "path": "apps/server/src/process/externalLauncher.ts", + "markers": [ + "availableEditorsSnapshot", + "warmAvailableEditors", + "freshCachedEditors", + "EMPTY_AVAILABLE_EDITORS" + ] + }, + { + "path": "apps/server/src/process/externalLauncher.test.ts", + "markers": [ + "answers a cold editor snapshot immediately without scanning inline", + "serves the discovered editors once the backgrounded warm completes", + "collapses a burst of cold snapshots into a single scan" + ] + }, + { + "path": "apps/server/src/ws.ts", + "markers": ["availableEditorsSnapshot"] + }, + { + "path": "apps/server/src/server.test.ts", + "markers": ["returns server config without waiting on a cold editor scan"] + } + ] } ] } diff --git a/SEAM.md b/SEAM.md index 28432a442d51..363b4013f447 100644 --- a/SEAM.md +++ b/SEAM.md @@ -500,6 +500,63 @@ On a nightly-sync conflict: take upstream's file, then re-add `ProjectionApplyCo `shouldRefreshThreadShellSummary` gate), `runProjectorBatch`, and the paging `bootstrapProjector`. Both test files must pass. +## Startup load shedding (`startup-load-shedding`) + +Background repository work used to run flat out while a client was still connecting, and on a large +install it starved the event loop the connection setup itself needs. Measured live on 0.0.49, one +backend, 28 projects / 125 threads / 72 live worktrees: + +| Symptom | Measurement (90s trace) | +| ---------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------- | +| Trivial local HTTP GET | 3-8.5s, so the client's 15s `CONNECTION_ESTABLISHMENT_TIMEOUT` (`packages/client-runtime/src/connection/supervisor.ts`) failed repeatedly | +| `VcsStatusBroadcaster.refreshRemoteStatus` | ~25 concurrent spans, 24-30s each, from `concurrency: "unbounded"` and one poller per worktree | +| `ThreadSettlementReactor.sweep` | 33s, `concurrency: 8`, one `gh pr list` per unsettled thread, re-run every minute | +| `checkClaudeProviderStatus` / `discoverClaudeSkills` | 30s / 25s | +| `shell` command resolution | 572 `runGitCommand` + 173 `shell.resolveSpawnCommand`, ~15k synchronous `shell.isExecutableFile` stats per trace rotation | +| `loadServerConfig` (`apps/server/src/ws.ts`) | `resolveAvailableEditors` on every `subscribeServerConfig` snapshot: 21 editors x PATHEXT x PATH, up to the 5s `CONFIG_DISCOVERY_TIMEOUT` | + +The fix sheds load at four points and changes no product behavior: + +- **`apps/server/src/vcs/VcsStatusBroadcaster.ts`** - automatic remote refreshes take a permit from a + `Semaphore.make(REMOTE_STATUS_REFRESH_CONCURRENCY)` (3), and `remainingStartupGrace` holds them for + `REMOTE_STATUS_STARTUP_GRACE` (90s) after the broadcaster is built, returning the remaining grace as + the poller's next delay so nothing is dropped. Local status, `getStatus`, and `refreshStatus` are + untouched, as is the exponential failure backoff. `RemoteStatusStartupGrace` is a + `Context.Reference` so tests can zero the grace. +- **`apps/server/src/orchestration/ThreadSettlementReactor.ts`** - `SETTLEMENT_SWEEP_CONCURRENCY` (2) + replaces the fan-out of 8, and `claimAutomaticSweep` gates the periodic sweep behind + `SETTLEMENT_SWEEP_BOOT_DELAY` (5 min) and `SETTLEMENT_SWEEP_MIN_INTERVAL` (10 min). The worker + payload is a `SweepTrigger`; only `"periodic"` is throttled, so a settings change still sweeps + immediately. Settlement semantics (`ThreadSettlementPolicy`) are unchanged. +- **`packages/shared/src/shell.ts`** - the explicit-path branch of `resolveCommandPathForPlatform` + now reads and writes the shared `CommandResolutionCache` under + `COMMAND_RESOLUTION_EXPLICIT_PATH_KEY`, storing hits only so a just-written binary is never masked + by a stale negative; `resolveSpawnExecutableWithNode` memoizes its synchronous scan + (`spawnExecutableCache`, `scanSpawnExecutableWithNode`, hits only, 30s, keyed on + platform + PATH + PATHEXT + command); and `isExecutableFile` is no longer an `Effect.fn`, so one + span per resolution replaces tens of thousands per connect. +- **`apps/server/src/process/externalLauncher.ts` + `apps/server/src/ws.ts`** - + `availableEditorsSnapshot` answers from the existing 60s `editorDiscoveryCache` when it is fresh and + otherwise returns `[]` at once while `warmAvailableEditors` (semaphore-deduped) fills the cache on a + detached fiber. Because the warm is detached it cannot be interrupted by a client timeout, which is + what previously left the cache cold on every connect. Accepted trade-off: on a genuinely cold cache + the first snapshot advertises no editors and they appear on the next one - there is no full-config + re-emit hook on `subscribeServerConfig` (only per-field deltas), and adding one would need a + contract change across web, desktop, and mobile. + +Related upstream work: pingdotgg/t3code#7231 and #7233. + +Deliberately **not** done: switching the desktop LAN bind from `0.0.0.0` to a dual-stack `::` +(`apps/desktop/src/backend/DesktopServerExposure.ts`). It is unrelated to the measured CPU +starvation, `listen("::")` hard-fails on hosts with IPv6 disabled where `0.0.0.0` always works, and +the WSL backend's wildcard bind carries its own documented forwarding rationale. + +On a nightly-sync conflict: take upstream's file, then re-add the named constants above and their +call sites. The two easy regressions are upstream reverting a `concurrency` back to `"unbounded"`/`8` +and `loadServerConfig` going back to `resolveAvailableEditorsForConfig(resolveAvailableEditors())` - +`resolveAvailableEditorsForConfig` is still used for `remoteOpenTargets`, so its presence is not +evidence the seam survived. + ## Nightly sync conflicts Resolve against the new upstream file first, then reapply only the behavior above; never take the diff --git a/apps/desktop/package.json b/apps/desktop/package.json index fc8f572e5d17..32035467933b 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -1,6 +1,6 @@ { "name": "@t3tools/desktop", - "version": "0.0.49", + "version": "0.0.50", "private": true, "type": "module", "main": "dist-electron/main.cjs", diff --git a/apps/server/package.json b/apps/server/package.json index ed0210dd5579..9aa21a5d2328 100644 --- a/apps/server/package.json +++ b/apps/server/package.json @@ -1,6 +1,6 @@ { "name": "t3", - "version": "0.0.49", + "version": "0.0.50", "license": "MIT", "repository": { "type": "git", diff --git a/apps/server/src/orchestration/ThreadSettlementReactor.test.ts b/apps/server/src/orchestration/ThreadSettlementReactor.test.ts index 5d8d47109faa..781a836f9013 100644 --- a/apps/server/src/orchestration/ThreadSettlementReactor.test.ts +++ b/apps/server/src/orchestration/ThreadSettlementReactor.test.ts @@ -37,6 +37,8 @@ import { ProjectionSnapshotQuery } from "./Services/ProjectionSnapshotQuery.ts"; import * as ThreadSettlementReactor from "./ThreadSettlementReactor.ts"; const NOW = "2026-08-28T12:00:00.000Z"; +/** Mirrors `SETTLEMENT_SWEEP_BOOT_DELAY`: automatic sweeps stay quiet this long. */ +const SWEEP_BOOT_DELAY = "5 minutes"; const PROJECT_ID = ProjectId.make("settlement-project"); const LINKED_PROJECT_ID = ProjectId.make("linked-settlement-project"); @@ -273,6 +275,9 @@ const startHarness = Effect.fn("startThreadSettlementHarness")(function* ( activation: Deferred.Deferred, snapshotReads: Queue.Queue, ) { + // Automatic sweeps are muted for the boot delay. Step past it before starting + // so the periodic tick fired by `start()` is allowed to sweep straight away. + yield* TestClock.adjust(SWEEP_BOOT_DELAY); yield* reactor.start(); yield* Deferred.succeed(activation, undefined); yield* Queue.take(snapshotReads); @@ -316,6 +321,7 @@ describe("ThreadSettlementReactor", () => { yield* Effect.gen(function* () { const reactor = yield* ThreadSettlementReactor.ThreadSettlementReactor; + yield* TestClock.adjust(SWEEP_BOOT_DELAY); yield* reactor.start(); assert.strictEqual(yield* Ref.get(fixture.snapshotReadCount), 0); @@ -350,15 +356,17 @@ describe("ThreadSettlementReactor", () => { ), ); - it.effect("reevaluates inactivity and pull request state once per minute", () => + it.effect("reevaluates inactivity and pull request state on the next automatic sweep", () => Effect.scoped( Effect.gen(function* () { yield* TestClock.setTime(Date.parse(NOW)); const pullRequest = yield* Ref.make<"open" | "merged">("open"); const fixture = yield* makeHarness({ snapshot: makeSnapshot([ + // Exactly the inactivity threshold at the first sweep, so it must not + // settle then, and past it by the time the next sweep runs. makeThread("at-boundary", { - latestUserMessageAt: "2026-08-25T12:00:00.000Z", + latestUserMessageAt: "2026-08-25T12:05:00.000Z", }), makeThread("open-pr", { branch: "saved-feature", @@ -375,6 +383,12 @@ describe("ThreadSettlementReactor", () => { assert.deepStrictEqual(yield* Ref.get(fixture.commands), []); yield* Ref.set(pullRequest, "merged"); + // One periodic tick fires per clock adjustment, so cross the minimum + // interval in two steps: the ticks before it must not sweep. + yield* TestClock.adjust("9 minutes"); + yield* reactor.drain; + assert.strictEqual(yield* Ref.get(fixture.snapshotReadCount), 1); + yield* TestClock.adjust("1 minute"); yield* Queue.take(fixture.snapshotReads); yield* reactor.drain; @@ -431,6 +445,7 @@ describe("ThreadSettlementReactor", () => { yield* Effect.gen(function* () { const reactor = yield* ThreadSettlementReactor.ThreadSettlementReactor; + yield* TestClock.adjust(SWEEP_BOOT_DELAY); yield* reactor.start(); yield* Deferred.succeed(fixture.activation, undefined); yield* Queue.take(fixture.snapshotReads); @@ -637,4 +652,90 @@ describe("ThreadSettlementReactor", () => { }), ), ); + + it.effect("holds automatic sweeps until the boot delay elapses", () => + Effect.scoped( + Effect.gen(function* () { + yield* TestClock.setTime(Date.parse(NOW)); + const fixture = yield* makeHarness({ + snapshot: makeSnapshot([makeThread("inactive")]), + }); + + yield* Effect.gen(function* () { + const reactor = yield* ThreadSettlementReactor.ThreadSettlementReactor; + yield* reactor.start(); + yield* Deferred.succeed(fixture.activation, undefined); + + // Five periodic ticks land inside the boot window and none may sweep. + yield* TestClock.adjust("4 minutes"); + yield* reactor.drain; + assert.strictEqual(yield* Ref.get(fixture.snapshotReadCount), 0); + assert.deepStrictEqual(yield* Ref.get(fixture.commands), []); + + yield* TestClock.adjust("1 minute"); + assert.strictEqual(yield* Queue.take(fixture.snapshotReads), 1); + yield* reactor.drain; + assert.deepStrictEqual( + (yield* Ref.get(fixture.commands)).map((command) => command.threadId), + [ThreadId.make("inactive")], + ); + }).pipe(Effect.provide(fixture.layer)); + }), + ), + ); + + it.effect("throttles automatic sweeps to the minimum interval", () => + Effect.scoped( + Effect.gen(function* () { + yield* TestClock.setTime(Date.parse(NOW)); + const fixture = yield* makeHarness({ + snapshot: makeSnapshot([makeThread("inactive")]), + }); + + yield* Effect.gen(function* () { + const reactor = yield* ThreadSettlementReactor.ThreadSettlementReactor; + yield* startHarness(reactor, fixture.activation, fixture.snapshotReads); + assert.strictEqual(yield* Ref.get(fixture.snapshotReadCount), 1); + + // Nine more periodic ticks, all inside the minimum interval. + yield* TestClock.adjust("9 minutes"); + yield* reactor.drain; + assert.strictEqual(yield* Ref.get(fixture.snapshotReadCount), 1); + + yield* TestClock.adjust("1 minute"); + assert.strictEqual(yield* Queue.take(fixture.snapshotReads), 2); + yield* reactor.drain; + assert.strictEqual(yield* Ref.get(fixture.snapshotReadCount), 2); + }).pipe(Effect.provide(fixture.layer)); + }), + ), + ); + + it.effect("sweeps on a settings change without waiting out the boot delay", () => + Effect.scoped( + Effect.gen(function* () { + yield* TestClock.setTime(Date.parse(NOW)); + const fixture = yield* makeHarness({ + snapshot: makeSnapshot([makeThread("inactive")]), + }); + + yield* Effect.gen(function* () { + const reactor = yield* ThreadSettlementReactor.ThreadSettlementReactor; + yield* reactor.start(); + yield* Deferred.succeed(fixture.activation, undefined); + yield* TestClock.adjust("4 minutes"); + yield* reactor.drain; + assert.strictEqual(yield* Ref.get(fixture.snapshotReadCount), 0); + + yield* fixture.updateSettings({ sidebarAutoSettleAfterDays: 4 }); + assert.strictEqual(yield* Queue.take(fixture.snapshotReads), 1); + yield* reactor.drain; + assert.deepStrictEqual( + (yield* Ref.get(fixture.commands)).map((command) => command.threadId), + [ThreadId.make("inactive")], + ); + }).pipe(Effect.provide(fixture.layer)); + }), + ), + ); }); diff --git a/apps/server/src/orchestration/ThreadSettlementReactor.ts b/apps/server/src/orchestration/ThreadSettlementReactor.ts index fd4486a9c406..9354077f75e4 100644 --- a/apps/server/src/orchestration/ThreadSettlementReactor.ts +++ b/apps/server/src/orchestration/ThreadSettlementReactor.ts @@ -1,11 +1,14 @@ import { CommandId } from "@t3tools/contracts"; import { makeDrainableWorker } from "@t3tools/shared/DrainableWorker"; import * as Cause from "effect/Cause"; +import * as Clock from "effect/Clock"; import * as Context from "effect/Context"; import * as Crypto from "effect/Crypto"; import * as DateTime from "effect/DateTime"; +import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; +import * as Ref from "effect/Ref"; import * as Schedule from "effect/Schedule"; import type * as Scope from "effect/Scope"; import * as Stream from "effect/Stream"; @@ -22,6 +25,30 @@ import { type SettlementPullRequest, } from "./ThreadSettlementPolicy.ts"; +/** + * Ceiling on how many settlement groups are resolved at once. Each group costs + * a `gh pr list` subprocess, so the old fan-out of 8 put eight GitHub CLI + * processes on the CPU during boot, competing with connection setup. + */ +const SETTLEMENT_SWEEP_CONCURRENCY = 2; +/** + * Floor between two automatic sweeps. Auto-settlement is a day-scale decision; + * the periodic tick used to re-walk every unsettled thread once a minute. + */ +const SETTLEMENT_SWEEP_MIN_INTERVAL = Duration.minutes(10); +/** + * How long automatic sweeps stay quiet after the reactor is built, so the + * first `gh pr list` storm does not land inside the client's connection-setup + * budget. A settings change still sweeps immediately. + */ +const SETTLEMENT_SWEEP_BOOT_DELAY = Duration.minutes(5); + +/** + * Why a sweep was queued. Only `"periodic"` is throttled - a settings change is + * a user action and must take effect straight away. + */ +type SweepTrigger = "periodic" | "settings-changed"; + export class ThreadSettlementReactor extends Context.Service< ThreadSettlementReactor, { @@ -37,6 +64,27 @@ export const make = Effect.gen(function* () { const git = yield* GitManager.GitManager; const pullRequests = yield* PullRequestService.PullRequestService; const crypto = yield* Crypto.Crypto; + const builtAtNanos = yield* Clock.currentTimeNanos; + const lastSweepAtNanos = yield* Ref.make(null); + + /** + * True when an automatic sweep is allowed to run now: past the boot delay, + * and at least {@link SETTLEMENT_SWEEP_MIN_INTERVAL} since the last one. + * Records the run time as a side effect so the caller cannot forget to. + */ + const claimAutomaticSweep = Effect.fn("ThreadSettlementReactor.claimAutomaticSweep")( + function* () { + const nowNanos = yield* Clock.currentTimeNanos; + const lastNanos = yield* Ref.get(lastSweepAtNanos); + const earliestNanos = + lastNanos === null + ? builtAtNanos + BigInt(Duration.toMillis(SETTLEMENT_SWEEP_BOOT_DELAY)) * 1_000_000n + : lastNanos + BigInt(Duration.toMillis(SETTLEMENT_SWEEP_MIN_INTERVAL)) * 1_000_000n; + if (nowNanos < earliestNanos) return false; + yield* Ref.set(lastSweepAtNanos, nowNanos); + return true; + }, + ); const sweep = Effect.fn("ThreadSettlementReactor.sweep")(function* () { const snapshot = yield* snapshots.getShellSnapshot(); @@ -135,12 +183,17 @@ export const make = Effect.gen(function* () { }), ), ), - { concurrency: 8, discard: true }, + { concurrency: SETTLEMENT_SWEEP_CONCURRENCY, discard: true }, ); }); - const worker = yield* makeDrainableWorker(() => - sweep().pipe( + const worker = yield* makeDrainableWorker((trigger: SweepTrigger) => + Effect.gen(function* () { + if (trigger === "periodic" && !(yield* claimAutomaticSweep())) { + return; + } + yield* sweep(); + }).pipe( Effect.catchCause((cause) => Cause.hasInterruptsOnly(cause) ? Effect.failCause(cause) @@ -160,7 +213,7 @@ export const make = Effect.gen(function* () { let lastOnMerge = initialSettings.sidebarAutoSettleOnMerge; yield* forkParked( Effect.gen(function* () { - yield* worker.enqueue(undefined); + yield* worker.enqueue("periodic"); yield* worker.drain; }).pipe(Effect.repeat(Schedule.spaced("1 minute")), Effect.asVoid), ); @@ -174,7 +227,7 @@ export const make = Effect.gen(function* () { } lastAfterDays = settings.sidebarAutoSettleAfterDays; lastOnMerge = settings.sidebarAutoSettleOnMerge; - return worker.enqueue(undefined); + return worker.enqueue("settings-changed"); }), ); }); diff --git a/apps/server/src/process/externalLauncher.test.ts b/apps/server/src/process/externalLauncher.test.ts index 16583ac7daa7..4a96f42b9c65 100644 --- a/apps/server/src/process/externalLauncher.test.ts +++ b/apps/server/src/process/externalLauncher.test.ts @@ -970,6 +970,139 @@ it.effect("rescans after an interrupted discovery instead of caching the interru ); }); +// `availableEditorsSnapshot` is the non-blocking view `server.getConfig` uses: +// it must answer from the memoized set or answer empty right away, never make +// the calling fiber wait on a PATH walk across every known editor. +const countingDiscoveryLayer = (onStat: () => void, env: Record) => { + const fileInfo = { type: "File" } as FileSystem.File.Info; + return Layer.mergeAll( + ExternalLauncher.layer.pipe( + Layer.provide( + Layer.mergeAll( + FileSystem.layerNoop({ + stat: () => + Effect.sync(() => { + onStat(); + return fileInfo; + }), + }), + Path.layer, + Layer.succeed( + ChildProcessSpawner.ChildProcessSpawner, + ChildProcessSpawner.make(() => Effect.sync(() => makeMockDetachedHandle())), + ), + ), + ), + ), + Layer.succeed(HostProcessPlatform, "win32"), + ConfigProvider.layer( + ConfigProvider.fromEnv({ env: { PATHEXT: ".COM;.EXE;.BAT;.CMD", ...env } }), + ), + ); +}; + +// Hands the scheduler to the detached warm fiber until it publishes a set. +// Yielding, never sleeping: the warm runs entirely on the same runtime. +const settledEditorsSnapshot = (launcher: ExternalLauncher.ExternalLauncher["Service"]) => + Effect.gen(function* () { + for (let attempt = 0; attempt < 100; attempt += 1) { + const editors = yield* launcher.availableEditorsSnapshot(); + if (editors.length > 0) return editors; + yield* Effect.yieldNow; + } + return yield* launcher.availableEditorsSnapshot(); + }); + +it.effect("answers a cold editor snapshot immediately without scanning inline", () => { + let statCalls = 0; + return Effect.gen(function* () { + const launcher = yield* ExternalLauncher.ExternalLauncher; + + const editors = yield* launcher.availableEditorsSnapshot(); + + assert.deepEqual([...editors], []); + // The scan is forked; nothing walked PATH on the calling fiber. + assert.equal(statCalls, 0); + }).pipe( + Effect.provide( + countingDiscoveryLayer( + () => { + statCalls += 1; + }, + { PATH: "C:\\t3-editor-snapshot-cold-test" }, + ), + ), + ); +}); + +it.effect("serves the discovered editors once the backgrounded warm completes", () => { + let statCalls = 0; + return Effect.gen(function* () { + const launcher = yield* ExternalLauncher.ExternalLauncher; + + assert.deepEqual([...(yield* launcher.availableEditorsSnapshot())], []); + + const editors = yield* settledEditorsSnapshot(launcher); + + assert.equal(editors.includes("vscode"), true); + assert.isAbove(statCalls, 0); + }).pipe( + Effect.provide( + countingDiscoveryLayer( + () => { + statCalls += 1; + }, + { PATH: "C:\\t3-editor-snapshot-warm-test" }, + ), + ), + ); +}); + +// A burst of connects on a cold cache each fork a warm. The warm permit plus +// its cache re-check must collapse them into a single PATH walk. +it.effect("collapses a burst of cold snapshots into a single scan", () => { + let statCalls = 0; + return Effect.gen(function* () { + const launcher = yield* ExternalLauncher.ExternalLauncher; + + const burst = yield* Effect.all( + Array.from({ length: 8 }, () => launcher.availableEditorsSnapshot()), + { concurrency: "unbounded" }, + ); + for (const editors of burst) { + assert.deepEqual([...editors], []); + } + + const editors = yield* settledEditorsSnapshot(launcher); + assert.equal(editors.includes("vscode"), true); + // Let any queued warm run; each must find the cache fresh and no-op. + yield* Effect.yieldNow; + yield* Effect.yieldNow; + const statCallsAfterBurst = statCalls; + assert.isAbove(statCallsAfterBurst, 0); + + // Past both the discovery window and the shared command-resolution cache, + // one blocking scan measures what a single PATH walk costs. + yield* TestClock.adjust("61 seconds"); + yield* launcher.resolveAvailableEditors(); + const singleScanStatCalls = statCalls - statCallsAfterBurst; + + assert.equal(statCallsAfterBurst, singleScanStatCalls); + }).pipe( + Effect.provide( + Layer.merge( + countingDiscoveryLayer( + () => { + statCalls += 1; + }, + { PATH: "C:\\t3-editor-snapshot-burst-test" }, + ), + TestClock.layer(), + ), + ), + ); +}); + it.effect("rejects unknown editors through the service API", () => Effect.gen(function* () { const launcher = yield* ExternalLauncher.ExternalLauncher; diff --git a/apps/server/src/process/externalLauncher.ts b/apps/server/src/process/externalLauncher.ts index a0b4115d0bf1..3b0d60dd7af2 100644 --- a/apps/server/src/process/externalLauncher.ts +++ b/apps/server/src/process/externalLauncher.ts @@ -32,6 +32,7 @@ import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; import * as Path from "effect/Path"; import * as Ref from "effect/Ref"; +import * as Semaphore from "effect/Semaphore"; import * as Stream from "effect/Stream"; import * as ChildProcess from "effect/unstable/process/ChildProcess"; import * as ChildProcessSpawner from "effect/unstable/process/ChildProcessSpawner"; @@ -494,6 +495,8 @@ interface EditorDiscoveryCacheEntry { readonly expiresAtNanos: bigint; } +const EMPTY_AVAILABLE_EDITORS: ReadonlyArray = []; + /** * ExternalLauncher - Service tag for browser/editor launch operations. */ @@ -501,6 +504,14 @@ export class ExternalLauncher extends Context.Service< ExternalLauncher, { readonly resolveAvailableEditors: () => Effect.Effect>; + /** + * Non-blocking view of {@link resolveAvailableEditors} for the server-config + * snapshot. Returns the memoized set when it is fresh; when the cache is + * cold it returns an empty set immediately and warms the cache on a + * detached fiber, so connection setup never pays for a PATH walk across + * every known editor. Editors then appear on the next snapshot. + */ + readonly availableEditorsSnapshot: () => Effect.Effect>; /** * Reveal kind for the host, or undefined when the executable a reveal * actually spawns is unavailable. Only meaningful when @@ -815,8 +826,37 @@ export const make = Effect.gen(function* () { return editors; }); + const freshCachedEditors = Effect.gen(function* () { + const nowNanos = yield* Clock.currentTimeNanos; + const entry = yield* Ref.get(editorDiscoveryCache); + return Option.isSome(entry) && entry.value.expiresAtNanos > nowNanos + ? Option.some(entry.value.editors) + : Option.none>(); + }); + + // One warm at a time: a burst of connects on a cold cache would otherwise + // each fork a full scan. The permit holder re-checks the cache so the + // queued warms become no-ops instead of repeat scans. + const warmPermit = yield* Semaphore.make(1); + const warmAvailableEditors = warmPermit.withPermits(1)( + Effect.gen(function* () { + if (Option.isSome(yield* freshCachedEditors)) return; + yield* cachedAvailableEditors; + }), + ); + + const availableEditorsSnapshot = Effect.gen(function* () { + const cached = yield* freshCachedEditors; + if (Option.isSome(cached)) return cached.value; + // Detached, so the caller's timeout or disconnect cannot interrupt the + // scan halfway and leave the cache cold for the next connect too. + yield* warmAvailableEditors.pipe(Effect.ignoreCause({ log: true }), Effect.forkDetach); + return EMPTY_AVAILABLE_EDITORS; + }); + return ExternalLauncher.of({ resolveAvailableEditors: () => cachedAvailableEditors, + availableEditorsSnapshot: () => availableEditorsSnapshot, resolveFileManagerRevealKind: () => provideCommandResolutionServices(resolveFileManagerRevealKind()).pipe( Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner), diff --git a/apps/server/src/server.test.ts b/apps/server/src/server.test.ts index bc1c240e6317..f72c45e52980 100644 --- a/apps/server/src/server.test.ts +++ b/apps/server/src/server.test.ts @@ -738,6 +738,7 @@ const buildAppUnderTest = (options?: { Layer.mergeAll( Layer.mock(ExternalLauncher.ExternalLauncher)({ resolveAvailableEditors: () => Effect.succeed([]), + availableEditorsSnapshot: () => Effect.succeed([]), resolveFileManagerRevealKind: () => Effect.sync((): undefined => undefined), ...options?.layers?.externalLauncher, }), @@ -4236,6 +4237,7 @@ it.layer(NodeServices.layer)("server router seam", (it) => { layers: { externalLauncher: { resolveAvailableEditors: () => Effect.succeed(["file-manager"]), + availableEditorsSnapshot: () => Effect.succeed(["file-manager"]), resolveFileManagerRevealKind: () => Effect.succeed("file-explorer"), }, }, @@ -4256,6 +4258,37 @@ it.layer(NodeServices.layer)("server router seam", (it) => { }).pipe(Effect.provide(NodeHttpServer.layerTest)), ); + // The config snapshot must read the launcher's non-blocking view. If it ever + // falls back to the blocking scan this test hangs: the mock's blocking scan + // never resolves and nothing here advances the clock past the 5s discovery + // timeout that would otherwise rescue it. + it.effect("returns server config without waiting on a cold editor scan", () => + Effect.gen(function* () { + yield* buildAppUnderTest({ + layers: { + externalLauncher: { + resolveAvailableEditors: () => Effect.never, + availableEditorsSnapshot: () => Effect.succeed([]), + resolveFileManagerRevealKind: () => Effect.never, + }, + }, + }); + + const { cookie } = yield* bootstrapBrowserSession(); + const wsUrl = appendSessionCookieToWsUrl( + yield* getWsServerUrl("/ws", { authenticated: false }), + cookie?.split(";")[0] ?? "", + ); + const response = yield* Effect.scoped( + withWsRpcClient(wsUrl, (client) => client[WS_METHODS.serverGetConfig]({})), + ); + + assert.deepEqual(response.availableEditors, []); + assert.isUndefined(response.shellRevealInFileManager); + assert.isUndefined(response.shellRevealInFileManagerKind); + }).pipe(Effect.provide(NodeHttpServer.layerTest)), + ); + it.effect("does not block server config when editor discovery never resolves", () => Effect.gen(function* () { const discoveryInterrupted = yield* Deferred.make(); diff --git a/apps/server/src/vcs/VcsStatusBroadcaster.test.ts b/apps/server/src/vcs/VcsStatusBroadcaster.test.ts index 6820a29e2c86..a250d7cf7486 100644 --- a/apps/server/src/vcs/VcsStatusBroadcaster.test.ts +++ b/apps/server/src/vcs/VcsStatusBroadcaster.test.ts @@ -24,6 +24,7 @@ import type { import { GitManagerError } from "@t3tools/contracts"; import * as VcsStatusBroadcaster from "./VcsStatusBroadcaster.ts"; +import { REMOTE_STATUS_REFRESH_CONCURRENCY } from "./VcsStatusBroadcaster.ts"; import * as BackgroundPolicy from "../background/BackgroundPolicy.ts"; import * as GitWorkflowService from "../git/GitWorkflowService.ts"; @@ -67,16 +68,20 @@ const baseStatus: VcsStatusResult = { ...baseRemoteStatus, }; -function makeTestLayer(state: { - currentLocalStatus: VcsStatusLocalResult; - currentRemoteStatus: VcsStatusRemoteResult | null; - localStatusCalls: number; - remoteStatusCalls: number; - localInvalidationCalls: number; - remoteInvalidationCalls: number; - remoteStatusRefreshUpstreamValues?: Array; -}) { +function makeTestLayer( + state: { + currentLocalStatus: VcsStatusLocalResult; + currentRemoteStatus: VcsStatusRemoteResult | null; + localStatusCalls: number; + remoteStatusCalls: number; + localInvalidationCalls: number; + remoteInvalidationCalls: number; + remoteStatusRefreshUpstreamValues?: Array; + }, + startupGrace: Duration.Duration = Duration.millis(0), +) { return VcsStatusBroadcaster.layer.pipe( + Layer.provide(Layer.succeed(VcsStatusBroadcaster.RemoteStatusStartupGrace, startupGrace)), Layer.provideMerge(NodeServices.layer), Layer.provide(makeBackgroundPolicyLayer(() => true)), Layer.provide( @@ -219,6 +224,9 @@ describe("VcsStatusBroadcaster", () => { failRemoteStatus: false, }; const testLayer = VcsStatusBroadcaster.layer.pipe( + Layer.provide( + Layer.succeed(VcsStatusBroadcaster.RemoteStatusStartupGrace, Duration.millis(0)), + ), Layer.provideMerge(NodeServices.layer), Layer.provide(makeBackgroundPolicyLayer(() => true)), Layer.provide( @@ -327,6 +335,9 @@ describe("VcsStatusBroadcaster", () => { remoteInvalidationCalls: 0, }; const testLayer = VcsStatusBroadcaster.layer.pipe( + Layer.provide( + Layer.succeed(VcsStatusBroadcaster.RemoteStatusStartupGrace, Duration.millis(0)), + ), Layer.provideMerge(NodeServices.layer), Layer.provide(makeBackgroundPolicyLayer(() => true)), Layer.provide( @@ -491,6 +502,9 @@ describe("VcsStatusBroadcaster", () => { }); let firstRemoteAttemptDeferred: Deferred.Deferred | null = null; const testLayer = VcsStatusBroadcaster.layer.pipe( + Layer.provide( + Layer.succeed(VcsStatusBroadcaster.RemoteStatusStartupGrace, Duration.millis(0)), + ), Layer.provideMerge(NodeServices.layer), Layer.provide(makeBackgroundPolicyLayer(() => true)), Layer.provide( @@ -691,6 +705,9 @@ describe("VcsStatusBroadcaster", () => { remoteInvalidationCalls: 0, }; const testLayer = VcsStatusBroadcaster.layer.pipe( + Layer.provide( + Layer.succeed(VcsStatusBroadcaster.RemoteStatusStartupGrace, Duration.millis(0)), + ), Layer.provideMerge(NodeServices.layer), Layer.provide(makeBackgroundPolicyLayer(() => false)), Layer.provide( @@ -744,6 +761,9 @@ describe("VcsStatusBroadcaster", () => { let remoteInterruptedDeferred: Deferred.Deferred | null = null; let remoteStartedDeferred: Deferred.Deferred | null = null; const testLayer = VcsStatusBroadcaster.layer.pipe( + Layer.provide( + Layer.succeed(VcsStatusBroadcaster.RemoteStatusStartupGrace, Duration.millis(0)), + ), Layer.provideMerge(NodeServices.layer), Layer.provide(makeBackgroundPolicyLayer(() => true)), Layer.provide( @@ -817,4 +837,101 @@ describe("VcsStatusBroadcaster", () => { assert.isTrue(Option.isSome(yield* Deferred.poll(remoteInterrupted))); }).pipe(Effect.provide(testLayer)); }); + + it.effect("holds automatic remote refresh for the startup grace", () => { + const state = { + currentLocalStatus: baseLocalStatus, + currentRemoteStatus: baseRemoteStatus, + localStatusCalls: 0, + remoteStatusCalls: 0, + localInvalidationCalls: 0, + remoteInvalidationCalls: 0, + }; + + return Effect.gen(function* () { + const broadcaster = yield* VcsStatusBroadcaster.VcsStatusBroadcaster; + const scope = yield* Scope.make(); + const snapshotDeferred = yield* Deferred.make(); + yield* Stream.runForEach( + broadcaster.streamStatus( + { cwd: "/repo" }, + { automaticRemoteRefreshInterval: Effect.succeed(Duration.seconds(30)) }, + ), + (event) => + event._tag === "snapshot" + ? Deferred.succeed(snapshotDeferred, event).pipe(Effect.ignore) + : Effect.void, + ).pipe(Effect.forkIn(scope)); + + // Local status is never held back - only the remote half is. + yield* Deferred.await(snapshotDeferred); + assert.equal(state.localStatusCalls, 1); + assert.equal(state.remoteStatusCalls, 0); + + yield* TestClock.adjust(Duration.seconds(89)); + yield* Effect.yieldNow; + assert.equal(state.remoteStatusCalls, 0); + + yield* TestClock.adjust(Duration.seconds(1)); + yield* Effect.yieldNow; + assert.equal(state.remoteStatusCalls, 1); + + yield* Scope.close(scope, Exit.void); + }).pipe( + Effect.provide( + makeTestLayer(state, VcsStatusBroadcaster.REMOTE_STATUS_STARTUP_GRACE).pipe( + Layer.provideMerge(TestClock.layer()), + ), + ), + ); + }); + + it.effect("caps how many automatic remote refreshes run at once", () => { + const state = { inFlight: 0, maxInFlight: 0 }; + const subscriberCount = REMOTE_STATUS_REFRESH_CONCURRENCY * 4; + const testLayer = VcsStatusBroadcaster.layer.pipe( + Layer.provide( + Layer.succeed(VcsStatusBroadcaster.RemoteStatusStartupGrace, Duration.millis(0)), + ), + Layer.provideMerge(NodeServices.layer), + Layer.provide(makeBackgroundPolicyLayer(() => true)), + Layer.provide( + Layer.mock(GitWorkflowService.GitWorkflowService)({ + localStatus: () => Effect.succeed(baseLocalStatus), + // Never completes, so every refresh that got a permit stays in + // flight and the peak is the permit count itself. + remoteStatus: () => + Effect.gen(function* () { + state.inFlight += 1; + state.maxInFlight = Math.max(state.maxInFlight, state.inFlight); + return yield* Effect.never; + }), + invalidateLocalStatus: () => Effect.void, + invalidateRemoteStatus: () => Effect.void, + invalidateStatus: () => Effect.void, + } satisfies Partial), + ), + ); + + return Effect.gen(function* () { + const broadcaster = yield* VcsStatusBroadcaster.VcsStatusBroadcaster; + const scope = yield* Scope.make(); + for (let index = 0; index < subscriberCount; index += 1) { + yield* Stream.runDrain( + broadcaster.streamStatus( + { cwd: `/repo-${index}` }, + { automaticRemoteRefreshInterval: Effect.succeed(Duration.seconds(30)) }, + ), + ).pipe(Effect.forkIn(scope)); + } + + for (let tick = 0; tick < subscriberCount * 4; tick += 1) { + yield* Effect.yieldNow; + } + + assert.equal(state.maxInFlight, REMOTE_STATUS_REFRESH_CONCURRENCY); + + yield* Scope.close(scope, Exit.void); + }).pipe(Effect.provide(testLayer)); + }); }); diff --git a/apps/server/src/vcs/VcsStatusBroadcaster.ts b/apps/server/src/vcs/VcsStatusBroadcaster.ts index f28069f6d8b2..8f65a2c31279 100644 --- a/apps/server/src/vcs/VcsStatusBroadcaster.ts +++ b/apps/server/src/vcs/VcsStatusBroadcaster.ts @@ -1,4 +1,5 @@ import * as Cause from "effect/Cause"; +import * as Clock from "effect/Clock"; import * as Context from "effect/Context"; import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; @@ -10,6 +11,7 @@ import * as PubSub from "effect/PubSub"; import * as Ref from "effect/Ref"; import * as Schedule from "effect/Schedule"; import * as Scope from "effect/Scope"; +import * as Semaphore from "effect/Semaphore"; import * as Stream from "effect/Stream"; import * as SynchronizedRef from "effect/SynchronizedRef"; import type { @@ -26,6 +28,30 @@ import * as BackgroundPolicy from "../background/BackgroundPolicy.ts"; import * as GitWorkflowService from "../git/GitWorkflowService.ts"; const DEFAULT_VCS_STATUS_REFRESH_INTERVAL = Duration.seconds(30); +/** + * Ceiling on how many automatic remote refreshes may run at once, and on the + * fan-out inside a single status read. One refresh is a `git fetch`-shaped + * subprocess tree plus a `gh` PR lookup; with a worktree per thread the + * previously unbounded fan-out put dozens of them on the CPU at the same time + * and starved the HTTP/WebSocket loop that the client's connection setup + * budget depends on. User-triggered refreshes never take a permit. + */ +export const REMOTE_STATUS_REFRESH_CONCURRENCY = 3; +/** + * Automatic remote refresh stays quiet for this long after the broadcaster is + * built. Boot is when every surface subscribes at once, so this is exactly the + * window where remote work competes with connection setup. Local status is + * unaffected, and so is an explicit user refresh. + */ +export const REMOTE_STATUS_STARTUP_GRACE = Duration.seconds(90); +/** + * Overrides {@link REMOTE_STATUS_STARTUP_GRACE}. Tests that assert refresh + * behavior set it to zero so they do not have to burn the grace window first. + */ +export const RemoteStatusStartupGrace = Context.Reference( + "t3/vcs/VcsStatusBroadcaster/RemoteStatusStartupGrace", + { defaultValue: () => REMOTE_STATUS_STARTUP_GRACE }, +); const VCS_STATUS_REFRESH_FAILURE_BASE_DELAY = Duration.seconds(30); const VCS_STATUS_REFRESH_FAILURE_MAX_DELAY = Duration.minutes(15); const MAX_FAILURE_DIAGNOSTIC_VALUES = 8; @@ -193,6 +219,11 @@ export const make = Effect.gen(function* () { ); const cacheRef = yield* Ref.make(new Map()); const pollersRef = yield* SynchronizedRef.make(new Map()); + // Automatic refreshes share these two: a permit pool that caps how many run + // at once, and the build timestamp that the startup grace measures from. + const remoteRefreshPermits = yield* Semaphore.make(REMOTE_STATUS_REFRESH_CONCURRENCY); + const startupGrace = yield* RemoteStatusStartupGrace; + const builtAtNanos = yield* Clock.currentTimeNanos; const getCachedStatus = Effect.fn("VcsStatusBroadcaster.getCachedStatus")(function* ( cwd: string, @@ -334,7 +365,7 @@ export const make = Effect.gen(function* () { cached?.local ? Effect.succeed(cached.local.value) : workflow.localStatus({ cwd }), cached?.remote ? Effect.succeed(cached.remote.value) : workflow.remoteStatus({ cwd }), ], - { concurrency: "unbounded" }, + { concurrency: REMOTE_STATUS_REFRESH_CONCURRENCY }, ); return yield* updateCachedStatus(cwd, local, remote); }); @@ -374,11 +405,24 @@ export const make = Effect.gen(function* () { yield* workflow.invalidateStatus(cwd); const [local, remote] = yield* Effect.all( [workflow.localStatus({ cwd }), workflow.remoteStatus({ cwd })], - { concurrency: "unbounded" }, + { concurrency: REMOTE_STATUS_REFRESH_CONCURRENCY }, ); return yield* updateCachedStatus(cwd, local, remote, { publish: true }); }); + /** + * `null` once the startup grace has elapsed, otherwise how much of it is + * left - which doubles as the delay before the poller tries again. + */ + const remainingStartupGrace = Effect.gen(function* () { + const graceMillis = Duration.toMillis(startupGrace); + if (graceMillis <= 0) return null; + const nowNanos = yield* Clock.currentTimeNanos; + const elapsedMillis = Number((nowNanos - builtAtNanos) / 1_000_000n); + const remainingMillis = graceMillis - elapsedMillis; + return remainingMillis > 0 ? Duration.millis(remainingMillis) : null; + }); + const makeRemoteRefreshLoop = ( cwd: string, demandCwdsRef: Ref.Ref>, @@ -398,6 +442,14 @@ export const make = Effect.gen(function* () { return activeInterval; } + // Startup grace: every surface subscribes at once during connection + // setup, so hold automatic remote work back and retry once the window + // closes. Nothing is dropped, only deferred. + const graceRemaining = yield* remainingStartupGrace; + if (graceRemaining !== null) { + return graceRemaining; + } + const demandCwds = yield* Ref.get(demandCwdsRef); const shouldRun = needsInitialRefresh || @@ -408,15 +460,19 @@ export const make = Effect.gen(function* () { cwd: demandCwd, }), ), - { concurrency: "unbounded" }, + { concurrency: REMOTE_STATUS_REFRESH_CONCURRENCY }, )).some(Boolean); if (!shouldRun) { return activeInterval; } - const exit = yield* refreshRemoteStatus(cwd, { - refreshUpstream: !Duration.isZero(configuredInterval), - }).pipe(Effect.exit); + const exit = yield* remoteRefreshPermits + .withPermits(1)( + refreshRemoteStatus(cwd, { + refreshUpstream: !Duration.isZero(configuredInterval), + }), + ) + .pipe(Effect.exit); if (Exit.isSuccess(exit)) { yield* Ref.set(needsInitialRefreshRef, false); yield* Ref.set(consecutiveFailuresRef, 0); diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index d23c21f4f08f..96e60b60cd2c 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -1173,9 +1173,11 @@ const makeWsRpcLayer = ( ); const environment = yield* serverEnvironment.getDescriptor; const auth = yield* serverAuth.getDescriptor(); - const availableEditors: ReadonlyArray = yield* resolveAvailableEditorsForConfig( - externalLauncher.resolveAvailableEditors(), - ); + // Never waits on a cold editor scan: a warm cache answers instantly and + // a cold one warms in the background, so the config snapshot cannot eat + // into the client's connection-setup budget. + const availableEditors: ReadonlyArray = + yield* externalLauncher.availableEditorsSnapshot(); const fileManagerRevealKind = availableEditors.includes("file-manager") ? yield* resolveFileManagerRevealKindForConfig( externalLauncher.resolveFileManagerRevealKind(), diff --git a/apps/web/package.json b/apps/web/package.json index 62a2f28640f0..4e4fbaad970f 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -1,6 +1,6 @@ { "name": "@t3tools/web", - "version": "0.0.49", + "version": "0.0.50", "private": true, "type": "module", "scripts": { diff --git a/docs/operations/turbo-changelog.md b/docs/operations/turbo-changelog.md index c2929d7c5213..05af4bbef799 100644 --- a/docs/operations/turbo-changelog.md +++ b/docs/operations/turbo-changelog.md @@ -8,6 +8,22 @@ per-commit — the ingestion PR entry records the upstream range instead. ## Unreleased — on `turbo`, not yet in a shipped build +- **0.0.50: connecting to a busy server is fast again.** On a large install (28 projects, 125 + threads, 72 live worktrees) the server was spending its event loop on background repository work + while a client was still connecting: trivial local HTTP GETs took 3–8.5s and the client's 15s + connection-setup budget failed over and over. Four places now shed that load. VCS remote status + refreshes are capped at three at a time and stay quiet for the first 90 seconds after the server + starts — local status, the badge you see immediately, and any refresh you trigger yourself are + untouched. The automatic thread-settlement sweep (one `gh pr list` per unsettled thread) drops + from eight at a time to two, waits five minutes after boot, and then runs at most every ten + minutes; changing an auto-settle setting still sweeps right away. Command lookup caches + explicit-path results and memoizes the Windows spawn resolver, so repeatedly spawning `git` or + `gh` no longer re-walks the filesystem thousands of times. And the server config snapshot no + longer waits on the editor scan: it answers from cache, or answers empty and scans in the + background, so available editors can be one snapshot late on a cold start. New seam + `startup-load-shedding`; related to pingdotgg/t3code#7231 and #7233. Not done: switching the + desktop LAN bind to dual-stack `::`, which is unrelated to the measured problem and hard-fails on + hosts with IPv6 disabled. - **0.0.49: T3 Turbo no longer hosts the legacy `~/.t3` (T3 Code personal) database as a second backend; import it once via the official-data-import path if needed.** The desktop bootstrap no longer probes for `~/.t3/userdata/state.sqlite` and no longer registers a `local:t3` instance in diff --git a/packages/contracts/package.json b/packages/contracts/package.json index 3f4079dd8e2a..3ab90177b6f7 100644 --- a/packages/contracts/package.json +++ b/packages/contracts/package.json @@ -1,6 +1,6 @@ { "name": "@t3tools/contracts", - "version": "0.0.49", + "version": "0.0.50", "private": true, "files": [ "dist" diff --git a/packages/shared/src/shell.test.ts b/packages/shared/src/shell.test.ts index e3046c03abed..449f13e318d9 100644 --- a/packages/shared/src/shell.test.ts +++ b/packages/shared/src/shell.test.ts @@ -2,12 +2,18 @@ import * as NodeServices from "@effect/platform-node/NodeServices"; import { it as effectIt } from "@effect/vitest"; import { HostProcessEnvironment, HostProcessPlatform } from "@t3tools/shared/hostProcess"; import * as Effect from "effect/Effect"; +import * as FileSystem from "effect/FileSystem"; +import * as Layer from "effect/Layer"; +import * as Option from "effect/Option"; +import * as Path from "effect/Path"; +import { TestClock } from "effect/testing"; import { describe, expect, it, vi } from "vite-plus/test"; import { extractPathFromShellOutput, CommandAvailability, type CommandAvailabilityChecker, + CommandResolutionCache, isCommandAvailable, listLoginShellCandidates, mergePathEntries, @@ -35,6 +41,73 @@ const withWindowsEnvironmentMocks = ( Effect.provideService(CommandAvailability, commandAvailable), ); +const WINDOWS_PROBE_ENV = { PATH: "", PATHEXT: ".COM;.EXE;.BAT;.CMD" } as const; + +interface StatProbe { + /** Paths the fake filesystem reports as existing executables. */ + readonly files: Set; + /** Every path handed to `stat`, in call order. */ + readonly statPaths: Array; +} + +const makeStatProbe = (files: ReadonlyArray = []): StatProbe => ({ + files: new Set(files), + statPaths: [], +}); + +const executableFileInfo: FileSystem.File.Info = { + type: "File", + mtime: Option.none(), + atime: Option.none(), + birthtime: Option.none(), + dev: 0, + ino: Option.none(), + mode: 0o755, + nlink: Option.none(), + uid: Option.none(), + gid: Option.none(), + rdev: Option.none(), + size: FileSystem.Size(0), + blksize: Option.none(), + blocks: Option.none(), +}; + +const noopFileSystem = FileSystem.makeNoop({}); + +/** + * Filesystem that records every `stat`, so a cache hit is provable: a resolution + * served from the cache must not add a single probe. + */ +const statProbeLayer = (probe: StatProbe) => + Layer.merge( + FileSystem.layerNoop({ + stat: (path: string) => + Effect.suspend(() => { + probe.statPaths.push(path); + return probe.files.has(path) + ? Effect.succeed(executableFileInfo) + : noopFileSystem.stat(path); + }), + }), + Path.layer, + ); + +/** + * Runs against the probing filesystem with a fresh, test-local + * `CommandResolutionCache` so cached entries never leak between tests. Uses + * `win32` because POSIX executability is decided by a real `access` syscall the + * fake filesystem cannot answer. + */ +const withStatProbe = ( + effect: Effect.Effect, + probe: StatProbe, +) => + effect.pipe( + Effect.provideService(HostProcessPlatform, "win32"), + Effect.provideService(CommandResolutionCache, new Map()), + Effect.provide(statProbeLayer(probe)), + ); + describe("extractPathFromShellOutput", () => { it("extracts the path between capture markers", () => { expect( @@ -377,6 +450,96 @@ effectIt.layer(NodeServices.layer)("resolveCommandPath", (it) => { ); }); +describe("resolveCommandPath explicit-path cache", () => { + effectIt.effect("serves a repeat explicit-path resolve without stating again", () => { + const probe = makeStatProbe(["/opt/tools/alpha.exe"]); + return withStatProbe( + Effect.gen(function* () { + const first = yield* resolveCommandPath("/opt/tools/alpha.exe", { + env: WINDOWS_PROBE_ENV, + }); + expect(first).toBe("/opt/tools/alpha.exe"); + + const statsAfterFirst = probe.statPaths.length; + expect(statsAfterFirst).toBeGreaterThan(0); + + const second = yield* resolveCommandPath("/opt/tools/alpha.exe", { + env: WINDOWS_PROBE_ENV, + }); + expect(second).toBe(first); + expect(probe.statPaths.length).toBe(statsAfterFirst); + }), + probe, + ); + }); + + effectIt.effect("does not cache an explicit path that is missing", () => { + const probe = makeStatProbe(); + return withStatProbe( + Effect.gen(function* () { + const missing = yield* resolveCommandPath("/opt/tools/beta.exe", { + env: WINDOWS_PROBE_ENV, + }).pipe(Effect.result); + expect(missing._tag).toBe("Failure"); + expect(probe.statPaths.length).toBeGreaterThan(0); + + // A caller that just wrote the binary must see it on the next probe. + probe.files.add("/opt/tools/beta.exe"); + + expect(yield* resolveCommandPath("/opt/tools/beta.exe", { env: WINDOWS_PROBE_ENV })).toBe( + "/opt/tools/beta.exe", + ); + }), + probe, + ); + }); + + effectIt.effect("keys cached explicit paths per command", () => { + const probe = makeStatProbe(["/opt/tools/alpha.exe", "/opt/tools/gamma.exe"]); + return withStatProbe( + Effect.gen(function* () { + expect(yield* resolveCommandPath("/opt/tools/alpha.exe", { env: WINDOWS_PROBE_ENV })).toBe( + "/opt/tools/alpha.exe", + ); + + const statsAfterAlpha = probe.statPaths.length; + + expect(yield* resolveCommandPath("/opt/tools/gamma.exe", { env: WINDOWS_PROBE_ENV })).toBe( + "/opt/tools/gamma.exe", + ); + expect(probe.statPaths.length).toBeGreaterThan(statsAfterAlpha); + + // Alpha is still cached, so the second command did not evict or shadow it. + expect(yield* resolveCommandPath("/opt/tools/alpha.exe", { env: WINDOWS_PROBE_ENV })).toBe( + "/opt/tools/alpha.exe", + ); + }), + probe, + ); + }); + + effectIt.effect("re-probes an explicit path once the 30s TTL expires", () => { + const probe = makeStatProbe(["/opt/tools/alpha.exe"]); + return withStatProbe( + Effect.gen(function* () { + yield* resolveCommandPath("/opt/tools/alpha.exe", { env: WINDOWS_PROBE_ENV }); + const statsAfterFirst = probe.statPaths.length; + + yield* TestClock.adjust(29_000); + yield* resolveCommandPath("/opt/tools/alpha.exe", { env: WINDOWS_PROBE_ENV }); + expect(probe.statPaths.length).toBe(statsAfterFirst); + + yield* TestClock.adjust(1_000); + expect(yield* resolveCommandPath("/opt/tools/alpha.exe", { env: WINDOWS_PROBE_ENV })).toBe( + "/opt/tools/alpha.exe", + ); + expect(probe.statPaths.length).toBeGreaterThan(statsAfterFirst); + }), + probe, + ).pipe(Effect.provide(TestClock.layer())); + }); +}); + effectIt.layer(NodeServices.layer)("resolveSpawnCommand", (it) => { it.effect("runs Windows executables directly without a shell", () => Effect.gen(function* () { diff --git a/packages/shared/src/shell.ts b/packages/shared/src/shell.ts index 4c86c8886312..a95da8efcc81 100644 --- a/packages/shared/src/shell.ts +++ b/packages/shared/src/shell.ts @@ -94,10 +94,57 @@ export type SpawnExecutableResolver = ( env: NodeJS.ProcessEnv, ) => string | undefined; +// `resolveSpawnCommand` runs on every Windows spawn and this scan is its whole +// cost: PATH entries x PATHEXT candidates synchronous `statSync` calls, hundreds +// per miss. Sessions spawn the same handful of commands (git, gh, the provider +// CLI) over and over, so memoize the *hits* for a short window, keyed on the +// full search environment so any PATH or PATHEXT change invalidates at once. +// Misses are never cached: a command that was not on PATH a moment ago may have +// just been installed, and a miss simply falls back to spawning `command` bare. +const SPAWN_EXECUTABLE_CACHE_TTL_NANOS = 30_000_000_000n; +const SPAWN_EXECUTABLE_CACHE_MAX_ENTRIES = 256; + +interface SpawnExecutableCacheEntry { + readonly resolvedPath: string; + readonly expiresAtNanos: bigint; +} + +const spawnExecutableCache = new Map(); + function resolveSpawnExecutableWithNode( command: string, platform: NodeJS.Platform, env: NodeJS.ProcessEnv, +): string | undefined { + const cacheKey = [platform, readEnvPath(env) ?? "", env.PATHEXT ?? "", command].join( + COMMAND_RESOLUTION_CACHE_KEY_SEPARATOR, + ); + const nowNanos = process.hrtime.bigint(); + const cached = spawnExecutableCache.get(cacheKey); + if (cached !== undefined && cached.expiresAtNanos > nowNanos) { + return cached.resolvedPath; + } + + const resolved = scanSpawnExecutableWithNode(command, platform, env); + if (resolved !== undefined) { + if (spawnExecutableCache.size >= SPAWN_EXECUTABLE_CACHE_MAX_ENTRIES) { + const oldestKey = spawnExecutableCache.keys().next().value; + if (oldestKey !== undefined) { + spawnExecutableCache.delete(oldestKey); + } + } + spawnExecutableCache.set(cacheKey, { + resolvedPath: resolved, + expiresAtNanos: nowNanos + SPAWN_EXECUTABLE_CACHE_TTL_NANOS, + }); + } + return resolved; +} + +function scanSpawnExecutableWithNode( + command: string, + platform: NodeJS.Platform, + env: NodeJS.ProcessEnv, ): string | undefined { const path = platform === "win32" ? NodePath.win32 : NodePath.posix; const windowsPathExtensions = platform === "win32" ? resolveWindowsPathExtensions(env) : []; @@ -501,15 +548,22 @@ function resolveCommandCandidates( // thousands per connect). Memoize the scan outcome per // (platform, PATH, PATHEXT, command) for a short window: repeat scans hit the // cache while any change to the search environment invalidates immediately. -// Explicit-path resolution is never cached - callers probe paths they have -// just written (e.g. managed binary installs). A "not-found" outcome is also -// cached for the TTL, so a just-installed binary can stay invisible for up to -// 30s unless resolved by explicit path. +// Explicit-path resolution shares the same map under +// COMMAND_RESOLUTION_EXPLICIT_PATH_KEY, but stores *hits only*: callers probe +// paths they have just written (e.g. managed binary installs), so a cached +// "not-found" there could hide a binary that now exists. Caching the hit is +// safe and is what the repeated-spawn case actually needs. +// The PATH branch, by contrast, does cache "not-found" for the TTL, so a +// just-installed binary can stay invisible there for up to 30s unless it is +// resolved by explicit path. // TTL expiry uses the monotonic clock (Clock.currentTimeNanos) so backward // wall-clock adjustments cannot keep expired entries alive. const COMMAND_RESOLUTION_CACHE_TTL_NANOS = 30_000_000_000n; const COMMAND_RESOLUTION_CACHE_MAX_ENTRIES = 512; const COMMAND_RESOLUTION_CACHE_KEY_SEPARATOR = String.fromCharCode(0); +// Stands in for PATH in the cache key when the command is an explicit path, so +// explicit-path entries can never collide with PATH-scan entries. +const COMMAND_RESOLUTION_EXPLICIT_PATH_KEY = "explicit-path"; interface CommandResolutionCacheEntry { readonly resolvedPath: string | null; @@ -544,24 +598,29 @@ function cacheCommandResolution( }); } -const isExecutableFile = Effect.fn("shell.isExecutableFile")(function* ( +// Deliberately span-free. A single PATH scan probes hundreds of candidates, so +// a span per probe buried the one span that matters +// ('shell.resolveCommandPathForPlatform') under tens of thousands of children +// per connect and cost real time in the tracer itself. +const isExecutableFile = ( filePath: string, platform: NodeJS.Platform, windowsPathExtensions: ReadonlyArray, -): Effect.fn.Return { - const fileSystem = yield* FileSystem.FileSystem; - const path = yield* Path.Path; - const stat = yield* fileSystem.stat(filePath).pipe(Effect.orElseSucceed(() => null)); - if (stat === null || stat.type !== "File") return false; - - if (platform === "win32") { - const extension = path.extname(filePath); - if (extension.length === 0) return false; - return windowsPathExtensions.includes(extension.toUpperCase()); - } +): Effect.Effect => + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const stat = yield* fileSystem.stat(filePath).pipe(Effect.orElseSucceed(() => null)); + if (stat === null || stat.type !== "File") return false; + + if (platform === "win32") { + const extension = path.extname(filePath); + if (extension.length === 0) return false; + return windowsPathExtensions.includes(extension.toUpperCase()); + } - return canExecuteFile(filePath); -}); + return canExecuteFile(filePath); + }); const resolveCommandPathForPlatform = Effect.fn("shell.resolveCommandPathForPlatform")(function* ( command: string, @@ -578,9 +637,29 @@ const resolveCommandPathForPlatform = Effect.fn("shell.resolveCommandPathForPlat path.extname, ); + const cache = yield* CommandResolutionCache; + if (command.includes("/") || command.includes("\\")) { + const explicitCacheKey = [ + platform, + COMMAND_RESOLUTION_EXPLICIT_PATH_KEY, + windowsPathExtensions.join(";"), + command, + ].join(COMMAND_RESOLUTION_CACHE_KEY_SEPARATOR); + const nowNanos = yield* Clock.currentTimeNanos; + // `resolvedPath !== null` restates the hits-only invariant rather than + // asserting it away. + const cachedExplicit = cache.get(explicitCacheKey); + if ( + cachedExplicit !== undefined && + cachedExplicit.resolvedPath !== null && + cachedExplicit.expiresAtNanos > nowNanos + ) { + return cachedExplicit.resolvedPath; + } for (const candidate of commandCandidates) { if (yield* isExecutableFile(candidate, platform, windowsPathExtensions)) { + cacheCommandResolution(cache, explicitCacheKey, candidate, nowNanos); return candidate; } } @@ -595,7 +674,6 @@ const resolveCommandPathForPlatform = Effect.fn("shell.resolveCommandPathForPlat const cacheKey = [platform, pathValue, windowsPathExtensions.join(";"), command].join( COMMAND_RESOLUTION_CACHE_KEY_SEPARATOR, ); - const cache = yield* CommandResolutionCache; const nowNanos = yield* Clock.currentTimeNanos; const cached = cache.get(cacheKey); if (cached !== undefined && cached.expiresAtNanos > nowNanos) { diff --git a/scripts/turbo-customization-manifest.test.ts b/scripts/turbo-customization-manifest.test.ts index a1f35b68f734..c33998c7deb2 100644 --- a/scripts/turbo-customization-manifest.test.ts +++ b/scripts/turbo-customization-manifest.test.ts @@ -165,6 +165,7 @@ it("verifies the checked-in Turbo manifest and tracks the implemented multi-chat "settled-lifecycle-sticky-pin", "shared-sha256-base64url", "sqlite-fast-mode-pragma", + "startup-load-shedding", "streaming-flag-cleared-on-turn-settle", "terminal-buffer-byte-budget", "terminal-drawer-redraw-gate",