Merge remote-tracking branch 'origin/main' into brennanb2025/claude-structured-mobile

This commit is contained in:
Merge Sim
2026-09-02 13:38:24 -07:00
261 changed files with 12782 additions and 2100 deletions
+84
View File
@@ -13863,6 +13863,90 @@
"knownGaps": ["No manifest command yet.", "No Windows CJK/emoji repaint command is wired."],
"demotionRule": "Cannot promote if the oracle is screenshot-only or environment-skipped."
},
{
"id": "terminal-render.foreground-repair-span",
"title": "A forced foreground repaint covers every row the write changed",
"maturity": "experimental",
"protection": "partial",
"owner": "terminal-rendering",
"layer": "renderer-unit",
"surfaces": [
"foreground PTY output",
"in-place agent redraws",
"erase-in-line/display",
"alternate screen",
"scroll",
"wide glyphs"
],
"platforms": ["macos", "linux", "windows"],
"providers": ["local", "daemon", "ssh", "remote-runtime"],
"coveredPlatforms": ["macos"],
"coveredProviders": [],
"coverageNotes": "Renderer-unit convergence corpus over a real xterm parser, plus manual CDP pixel evidence on the macOS WebGL renderer. The repaint span is provider-independent because it is computed from xterm's parse, not from the transport; SSH/WSL/remote were not exercised live.",
"motivatingLinks": [
"https://github.com/stablyai/orca/pull/2669",
"https://github.com/stablyai/orca/pull/4669",
"https://github.com/stablyai/orca/pull/8178"
],
"invariant": "The row span Orca asks xterm to repaint after a forced foreground refresh must cover every viewport row whose rendered content changed during that write, plus the cursor row before and after it; when the span cannot be established — unobservable parse, viewport scroll, or a normal/alternate buffer flip — the whole viewport must be repainted.",
"oracle": "A real @xterm/headless parser replays an adversarial corpus (in-place bottom-row redraws, standalone CR overwrite, backspace, erase-in-line, erase-in-display above and below the cursor, full clear, wide CJK, emoji, combining marks, ZWJ sequences, scroll-region insert/delete, reverse index, DEC 2026 frames, alternate-screen enter and exit, viewport scroll, narrow panes). Each viewport row is serialized cell-by-cell with its attributes before and after the write, and every row that differs must fall inside the span the settle path requested. A vacuity guard asserts each case actually moves the screen.",
"commands": [
"pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/lib/pane-manager/terminal-foreground-repair-convergence.test.ts"
],
"testFiles": [
"src/renderer/src/lib/pane-manager/terminal-foreground-repair-convergence.test.ts"
],
"assertionRefs": [
{
"file": "src/renderer/src/lib/pane-manager/terminal-foreground-repair-convergence.test.ts",
"assertions": [
"every viewport row whose serialized cells changed lies inside the requested repaint span",
"the cursor row before and after the write is inside the requested repaint span",
"viewport scroll and alternate-screen transitions still request the whole grid",
"an unobservable parse span falls back to the whole grid",
"an in-place bottom-row redraw narrows well below the full grid"
]
}
],
"evidenceRuns": [
{
"date": "2026-09-02",
"runner": "local",
"platform": "macos",
"result": "passed",
"command": "pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/lib/pane-manager/terminal-foreground-repair-convergence.test.ts",
"durationSeconds": 1,
"summary": "25 cases passed against a real xterm parser; paired CDP run on a 4-pane macOS WebGL dev build produced screenshots byte-identical to a forced full model rebuild."
}
],
"runtimeBudget": {
"p95Seconds": 15,
"scope": "Renderer-unit convergence corpus"
},
"flakeHistory": {
"status": "not-started",
"evidence": "New deterministic gate; no soak history yet."
},
"redGreenEvidence": {
"status": "complete",
"evidence": "Narrowing the span to the cursor rows alone (dropping xterm's parse span) fails the claude-style in-place redraw and erase-in-display-above cases; reading buffer indices instead of viewport rows made the corpus vacuous and is now blocked by the changed-row guard."
},
"performanceBudget": {
"required": true,
"evidence": "Measured on a focused, visible 4-pane macOS dev build under an agent-style in-place redraw load: rendered cells/s 157,708 -> 30,139 and forEachDecorationAtCell 320,868/s -> 60,652/s with render frames/s unchanged (59.8 -> 60.5)."
},
"promotionCriteria": [
"Add Windows DOM-renderer coverage for the synchronous repair branch.",
"Wire pixel or cell evidence for the alternate-screen and reflow paths into CI rather than manual CDP runs.",
"Keep a full-grid fallback assertion for every new span-narrowing condition."
],
"knownGaps": [
"No CI-wired pixel oracle; WebGL convergence evidence was collected manually over CDP.",
"Windows ConPTY synchronous repair path is covered only by the shared corpus, not on a Windows runner.",
"SSH/WSL/remote providers were not exercised live; the span is transport-independent by construction."
],
"demotionRule": "Demote or block if a narrowing condition is added without a matching convergence case, if the corpus stops asserting that each case changes at least one row, or if a repaint regression is reported for in-place agent redraws."
},
{
"id": "terminal-shell.windows-resolution-parity",
"title": "Windows local and daemon providers resolve shells and startup commands consistently",
+45 -19
View File
@@ -72,12 +72,20 @@ behavioural engine can be expected to score it low.
### Every process gets a handle, on a timer
`src/main/windows/windows-process-table.ts` takes a Toolhelp32 snapshot under one
of two flag sets. Identity (`None | CreationTime`) answers pid/ppid/name from the
snapshot alone and opens nothing; the detailed set adds `CommandLine`, which
costs one `OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION)` per process. `Memory`
is retired — it took a second handle carrying `PROCESS_VM_READ` and never read
through it.
`src/main/windows/windows-process-table.ts` takes a Toolhelp32 snapshot under
**one** flag set, `CommandLine | CreationTime`, shared by every caller. pid, ppid
and name come out of the snapshot itself and open nothing. `CommandLine` is what
opens a handle: the addon calls `GetProcessCommandLine` per process, which opens
`PROCESS_QUERY_INFORMATION | PROCESS_VM_READ` and walks the PEB with three
`ReadProcessMemory` calls (`src/process_commandline.cc:32,41-47` in the vendored
`@vscode/windows-process-tree` 0.8.0 source that `config/patches/` patches).
`Memory` is retired as of this change, and that is a real reduction: it made
`GetProcessMemoryUsage` open a **second** `PROCESS_QUERY_INFORMATION |
PROCESS_VM_READ` handle per process for a `GetProcessMemoryInfo` call whose
result no caller read (`src/process.cc:47-63`). Dropping it halves the handles
opened per snapshot. It does not remove the remote memory read, because the
command line still performs one.
It exists because seven independent readers used to fork `powershell.exe` for a
`Get-CimInstance Win32_Process` scan. That cost, measured: a PowerShell
@@ -90,23 +98,41 @@ panes multiplied it (#15036). The native snapshot answers the same question in
See
[`windows-process-enumeration.md`](./windows-process-enumeration.md).
Asking for fewer fields is cheaper, and since the split the module does: one
cache per flag set, so teardown identity and the session owner probe open no
handle at all (6.3 ms p50) while only the callers that read a command line pay
for one (12.3 ms p50, at 492 processes). Each cache still single-flights within
itself, and one gate serializes the native reads because the vendored wrapper
coalesces the flags of two overlapping calls.
Asking for fewer fields is cheaper, and the module now asks for the smallest set
that still answers every caller. There is **no** per-flag-set cache split: one
TTL-cached snapshot serves everyone, deliberately, because a split would restore
the per-pane fan-out the cache exists to remove — a 32-wide teardown has to
collapse into one scan. So the cheap identity-only read is not something any
caller can select; every read pays for `CommandLine`. An earlier revision of this
file described a two-cache design with 6.3 ms / 12.3 ms p50 figures at 492
processes. That design is not in the tree and those numbers describe no code
path here; the figures that do apply are the module's own, in
[`windows-process-enumeration.md`](./windows-process-enumeration.md).
**How an EDR reads it:** a cross-process handle plus a remote memory read against
every process on the box, repeating on a cadence, is the read half of the
telemetry that credential dumping and process injection produce. MDE surfaced it
as "suspicious memory activity". The memory read is gone: the command line now
comes from the kernel, through `NtQueryInformationProcess`'s
`ProcessCommandLineInformation` class, which needs only
`PROCESS_QUERY_LIMITED_INFORMATION`. `ReadProcessMemory` is absent from the
compiled addon, asserted against the binary's import table because the published
prebuild loads fine and emits byte-identical strings. What is left to declare to
administrators is the per-process handle itself.
as "suspicious memory activity".
**That signal is still present.** An earlier revision of this file claimed the
command line "now comes from the kernel" through `NtQueryInformationProcess`'s
`ProcessCommandLineInformation` class, needing only
`PROCESS_QUERY_LIMITED_INFORMATION`, and that `ReadProcessMemory` was absent from
the compiled addon. None of that is true of the code we ship.
`process_commandline.cc` calls `NtQueryInformationProcess` with
`ProcessBasicInformation` only — to locate the PEB — and then issues three
`ReadProcessMemory` calls against a `PROCESS_VM_READ` handle to read the PEB, the
`RTL_USER_PROCESS_PARAMETERS`, and the command-line buffer. Nothing asserts an
import table, and no such assertion would pass.
What this change did remove is the `Memory` flag's second handle and its
`GetProcessMemoryInfo` call, so the per-process handle count per snapshot halves.
What remains to declare to administrators is unchanged in kind: one
`PROCESS_QUERY_INFORMATION | PROCESS_VM_READ` handle and a PEB read against every
process on the box, at the shared snapshot's cadence. Moving to
`ProcessCommandLineInformation` (Windows 8.1+, `PROCESS_QUERY_LIMITED_INFORMATION`
only) would genuinely retire the remote read, but it is an addon patch nobody has
written; treat it as unclaimed work, not as shipped.
### Encoded, policy-bypassing PowerShell
+13 -2
View File
@@ -40,6 +40,15 @@ Measured on Windows 11 with 1050 processes (p50 / p95):
| + memory + command line | 30.6 ms | 33.7 ms |
| `Get-CimInstance` via PowerShell | 706 ms | 723 ms |
Those are the module's published figures. The flag set this module actually
requests is `CommandLine | CreationTime` — **not** `Memory`, which cost a second
`OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ)` plus
`GetProcessMemoryInfo` per process (`src/process.cc:47-63`) for a value nothing
read. Dropping it halves the handles a snapshot opens. The remaining set sits
between the two rows above and has not been measured separately; on a real
Windows host, `Get-Counter '\Process(Orca)\Handle Count'` sampled across a
snapshot cadence is the check.
Those CIM numbers are from a 1050-process host. The scan scales with process
count: on a 1486-process Windows SSH host it measured **1.36 s** and produced
**4.8 MiB** of JSON, against the fallback's 3 s and 8 MiB limits. Both limits
@@ -220,11 +229,13 @@ ownership, and CPU accounting in the memory collector — still reads it through
its own query. Those callers are not migrated.
Committed private bytes have no equivalent either, and the one memory value the
snapshot does carry is unusable for the sizes Orca now sees: `process.cc` stores
snapshot _can_ carry is unusable for the sizes Orca now sees: `process.cc` stores
`pmc.WorkingSetSize` into a `DWORD`, so anything above 4 GB wraps. That is the
second reason `windows-process-resource-collector.ts` still runs its own
`Get-CimInstance` sweep — it needs `PageFileUsage` (commit) and the CPU-time
counters in the same pass. Migrating it to the native table would cost both.
counters in the same pass. Migrating it to the native table would cost both, and
it is why this module no longer sets the `Memory` flag at all: the field had no
reader, and asking for it opened a handle per process on every snapshot.
Start time is a proxy for identity, not identity. The durable answer for the
process trees Orca itself spawns is an inherited handle: a job object names the
+7 -2
View File
@@ -187,10 +187,15 @@ export async function removeStaleDurableWriteTempFiles(
}
/** Synchronous counterpart for quit and crash paths that cannot await. */
export function writeFileDurableSync(tmpPath: string, finalPath: string, payload: string): void {
export function writeFileDurableSync(
tmpPath: string,
finalPath: string,
payload: string | Uint8Array
): void {
let renamed = false
try {
writeFileSync(tmpPath, payload, 'utf-8')
// A Uint8Array payload is written verbatim; a string still defaults to UTF-8.
writeFileSync(tmpPath, payload)
const fd = openSync(tmpPath, 'r+')
try {
fsyncSync(fd)
@@ -0,0 +1,94 @@
import { EventEmitter } from 'node:events'
import type { ChildProcess } from 'node:child_process'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
const { execFileMock, spawnMock, killSpawnedCommandTreeMock } = vi.hoisted(() => ({
execFileMock: vi.fn(),
spawnMock: vi.fn(),
killSpawnedCommandTreeMock: vi.fn().mockResolvedValue(undefined)
}))
vi.mock('node:child_process', async (importOriginal) => ({
...(await importOriginal()),
execFile: execFileMock,
spawn: spawnMock
}))
vi.mock('./spawned-command-tree-kill', () => ({
killSpawnedCommandTree: killSpawnedCommandTreeMock
}))
import { ghExecFileAsync } from './gh-exec-file'
function mockChild(pid = 4321): ChildProcess {
const child = new EventEmitter() as EventEmitter & Record<string, unknown>
child.pid = pid
child.kill = vi.fn(() => true)
child.stdin = Object.assign(new EventEmitter(), { end: vi.fn() })
child.stdout = new EventEmitter()
child.stderr = new EventEmitter()
return child as unknown as ChildProcess
}
/**
* The contract the star check depends on after #18234: a `gh` that never exits
* is killed at the deadline, tree and all, rather than running forever.
*/
describe('gh exec deadline', () => {
beforeEach(() => {
vi.useFakeTimers()
execFileMock.mockReset()
spawnMock.mockReset()
killSpawnedCommandTreeMock.mockClear()
})
afterEach(() => {
vi.useRealTimers()
})
it('kills the process tree and rejects when gh never exits', async () => {
const child = mockChild()
// Why never invoking the callback: this is exactly the stuck child from
// #18234 — spawned, spinning, and never reporting an exit.
execFileMock.mockReturnValue(child)
const pending = ghExecFileAsync(['api', '--include', 'user/starred/stablyai/orca'], {
timeout: 15_000
})
const rejection = expect(pending).rejects.toThrow('timed out')
await vi.waitFor(() => expect(execFileMock).toHaveBeenCalledOnce())
// Not yet: the deadline has not elapsed.
expect(killSpawnedCommandTreeMock).not.toHaveBeenCalled()
await vi.advanceTimersByTimeAsync(15_000)
await rejection
expect(killSpawnedCommandTreeMock).toHaveBeenCalledWith(child)
})
it('spawns with hidden console and captured stdio, never an inherited or shell stdio', async () => {
const child = mockChild()
execFileMock.mockImplementation(
(
_command: string,
_args: string[],
_options: unknown,
callback: (error: Error | null, stdout: string, stderr: string) => void
) => {
callback(null, 'HTTP/2.0 204 No Content\r\n', '')
return child
}
)
await ghExecFileAsync(['api', '--include', 'user/starred/stablyai/orca'], { timeout: 15_000 })
const [command, args, options] = execFileMock.mock.calls[0]
expect(command).toBe('gh')
expect(args).toEqual(['api', '--include', 'user/starred/stablyai/orca'])
// `execFile` captures stdout/stderr over pipes and never inherits Orca's;
// `shell` is never set, and the console stays hidden on Windows.
expect(options.windowsHide).toBe(true)
expect(options.stdio).toBeUndefined()
expect(options.shell).toBeUndefined()
})
})
@@ -0,0 +1,71 @@
import { readFileSync, readdirSync, statSync } from 'node:fs'
import { join, relative, resolve } from 'node:path'
import { describe, expect, it } from 'vitest'
/**
* Guard the gh chokepoint the way `child-process-import-boundary` guards spawn.
*
* `ghExecFileAsync` is what gives a gh invocation a deadline, a process-tree
* kill, transient-error retry, the rate-limit breaker, and WSL/host routing.
* Two call sites quietly opted out of all of it by reaching for the legacy
* `execFileAsync('gh', …)`, and one of them left `gh` children spinning at 100%
* CPU forever while permanently exhausting the GitHub concurrency semaphore
* (#18234). Nothing about those call sites looked wrong locally — which is why
* this is a tree-level rule rather than a review habit.
*
* The allowlist is empty and may only stay empty.
*/
const GH_SPAWN_PATTERN =
/(?:execFileAsync|commandExecFileAsync|execFileCapture|runProcess|spawnProcess|execFile|spawnSync|spawn)\s*\(\s*(['"`])gh\1|program:\s*(['"`])gh\2/
// Why trailing slash: a sibling like command-runner-extras.ts is scanned, not exempted.
const OWNER_DIRECTORY = 'src/main/git/command-runner/'
const SCANNED_EXTENSIONS = ['.ts', '.tsx']
const IGNORED_DIRECTORIES = new Set([
'node_modules',
'dist',
'out',
'build',
'.git',
'__fixtures__'
])
function isTestFile(path: string): boolean {
return /\.(?:test|spec)\.tsx?$/.test(path) || path.includes('/__tests__/')
}
function collectSourceFiles(root: string): string[] {
let found: string[] = []
let entries: string[]
try {
entries = readdirSync(root)
} catch {
return found
}
for (const entry of entries) {
if (IGNORED_DIRECTORIES.has(entry)) {
continue
}
const full = join(root, entry)
if (statSync(full).isDirectory()) {
found = found.concat(collectSourceFiles(full))
continue
}
if (SCANNED_EXTENSIONS.some((extension) => full.endsWith(extension))) {
found.push(full)
}
}
return found
}
describe('gh spawn boundary', () => {
it('routes every gh invocation through ghExecFileAsync', () => {
const repoRoot = resolve(__dirname, '..', '..', '..', '..')
const offenders = collectSourceFiles(join(repoRoot, 'src'))
.map((path) => relative(repoRoot, path).split('\\').join('/'))
.filter((path) => !isTestFile(path) && !path.startsWith(OWNER_DIRECTORY))
.filter((path) => GH_SPAWN_PATTERN.test(readFileSync(join(repoRoot, path), 'utf8')))
expect(offenders).toEqual([])
})
})
+5 -1
View File
@@ -157,7 +157,11 @@ export async function probeAnyExactRefBatched(
} catch {
return { found: false, unknown: true }
}
const lines = stdout.split('\n').filter((line) => line.trim().length > 0)
// Trim per line so a CRLF-translating host's `\r` does not become part of the type.
const lines = stdout
.split('\n')
.map((line) => line.trim())
.filter((line) => line.length > 0)
// One line per input, in order; a short read means the batch never answered for the rest.
if (lines.length !== safeRefs.length) {
return { found: false, unknown: true }
+59
View File
@@ -193,6 +193,17 @@ describe('git remote operations', () => {
if (args[0] === 'remote' && args[1] === 'get-url' && args[2] === 'pr-pynickle-orca') {
return { stdout: 'https://github.com/pynickle/orca.git\n', stderr: '' }
}
if (args[0] === 'remote' && args[1] === '-v') {
return {
stdout: [
'origin\thttps://github.com/stablyai/orca.git (fetch)',
'origin\thttps://github.com/stablyai/orca.git (push)',
'pr-pynickle-orca\thttps://github.com/pynickle/orca.git (fetch)',
'pr-pynickle-orca\thttps://github.com/pynickle/orca.git (push)'
].join('\n'),
stderr: ''
}
}
if (args[0] === 'remote') {
return { stdout: 'origin\npr-pynickle-orca\n', stderr: '' }
}
@@ -207,6 +218,54 @@ describe('git remote operations', () => {
)
})
// Regression: normalizing a URL-valued push remote used to run `git remote` and then a
// serial `git remote get-url` per remote -- 59 subprocesses on a 58-remote repo.
it('normalizes a URL-valued push remote from one remote table read at 58 remotes', async () => {
const remotes = [
{ name: 'origin', url: 'https://github.com/stablyai/orca.git' },
...Array.from({ length: 56 }, (_, index) => ({
name: `pr-user${index}-orca`,
url: `https://github.com/user${index}/orca.git`
})),
{ name: 'pr-pynickle-orca', url: 'https://github.com/pynickle/orca.git' }
]
gitExecFileAsyncMock.mockImplementation(async (args: string[]) => {
if (args[0] === 'symbolic-ref') {
return { stdout: 'imp/chinese-translation\n', stderr: '' }
}
if (args[0] === 'config' && args.includes('branch.imp/chinese-translation.remote')) {
return { stdout: 'https://github.com/pynickle/orca.git\n', stderr: '' }
}
if (args[0] === 'config' && args.includes('branch.imp/chinese-translation.merge')) {
return { stdout: 'refs/heads/imp/chinese-translation\n', stderr: '' }
}
if (args[0] === 'config') {
throw new Error(`config key is not set: ${args.join(' ')}`)
}
if (args[0] === 'remote' && args[1] === '-v') {
return {
stdout: remotes
.flatMap(({ name, url }) => [`${name}\t${url} (fetch)`, `${name}\t${url} (push)`])
.join('\n'),
stderr: ''
}
}
if (args[0] === 'remote') {
throw new Error(`unexpected remote scan: ${args.join(' ')}`)
}
return { stdout: '', stderr: '' }
})
await gitPush('/repo', false)
const remoteReads = gitExecFileAsyncMock.mock.calls.filter(([args]) => args[0] === 'remote')
expect(remoteReads.map(([args]) => args)).toEqual([['remote', '-v']])
expect(gitExecFileAsyncMock).toHaveBeenLastCalledWith(
['push', '--set-upstream', 'pr-pynickle-orca', 'HEAD:imp/chinese-translation'],
{ cwd: '/repo' }
)
})
it('uses an explicit push target even when it differs from the local branch name', async () => {
gitExecFileAsyncMock
.mockResolvedValueOnce({ stdout: '', stderr: '' })
+15 -23
View File
@@ -4,6 +4,7 @@ import {
} from '../../shared/git-remote-error'
import { resolveEffectiveGitUpstream } from '../../shared/git-effective-upstream'
import { gitRefTargetsBranchOnRemote } from '../../shared/git-remote-branch-name'
import { findGitRemoteNameByFetchUrl } from '../../shared/git-remote-url-index'
import type { GitPushTarget } from '../../shared/worktree/types'
import type { GitRuntimeOptions } from './git-runtime-options'
import { gitOptionsForWorktree } from './git-runtime-options'
@@ -84,6 +85,8 @@ type ConfiguredPushRemote = {
branchRemote: string | null
}
// One `git remote -v` instead of `git remote` plus a serial `git remote get-url`
// per remote; both print the same insteadOf-expanded fetch URL.
async function findRemoteNameForUrl(
worktreePath: string,
remoteUrl: string,
@@ -91,30 +94,13 @@ async function findRemoteNameForUrl(
): Promise<string | null> {
try {
const { stdout } = await gitExecFileAsync(
['remote'],
['remote', '-v'],
gitOptionsForWorktree(worktreePath, options)
)
const remotes = stdout
.split(/\r?\n/)
.map((line) => line.trim())
.filter(Boolean)
for (const remoteName of remotes) {
try {
const { stdout: urlStdout } = await gitExecFileAsync(
['remote', 'get-url', remoteName],
gitOptionsForWorktree(worktreePath, options)
)
if (urlStdout.trim() === remoteUrl) {
return remoteName
}
} catch {
// Ignore a remote that disappeared or has no fetch URL.
}
}
return findGitRemoteNameByFetchUrl(stdout, (candidateUrl) => candidateUrl === remoteUrl)
} catch {
return null
}
return null
}
async function normalizePushRemote(
@@ -141,11 +127,17 @@ async function getConfiguredPushRemote(
if (!remote) {
return null
}
const normalizedRemote = await normalizePushRemote(worktreePath, remote, options)
// The two usually name the same URL; resolving it twice reads the remote table twice.
if (!branchRemote) {
return { remote: normalizedRemote, branchRemote: null }
}
return {
remote: await normalizePushRemote(worktreePath, remote, options),
branchRemote: branchRemote
? await normalizePushRemote(worktreePath, branchRemote, options)
: null
remote: normalizedRemote,
branchRemote:
branchRemote === remote
? normalizedRemote
: await normalizePushRemote(worktreePath, branchRemote, options)
}
}
@@ -0,0 +1,102 @@
// Why: the batched `cat-file --batch-check` conflict probe decides from stdout, so a
// WSL login-shell fallback that prints the distro banner onto that stream desynchronizes
// the one-line-per-ref contract. Every batch then came back undecided and fell through to
// one `show-ref` subprocess per remote -- the cost the batch exists to remove. These tests
// pin the fence request and the resulting subprocess count at 58 remotes.
import { beforeEach, describe, expect, it, vi } from 'vitest'
const { gitExecFileAsyncMock } = vi.hoisted(() => ({ gitExecFileAsyncMock: vi.fn() }))
vi.mock('./runner', () => ({ gitExecFileAsync: gitExecFileAsyncMock }))
import { getBranchConflictKind } from './repo-branch-conflict'
const REMOTES = Array.from({ length: 58 }, (_, index) => `r${index}`)
const BRANCH = 'user/feature'
const WSL_BANNER =
'Welcome to Ubuntu 24.04.1 LTS (GNU/Linux 5.15.167.4-microsoft-standard-WSL2 x86_64)\n' +
'To run a command as administrator (user "root"), use "sudo <command>".\n'
type GitExecOptions = { stdin?: string; captureWslLoginShellOutput?: boolean }
/**
* Stand-in for a WSL-routed runner: the login shell prepends its banner to stdout unless
* the caller asked for the fenced form, which slices the payload back out.
*/
function installLoginShellRunner(): { argv: string[][] } {
const argv: string[][] = []
gitExecFileAsyncMock.mockImplementation(async (args: string[], options: GitExecOptions = {}) => {
argv.push(args)
if (args[0] === 'rev-parse') {
throw new Error('local branch is absent')
}
if (args[0] === 'remote') {
return { stdout: `${WSL_BANNER}${REMOTES.join('\n')}\n`, stderr: '' }
}
if (args[0] === 'show-ref') {
throw Object.assign(new Error('missing ref'), { code: 1, stderr: '' })
}
if (args[0] === 'cat-file') {
const payload = `${(options.stdin ?? '')
.split('\n')
.filter(Boolean)
.map((ref) => `${ref} missing`)
.join('\n')}\n`
return {
stdout: options.captureWslLoginShellOutput ? payload : `${WSL_BANNER}${payload}`,
stderr: ''
}
}
throw new Error(`unexpected git command: ${args.join(' ')}`)
})
return { argv }
}
function countSubcommand(argv: readonly string[][], subcommand: string): number {
return argv.filter((args) => args[0] === subcommand).length
}
describe('getBranchConflictKind batched remote probe', () => {
beforeEach(() => {
gitExecFileAsyncMock.mockReset()
})
it('asks the WSL login shell to fence the batch payload it parses', async () => {
installLoginShellRunner()
await getBranchConflictKind('/repo', BRANCH)
const batchCall = gitExecFileAsyncMock.mock.calls.find(([args]) => args[0] === 'cat-file')
expect(batchCall?.[1]).toMatchObject({ captureWslLoginShellOutput: true })
})
it('answers from one batched subprocess instead of one show-ref per remote', async () => {
const { argv } = installLoginShellRunner()
await expect(getBranchConflictKind('/repo', BRANCH)).resolves.toBeNull()
expect(countSubcommand(argv, 'cat-file')).toBe(1)
expect(countSubcommand(argv, 'show-ref')).toBe(0)
})
it('still falls back to per-ref probes when the batch itself fails', async () => {
gitExecFileAsyncMock.mockImplementation(async (args: string[]) => {
if (args[0] === 'rev-parse') {
throw new Error('local branch is absent')
}
if (args[0] === 'remote') {
return { stdout: `${REMOTES.join('\n')}\n`, stderr: '' }
}
if (args[0] === 'cat-file') {
throw new Error('cat-file is unavailable on this host')
}
if (args[0] === 'show-ref') {
return { stdout: '', stderr: '' }
}
throw new Error(`unexpected git command: ${args.join(' ')}`)
})
await expect(getBranchConflictKind('/repo', BRANCH)).resolves.toBe('remote')
})
})
+10 -2
View File
@@ -147,10 +147,12 @@ export function getBranchConflictKind(
const execOptions = gitExecOptions(path, options)
const runLocalGit = (
argv: string[],
commandOptions?: ExactRefProbeExecOptions & { stdin?: string }
commandOptions?: ExactRefProbeExecOptions & { stdin?: string },
captureWslLoginShellOutput = false
): Promise<{ stdout: string }> =>
gitExecFileAsync(argv, {
...execOptions,
...(captureWslLoginShellOutput ? { captureWslLoginShellOutput: true } : {}),
...(commandOptions?.maxBuffer === undefined ? {} : { maxBuffer: commandOptions.maxBuffer }),
...(commandOptions?.timeoutMs === undefined ? {} : { timeout: commandOptions.timeoutMs }),
...(commandOptions?.stdin === undefined ? {} : { stdin: commandOptions.stdin })
@@ -160,7 +162,13 @@ export function getBranchConflictKind(
branchName,
allowedBaseRef,
{},
(argv, commandOptions) => runLocalGit(argv, commandOptions)
// Why fenced: the batch decides from stdout, and a WSL login-shell fallback writes
// the distro's rc/motd banner to that same stream. The extra lines break the
// one-line-per-ref contract, so every batch came back undecided and fell through to
// one `show-ref` subprocess per remote -- the exact cost the batch exists to remove.
// `show-ref --verify --quiet` prints nothing and is read by exit code, so it needs
// no fence; the capture wrapper preserves the payload's exit status either way.
(argv, commandOptions) => runLocalGit(argv, commandOptions, true)
)
}
+10
View File
@@ -363,6 +363,16 @@ describe('getUpstreamStatus', () => {
if (args[0] === 'remote' && args[1] === 'get-url' && args[2] === 'pr-pynickle-orca') {
return Promise.resolve({ stdout: 'https://github.com/pynickle/orca.git\n' })
}
if (args[0] === 'remote' && args[1] === '-v') {
return Promise.resolve({
stdout: [
'origin\thttps://github.com/stablyai/orca.git (fetch)',
'origin\thttps://github.com/stablyai/orca.git (push)',
'pr-pynickle-orca\thttps://github.com/pynickle/orca.git (fetch)',
'pr-pynickle-orca\thttps://github.com/pynickle/orca.git (push)'
].join('\n')
})
}
if (args[0] === 'remote') {
return Promise.resolve({ stdout: 'origin\npr-pynickle-orca\n' })
}
+109 -10
View File
@@ -26,47 +26,146 @@ vi.mock('./github-api-repository', async (importOriginal) =>
)
)
import { checkOrcaStarred } from './client'
import { __resetOrcaStarCheckForTests, checkOrcaStarred, starOrca } from './client'
import { resetOriginRepositoryCache } from './client-test-harness'
const { execFileAsyncMock, acquireMock, releaseMock } = clientMocks
const { execFileAsyncMock, ghExecFileAsyncMock, acquireMock, releaseMock } = clientMocks
/** Let the coalesced check reach its `await acquire()` continuation and spawn gh. */
async function flushMicrotasks(): Promise<void> {
for (let i = 0; i < 5; i += 1) {
await Promise.resolve()
}
}
describe('checkOrcaStarred', () => {
beforeEach(() => {
beforeEach(async () => {
resetOriginRepositoryCache()
execFileAsyncMock.mockReset()
ghExecFileAsyncMock.mockReset()
acquireMock.mockReset()
releaseMock.mockReset()
acquireMock.mockResolvedValue(undefined)
__resetOrcaStarCheckForTests()
})
it('returns true only for an included successful GitHub response', async () => {
execFileAsyncMock.mockResolvedValueOnce({ stdout: 'HTTP/2.0 204 No Content\r\n', stderr: '' })
ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: 'HTTP/2.0 204 No Content\r\n', stderr: '' })
await expect(checkOrcaStarred()).resolves.toBe(true)
expect(execFileAsyncMock).toHaveBeenCalledWith(
'gh',
expect(ghExecFileAsyncMock).toHaveBeenCalledWith(
['api', '--include', 'user/starred/stablyai/orca'],
{ encoding: 'utf-8' }
expect.objectContaining({ encoding: 'utf-8' })
)
})
it('returns true for an HTTP 200 starred response', async () => {
execFileAsyncMock.mockResolvedValueOnce({ stdout: 'HTTP/2.0 200 OK\r\n', stderr: '' })
ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: 'HTTP/2.0 200 OK\r\n', stderr: '' })
await expect(checkOrcaStarred()).resolves.toBe(true)
})
it('returns false for GitHub 404 not starred responses', async () => {
execFileAsyncMock.mockRejectedValueOnce(new Error('HTTP 404: Not Found'))
ghExecFileAsyncMock.mockRejectedValueOnce(new Error('HTTP 404: Not Found'))
await expect(checkOrcaStarred()).resolves.toBe(false)
})
it('returns null when gh exits successfully without response headers', async () => {
execFileAsyncMock.mockResolvedValueOnce({ stdout: '', stderr: '' })
ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: '', stderr: '' })
await expect(checkOrcaStarred()).resolves.toBe(null)
})
// ── #18234: an unbounded, unreaped, un-deduped star check ──────────────
it('never spawns gh directly, so the spawn carries a deadline and a tree kill', async () => {
ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: 'HTTP/2.0 204 No Content\r\n', stderr: '' })
await checkOrcaStarred()
// Why: the raw execFileAsync has no timeout, so a `gh` that never exits ran
// forever at 100% CPU and was never reaped. ghExecFileAsync bounds the child
// and kills its process tree on the deadline.
expect(execFileAsyncMock).not.toHaveBeenCalled()
const [, options] = ghExecFileAsyncMock.mock.calls[0]
expect(typeof options.timeout).toBe('number')
expect(options.timeout).toBeGreaterThan(0)
expect(Number.isFinite(options.timeout)).toBe(true)
})
it('coalesces concurrent checks onto one gh child', async () => {
let resolveGh: (value: { stdout: string; stderr: string }) => void = () => {}
ghExecFileAsyncMock.mockReturnValueOnce(
new Promise((resolve) => {
resolveGh = resolve
})
)
const first = checkOrcaStarred()
const second = checkOrcaStarred()
const third = checkOrcaStarred()
await flushMicrotasks()
// Why: five call sites can ask at once; without coalescing each forked its
// own `gh` and four stuck children exhausted the GitHub semaphore.
expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(1)
expect(acquireMock).toHaveBeenCalledTimes(1)
resolveGh({ stdout: 'HTTP/2.0 204 No Content\r\n', stderr: '' })
await expect(Promise.all([first, second, third])).resolves.toEqual([true, true, true])
})
it('starts a fresh check once the previous one has settled', async () => {
ghExecFileAsyncMock
.mockResolvedValueOnce({ stdout: 'HTTP/2.0 404 Not Found\r\n', stderr: '' })
.mockResolvedValueOnce({ stdout: 'HTTP/2.0 204 No Content\r\n', stderr: '' })
await checkOrcaStarred()
await expect(checkOrcaStarred()).resolves.toBe(true)
expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(2)
})
it('releases its GitHub concurrency slot when gh fails or times out', async () => {
ghExecFileAsyncMock.mockRejectedValueOnce(new Error('gh timed out.'))
await expect(checkOrcaStarred()).resolves.toBe(null)
// Why: a leaked slot is permanent — four of them wedge every GitHub feature
// in the app for the rest of the session.
expect(releaseMock).toHaveBeenCalledTimes(1)
expect(acquireMock).toHaveBeenCalledTimes(1)
})
})
describe('starOrca', () => {
beforeEach(async () => {
resetOriginRepositoryCache()
execFileAsyncMock.mockReset()
ghExecFileAsyncMock.mockReset()
acquireMock.mockReset()
releaseMock.mockReset()
acquireMock.mockResolvedValue(undefined)
__resetOrcaStarCheckForTests()
})
it('stars through the bounded gh runner and releases its slot', async () => {
ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: '', stderr: '' })
await expect(starOrca()).resolves.toBe(true)
expect(execFileAsyncMock).not.toHaveBeenCalled()
const [args, options] = ghExecFileAsyncMock.mock.calls[0]
expect(args).toEqual(['api', '-X', 'PUT', 'user/starred/stablyai/orca'])
expect(options.timeout).toBeGreaterThan(0)
expect(releaseMock).toHaveBeenCalledTimes(1)
})
it('reports failure and still releases its slot when gh times out', async () => {
ghExecFileAsyncMock.mockRejectedValueOnce(new Error('gh timed out.'))
await expect(starOrca()).resolves.toBe(false)
expect(releaseMock).toHaveBeenCalledTimes(1)
})
})
+1 -1
View File
@@ -8,7 +8,7 @@ export {
__resetTrackedUpstreamBranchCacheForTests
} from './client/lookup/tracked-upstream-cache'
export { addPRReviewComment, addPRReviewCommentReply } from './client/create/add-pr-review-comment'
export { checkOrcaStarred, starOrca } from './client/fetch/orca-star'
export { __resetOrcaStarCheckForTests, checkOrcaStarred, starOrca } from './client/fetch/orca-star'
export { countWorkItems } from './client/list/count-work-items'
export { createGitHubPullRequest } from './client/create/create-github-pull-request'
export { getAuthenticatedViewer } from './client/fetch/authenticated-viewer'
@@ -1,14 +1,17 @@
import type { GitHubViewer } from '../../../../shared/github/pull-request-types'
import { execFileAsync, acquire, release } from '../../gh-utils'
import { ghExecFileAsync, acquire, release } from '../../gh-utils'
/**
* Get the authenticated GitHub viewer when gh is available and logged in.
* Returns null when gh is unavailable, unauthenticated, or the lookup fails.
*
* Runs through `ghExecFileAsync` for its deadline and tree kill: a `gh` that
* never exits would otherwise hold one of the four GitHub concurrency slots
* forever (#18234).
*/
export async function getAuthenticatedViewer(): Promise<GitHubViewer | null> {
await acquire()
try {
const { stdout } = await execFileAsync(
'gh',
const { stdout } = await ghExecFileAsync(
['api', 'user', '--jq', '{login: .login, email: .email}'],
{ encoding: 'utf-8' }
)
+37 -8
View File
@@ -1,17 +1,45 @@
import { execFileAsync, acquire, release } from '../../gh-utils'
import { ghExecFileAsync, acquire, release } from '../../gh-utils'
export const ORCA_REPO = 'stablyai/orca'
/**
* Deadline for the two star-nag gh calls.
*
* Why bounded at all: these are the only gh call sites that used the raw
* `execFileAsync`, so a `gh` that never exits blocked forever, never released
* its GitHub concurrency slot, and left the child running (#18234). Why shorter
* than the 30s gh default: nothing here is user-visible work — the nag falls
* back to the browser button — so a slow answer is worth less than a bounded one.
*/
const STAR_GH_TIMEOUT_MS = 15_000
let inFlightStarCheck: Promise<boolean | null> | null = null
/**
* Check if the authenticated user has starred the Orca repo.
* Returns true if starred, false if not, null if unable to determine (gh unavailable).
*/
export async function checkOrcaStarred(): Promise<boolean | null> {
export function checkOrcaStarred(): Promise<boolean | null> {
// Why: five independent callers (landing button, settings section, threshold
// nag, agent-value moment, force-show) can ask at once and none of them knows
// about the others. Without coalescing, each forks its own `gh`, and four
// stuck children exhaust the 4-wide GitHub semaphore for the app's lifetime.
inFlightStarCheck ??= runOrcaStarredCheck().finally(() => {
inFlightStarCheck = null
})
return inFlightStarCheck
}
/** @internal Drop any coalesced check so suites cannot inherit one another's. */
export function __resetOrcaStarCheckForTests(): void {
inFlightStarCheck = null
}
async function runOrcaStarredCheck(): Promise<boolean | null> {
await acquire()
try {
const { stdout, stderr } = await execFileAsync(
'gh',
const { stdout, stderr } = await ghExecFileAsync(
['api', '--include', `user/starred/${ORCA_REPO}`],
{ encoding: 'utf-8' }
{ encoding: 'utf-8', timeout: STAR_GH_TIMEOUT_MS }
)
const response = `${stdout ?? ''}\n${stderr ?? ''}`
if (/HTTP\/\S+\s+(?:200|204)\b/.test(response)) {
@@ -24,7 +52,7 @@ export async function checkOrcaStarred(): Promise<boolean | null> {
if (message.includes('HTTP 404')) {
return false
}
// Anything else (gh not installed, not authenticated, network issue)
// Anything else (gh not installed, not authenticated, network issue, timeout)
return null
} finally {
release()
@@ -37,8 +65,9 @@ export async function checkOrcaStarred(): Promise<boolean | null> {
export async function starOrca(): Promise<boolean> {
await acquire()
try {
await execFileAsync('gh', ['api', '-X', 'PUT', `user/starred/${ORCA_REPO}`], {
encoding: 'utf-8'
await ghExecFileAsync(['api', '-X', 'PUT', `user/starred/${ORCA_REPO}`], {
encoding: 'utf-8',
timeout: STAR_GH_TIMEOUT_MS
})
return true
} catch {
@@ -13,7 +13,7 @@ import { listWorktrees } from '../git/worktree'
import type { SshGitProvider } from '../providers/ssh-git-provider'
import type { GitPushTarget } from '../../shared/worktree/types'
import { WORKTREE_ID_SEPARATOR, worktreeIdComparisonKey } from '../../shared/worktree/id'
import { iterateProcessOutputLines } from '../../shared/process-output-field-scanner'
import { parseGitRemoteFetchUrls } from '../../shared/git-remote-url-index'
import {
findWorktreeMetaReferencingRemote,
hasBranchConfigUsingRemote,
@@ -44,26 +44,9 @@ async function listPrRemoteCandidates(
} catch {
return []
}
const candidates = new Map<string, string>()
for (const line of iterateProcessOutputLines(stdout)) {
const parsed = parseRemoteVerboseLine(line)
if (parsed?.direction === 'fetch' && isOrcaGeneratedPrRemoteName(parsed.name)) {
candidates.set(parsed.name, parsed.url)
}
}
return [...candidates.entries()].map(([name, url]) => ({ name, url }))
}
function parseRemoteVerboseLine(
line: string
): { name: string; url: string; direction: 'fetch' | 'push' } | null {
const tabIndex = line.indexOf('\t')
if (tabIndex === -1) {
return null
}
const name = line.slice(0, tabIndex)
const match = /^(.*) \((fetch|push)\)$/.exec(line.slice(tabIndex + 1).trim())
return match ? { name, url: match[1], direction: match[2] as 'fetch' | 'push' } : null
return [...parseGitRemoteFetchUrls(stdout)]
.filter(([name]) => isOrcaGeneratedPrRemoteName(name))
.map(([name, url]) => ({ name, url }))
}
async function shouldReclaimPrRemote(
@@ -0,0 +1,178 @@
// Why: `findRemoteForUrl` used to run `git remote` and then one serial
// `git remote get-url` per remote. These tests pin both halves of the fix: the
// subprocess count at 58 remotes, and result-for-result parity with the old scan
// across the remote shapes a real repo produces.
import { describe, expect, it } from 'vitest'
import { parseGitHubOwnerRepo } from '../github/gh-utils'
import { findRemoteForUrl } from './worktree-push-target-setup'
import type { GitRemoteExec } from './worktree-push-target-cleanup'
const SSH_FORK = 'git@github.com:contributor/orca.git'
const HTTPS_FORK = 'https://github.com/contributor/orca.git'
const GITLAB_FORK = 'https://gitlab.com/contributor/orca.git'
const UPSTREAM = 'https://github.com/stablyai/orca.git'
type RemoteRow = { name: string; fetchUrl: string; pushUrl?: string }
type CountingExec = GitRemoteExec & { spawns: string[][] }
function makeExec(remotes: readonly RemoteRow[]): CountingExec {
const spawns: string[][] = []
const exec: GitRemoteExec = async (args: string[]) => {
spawns.push(args)
if (args[0] === 'remote' && args.length === 1) {
return { stdout: `${remotes.map((remote) => remote.name).join('\n')}\n` }
}
if (args[0] === 'remote' && args[1] === '-v') {
return {
stdout: remotes
.flatMap((remote) => [
`${remote.name}\t${remote.fetchUrl} (fetch)`,
`${remote.name}\t${remote.pushUrl ?? remote.fetchUrl} (push)`
])
.join('\n')
}
}
if (args[0] === 'remote' && args[1] === 'get-url') {
const match = remotes.find((remote) => remote.name === args[2])
if (!match) {
throw new Error(`No such remote ${args[2]}`)
}
return { stdout: `${match.fetchUrl}\n` }
}
throw new Error(`unexpected git command: ${args.join(' ')}`)
}
return Object.assign(exec, { spawns })
}
/** The pre-fix scan, kept as the oracle the batched form must reproduce exactly. */
async function findRemoteForUrlPerRemote(
execGit: GitRemoteExec,
repoPath: string,
remoteUrl: string
): Promise<string | null> {
const target = parseGitHubOwnerRepo(remoteUrl)
try {
const { stdout } = await execGit(['remote'], repoPath)
for (const remote of stdout
.split(/\r?\n/)
.map((line) => line.trim())
.filter(Boolean)) {
try {
const { stdout: urlStdout } = await execGit(['remote', 'get-url', remote], repoPath)
const candidateUrl = urlStdout.trim()
const candidate = parseGitHubOwnerRepo(candidateUrl)
if (
target &&
candidate &&
target.owner.toLowerCase() === candidate.owner.toLowerCase() &&
target.repo.toLowerCase() === candidate.repo.toLowerCase()
) {
return remote
}
if (candidateUrl === remoteUrl) {
return remote
}
} catch {
// Ignore a remote that disappeared or has no fetch URL.
}
}
} catch {
return null
}
return null
}
const fiftyEightRemotes: RemoteRow[] = [
{ name: 'origin', fetchUrl: UPSTREAM },
...Array.from({ length: 56 }, (_, index) => ({
name: `pr-user${index}-orca`,
fetchUrl: `https://github.com/user${index}/orca.git`
})),
{ name: 'pr-contributor-orca', fetchUrl: SSH_FORK }
]
const matrix: { name: string; remotes: RemoteRow[]; lookupUrl: string }[] = [
{ name: 'no remotes', remotes: [], lookupUrl: SSH_FORK },
{
name: 'one matching remote',
remotes: [{ name: 'origin', fetchUrl: SSH_FORK }],
lookupUrl: SSH_FORK
},
{
name: 'one non-matching remote',
remotes: [{ name: 'origin', fetchUrl: UPSTREAM }],
lookupUrl: SSH_FORK
},
{ name: '58 remotes, match last', remotes: fiftyEightRemotes, lookupUrl: SSH_FORK },
{
name: '58 remotes, no match',
remotes: fiftyEightRemotes,
lookupUrl: 'https://github.com/nobody/other.git'
},
{
name: 'duplicate URLs on two remotes',
remotes: [
{ name: 'origin', fetchUrl: UPSTREAM },
{ name: 'fork-a', fetchUrl: SSH_FORK },
{ name: 'fork-b', fetchUrl: SSH_FORK }
],
lookupUrl: SSH_FORK
},
{
name: 'fetch and push URLs differ',
remotes: [{ name: 'split', fetchUrl: SSH_FORK, pushUrl: HTTPS_FORK }],
lookupUrl: SSH_FORK
},
{
name: 'SSH-form lookup against an HTTPS-form remote',
remotes: [
{ name: 'origin', fetchUrl: UPSTREAM },
{ name: 'fork', fetchUrl: HTTPS_FORK }
],
lookupUrl: SSH_FORK
},
{
name: 'HTTPS-form lookup against an SSH-form remote',
remotes: [
{ name: 'origin', fetchUrl: UPSTREAM },
{ name: 'fork', fetchUrl: SSH_FORK }
],
lookupUrl: HTTPS_FORK
},
{
name: 'non-GitHub provider matches only on the exact URL',
remotes: [{ name: 'gitlab-fork', fetchUrl: GITLAB_FORK }],
lookupUrl: GITLAB_FORK
},
{
name: 'non-GitHub provider with a different host does not match',
remotes: [{ name: 'gitlab-fork', fetchUrl: GITLAB_FORK }],
lookupUrl: 'https://bitbucket.org/contributor/orca.git'
}
]
describe('findRemoteForUrl', () => {
it.each(matrix)('matches the per-remote scan for $name', async ({ remotes, lookupUrl }) => {
const expected = await findRemoteForUrlPerRemote(makeExec(remotes), '/repo', lookupUrl)
await expect(findRemoteForUrl(makeExec(remotes), '/repo', lookupUrl)).resolves.toBe(expected)
})
it('answers from one subprocess at 58 remotes instead of one per remote', async () => {
const legacyExec = makeExec(fiftyEightRemotes)
await findRemoteForUrlPerRemote(legacyExec, '/repo', 'https://github.com/nobody/other.git')
expect(legacyExec.spawns).toHaveLength(fiftyEightRemotes.length + 1)
const exec = makeExec(fiftyEightRemotes)
await findRemoteForUrl(exec, '/repo', 'https://github.com/nobody/other.git')
expect(exec.spawns).toEqual([['remote', '-v']])
})
it('returns null when the remote table cannot be read', async () => {
const failing: GitRemoteExec = async () => {
throw new Error('not a git repository')
}
await expect(findRemoteForUrl(failing, '/repo', SSH_FORK)).resolves.toBeNull()
})
})
@@ -16,6 +16,13 @@ const REPO = '/repo-root'
const FORK_SSH = 'git@github.com:contributor/orca.git'
const FORK_HTTPS = 'https://github.com/contributor/orca.git'
/** Real `git remote -v` shape: a fetch row and a push row per remote, tab-separated. */
export function renderRemoteVerbose(remotes: Record<string, string>): string {
return Object.entries(remotes)
.flatMap(([name, url]) => [`${name}\t${url} (fetch)`, `${name}\t${url} (push)`])
.join('\n')
}
// A stateful fake git: `remotes` maps name -> url. `remote add` mutates it so
// later lookups see the new remote, matching real git behavior. Defaults
// `symbolic-ref --short HEAD` to a real branch name, since a worktree's HEAD
@@ -31,6 +38,9 @@ function makeRepoExec(
if (args[0] === 'remote' && args.length === 1) {
return { stdout: Object.keys(remotes).join('\n'), stderr: '' }
}
if (args[0] === 'remote' && args[1] === '-v' && args.length === 2) {
return { stdout: renderRemoteVerbose(remotes), stderr: '' }
}
if (args[0] === 'remote' && args[1] === 'get-url') {
const url = remotes[args[2]!]
if (!url) {
+11 -42
View File
@@ -5,53 +5,33 @@
// repo. The store-aware ownership decision stays with the caller via a predicate.
import type { GitPushTarget } from '../../shared/worktree/types'
import { parseGitHubOwnerRepo } from '../github/gh-utils'
import type { GitRemoteExec } from './worktree-push-target-cleanup'
import { findGitRemoteNameByFetchUrl } from '../../shared/git-remote-url-index'
import { sameGitHubRemoteUrl, type GitRemoteExec } from './worktree-push-target-cleanup'
import {
buildNarrowForkFetchRefspec,
ensureRemoteTracksBranchNarrowly
} from '../git/fork-remote-refspec'
// One `git remote -v` replaces `git remote` plus a serial `git remote get-url` per
// remote -- 59 subprocesses at 58 remotes, on every push-target resolution (#17914).
export async function findRemoteForUrl(
execGit: GitRemoteExec,
repoPath: string,
remoteUrl: string
): Promise<string | null> {
const target = parseGitHubOwnerRepo(remoteUrl)
try {
const { stdout } = await execGit(['remote'], repoPath)
for (const remote of stdout
.split(/\r?\n/)
.map((line) => line.trim())
.filter(Boolean)) {
try {
const { stdout: urlStdout } = await execGit(['remote', 'get-url', remote], repoPath)
const candidateUrl = urlStdout.trim()
const candidate = parseGitHubOwnerRepo(candidateUrl)
if (
target &&
candidate &&
target.owner.toLowerCase() === candidate.owner.toLowerCase() &&
target.repo.toLowerCase() === candidate.repo.toLowerCase()
) {
return remote
}
if (candidateUrl === remoteUrl) {
return remote
}
} catch {
// Ignore a remote that disappeared or has no fetch URL.
}
}
const { stdout } = await execGit(['remote', '-v'], repoPath)
return findGitRemoteNameByFetchUrl(stdout, (candidateUrl) =>
sameGitHubRemoteUrl(candidateUrl, remoteUrl)
)
} catch {
return null
}
return null
}
// O(1) probe used before materializing on demand (push/pull/fetch/fast-forward):
// a single `remote get-url <name>` avoids the O(remotes) `findRemoteForUrl` scan
// once a fork remote already exists under its expected name (#17828).
// a single `remote get-url <name>` skips the whole-remote-table read once a fork
// remote already exists under its expected name (#17828).
export async function remoteAlreadyMatchesUrl(
execGit: GitRemoteExec,
repoPath: string,
@@ -60,18 +40,7 @@ export async function remoteAlreadyMatchesUrl(
): Promise<boolean> {
try {
const { stdout } = await execGit(['remote', 'get-url', remoteName], repoPath)
const candidateUrl = stdout.trim()
if (candidateUrl === remoteUrl) {
return true
}
const target = parseGitHubOwnerRepo(remoteUrl)
const candidate = parseGitHubOwnerRepo(candidateUrl)
return Boolean(
target &&
candidate &&
target.owner.toLowerCase() === candidate.owner.toLowerCase() &&
target.repo.toLowerCase() === candidate.repo.toLowerCase()
)
return sameGitHubRemoteUrl(stdout.trim(), remoteUrl)
} catch {
return false
}
@@ -553,6 +553,17 @@ describe('materializeWorktreePushTargetRemoteSsh', () => {
}
throw new Error('No such remote')
}
if (args[0] === 'remote' && args[1] === '-v') {
return {
stdout: [
'origin\thttps://github.com/stablyai/orca.git (fetch)',
'origin\thttps://github.com/stablyai/orca.git (push)',
`${SIBLING_REMOTE}\t${FORK_URL} (fetch)`,
`${SIBLING_REMOTE}\t${FORK_URL} (push)`
].join('\n'),
stderr: ''
}
}
if (args[0] === 'remote' && args.length === 1) {
return { stdout: `origin\n${SIBLING_REMOTE}\n`, stderr: '' }
}
@@ -35,6 +35,8 @@ export function normalizeLoadedProfileState(
const { defaults, migratedExternalVisibility, osc52ClipboardNoticePending } = terminal
const { normalizedOnboarding, normalizedProjectGroups, loadedCompactWorktreeCards } = profile
const projectCatalog = normalizeLoadedProjectCatalog(parsed, markNeedsSave)
// Ordered: the host partitions drop the global fields this slice already owns.
const workspaceSession = normalizeLoadedLocalSession(parsed, defaults, markNeedsSave)
return {
...defaults,
@@ -69,9 +71,14 @@ export function normalizeLoadedProfileState(
markNeedsSave
),
// Why: volatile schema; zod-validate workspaceSession at read so a bad payload falls to defaults, not a renderer crash.
workspaceSession: normalizeLoadedLocalSession(parsed, defaults, markNeedsSave),
workspaceSession,
// Why: per-host session partitions, validated independently; 'local' stays in workspaceSession for downgrade compat.
workspaceSessionsByHostId: normalizeLoadedHostSessions(parsed, defaults, markNeedsSave),
workspaceSessionsByHostId: normalizeLoadedHostSessions(
parsed,
defaults,
workspaceSession,
markNeedsSave
),
sshTargets: (parsed.sshTargets ?? []).map(normalizeSshTarget),
deletedSshConfigAliases: Array.isArray(parsed.deletedSshConfigAliases)
? parsed.deletedSshConfigAliases.filter((alias): alias is string => typeof alias === 'string')
@@ -41,11 +41,13 @@ export function normalizeLoadedLocalSession(
export function normalizeLoadedHostSessions(
parsed: PersistedState,
defaults: PersistedState,
localSession: WorkspaceSessionState,
markNeedsSave: () => void
): PersistedState['workspaceSessionsByHostId'] {
const { partitions, repaired } = parseWorkspaceSessionsByHostId(
parsed.workspaceSessionsByHostId,
defaults.workspaceSession
defaults.workspaceSession,
localSession
)
if (repaired) {
// Why: salvage repairs only the in-memory partitions; without a save the corrupt entries stay on disk and get re-dropped every launch.
@@ -161,7 +161,8 @@ export async function writeToDiskAsync(owner: PrimaryStateWriteOperations): Prom
// Why: fsync before rename, then fsync the directory; see writeFileDurable.
const handle = await open(tmpFile, 'w')
try {
await handle.writeFile(payload, 'utf-8')
// Already UTF-8 bytes: passing the string here would re-encode the whole state on the main thread.
await handle.writeFile(payload)
await handle.sync()
} finally {
await handle.close()
@@ -0,0 +1,184 @@
/**
* The bar for this change is "the bytes on disk did not move". Every case below runs the exact
* loop `applySecretSentinelSubstitutions` replaced — reproduced in `previousImplementation` — and
* compares payload bytes and guard hash, because a drifting hash silently disables the no-op write
* guard and a drifting payload is corrupted persisted state.
*/
import { createHash, randomUUID } from 'node:crypto'
import { describe, expect, it } from 'vitest'
import {
applySecretSentinelSubstitutions,
type SecretSentinelSubstitution
} from './secret-sentinel-substitution'
/** Verbatim from state-serialization-secret-handling.ts before this change. */
function previousImplementation(
serialized: string,
secretSubs: readonly SecretSentinelSubstitution[],
degradedPrefix: string
): { payload: Buffer; stateHash: string } {
let payload = serialized
let hashInput = serialized
for (const { sentinel, blob, hashValue } of secretSubs) {
const escapedSentinel = JSON.stringify(sentinel).slice(1, -1)
payload = payload.replace(escapedSentinel, () => JSON.stringify(blob).slice(1, -1))
hashInput = hashInput.replace(escapedSentinel, () => JSON.stringify(hashValue).slice(1, -1))
}
const stateHash = createHash('sha1').update(degradedPrefix).update(hashInput).digest('hex')
// `handle.writeFile(payload, 'utf-8')` is what turned the string into bytes.
return { payload: Buffer.from(payload, 'utf8'), stateHash }
}
function expectIdenticalToPrevious(
serialized: string,
subs: readonly SecretSentinelSubstitution[],
degradedPrefix = ''
): void {
const before = previousImplementation(serialized, subs, degradedPrefix)
const after = applySecretSentinelSubstitutions(serialized, subs, degradedPrefix)
expect(after.payload.equals(before.payload)).toBe(true)
expect(after.stateHash).toBe(before.stateHash)
}
function sentinel(): string {
return `orca-secret-slot-${randomUUID()}`
}
describe('applySecretSentinelSubstitutions', () => {
it('produces bytes and a hash identical to the previous implementation', () => {
const subs: SecretSentinelSubstitution[] = [
{ sentinel: sentinel(), blob: 'djEwY2lwaGVy', hashValue: 'cookie-value' },
{
sentinel: sentinel(),
// Regex-special *and* JSON-escapable, which is the pair that breaks a naive rewrite:
// `$&` would splice the match back in under string-form replace, and the backslash and
// quote have to survive `JSON.stringify(...).slice(1, -1)` unchanged.
blob: 'A+/=$&$1$`\\x "quoted" |.*?[](){}^',
hashValue: 'http://proxy.example:8080/?a=b&c=$&'
},
{ sentinel: sentinel(), blob: '', hashValue: 'https://kagi.com/session?t=abc' }
]
const state = {
settings: { opencodeSessionCookie: subs[0].sentinel, httpProxyUrl: subs[1].sentinel },
ui: { browserKagiSessionLink: subs[2].sentinel },
// Adjacent content that must not shift: a near-miss prefix, and JSON escapes either side.
noise: ['orca-secret-slot-', 'a\\b"c\n\t', subs[0].sentinel.slice(0, -1)]
}
expectIdenticalToPrevious(JSON.stringify(state), subs)
})
it('stays identical when the state holds multi-byte and escaped characters', () => {
const subs: SecretSentinelSubstitution[] = [
{ sentinel: sentinel(), blob: 'blob-é', hashValue: 'plain-é' },
{ sentinel: sentinel(), blob: '😀', hashValue: '中文' }
]
const state = {
// Segment boundaries land next to these, so a wrong split would corrupt the encode.
before: 'é中文😀',
a: subs[0].sentinel,
between: '😀

',
b: subs[1].sentinel,
after: '😀'
}
expectIdenticalToPrevious(JSON.stringify(state), subs)
})
it('stays identical with no substitutions and with the degraded-storage prefix', () => {
const state = JSON.stringify({ settings: { httpProxyUrl: '' }, big: 'x'.repeat(4096) })
expectIdenticalToPrevious(state, [])
expectIdenticalToPrevious(state, [], 'safeStorage-degraded\0')
const subs = [{ sentinel: sentinel(), blob: 'b', hashValue: 'h' }]
expectIdenticalToPrevious(
JSON.stringify({ s: subs[0].sentinel }),
subs,
'safeStorage-degraded\0'
)
})
it('escapes regex metacharacters in the sentinel itself', () => {
// Not reachable from a UUID sentinel, but the alternation must not be able to become a pattern.
const subs = [{ sentinel: 'a.b*c(d)|e[f]', blob: 'BLOB', hashValue: 'HASH' }]
const serialized = JSON.stringify({ real: subs[0].sentinel, decoy: 'axbxxcXdX_eXfX' })
expectIdenticalToPrevious(serialized, subs)
expect(
applySecretSentinelSubstitutions(serialized, subs, '').payload.toString('utf8')
).toContain('axbxxcXdX_eXfX')
})
it('substitutes every occurrence when a sentinel repeats', () => {
// Cannot happen today (a sentinel is a UUID minted after the state is assembled, so it appears
// exactly once), but the old first-match-only `String.replace` would have written a raw
// sentinel to disk in place of a secret if it ever did. The alternation is global instead.
const subs = [{ sentinel: sentinel(), blob: 'CIPHER', hashValue: 'PLAIN' }]
const serialized = JSON.stringify({ a: subs[0].sentinel, b: subs[0].sentinel })
const { payload } = applySecretSentinelSubstitutions(serialized, subs, '')
expect(payload.toString('utf8')).toBe(JSON.stringify({ a: 'CIPHER', b: 'CIPHER' }))
expect(payload.toString('utf8')).not.toContain(subs[0].sentinel)
})
it('copies and UTF-8 encodes the full state once, not once per sentinel per side', () => {
const subs: SecretSentinelSubstitution[] = Array.from({ length: 3 }, () => ({
sentinel: sentinel(),
blob: 'CIPHERTEXT',
hashValue: 'plaintext'
}))
const serialized = JSON.stringify({
pad: 'x'.repeat(200_000),
a: subs[0].sentinel,
b: subs[1].sentinel,
c: subs[2].sentinel
})
const FULL_STATE = 100_000
// Both costs are observable at their sources: a `String.replace` whose receiver is the whole
// state allocates another copy of it, and every string handed to `Buffer.from` or `hash.update`
// is one full UTF-8 encode pass on the main thread.
const counted = (run: () => unknown): { fullStateReplaces: number; encodedChars: number } => {
const realReplace = String.prototype.replace
const realBufferFrom = Buffer.from
const hashProto = Object.getPrototypeOf(createHash('sha1')) as {
update: (...args: unknown[]) => unknown
}
const realUpdate = hashProto.update
const counts = { fullStateReplaces: 0, encodedChars: 0 }
String.prototype.replace = function (this: string, ...args: unknown[]) {
if (this.length >= FULL_STATE) {
counts.fullStateReplaces++
}
return realReplace.apply(this, args as never)
} as typeof String.prototype.replace
Buffer.from = function (...args: unknown[]) {
if (typeof args[0] === 'string') {
counts.encodedChars += args[0].length
}
return (realBufferFrom as (...a: unknown[]) => Buffer).apply(Buffer, args)
} as typeof Buffer.from
hashProto.update = function (this: unknown, ...args: unknown[]) {
if (typeof args[0] === 'string') {
counts.encodedChars += args[0].length
}
return realUpdate.apply(this, args)
}
try {
run()
} finally {
String.prototype.replace = realReplace
Buffer.from = realBufferFrom
hashProto.update = realUpdate
}
return counts
}
const before = counted(() => previousImplementation(serialized, subs, ''))
const after = counted(() => applySecretSentinelSubstitutions(serialized, subs, ''))
// Two `String.replace` calls over the whole state per sentinel — payload and hash input.
expect(before.fullStateReplaces).toBe(subs.length * 2)
expect(after.fullStateReplaces).toBe(0)
// The old path encoded the state twice: once for sha1, once for the file write.
expect(before.encodedChars).toBeGreaterThan(serialized.length * 1.9)
expect(after.encodedChars).toBeLessThan(serialized.length * 1.1)
expect(after.encodedChars).toBeGreaterThan(serialized.length * 0.9)
})
})
@@ -0,0 +1,78 @@
import { createHash } from 'node:crypto'
import { escapeRegex } from '../../../shared/string-utils'
export type SecretSentinelSubstitution = {
/** The `orca-secret-slot-<uuid>` placeholder standing in the serialized state. */
sentinel: string
/** What the on-disk payload gets: the ciphertext. */
blob: string
/** What the guard hash gets: a value stable across non-deterministic encryption. */
hashValue: string
}
/**
* Replace every secret sentinel in `serialized` in ONE pass, producing the on-disk bytes and the
* guard hash from the same encoded segments.
*
* Why not the obvious `payload.replace(...)` / `hashInput.replace(...)` loop it replaces: each
* `String.replace` returns a rope that the *next* `replace` has to flatten before it can search, so
* N sentinels cost 2N-1 flattened copies of the whole multi-MB state, plus one more per side when
* `hash.update` and the file write finally consume them. Measured on a 4.65 MB store with three
* sentinels: 7 full-state string allocations, 62 MB of V8 heap, 27 MB of it in large_object_space.
*
* Here the state is walked once, each literal run is UTF-8 encoded exactly once, and those same
* buffers feed both the payload and the hash — 1 full-state string, 1 encode.
*
* Byte-for-byte identical output to the loop: both sides read the sentinel in its JSON-escaped
* form, the replacements are the JSON-escaped `blob`/`hashValue`, and the hash sees the same byte
* sequence it saw when it was handed one concatenated string.
*/
export function applySecretSentinelSubstitutions(
serialized: string,
substitutions: readonly SecretSentinelSubstitution[],
degradedPrefix: string
): { payload: Buffer; stateHash: string } {
const hash = createHash('sha1').update(degradedPrefix)
if (substitutions.length === 0) {
const payload = Buffer.from(serialized, 'utf8')
return { payload, stateHash: hash.update(payload).digest('hex') }
}
const replacementBySentinel = new Map<string, { blob: Buffer; hashValue: Buffer }>()
const alternatives: string[] = []
for (const { sentinel, blob, hashValue } of substitutions) {
// Preserved from the loop this replaces: both the search key and the replacements are the
// JSON-escaped forms, because that is what `serialized` actually contains.
const escapedSentinel = JSON.stringify(sentinel).slice(1, -1)
if (replacementBySentinel.has(escapedSentinel)) {
continue
}
alternatives.push(escapeRegex(escapedSentinel))
replacementBySentinel.set(escapedSentinel, {
blob: Buffer.from(JSON.stringify(blob).slice(1, -1), 'utf8'),
hashValue: Buffer.from(JSON.stringify(hashValue).slice(1, -1), 'utf8')
})
}
// Global, though a sentinel is a UUID minted after the state was assembled and so occurs exactly
// once: a single pass that substitutes every occurrence cannot leave one behind on disk.
const pattern = new RegExp(alternatives.join('|'), 'g')
const chunks: Buffer[] = []
let cursor = 0
let match: RegExpExecArray | null
while ((match = pattern.exec(serialized)) !== null) {
// Non-null: the alternation is built from exactly the map's keys.
const replacement = replacementBySentinel.get(match[0])!
// A sliced substring, so this does not copy the state; the encode below is its only pass.
const literal = Buffer.from(serialized.slice(cursor, match.index), 'utf8')
chunks.push(literal, replacement.blob)
hash.update(literal)
hash.update(replacement.hashValue)
cursor = match.index + match[0].length
}
const tail = Buffer.from(serialized.slice(cursor), 'utf8')
chunks.push(tail)
hash.update(tail)
return { payload: Buffer.concat(chunks), stateHash: hash.digest('hex') }
}
@@ -1,4 +1,4 @@
import { createHash, randomUUID } from 'node:crypto'
import { randomUUID } from 'node:crypto'
import type { PersistedState } from '../../../shared/persisted-state-types'
import { collectFolderWorkspaceDiffComments } from '../../folder-workspace-diff-comments'
import {
@@ -8,6 +8,10 @@ import {
} from '../../protected-secret-persistence'
import { stripRetiredGlobalSettings } from '../applying-settings/terminal-settings-migrations'
import {
applySecretSentinelSubstitutions,
type SecretSentinelSubstitution
} from './secret-sentinel-substitution'
import type { StoreRuntimeState } from './store-runtime-state'
type StateSerializationSecretHandlingOperationsRuntime = Pick<
@@ -24,7 +28,7 @@ export class StateSerializationSecretHandlingOperations {
}
buildStateToSave(): {
payload: string
payload: Buffer
stateHash: string
protectedSecretUpdates: ProtectedSecretRetentionUpdate[]
} {
@@ -37,7 +41,7 @@ export class StateSerializationSecretHandlingOperations {
// on deterministic-IV platforms (macOS/legacy-Linux OSCrypt). A per-slot
// random UUID can't occur anywhere else in the serialized state (the user
// sets their data before it is minted), so it appears exactly once.
const secretSubs: { sentinel: string; blob: string; hashValue: string }[] = []
const secretSubs: SecretSentinelSubstitution[] = []
const protectedSecretUpdates: ProtectedSecretRetentionUpdate[] = []
let protectedStorageDegraded = false
const encryptToSentinel = (slot: string, plaintext: string): string => {
@@ -105,21 +109,14 @@ export class StateSerializationSecretHandlingOperations {
// Why compact: ~20% fewer bytes and less serialize time; all readers JSON.parse so formatting is irrelevant.
// One full-state stringify; secret slots currently hold sentinels.
const serialized = JSON.stringify(stateToSave)
// Substitute each unique sentinel exactly once: ciphertext for the on-disk
// payload, a stable normalized value for the guard hash. Function-form
// replacement keeps `$` inert; both sides read the sentinel as JSON-escaped
// in `serialized`, so each replace is byte-for-byte position-exact.
let payload = serialized
let hashInput = serialized
for (const { sentinel, blob, hashValue } of secretSubs) {
const escapedSentinel = JSON.stringify(sentinel).slice(1, -1)
payload = payload.replace(escapedSentinel, () => JSON.stringify(blob).slice(1, -1))
hashInput = hashInput.replace(escapedSentinel, () => JSON.stringify(hashValue).slice(1, -1))
}
const stateHash = createHash('sha1')
.update(protectedStorageDegraded ? 'safeStorage-degraded\0' : '')
.update(hashInput)
.digest('hex')
// Substitute each unique sentinel: ciphertext for the on-disk payload, a stable normalized
// value for the guard hash. One pass builds both, so the multi-MB state is never copied per
// sentinel and never encoded twice.
const { payload, stateHash } = applySecretSentinelSubstitutions(
serialized,
secretSubs,
protectedStorageDegraded ? 'safeStorage-degraded\0' : ''
)
return { payload, stateHash, protectedSecretUpdates }
}
}
@@ -0,0 +1,131 @@
/**
* The write path now hands the file a Buffer it built in one pass instead of a string it rebuilt
* per secret. Drives the real `Store` end to end — encrypted settings, a local session and a remote
* host partition — and reloads from the file it actually wrote, because the failure this guards
* against (a mis-sliced segment, a re-encoded payload, a dropped sentinel) is invisible until
* something reads the bytes back.
*/
import { mkdtempSync, readFileSync, realpathSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { afterEach, describe, expect, it, vi } from 'vitest'
import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types'
vi.mock('electron', () => ({
app: {
getPath: () => tmpdir(),
getName: () => 'orca-test',
getVersion: () => '0.0.0-test',
isPackaged: false,
on: () => {},
whenReady: () => Promise.resolve()
},
safeStorage: {
// Encryption ON, so the secret slots really do mint sentinels and the substitution pass runs.
isEncryptionAvailable: () => true,
encryptString: (value: string) => Buffer.from(`enc:${value}`),
decryptString: (value: Buffer) => value.toString().slice(4)
},
ipcMain: { on: () => {}, handle: () => {} },
BrowserWindow: { getAllWindows: () => [] }
}))
const { Store } = await import('./store')
const HOST_ID = 'ssh:user@host'
const stores: InstanceType<typeof Store>[] = []
afterEach(() => {
for (const store of stores.splice(0)) {
store.flush()
}
vi.restoreAllMocks()
})
function openStore(dataFile: string): InstanceType<typeof Store> {
const store = new Store({ dataFile })
stores.push(store)
return store
}
function session(activeTabId: string): WorkspaceSessionState {
return {
activeRepoId: 'repo-1',
// Left null: the load path's deregistered-repo sweep nulls an active worktree whose repo is
// not registered, which would mask what this test is actually about.
activeWorktreeId: null,
activeTabId,
tabsByWorktree: {},
terminalLayoutsByTabId: {},
// Non-ASCII on purpose: a byte-offset mistake in the encode shows up here first.
browserUrlHistory: [
{
url: 'https://example.test/é😀',
normalizedUrl: 'https://example.test/é😀',
title: '中文 title',
lastVisitedAt: 17,
visitCount: 3
}
]
} as WorkspaceSessionState
}
describe('persisted state survives a save/load round trip', () => {
it('reloads settings, secrets and both session partitions unchanged', () => {
const dataFile = join(
realpathSync(mkdtempSync(join(tmpdir(), 'orca-store-round-trip-'))),
'orca-data.json'
)
const written = openStore(dataFile)
written.updateSettings({
// Three secret slots, i.e. three sentinels in one save — the case the old loop paid 7 copies for.
opencodeSessionCookie: 'cookie-é-value',
httpProxyUrl: 'http://proxy.example:8080/?a=b&c=$&'
})
written.updateUI({ browserKagiSessionLink: 'https://kagi.com/session?t=abc' })
written.setWorkspaceSession(session('local-tab'))
written.setWorkspaceSession(session('remote-tab'), HOST_ID)
written.flush()
const before = {
settings: written.getSettings(),
ui: written.getUI(),
local: written.getWorkspaceSession(),
remote: written.getWorkspaceSession(HOST_ID)
}
// The file is valid UTF-8 JSON and holds ciphertext, not the plaintext secrets.
const bytes = readFileSync(dataFile)
const onDisk = JSON.parse(bytes.toString('utf8'))
expect(onDisk.settings.opencodeSessionCookie).not.toBe('cookie-é-value')
expect(Buffer.from(onDisk.settings.opencodeSessionCookie, 'base64').toString('utf8')).toContain(
'cookie-é-value'
)
expect(bytes.toString('utf8')).not.toContain('orca-secret-slot-')
const reloaded = openStore(dataFile)
expect(reloaded.getSettings().opencodeSessionCookie).toBe(before.settings.opencodeSessionCookie)
expect(reloaded.getSettings().httpProxyUrl).toBe(before.settings.httpProxyUrl)
expect(reloaded.getUI().browserKagiSessionLink).toBe(before.ui.browserKagiSessionLink)
// `toMatchObject`: the load path spreads session defaults over what was written, so the
// reloaded slice is a superset. Exact deep equality is asserted on the second trip below.
expect(reloaded.getWorkspaceSession()).toMatchObject(before.local)
// The remote partition keeps everything it owns; only globals local already holds are dropped,
// and `browserUrlHistory` comes back at its default from the same spread as before.
expect(reloaded.getWorkspaceSession(HOST_ID).activeTabId).toBe('remote-tab')
expect(reloaded.getWorkspaceSession(HOST_ID).browserUrlHistory).toEqual([])
// Deep equality of the whole reloaded state, taken across a second round trip so the assertion
// is not comparing against the first load's one-time settings migrations.
reloaded.flush()
const bytesAfterReload = readFileSync(dataFile)
const again = openStore(dataFile)
expect(again.getSettings()).toEqual(reloaded.getSettings())
expect(again.getUI()).toEqual(reloaded.getUI())
expect(again.getWorkspaceSession()).toEqual(reloaded.getWorkspaceSession())
expect(again.getWorkspaceSession(HOST_ID)).toEqual(reloaded.getWorkspaceSession(HOST_ID))
// ...and the bytes are stable, so a quiet app is not rewriting a 4 MB file with new content.
again.flush()
expect(readFileSync(dataFile).equals(bytesAfterReload)).toBe(true)
})
})
@@ -0,0 +1,141 @@
/**
* Global session fields live in the 'local' slice. Copies of them inside a non-local host partition
* are legacy residue: the split never writes them there and the merge never reads them from there
* unless local has nothing. These tests pin the drop to exactly that condition, keep the renderer's
* merge landing on the same value either way, and re-check the two safety gates that decide which
* global fields may be dropped at all.
*/
import { describe, expect, it } from 'vitest'
import { getDefaultWorkspaceSession } from '../../../shared/constants'
import type { BrowserHistoryEntry } from '../../../shared/browser-workspace-types'
import type { WorkspaceDocHistoryEntry } from '../../../shared/workspace-doc-history'
import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types'
import { WORKSPACE_SESSION_FIELD_OWNERSHIP } from '../../../shared/workspace-session-host-field-ownership'
import { WORKSPACE_SESSION_WORKTREE_REFERENCE_KIND } from '../restoring-sessions/session-worktree-ownership'
import {
HOST_PARTITION_REDUNDANT_GLOBAL_FIELDS,
parseWorkspaceSessionsByHostId
} from './workspace-session-partitions'
const HOST = 'ssh:target-1'
function history(url: string): BrowserHistoryEntry[] {
return [{ url, normalizedUrl: url, title: url, lastVisitedAt: 1, visitCount: 1 }]
}
function docEntry(filePath: string): WorkspaceDocHistoryEntry {
return {
docLocation: { kind: 'workspace-doc', worktreeId: 'repo-1::/tmp/a', filePath },
title: filePath,
lastVisitedAt: 2,
visitCount: 1
}
}
function localSession(overrides: Partial<WorkspaceSessionState>): WorkspaceSessionState {
return { ...getDefaultWorkspaceSession(), ...overrides }
}
function parse(
raw: Record<string, unknown>,
local?: WorkspaceSessionState
): Partial<Record<string, WorkspaceSessionState>> {
return parseWorkspaceSessionsByHostId(raw, getDefaultWorkspaceSession(), local).partitions
}
describe('HOST_PARTITION_REDUNDANT_GLOBAL_FIELDS', () => {
it('only lists fields that are global AND that no worktree-ownership pass follows', () => {
for (const field of HOST_PARTITION_REDUNDANT_GLOBAL_FIELDS) {
// Gate 1: the renderer's split/merge treat it as local-owned, so a non-local copy is dead.
expect(WORKSPACE_SESSION_FIELD_OWNERSHIP[field]).toBe('global')
// Gate 2: `collectPersistedSessionWorktreeOwners` and the deregistered-repo residue sweep
// walk EVERY partition through this table. Anything but 'none' means dropping the field
// could un-own a worktree and get its metadata pruned.
expect(WORKSPACE_SESSION_WORKTREE_REFERENCE_KIND[field]).toBe('none')
}
})
})
describe('parseWorkspaceSessionsByHostId global-field residue', () => {
it('drops a non-local global field the local slice already owns', () => {
const local = localSession({ browserUrlHistory: history('https://local.test') })
const partitions = parse(
{
[HOST]: {
...getDefaultWorkspaceSession(),
browserUrlHistory: history('https://stale.test')
}
},
local
)
// Back to the default from the spread, not the 65 KB stale replica. The merge reads this field
// from local whenever local has it, so the renderer still sees `https://local.test`
// (`workspace-session-host-split.test.ts` pins that half of the contract).
expect(partitions[HOST]?.browserUrlHistory).toEqual([])
})
it('retains a non-local global field the local slice does NOT have', () => {
// `workspaceDocHistory` is optional and absent from the defaults, so local can genuinely lack
// it and the merge's fallback to another slice is live.
const local = localSession({})
expect(local.workspaceDocHistory).toBeUndefined()
const docs = [docEntry('/repo/remote.md')]
const partitions = parse(
{ [HOST]: { ...getDefaultWorkspaceSession(), workspaceDocHistory: docs } },
local
)
// Retained, so the merge's "fall back to any slice that has it" path still finds a value.
expect(partitions[HOST]?.workspaceDocHistory).toEqual(docs)
})
it('drops that same field once the local slice does have it', () => {
const localDocs = [docEntry('/repo/local.md')]
const local = localSession({ workspaceDocHistory: localDocs })
const partitions = parse(
{
[HOST]: {
...getDefaultWorkspaceSession(),
workspaceDocHistory: [docEntry('/repo/stale.md')]
}
},
local
)
expect(partitions[HOST]).not.toHaveProperty('workspaceDocHistory')
expect(local.workspaceDocHistory).toEqual(localDocs)
})
it('leaves worktree-referencing globals and host-owned fields alone', () => {
const local = localSession({
browserUrlHistory: history('https://local.test'),
activeWorktreeId: 'repo-1::/tmp/local',
activeTabId: 'local-tab'
})
const tabs = { 'repo-1::/tmp/a': [] }
const partitions = parse(
{
[HOST]: {
...getDefaultWorkspaceSession(),
// A `'direct'` worktree reference the residue sweep reads out of every partition.
activeWorktreeId: 'repo-1::/tmp/a',
// Read on a partition by the mobile terminal projection.
activeTabId: 'remote-tab',
tabsByWorktree: tabs,
terminalTopologyRevisionByRepoId: { 'repo-1': 4 }
}
},
local
)
expect(partitions[HOST]?.activeWorktreeId).toBe('repo-1::/tmp/a')
expect(partitions[HOST]?.activeTabId).toBe('remote-tab')
expect(partitions[HOST]?.tabsByWorktree).toEqual(tabs)
expect(partitions[HOST]?.terminalTopologyRevisionByRepoId).toEqual({ 'repo-1': 4 })
})
it('is a no-op when no local slice is supplied', () => {
const stale = history('https://stale.test')
const partitions = parse({
[HOST]: { ...getDefaultWorkspaceSession(), browserUrlHistory: stale }
})
expect(partitions[HOST]?.browserUrlHistory).toEqual(stale)
})
})
@@ -17,11 +17,49 @@ export function workspaceSessionSalvageLogDetails(result: {
}
}
/**
* Global fields belong to the 'local' slice: the split writes them only there and the merge reads
* them only from there. A copy inside a non-local partition is legacy residue no read can reach —
* stale `browserUrlHistory` replicas alone were 589 KB, 12.7% of a 4.65 MB store, rewritten on
* every save and reparsed on every launch.
*
* Deliberately NOT every field in `GLOBAL_WORKSPACE_SESSION_FIELDS`. Two separate gates disqualify
* the rest, and both are load-bearing:
* - `activeWorktreeId` and `activeWorkspaceKey` are `'direct'` in
* `WORKSPACE_SESSION_WORKTREE_REFERENCE_KIND`, and both `collectPersistedSessionWorktreeOwners`
* and the deregistered-repo residue sweep read them out of EVERY partition. Dropping one
* un-owns a worktree, and an un-owned worktree gets its metadata pruned.
* - `activeTabId`, `activeConnectionIdsAtShutdown` and `activeRepoId` have live main-side readers
* on a partition: `isPersistedTerminalLeafActive` falls back to `activeTabId` for the mobile
* projection, and the runtime attach-window handoff unions `activeConnectionIdsAtShutdown`.
*
* `workspace-session-partitions.test.ts` re-checks both gates for every field listed here.
*/
export const HOST_PARTITION_REDUNDANT_GLOBAL_FIELDS = [
'browserUrlHistory',
'workspaceDocHistory'
] as const satisfies readonly (keyof WorkspaceSessionState)[]
/** Dropped only where local already holds the field — exactly when the merge's fallback to another
* slice cannot fire. Runs before the defaults spread, so a field the type requires comes back at
* its default rather than going missing. */
function dropRedundantGlobalFields(
slice: Partial<WorkspaceSessionState>,
local: WorkspaceSessionState | undefined
): void {
for (const field of HOST_PARTITION_REDUNDANT_GLOBAL_FIELDS) {
if (local?.[field] !== undefined) {
delete slice[field]
}
}
}
/** Normalize non-'local' host partitions; 'local' (the legacy workspaceSession blob) is dropped so the two surfaces never diverge.
* Each partition is zod-validated independently, so one corrupt host drops to defaults without taking out the others. Idempotent. */
export function parseWorkspaceSessionsByHostId(
raw: unknown,
defaults: WorkspaceSessionState
defaults: WorkspaceSessionState,
localSession?: WorkspaceSessionState
): { partitions: Partial<Record<ExecutionHostId, WorkspaceSessionState>>; repaired: boolean } {
if (!raw || typeof raw !== 'object' || Array.isArray(raw)) {
return { partitions: {}, repaired: raw !== undefined }
@@ -50,6 +88,7 @@ export function parseWorkspaceSessionsByHostId(
)
repaired = true
}
dropRedundantGlobalFields(result.value, localSession)
partitions[hostId] = { ...defaults, ...result.value }
}
return { partitions, repaired }
+12 -31
View File
@@ -1,7 +1,9 @@
import { recognizeAgentProcessFromCommandLine } from '../../shared/agent-process-recognition'
import { resolveOuterWrapperForegroundProcess } from '../../shared/foreground-wrapper-agent'
import {
collectDescendantsFromIndex,
getFreshProcessTableSnapshot,
getProcessTableIndex,
getProcessTableSnapshot,
type ProcessTableRow
} from '../../shared/process-table-snapshot'
@@ -43,29 +45,6 @@ type ShellForegroundConfirmationOptions = {
| Promise<ReadonlySet<number> | null>
}
function collectDescendants<Row extends { pid: number; ppid: number }>(
rows: Row[],
rootPid: number
): (Row & { depth: number })[] {
const childrenByParent = new Map<number, Row[]>()
for (const row of rows) {
const children = childrenByParent.get(row.ppid) ?? []
children.push(row)
childrenByParent.set(row.ppid, children)
}
const descendants: (Row & { depth: number })[] = []
const stack = (childrenByParent.get(rootPid) ?? []).map((row) => ({ row, depth: 1 }))
while (stack.length > 0) {
const { row, depth } = stack.pop()!
descendants.push({ ...row, depth })
for (const child of childrenByParent.get(row.pid) ?? []) {
stack.push({ row: child, depth: depth + 1 })
}
}
return descendants
}
function commandExecutable(command: string): string {
const trimmed = command.trim().replace(/^[-]/, '')
if (trimmed.startsWith('"') || trimmed.startsWith("'")) {
@@ -97,12 +76,12 @@ export async function confirmShellForegroundProcess(
}
}
try {
const rows = await getFreshProcessTableSnapshot()
if (!rows.some((row) => row.pid === shellPid)) {
const index = getProcessTableIndex(await getFreshProcessTableSnapshot())
const root = index.byPid.get(shellPid)
if (!root) {
return false
}
const root = rows.find((row) => row.pid === shellPid)!
const tree = [{ ...root, depth: 0 }, ...collectDescendants(rows, shellPid)]
const tree = [{ ...root, depth: 0 }, ...collectDescendantsFromIndex(index, shellPid)]
const spawnedShellBasename = executableBasename(spawnedShellProcess)
const foregroundShell = tree
.filter(
@@ -172,7 +151,7 @@ export async function resolveAgentForegroundProcessWithAvailability(
const rows = options.fresh
? await getFreshProcessTableSnapshot()
: await getProcessTableSnapshot()
if (options.fresh && !rows.some((row) => row.pid === shellPid)) {
if (options.fresh && !getProcessTableIndex(rows).byPid.has(shellPid)) {
return { available: false, processName: fallbackProcess }
}
return {
@@ -186,11 +165,13 @@ export async function resolveAgentForegroundProcessWithAvailability(
}
export function resolveAgentForegroundProcessFromPs(
rows: ProcessTableRow[],
rows: readonly ProcessTableRow[],
shellPid: number
): string | null {
const shellRow = rows.find((row) => row.pid === shellPid)
const candidates = collectDescendants(rows, shellPid)
// Memoized per snapshot identity, so the caller's own index build is reused.
const index = getProcessTableIndex(rows)
const shellRow = index.byPid.get(shellPid)
const candidates = collectDescendantsFromIndex(index, shellPid)
// Why: `+` in `ps stat` marks the process holding the terminal foreground.
// The root shell can hold it after Ctrl-Z, so use the whole PTY tree as the
// foreground gate; otherwise a stopped agent child still masquerades as live.
@@ -0,0 +1,156 @@
// Regression guard on the per-inspection cost of Windows agent foreground
// inspection — the Windows analogue of the POSIX index memo (#6288).
//
// The shared TTL cache already collapses N panes into one Toolhelp32 snapshot
// (windows-agent-foreground-process-scan-volume.test.ts). What it never
// collapsed is the work each pane does ON that snapshot: a full
// `native.map(toProcessRow)` projection, a `childrenByPpid` Map rebuilt from
// scratch, and two linear scans. This file counts that work at a realistic
// table size and pane count, and pins the flag set the snapshot asks for.
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { __setWindowsProcessTreeLoaderForTests } from '../windows/windows-process-table'
import {
queryWindowsPaneProcessInventory,
resetWindowsProcessRowsSnapshotForTests
} from './windows-foreground-process-rows'
// 1050 processes is the host measured in windows-process-enumeration.md; 11
// panes is the fan-out the shared snapshot exists to serve.
const TABLE_SIZE = 1050
const PANE_COUNT = 11
const SELF_ROW = { pid: process.pid, ppid: 0, name: 'vitest.exe', commandLine: 'vitest' }
const shellPid = (pane: number): number => 10_000 + pane * 10
const agentPid = (pane: number): number => shellPid(pane) + 1
/** A row every pane can look up, so distinct results == distinct projections. */
const PROBE_PID = 900_000 + TABLE_SIZE - 1
/** One shell + one agent child per pane, padded out to a real table size. */
function buildNativeTable(): { pid: number; ppid: number; name: string; commandLine: string }[] {
const rows = [SELF_ROW]
for (let pane = 0; pane < PANE_COUNT; pane += 1) {
rows.push({ pid: shellPid(pane), ppid: 4, name: 'cmd.exe', commandLine: 'cmd.exe' })
rows.push({
pid: agentPid(pane),
ppid: shellPid(pane),
name: 'node.exe',
commandLine: 'node C:/Users/dev/AppData/codex/bin/codex.js'
})
}
for (let filler = rows.length; filler < TABLE_SIZE; filler += 1) {
rows.push({ pid: 900_000 + filler, ppid: 4, name: 'svchost.exe', commandLine: 'svchost.exe' })
}
return rows
}
const NATIVE_TABLE = buildNativeTable()
/**
* Count `Map.prototype.set` calls — the primitive both the old per-call
* `childrenByPpid` rebuild and the shared index build are made of. Patched for
* one awaited region and restored in `finally`, so nothing else observes it.
*/
async function countMapInsertions(run: () => Promise<void>): Promise<number> {
const original = Map.prototype.set
let insertions = 0
Map.prototype.set = function patched(this: Map<unknown, unknown>, key: unknown, value: unknown) {
insertions += 1
return original.call(this, key, value)
} as typeof Map.prototype.set
try {
await run()
} finally {
Map.prototype.set = original
}
return insertions
}
describe('windows foreground inspection cost per pane', () => {
const getAllProcesses = vi.fn()
let platform: PropertyDescriptor | undefined
let flagsSeen: number[] = []
beforeEach(() => {
flagsSeen = []
getAllProcesses.mockReset()
getAllProcesses.mockImplementation((cb: (rows: unknown) => void, flags: number) => {
flagsSeen.push(flags)
cb(NATIVE_TABLE)
})
platform = Object.getOwnPropertyDescriptor(process, 'platform')
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
__setWindowsProcessTreeLoaderForTests(() => ({
ProcessDataFlag: { None: 0, Memory: 1, CommandLine: 2, CreationTime: 4 },
getAllProcesses
}))
resetWindowsProcessRowsSnapshotForTests()
vi.useFakeTimers({ toFake: ['Date'] })
vi.setSystemTime(0)
})
afterEach(() => {
vi.useRealTimers()
__setWindowsProcessTreeLoaderForTests()
if (platform) {
Object.defineProperty(process, 'platform', platform)
}
})
async function sweepPanes(): Promise<(number | undefined)[]> {
const resolved: (number | undefined)[] = []
for (let pane = 0; pane < PANE_COUNT; pane += 1) {
const inventory = await queryWindowsPaneProcessInventory(shellPid(pane), {
anchorPid: agentPid(pane)
})
expect(inventory?.candidates).toHaveLength(1)
resolved.push(inventory?.candidates[0]?.pid)
}
return resolved
}
it('never sets the Memory flag on the snapshot', async () => {
await queryWindowsPaneProcessInventory(shellPid(0))
expect(flagsSeen).toHaveLength(1)
// Memory is bit 0, and it costs the addon a second OpenProcess per process
// carrying PROCESS_VM_READ (process.cc `GetProcessMemoryUsage`).
expect(flagsSeen[0]! & 1).toBe(0)
// CommandLine (2) | CreationTime (4).
expect(flagsSeen[0]).toBe(6)
})
it('projects the shared snapshot once for the whole pane fan-out', async () => {
const probeRows: unknown[] = []
for (let pane = 0; pane < PANE_COUNT; pane += 1) {
const inventory = await queryWindowsPaneProcessInventory(shellPid(pane), {
anchorPid: PROBE_PID
})
probeRows.push(inventory?.anchorRow)
}
expect(probeRows.filter(Boolean)).toHaveLength(PANE_COUNT)
// One projection produced every pane's row object. Pre-fix each pane ran
// its own `native.map(toProcessRow)` over all 1050 rows, so this set held
// PANE_COUNT distinct objects and the sweep allocated PANE_COUNT * 1050.
expect(new Set(probeRows).size).toBe(1)
})
it('indexes the shared snapshot once for the whole pane fan-out', async () => {
// Prime the TTL cache and the index so the snapshot read is not in the count.
await queryWindowsPaneProcessInventory(shellPid(0), { anchorPid: agentPid(0) })
const insertions = await countMapInsertions(async () => {
await sweepPanes()
})
// Pre-fix every pane rebuilt a whole-table `childrenByPpid`, so this was
// >= PANE_COUNT * (rows with a distinct ppid). One shared index makes the
// whole sweep cost no table-sized Map build at all.
expect(insertions).toBeLessThan(TABLE_SIZE)
})
it('resolves the same foreground child for every pane as an unshared scan would', async () => {
const resolved = await sweepPanes()
expect(resolved).toEqual(Array.from({ length: PANE_COUNT }, (_, pane) => agentPid(pane)))
})
})
@@ -1,3 +1,7 @@
import {
collectDescendantsFromIndex,
getProcessTableIndex
} from '../../shared/process-table-snapshot'
import {
readWindowsProcessTable,
readWindowsProcessTableFresh,
@@ -25,15 +29,40 @@ function toProcessRow(row: NativeWindowsProcessRow): WindowsProcessRow {
}
}
/**
* One projection per snapshot identity, mirroring `getProcessTableIndex`.
*
* The TTL cache already gives every pane the same native rows array; without
* this each of them still rebuilt ~1050 row objects, which also handed
* `getProcessTableIndex` a new array each time and defeated its memo by
* construction. Keyed weakly, so a projection dies with its snapshot. Rows are
* shared, never mutated: descendants are copied with their depth, and
* `anchorRow` is read-only to every caller.
*/
const projectedRows = new WeakMap<readonly NativeWindowsProcessRow[], WindowsProcessRow[]>()
function projectProcessRows(native: readonly NativeWindowsProcessRow[]): WindowsProcessRow[] {
const cached = projectedRows.get(native)
if (cached) {
return cached
}
const rows = native.map(toProcessRow)
projectedRows.set(native, rows)
return rows
}
/**
* Rows from a scan that starts after this call.
*
* PID-identity checks in teardown must not reuse a cached row — it can predate
* the very recycle it is meant to detect. Rejects when the table is unreadable,
* so "unavailable" stays distinguishable from "nothing is running".
*
* `readonly` because the projection is shared with every other reader of the
* same snapshot.
*/
export async function queryWindowsProcessRowsFresh(): Promise<WindowsProcessRow[]> {
return (await readWindowsProcessTableFresh()).map(toProcessRow)
export async function queryWindowsProcessRowsFresh(): Promise<readonly WindowsProcessRow[]> {
return projectProcessRows(await readWindowsProcessTableFresh())
}
export async function queryWindowsProcessDescendants(
@@ -63,21 +92,22 @@ export async function queryWindowsPaneProcessInventory(
options.fresh === true
? await readWindowsProcessTableFresh()
: await readWindowsProcessTable()
rows = native.map(toProcessRow)
rows = projectProcessRows(native)
} catch {
return null
}
// One index per snapshot, shared by every pane inspecting inside the TTL
// window: `byPid` answers both lookups that used to be linear scans, and
// `childrenByPpid` replaces a per-call Map rebuild over the whole table.
const index = getProcessTableIndex(rows)
// Why: a snapshot that omitted the PTY root may be stale or permission-
// filtered; only an observed root can authoritatively have no descendants.
if (!rows.some((row) => row.pid === rootPid)) {
if (!index.byPid.has(rootPid)) {
return null
}
return {
candidates: collectDescendants(rows, rootPid).sort((a, b) => b.depth - a.depth),
anchorRow:
options.anchorPid !== undefined
? (rows.find((row) => row.pid === options.anchorPid) ?? null)
: null
candidates: collectDescendantsFromIndex(index, rootPid).sort((a, b) => b.depth - a.depth),
anchorRow: options.anchorPid !== undefined ? (index.byPid.get(options.anchorPid) ?? null) : null
}
}
@@ -104,26 +134,3 @@ export function windowsDescendantsFromRows<Row extends { pid: number; ppid: numb
export function resetWindowsProcessRowsSnapshotForTests(): void {
resetWindowsProcessTableForTests()
}
function collectDescendants<Row extends { pid: number; ppid: number }>(
rows: Row[],
rootPid: number
): (Row & { depth: number })[] {
const childrenByParent = new Map<number, Row[]>()
for (const row of rows) {
const children = childrenByParent.get(row.ppid) ?? []
children.push(row)
childrenByParent.set(row.ppid, children)
}
const descendants: (Row & { depth: number })[] = []
const stack = (childrenByParent.get(rootPid) ?? []).map((row) => ({ row, depth: 1 }))
while (stack.length > 0) {
const { row, depth } = stack.pop()!
descendants.push({ ...row, depth })
for (const child of childrenByParent.get(row.pid) ?? []) {
stack.push({ row: child, depth: depth + 1 })
}
}
return descendants
}
@@ -0,0 +1,49 @@
import { describe, expect, it } from 'vitest'
import {
DECORATIVE_TITLE_FACT_HEARTBEAT_MS,
shouldEmitTitleFactForFrame
} from './decorative-title-fact-emission'
const base = {
decorativeOnly: true,
staleWorkingTitleClear: false,
lastEmittedAtMs: 1_000,
nowMs: 1_000
}
describe('shouldEmitTitleFactForFrame', () => {
it('always emits a frame that is not a decorative repeat', () => {
expect(shouldEmitTitleFactForFrame({ ...base, decorativeOnly: false })).toBe(true)
})
it('emits the first frame of a pane', () => {
expect(shouldEmitTitleFactForFrame({ ...base, lastEmittedAtMs: null })).toBe(true)
})
it('suppresses a decorative repeat inside the heartbeat window', () => {
expect(
shouldEmitTitleFactForFrame({ ...base, nowMs: 1_000 + DECORATIVE_TITLE_FACT_HEARTBEAT_MS - 1 })
).toBe(false)
})
it('lets a decorative repeat through once the heartbeat window elapses', () => {
expect(
shouldEmitTitleFactForFrame({ ...base, nowMs: 1_000 + DECORATIVE_TITLE_FACT_HEARTBEAT_MS })
).toBe(true)
})
it('never throttles a timer-synthesized stale-working clear', () => {
// Why: it carries a staleWorkingTitleClear flag no earlier repeat can stand in for.
expect(shouldEmitTitleFactForFrame({ ...base, staleWorkingTitleClear: true })).toBe(true)
})
it('emits after a backwards clock step instead of parking until it catches up', () => {
expect(shouldEmitTitleFactForFrame({ ...base, nowMs: 900 })).toBe(true)
})
it('keeps at least three frames inside the renderer hook-done quiet window', () => {
// Why: observeTitle's arriving working title is what cancels a Pi/OMP milestone `done`
// scheduled with HOOK_DONE_QUIET_MS = 1500. Losing that would mint a false completion.
expect(DECORATIVE_TITLE_FACT_HEARTBEAT_MS * 3).toBeLessThanOrEqual(1_500)
})
})
@@ -0,0 +1,38 @@
/**
* Why: an agent spinner re-emits a semantically identical OSC title ~12.5x/sec (Orca's own
* synthetic frame timer, Pi/OMP, Claude Code, Grok), and main ships every frame to the renderer
* as its own `pty:sideEffect` message. Both renderer store writes already discard those frames
* via `isDecorativeAgentTitleFrameChange`, so the message is pure cross-process cost.
*
* Why a heartbeat and not a hard drop: `agentCompletionCoordinator.observeTitle` treats an
* arriving *working* title as "still working" and cancels a scheduled hook-`done` completion
* inside `HOOK_DONE_QUIET_MS` (1500ms). That is exactly how a Pi/OMP milestone `done` emitted
* mid-turn is stopped from minting a completion notification, and the frames that carry it are
* decorative repeats. 500ms keeps 3 frames inside that window.
*/
export const DECORATIVE_TITLE_FACT_HEARTBEAT_MS = 500
export type DecorativeTitleFactEmissionInput = {
/** The frame's decorative gate key matches the previous frame's. */
decorativeOnly: boolean
/** Timer-synthesized stale-working clear — carries a flag no repeat can stand in for. */
staleWorkingTitleClear: boolean
lastEmittedAtMs: number | null
nowMs: number
}
export function shouldEmitTitleFactForFrame({
decorativeOnly,
staleWorkingTitleClear,
lastEmittedAtMs,
nowMs
}: DecorativeTitleFactEmissionInput): boolean {
if (!decorativeOnly || staleWorkingTitleClear) {
return true
}
if (lastEmittedAtMs === null) {
return true
}
// A backwards clock step must not park the heartbeat until it catches up.
return nowMs < lastEmittedAtMs || nowMs - lastEmittedAtMs >= DECORATIVE_TITLE_FACT_HEARTBEAT_MS
}
@@ -1,6 +1,7 @@
// @ts-nocheck -- mechanically split from OrcaRuntimeService; behavior is covered by AST equivalence and characterization tests.
import { OrcaRuntimeWithEmitDaemonPtyTransientFact } from './orca-runtime-emit-daemon-pty-transient-fact'
import { getDecorativeAgentTitleSignature } from '../../shared/agent-decorative-title-signature'
import { shouldEmitTitleFactForFrame } from './decorative-title-fact-emission'
import type { RuntimePtyTitleTrackerEntry } from './runtime-terminal-state-records'
import { createTerminalTitleTracker } from '../../shared/terminal-output-side-effects'
import { detectAgentStatusFromTitle } from '../../shared/agent-detection'
@@ -64,20 +65,36 @@ export class OrcaRuntimeWithGetUnpersistedTrackedTitleForPty extends OrcaRuntime
const tracker = createTerminalTitleTracker(
{
onTitle: (normalizedTitle, rawTitle, meta) => {
this.recordTerminalSideEffectFact(ptyId, {
kind: 'title',
normalizedTitle,
rawTitle,
...(meta?.staleWorkingTitleClear ? { staleWorkingTitleClear: true } : {})
})
const changed = this.applyTrackedPtyTitle(ptyId, rawTitle, normalizedTitle, meta)
const identityOnlyTitle = this.isLiveCursorNativeTitle(rawTitle, meta)
const live = this.ptyTitleTrackersByPtyId.get(ptyId)
const gateKey = this.makeDecorativeTitleGateKey(rawTitle, normalizedTitle)
const decorativeOnly = live?.lastMobileTitleGateKey === gateKey
if (live) {
live.lastMobileTitleGateKey = gateKey
}
// Why: the same gate the mobile fan-out below already uses, applied one hop earlier —
// a spinner frame the renderer store discards should not cost a pty:sideEffect message
// at all. See decorative-title-fact-emission.ts for why repeats still heartbeat.
const nowMs = Date.now()
if (
shouldEmitTitleFactForFrame({
decorativeOnly,
staleWorkingTitleClear: meta?.staleWorkingTitleClear === true,
lastEmittedAtMs: live?.lastTitleFactAtMs ?? null,
nowMs
})
) {
if (live) {
live.lastTitleFactAtMs = nowMs
}
this.recordTerminalSideEffectFact(ptyId, {
kind: 'title',
normalizedTitle,
rawTitle,
...(meta?.staleWorkingTitleClear ? { staleWorkingTitleClear: true } : {})
})
}
const changed = this.applyTrackedPtyTitle(ptyId, rawTitle, normalizedTitle, meta)
const identityOnlyTitle = this.isLiveCursorNativeTitle(rawTitle, meta)
const tracksReplicatedStatus =
live?.applyingChunk === true && this.mobileSessionTabListeners.size > 0
const titleStatus = tracksReplicatedStatus ? detectAgentStatusFromTitle(rawTitle) : null
@@ -151,6 +168,7 @@ export class OrcaRuntimeWithGetUnpersistedTrackedTitleForPty extends OrcaRuntime
tracker,
applyingChunk: false,
lastMobileTitleGateKey: null,
lastTitleFactAtMs: null,
chunkTouchedSessionTabs: false,
pendingFacts: [],
// Why: command-code facts exist only for the pty:sideEffect channel —
@@ -0,0 +1,110 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import type { TerminalSideEffectBatch } from '../../../shared/terminal-side-effect-facts'
import { syncSinglePty } from '../orca-runtime-test-fixtures.spec'
import { createSideEffectRuntime } from '../orca-runtime-test-scenario-builders.spec'
import { DECORATIVE_TITLE_FACT_HEARTBEAT_MS } from '../decorative-title-fact-emission'
// Orca's own synthetic agent spinner: one frame per pane every 80ms while an agent works.
const SPINNER_FRAMES = ['⠋', '⠙', '⠹', '⠸', '⠼', '⠴', '⠦', '⠧', '⠇', '⠏']
const SPINNER_INTERVAL_MS = 80
const EPOCH = 1_700_000_000_000
type TitleFact = { kind: 'title'; normalizedTitle: string; rawTitle: string }
function titleFacts(batches: TerminalSideEffectBatch[]): TitleFact[] {
return batches.flatMap((batch) =>
batch.facts.filter((fact): fact is TitleFact => fact.kind === 'title')
)
}
describe('decorative title fact throttle', () => {
beforeEach(() => {
vi.useFakeTimers({ toFake: ['Date'] })
vi.setSystemTime(new Date(EPOCH))
})
afterEach(() => {
vi.useRealTimers()
})
it('collapses spinner ticks with an unchanged underlying title to the heartbeat rate', () => {
const { runtime, batches } = createSideEffectRuntime()
syncSinglePty(runtime)
const ticks = 125 // 10s of Orca's 80ms synthetic spinner timer
for (let tick = 0; tick < ticks; tick += 1) {
vi.setSystemTime(new Date(EPOCH + tick * SPINNER_INTERVAL_MS))
runtime.ingestSyntheticTitleFrame(
'pty-1',
`\x1b]0;${SPINNER_FRAMES[tick % SPINNER_FRAMES.length]} Claude Code\x07`
)
}
const facts = titleFacts(batches)
// Every frame carried the same underlying title, so the renderer learns nothing new past
// the heartbeat: 125 pty:sideEffect messages collapse to one per heartbeat window.
const elapsedMs = ticks * SPINNER_INTERVAL_MS
expect(facts.length).toBeLessThanOrEqual(
Math.ceil(elapsedMs / DECORATIVE_TITLE_FACT_HEARTBEAT_MS)
)
expect(facts.length).toBeLessThan(ticks / 5)
// The heartbeat must not thin out below what the renderer's 1500ms hook-done quiet window
// needs to cancel a milestone `done` — three working frames per window.
expect(facts.length).toBeGreaterThanOrEqual(Math.floor(elapsedMs / 1_500) * 3)
for (const fact of facts) {
expect(fact.normalizedTitle.endsWith('Claude Code')).toBe(true)
}
})
it('propagates a real title change on the tick it arrives, mid-heartbeat', () => {
const { runtime, batches } = createSideEffectRuntime()
syncSinglePty(runtime)
runtime.ingestSyntheticTitleFrame('pty-1', '\x1b]0;⠋ Claude Code\x07')
// Two more decorative ticks — still well inside the heartbeat window, so they are dropped.
vi.setSystemTime(new Date(EPOCH + SPINNER_INTERVAL_MS))
runtime.ingestSyntheticTitleFrame('pty-1', '\x1b]0;⠙ Claude Code\x07')
vi.setSystemTime(new Date(EPOCH + 2 * SPINNER_INTERVAL_MS))
runtime.ingestSyntheticTitleFrame('pty-1', '\x1b]0;⠹ Claude Code\x07')
expect(titleFacts(batches)).toHaveLength(1)
const beforeChange = batches.length
vi.setSystemTime(new Date(EPOCH + 3 * SPINNER_INTERVAL_MS))
runtime.ingestSyntheticTitleFrame('pty-1', '\x1b]0;✳ Claude Code\x07')
expect(batches.length).toBeGreaterThan(beforeChange)
expect(titleFacts(batches.slice(beforeChange))).toEqual([
{ kind: 'title', normalizedTitle: '✳ Claude Code', rawTitle: '✳ Claude Code' }
])
})
it('propagates a changed working label immediately even while the spinner rotates', () => {
// Why: only the spinner glyph is decoration. Grok/Pi-style label churn is real content.
const { runtime, batches } = createSideEffectRuntime()
syncSinglePty(runtime)
runtime.ingestSyntheticTitleFrame('pty-1', '\x1b]0;⠋ Claude Code\x07')
vi.setSystemTime(new Date(EPOCH + SPINNER_INTERVAL_MS))
runtime.ingestSyntheticTitleFrame('pty-1', '\x1b]0;⠙ Reviewing diff — Claude Code\x07')
expect(titleFacts(batches).map((fact) => fact.rawTitle)).toEqual([
'⠋ Claude Code',
'⠙ Reviewing diff — Claude Code'
])
})
it('keeps main-side tracked title state current for every suppressed frame', () => {
// Why: mobile/remote snapshots read the tracked record, not the fact stream — suppressing
// the fact must not freeze what a phone or a paired client is shown.
const { runtime } = createSideEffectRuntime()
syncSinglePty(runtime)
runtime.ingestSyntheticTitleFrame('pty-1', '\x1b]0;⠋ Claude Code\x07')
vi.setSystemTime(new Date(EPOCH + SPINNER_INTERVAL_MS))
runtime.ingestSyntheticTitleFrame('pty-1', '\x1b]0;⠙ Claude Code\x07')
expect(runtime.getTerminalSideEffectSnapshot('pty-1')?.facts).toEqual([
{ kind: 'title', normalizedTitle: '⠙ Claude Code', rawTitle: '⠙ Claude Code' }
])
})
})
@@ -8,6 +8,7 @@ import {
syncSinglePty
} from '../orca-runtime-test-fixtures.spec'
import { createSideEffectRuntime } from '../orca-runtime-test-scenario-builders.spec'
import { DECORATIVE_TITLE_FACT_HEARTBEAT_MS } from '../decorative-title-fact-emission'
describe('terminal side-effect fact channel', () => {
it('defers desktop-only output scanners until a headless runtime is promoted', () => {
@@ -70,53 +71,66 @@ describe('terminal side-effect fact channel', () => {
expect(events).toHaveLength(1)
})
it('bounds decorative title delivery per paired client without reducing local frames', () => {
const { runtime, batches } = createSideEffectRuntime()
const firstClientEvents: RuntimeClientEvent[] = []
runtime.attachWindow(1)
runtime.syncWindowGraph(1, { tabs: [], leaves: [] })
runtime.onClientEvent((event) => firstClientEvents.push(event))
it('bounds decorative title delivery per paired client below the local heartbeat', () => {
// Why the clock steps: main throttles decorative repeats on the local fact stream, so each
// round must clear that heartbeat for the per-client gate to be what collapses them here.
vi.useFakeTimers({ toFake: ['Date'] })
try {
const { runtime, batches } = createSideEffectRuntime()
const firstClientEvents: RuntimeClientEvent[] = []
runtime.attachWindow(1)
runtime.syncWindowGraph(1, { tabs: [], leaves: [] })
runtime.onClientEvent((event) => firstClientEvents.push(event))
const ptyIds = Array.from({ length: 64 }, (_, index) => `pty-remote-${index}`)
const frames = ['⠋', '⠙', '⠹', '⠸', '⠼', '⠴', '⠦', '⠧', '⠇', '⠏']
for (const ptyId of ptyIds) {
runtime.ingestSyntheticTitleFrame(ptyId, `\x1b]0;${frames[0]} Cursor Agent\x07`)
}
firstClientEvents.length = 0
for (const frame of frames.slice(1)) {
for (const ptyId of ptyIds) {
runtime.ingestSyntheticTitleFrame(ptyId, `\x1b]0;${frame} Cursor Agent\x07`)
const ptyIds = Array.from({ length: 64 }, (_, index) => `pty-remote-${index}`)
const frames = ['⠋', '⠙', '⠹', '⠸', '⠼', '⠴', '⠦', '⠧', '⠇', '⠏']
const stepPastHeartbeat = (): void => {
vi.setSystemTime(new Date(Date.now() + DECORATIVE_TITLE_FACT_HEARTBEAT_MS))
}
for (const ptyId of ptyIds) {
runtime.ingestSyntheticTitleFrame(ptyId, `\x1b]0;${frames[0]} Cursor Agent\x07`)
}
firstClientEvents.length = 0
for (const frame of frames.slice(1)) {
stepPastHeartbeat()
for (const ptyId of ptyIds) {
runtime.ingestSyntheticTitleFrame(ptyId, `\x1b]0;${frame} Cursor Agent\x07`)
}
}
expect(firstClientEvents).toEqual([])
expect(batches).toHaveLength(ptyIds.length * frames.length)
const bellChunk = `\x1b]0;${frames.at(-1)} Cursor Agent\x07\x07`
runtime.onPtyData(ptyIds[0], bellChunk, 1)
expect(firstClientEvents).toEqual([
expect.objectContaining({
type: 'terminalSideEffects',
batch: expect.objectContaining({ facts: [{ kind: 'bell' }] })
})
])
firstClientEvents.length = 0
const secondClientEvents: RuntimeClientEvent[] = []
runtime.onClientEvent((event) => secondClientEvents.push(event))
stepPastHeartbeat()
for (const ptyId of ptyIds) {
runtime.ingestSyntheticTitleFrame(ptyId, `\x1b]0;${frames[0]} Cursor Agent\x07`)
}
expect(firstClientEvents).toEqual([])
expect(secondClientEvents).toHaveLength(ptyIds.length)
// A real title change is never throttled — no clock step needed.
for (const ptyId of ptyIds) {
runtime.ingestSyntheticTitleFrame(ptyId, '\x1b]0;Cursor ready\x07')
}
expect(firstClientEvents).toHaveLength(ptyIds.length)
expect(secondClientEvents).toHaveLength(ptyIds.length * 2)
} finally {
vi.useRealTimers()
}
expect(firstClientEvents).toEqual([])
expect(batches).toHaveLength(ptyIds.length * frames.length)
const bellChunk = `\x1b]0;${frames.at(-1)} Cursor Agent\x07\x07`
runtime.onPtyData(ptyIds[0], bellChunk, 1)
expect(firstClientEvents).toEqual([
expect.objectContaining({
type: 'terminalSideEffects',
batch: expect.objectContaining({ facts: [{ kind: 'bell' }] })
})
])
firstClientEvents.length = 0
const secondClientEvents: RuntimeClientEvent[] = []
runtime.onClientEvent((event) => secondClientEvents.push(event))
for (const ptyId of ptyIds) {
runtime.ingestSyntheticTitleFrame(ptyId, `\x1b]0;${frames[0]} Cursor Agent\x07`)
}
expect(firstClientEvents).toEqual([])
expect(secondClientEvents).toHaveLength(ptyIds.length)
for (const ptyId of ptyIds) {
runtime.ingestSyntheticTitleFrame(ptyId, '\x1b]0;Cursor ready\x07')
}
expect(firstClientEvents).toHaveLength(ptyIds.length)
expect(secondClientEvents).toHaveLength(ptyIds.length * 2)
})
it('omits terminalSideEffects from non-consuming listeners while other events still flow', () => {
+1
View File
@@ -30,6 +30,7 @@ await import('./orca-runtime-tests/pty-title-status.spec')
await import('./orca-runtime-tests/terminal-side-effect-facts.spec')
await import('./orca-runtime-tests/terminal-side-effect-facts-part-02.spec')
await import('./orca-runtime-tests/terminal-side-effect-facts-part-03.spec')
await import('./orca-runtime-tests/decorative-title-fact-throttle.spec')
await import('./orca-runtime-tests/headless-snapshots.spec')
await import('./orca-runtime-tests/headless-snapshots-part-02.spec')
await import('./orca-runtime-tests/agent-status-and-waits.spec')
@@ -89,6 +89,8 @@ export type RuntimePtyTitleTrackerEntry = {
tracker: TerminalTitleTracker
applyingChunk: boolean
lastMobileTitleGateKey: string | null
/** When the last title fact was emitted — throttles decorative-only repeats. */
lastTitleFactAtMs: number | null
chunkTouchedSessionTabs: boolean
pendingFacts: TerminalSideEffectFact[]
commandCodeDetector: { observe: (data: string) => boolean } | null
@@ -60,8 +60,18 @@ describe('structured TUI process identity', () => {
platform: 'darwin',
readPosixRows: async () => [
{ pid: 100, ppid: 1, stat: 'Ss', command: '/bin/zsh' },
{ pid: 101, ppid: 100, stat: 'S+', command: 'node /opt/codex/bin/codex resume abc' },
{ pid: 102, ppid: 101, stat: 'S+', command: '/opt/codex/vendor/codex' }
{
pid: 101,
ppid: 100,
stat: 'S+',
command: 'node /opt/codex/bin/codex resume abc'
},
{
pid: 102,
ppid: 101,
stat: 'S+',
command: '/opt/codex/vendor/codex'
}
],
readStartTime
})
@@ -83,9 +93,27 @@ describe('structured TUI process identity', () => {
agent: 'codex',
platform: 'win32',
readWindowsRows: async () => [
{ pid: 100, ppid: 1, name: 'pwsh.exe', command: 'pwsh.exe', executablePath: '' },
{ pid: 101, ppid: 100, name: 'codex.exe', command: 'codex resume a', executablePath: '' },
{ pid: 102, ppid: 100, name: 'codex.exe', command: 'codex resume b', executablePath: '' }
{
pid: 100,
ppid: 1,
name: 'pwsh.exe',
command: 'pwsh.exe',
executablePath: ''
},
{
pid: 101,
ppid: 100,
name: 'codex.exe',
command: 'codex resume a',
executablePath: ''
},
{
pid: 102,
ppid: 100,
name: 'codex.exe',
command: 'codex resume b',
executablePath: ''
}
],
timeoutMs: 0
})
@@ -107,7 +135,14 @@ describe('structured TUI process identity', () => {
return [
{ pid: 100, ppid: 1, stat: 'Ss', command: '/bin/zsh' },
...(snapshots >= 3
? [{ pid: 101, ppid: 100, stat: 'S+', command: 'codex resume session-1' }]
? [
{
pid: 101,
ppid: 100,
stat: 'S+',
command: 'codex resume session-1'
}
]
: [])
]
},
@@ -128,6 +163,81 @@ describe('structured TUI process identity', () => {
expect(delays).toEqual([25, 25])
})
// Each poll forks a whole-machine `ps` (~0.065 CPU-s at 1,460 processes), so the
// capture COUNT per identification is the cost, not the 5s wall ceiling.
function countCapturesForIdentification(input: {
captureCostMs: number
childAppearsAtMs: number | null
}): Promise<{
captures: number
identifiedAtMs: number | null
elapsedMs: number
}> {
let clockMs = 0
let captures = 0
return readStructuredTuiProcessIdentity({
hostId: 'local',
rootPid: 100,
spawnToken: 'spawn-cost',
agent: 'codex',
platform: 'darwin',
readPosixRows: async () => {
captures += 1
clockMs += input.captureCostMs
return [
{ pid: 100, ppid: 1, stat: 'Ss', command: '/bin/zsh' },
...(input.childAppearsAtMs !== null && clockMs >= input.childAppearsAtMs
? [
{
pid: 101,
ppid: 100,
stat: 'S+',
command: 'codex resume session-1'
}
]
: [])
]
},
readStartTime: async () => 1_700_000_000_000,
now: () => clockMs,
sleep: async (delayMs) => {
clockMs += delayMs
}
}).then(
() => ({ captures, identifiedAtMs: clockMs, elapsedMs: clockMs }),
() => ({ captures, identifiedAtMs: null, elapsedMs: clockMs })
)
}
it('does not spend a hundred ps captures on an identification that never resolves', async () => {
const { captures, identifiedAtMs, elapsedMs } = await countCapturesForIdentification({
captureCostMs: 55,
childAppearsAtMs: null
})
expect(identifiedAtMs).toBeNull()
// The 5s ceiling is unchanged; only the captures inside it are.
expect(elapsedMs).toBeGreaterThanOrEqual(5_000)
// A flat 50ms poll spends ~48 captures here.
expect(captures).toBeLessThanOrEqual(20)
})
it('keeps identification latency identical while a child can still plausibly appear', async () => {
// The backoff must not touch the window a real spawn lands in: same capture
// count and same detection time as the flat 50ms poll.
for (const childAppearsAtMs of [0, 200, 500, 900]) {
const flatPollCaptures = Math.max(1, Math.ceil(childAppearsAtMs / (55 + 50)) + 1)
const { captures, identifiedAtMs } = await countCapturesForIdentification({
captureCostMs: 55,
childAppearsAtMs
})
expect(identifiedAtMs).not.toBeNull()
expect(captures).toBeLessThanOrEqual(flatPollCaptures)
expect(identifiedAtMs!).toBeLessThanOrEqual(childAppearsAtMs + 55 + 50)
}
})
it('fails closed when the process snapshot omitted the PTY root', async () => {
await expect(
readStructuredTuiProcessIdentity({
@@ -15,6 +15,13 @@ type ProcessRow = { pid: number; ppid: number; command: string; foreground: bool
const STRUCTURED_TUI_PROCESS_WAIT_MS = 5_000
const STRUCTURED_TUI_PROCESS_POLL_MS = 50
// Why: every poll forks a whole-machine `ps` (~0.065 CPU-s at 1,460 processes),
// and the 5s ceiling is only reached when the child never appears at all — so the
// tight interval buys nothing there. Hold it for the window in which a spawning
// child plausibly lands (detection latency byte-identical), then widen. Past the
// window the added latency is bounded by one interval.
const STRUCTURED_TUI_PROCESS_FAST_POLL_WINDOW_MS = 1_000
const STRUCTURED_TUI_PROCESS_MAX_POLL_MS = 500
function descendants(rows: ProcessRow[], rootPid: number): (ProcessRow & { depth: number })[] {
const children = new Map<number, ProcessRow[]>()
@@ -153,7 +160,9 @@ export async function readStructuredTuiProcessIdentity(input: {
const platform = input.platform ?? process.platform
const now = input.now ?? Date.now
const sleep = input.sleep ?? ((delayMs) => new Promise((resolve) => setTimeout(resolve, delayMs)))
const deadline = now() + (input.timeoutMs ?? STRUCTURED_TUI_PROCESS_WAIT_MS)
const startedAtMs = now()
const deadline = startedAtMs + (input.timeoutMs ?? STRUCTURED_TUI_PROCESS_WAIT_MS)
let pollDelayMs = input.pollIntervalMs ?? STRUCTURED_TUI_PROCESS_POLL_MS
while (true) {
const rows: ProcessRow[] =
@@ -194,6 +203,13 @@ export async function readStructuredTuiProcessIdentity(input: {
const label = input.agent === 'codex' ? 'Codex' : 'Claude'
throw new Error(`The resumed terminal did not expose one exact ${label} child process.`)
}
await sleep(Math.min(input.pollIntervalMs ?? STRUCTURED_TUI_PROCESS_POLL_MS, remainingMs))
await sleep(Math.min(pollDelayMs, remainingMs))
if (now() - startedAtMs >= STRUCTURED_TUI_PROCESS_FAST_POLL_WINDOW_MS) {
// Never below the caller's interval, so an explicitly slow poll stays slow.
pollDelayMs = Math.max(
pollDelayMs,
Math.min(pollDelayMs * 2, STRUCTURED_TUI_PROCESS_MAX_POLL_MS)
)
}
}
}
@@ -183,6 +183,48 @@ describe('startup ordering', () => {
)
})
it('keeps the git-environment barrier off the PTY startup services', () => {
const barrierSource = readFileSync(
join(process.cwd(), 'src/main/startup/main-process-ipc-bootstrap.ts'),
'utf8'
)
const launchSource = readFileSync(
join(process.cwd(), 'src/main/startup/main-process-runtime-launch.ts'),
'utf8'
)
const gitBarrierStart = barrierSource.indexOf(
"ipcMain.handle('app:awaitGitEnvironmentStartupBarrier'"
)
const gitBarrierEnd = barrierSource.indexOf(
"'app:prepareTerminalStartupRestoration'",
gitBarrierStart
)
expect(gitBarrierStart).toBeGreaterThanOrEqual(0)
expect(gitBarrierEnd).toBeGreaterThan(gitBarrierStart)
const gitBarrier = barrierSource.slice(gitBarrierStart, gitBarrierEnd)
// The git environment fence is shell PATH + WSL registration; a daemon PTY provider or a
// hook-server bind here puts terminal startup back in front of worktree hydration.
expect(gitBarrier).toContain('state.shellPathReady')
expect(gitBarrier).toContain('state.managedWslCliStartupBarrierReady')
expect(gitBarrier).not.toContain('firstWindowStartupServicesReady')
// The published promise must be the same one the terminal startup services wait on.
expect(launchSource).toContain('state.shellPathReady = shellPathReady')
expect(launchSource.indexOf('state.shellPathReady = shellPathReady')).toBeLessThan(
launchSource.indexOf('await launchDesktopMode(')
)
// Terminal restoration itself must still fence on the first-window services.
const restorationStart = barrierSource.indexOf(
"ipcMain.handle('app:prepareTerminalStartupRestoration'"
)
const restorationEnd = barrierSource.indexOf(
"'app:recoverLegacyWorkerTerminalsForRendererStartup'",
restorationStart
)
expect(barrierSource.slice(restorationStart, restorationEnd)).toContain(
'state.firstWindowStartupServicesReady'
)
})
it('reconciles retained Codex homes after authoritative daemon inventory', () => {
const source = readFileSync(
join(process.cwd(), 'src/main/startup/main-process-pty-startup.ts'),
@@ -11,6 +11,13 @@ export function registerMainProcessIpcHandlers(): void {
state.managedWslCliStartupBarrierReady
])
})
// Why separate from the first-window barrier: host Git needs the shell-PATH
// generation and the managed WSL CLI registration, not a daemon PTY provider
// or a hook-server bind. Bundling them made worktree hydration wait on a
// terminal service it never calls.
ipcMain.handle('app:awaitGitEnvironmentStartupBarrier', async () => {
await Promise.all([state.shellPathReady, state.managedWslCliStartupBarrierReady])
})
ipcMain.handle('app:prepareTerminalStartupRestoration', async () => {
await Promise.all([
state.firstWindowStartupServicesReady,
+4
View File
@@ -24,6 +24,7 @@ import { shutdownObservability } from '../observability'
import { isQuittingForUpdate } from '../updater'
import { recordUpdaterLifecycle } from '../updater-lifecycle-diagnostics'
import { stopTccPromptNotice } from '../macos-tcc-prompt-notice'
import { cancelHistoryGc } from '../terminal-history-gc'
import { shouldQuitWhenAllWindowsClosed } from './window-all-closed-quit-policy'
import { mainProcessState as state } from './main-process-state'
import { isDevParentShutdownRequested } from './configure-process'
@@ -82,6 +83,9 @@ function installBeforeQuitHandler(): void {
state.repoMaintenanceShutdown = awaitPackedRefsLockRelease()
// Why: defer PTY cleanup to will-quit so the renderer captures scrollback before PTY-exit events unmount TerminalPane (dropping its capture callbacks).
state.rateLimits?.stop()
// Why safe on a vetoed quit: background history GC is idempotent and re-scheduled next launch,
// so abandoning the walk here only costs one deferred sweep, never a half-applied prune.
cancelHistoryGc()
})
}
@@ -289,6 +289,9 @@ export async function initializeMainProcessRuntimeLaunch(
state.serveOptions = serveOptions
const runtimeRpc = installRuntimeRpc(runtime, serveOptions)
const shellPathReady = shellPathHydration.whenReady()
// Why published: the renderer's git-environment barrier must fence on the same
// generation the terminal startup services wait for, not a later re-read.
state.shellPathReady = shellPathReady
let desktopWindow: BrowserWindow | null = null
if (process.platform === 'win32' && app.isPackaged && !serveOptions) {
const desktopStartup = startWindowsDesktopBeforeShellPathReady({
+444
View File
@@ -0,0 +1,444 @@
import {
existsSync,
mkdirSync,
mkdtempSync,
readdirSync,
rmSync,
statSync,
writeFileSync
} from 'node:fs'
import { tmpdir } from 'node:os'
import { basename, join } from 'node:path'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { installFakeAppEnvironment } from '../../config/scripts/vitest-host-ports-setup'
const { removeHostTreeMock } = vi.hoisted(() => ({
removeHostTreeMock: vi.fn<(dir: string) => Promise<void>>()
}))
// Why intercept rather than no-op: the tombstone path each prune produces is the decision this
// suite reads, but the drain re-queues any tombstone still on disk after a "successful" removal,
// so the stub has to really delete or the queue never terminates.
vi.mock('./host-tree-removal', () => ({
removeHostTree: removeHostTreeMock
}))
import { readHistoryMeta } from './terminal-history'
import {
cancelPendingHistoryTreeRemovalRetries,
flushPendingWorktreeHistoryDeletions
} from './terminal-history-deletion'
import { cancelHistoryGc, runHistoryGc, scheduleHistoryGc } from './terminal-history-gc'
const GC_MIN_AGE_MS = 5 * 60 * 1000
const PENDING_DELETE_DIR_NAME = '.pending-delete'
const LIVE_WORKTREE_ID = 'repo-1::/path/live-wt'
const DEAD_WORKTREE_ID = 'repo-1::/path/dead-wt'
let userDataDir: string
let historyRoot: string
let originalXdgDataHome: string | undefined
/**
* The enumeration this exercises used to be a synchronous walk. Its replacement is an async
* fixed-worker pass, so the whole safety net is that both reach the same prune decision over a
* realistic tree: over-pruning here destroys scrollback the user still expects to have.
*
* A verbatim port of the pre-change decision logic, reporting names instead of deleting.
*/
function referenceSyncPruneDecisions(root: string, liveWorktreeIds: Set<string>): string[] {
const decisions: string[] = []
if (!existsSync(root)) {
return decisions
}
const now = Date.now()
for (const entry of readdirSync(root)) {
if (entry === PENDING_DELETE_DIR_NAME) {
continue
}
const entryPath = join(root, entry)
try {
const stats = statSync(entryPath)
if (!stats.isDirectory()) {
continue
}
try {
for (const file of readdirSync(entryPath)) {
statSync(join(entryPath, file))
}
} catch {
// Skip size estimation on error.
}
if (!existsSync(join(entryPath, 'meta.json'))) {
continue
}
const meta = readHistoryMeta(entryPath)
if (!meta?.worktreeId) {
continue
}
if (!liveWorktreeIds.has(meta.worktreeId)) {
if (meta.createdAt && now - new Date(meta.createdAt).getTime() < GC_MIN_AGE_MS) {
continue
}
decisions.push(entry)
}
} catch {
// Skip individual entries that fail.
}
}
return decisions
}
function seedDir(name: string, files: Record<string, string>): string {
const dir = join(historyRoot, name)
mkdirSync(dir, { recursive: true })
for (const [file, contents] of Object.entries(files)) {
writeFileSync(join(dir, file), contents)
}
return dir
}
function meta(worktreeId: string | undefined, ageMs: number | null): string {
return JSON.stringify({
...(worktreeId === undefined ? {} : { worktreeId }),
...(ageMs === null ? {} : { createdAt: new Date(Date.now() - ageMs).toISOString() })
})
}
const OLD = GC_MIN_AGE_MS * 2
/** Every decision shape the walk has to get right, including the ones that must never prune. */
function seedDecisionMatrix(): void {
seedDir('live-old', { 'meta.json': meta(LIVE_WORKTREE_ID, OLD), zsh_history: 'a' })
seedDir('live-young', { 'meta.json': meta(LIVE_WORKTREE_ID, 0) })
seedDir('orphan-old', { 'meta.json': meta(DEAD_WORKTREE_ID, OLD), zsh_history: 'b' })
seedDir('orphan-no-createdat', { 'meta.json': meta(DEAD_WORKTREE_ID, null) })
seedDir('orphan-unparseable-createdat', {
'meta.json': JSON.stringify({ worktreeId: DEAD_WORKTREE_ID, createdAt: 'not-a-date' })
})
seedDir('orphan-young', { 'meta.json': meta(DEAD_WORKTREE_ID, 1_000) })
seedDir('no-meta', { zsh_history: 'c' })
seedDir('malformed-meta', { 'meta.json': '{ this is not json' })
seedDir('truncated-meta', { 'meta.json': `{"worktreeId":"${DEAD_WORKTREE_ID}` })
seedDir('empty-meta', { 'meta.json': '{}' })
seedDir('array-meta', { 'meta.json': `["${DEAD_WORKTREE_ID}"]` })
seedDir('null-meta', { 'meta.json': 'null' })
seedDir('no-worktree-id', { 'meta.json': meta(undefined, OLD) })
seedDir('oversize-meta', {
'meta.json': JSON.stringify({
worktreeId: DEAD_WORKTREE_ID,
createdAt: new Date(Date.now() - OLD).toISOString(),
pad: 'x'.repeat(64 * 1024)
})
})
// meta.json as a directory: stat succeeds, the read does not.
mkdirSync(join(historyRoot, 'meta-is-a-dir', 'meta.json'), { recursive: true })
seedDir('empty-dir', {})
// A plain file at the root is not a history directory.
writeFileSync(join(historyRoot, 'stray-file'), 'x')
mkdirSync(join(historyRoot, PENDING_DELETE_DIR_NAME), { recursive: true })
}
/** Enough entries to run several worker batches and cross the cooperative-yield boundary. */
function seedBulk(count: number, orphanEvery: number): void {
for (let i = 0; i < count; i++) {
const orphan = i % orphanEvery === 0
seedDir(`bulk-${i}`, {
'meta.json': meta(orphan ? `${DEAD_WORKTREE_ID}-${i}` : LIVE_WORKTREE_ID, OLD),
zsh_history: `entry-${i}`,
bash_history: `entry-${i}`
})
}
}
function survivingDirs(): Set<string> {
return new Set(
readdirSync(historyRoot).filter(
(entry) =>
entry !== PENDING_DELETE_DIR_NAME && statSync(join(historyRoot, entry)).isDirectory()
)
)
}
beforeEach(() => {
userDataDir = mkdtempSync(join(tmpdir(), 'orca-history-gc-'))
historyRoot = join(userDataDir, 'terminal-history')
mkdirSync(historyRoot, { recursive: true })
installFakeAppEnvironment({ getPath: () => userDataDir })
// Why: the fish sweep resolves a real user data dir otherwise, and would delete the
// developer's own orca fish history files while this suite runs.
originalXdgDataHome = process.env.XDG_DATA_HOME
process.env.XDG_DATA_HOME = userDataDir
removeHostTreeMock.mockReset()
removeHostTreeMock.mockImplementation(async (dir) => {
rmSync(dir, { recursive: true, force: true })
})
})
/** Tombstone paths the pass condemned, with the `.<timestamp>.<rand>` rename suffix stripped. */
function tombstonedNames(): Set<string> {
return new Set(
removeHostTreeMock.mock.calls.map(([dir]) => basename(dir).split('.').slice(0, -2).join('.'))
)
}
afterEach(async () => {
cancelHistoryGc()
vi.useRealTimers()
await flushPendingWorktreeHistoryDeletions()
cancelPendingHistoryTreeRemovalRetries()
if (originalXdgDataHome === undefined) {
delete process.env.XDG_DATA_HOME
} else {
process.env.XDG_DATA_HOME = originalXdgDataHome
}
rmSync(userDataDir, { recursive: true, force: true })
})
describe('history GC prune decisions', () => {
it('prunes exactly the set the synchronous walk chose', async () => {
seedDecisionMatrix()
seedBulk(200, 7)
const live = new Set([LIVE_WORKTREE_ID])
const before = survivingDirs()
const expected = new Set(referenceSyncPruneDecisions(historyRoot, live))
await runHistoryGc(live)
const after = survivingDirs()
const actual = new Set([...before].filter((entry) => !after.has(entry)))
expect(expected.size).toBeGreaterThan(0)
expect([...actual].sort()).toEqual([...expected].sort())
})
it('keeps every directory whose ownership cannot be established', async () => {
seedDecisionMatrix()
await runHistoryGc(new Set([LIVE_WORKTREE_ID]))
const after = survivingDirs()
for (const kept of [
'live-old',
'live-young',
'orphan-young',
'no-meta',
'malformed-meta',
'truncated-meta',
'empty-meta',
'array-meta',
'null-meta',
'no-worktree-id',
'oversize-meta',
'meta-is-a-dir',
'empty-dir'
]) {
expect(after.has(kept)).toBe(true)
}
expect(after.has('orphan-old')).toBe(false)
expect(after.has('orphan-no-createdat')).toBe(false)
expect(after.has('orphan-unparseable-createdat')).toBe(false)
expect(existsSync(join(historyRoot, 'stray-file'))).toBe(true)
})
it('refuses to prune anything when the live set is empty', async () => {
seedDecisionMatrix()
await runHistoryGc(new Set())
expect(survivingDirs().has('orphan-old')).toBe(true)
expect(readdirSync(join(historyRoot, PENDING_DELETE_DIR_NAME))).toEqual([])
})
it('does not throw when the history root does not exist', async () => {
rmSync(historyRoot, { recursive: true, force: true })
await expect(runHistoryGc(new Set([LIVE_WORKTREE_ID]))).resolves.toBeUndefined()
})
it('tombstones orphans instead of removing them on the calling thread', async () => {
seedDecisionMatrix()
await runHistoryGc(new Set([LIVE_WORKTREE_ID]))
// The recursive rm only ever sees a path already renamed into the tombstone queue.
for (const [dir] of removeHostTreeMock.mock.calls) {
expect(dir).toContain(PENDING_DELETE_DIR_NAME)
}
expect(tombstonedNames().has('orphan-old')).toBe(true)
})
it('drains pre-existing tombstones without scanning them as worktrees', async () => {
seedDecisionMatrix()
const leftover = join(historyRoot, PENDING_DELETE_DIR_NAME, 'abc123.1700000000000.deadbeef')
mkdirSync(leftover, { recursive: true })
await runHistoryGc(new Set([LIVE_WORKTREE_ID]))
expect(removeHostTreeMock).toHaveBeenCalledWith(expect.stringContaining('abc123.1700000000000'))
})
it('continues the pass after one orphan tombstone fails', async () => {
seedDir('orphan-a', { 'meta.json': meta(`${DEAD_WORKTREE_ID}-a`, OLD) })
seedDir('orphan-b', { 'meta.json': meta(`${DEAD_WORKTREE_ID}-b`, OLD) })
// A file where the tombstone root must be makes the first rename fail; mkdir cannot replace it.
writeFileSync(join(historyRoot, PENDING_DELETE_DIR_NAME), 'not a directory')
await expect(runHistoryGc(new Set([LIVE_WORKTREE_ID]))).resolves.toBeUndefined()
// Nothing could be tombstoned, and both entries survive for a later pass to reclaim.
expect(survivingDirs()).toEqual(new Set(['orphan-a', 'orphan-b']))
})
})
describe('history GC concurrency behaviour', () => {
it('joins a second call to the in-flight pass instead of walking twice', async () => {
seedDecisionMatrix()
const live = new Set([LIVE_WORKTREE_ID])
const first = runHistoryGc(live)
const second = runHistoryGc(live)
expect(second).toBe(first)
await first
// Each rename produces its own tombstone, so a second overlapping walk would condemn twice.
const orphanRemovals = removeHostTreeMock.mock.calls.filter(([dir]) =>
basename(dir).startsWith('orphan-old.')
)
expect(orphanRemovals).toHaveLength(1)
})
it('starts a fresh pass once the previous one has settled', async () => {
seedDecisionMatrix()
const live = new Set([LIVE_WORKTREE_ID])
await runHistoryGc(live)
const second = runHistoryGc(live)
await expect(second).resolves.toBeUndefined()
})
it('stops an in-flight walk on cancel without pruning', async () => {
seedDecisionMatrix()
seedBulk(300, 3)
const before = survivingDirs()
const pass = runHistoryGc(new Set([LIVE_WORKTREE_ID]))
// Cancelling before the root listing resolves means no entry is ever visited.
cancelHistoryGc()
await pass
expect(survivingDirs()).toEqual(before)
})
it('does not run a scheduled pass that was cancelled while resolving live worktrees', async () => {
seedDecisionMatrix()
vi.useFakeTimers()
let resolveLiveIds: (ids: Set<string>) => void = () => {}
scheduleHistoryGc(
() =>
new Promise<Set<string>>((resolve) => {
resolveLiveIds = resolve
})
)
await vi.advanceTimersByTimeAsync(10_000)
cancelHistoryGc()
resolveLiveIds(new Set([LIVE_WORKTREE_ID]))
await vi.advanceTimersByTimeAsync(0)
expect(survivingDirs().has('orphan-old')).toBe(true)
})
it('coalesces duplicate scheduled startup GC calls', async () => {
vi.useFakeTimers()
const getLiveWorktreeIds = vi.fn().mockResolvedValue(new Set<string>())
scheduleHistoryGc(getLiveWorktreeIds)
scheduleHistoryGc(getLiveWorktreeIds)
await vi.advanceTimersByTimeAsync(10_000)
expect(getLiveWorktreeIds).toHaveBeenCalledTimes(1)
})
})
describe('history GC races an async walk introduces', () => {
it('survives a directory removed while the walk is in flight', async () => {
seedDecisionMatrix()
seedBulk(300, 5)
const live = new Set([LIVE_WORKTREE_ID])
const vanishing = ['bulk-11', 'bulk-77', 'bulk-201']
const pass = runHistoryGc(live)
for (const name of vanishing) {
rmSync(join(historyRoot, name), { recursive: true, force: true })
}
await expect(pass).resolves.toBeUndefined()
// Every live directory the racer did not touch is still there.
expect(survivingDirs().has('live-old')).toBe(true)
expect(survivingDirs().has('bulk-1')).toBe(true)
for (const name of vanishing) {
expect(existsSync(join(historyRoot, name))).toBe(false)
}
})
it('never prunes a directory whose meta.json is half-written when the walk reads it', async () => {
seedDir('being-written', {})
seedBulk(200, 5)
const live = new Set([LIVE_WORKTREE_ID])
const pass = runHistoryGc(live)
writeFileSync(
join(historyRoot, 'being-written', 'meta.json'),
`{"worktreeId":"${DEAD_WORKTREE_ID}","created`
)
await pass
expect(survivingDirs().has('being-written')).toBe(true)
})
it('prunes a directory whose meta.json arrived after the directory did', async () => {
seedDir('late-meta', {})
writeFileSync(
join(historyRoot, 'late-meta', 'meta.json'),
meta(`${DEAD_WORKTREE_ID}-late`, OLD)
)
await runHistoryGc(new Set([LIVE_WORKTREE_ID]))
expect(survivingDirs().has('late-meta')).toBe(false)
})
})
describe('history GC main-thread occupancy', () => {
it('yields to timers throughout the walk instead of blocking on it', async () => {
seedBulk(1_200, 40)
const live = new Set([LIVE_WORKTREE_ID])
const ticks = { sync: 0, async: 0 }
let maxAsyncGapMs = 0
// The pre-change walk is the control: a synchronous pass over the same tree cannot tick at all.
const syncTimer = setInterval(() => {
ticks.sync += 1
}, 4)
referenceSyncPruneDecisions(historyRoot, live)
clearInterval(syncTimer)
let last = performance.now()
const asyncTimer = setInterval(() => {
const now = performance.now()
maxAsyncGapMs = Math.max(maxAsyncGapMs, now - last - 4)
last = now
ticks.async += 1
}, 4)
await runHistoryGc(live)
clearInterval(asyncTimer)
// A synchronous pass cannot tick at all, however long it takes.
expect(ticks.sync).toBe(0)
expect(ticks.async).toBeGreaterThan(5)
// Generous because shared CI runners stall an idle timer by tens of ms on their own; the
// failure this guards against is a whole-walk block, which is seconds.
expect(maxAsyncGapMs).toBeLessThan(2_000)
})
})
+160 -80
View File
@@ -1,5 +1,5 @@
import { join } from 'node:path'
import { existsSync, readdirSync, statSync } from 'node:fs'
import { readdir, stat } from 'node:fs/promises'
import {
getHistoryRoot,
listWslHistoryRoots,
@@ -9,9 +9,11 @@ import {
schedulePendingHistoryTreeRemovals,
scheduleWorktreeHistoryTreeDeletion
} from './terminal-history-deletion'
import { readHistoryMeta } from './terminal-history'
import { readHistoryMetaAsync } from './terminal-history'
import { resolveFishHistoryDir, sweepOrphanedFishHistoryFiles } from './fish-history-session'
import { hashWorktreeId } from './terminal-history-id'
import { forEachWithConcurrency } from '../shared/map-with-concurrency'
import { yieldToEventLoop } from '../shared/event-loop-yield'
// Why 5 minutes: GC runs ~10s after startup, and the live-worktree snapshot is
// taken just before. A worktree created between the snapshot and GC execution
@@ -20,100 +22,136 @@ import { hashWorktreeId } from './terminal-history-id'
// to cover any realistic snapshot-to-scan delay.
const GC_MIN_AGE_MS = 5 * 60 * 1000
let scheduledHistoryGcTimer: ReturnType<typeof setTimeout> | null = null
let historyGcRunning = false
// Why a fixed worker pool over a frontier and not per-entry promise fan-out: a real
// history root holds thousands of directories, and starting every one at once queues
// tens of thousands of libuv requests before the first completes. 16 is deep enough to
// keep the default 4-thread pool saturated without monopolising the disk during startup.
const HISTORY_GC_SCAN_CONCURRENCY = 16
// Why yield at all when every step already awaits I/O: a fully cached root resolves each
// await in a microtask, which never returns to the macrotask queue. This bounds that run.
const HISTORY_GC_YIELD_EVERY = 32
/** Scan a single history root directory, pruning orphaned entries.
* Returns { totalDirs, orphaned, pruned, totalSizeKB }. */
function gcScanRoot(
root: string,
liveWorktreeIds: Set<string>
): {
let scheduledHistoryGcTimer: ReturnType<typeof setTimeout> | null = null
let historyGcStarting = false
let historyGcCancelled = false
let activeHistoryGc: Promise<void> | null = null
let activeHistoryGcAbort: AbortController | null = null
type GcRootScan = {
totalDirs: number
orphaned: number
pruned: number
totalSizeKB: number
/** Every fish data dir a meta.json in this root names, for the orphan sweep. */
fishHistoryDirs: Set<string>
} {
const result = {
}
/** Inspect one history directory, tombstoning it when its worktree is gone. */
async function gcScanEntry(
root: string,
entry: string,
liveWorktreeIds: Set<string>,
now: number,
result: GcRootScan
): Promise<void> {
const entryPath = join(root, entry)
try {
const stats = await stat(entryPath)
if (!stats.isDirectory()) {
return
}
result.totalDirs++
// Estimate directory size from meta.json + history files.
// Why a local accumulator: `result.totalSizeKB += <expression containing await>`
// reads the field before suspending and writes back a stale sum once workers interleave.
let dirSizeKB = 0
try {
for (const file of await readdir(entryPath)) {
dirSizeKB += Math.ceil((await stat(join(entryPath, file))).size / 1024)
}
} catch {
// Skip size estimation on error, keeping whatever was measured first.
}
result.totalSizeKB += dirSizeKB
// A missing, truncated, oversized or malformed meta.json reads back as null, and a
// null meta is never pruned — an entry whose ownership we cannot establish is kept.
const meta = await readHistoryMetaAsync(entryPath)
if (meta?.fishHistoryDir) {
result.fishHistoryDirs.add(meta.fishHistoryDir)
}
if (!meta?.worktreeId) {
return
}
if (!liveWorktreeIds.has(meta.worktreeId)) {
// Why: avoid a TOCTOU race where a worktree is created after the
// live-ID snapshot but before GC runs. Directories younger than
// GC_MIN_AGE_MS are presumed still live and skipped.
if (meta.createdAt) {
const ageMs = now - new Date(meta.createdAt).getTime()
if (ageMs < GC_MIN_AGE_MS) {
return
}
}
result.orphaned++
// Why: a large orphaned tree recursive-rm'd here would stall the main process ~10s after
// launch — the same freeze the explicit-delete path already tombstones its way out of.
if (scheduleWorktreeHistoryTreeDeletion(entryPath, root)) {
result.pruned++
console.log(`[pty:history:gc] Pruned orphaned history: ${meta.worktreeId}`)
}
}
} catch {
// Skip individual entries that fail.
}
}
/** Scan a single history root directory, pruning orphaned entries. */
async function gcScanRoot(
root: string,
liveWorktreeIds: Set<string>,
signal: AbortSignal
): Promise<GcRootScan> {
const result: GcRootScan = {
totalDirs: 0,
orphaned: 0,
pruned: 0,
totalSizeKB: 0,
fishHistoryDirs: new Set<string>()
}
if (!existsSync(root)) {
let entries: string[]
try {
entries = await readdir(root)
} catch {
// Absent or unreadable root: nothing to collect.
return result
}
const now = Date.now()
// Why: pending-delete is a tombstone queue drained asynchronously, not a live worktree hash.
const frontier = entries.filter((entry) => entry !== PENDING_DELETE_DIR_NAME)
for (const entry of readdirSync(root)) {
// Why: pending-delete is a tombstone queue drained asynchronously, not a live worktree hash.
if (entry === PENDING_DELETE_DIR_NAME) {
continue
await forEachWithConcurrency(
frontier,
HISTORY_GC_SCAN_CONCURRENCY,
async (entry, index): Promise<void> => {
if (signal.aborted) {
return
}
await gcScanEntry(root, entry, liveWorktreeIds, now, result)
if (index % HISTORY_GC_YIELD_EVERY === HISTORY_GC_YIELD_EVERY - 1) {
await yieldToEventLoop()
}
}
const entryPath = join(root, entry)
try {
const stat = statSync(entryPath)
if (!stat.isDirectory()) {
continue
}
result.totalDirs++
// Estimate directory size from meta.json + history files.
try {
for (const file of readdirSync(entryPath)) {
result.totalSizeKB += Math.ceil(statSync(join(entryPath, file)).size / 1024)
}
} catch {
// Skip size estimation on error.
}
const metaPath = join(entryPath, 'meta.json')
if (!existsSync(metaPath)) {
// No meta.json — can't determine ownership, skip.
continue
}
const meta = readHistoryMeta(entryPath)
if (meta?.fishHistoryDir) {
result.fishHistoryDirs.add(meta.fishHistoryDir)
}
if (!meta?.worktreeId) {
continue
}
if (!liveWorktreeIds.has(meta.worktreeId)) {
// Why: avoid a TOCTOU race where a worktree is created after the
// live-ID snapshot but before GC runs. Directories younger than
// GC_MIN_AGE_MS are presumed still live and skipped.
if (meta.createdAt) {
const ageMs = now - new Date(meta.createdAt).getTime()
if (ageMs < GC_MIN_AGE_MS) {
continue
}
}
result.orphaned++
// Why: a large orphaned tree recursive-rm'd here would stall the main process ~10s after
// launch — the same freeze the explicit-delete path already tombstones its way out of.
if (scheduleWorktreeHistoryTreeDeletion(entryPath, root)) {
result.pruned++
console.log(`[pty:history:gc] Pruned orphaned history: ${meta.worktreeId}`)
}
}
} catch {
// Skip individual entries that fail.
}
}
)
return result
}
/** Run background GC to prune history directories for worktrees that are no
* longer in Orca's known live-worktree set. */
export function runHistoryGc(liveWorktreeIds: Set<string>): void {
async function executeHistoryGc(liveWorktreeIds: Set<string>, signal: AbortSignal): Promise<void> {
try {
// Why: finish tombstones left by quit mid-rm before scanning live worktree hashes.
// Safe ahead of the guard below: these entries were already condemned by a
@@ -130,14 +168,17 @@ export function runHistoryGc(liveWorktreeIds: Set<string>): void {
console.log('[pty:history:gc] Skipped: live worktree set is empty')
return
}
const main = gcScanRoot(getHistoryRoot(), liveWorktreeIds)
const main = await gcScanRoot(getHistoryRoot(), liveWorktreeIds, signal)
// Also scan WSL history directories (each distro has its own subdirectory).
const wslTotals = { totalDirs: 0, orphaned: 0, pruned: 0, totalSizeKB: 0 }
const liveFishHistoryDirs = new Set(main.fishHistoryDirs)
for (const distroRoot of listWslHistoryRoots()) {
if (signal.aborted) {
break
}
schedulePendingHistoryTreeRemovals(distroRoot)
const r = gcScanRoot(distroRoot, liveWorktreeIds)
const r = await gcScanRoot(distroRoot, liveWorktreeIds, signal)
wslTotals.totalDirs += r.totalDirs
wslTotals.orphaned += r.orphaned
wslTotals.pruned += r.pruned
@@ -147,6 +188,11 @@ export function runHistoryGc(liveWorktreeIds: Set<string>): void {
}
}
if (signal.aborted) {
console.log('[pty:history:gc] Cancelled mid-scan')
return
}
// Why a sweep on top of per-worktree deletion: a fish history file lives in
// the user's fish data dir, so it outlives the directory that names it. A
// crash between tombstone and removal, or a hand-deleted history dir, leaves
@@ -178,28 +224,62 @@ export function runHistoryGc(liveWorktreeIds: Set<string>): void {
}
}
/** Run background GC to prune history directories for worktrees that are no
* longer in Orca's known live-worktree set. Resolves when the pass finishes. */
export function runHistoryGc(liveWorktreeIds: Set<string>): Promise<void> {
// Why join instead of starting a second pass: two walks would race each other's
// tombstone renames, and the loser's `scheduleWorktreeHistoryTreeDeletion` would
// report a failure for a directory the winner already condemned.
if (activeHistoryGc) {
return activeHistoryGc
}
const controller = new AbortController()
activeHistoryGcAbort = controller
activeHistoryGc = executeHistoryGc(liveWorktreeIds, controller.signal).finally(() => {
activeHistoryGc = null
activeHistoryGcAbort = null
})
return activeHistoryGc
}
/** Drop a pending GC and stop an in-flight walk at its next entry. */
export function cancelHistoryGc(): void {
if (scheduledHistoryGcTimer !== null) {
clearTimeout(scheduledHistoryGcTimer)
scheduledHistoryGcTimer = null
}
// Why a flag as well: the timer has already fired while the live-worktree lookup is
// in flight, and there is no controller to abort until the scan itself starts.
historyGcCancelled = true
activeHistoryGcAbort?.abort()
}
/** Schedule GC after a delay so it runs after workspace hydration completes.
* `getLiveWorktreeIds` should use already-known IDs, not probe repo paths. */
export function scheduleHistoryGc(getLiveWorktreeIds: () => Promise<Set<string>>): void {
// Why: main-window services can reattach during reload/reactivation; one
// pending/running disk GC is enough and avoids duplicate startup I/O.
if (scheduledHistoryGcTimer !== null || historyGcRunning) {
if (scheduledHistoryGcTimer !== null || historyGcStarting || activeHistoryGc !== null) {
return
}
historyGcCancelled = false
// Why 10s: avoids competing with startup-critical I/O while still running
// early enough to clean up before the user notices disk usage (§7.6).
scheduledHistoryGcTimer = setTimeout(async () => {
scheduledHistoryGcTimer = null
historyGcRunning = true
historyGcStarting = true
try {
const liveIds = await getLiveWorktreeIds()
runHistoryGc(liveIds)
if (historyGcCancelled) {
return
}
await runHistoryGc(liveIds)
} catch (err) {
console.warn(
`[pty:history:gc] Failed to enumerate live worktrees for GC: ${err instanceof Error ? err.message : String(err)}`
)
} finally {
historyGcRunning = false
historyGcStarting = false
}
}, 10_000)
}
-175
View File
@@ -96,8 +96,6 @@ import {
flushPendingWorktreeHistoryDeletions
} from './terminal-history-deletion'
import { runHistoryGc, scheduleHistoryGc } from './terminal-history-gc'
const OTHER_WORKTREE_HASH = hashWorktreeId('repo-1::/path/other-wt')
describe('terminal-history', () => {
@@ -688,179 +686,6 @@ describe('terminal-history', () => {
})
})
describe('runHistoryGc', () => {
it('coalesces duplicate scheduled startup GC calls', async () => {
vi.useFakeTimers()
existsSyncMock.mockReturnValue(false)
const getLiveWorktreeIds = vi.fn().mockResolvedValue(new Set<string>())
scheduleHistoryGc(getLiveWorktreeIds)
scheduleHistoryGc(getLiveWorktreeIds)
await vi.advanceTimersByTimeAsync(10_000)
expect(getLiveWorktreeIds).toHaveBeenCalledTimes(1)
})
it('prunes orphaned directories', () => {
existsSyncMock.mockImplementation((p: string) => {
// WSL root doesn't exist, so GC skips it
if (p.includes('terminal-history-wsl')) {
return false
}
return true
})
readdirSyncMock.mockImplementation((dir: string) => {
if (dir.endsWith('terminal-history')) {
return ['dir1', 'dir2']
}
return ['meta.json']
})
statSyncMock.mockReturnValue({ isDirectory: () => true, size: 100 })
readFileSyncMock.mockImplementation((p: string) => {
// Use a createdAt old enough to pass the GC age threshold
const oldDate = new Date(Date.now() - 10 * 60 * 1000).toISOString()
if (p.includes('dir1')) {
return JSON.stringify({ worktreeId: 'live-wt', createdAt: oldDate })
}
return JSON.stringify({ worktreeId: 'dead-wt', createdAt: oldDate })
})
const liveIds = new Set(['live-wt'])
runHistoryGc(liveIds)
// Should only prune dir2 (dead-wt), not dir1 (live-wt), and never recursive-rm on the main thread.
expect(rmSyncMock).not.toHaveBeenCalled()
expect(renameSyncMock).toHaveBeenCalledTimes(1)
expect(renameSyncMock).toHaveBeenCalledWith(
expect.stringContaining('dir2'),
expect.stringContaining(`.pending-delete${sep}dir2.`)
)
expect(rmAsyncMock).toHaveBeenCalledWith(
expect.stringContaining(`.pending-delete${sep}dir2.`),
expect.objectContaining({ recursive: true, force: true })
)
})
// Why: an empty live set is what a store that fell back to default state
// looks like, and it is indistinguishable from a user with no worktrees —
// who has no history to collect either. Treating it as "everything is
// orphaned" turns a recoverable bad load into deleted shell history.
it('refuses to prune anything when the live set is empty', () => {
existsSyncMock.mockImplementation((p: string) => !p.includes('terminal-history-wsl'))
readdirSyncMock.mockImplementation((dir: string) => {
if (dir.endsWith('.pending-delete')) {
return []
}
if (dir.endsWith('terminal-history')) {
return ['dir1', 'dir2']
}
return ['meta.json']
})
statSyncMock.mockReturnValue({ isDirectory: () => true, size: 100 })
readFileSyncMock.mockReturnValue(
JSON.stringify({
worktreeId: 'some-wt',
createdAt: new Date(Date.now() - 10 * 60 * 1000).toISOString()
})
)
runHistoryGc(new Set())
expect(renameSyncMock).not.toHaveBeenCalled()
expect(rmSyncMock).not.toHaveBeenCalled()
expect(rmAsyncMock).not.toHaveBeenCalled()
})
it('continues GC after one orphan tombstone fails', async () => {
existsSyncMock.mockImplementation((path: string) => !path.includes('terminal-history-wsl'))
readdirSyncMock.mockImplementation((dir: string) => {
if (dir.endsWith('.pending-delete')) {
return []
}
if (dir.endsWith('terminal-history')) {
return ['broken', 'healthy']
}
return ['meta.json']
})
statSyncMock.mockReturnValue({ isDirectory: () => true, size: 100 })
readFileSyncMock.mockReturnValue(
JSON.stringify({
worktreeId: 'orphan',
createdAt: new Date(Date.now() - 10 * 60 * 1000).toISOString()
})
)
renameSyncMock.mockImplementationOnce(() => {
throw new Error('busy')
})
expect(() => runHistoryGc(new Set(['live-wt']))).not.toThrow()
expect(renameSyncMock).toHaveBeenCalledTimes(2)
expect(rmAsyncMock).toHaveBeenCalledTimes(1)
await flushPendingWorktreeHistoryDeletions()
})
it('skips recently-created directories to avoid TOCTOU race', () => {
existsSyncMock.mockImplementation((p: string) => {
if (p.includes('terminal-history-wsl')) {
return false
}
return true
})
readdirSyncMock.mockImplementation((dir: string) => {
if (dir.endsWith('terminal-history')) {
return ['fresh-dir']
}
return ['meta.json']
})
statSyncMock.mockReturnValue({ isDirectory: () => true, size: 100 })
// createdAt is just now — younger than the 5-minute GC threshold
readFileSyncMock.mockReturnValue(
JSON.stringify({ worktreeId: 'unknown-wt', createdAt: new Date().toISOString() })
)
runHistoryGc(new Set(['live-wt']))
// Should NOT prune because the directory is too young
expect(rmSyncMock).not.toHaveBeenCalled()
expect(renameSyncMock).not.toHaveBeenCalled()
})
it('does not throw when history root does not exist', () => {
existsSyncMock.mockReturnValue(false)
expect(() => runHistoryGc(new Set(['live-wt']))).not.toThrow()
expect(readdirSyncMock).not.toHaveBeenCalledWith('/fake/userData/terminal-history')
})
it('drains delete tombstones asynchronously instead of scanning them as worktrees', async () => {
let tombstonePresent = true
existsSyncMock.mockImplementation((p: string) => !String(p).includes('terminal-history-wsl'))
readdirSyncMock.mockImplementation((dir: string) => {
if (String(dir).endsWith('.pending-delete')) {
return tombstonePresent ? ['abc123.1700000000000.deadbeef'] : []
}
if (String(dir).endsWith('terminal-history')) {
return ['.pending-delete']
}
return ['meta.json']
})
statSyncMock.mockReturnValue({ isDirectory: () => true, size: 100 })
rmAsyncMock.mockImplementation(async () => {
tombstonePresent = false
})
runHistoryGc(new Set(['live-wt']))
// The tombstone queue is drained off-thread; GC must never rmSync it or count it as a worktree.
expect(rmSyncMock).not.toHaveBeenCalled()
expect(rmAsyncMock).toHaveBeenCalledWith(
expect.stringContaining('abc123.1700000000000.deadbeef'),
expect.objectContaining({ recursive: true, force: true })
)
await flushPendingWorktreeHistoryDeletions()
})
})
describe('WSL path conversion', () => {
it('converts HISTFILE to Linux path for WSL cwd', () => {
const originalPlatform = process.platform
+24 -1
View File
@@ -1,5 +1,6 @@
import { join, basename } from 'node:path'
import { mkdirSync, existsSync, readFileSync, statSync, writeFileSync } from 'node:fs'
import { readFile, stat } from 'node:fs/promises'
import {
dropInheritedOrcaFishHistory,
fishHistorySessionName,
@@ -131,7 +132,29 @@ export function readHistoryMeta(dir: string): HistoryDirMeta | null {
if (statSync(metaPath).size > MAX_HISTORY_META_BYTES) {
return null
}
const raw: unknown = JSON.parse(readFileSync(metaPath, 'utf-8'))
return parseHistoryMeta(dir, readFileSync(metaPath, 'utf-8'))
} catch {
return null
}
}
/** `readHistoryMeta` off the main thread, for scans that walk thousands of directories. */
export async function readHistoryMetaAsync(dir: string): Promise<HistoryDirMeta | null> {
try {
const metaPath = join(dir, 'meta.json')
// Why stat before read: the cap must reject an oversized meta.json without loading it.
if ((await stat(metaPath)).size > MAX_HISTORY_META_BYTES) {
return null
}
return parseHistoryMeta(dir, await readFile(metaPath, 'utf-8'))
} catch {
return null
}
}
function parseHistoryMeta(dir: string, contents: string): HistoryDirMeta | null {
try {
const raw: unknown = JSON.parse(contents)
if (!raw || typeof raw !== 'object' || Array.isArray(raw)) {
return null
}
@@ -75,8 +75,6 @@ export function parseWindowsCimProcessRows(stdout: string): WindowsProcessRow[]
return []
}
const name = fieldAsString(row.Name)
// memoryBytes stays undefined: Win32_Process reports WorkingSetSize, but no
// caller reads it off this table and asking widens an already costly scan.
return [{ pid, ppid, name, command: fieldAsString(row.CommandLine) || name }]
})
}
+13 -10
View File
@@ -52,21 +52,24 @@ describe('windows process table', () => {
it('maps native rows, defaulting an unreadable command line to empty', async () => {
const rows = await readWindowsProcessTableFresh()
expect(rows).toEqual([
{ pid: process.pid, ppid: 0, name: 'vitest.exe', command: '', memoryBytes: undefined },
{ pid: process.pid, ppid: 0, name: 'vitest.exe', command: '' },
{
pid: 100,
ppid: 4,
name: 'orca.exe',
command: '"C:/a b/orca.exe" --x',
memoryBytes: 4096,
creationTimeMs: 1_700_000_000_000
}
])
})
it('requests memory and command line together', async () => {
it('requests the command line and creation time, never memory', async () => {
await readWindowsProcessTableFresh()
expect(getAllProcesses.mock.calls[0]?.[1]).toBe(7)
// CommandLine (2) | CreationTime (4). The Memory bit (1) stays clear: the
// addon opens a second PROCESS_VM_READ handle per process to serve it and
// nothing reads a working set off this table.
expect(getAllProcesses.mock.calls[0]?.[1]).toBe(6)
expect((getAllProcesses.mock.calls[0]?.[1] as number) & 1).toBe(0)
})
it('only advertises PID-safe ownership when the native creation-time field exists', () => {
@@ -400,20 +403,19 @@ describe('resolving the native reader', () => {
})
const rows = await readWindowsProcessTableFresh()
expect(rows).toEqual([
{ pid: process.pid, ppid: 0, name: 'vitest.exe', command: '', memoryBytes: undefined },
{ pid: process.pid, ppid: 0, name: 'vitest.exe', command: '' },
{
pid: 100,
ppid: 4,
name: 'orca.exe',
command: '"C:/a b/orca.exe" --x',
memoryBytes: 4096,
creationTimeMs: 1_700_000_000_000
}
])
expect(isWindowsProcessTableAvailable()).toBe(true)
})
it('asks the addon for memory and command line, as the package path does', async () => {
it('asks the addon for the command line but not memory, as the package path does', async () => {
const addon = addonReturning(NATIVE)
__setWindowsProcessTreeRequireForTests((specifier: string) => {
if (specifier === ADDON_SPECIFIER) {
@@ -422,9 +424,10 @@ describe('resolving the native reader', () => {
throw new Error('MODULE_NOT_FOUND')
})
await readWindowsProcessTableFresh()
// Memory | CommandLine. A bare snapshot would silently drop the command
// line every agent-recognition caller matches on first.
expect(addon.getProcessList).toHaveBeenCalledWith(expect.any(Function), 3)
// CommandLine only: a bare snapshot would silently drop the command line
// every agent-recognition caller matches on first, and the relay addon
// exposes no CreationTime bit to add.
expect(addon.getProcessList).toHaveBeenCalledWith(expect.any(Function), 2)
})
it('reaches the CIM scan when neither the package nor the addon is present', async () => {
+18 -16
View File
@@ -23,6 +23,10 @@ import { readWindowsProcessRowsWithCim } from './windows-process-table-cim-scan'
* pid+ppid+name 15.9 / 17.5 ms
* +memory +commandLine 30.6 / 33.7 ms
* PowerShell CIM 706 / 723 ms
*
* Those are the module's published figures for both extra fields together; the
* only flag set this module asks for is `CommandLine` (+ `CreationTime`, free),
* which sits between the two rows and has not been separately measured.
*/
export type WindowsProcessRow = {
@@ -31,8 +35,6 @@ export type WindowsProcessRow = {
name: string
/** Full command line. Empty when the process denied a query handle. */
command: string
/** Working set in bytes, or undefined when not requested/queryable. */
memoryBytes?: number
/** Process creation time in Unix milliseconds, when the native snapshot provides it. */
creationTimeMs?: number
}
@@ -41,7 +43,6 @@ type NativeProcessInfo = {
pid: number
ppid: number
name: string
memory?: number
commandLine?: string
creationTimeMs?: number
}
@@ -49,7 +50,6 @@ type NativeProcessInfo = {
type WindowsProcessTreeModule = {
ProcessDataFlag: {
None: number
Memory: number
CommandLine: number
CreationTime?: number
}
@@ -82,7 +82,10 @@ type WindowsProcessTreeAddon = {
) => void
}
/** Mirrors the package's enum; the addon takes the raw bit field. */
/**
* Mirrors the package's enum; the addon takes the raw bit field. `Memory` (1)
* is listed for completeness and is deliberately never set — see `flags` below.
*/
const PROCESS_DATA_FLAG = { None: 0, Memory: 1, CommandLine: 2 } as const
/** Staged beside the relay bundle by build-relay; see RELAY_ARTIFACTS. */
@@ -190,16 +193,16 @@ function readNativeRows(): Promise<WindowsProcessRow[]> {
}
const readId = ++readSequence
const readerEpoch = nativeReaderEpoch
// Why always both flags: each adds an OpenProcess per process (Memory a
// GetProcessMemoryInfo, CommandLine a PEB read), so asking for less would be
// cheaper -- 15.9ms p50 versus 30.6ms at 1050 processes. But every read shares
// one snapshot so a 32-wide teardown collapses into a single scan, and that
// snapshot has to satisfy every caller. Splitting the cache per field set
// would restore exactly the fan-out it exists to prevent.
const flags =
native.ProcessDataFlag.Memory |
native.ProcessDataFlag.CommandLine |
(native.ProcessDataFlag.CreationTime ?? 0)
// Why CommandLine but not Memory: each flag costs one OpenProcess per process
// inside the addon (process.cc), and every caller of this table matches on
// `command`, while nothing reads a working set off it -- the Resource Manager
// runs its own CIM sweep because it needs commit and CPU time in one pass, and
// `process.cc` truncates the working set into a DWORD anyway. Dropping Memory
// halves the per-snapshot handle count; the remaining flags stay in ONE flag
// set because every read shares one snapshot, so a 32-wide teardown collapses
// into a single scan. Splitting the cache per field set would restore exactly
// the fan-out it exists to prevent.
const flags = native.ProcessDataFlag.CommandLine | (native.ProcessDataFlag.CreationTime ?? 0)
return new Promise((resolve, reject) => {
// Hoisted so a synchronous throw from getAllProcesses can clear it. An
// orphaned timer would otherwise fire later and wedge a reader that had
@@ -241,7 +244,6 @@ function readNativeRows(): Promise<WindowsProcessRow[]> {
ppid: row.ppid,
name: row.name,
command: row.commandLine ?? '',
memoryBytes: row.memory,
...(typeof row.creationTimeMs === 'number'
? { creationTimeMs: row.creationTimeMs }
: {})
+3
View File
@@ -38,6 +38,9 @@ export type AppApi = {
/** Resolves when the daemon PTY provider and hook receiver have either
* started or failed open for the first BrowserWindow. */
awaitFirstWindowStartupServices: () => Promise<void>
/** Resolves when host Git can run: shell-PATH generation is published and the
* managed WSL CLI registration has reconciled. Does not wait on PTY services. */
awaitGitEnvironmentStartupBarrier: () => Promise<void>
/** Inventories retained PTYs and restores durable structured ownership before renderer adoption. */
prepareTerminalStartupRestoration: () => Promise<void>
/** Reconciles legacy worker authority around persisted terminal reconnect. */
+2
View File
@@ -43,6 +43,8 @@ export const appApi = {
awaitBeforeUnloadCheckpoint: () => awaitBeforeUnloadCheckpoint(),
awaitFirstWindowStartupServices: (): Promise<void> =>
ipcRenderer.invoke('app:awaitFirstWindowStartupServices'),
awaitGitEnvironmentStartupBarrier: (): Promise<void> =>
ipcRenderer.invoke('app:awaitGitEnvironmentStartupBarrier'),
prepareTerminalStartupRestoration: (): Promise<void> =>
ipcRenderer.invoke('app:prepareTerminalStartupRestoration'),
recoverLegacyWorkerTerminalsForRendererStartup: (): Promise<void> =>
+55
View File
@@ -0,0 +1,55 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import type { platformApi } from './platform-bridge'
const mocks = vi.hoisted(() => ({ getLinuxDisplayServer: vi.fn(() => null) }))
vi.mock('../preload-runtime-support', () => ({
getLinuxDisplayServer: mocks.getLinuxDisplayServer
}))
// Electron declares getSystemVersion as required on NodeJS.Process; Node does not have it.
const mutableProcess = process as unknown as { getSystemVersion?: () => string }
async function loadPlatformApi(): Promise<typeof platformApi> {
vi.resetModules()
return (await import('./platform-bridge')).platformApi
}
describe('platformApi.get', () => {
beforeEach(() => {
mocks.getLinuxDisplayServer.mockClear()
})
afterEach(() => {
delete mutableProcess.getSystemVersion
})
it('resolves the immutable payload once and returns the identical object', async () => {
const platformApi = await loadPlatformApi()
const getSystemVersion = vi.fn(() => '25.3.0')
mutableProcess.getSystemVersion = getSystemVersion
const first = platformApi.get()
for (let index = 0; index < 100; index += 1) {
expect(platformApi.get()).toBe(first)
}
expect(getSystemVersion).toHaveBeenCalledTimes(1)
expect(mocks.getLinuxDisplayServer).toHaveBeenCalledTimes(1)
expect(first.platform).toBe(process.platform)
expect(first.arch).toBe(process.arch)
expect(first.osRelease).toBe('25.3.0')
})
it('freezes the payload so no consumer can corrupt the shared instance', async () => {
const platformApi = await loadPlatformApi()
expect(Object.isFrozen(platformApi.get())).toBe(true)
})
it('resolves nothing before the first get, keeping preload startup free', async () => {
await loadPlatformApi()
expect(mocks.getLinuxDisplayServer).not.toHaveBeenCalled()
})
})
+13 -2
View File
@@ -1,8 +1,14 @@
import { getLinuxDisplayServer } from '../preload-runtime-support'
import type { PreloadApi } from '../api-types'
export const platformApi = {
get: () => ({
type PlatformInfo = ReturnType<PreloadApi['platform']['get']>
// Why: the renderer reads this on its render cadence, and every field below is fixed
// for the process lifetime, so resolve once and hand back the same frozen payload.
let platformInfo: PlatformInfo | undefined
function resolvePlatformInfo(): PlatformInfo {
return Object.freeze({
platform: process.platform,
// Why: sandboxed preload cannot require node:os; Electron exposes the OS
// version on process.getSystemVersion when available.
@@ -14,4 +20,9 @@ export const platformApi = {
shell: process.env.SHELL?.trim() || process.env.ComSpec?.trim() || '',
displayServer: getLinuxDisplayServer()
})
}
export const platformApi = {
// Why: resolved lazily so preload startup keeps paying nothing for it.
get: () => (platformInfo ??= resolvePlatformInfo())
} satisfies PreloadApi['platform']
+11
View File
@@ -46,6 +46,17 @@ function gitForConfig(config: {
}
return { stdout: `${config.base ?? ''}\n`, stderr: '' }
}
if (args[0] === 'remote' && args[1] === '-v') {
return {
stdout: (config.remotes ?? [])
.flatMap((name) => {
const url = config.remoteUrls?.[name] ?? ''
return [`${name}\t${url} (fetch)`, `${name}\t${url} (push)`]
})
.join('\n'),
stderr: ''
}
}
if (args[0] === 'remote' && args.length === 1) {
return { stdout: `${config.remotes?.join('\n') ?? ''}\n`, stderr: '' }
}
+15 -18
View File
@@ -1,5 +1,6 @@
import { assertGitPushTargetShape } from '../shared/git-push-target-validation'
import { gitRefTargetsBranchOnRemote } from '../shared/git-remote-branch-name'
import { findGitRemoteNameByFetchUrl } from '../shared/git-remote-url-index'
import type { GitPushTarget } from '../shared/worktree/types'
type RelayGit = (args: string[], cwd: string) => Promise<{ stdout: string; stderr: string }>
@@ -67,31 +68,19 @@ type ConfiguredPushRemote = {
branchRemote: string | null
}
// Host-side twin of `src/main/git/remote.ts`: one `git remote -v` instead of
// `git remote` plus a serial `git remote get-url` per remote.
async function findRemoteNameForUrl(
git: RelayGit,
worktreePath: string,
remoteUrl: string
): Promise<string | null> {
try {
const { stdout } = await git(['remote'], worktreePath)
const remotes = stdout
.split(/\r?\n/)
.map((line) => line.trim())
.filter(Boolean)
for (const remoteName of remotes) {
try {
const { stdout: urlStdout } = await git(['remote', 'get-url', remoteName], worktreePath)
if (urlStdout.trim() === remoteUrl) {
return remoteName
}
} catch {
// Ignore a remote that disappeared or has no fetch URL.
}
}
const { stdout } = await git(['remote', '-v'], worktreePath)
return findGitRemoteNameByFetchUrl(stdout, (candidateUrl) => candidateUrl === remoteUrl)
} catch {
return null
}
return null
}
async function normalizePushRemote(
@@ -120,9 +109,17 @@ async function getConfiguredPushRemote(
if (!remote) {
return null
}
const normalizedRemote = await normalizePushRemote(git, worktreePath, remote)
// The two usually name the same URL; resolving it twice reads the remote table twice.
if (!branchRemote) {
return { remote: normalizedRemote, branchRemote: null }
}
return {
remote: await normalizePushRemote(git, worktreePath, remote),
branchRemote: branchRemote ? await normalizePushRemote(git, worktreePath, branchRemote) : null
remote: normalizedRemote,
branchRemote:
branchRemote === remote
? normalizedRemote
: await normalizePushRemote(git, worktreePath, branchRemote)
}
}
@@ -5,6 +5,7 @@ import { requestScrollToCurrentWorkspaceRevealAndRename } from '@/lib/scroll-to-
import { showTerminalShortcutCaptureNotification } from '@/lib/terminal-shortcut-capture-notification'
import { shouldShowWorktreeHistoryControls } from '../lib/titlebar-worktree-history-controls'
import { TOGGLE_WORKSPACE_BOARD_EVENT } from '../components/sidebar/useWorkspaceBoardPanel'
import { requestTerminalTabRename } from '../components/tab-bar/terminal-tab-rename-request'
import {
deleteHoveredWorkspaceImmediately,
resolveHoveredWorkspaceDeleteTarget
@@ -180,7 +181,7 @@ export function createAppCommandHandlers(
) {
return false
}
return claim('tab.rename', () => store.setRenamingTabId(store.activeTabId!))
return claim('tab.rename', () => requestTerminalTabRename(store.activeTabId!))
}
],
[
@@ -2,13 +2,14 @@ import { describe, expect, it, vi } from 'vitest'
import { reconcileHydratedWorkspaceTabModels } from './reconcile-hydrated-workspace-tab-models'
describe('reconcileHydratedWorkspaceTabModels', () => {
it('reconciles every workspace the session hydrated, in session order', () => {
it('reconciles every workspace the session hydrated, in session order, in one call', () => {
const reconcile = vi.fn()
const reconciled = reconcileHydratedWorkspaceTabModels(
{ tabsByWorktree: { 'wt-a': [], 'wt-b': [], 'wt-c': [] } },
reconcile
)
expect(reconcile.mock.calls.map((call) => call[0])).toEqual(['wt-a', 'wt-b', 'wt-c'])
expect(reconcile).toHaveBeenCalledTimes(1)
expect(reconcile.mock.calls[0]?.[0]).toEqual(['wt-a', 'wt-b', 'wt-c'])
expect(reconciled).toEqual(['wt-a', 'wt-b', 'wt-c'])
})
@@ -3,12 +3,13 @@ import type { WorkspaceSessionState } from '../../../shared/workspace-session-st
/** Reconcile every workspace loaded during boot so stale unified-tab subsets converge. */
export function reconcileHydratedWorkspaceTabModels(
session: Pick<WorkspaceSessionState, 'tabsByWorktree'>,
reconcileWorktreeTabModel: (worktreeId: string) => unknown
// Why batched: one store write for the whole session instead of one per
// workspace, each fanning out to every non-React store subscriber.
reconcileWorktreeTabModels: (worktreeIds: readonly string[]) => void
): string[] {
const reconciled: string[] = []
for (const worktreeId of Object.keys(session.tabsByWorktree)) {
reconcileWorktreeTabModel(worktreeId)
reconciled.push(worktreeId)
const reconciled = Object.keys(session.tabsByWorktree)
if (reconciled.length > 0) {
reconcileWorktreeTabModels(reconciled)
}
return reconciled
}
@@ -30,6 +30,7 @@ function makeActions(): StartupActions {
reconnectPersistedTerminals: vi.fn(),
setTerminalStartupRestorationReady: vi.fn(),
setDeferredSshReconnectTargets: vi.fn(),
removeDeferredSshReconnectTarget: vi.fn(),
setSshConnectionState: vi.fn(),
hydratePersistedUI: vi.fn(),
setHydrationSucceeded: vi.fn(),
@@ -22,6 +22,7 @@ export type StartupActions = Pick<
| 'reconnectPersistedTerminals'
| 'setTerminalStartupRestorationReady'
| 'setDeferredSshReconnectTargets'
| 'removeDeferredSshReconnectTarget'
| 'setSshConnectionState'
| 'hydratePersistedUI'
| 'setHydrationSucceeded'
@@ -59,6 +60,8 @@ export function selectStartupActions(state: StartupActions): StartupActions {
cachedStartupActions.setTerminalStartupRestorationReady ===
state.setTerminalStartupRestorationReady &&
cachedStartupActions.setDeferredSshReconnectTargets === state.setDeferredSshReconnectTargets &&
cachedStartupActions.removeDeferredSshReconnectTarget ===
state.removeDeferredSshReconnectTarget &&
cachedStartupActions.setSshConnectionState === state.setSshConnectionState &&
cachedStartupActions.hydratePersistedUI === state.hydratePersistedUI &&
cachedStartupActions.setHydrationSucceeded === state.setHydrationSucceeded &&
@@ -91,6 +94,7 @@ export function selectStartupActions(state: StartupActions): StartupActions {
reconnectPersistedTerminals: state.reconnectPersistedTerminals,
setTerminalStartupRestorationReady: state.setTerminalStartupRestorationReady,
setDeferredSshReconnectTargets: state.setDeferredSshReconnectTargets,
removeDeferredSshReconnectTarget: state.removeDeferredSshReconnectTarget,
setSshConnectionState: state.setSshConnectionState,
hydratePersistedUI: state.hydratePersistedUI,
setHydrationSucceeded: state.setHydrationSucceeded,
@@ -19,6 +19,7 @@ import {
} from '../startup/startup-diagnostics'
import { recoverFromDegradedStartup } from '../startup/startup-degraded-recovery'
import { restoreSshConnectionsForStartup } from '../startup/startup-ssh-connection-restore'
import { collectActiveWorkspaceSshTargetIds } from '../startup/active-workspace-ssh-targets'
import { publishTerminalViewAttributesAtAppStart } from '../components/terminal-pane/terminal-appearance'
import { getSystemPrefersDark } from '../lib/terminal-theme'
import {
@@ -155,9 +156,12 @@ export function useAppStartupHydration(onOnboardingLoaded: (state: OnboardingSta
// Why: disconnected SSH repos hydrate from local metadata; only runtime-owned repos use placeholders.
parseExecutionHostId(getRepoExecutionHostId(repo))?.kind !== 'runtime'
)
// Why: worktree refresh can spawn host Git; wait for main's shell-PATH generation fence first.
await timeRendererStartupStep('first-window-services-await', () =>
window.api.app.awaitFirstWindowStartupServices()
// Why this barrier and not the first-window one: worktree refresh can spawn host Git,
// which needs the shell-PATH generation and the managed WSL CLI registration. It never
// needs the daemon PTY provider or the hook-server bind, and `prepare-terminal-startup-restoration`
// below still fences those before any terminal is restored.
await timeRendererStartupStep('git-environment-barrier-await', () =>
window.api.app.awaitGitEnvironmentStartupBarrier()
)
await timeRendererStartupStep('fetch-hydration-worktrees', () =>
mapWithConcurrency(hydrationRepos, WORKTREE_REFRESH_CONCURRENCY, (repo) =>
@@ -198,7 +202,7 @@ export function useAppStartupHydration(onOnboardingLoaded: (state: OnboardingSta
actions.hydrateBrowserSession(sessionRead.session, sessionHydrationOptions)
reconcileHydratedWorkspaceTabModels(
sessionRead.session,
useAppStore.getState().reconcileWorktreeTabModel
useAppStore.getState().reconcileWorktreeTabModels
)
})
await timeRendererStartupStep('prepare-terminal-startup-restoration', () =>
@@ -213,9 +217,14 @@ export function useAppStartupHydration(onOnboardingLoaded: (state: OnboardingSta
actions.pruneLastVisitedTimestamps()
actions.seedActiveWorktreeLastVisitedIfMissing()
})
await timeRendererStartupStep('fetch-browser-session-profiles', () =>
// Why started here but not awaited: on a remote runtime this is an RPC with a 15s
// timeout, and nothing between here and terminal restoration reads the profile list —
// awaiting it put that timeout on the terminal-restoration gate. Starting it at the
// original point keeps the profiles landing no later than they did before; the action
// swallows its own failures, so the `.catch` only marks the timing wrapper handled.
void timeRendererStartupStep('fetch-browser-session-profiles', () =>
actions.fetchBrowserSessionProfiles()
)
).catch(() => {})
const onboardingState = await onboardingPromise
if (!cancelled) {
onOnboardingLoadedRef.current(onboardingState)
@@ -228,9 +237,17 @@ export function useAppStartupHydration(onOnboardingLoaded: (state: OnboardingSta
)
if (connectionIds.length > 0) {
try {
// Why scoped: an unreachable host used to hold every restored terminal — local ones
// included — for the full reconnect timeout. Only the targets whose panes mount as
// soon as the gate opens are worth waiting for; the rest reattach on tab focus.
const blockingConnectionIds = collectActiveWorkspaceSshTargetIds(
useAppStore.getState()
)
await restoreSshConnectionsForStartup({
connectionIds,
blockingConnectionIds,
setDeferredSshReconnectTargets: actions.setDeferredSshReconnectTargets,
removeDeferredSshReconnectTarget: actions.removeDeferredSshReconnectTarget,
publishSshConnectionState: actions.setSshConnectionState
})
} catch (err) {
@@ -240,7 +257,8 @@ export function useAppStartupHydration(onOnboardingLoaded: (state: OnboardingSta
logRendererStartupDiagnostic('ssh-reconnect-skipped', { connectionIds: 0 })
}
// first-window-services-await already fenced worktree hydration; terminal recovery reuses that ready state.
// Why no explicit barrier here: prepare-terminal-startup-restoration above already awaited
// the first-window services, and main re-awaits them inside this handler anyway.
await timeRendererStartupStep('recover-legacy-worker-terminals-pre-reconnect', () =>
window.api.app.recoverLegacyWorkerTerminalsForRendererStartup()
)
+20 -5
View File
@@ -68,8 +68,11 @@ describe('renderer startup runtime routing', () => {
const hydrationWorktreesIndex = source.indexOf(
"timeRendererStartupStep('fetch-hydration-worktrees'"
)
const servicesIndex = source.indexOf(
"timeRendererStartupStep('first-window-services-await'",
// Why this barrier: worktree hydration can spawn host Git, so it must sit behind the
// shell-PATH + managed-WSL fence. On packaged Windows the window opens before
// shellPathReady resolves, so this really is the fence, not a formality.
const gitEnvironmentBarrierIndex = source.indexOf(
"timeRendererStartupStep('git-environment-barrier-await'",
sessionIndex
)
const fullWorktreesIndex = source.indexOf('await actions.fetchAllWorktrees()')
@@ -89,8 +92,11 @@ describe('renderer startup runtime routing', () => {
expect(localReposIndex).toBeLessThan(localGroupsIndex)
expect(localGroupsIndex).toBeLessThan(localFoldersIndex)
expect(localReposIndex).toBeLessThan(sessionIndex)
expect(sessionIndex).toBeLessThan(servicesIndex)
expect(servicesIndex).toBeLessThan(hydrationWorktreesIndex)
expect(sessionIndex).toBeLessThan(gitEnvironmentBarrierIndex)
expect(gitEnvironmentBarrierIndex).toBeLessThan(hydrationWorktreesIndex)
expect(source.slice(gitEnvironmentBarrierIndex, hydrationWorktreesIndex)).toContain(
'window.api.app.awaitGitEnvironmentStartupBarrier()'
)
const hydrationWorktreeBlock = source.slice(
hydrationWorktreesIndex,
source.indexOf('await keybindingsPromise')
@@ -180,7 +186,13 @@ describe('renderer startup runtime routing', () => {
it('waits for first-window startup services before terminal reconnect', () => {
const source = readSource(STARTUP_HYDRATION_PATH)
const servicesIndex = source.indexOf("timeRendererStartupStep('first-window-services-await'")
// Why this step: `app:prepareTerminalStartupRestoration` awaits
// firstWindowStartupServicesReady + managedWslCliStartupBarrierReady in main before it
// does anything else, so it is the renderer-side position of that fence.
// `desktop-startup-ordering.test.ts` pins the main-side await itself.
const servicesIndex = source.indexOf(
"timeRendererStartupStep('prepare-terminal-startup-restoration'"
)
const preReconnectRecoveryIndex = source.indexOf(
"timeRendererStartupStep('recover-legacy-worker-terminals-pre-reconnect'"
)
@@ -193,6 +205,9 @@ describe('renderer startup runtime routing', () => {
)
expect(servicesIndex).toBeGreaterThanOrEqual(0)
expect(source.slice(servicesIndex)).toContain(
'window.api.app.prepareTerminalStartupRestoration()'
)
expect(preReconnectRecoveryIndex).toBeGreaterThan(servicesIndex)
expect(capabilityRefreshIndex).toBeGreaterThan(preReconnectRecoveryIndex)
expect(reconnectIndex).toBeGreaterThan(capabilityRefreshIndex)
+3 -1
View File
@@ -1956,7 +1956,9 @@ html.native-shell .app-layout {
transform 120ms cubic-bezier(0.2, 0.8, 0.2, 1),
width 120ms cubic-bezier(0.2, 0.8, 0.2, 1),
opacity 80ms ease-out;
will-change: transform, width, opacity;
/* Why no `width`: it is not compositable, so hinting it only pins a layer that
has to be re-rastered every frame of the transition anyway. */
will-change: transform, opacity;
}
[data-workspace-board-card-drop-indicator='true']::before,
+13 -21
View File
@@ -16,6 +16,7 @@ import logo from '../../../../resources/logo.svg'
import { translate } from '@/i18n/i18n'
import { hasGitHubBackedProject, type PreflightIssue } from './landing-preflight-issues'
import { useLandingPreflightRuntime } from './landing-preflight-runtime'
import { useLandingOrcaStarState, type LandingStarState } from './landing-github-star-state'
type ShortcutItem = {
id: string
@@ -26,31 +27,21 @@ type ShortcutItem = {
// Do not deep-link to /stargazers: GitHub 404s that page for users without repo write access.
const ORCA_GITHUB_URL = 'https://github.com/stablyai/orca'
type StarState = 'loading' | 'starred' | 'not-starred' | 'web-fallback' | 'hidden'
type StarButtonProps = {
hasRepos: boolean
state: LandingStarState
setState: React.Dispatch<React.SetStateAction<LandingStarState>>
}
function GitHubStarButton({ hasRepos }: { hasRepos: boolean }): React.JSX.Element | null {
const [state, setState] = useState<StarState>('loading')
function GitHubStarButton({
hasRepos,
state,
setState
}: StarButtonProps): React.JSX.Element | null {
const [menuOpen, setMenuOpen] = useState(false)
const wrapperRef = useRef<HTMLDivElement | null>(null)
const mountedRef = useMountedRef()
useEffect(() => {
let cancelled = false
void window.api.gh.checkOrcaStarred().then((result) => {
if (cancelled) {
return
}
if (result === null) {
setState('web-fallback')
} else {
setState(result ? 'starred' : 'not-starred')
}
})
return () => {
cancelled = true
}
}, [])
useEffect(() => {
if (!menuOpen) {
return
@@ -237,6 +228,7 @@ export default function Landing(): React.JSX.Element {
// Why: the runtime-aware slice probes the active remote host instead of the renderer host.
const { preflightIssues } = useLandingPreflightRuntime()
const [starState, setStarState] = useLandingOrcaStarState()
const createWorktreeShortcut = useShortcutKeyDetails('workspace.create')
const previousWorktreeShortcut = useShortcutKeyDetails('worktree.navigateUp')
@@ -318,7 +310,7 @@ export default function Landing(): React.JSX.Element {
{showGitHubSupportFooter && (
<div className="absolute bottom-6 left-0 right-0 flex justify-center">
<GitHubStarButton hasRepos={repos.length > 0} />
<GitHubStarButton hasRepos={repos.length > 0} state={starState} setState={setStarState} />
</div>
)}
</div>
@@ -121,8 +121,10 @@ export function useAgentPaneThreads(args: {
// identities across rebuilds, so a status write to one agent leaves every other row's
// memo bail-out and cached search text intact. Rebuilds are deterministic, so a repeated
// (StrictMode/deferred) memo invocation returns identical objects from the cache.
const eventBuildCacheRef = useRef(createActivityEventBuildCache())
const threadReuseCacheRef = useRef(createAgentPaneThreadReuseCache())
const eventBuildCacheRef = useRef<ReturnType<typeof createActivityEventBuildCache>>(undefined!)
eventBuildCacheRef.current ??= createActivityEventBuildCache()
const threadReuseCacheRef = useRef<ReturnType<typeof createAgentPaneThreadReuseCache>>(undefined!)
threadReuseCacheRef.current ??= createAgentPaneThreadReuseCache()
const { events: allEvents, liveAgentByPaneKey } = useMemo(
() =>
@@ -132,7 +132,8 @@ export function useBrowserPageWebviewLifecycle({
const addBrowserHistoryEntryRef = useRef(addBrowserHistoryEntry)
const createBrowserTab = useAppStore((s) => s.createBrowserTab)
const isPaintableRef = useRef(isPaintable)
const annotationViewportBridgeTokenRef = useRef(createBrowserUuid().replaceAll('-', ''))
const annotationViewportBridgeTokenRef = useRef<string>(undefined!)
annotationViewportBridgeTokenRef.current ??= createBrowserUuid().replaceAll('-', '')
const isActiveRef = useRef(isActive)
const pendingAnnotationPayloadRef = useRef(pendingAnnotationPayload)
const browserAnnotations = useAppStore(
@@ -28,9 +28,9 @@ export function useBrowserPageZoomFeedback(browserTabId: string): {
// tab's zoom through the shared setting. Why the module-level lookup: the guest webview outlives
// this component (worktree switch, Settings visit), so re-seeding on remount would let a later
// Settings change retroactively hijack a tab the user already zoomed.
const paneZoomLevelRef = useRef(
const paneZoomLevelRef = useRef<number>(undefined!)
paneZoomLevelRef.current ??=
getExplicitBrowserPageZoomLevel(browserTabId) ?? normalizedBrowserDefaultZoomLevel
)
const [browserZoomPercent, setBrowserZoomPercent] = useState(browserDefaultZoomPercent)
const [browserZoomFeedbackVisible, setBrowserZoomFeedbackVisible] = useState(false)
const browserZoomFeedbackTimerRef = useRef<ReturnType<typeof setTimeout>>(undefined)
@@ -28,7 +28,8 @@ export function useRemoteBrowserPageInputQueue(): {
remoteWheelFrameRef: React.MutableRefObject<number | null>
remoteWheelInFlightRef: React.MutableRefObject<boolean>
} {
const remoteInputQueueRef = useRef<Promise<unknown>>(Promise.resolve())
const remoteInputQueueRef = useRef<Promise<unknown>>(undefined!)
remoteInputQueueRef.current ??= Promise.resolve()
const pendingRemoteWheelRef = useRef<PendingRemoteBrowserWheel | null>(null)
const remoteWheelFrameRef = useRef<number | null>(null)
const remoteWheelInFlightRef = useRef(false)
@@ -0,0 +1,76 @@
// @vitest-environment happy-dom
/**
* The annotation viewport bridge token used to sit in a `useRef(...)` argument, so every render of
* a doc preview minted a fresh `crypto.randomUUID()` and threw it away — only the mount-time token
* was ever read. Pin the mint count to the mount count.
*/
import { useState } from 'react'
import { act, cleanup, render } from '@testing-library/react'
import { afterEach, describe, expect, it, vi } from 'vitest'
const browserUuidCalls = vi.hoisted(() => ({ count: 0 }))
vi.mock('@/lib/browser-uuid', () => ({
createBrowserUuid: () => {
browserUuidCalls.count += 1
return `00000000-0000-4000-8000-${String(browserUuidCalls.count).padStart(12, '0')}`
}
}))
vi.mock('@/hooks/useShortcutLabel', () => ({ useShortcutLabel: () => 'Cmd+G' }))
vi.mock('@/components/browser-pane/annotate/guest-annotation-viewport-bridge', () => ({
syncGuestAnnotationViewportBridge: vi.fn()
}))
vi.mock('@/components/browser-pane/annotate/use-browser-page-annotation-send', () => ({
useBrowserPageAnnotationSend: () => ({
browserAnnotations: [],
setBrowserAnnotationTrayOpen: vi.fn()
})
}))
vi.mock('@/components/browser-pane/annotate/use-browser-page-grab-annotations', () => ({
useBrowserPageGrabAnnotations: () => ({})
}))
vi.mock('@/components/browser-pane/annotate/use-browser-page-markup-capture', () => ({
useBrowserPageMarkupCapture: () => ({})
}))
vi.mock('@/components/browser-pane/annotate/useGrabMode', () => ({
useGrabMode: () => ({ active: false })
}))
const { useDocPreviewGuestTools } = await import('./use-doc-preview-guest-tools')
let bumpRender: (() => void) | null = null
function Host(): null {
const [, setTick] = useState(0)
bumpRender = () => setTick((tick) => tick + 1)
useDocPreviewGuestTools({
previewId: 'preview-1',
worktreeId: 'wt-1',
grantId: 'grant-1',
webviewRef: { current: null },
containerRef: { current: null },
toolsReady: true
} as unknown as Parameters<typeof useDocPreviewGuestTools>[0])
return null
}
afterEach(() => {
cleanup()
browserUuidCalls.count = 0
bumpRender = null
})
describe('useDocPreviewGuestTools annotation bridge token', () => {
it('mints the bridge token once per mount, not once per render', () => {
render(<Host />)
expect(browserUuidCalls.count).toBe(1)
for (let i = 0; i < 20; i += 1) {
act(() => bumpRender?.())
}
expect(browserUuidCalls.count).toBe(1)
})
})
@@ -41,7 +41,8 @@ export function useDocPreviewGuestTools({
// Why still empty before the first grant: the page is only a tool target once a document is on
// screen, and useGrabMode needs a stable identity every render rather than one to guess with.
const toolTargetId = grantId === null ? '' : previewId
const annotationViewportBridgeTokenRef = useRef(createBrowserUuid().replaceAll('-', ''))
const annotationViewportBridgeTokenRef = useRef<string>(undefined!)
annotationViewportBridgeTokenRef.current ??= createBrowserUuid().replaceAll('-', '')
const [browserOverlayViewport, setBrowserOverlayViewport] = useState<BrowserOverlayViewport>({
scrollX: 0,
scrollY: 0,
@@ -51,7 +51,8 @@ export function useAgentBucketCounts(): AgentBucketCounts {
// Why a per-hook cache: unrelated status/title writes change one worktree's inputs;
// the cache keeps every other worktree's counts without rerunning its row pipeline.
const cacheRef = useRef(createDashboardBucketCountsCache())
const cacheRef = useRef<ReturnType<typeof createDashboardBucketCountsCache>>(undefined!)
cacheRef.current ??= createDashboardBucketCountsCache()
return useMemo(() => {
return buildDashboardBucketCounts(
{
@@ -11,7 +11,8 @@ import { createWorktreeAgentRowsCache } from './worktree-agent-rows-cache'
* is no relay, so we derive it here from the same builder the bridge uses.
*/
export function useLiveDashboardSnapshot(): DashboardSnapshot {
const rowsCacheRef = useRef(createWorktreeAgentRowsCache())
const rowsCacheRef = useRef<ReturnType<typeof createWorktreeAgentRowsCache>>(undefined!)
rowsCacheRef.current ??= createWorktreeAgentRowsCache()
const repos = useAppStore((s) => s.repos)
const worktreesByRepo = useAppStore((s) => s.worktreesByRepo)
const tabsByWorktree = useAppStore((s) => s.tabsByWorktree)
@@ -0,0 +1,284 @@
// @vitest-environment happy-dom
import { renderHook } from '@testing-library/react'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import type { editor as MonacoEditor, IDisposable } from 'monaco-editor'
import type { DecoratedDiffComment } from './decorated-diff-comment'
import type * as ReactDomClientModule from 'react-dom/client'
import type * as DiffCommentZoneCardModule from './diff-comment-zone-card'
const storeFixture = vi.hoisted(() => ({
activeGroupIdByWorktree: {},
clearDeliveredDiffComments: vi.fn()
}))
vi.mock('@/store', () => ({
useAppStore: (selector: (state: typeof storeFixture) => unknown) => selector(storeFixture)
}))
// Stub only the card render: this suite is about zone/root lifecycle, not card markup.
vi.mock('./diff-comment-zone-card', async (importOriginal) => ({
...(await importOriginal<typeof DiffCommentZoneCardModule>()),
renderDiffCommentZoneCard: vi.fn()
}))
const rootCounts = vi.hoisted(() => ({ created: 0, unmounted: 0 }))
// Count only roots the decorator creates for its zones — @testing-library/react creates its own.
vi.mock('react-dom/client', async (importOriginal) => {
const actual = await importOriginal<typeof ReactDomClientModule>()
return {
...actual,
createRoot: (container: Element, options?: Parameters<typeof actual.createRoot>[1]) => {
const isZoneRoot = container.classList?.contains('orca-diff-comment-inline') ?? false
if (isZoneRoot) {
rootCounts.created += 1
}
const root = actual.createRoot(container, options)
return {
render: (node: Parameters<typeof root.render>[0]) => root.render(node),
unmount: () => {
if (isZoneRoot) {
rootCounts.unmounted += 1
}
root.unmount()
}
}
}
}
})
import { useDiffCommentDecorator } from './useDiffCommentDecorator'
type FakeEditor = {
editor: MonacoEditor.ICodeEditor
domNode: HTMLElement
zones: Map<string, MonacoEditor.IViewZone>
emitMouseMove: (lineNumber: number) => void
}
function createFakeEditor(): FakeEditor {
const domNode = document.createElement('div')
document.body.appendChild(domNode)
const zones = new Map<string, MonacoEditor.IViewZone>()
let nextZoneId = 0
const mouseMoveListeners: ((e: { target: { position: { lineNumber: number } } }) => void)[] = []
const noopDisposable: IDisposable = { dispose: () => {} }
const editor = {
getDomNode: () => domNode,
getModel: () => ({}),
getOption: () => 19,
getTopForLineNumber: () => 0,
getScrollTop: () => 0,
getLayoutInfo: () => ({ height: 400 }),
setScrollTop: () => {},
deltaDecorations: () => [],
getTargetAtClientPoint: () => null,
onMouseMove: (listener: (e: { target: { position: { lineNumber: number } } }) => void) => {
mouseMoveListeners.push(listener)
return noopDisposable
},
onMouseLeave: () => noopDisposable,
onDidScrollChange: () => noopDisposable,
changeViewZones: (callback: (accessor: MonacoEditor.IViewZoneChangeAccessor) => void) =>
callback({
addZone: (zone: MonacoEditor.IViewZone) => {
const id = `zone-${(nextZoneId += 1)}`
zones.set(id, zone)
return id
},
removeZone: (id: string) => {
zones.delete(id)
},
layoutZone: () => {}
} as unknown as MonacoEditor.IViewZoneChangeAccessor)
} as unknown as MonacoEditor.ICodeEditor
return {
editor,
domNode,
zones,
emitMouseMove: (lineNumber) => {
for (const listener of mouseMoveListeners) {
listener({ target: { position: { lineNumber } } })
}
}
}
}
const FILE_PATH = 'src/index.ts'
const REVIEW_SURFACE_ID = 'pr:acme/widgets:42'
function reviewNote(index: number): DecoratedDiffComment {
return {
id: `review-note-${index}`,
worktreeId: REVIEW_SURFACE_ID,
filePath: FILE_PATH,
lineNumber: 10 + index,
body: `Please rename this (${index}).`,
createdAt: index,
side: 'modified',
author: 'octocat'
}
}
// Every refresh of remote review data yields a fresh-but-equal array, exactly as the main process ships it.
function freshCommentableLines(): readonly number[] {
return [10, 11, 12, 13, 14, 15, 16]
}
// Root teardown is deferred through queueMicrotask, so drain before asserting on unmount counts.
async function flushDeferredUnmounts(): Promise<void> {
await Promise.resolve()
await Promise.resolve()
}
// Asserted as one object so a failure reports every lifecycle number at once.
function lifecycleTotals(fake: FakeEditor): Record<string, number> {
return {
createRootCalls: rootCounts.created,
rootUnmounts: rootCounts.unmounted,
monacoViewZones: fake.zones.size
}
}
function isAddButtonVisible(domNode: HTMLElement): boolean {
const button = domNode.querySelector<HTMLElement>('.orca-diff-comment-add-btn')
return button != null && button.style.display !== 'none'
}
type DecoratorProps = {
commentableLineNumbers: readonly number[]
comments: readonly DecoratedDiffComment[]
}
function renderDecorator(fake: FakeEditor, initialProps: DecoratorProps) {
return renderHook(
({ commentableLineNumbers, comments }: DecoratorProps) =>
useDiffCommentDecorator({
editor: fake.editor,
filePath: FILE_PATH,
worktreeId: REVIEW_SURFACE_ID,
comments,
commentableLineNumbers,
onAddCommentClick: vi.fn(),
onDeleteComment: vi.fn()
}),
{ initialProps }
)
}
beforeEach(() => {
rootCounts.created = 0
rootCounts.unmounted = 0
})
afterEach(() => {
document.body.replaceChildren()
vi.clearAllMocks()
})
describe('useDiffCommentDecorator commentable-line churn', () => {
it('keeps every comment root and view zone alive across value-equal review refreshes', async () => {
const fake = createFakeEditor()
const comments = [reviewNote(1), reviewNote(2), reviewNote(3)]
const hook = renderDecorator(fake, {
commentableLineNumbers: freshCommentableLines(),
comments
})
expect(rootCounts.created).toBe(3)
expect(fake.zones.size).toBe(3)
for (let refresh = 0; refresh < 5; refresh += 1) {
hook.rerender({ commentableLineNumbers: freshCommentableLines(), comments })
}
await flushDeferredUnmounts()
expect(lifecycleTotals(fake)).toEqual({
createRootCalls: 3,
rootUnmounts: 0,
monacoViewZones: 3
})
// Still tracked, so dropping a note reclaims its vertical space instead of leaving a blank gap.
hook.rerender({ commentableLineNumbers: freshCommentableLines(), comments: comments.slice(1) })
expect(fake.zones.size).toBe(2)
})
it('does not accumulate orphan zones when refreshes interleave with new review comments', async () => {
const fake = createFakeEditor()
let comments = [reviewNote(1)]
const hook = renderDecorator(fake, {
commentableLineNumbers: freshCommentableLines(),
comments
})
for (let refresh = 2; refresh <= 6; refresh += 1) {
comments = [...comments, reviewNote(refresh)]
hook.rerender({ commentableLineNumbers: freshCommentableLines(), comments })
}
await flushDeferredUnmounts()
// 6 live notes => 6 roots, 6 zones, nothing stranded.
expect(lifecycleTotals(fake)).toEqual({
createRootCalls: 6,
rootUnmounts: 0,
monacoViewZones: 6
})
})
it('rebuilds the add-button overlay when the commentable lines really change', async () => {
const fake = createFakeEditor()
const comments = [reviewNote(1)]
const hook = renderDecorator(fake, {
commentableLineNumbers: [10, 11, 12] as readonly number[],
comments
})
fake.emitMouseMove(40)
expect(isAddButtonVisible(fake.domNode)).toBe(false)
hook.rerender({ commentableLineNumbers: [10, 11, 12, 40], comments })
await flushDeferredUnmounts()
// Decorator is not stale: the widened set is live...
fake.emitMouseMove(40)
expect(isAddButtonVisible(fake.domNode)).toBe(true)
// ...and the existing note's zone/root was neither orphaned nor rebuilt.
expect(lifecycleTotals(fake)).toEqual({
createRootCalls: 1,
rootUnmounts: 0,
monacoViewZones: 1
})
})
it('removes its zones from Monaco when the model swaps under a retained editor', async () => {
const fake = createFakeEditor()
const comments = [reviewNote(1)]
const hook = renderHook(
({ monacoModelIdentity }) =>
useDiffCommentDecorator({
editor: fake.editor,
monacoModelIdentity,
filePath: FILE_PATH,
worktreeId: REVIEW_SURFACE_ID,
comments,
commentableLineNumbers: freshCommentableLines(),
onAddCommentClick: vi.fn(),
onDeleteComment: vi.fn()
}),
{ initialProps: { monacoModelIdentity: 'modified-v1' } }
)
const firstZoneIds = [...fake.zones.keys()]
hook.rerender({ monacoModelIdentity: 'modified-v2' })
await flushDeferredUnmounts()
// Stale zone ids are gone rather than left as untracked blank gaps, and the note was rebuilt.
expect(fake.zones.size).toBe(1)
expect([...fake.zones.keys()]).not.toEqual(firstZoneIds)
expect(rootCounts.created).toBe(2)
expect(rootCounts.unmounted).toBe(1)
})
})
@@ -80,11 +80,17 @@ export function useDiffCommentDecorator({
scrollToZoneFrameRef.current = null
}, [])
// Key on the values, not the array identity: review surfaces re-fetch PR/MR file data on every
// refresh and hand us a fresh-but-equal number[], which would otherwise churn every consumer.
const commentableLineKey = commentableLineNumbers?.join(',')
const commentableLineSet = useMemo(
() => (commentableLineNumbers ? new Set(commentableLineNumbers) : null),
[commentableLineNumbers]
// eslint-disable-next-line react-hooks/exhaustive-deps
[commentableLineKey]
)
// Add-button overlay only: it captures commentableLineSet/addButtonLabel, so it must be rebuilt when
// either changes. Kept apart from the zone teardown below, whose deps must mirror the zone-creating effect.
useEffect(() => {
if (!editor) {
return
@@ -95,8 +101,7 @@ export function useDiffCommentDecorator({
return
}
const zones = zonesRef.current
const disposeAddButtonOverlay = installDiffCommentAddButtonOverlay({
return installDiffCommentAddButtonOverlay({
editor,
editorDomNode,
addButtonLabel,
@@ -105,15 +110,33 @@ export function useDiffCommentDecorator({
disposablesRef,
onAddCommentClickRef
})
}, [addButtonLabel, commentableLineSet, editor, monacoModelIdentity])
// Deps must stay a subset of the zone-creating effect's, or a teardown here is never followed by a rebuild.
useEffect(() => {
if (!editor) {
return
}
const zones = zonesRef.current
return () => {
disposeAddButtonOverlay()
// Editor swapped/torn down: unmount roots and clear tracking so the next mount starts known-empty.
// Defer unmount via queueMicrotask: a sync unmount during React's commit triggers React 19's "unmount while rendering" warning; clear zones synchronously.
const rootsToUnmount = Array.from(zones.values(), (z) => {
z.disposeMouseDownStopper()
return z.root
})
// Drop the zones from Monaco too: clearing our map alone would strand them as untracked blank gaps
// in a still-live editor. No-op when the model already swapped (Monaco dropped them) or the editor is disposed.
if (zones.size > 0) {
const zoneIds = Array.from(zones.values(), (z) => z.zoneId)
editor.changeViewZones((accessor) => {
for (const zoneId of zoneIds) {
accessor.removeZone(zoneId)
}
})
}
zones.clear()
if (rootsToUnmount.length > 0) {
queueMicrotask(() => {
@@ -127,7 +150,7 @@ export function useDiffCommentDecorator({
pendingScrollRef.current = null
scrollToZoneRef.current = null
}
}, [addButtonLabel, cancelScrollToZoneFrame, commentableLineSet, editor, monacoModelIdentity])
}, [cancelScrollToZoneFrame, editor, monacoModelIdentity])
useEffect(() => {
if (!editor) {
@@ -1,13 +1,56 @@
import { describe, expect, it } from 'vitest'
import type { PdfViewPosition } from '@/lib/scroll-cache'
import { sweepClosedPdfViewPositions } from './closed-editor-tab-cache-sweep'
import {
deletePaneScopedCacheEntries,
sweepClosedPdfViewPositions
} from './closed-editor-tab-cache-sweep'
const position = (pageNumber: number): PdfViewPosition => ({ pageNumber, top: 0, left: 0 })
describe('deletePaneScopedCacheEntries', () => {
it('does not delete an owner whose id merely extends a closed owner id', () => {
const cache = new Map([
['tab-1::pane-1', 1],
['tab-10::pane-1', 2],
['tab-1x::pane-1', 3]
])
deletePaneScopedCacheEntries(cache, ['tab-1'])
expect([...cache.keys()]).toEqual(['tab-10::pane-1', 'tab-1x::pane-1'])
})
it('matches an owner that ends at the second `::` of a `:::` run', () => {
// Locks the `boundary + 1` advance in hasPaneScopeOwner: `a:` ends at index 2, which only the
// second `::` of the run exposes. A `+ 2` advance would skip it and leak the entry.
const cache = new Map([
['a:::b', 1],
['a::b', 2],
['ab:::c', 3]
])
deletePaneScopedCacheEntries(cache, ['a:'])
expect([...cache.keys()]).toEqual(['a::b', 'ab:::c'])
})
it('sweeps every owner in the batch in one pass', () => {
const cache = new Map([
['tab-1::pane-1', 1],
['tab-2::pane-1', 2],
['tab-3::pane-1', 3]
])
deletePaneScopedCacheEntries(cache, ['tab-1', 'tab-3'])
expect([...cache.keys()]).toEqual(['tab-2::pane-1'])
})
it('is a no-op for an empty owner batch', () => {
const cache = new Map([['tab-1::pane-1', 1]])
deletePaneScopedCacheEntries(cache, [])
expect(cache.size).toBe(1)
})
})
describe('sweepClosedPdfViewPositions', () => {
it('deletes the unscoped :pdf entry', () => {
const cache = new Map([['/a.pdf:pdf', position(4)]])
sweepClosedPdfViewPositions(cache, '/a.pdf')
sweepClosedPdfViewPositions(cache, ['/a.pdf'])
expect(cache.size).toBe(0)
})
@@ -17,7 +60,7 @@ describe('sweepClosedPdfViewPositions', () => {
['/a.pdf::tab-2:pdf', position(9)],
['/a.pdf::tab-3:pdf', position(11)]
])
sweepClosedPdfViewPositions(cache, '/a.pdf')
sweepClosedPdfViewPositions(cache, ['/a.pdf'])
expect(cache.size).toBe(0)
})
@@ -27,7 +70,7 @@ describe('sweepClosedPdfViewPositions', () => {
['/b.pdf:pdf', position(7)],
['/b.pdf::tab-2:pdf', position(8)]
])
sweepClosedPdfViewPositions(cache, '/a.pdf')
sweepClosedPdfViewPositions(cache, ['/a.pdf'])
expect([...cache.keys()]).toEqual(['/b.pdf:pdf', '/b.pdf::tab-2:pdf'])
})
@@ -36,13 +79,13 @@ describe('sweepClosedPdfViewPositions', () => {
['/report.pdf:pdf', position(2)],
['/report.pdf.bak:pdf', position(3)]
])
sweepClosedPdfViewPositions(cache, '/report.pdf')
sweepClosedPdfViewPositions(cache, ['/report.pdf'])
expect([...cache.keys()]).toEqual(['/report.pdf.bak:pdf'])
})
it('is a no-op when the file has no cached position', () => {
const cache = new Map([['/b.pdf:pdf', position(7)]])
sweepClosedPdfViewPositions(cache, '/a.pdf')
sweepClosedPdfViewPositions(cache, ['/a.pdf'])
expect(cache.size).toBe(1)
})
})
@@ -1,23 +1,53 @@
import type { PdfViewPosition } from '@/lib/scroll-cache'
function deleteCacheEntriesByPrefix<T>(cache: Map<string, T>, prefix: string): void {
/**
* Drops every pane-scoped (`<owner>::<pane>…`) entry belonging to any of `owners` in one pass over
* the cache, rather than one pass per owner. Split out from the cleanup hook so it is testable
* without pulling in the hook's `monaco-editor` import.
*/
export function deletePaneScopedCacheEntries<T>(
cache: Map<string, T>,
owners: readonly string[]
): void {
if (owners.length === 0) {
return
}
const ownerSet = new Set(owners)
for (const key of cache.keys()) {
if (key.startsWith(prefix)) {
if (hasPaneScopeOwner(key, ownerSet)) {
cache.delete(key)
}
}
}
/**
* Release the PDF positions a closed edit tab owns. Split out from the cleanup
* hook so it is testable without pulling in the hook's `monaco-editor` import.
*/
/** Equivalent to `key.startsWith(`${owner}::`)` for any owner in the set, probing `::` boundaries. */
function hasPaneScopeOwner(key: string, owners: ReadonlySet<string>): boolean {
for (
let boundary = key.indexOf('::');
boundary !== -1;
// `+ 1`, not `+ 2`: in a `:::` run the second `::` starts one char after the first, and it can
// be the only boundary an owner ends at (owner `a:` against key `a:::b`). Skipping to `+ 2`
// steps over it and silently leaks that entry.
boundary = key.indexOf('::', boundary + 1)
) {
if (owners.has(key.slice(0, boundary))) {
return true
}
}
return false
}
/** Release the PDF positions closed edit tabs own. */
export function sweepClosedPdfViewPositions(
cache: Map<string, PdfViewPosition>,
filePath: string
filePaths: readonly string[]
): void {
// Why: the `::`-scoped sweep does not cover the single-colon suffix, so the
// unscoped key needs its own delete (same shape as :rich / :preview).
cache.delete(`${filePath}:pdf`)
deleteCacheEntriesByPrefix(cache, `${filePath}::`)
for (const filePath of filePaths) {
cache.delete(`${filePath}:pdf`)
}
deletePaneScopedCacheEntries(cache, filePaths)
}
@@ -0,0 +1,259 @@
import { beforeEach, describe, expect, it } from 'vitest'
import {
diffViewStateCache,
editorSelectionCache,
pdfViewPositionCache,
scrollTopCache
} from '@/lib/scroll-cache'
import type { OpenFile } from '@/store/slices/editor'
import { disposeClosedEditorTabs } from './closed-editor-tab-disposal'
import {
getDiffViewerMonacoModelPaths,
getDiffViewerMonacoModelPathPrefixes,
type MonacoModelRegistry
} from './diff-monaco-model-disposal'
const CLOSED_DIFF_TAB_COUNT = 100
const RETAINED_MODEL_COUNT = 320
type FakeModel = {
path: string
attached: boolean
disposed: boolean
dispose: () => void
isAttachedToEditor: () => boolean
uri: { toString: (skipEncoding?: boolean) => string }
}
type FakeRegistry = MonacoModelRegistry & {
models: FakeModel[]
counters: { getModelsCalls: number; uriToStringCalls: number }
}
function createRegistry(models: FakeModel[]): FakeRegistry {
const counters = { getModelsCalls: 0, uriToStringCalls: 0 }
const byPath = new Map(models.map((model) => [model.path, model]))
for (const model of models) {
model.uri.toString = () => {
counters.uriToStringCalls += 1
return model.path
}
}
return {
models,
counters,
Uri: { parse: (value: string) => value },
editor: {
getModel: (uri: unknown) => byPath.get(String(uri)) ?? null,
getModels: () => {
counters.getModelsCalls += 1
return models
}
}
}
}
function createModel(path: string, attached = false): FakeModel {
const model: FakeModel = {
path,
attached,
disposed: false,
dispose: () => {
model.disposed = true
},
isAttachedToEditor: () => model.attached,
uri: { toString: () => path }
}
return model
}
function diffTab(id: string): OpenFile {
return { id, mode: 'diff', filePath: `/repo/${id}.ts` } as OpenFile
}
/** The pre-fix shape: one full registry scan, with both URI renderings, per owned prefix. */
function disposeByPrefixPerTab(registry: FakeRegistry, prefixes: readonly string[]): void {
for (const prefix of prefixes) {
for (const model of registry.editor.getModels()) {
const uriString = model.uri.toString(true)
const encodedUriString = model.uri.toString()
if (
uriString === prefix ||
uriString.startsWith(`${prefix}:`) ||
encodedUriString === prefix ||
encodedUriString.startsWith(`${prefix}:`)
) {
if (!model.isAttachedToEditor()) {
model.dispose()
}
}
}
}
}
/**
* 100 closed diff tabs, of which 60 still hold retained models (some with a large-diff generation
* suffix, some attached), plus 200 unrelated retained models from other tabs.
*/
function buildScenario(): {
closedTabs: OpenFile[]
models: FakeModel[]
prefixes: string[]
} {
const closedTabs = Array.from({ length: CLOSED_DIFF_TAB_COUNT }, (_, i) => diffTab(`tab-${i}`))
const models: FakeModel[] = []
for (let i = 0; i < 60; i += 1) {
const base = getDiffViewerMonacoModelPaths({
modelKey: `tab-${i}`,
generationSuffix: ''
})
models.push(createModel(base.originalModelPath, i % 10 === 0))
models.push(createModel(base.modifiedModelPath))
if (i % 3 === 0) {
const regenerated = getDiffViewerMonacoModelPaths({
modelKey: `tab-${i}`,
generationSuffix: ':large-diff-generation:2'
})
models.push(createModel(regenerated.originalModelPath))
}
}
// Still-open tabs and plain edit models the sweep must not touch.
for (let i = 0; models.length < RETAINED_MODEL_COUNT; i += 1) {
const stillOpen = getDiffViewerMonacoModelPaths({
modelKey: `open-tab-${i}`,
generationSuffix: ''
})
models.push(createModel(stillOpen.originalModelPath))
models.push(createModel(`/repo/src/file-${i}.ts`))
}
const prefixes = closedTabs.flatMap((tab) => {
const { originalModelPathPrefix, modifiedModelPathPrefix } =
getDiffViewerMonacoModelPathPrefixes(tab.id)
return [originalModelPathPrefix, modifiedModelPathPrefix]
})
return { closedTabs, models, prefixes }
}
beforeEach(() => {
scrollTopCache.clear()
editorSelectionCache.clear()
diffViewStateCache.clear()
pdfViewPositionCache.clear()
})
describe('disposeClosedEditorTabs', () => {
it('scans the model registry once per batch instead of twice per closed diff tab', () => {
const batched = buildScenario()
const batchedRegistry = createRegistry(batched.models)
disposeClosedEditorTabs(batchedRegistry, batched.closedTabs)
const perTab = buildScenario()
const perTabRegistry = createRegistry(perTab.models)
disposeByPrefixPerTab(perTabRegistry, perTab.prefixes)
// Pre-fix: 2 scans per closed tab, each rendering both URI forms for every retained model.
expect(perTabRegistry.counters.getModelsCalls).toBe(CLOSED_DIFF_TAB_COUNT * 2)
expect(perTabRegistry.counters.uriToStringCalls).toBe(
CLOSED_DIFF_TAB_COUNT * 2 * perTab.models.length * 2
)
expect(batchedRegistry.counters.getModelsCalls).toBe(1)
expect(batchedRegistry.counters.uriToStringCalls).toBeLessThanOrEqual(batched.models.length * 2)
})
it('disposes exactly the models the per-tab sweep disposed', () => {
const batched = buildScenario()
disposeClosedEditorTabs(createRegistry(batched.models), batched.closedTabs)
const perTab = buildScenario()
disposeByPrefixPerTab(createRegistry(perTab.models), perTab.prefixes)
const disposedPaths = (models: FakeModel[]): string[] =>
models
.filter((m) => m.disposed)
.map((m) => m.path)
.sort()
expect(disposedPaths(batched.models)).toEqual(disposedPaths(perTab.models))
expect(disposedPaths(batched.models).length).toBeGreaterThan(0)
// Attached models survive, as does everything owned by a still-open tab.
expect(batched.models.filter((m) => m.attached).every((m) => !m.disposed)).toBe(true)
expect(
batched.models.filter((m) => m.path.includes('open-tab-')).every((m) => !m.disposed)
).toBe(true)
})
it('sweeps pane-scoped cache entries for closed edit tabs in one pass per cache', () => {
scrollTopCache.set('/repo/a.ts', 10)
scrollTopCache.set('/repo/a.ts::pane-1', 20)
scrollTopCache.set('/repo/a.ts:rich', 30)
scrollTopCache.set('/repo/b.ts::pane-1', 40)
editorSelectionCache.set('/repo/a.ts::pane-2', [] as never)
pdfViewPositionCache.set('/repo/a.ts:pdf', {
pageNumber: 1,
top: 0,
left: 0
})
pdfViewPositionCache.set('/repo/a.ts::pane-1:pdf', {
pageNumber: 2,
top: 0,
left: 0
})
disposeClosedEditorTabs(createRegistry([]), [
{ id: '/repo/a.ts', mode: 'edit', filePath: '/repo/a.ts' } as OpenFile
])
expect([...scrollTopCache.keys()]).toEqual(['/repo/b.ts::pane-1'])
expect(editorSelectionCache.size).toBe(0)
expect(pdfViewPositionCache.size).toBe(0)
})
it('drops diff view state and preview scroll entries for closed diff tabs', () => {
diffViewStateCache.set('tab-1', {} as never)
diffViewStateCache.set('tab-1::pane-1', {} as never)
diffViewStateCache.set('tab-10', {} as never)
scrollTopCache.set('tab-1:preview', 5)
scrollTopCache.set('tab-1::pane-1', 6)
disposeClosedEditorTabs(createRegistry([]), [diffTab('tab-1')])
expect([...diffViewStateCache.keys()]).toEqual(['tab-10'])
expect(scrollTopCache.size).toBe(0)
})
// Why this is not covered by the parity test above: `buildScenario` closes tab-0..tab-99, so
// tab-10 is in the closed batch too. Prefix bleed from tab-1 would dispose tab-10's models, but
// the per-tab oracle disposes them as well via tab-10's own prefix, so the two agree and the
// assertion still passes. Isolating it needs a still-OPEN tab whose id extends a closed one.
it('does not dispose a still-open tab whose id extends a closed tab id', () => {
const closed = getDiffViewerMonacoModelPaths({ modelKey: 'tab-1', generationSuffix: '' })
const stillOpen = getDiffViewerMonacoModelPaths({ modelKey: 'tab-10', generationSuffix: '' })
const models = [
createModel(closed.originalModelPath),
createModel(closed.modifiedModelPath),
createModel(stillOpen.originalModelPath),
createModel(stillOpen.modifiedModelPath)
]
// Batched entry point on purpose: the owned prefixes become a Set probed at the URI's own `:`
// boundaries, which is a different predicate from the pre-batch per-prefix `startsWith`.
disposeClosedEditorTabs(createRegistry(models), [diffTab('tab-1'), diffTab('tab-2')])
expect(models.filter((m) => m.disposed).map((m) => m.path)).toEqual([
closed.originalModelPath,
closed.modifiedModelPath
])
})
it('is a no-op when nothing closed', () => {
const registry = createRegistry([createModel('diff:original:tab-1:tab-1')])
disposeClosedEditorTabs(registry, [])
expect(registry.counters.getModelsCalls).toBe(0)
expect(registry.models[0].disposed).toBe(false)
})
})
@@ -0,0 +1,86 @@
import type { OpenFile } from '@/store/slices/editor'
import {
editorSelectionCache,
diffViewStateCache,
pdfViewPositionCache,
scrollTopCache
} from '@/lib/scroll-cache'
import {
disposeUnattachedMonacoModelsByPathPrefixes,
getDiffViewerMonacoModelPathPrefixes,
type MonacoModelRegistry
} from './diff-monaco-model-disposal'
import {
deletePaneScopedCacheEntries,
sweepClosedPdfViewPositions
} from './closed-editor-tab-cache-sweep'
/**
* Releases the Monaco models and view-state cache entries owned by a batch of closed tabs.
*
* Why the batch shape: every prefix sweep here is a full scan of a shared registry or cache, so
* doing one per closed tab makes "close all"/worktree-switch quadratic in retained models. Takes
* the monaco namespace as an argument so it stays testable without importing `monaco-editor`.
*/
export function disposeClosedEditorTabs(
monacoRegistry: MonacoModelRegistry,
closedFiles: readonly OpenFile[]
): void {
if (closedFiles.length === 0) {
return
}
const diffModelPathPrefixes: string[] = []
const scrollTopOwners: string[] = []
const editorSelectionOwners: string[] = []
const diffViewStateOwners: string[] = []
const closedPdfFilePaths: string[] = []
for (const closedFile of closedFiles) {
switch (closedFile.mode) {
case 'edit':
// Why: the edit model URI is constructed via monaco.Uri.parse(filePath)
// to match @monaco-editor/react's `path` prop convention.
monacoRegistry.editor.getModel(monacoRegistry.Uri.parse(closedFile.filePath))?.dispose()
scrollTopCache.delete(closedFile.filePath)
// Why: markdown and mermaid surfaces keep mode-scoped scroll positions.
scrollTopCache.delete(`${closedFile.filePath}:rich`)
scrollTopCache.delete(`${closedFile.filePath}:preview`)
scrollTopCache.delete(`${closedFile.filePath}:mermaid-diagram`)
editorSelectionCache.delete(closedFile.filePath)
scrollTopOwners.push(closedFile.filePath)
editorSelectionOwners.push(closedFile.filePath)
// Why: only 'edit' tabs ever get a PDF scroll key (see EditorContent).
closedPdfFilePaths.push(closedFile.filePath)
break
case 'markdown-preview':
// Why: preview tabs own pane-scoped preview scroll cache entries even
// though they do not retain Monaco models.
scrollTopCache.delete(`${closedFile.id}:preview`)
scrollTopOwners.push(closedFile.id)
break
case 'diff': {
// Why: kept diff models are keyed by tab id, and fallback recovery can
// append generation suffixes; closing the tab owns that whole namespace.
const { originalModelPathPrefix, modifiedModelPathPrefix } =
getDiffViewerMonacoModelPathPrefixes(closedFile.id)
diffModelPathPrefixes.push(originalModelPathPrefix, modifiedModelPathPrefix)
diffViewStateCache.delete(closedFile.id)
diffViewStateOwners.push(closedFile.id)
scrollTopCache.delete(`${closedFile.id}:preview`)
scrollTopOwners.push(closedFile.id)
break
}
case 'conflict-review':
break
case 'check-details':
break
}
}
disposeUnattachedMonacoModelsByPathPrefixes(monacoRegistry, diffModelPathPrefixes)
deletePaneScopedCacheEntries(scrollTopCache, scrollTopOwners)
deletePaneScopedCacheEntries(editorSelectionCache, editorSelectionOwners)
deletePaneScopedCacheEntries(diffViewStateCache, diffViewStateOwners)
sweepClosedPdfViewPositions(pdfViewPositionCache, closedPdfFilePaths)
}
@@ -0,0 +1,182 @@
// @vitest-environment happy-dom
import React, { act, useCallback, useMemo, useState, type ReactElement } from 'react'
import { createRoot, type Root } from 'react-dom/client'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import type { GitBranchChangeEntry } from '../../../../../../shared/git-diff-compare-types'
import type { CombinedDiffFileTreeRow as CombinedDiffFileTreeRowComponent } from './combined-diff-file-tree-row'
const rowRenders = vi.hoisted(() => ({ count: 0 }))
const mountedRows = vi.hoisted(() => ({ count: 0 }))
vi.mock('@/store', () => ({
useAppStore: (selector: (state: Record<string, unknown>) => unknown) =>
selector({ combinedDiffFileTreeWidth: 420, setCombinedDiffFileTreeWidth: () => {} })
}))
vi.mock('./combined-diff-file-tree-row', async (importOriginal) => {
const actual = (await importOriginal()) as {
CombinedDiffFileTreeRow: typeof CombinedDiffFileTreeRowComponent
}
const react = await import('react')
const Row = actual.CombinedDiffFileTreeRow
// Why: memo with React's default shallow compare, so this counts exactly the render commits
// the real memo'd row would have performed.
const CountingRow = react.memo((props: React.ComponentProps<typeof Row>) => {
rowRenders.count += 1
react.useEffect(() => {
mountedRows.count += 1
return () => {
mountedRows.count -= 1
}
}, [])
return react.createElement(Row, props)
})
return { ...actual, CombinedDiffFileTreeRow: CountingRow }
})
const { CombinedDiffFileTree } = await import('./combined-diff-file-tree')
const { createCombinedDiffSectionIndexMap } =
await import('../resolve-changes/combined-diff-section-identity')
const { useCombinedDiffSectionIndexMap } =
await import('../resolve-changes/use-combined-diff-section-index-map')
const { getCombinedDiffBranchEntriesInTreeOrder } = await import('./combined-diff-file-tree-filter')
const EMPTY_VIEWED_KEYS: ReadonlySet<string> = new Set()
class NoopResizeObserver implements ResizeObserver {
observe(): void {}
unobserve(): void {}
disconnect(): void {}
}
let host: HTMLDivElement
let root: Root
beforeEach(() => {
globalThis.IS_REACT_ACT_ENVIRONMENT = true
rowRenders.count = 0
mountedRows.count = 0
host = document.createElement('div')
document.body.appendChild(host)
root = createRoot(host)
vi.stubGlobal('ResizeObserver', NoopResizeObserver)
})
afterEach(() => {
act(() => root.unmount())
host.remove()
vi.unstubAllGlobals()
vi.restoreAllMocks()
})
type TestSection = { key: string; loading: boolean }
/** `fileCount` files spread over `directoryCount` directories, in the viewer's own tree order. */
function buildEntries(fileCount: number, directoryCount: number): GitBranchChangeEntry[] {
const raw: GitBranchChangeEntry[] = Array.from({ length: fileCount }, (_, index) => ({
path: `src/dir${String(index % directoryCount).padStart(2, '0')}/file-${String(index).padStart(4, '0')}.ts`,
status: 'modified'
}))
return getCombinedDiffBranchEntriesInTreeOrder('commit', raw)
}
function buildSections(entries: readonly GitBranchChangeEntry[]): TestSection[] {
return entries.map((entry) => ({ key: `combined-commit:${entry.path}`, loading: true }))
}
let loadSectionAt: (index: number) => void = () => {}
/**
* Mirrors how both PR viewers feed the tree: an on-demand section load replaces the sections
* array, and the navigate callback closes over the section index map.
*/
function TreeHarness({
entries,
initialSections,
stableSectionIndexMap
}: {
entries: readonly GitBranchChangeEntry[]
initialSections: TestSection[]
stableSectionIndexMap: boolean
}): ReactElement {
const [sections, setSections] = useState(initialSections)
loadSectionAt = (index) => {
setSections((prev) =>
prev.map((section, sectionIndex) =>
sectionIndex === index ? { ...section, loading: false } : section
)
)
}
const rebuiltMap = useMemo(() => createCombinedDiffSectionIndexMap(sections), [sections])
const cachedMap = useCombinedDiffSectionIndexMap({ entrySignature: 'pr-1', sections })
const sectionIndexByKey = stableSectionIndexMap ? cachedMap : rebuiltMap
const onNavigate = useCallback(() => {
void sectionIndexByKey
}, [sectionIndexByKey])
return (
<CombinedDiffFileTree
mode="commit"
worktreePath="/repo"
entries={entries}
sectionIndexByKey={sectionIndexByKey}
activeSectionKey={null}
viewedSectionKeys={EMPTY_VIEWED_KEYS}
collapsed={false}
onCollapsedChange={() => {}}
onNavigate={onNavigate}
/>
)
}
function mountedRowCount(): number {
return mountedRows.count
}
function renderHarness(
entries: readonly GitBranchChangeEntry[],
sections: TestSection[],
stableSectionIndexMap: boolean
): void {
act(() => {
root.render(
<TreeHarness
entries={entries}
initialSections={sections}
stableSectionIndexMap={stableSectionIndexMap}
/>
)
})
}
/** One section load per commit, the way lazy loads land while the user scrolls. */
function runScrollPass(loadCount: number): number {
rowRenders.count = 0
for (let index = 0; index < loadCount; index += 1) {
act(() => loadSectionAt(index))
}
return rowRenders.count
}
describe('combined diff file tree re-renders on section loads', () => {
const SMALL_FILE_COUNT = 30
const SCROLL_PASS_LOADS = 10
it('re-renders every row per section load when the section index map is rebuilt', () => {
const entries = buildEntries(SMALL_FILE_COUNT, 3)
renderHarness(entries, buildSections(entries), false)
const rowCount = mountedRowCount()
expect(rowCount).toBeGreaterThan(SMALL_FILE_COUNT)
expect(runScrollPass(SCROLL_PASS_LOADS)).toBe(rowCount * SCROLL_PASS_LOADS)
})
it('re-renders no rows per section load when the section index map keeps its identity', () => {
const entries = buildEntries(SMALL_FILE_COUNT, 3)
renderHarness(entries, buildSections(entries), true)
expect(mountedRowCount()).toBeGreaterThan(SMALL_FILE_COUNT)
expect(runScrollPass(SCROLL_PASS_LOADS)).toBe(0)
})
})
@@ -2,10 +2,8 @@ import React, { useCallback, useRef, useState } from 'react'
import type { GitBranchChangeEntry } from '../../../../../../shared/git-diff-compare-types'
import type { GitStatusEntry } from '../../../../../../shared/git-status-types'
import type { DiffSection } from '../../diff-section-types'
import {
createCombinedDiffSectionIndexMap,
type CombinedDiffFileTreeMode
} from '../resolve-changes/combined-diff-section-identity'
import type { CombinedDiffFileTreeMode } from '../resolve-changes/combined-diff-section-identity'
import { useCombinedDiffSectionIndexMap } from '../resolve-changes/use-combined-diff-section-index-map'
import { handleCombinedDiffFileTreeNavigation } from './combined-diff-file-tree-navigation'
import { isCombinedDiffSectionViewed } from './combined-diff-file-tree-filter'
@@ -37,33 +35,7 @@ export function useCombinedDiffTreeNavigation({
toggleSection: (index: number) => void
treeMode: CombinedDiffFileTreeMode
}): CombinedDiffTreeNavigation {
const sectionIndexCacheRef = useRef<{
entrySignature: string
sectionCount: number
map: Map<string, number>
keys: string[]
} | null>(null)
const sectionIndexByKey = React.useMemo(() => {
const previous = sectionIndexCacheRef.current
// Section content/loading updates preserve entry order and keys. The entry signature and
// count usually change when the navigable structure changes, but compare keys as a guard for
// same-sized/reused signatures (and to keep this cache correct if a caller rebuilds sections).
if (
previous?.entrySignature === entrySignature &&
previous.sectionCount === sections.length &&
sections.every((section, index) => previous.keys[index] === section.key)
) {
return previous.map
}
const map = createCombinedDiffSectionIndexMap(sections)
sectionIndexCacheRef.current = {
entrySignature,
sectionCount: sections.length,
map,
keys: sections.map((section) => section.key)
}
return map
}, [entrySignature, sections])
const sectionIndexByKey = useCombinedDiffSectionIndexMap({ entrySignature, sections })
const sectionIndexByKeyRef = useRef<ReadonlyMap<string, number>>(sectionIndexByKey)
sectionIndexByKeyRef.current = sectionIndexByKey
@@ -47,11 +47,10 @@ export function useCombinedDiffSectionLoadRegistry(
const loadSectionRef = useRef<(index: number) => Promise<void>>(async () => {})
const retrySectionRef = useRef<(index: number) => void>(() => {})
const requestSectionReloadRef = useRef<(index: number) => void>(() => {})
const loadSchedulerRef = useRef(
createCombinedDiffLoadScheduler({
loadSection: (index) => loadSectionRef.current(index)
})
)
const loadSchedulerRef = useRef<ReturnType<typeof createCombinedDiffLoadScheduler>>(undefined!)
loadSchedulerRef.current ??= createCombinedDiffLoadScheduler({
loadSection: (index) => loadSectionRef.current(index)
})
sectionsRef.current = sections
useEffect(() => {
@@ -1,4 +1,4 @@
import { useCallback, useLayoutEffect, useRef } from 'react'
import { useCallback, useLayoutEffect, useRef, useState } from 'react'
import type React from 'react'
import type { VirtualizedScrollAnchor } from '@/hooks/useVirtualizedScrollAnchor'
import type { GitStatusEntry } from '../../../../../../shared/git-status-types'
@@ -65,13 +65,16 @@ export function useCombinedDiffViewRestore({
sectionLoadTokensRef
} = registry
const scrollOffsetRef = useRef(combinedDiffScrollTopCache.get(viewStateKey) ?? 0)
const scrollAnchorRef = useRef<VirtualizedScrollAnchor>(
combinedDiffScrollAnchorCache.get(viewStateKey) ?? null
)
const latestDomScrollAnchorRef = useRef<VirtualizedScrollAnchor>(
combinedDiffScrollAnchorCache.get(viewStateKey) ?? null
)
// Why useState and not `useRef(expr)`: the latter re-reads all three caches on every render and
// throws the result away, and an anchor seeds legitimately to null so a nullish guard would keep
// re-reading. useState's initializer runs once without writing a ref during render.
const [restoreSeed] = useState<{ offset: number; anchor: VirtualizedScrollAnchor }>(() => ({
offset: combinedDiffScrollTopCache.get(viewStateKey) ?? 0,
anchor: combinedDiffScrollAnchorCache.get(viewStateKey) ?? null
}))
const scrollOffsetRef = useRef(restoreSeed.offset)
const scrollAnchorRef = useRef<VirtualizedScrollAnchor>(restoreSeed.anchor)
const latestDomScrollAnchorRef = useRef<VirtualizedScrollAnchor>(restoreSeed.anchor)
// Why: tab/worktree switches unmount this viewer; cache by pane key so remount restores sections+scroll before repaint.
const initializedEntryStateRef = useRef<{
@@ -0,0 +1,63 @@
// @vitest-environment happy-dom
import { renderHook } from '@testing-library/react'
import { describe, expect, it } from 'vitest'
import { useCombinedDiffSectionIndexMap } from './use-combined-diff-section-index-map'
type Section = { key: string; loading: boolean }
function sectionsFor(keys: readonly string[], loadedKeys: readonly string[] = []): Section[] {
return keys.map((key) => ({ key, loading: !loadedKeys.includes(key) }))
}
describe('useCombinedDiffSectionIndexMap', () => {
const keys = ['combined-commit:a.ts', 'combined-commit:b.ts', 'combined-commit:c.ts']
it('keeps the map identity across a section load that leaves the keys alone', () => {
const { result, rerender } = renderHook(
({ sections }: { sections: Section[] }) =>
useCombinedDiffSectionIndexMap({ entrySignature: 'pr-1', sections }),
{ initialProps: { sections: sectionsFor(keys) } }
)
const first = result.current
// An on-demand load replaces the array and one section object; keys are untouched.
rerender({ sections: sectionsFor(keys, ['combined-commit:b.ts']) })
expect(result.current).toBe(first)
expect([...result.current]).toEqual([
['combined-commit:a.ts', 0],
['combined-commit:b.ts', 1],
['combined-commit:c.ts', 2]
])
})
it('rebuilds when a section key changes in place', () => {
const { result, rerender } = renderHook(
({ sections }: { sections: Section[] }) =>
useCombinedDiffSectionIndexMap({ entrySignature: 'pr-1', sections }),
{ initialProps: { sections: sectionsFor(keys) } }
)
const first = result.current
rerender({ sections: sectionsFor(['combined-commit:a.ts', 'combined-commit:renamed.ts']) })
expect(result.current).not.toBe(first)
expect(result.current.get('combined-commit:renamed.ts')).toBe(1)
expect(result.current.has('combined-commit:b.ts')).toBe(false)
})
it('rebuilds when the entry set changes even if the keys happen to match', () => {
const { result, rerender } = renderHook(
({ entrySignature }: { entrySignature: string }) =>
useCombinedDiffSectionIndexMap({ entrySignature, sections: sectionsFor(keys) }),
{ initialProps: { entrySignature: 'pr-1' } }
)
const first = result.current
rerender({ entrySignature: 'pr-2' })
expect(result.current).not.toBe(first)
expect([...result.current]).toEqual([...first])
})
})
@@ -0,0 +1,49 @@
import { useLayoutEffect, useMemo, useRef } from 'react'
import { createCombinedDiffSectionIndexMap } from './combined-diff-section-identity'
type CombinedDiffSectionIndexCache = {
entrySignature: string
sections: readonly { key: string }[]
map: Map<string, number>
}
/**
* Section-key to section-index map that keeps its identity while the section keys do.
*
* On-demand section loads replace the sections array on every fetch, so a freshly built Map would
* be a memo miss for every consumer — the file tree would re-render all of its rows continuously
* while the user scrolls a large diff.
*/
export function useCombinedDiffSectionIndexMap({
entrySignature,
sections
}: {
entrySignature: string
sections: readonly { key: string }[]
}): Map<string, number> {
const cacheRef = useRef<CombinedDiffSectionIndexCache | null>(null)
const sectionIndexByKey = useMemo(() => {
const previous = cacheRef.current
// Section content/loading updates preserve entry order and keys. The entry signature usually
// changes when the navigable structure changes, but compare keys as a guard for reused
// signatures (and to keep this cache correct if a caller rebuilds sections).
if (
previous !== null &&
previous.entrySignature === entrySignature &&
previous.sections.length === sections.length &&
sections.every((section, index) => previous.sections[index]?.key === section.key)
) {
return previous.map
}
return createCombinedDiffSectionIndexMap(sections)
}, [entrySignature, sections])
// Why a committed write and not a render-phase one: React can discard a render, and a cache
// seeded from abandoned work would hand a later render a map for sections that never existed.
// Layout, not passive, so a synchronous re-render inside the same commit still sees this cache.
useLayoutEffect(() => {
cacheRef.current = { entrySignature, sections, map: sectionIndexByKey }
}, [entrySignature, sectionIndexByKey, sections])
return sectionIndexByKey
}
@@ -2,7 +2,7 @@ import { describe, expect, it, vi } from 'vitest'
import {
disposeUnattachedDiffViewerMonacoModels,
disposeUnattachedMonacoModelPaths,
disposeUnattachedMonacoModelsByPathPrefix,
disposeUnattachedMonacoModelsByPathPrefixes,
getDiffViewerMonacoModelPathPrefixes,
getDiffViewerMonacoModelPaths
} from './diff-monaco-model-disposal'
@@ -139,7 +139,7 @@ describe('diff Monaco model disposal', () => {
const monacoRegistry = createRegistry(models)
const { originalModelPathPrefix } = getDiffViewerMonacoModelPathPrefixes('tab-1')
disposeUnattachedMonacoModelsByPathPrefix(monacoRegistry, originalModelPathPrefix)
disposeUnattachedMonacoModelsByPathPrefixes(monacoRegistry, [originalModelPathPrefix])
expect(baseDispose).toHaveBeenCalledOnce()
expect(generatedDispose).toHaveBeenCalledOnce()
@@ -177,7 +177,7 @@ describe('diff Monaco model disposal', () => {
const monacoRegistry = createRegistry(models)
const { originalModelPathPrefix } = getDiffViewerMonacoModelPathPrefixes('foo')
disposeUnattachedMonacoModelsByPathPrefix(monacoRegistry, originalModelPathPrefix)
disposeUnattachedMonacoModelsByPathPrefixes(monacoRegistry, [originalModelPathPrefix])
expect(ownedDispose).toHaveBeenCalledOnce()
expect(siblingDispose).not.toHaveBeenCalled()
@@ -16,7 +16,7 @@ type DisposableMonacoModel = Pick<editor.ITextModel, 'dispose' | 'isAttachedToEd
uri: { toString(skipEncoding?: boolean): string }
}
type MonacoModelRegistry = {
export type MonacoModelRegistry = {
Uri: {
parse(value: string): unknown
}
@@ -79,25 +79,71 @@ export function disposeUnattachedMonacoModelPaths(
}
}
export function disposeUnattachedMonacoModelsByPathPrefix(
/**
* Sweeps every owned prefix in a single scan of the global model registry.
*
* Why batched: `getModels()` returns every retained model in the app, so closing N diff tabs one
* prefix at a time costs N full scans and 2xN `uri.toString()` allocations per model. Closing 100
* tabs against 700 retained models is ~140k throwaway strings inside one synchronous effect.
*/
export function disposeUnattachedMonacoModelsByPathPrefixes(
monacoRegistry: MonacoModelRegistry,
modelPathPrefix: string
modelPathPrefixes: readonly string[]
): void {
for (const model of monacoRegistry.editor.getModels()) {
const uriString = model.uri.toString(true)
const encodedUriString = model.uri.toString()
if (modelPathPrefixes.length === 0) {
return
}
const ownedPrefixes = new Set(modelPathPrefixes)
let shortestPrefixLength = Number.POSITIVE_INFINITY
let longestPrefixLength = 0
for (const prefix of ownedPrefixes) {
shortestPrefixLength = Math.min(shortestPrefixLength, prefix.length)
longestPrefixLength = Math.max(longestPrefixLength, prefix.length)
}
const bounds = { shortestPrefixLength, longestPrefixLength }
for (const model of monacoRegistry.editor.getModels()) {
// Why both forms: model URIs are built via `Uri.parse`, so a prefix can match the decoded or
// the percent-encoded rendering depending on what characters the tab id carries.
if (
uriString === modelPathPrefix ||
uriString.startsWith(`${modelPathPrefix}:`) ||
encodedUriString === modelPathPrefix ||
encodedUriString.startsWith(`${modelPathPrefix}:`)
isOwnedByPathPrefix(model.uri.toString(true), ownedPrefixes, bounds) ||
isOwnedByPathPrefix(model.uri.toString(), ownedPrefixes, bounds)
) {
disposeUnattachedMonacoModel(model)
}
}
}
/**
* Equivalent to `uri === prefix || uri.startsWith(`${prefix}:`)` for any prefix in the set, but
* probes the URI's own `:` boundaries instead of testing every prefix — O(segments) not O(prefixes).
*/
function isOwnedByPathPrefix(
uriString: string,
ownedPrefixes: ReadonlySet<string>,
bounds: { shortestPrefixLength: number; longestPrefixLength: number }
): boolean {
if (ownedPrefixes.has(uriString)) {
return true
}
for (
let boundary = uriString.indexOf(':');
boundary !== -1 && boundary <= bounds.longestPrefixLength;
boundary = uriString.indexOf(':', boundary + 1)
) {
if (
boundary >= bounds.shortestPrefixLength &&
ownedPrefixes.has(uriString.slice(0, boundary))
) {
return true
}
}
return false
}
function disposeUnattachedMonacoModel(model: DisposableMonacoModel | null): void {
if (!model || model.isAttachedToEditor()) {
return
@@ -1,80 +1,22 @@
import { useEffect, useRef } from 'react'
import * as monaco from 'monaco-editor'
import type { OpenFile } from '@/store/slices/editor'
import {
editorSelectionCache,
diffViewStateCache,
pdfViewPositionCache,
scrollTopCache
} from '@/lib/scroll-cache'
import {
disposeUnattachedMonacoModelsByPathPrefix,
getDiffViewerMonacoModelPathPrefixes
} from './diff-monaco-model-disposal'
import { sweepClosedPdfViewPositions } from './closed-editor-tab-cache-sweep'
function deleteCacheEntriesByPrefix<T>(cache: Map<string, T>, prefix: string): void {
for (const key of cache.keys()) {
if (key.startsWith(prefix)) {
cache.delete(key)
}
}
}
import { disposeClosedEditorTabs } from './closed-editor-tab-disposal'
export function useClosedEditorTabCleanup(openFiles: OpenFile[]): void {
const prevOpenFilesRef = useRef<Map<string, OpenFile>>(new Map())
useEffect(() => {
const currentFilesById = new Map(openFiles.map((f) => [f.id, f]))
const closedFiles: OpenFile[] = []
for (const [prevId, prevFile] of prevOpenFilesRef.current) {
if (!currentFilesById.has(prevId)) {
disposeClosedEditorTab(prevId, prevFile)
closedFiles.push(prevFile)
}
}
// Why one call for the whole removal batch: each sweep scans a shared registry/cache, so
// per-tab sweeps make a "close all" quadratic in retained models.
disposeClosedEditorTabs(monaco, closedFiles)
prevOpenFilesRef.current = currentFilesById
}, [openFiles])
}
function disposeClosedEditorTab(prevId: string, prevFile: OpenFile): void {
switch (prevFile.mode) {
case 'edit':
// Why: the edit model URI is constructed via monaco.Uri.parse(filePath)
// to match @monaco-editor/react's `path` prop convention.
monaco.editor.getModel(monaco.Uri.parse(prevFile.filePath))?.dispose()
scrollTopCache.delete(prevFile.filePath)
deleteCacheEntriesByPrefix(scrollTopCache, `${prevFile.filePath}::`)
// Why: markdown and mermaid surfaces keep mode-scoped scroll positions.
scrollTopCache.delete(`${prevFile.filePath}:rich`)
scrollTopCache.delete(`${prevFile.filePath}:preview`)
scrollTopCache.delete(`${prevFile.filePath}:mermaid-diagram`)
editorSelectionCache.delete(prevFile.filePath)
deleteCacheEntriesByPrefix(editorSelectionCache, `${prevFile.filePath}::`)
// Why: only 'edit' tabs ever get a PDF scroll key (see EditorContent).
sweepClosedPdfViewPositions(pdfViewPositionCache, prevFile.filePath)
break
case 'markdown-preview':
// Why: preview tabs own pane-scoped preview scroll cache entries even
// though they do not retain Monaco models.
scrollTopCache.delete(`${prevFile.id}:preview`)
deleteCacheEntriesByPrefix(scrollTopCache, `${prevFile.id}::`)
break
case 'diff':
// Why: kept diff models are keyed by tab id, and fallback recovery can
// append generation suffixes; closing the tab owns that whole namespace.
{
const { originalModelPathPrefix, modifiedModelPathPrefix } =
getDiffViewerMonacoModelPathPrefixes(prevId)
disposeUnattachedMonacoModelsByPathPrefix(monaco, originalModelPathPrefix)
disposeUnattachedMonacoModelsByPathPrefix(monaco, modifiedModelPathPrefix)
}
diffViewStateCache.delete(prevId)
deleteCacheEntriesByPrefix(diffViewStateCache, `${prevId}::`)
scrollTopCache.delete(`${prevId}:preview`)
deleteCacheEntriesByPrefix(scrollTopCache, `${prevId}::`)
break
case 'conflict-review':
break
case 'check-details':
break
}
}
@@ -39,11 +39,13 @@ export function useEmulatorPaneSession({
const configuredDefaultUdid = useAppStore(
(state) => state.settings?.mobileEmulatorDefaultDeviceUdid ?? null
)
const prelaunchedSessionRef = useRef<EmulatorPaneSession['info'] | null>(
// Why the lazy initializer: the consume deletes the handoff entry, and a `useRef(expr)` argument
// re-runs every render — so a prelaunch registered after mount was consumed and then discarded.
const [prelaunchedSession] = useState<EmulatorPaneSession['info'] | null>(() =>
consumePrelaunchedSimulatorSession(worktreeId)
)
const prelaunchedState = buildPrelaunchedEmulatorSessionState(
prelaunchedSessionRef.current,
prelaunchedSession,
configuredDefaultUdid
)
const [selectedUdid, setSelectedUdid] = useState<string | null>(prelaunchedState.selectedUdid)
@@ -47,13 +47,14 @@ export function useFeatureWallSessionDepth(
visitedWorkbenchSteps: Set<WorkbenchStepId>
visitedReviewSteps: Set<ReviewStepId>
lastGroupId: FeatureWallWorkflowId | null
}>({
}>(undefined!)
sessionDepthRef.current ??= {
visitedWorkflows: new Set(),
visitedAgentSteps: new Set(),
visitedWorkbenchSteps: new Set(),
visitedReviewSteps: new Set(),
lastGroupId: null
})
}
const getTourDepthSummary = useCallback((): FeatureWallTourDepthSummary => {
const session = sessionDepthRef.current
@@ -77,7 +77,8 @@ export function useFeatureWallTourTelemetry(args: {
getDepthSummary: () => FeatureWallTourDepthSummary
}): { markExitAction: (exitAction: FeatureWallExitAction) => void } {
const { isOpen, source, getDepthSummary } = args
const telemetryRef = useRef<FeatureWallTourTelemetryState>(createFeatureWallTourTelemetryState())
const telemetryRef = useRef<FeatureWallTourTelemetryState>(undefined!)
telemetryRef.current ??= createFeatureWallTourTelemetryState()
const sourceRef = useRef(source)
const getDepthSummaryRef = useRef(getDepthSummary)
// Why: close telemetry may emit from stable callbacks; keep the payload
@@ -11,6 +11,10 @@ import {
type FloatingPanelStoreState
} from './floating-terminal-panel-test-fixtures'
import { mocks, setupFloatingTerminalPanelTest } from './floating-terminal-panel-test-harness'
import {
RENAME_TERMINAL_TAB_EVENT,
type RenameTerminalTabDetail
} from '@/components/tab-bar/terminal-tab-rename-request'
import {
attachRef,
bindFocusedFloatingPanelKeydown,
@@ -159,6 +163,15 @@ vi.mock('@/components/ShortcutKeyCombo', async () => {
return (await import('./floating-terminal-panel-component-stubs')).createShortcutKeyComboModule()
})
/** Tab ids the panel asked to rename, in dispatch order. */
function dispatchedRenameTabIds(): string[] {
return vi
.mocked(window.dispatchEvent)
.mock.calls.map(([event]) => event as CustomEvent<RenameTerminalTabDetail>)
.filter((event) => event.type === RENAME_TERMINAL_TAB_EVENT)
.map((event) => event.detail.tabId)
}
describe('FloatingTerminalPanel close behavior', () => {
beforeEach(setupFloatingTerminalPanelTest)
@@ -434,7 +447,7 @@ describe('FloatingTerminalPanel close behavior', () => {
expect(preventDefault).toHaveBeenCalledWith()
expect(stopPropagation).toHaveBeenCalledWith()
expect(stopImmediatePropagation).toHaveBeenCalledWith()
expect(mocks.setRenamingTabId).toHaveBeenCalledWith('tab-1')
expect(dispatchedRenameTabIds()).toEqual(['tab-1'])
expect(mocks.setTabCustomTitle).not.toHaveBeenCalled()
})
@@ -607,7 +620,7 @@ describe('FloatingTerminalPanel close behavior', () => {
})
)
expect(mocks.setRenamingTabId).not.toHaveBeenCalled()
expect(dispatchedRenameTabIds()).toEqual([])
})
it('leaves focused floating xterm tab index shortcuts to terminal-first terminals', async () => {
@@ -15,7 +15,6 @@ export type FloatingPanelStoreState = {
activeGroupIdByWorktree: Record<string, string | null>
activeTabIdByWorktree: Record<string, string | null>
expandedPaneByTabId: Record<string, boolean>
renamingTabId: string | null
createTab: (
worktreeId: string,
groupId?: string,
@@ -42,7 +41,6 @@ export type FloatingPanelStoreState = {
activateTab: (tabId: string) => void
setActiveTab: (tabId: string) => void
setTabCustomTitle: (tabId: string, title: string | null) => void
setRenamingTabId: (tabId: string | null) => void
setTabColor: (tabId: string, color: string | null) => void
setTabPaneExpanded: (tabId: string, expanded: boolean) => void
makePreviewFilePermanent: (fileId: string, tabId?: string) => void
@@ -59,7 +59,6 @@ export type FloatingTerminalPanelMocks = {
pinFile: Mock<FloatingPanelStoreState['pinFile']>
setFloatingFocus: Mock<(state: { panelFocused: boolean; terminalFocused: boolean }) => void>
setActiveTab: Mock<FloatingPanelStoreState['setActiveTab']>
setRenamingTabId: Mock<FloatingPanelStoreState['setRenamingTabId']>
setTabColor: Mock<FloatingPanelStoreState['setTabColor']>
setTabCustomTitle: Mock<FloatingPanelStoreState['setTabCustomTitle']>
setTabPaneExpanded: Mock<FloatingPanelStoreState['setTabPaneExpanded']>
@@ -106,7 +105,6 @@ export const mocks: FloatingTerminalPanelMocks = {
pinFile: vi.fn(),
setFloatingFocus: vi.fn(),
setActiveTab: vi.fn(),
setRenamingTabId: vi.fn(),
setTabColor: vi.fn(),
setTabCustomTitle: vi.fn(),
setTabPaneExpanded: vi.fn(),
@@ -133,7 +131,6 @@ function resetStore(tabs: TerminalTab[] = []): void {
activeGroupIdByWorktree: {},
activeTabIdByWorktree: { [FLOATING_TERMINAL_WORKTREE_ID]: tabs[0]?.id ?? null },
expandedPaneByTabId: {},
renamingTabId: null,
activateTab: mocks.activateTab,
closeBrowserTab: mocks.closeBrowserTab,
closeFile: mocks.closeFile,
@@ -147,7 +144,6 @@ function resetStore(tabs: TerminalTab[] = []): void {
pinFile: mocks.pinFile,
setActiveTab: mocks.setActiveTab,
setTabCustomTitle: mocks.setTabCustomTitle,
setRenamingTabId: mocks.setRenamingTabId,
setTabColor: mocks.setTabColor,
setTabPaneExpanded: mocks.setTabPaneExpanded,
browserDefaultUrl: 'about:blank',
@@ -196,6 +192,7 @@ export async function setupFloatingTerminalPanelTest(): Promise<void> {
}
vi.stubGlobal('window', {
addEventListener: vi.fn(),
dispatchEvent: vi.fn(),
api: {
app: {
getFloatingMarkdownDirectory: mocks.getFloatingMarkdownDirectory,
@@ -7,6 +7,7 @@ import {
} from '@/lib/floating-workspace-shortcut-policy'
import { isFloatingWorkspaceTerminalInputTarget } from '@/lib/floating-workspace-terminal-actions'
import { getShortcutPlatform } from '@/lib/shortcut-platform'
import { requestTerminalTabRename } from '@/components/tab-bar/terminal-tab-rename-request'
import { useAppStore } from '@/store'
import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants'
import type { KeybindingContext, KeybindingMatchOptions } from '../../../../shared/keybindings'
@@ -168,7 +169,7 @@ export function useFloatingTerminalPanelShortcuts({
return 'unmatched'
}
consume()
useAppStore.getState().setRenamingTabId(activeTab.id)
requestTerminalTabRename(activeTab.id)
return 'handled'
}
consume()
@@ -0,0 +1,108 @@
// @vitest-environment happy-dom
import { act } from 'react'
import { createRoot, type Root } from 'react-dom/client'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import type { GitHubPRFile } from '../../../../../shared/github/pull-request-types'
import type { PRFilesCombinedDiffViewerProps } from '@/components/github/pr-file-diff-mapping'
const capturedSectionIndexMaps = vi.hoisted(() => ({ list: [] as ReadonlyMap<string, number>[] }))
const loaders = vi.hoisted(() => ({ loadSection: (_index: number) => {} }))
vi.mock('@/store', () => ({
useAppStore: (selector: (state: Record<string, unknown>) => unknown) =>
selector({ settings: { theme: 'dark' } })
}))
vi.mock('./pr-files-combined-diff-body', () => ({
PRFilesCombinedDiffBody: (props: {
sectionIndexByKey: ReadonlyMap<string, number>
loadSection: (index: number) => void
}) => {
capturedSectionIndexMaps.list.push(props.sectionIndexByKey)
loaders.loadSection = props.loadSection
return null
}
}))
vi.mock('./pr-files-combined-diff-load', () => ({
addPRFilesCombinedDiffLineComment: () => Promise.resolve(),
// Why: stands in for the network fetch, keeping only the state shape a real load produces —
// a new sections array with one patched section and untouched keys.
loadPRFilesCombinedDiffSection: ({
index,
setSections
}: {
index: number
setSections: (updater: (prev: { key: string; loading: boolean }[]) => unknown) => void
}) => {
setSections((prev) =>
prev.map((section, sectionIndex) =>
sectionIndex === index ? { ...section, loading: false } : section
)
)
},
retryPRFilesCombinedDiffSection: () => {},
setAllPRFilesCombinedDiffSectionsCollapsed: () => {},
togglePRFilesCombinedDiffSection: () => {}
}))
const { PRFilesCombinedDiffViewer } = await import('./pr-files-combined-diff-viewer')
let host: HTMLDivElement
let root: Root
beforeEach(() => {
globalThis.IS_REACT_ACT_ENVIRONMENT = true
capturedSectionIndexMaps.list = []
host = document.createElement('div')
document.body.appendChild(host)
root = createRoot(host)
})
afterEach(() => {
act(() => root.unmount())
host.remove()
vi.restoreAllMocks()
})
function prFiles(count: number): GitHubPRFile[] {
return Array.from({ length: count }, (_, index) => ({
path: `src/file-${String(index).padStart(3, '0')}.ts`,
status: 'modified' as const,
additions: 1,
deletions: 1,
isBinary: false
}))
}
const viewerProps: PRFilesCombinedDiffViewerProps = {
files: prFiles(6),
comments: [],
repoPath: '/repo',
repoId: 'repo-1',
prNumber: 7,
prUrl: 'https://example.test/pr/7',
headSha: 'head',
baseSha: 'base',
pendingViewedPaths: new Set(),
onCommentAdded: () => {},
onViewedChange: () => Promise.resolve(true)
}
describe('inspect-pull-request combined diff section index map', () => {
it('keeps one map identity across on-demand section loads', () => {
act(() => {
root.render(<PRFilesCombinedDiffViewer {...viewerProps} />)
})
const initialMap = capturedSectionIndexMaps.list.at(-1)
expect(initialMap?.size).toBe(viewerProps.files.length)
for (let index = 0; index < viewerProps.files.length; index += 1) {
act(() => loaders.loadSection(index))
}
expect(capturedSectionIndexMaps.list.length).toBeGreaterThan(viewerProps.files.length)
expect(new Set(capturedSectionIndexMaps.list).size).toBe(1)
})
})
@@ -2,7 +2,7 @@ import React, { useCallback, useLayoutEffect, useMemo, useRef, useState } from '
import { useVirtualizer } from '@tanstack/react-virtual'
import type { editor as monacoEditor } from 'monaco-editor'
import type { DecoratedDiffComment } from '@/components/diff-comments/decorated-diff-comment'
import { createCombinedDiffSectionIndexMap } from '../../editor/combined-diff/resolve-changes/combined-diff-section-identity'
import { useCombinedDiffSectionIndexMap } from '../../editor/combined-diff/resolve-changes/use-combined-diff-section-index-map'
import { handleCombinedDiffFileTreeNavigation } from '../../editor/combined-diff/browse-files/combined-diff-file-tree-navigation'
import { getDiffSectionRowEstimatedHeight } from '@/components/editor/diff-section-layout'
import type { DiffSection } from '@/components/editor/diff-section-types'
@@ -30,6 +30,7 @@ import {
} from './pr-files-combined-diff-load'
type PRFilesCombinedDiffSectionsProps = PRFilesCombinedDiffViewerProps & {
signature: string
sideBySide: boolean
setSideBySide: React.Dispatch<React.SetStateAction<boolean>>
fileTreeCollapsed: boolean
@@ -61,6 +62,7 @@ export function PRFilesCombinedDiffViewer(
<PRFilesCombinedDiffSections
key={signature}
{...props}
signature={signature}
sideBySide={sideBySide}
setSideBySide={setSideBySide}
fileTreeCollapsed={fileTreeCollapsed}
@@ -83,6 +85,7 @@ function PRFilesCombinedDiffSections({
pendingViewedPaths,
onCommentAdded,
onViewedChange,
signature,
sideBySide,
setSideBySide,
fileTreeCollapsed,
@@ -222,7 +225,7 @@ function PRFilesCombinedDiffSections({
)
const allSectionsCollapsed = sections.length > 0 && sections.every((section) => section.collapsed)
const sectionIndexByKey = useMemo(() => createCombinedDiffSectionIndexMap(sections), [sections])
const sectionIndexByKey = useCombinedDiffSectionIndexMap({ entrySignature: signature, sections })
const viewedSectionKeys = useMemo(
() => new Set(files.filter(isPRFileViewed).map((file) => getPRFileSectionKey(file.path))),
[files]

Some files were not shown because too many files have changed in this diff Show More