From fdfe354045680a01b3743600362fd37262031af4 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 1 Sep 2026 18:20:47 -0700 Subject: [PATCH 1/3] test(relay): bind test WebSocket servers to loopback A control-handshake test that expects a timeout was instead getting 'Unexpected server response: 401' about once in fourteen runs. A slow machine cannot turn a timeout into a 401 -- that needs a real HTTP response, so the connection was reaching a different server. new WebSocketServer({ port: 0 }) binds the wildcard address while the client dials 127.0.0.1. On macOS those differ, and with SO_REUSEADDR a foreign process can hold the more specific 127.0.0.1:P and win the connection. Caught live: a wildcard bind took port 52584, which a running Orca app already held on loopback, and Orca answered the probe. A listener that checks a token answers 401. Ten constructions across seven files now pass host: '127.0.0.1', so the reservation covers the address the client dials and a duplicate bind is refused. Adds a ratchet, because this is not authors forgetting a convention: all 30+ .listen(0, ...) sites already pass '127.0.0.1', while 7 of 7 ws constructions did not. ws accepts { port } alone and binds the wildcard silently, so nothing told them. The guard pins the wildcard count, and pins separately at zero the option shapes it cannot read -- spreads and variable option objects fail rather than being exempted, and a recognized-construction floor catches the matcher going blind, which otherwise reads exactly like a clean tree. mobile/scripts/mock-server.ts stays on the wildcard deliberately: a phone reaches it over the LAN. --- ...bsocket-server-wildcard-bind-allowlist.txt | 14 ++ config/scripts/call-site-option-keys.ts | 204 ++++++++++++++++++ config/scripts/websocket-server-bind-scan.ts | 172 +++++++++++++++ .../websocket-server-loopback-bind.test.ts | 106 +++++++++ .../rpc-client-live-recovery.test.ts | 3 +- .../mobile-relay-e2ee.integration.test.ts | 3 +- .../relay/relay-control-client.test.ts | 7 +- src/main/runtime/rpc/relay-transport.test.ts | 3 +- .../src/web/web-runtime-client.test.ts | 3 +- src/shared/remote-runtime-client.test.ts | 13 +- .../remote-runtime-outbound-admission.test.ts | 3 +- .../remote-runtime-request-connection.test.ts | 3 +- ...mote-runtime-shared-control-test-server.ts | 7 +- ...emote-runtime-subscription-request.test.ts | 3 +- 14 files changed, 529 insertions(+), 15 deletions(-) create mode 100644 config/scripts/__fixtures__/websocket-server-wildcard-bind-allowlist.txt create mode 100644 config/scripts/call-site-option-keys.ts create mode 100644 config/scripts/websocket-server-bind-scan.ts create mode 100644 config/scripts/websocket-server-loopback-bind.test.ts diff --git a/config/scripts/__fixtures__/websocket-server-wildcard-bind-allowlist.txt b/config/scripts/__fixtures__/websocket-server-wildcard-bind-allowlist.txt new file mode 100644 index 00000000000..4b76622a9f2 --- /dev/null +++ b/config/scripts/__fixtures__/websocket-server-wildcard-bind-allowlist.txt @@ -0,0 +1,14 @@ +# Files allowed to construct a `ws` server that binds a port without pinning `host`. +# +# `ws` accepts `{ port }` alone and silently binds the wildcard address. A server +# reached over 127.0.0.1 must pin `host: '127.0.0.1'`, or a foreign loopback +# listener can hold the same port and answer in its place -- which is how +# relay-control-client.test.ts came to fail with a real HTTP 401 in a test that +# was simulating silence. +# +# This list only shrinks. Adding a line also requires raising the pin in +# websocket-server-loopback-bind.test.ts, which is deliberate friction. + +# Deliberate, not drift: this mock is dialled by a phone on the LAN, so it has to +# be reachable on a real interface. A loopback bind would make it unreachable. +mobile/scripts/mock-server.ts diff --git a/config/scripts/call-site-option-keys.ts b/config/scripts/call-site-option-keys.ts new file mode 100644 index 00000000000..dff3a971c45 --- /dev/null +++ b/config/scripts/call-site-option-keys.ts @@ -0,0 +1,204 @@ +/** + * Read the top-level option keys of a call's object-literal argument out of raw + * source text. + * + * Text rather than an AST because typescript@7 no longer ships the classic + * compiler API and every installed parser is a transitive dependency. The + * tradeoff is handled by refusing to guess: any shape this cannot read comes + * back as `unreadable` with a reason, and callers must treat that as a failure + * rather than as an absence of keys. + */ + +export type CallOptionKeys = + | { readonly readable: true; readonly keys: readonly string[] } + | { readonly readable: false; readonly reason: string } + +type ScanState = 'code' | 'line' | 'block' | 'single' | 'double' | 'template' + +function closesString(state: ScanState, current: string): boolean { + return ( + (state === 'single' && current === "'") || + (state === 'double' && current === '"') || + (state === 'template' && current === '`') + ) +} + +function opensNonCode(current: string, next: string | undefined): ScanState | null { + if (current === '/' && next === '/') { + return 'line' + } + if (current === '/' && next === '*') { + return 'block' + } + if (current === "'") { + return 'single' + } + if (current === '"') { + return 'double' + } + if (current === '`') { + return 'template' + } + return null +} + +/** + * Text between an open paren and its match, tracking strings and comments so a + * brace inside either cannot unbalance the count. Null when it never closes. + */ +function balancedArguments(text: string, openIndex: number): string | null { + let depth = 0 + let state: ScanState = 'code' + for (let index = openIndex; index < text.length; index++) { + const current = text[index] + const next = text[index + 1] + if (state === 'code') { + const opened = opensNonCode(current, next) + if (opened) { + state = opened + if (opened === 'line' || opened === 'block') { + index++ + } + } else if (current === '(' || current === '{' || current === '[') { + depth++ + } else if (current === ')' || current === '}' || current === ']') { + depth-- + if (depth === 0) { + return text.slice(openIndex + 1, index) + } + if (depth < 0) { + return null + } + } + continue + } + if (state === 'line') { + if (current === '\n') { + state = 'code' + } + continue + } + if (state === 'block') { + if (current === '*' && next === '/') { + state = 'code' + index++ + } + continue + } + if (current === '\\') { + index++ + continue + } + // Brace tracking inside `${}` would need its own depth; templates never + // appear as options, so report one as unreadable instead of guessing. + if (state === 'template' && current === '$' && next === '{') { + return null + } + if (closesString(state, current)) { + state = 'code' + } + } + return null +} + +/** Keys at depth 0 of an object literal body, with anything non-identifier kept verbatim. */ +function objectLiteralKeys(body: string): string[] { + const keys: string[] = [] + let depth = 0 + let state: ScanState = 'code' + let inValue = false + let token = '' + const flush = (): void => { + const name = token.trim() + token = '' + if (name && depth === 0) { + keys.push(name) + } + } + for (let index = 0; index < body.length; index++) { + const current = body[index] + const next = body[index + 1] + if (state === 'code') { + const opened = opensNonCode(current, next) + if (opened) { + state = opened + if (opened === 'line' || opened === 'block') { + index++ + } + } else if (current === '(' || current === '{' || current === '[') { + depth++ + if (!inValue) { + token += current + } + } else if (current === ')' || current === '}' || current === ']') { + depth-- + if (!inValue) { + token += current + } + } else if (current === ':' && depth === 0 && !inValue) { + flush() + inValue = true + } else if (current === ',' && depth === 0) { + // A shorthand or a spread ends here having never seen a colon. + if (inValue) { + inValue = false + token = '' + } else { + flush() + } + } else if (!inValue) { + token += current + } + continue + } + if (state === 'line') { + if (current === '\n') { + state = 'code' + } + continue + } + if (state === 'block') { + if (current === '*' && next === '/') { + state = 'code' + index++ + } + continue + } + if (current === '\\') { + index++ + continue + } + if (closesString(state, current)) { + state = 'code' + } + } + if (!inValue) { + flush() + } + return keys +} + +/** + * Option keys of the call whose argument list opens at `parenIndex`, or the + * reason the shape could not be read. Spreads and computed keys land in the + * latter: either can carry a key this would otherwise report as absent. + */ +export function readCallOptionKeys(text: string, parenIndex: number): CallOptionKeys { + const args = balancedArguments(text, parenIndex) + if (args === null) { + return { readable: false, reason: 'argument list never closes' } + } + if (!args.trim()) { + return { readable: false, reason: 'called with no options argument' } + } + const trimmed = args.trim() + if (!trimmed.startsWith('{') || !trimmed.endsWith('}')) { + return { readable: false, reason: 'options are not an object literal' } + } + const keys = objectLiteralKeys(trimmed.slice(1, -1)) + const unreadable = keys.find((key) => !/^[A-Za-z_$][\w$]*$/.test(key)) + if (unreadable !== undefined) { + return { readable: false, reason: `unreadable option key \`${unreadable}\`` } + } + return { readable: true, keys } +} diff --git a/config/scripts/websocket-server-bind-scan.ts b/config/scripts/websocket-server-bind-scan.ts new file mode 100644 index 00000000000..9054f8cc38e --- /dev/null +++ b/config/scripts/websocket-server-bind-scan.ts @@ -0,0 +1,172 @@ +import { readFileSync, readdirSync } from 'node:fs' +import { join, relative } from 'node:path' +import { readCallOptionKeys } from './call-site-option-keys' + +/** + * Locate every `new WebSocketServer(...)` in the tree and say, for each, whether + * it pins a bind address. + * + * `ws` accepts `{ port }` alone and silently binds the wildcard address, so a + * server the caller then dials on 127.0.0.1 sits at a port a foreign loopback + * listener can also hold -- and the more specific listener wins the connection, + * answering in that server's place. + * + * Anything unreadable is reported as `opaque` rather than skipped. A matcher + * that silently exempts the shapes it fails to parse is worse than no matcher, + * because it reads as coverage. + */ + +export type BindSite = { path: string; line: number } +export type OpaqueSite = BindSite & { reason: string } + +export type WebSocketServerBindScan = { + filesScanned: number + /** Every construction recognized, however it was then classified. */ + constructions: number + /** Binds a port with no `host`: reachable at an address the dialer never named. */ + wildcardBound: BindSite[] + /** Shape that could not be read; never treated as safe. */ + opaque: OpaqueSite[] + /** Binds a port and pins `host`. */ + loopbackBound: BindSite[] + /** No `port`: attaches to a server that owns the bind itself. */ + attached: BindSite[] +} + +const IGNORED_DIRECTORIES = new Set([ + 'node_modules', + 'dist', + 'out', + 'build', + '.git', + '__fixtures__', + 'coverage', + // Full snapshots of older releases; their bind sites are not this tree's to fix. + '.cross-version-checkouts' +]) +const SCANNED_EXTENSIONS = /\.(?:ts|tsx|mts|cts)$/ +const SCANNED_ROOTS = ['src', 'mobile', 'config', 'tests'] +const WS_IMPORT_HINT = /from\s*['"]ws['"]/ + +function collectSourceFiles(root: string, found: string[] = []): string[] { + let entries: ReturnType> + try { + entries = readdirSync(root, { withFileTypes: true }) + } catch { + return found + } + for (const entry of entries) { + if (IGNORED_DIRECTORIES.has(entry.name)) { + continue + } + const full = join(root, entry.name) + if (entry.isDirectory()) { + collectSourceFiles(full, found) + } else if (SCANNED_EXTENSIONS.test(entry.name)) { + found.push(full) + } + } + return found +} + +/** Local names bound to ws's server class, following `as` aliases and namespace imports. */ +function webSocketServerNames(text: string): { direct: Set; namespaces: Set } { + const direct = new Set() + const namespaces = new Set() + // One statement at a time: a pattern reaching for `from 'ws'` would swallow + // every import above it and lose the specifier names in the blob. + for (const match of text.matchAll(/\bimport\b([\s\S]*?)\bfrom\s*(['"])([^'"]+)\2/g)) { + if (match[3] !== 'ws') { + continue + } + const clause = match[1] + if (/^\s*type\b/.test(clause)) { + continue + } + const namespace = clause.match(/\*\s+as\s+([A-Za-z_$][\w$]*)/) + if (namespace) { + namespaces.add(namespace[1]) + } + const named = clause.match(/\{([\s\S]*)\}/) + if (!named) { + continue + } + for (const specifier of named[1].split(',')) { + const trimmed = specifier.trim() + if (!trimmed || /^type\s/.test(trimmed)) { + continue + } + const parts = trimmed.split(/\s+as\s+/) + // `Server` is ws's own alias for WebSocketServer. + if (parts[0].trim() === 'WebSocketServer' || parts[0].trim() === 'Server') { + direct.add((parts[1] ?? parts[0]).trim()) + } + } + } + return { direct, namespaces } +} + +function classify( + scan: WebSocketServerBindScan, + site: BindSite, + text: string, + paren: number +): void { + const options = readCallOptionKeys(text, paren) + if (!options.readable) { + scan.opaque.push({ ...site, reason: options.reason }) + return + } + if (!options.keys.includes('port')) { + scan.attached.push(site) + return + } + if (!options.keys.includes('host')) { + scan.wildcardBound.push(site) + return + } + scan.loopbackBound.push(site) +} + +export function scanWebSocketServerBinds(repoRoot: string): WebSocketServerBindScan { + const files = SCANNED_ROOTS.flatMap((directory) => collectSourceFiles(join(repoRoot, directory))) + const scan: WebSocketServerBindScan = { + filesScanned: files.length, + constructions: 0, + wildcardBound: [], + opaque: [], + loopbackBound: [], + attached: [] + } + for (const file of files) { + const text = readFileSync(file, 'utf8') + // Filter on the import, not on the class name: `Server as Wss` never spells + // WebSocketServer, and keying on that name silently skipped the whole alias. + if (!WS_IMPORT_HINT.test(text)) { + continue + } + const { direct, namespaces } = webSocketServerNames(text) + if (!direct.size && !namespaces.size) { + continue + } + const path = relative(repoRoot, file).split('\\').join('/') + const patterns = [ + ...[...direct].map((name) => new RegExp(`\\bnew\\s+${name}\\s*\\(`, 'g')), + ...[...namespaces].map( + (name) => new RegExp(`\\bnew\\s+${name}\\.(?:WebSocketServer|Server)\\s*\\(`, 'g') + ) + ] + for (const pattern of patterns) { + for (const match of text.matchAll(pattern)) { + scan.constructions++ + const line = text.slice(0, match.index).split('\n').length + classify(scan, { path, line }, text, match.index + match[0].length - 1) + } + } + } + return scan +} + +export function formatSites(sites: readonly BindSite[]): string[] { + return sites.map((site) => `${site.path}:${site.line}`) +} diff --git a/config/scripts/websocket-server-loopback-bind.test.ts b/config/scripts/websocket-server-loopback-bind.test.ts new file mode 100644 index 00000000000..9f32f8eda1a --- /dev/null +++ b/config/scripts/websocket-server-loopback-bind.test.ts @@ -0,0 +1,106 @@ +import { readFileSync } from 'node:fs' +import { join, resolve } from 'node:path' +import { describe, expect, it } from 'vitest' +import { formatSites, scanWebSocketServerBinds } from './websocket-server-bind-scan' + +/** + * Hold the bind address at the tree level rather than per call site. + * + * Every one of the ~30 `.listen(0, ...)` calls in this repo already passes + * '127.0.0.1'; 7 of 7 `new WebSocketServer({ port })` calls did not. Authors know + * the convention -- `ws` just never asks, because `{ port }` alone binds the + * wildcard without a word. That silence is what this test replaces. + * + * The allowlist only shrinks. A new wildcard bind fails here even where it looks + * harmless today, because harmless-looking is exactly what the seven were. + */ +/** The ratchet, held as data so it reads as the list it is. */ +const WILDCARD_BIND_ALLOWLIST: readonly string[] = readFileSync( + join(__dirname, '__fixtures__', 'websocket-server-wildcard-bind-allowlist.txt'), + 'utf8' +) + .split('\n') + .map((line) => line.trim()) + .filter((line) => line.length > 0 && !line.startsWith('#')) + +/** + * The true count of constructions that bind a port without pinning a host. + * + * May only ever be DECREASED, and only by pinning a host. Raising it is never + * the fix. + */ +const WILDCARD_BIND_PIN = 1 + +/** + * A floor under the constructions the scanner still recognizes. + * + * This is the guard against the scanner going blind: an import pattern it stops + * following reports zero offenders and reads exactly like a clean tree. During + * development a single wrong regex dropped this from 24 to 3. + */ +const RECOGNIZED_CONSTRUCTION_FLOOR = 20 + +describe('WebSocketServer loopback bind boundary', () => { + const repoRoot = resolve(__dirname, '..', '..') + const scan = scanWebSocketServerBinds(repoRoot) + const offenders = scan.wildcardBound.map((site) => site.path) + + it('scans a plausible number of files', () => { + // A broken root or extension list would make the guard silently vacuous. + expect(scan.filesScanned).toBeGreaterThan(5_000) + }) + + it('still recognizes the known construction sites', () => { + expect( + scan.constructions, + `Only ${scan.constructions} WebSocketServer constructions were recognized; the floor is ` + + `${RECOGNIZED_CONSTRUCTION_FLOOR}. The scanner has probably stopped following an import ` + + 'shape rather than the tree having lost that many servers.' + ).toBeGreaterThanOrEqual(RECOGNIZED_CONSTRUCTION_FLOOR) + }) + + it('can read the options of every construction it found', () => { + // An unreadable shape is never assumed safe: it could be hiding a host, or + // hiding the absence of one. Rewrite it as a plain object literal. + expect( + scan.opaque.map((site) => `${site.path}:${site.line} -- ${site.reason}`), + 'WebSocketServer options that this guard cannot read.' + ).toEqual([]) + }) + + it('has no wildcard-bound server outside the allowlist', () => { + const unlisted = scan.wildcardBound.filter( + (site) => !WILDCARD_BIND_ALLOWLIST.includes(site.path) + ) + expect( + formatSites(unlisted), + "New WebSocketServer that binds a port without a host. Pass host: '127.0.0.1' so a foreign " + + 'loopback listener cannot claim the port and answer in its place.' + ).toEqual([]) + }) + + it('has no stale allowlist entry', () => { + // Why this direction matters too: an entry left behind after the file was + // fixed hides the next regression in that same path. + const stale = WILDCARD_BIND_ALLOWLIST.filter((path) => !offenders.includes(path)) + expect(stale, 'Allowlist entry no longer binds the wildcard — delete the line.').toEqual([]) + }) + + it('holds the wildcard-bind count at the pin', () => { + // Bounding by the allowlist's own length would prove nothing: the two move + // together, so appending a line to silence a failure would keep the bound + // satisfied. The pin is a literal so that widening takes a second edit. + expect( + scan.wildcardBound.length, + `${scan.wildcardBound.length} constructions bind the wildcard; the pin is ` + + `${WILDCARD_BIND_PIN}. Never raise the pin -- pass host: '127.0.0.1' instead.` + ).toBeLessThanOrEqual(WILDCARD_BIND_PIN) + // A pin left above reality is how a ratchet rots: it re-opens room for the + // next wildcard bind to land for free. + expect( + scan.wildcardBound.length, + `Only ${scan.wildcardBound.length} constructions bind the wildcard. Lower ` + + `WILDCARD_BIND_PIN to ${scan.wildcardBound.length} to keep the ground you just took.` + ).toBeGreaterThanOrEqual(WILDCARD_BIND_PIN) + }) +}) diff --git a/mobile/src/transport/rpc-client-live-recovery.test.ts b/mobile/src/transport/rpc-client-live-recovery.test.ts index 471a53be740..bef276f918d 100644 --- a/mobile/src/transport/rpc-client-live-recovery.test.ts +++ b/mobile/src/transport/rpc-client-live-recovery.test.ts @@ -59,7 +59,8 @@ function e2eeDecrypt(encrypted: string, sharedKey: Uint8Array): string | null { // fail with EADDRINUSE; the full scenario restarts on the captured port // because the client keeps reconnecting to its original URL. function startServer(port = 0): Promise { - const wss = new WebSocketServer({ port }) + // host must match the 127.0.0.1 clients dial: a wildcard bind lets a foreign loopback listener claim the port and answer here. + const wss = new WebSocketServer({ host: '127.0.0.1', port }) wss.on('connection', (ws: ServerSocket) => { let sharedKey: Uint8Array | null = null let authenticated = false diff --git a/src/main/runtime/relay/mobile-relay-e2ee.integration.test.ts b/src/main/runtime/relay/mobile-relay-e2ee.integration.test.ts index 1f2b50e0648..bf9ec2ce431 100644 --- a/src/main/runtime/relay/mobile-relay-e2ee.integration.test.ts +++ b/src/main/runtime/relay/mobile-relay-e2ee.integration.test.ts @@ -61,7 +61,8 @@ describe('desktop relay E2EE integration', () => { }) it('splices a simulated phone through CloudRelayTransport with real NaCl E2EE v2', async () => { - const relay = new WebSocketServer({ port: 0, perMessageDeflate: false }) + // host must match the 127.0.0.1 clients dial: a wildcard bind lets a foreign loopback listener claim the port and answer here. + const relay = new WebSocketServer({ host: '127.0.0.1', port: 0, perMessageDeflate: false }) servers.push(relay) await new Promise((resolve) => relay.once('listening', resolve)) const address = relay.address() diff --git a/src/main/runtime/relay/relay-control-client.test.ts b/src/main/runtime/relay/relay-control-client.test.ts index a1ad5c03482..2975b67e631 100644 --- a/src/main/runtime/relay/relay-control-client.test.ts +++ b/src/main/runtime/relay/relay-control-client.test.ts @@ -99,7 +99,8 @@ describe('RelayControlClient', () => { }) it('rejects a control handshake that never receives a proof response', async () => { - const server = new WebSocketServer({ port: 0, perMessageDeflate: false }) + // host must match the 127.0.0.1 clients dial: a wildcard bind lets a foreign loopback listener claim the port and answer here. + const server = new WebSocketServer({ host: '127.0.0.1', port: 0, perMessageDeflate: false }) servers.push(server) await new Promise((resolve) => server.once('listening', resolve)) const address = server.address() @@ -129,7 +130,7 @@ describe('RelayControlClient', () => { }) it('settles an opening control immediately when ownership closes', async () => { - const server = new WebSocketServer({ port: 0, perMessageDeflate: false }) + const server = new WebSocketServer({ host: '127.0.0.1', port: 0, perMessageDeflate: false }) servers.push(server) await new Promise((resolve) => server.once('listening', resolve)) const address = server.address() @@ -165,7 +166,7 @@ describe('RelayControlClient', () => { }) it('proves the host key and drives control/data commands without URL credentials', async () => { - const server = new WebSocketServer({ port: 0, perMessageDeflate: false }) + const server = new WebSocketServer({ host: '127.0.0.1', port: 0, perMessageDeflate: false }) servers.push(server) await new Promise((resolve) => server.once('listening', resolve)) const address = server.address() diff --git a/src/main/runtime/rpc/relay-transport.test.ts b/src/main/runtime/rpc/relay-transport.test.ts index 8b7303ed805..21b520c8c9e 100644 --- a/src/main/runtime/rpc/relay-transport.test.ts +++ b/src/main/runtime/rpc/relay-transport.test.ts @@ -28,7 +28,8 @@ describe('CloudRelayTransport', () => { }) it('authenticates one query-free host-data socket and forwards messages verbatim', async () => { - const server = new WebSocketServer({ port: 0, perMessageDeflate: false }) + // host must match the 127.0.0.1 clients dial: a wildcard bind lets a foreign loopback listener claim the port and answer here. + const server = new WebSocketServer({ host: '127.0.0.1', port: 0, perMessageDeflate: false }) servers.push(server) await new Promise((resolve) => server.once('listening', resolve)) const address = server.address() diff --git a/src/renderer/src/web/web-runtime-client.test.ts b/src/renderer/src/web/web-runtime-client.test.ts index 1aa36f06d0f..7373ede6b8c 100644 --- a/src/renderer/src/web/web-runtime-client.test.ts +++ b/src/renderer/src/web/web-runtime-client.test.ts @@ -658,7 +658,8 @@ describe('WebRuntimeClient', () => { vi.stubGlobal('WebSocket', WebSocket) const serverKeys = generateKeyPair() const frame = new Uint8Array([9, 8, 7]) - const wss = new WebSocketServer({ port: 0 }) + // host must match the 127.0.0.1 clients dial: a wildcard bind lets a foreign loopback listener claim the port and answer here. + const wss = new WebSocketServer({ host: '127.0.0.1', port: 0 }) const sockets = new Set() wss.on('connection', (socket) => { sockets.add(socket) diff --git a/src/shared/remote-runtime-client.test.ts b/src/shared/remote-runtime-client.test.ts index 3e3bba921ea..d7a76417e84 100644 --- a/src/shared/remote-runtime-client.test.ts +++ b/src/shared/remote-runtime-client.test.ts @@ -539,7 +539,12 @@ async function createSubscriptionServer( const nextAuth = new Promise((resolve) => { resolveAuth = resolve }) - const wss = new WebSocketServer({ port: 0, autoPong: options.disableAutoPong !== true }) + // host must match the 127.0.0.1 clients dial: a wildcard bind lets a foreign loopback listener claim the port and answer here. + const wss = new WebSocketServer({ + host: '127.0.0.1', + port: 0, + autoPong: options.disableAutoPong !== true + }) servers.push(wss) wss.on('connection', (ws) => { @@ -625,7 +630,7 @@ async function createClosingServer( reason: string ): Promise<{ pairing: PairingOffer }> { const serverKeyPair = generateKeyPair() - const wss = new WebSocketServer({ port: 0 }) + const wss = new WebSocketServer({ host: '127.0.0.1', port: 0 }) servers.push(wss) wss.on('connection', (ws) => { ws.close(code, reason) @@ -649,7 +654,7 @@ async function createClosingServer( async function createInvalidHandshakeServer(): Promise<{ pairing: PairingOffer }> { const serverKeyPair = generateKeyPair() - const wss = new WebSocketServer({ port: 0 }) + const wss = new WebSocketServer({ host: '127.0.0.1', port: 0 }) servers.push(wss) wss.on('connection', (ws) => { ws.once('message', () => ws.send(JSON.stringify({ type: 'not_orca' }))) @@ -680,7 +685,7 @@ async function createOneShotServer( } = {} ): Promise<{ pairing: PairingOffer }> { const serverKeyPair = generateKeyPair() - const wss = new WebSocketServer({ port: 0 }) + const wss = new WebSocketServer({ host: '127.0.0.1', port: 0 }) servers.push(wss) wss.on('connection', (ws) => { diff --git a/src/shared/remote-runtime-outbound-admission.test.ts b/src/shared/remote-runtime-outbound-admission.test.ts index f536549ca46..eb5f902a16a 100644 --- a/src/shared/remote-runtime-outbound-admission.test.ts +++ b/src/shared/remote-runtime-outbound-admission.test.ts @@ -352,7 +352,8 @@ describe('remote runtime outbound admission', () => { async function createServer(): Promise<{ pairing: PairingOffer; server: WebSocketServer }> { const keyPair = generateKeyPair() - const server = new WebSocketServer({ port: 0 }) + // host must match the 127.0.0.1 clients dial: a wildcard bind lets a foreign loopback listener claim the port and answer here. + const server = new WebSocketServer({ host: '127.0.0.1', port: 0 }) servers.push(server) await new Promise((resolve) => server.once('listening', resolve)) const address = server.address() as AddressInfo diff --git a/src/shared/remote-runtime-request-connection.test.ts b/src/shared/remote-runtime-request-connection.test.ts index 4d812322ba0..eb8f0b06e89 100644 --- a/src/shared/remote-runtime-request-connection.test.ts +++ b/src/shared/remote-runtime-request-connection.test.ts @@ -98,7 +98,8 @@ async function createServer(): Promise { const requests: unknown[] = [] const auths: unknown[] = [] let connectionCount = 0 - const wss = new WebSocketServer({ port: 0 }) + // host must match the 127.0.0.1 clients dial: a wildcard bind lets a foreign loopback listener claim the port and answer here. + const wss = new WebSocketServer({ host: '127.0.0.1', port: 0 }) servers.push(wss) wss.on('connection', (ws) => { diff --git a/src/shared/remote-runtime-shared-control-test-server.ts b/src/shared/remote-runtime-shared-control-test-server.ts index 33b535c8405..0e4adb1a69c 100644 --- a/src/shared/remote-runtime-shared-control-test-server.ts +++ b/src/shared/remote-runtime-shared-control-test-server.ts @@ -60,7 +60,12 @@ export async function createSharedControlTestServer( const delayedResponses: (() => void)[] = [] let connectionCount = 0 let closedAfterFirstStreamingResponse = false - const wss = new WebSocketServer({ port: 0, autoPong: options.disableAutoPong !== true }) + // host must match the 127.0.0.1 clients dial: a wildcard bind lets a foreign loopback listener claim the port and answer here. + const wss = new WebSocketServer({ + host: '127.0.0.1', + port: 0, + autoPong: options.disableAutoPong !== true + }) servers.push(wss) wss.on('connection', (ws) => { diff --git a/src/shared/remote-runtime-subscription-request.test.ts b/src/shared/remote-runtime-subscription-request.test.ts index dabc51e6a24..14f904308cc 100644 --- a/src/shared/remote-runtime-subscription-request.test.ts +++ b/src/shared/remote-runtime-subscription-request.test.ts @@ -262,7 +262,8 @@ async function createServer(options: ServerOptions = {}): Promise<{ const nextRequest = new Promise((resolve) => { resolveRequest = resolve }) - const wss = new WebSocketServer({ port: 0 }) + // host must match the 127.0.0.1 clients dial: a wildcard bind lets a foreign loopback listener claim the port and answer here. + const wss = new WebSocketServer({ host: '127.0.0.1', port: 0 }) servers.push(wss) wss.on('connection', (ws) => { let sharedKey: Uint8Array | null = null From d7123591cebd103658c6d5c8f601eebe1dc0cb3e Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 1 Sep 2026 19:06:44 -0700 Subject: [PATCH 2/3] perf(git): pack the loose refs Orca's own fetches leave behind (#17857) * perf(git): pack the loose refs Orca's own fetches leave behind Orca strips git's auto-maintenance off every fetch it issues (GIT_FETCH_SKIP_AUTO_MAINTENANCE_CONFIG_ARGS) and never compensated, so nothing in an Orca-driven checkout ever packs refs. One real machine reached 36,574 loose refs, where `git show-ref -- main` costs 5.2s and every worktree create pays for it. Add an idle-time, per-repo `git pack-refs --all --prune`, armed by the fetches that create the debt. It runs only after ten minutes of quiet on that repo, only above 1000 loose refs (probed with a walk bounded by that threshold, not by the backlog), one at a time across the whole app, at the background admission tier, and never while an agent is working, a create is prepared or in flight, a worktree removal is deleting refs, the app is quitting, or the machine is on battery. A user who set `maintenance.auto=false` or `gc.auto=0` has opted out. Measured on a 36,001-loose-ref fixture (macOS/APFS, git 2.44): `show-ref` 5.5-12.2s -> 30-49ms, `for-each-ref` 4.0-10.8s -> 43-48ms. Also fixes a pre-existing bug the split exposed: `--path-format=absolute` is ignored before git 2.31, and taking rev-parse's stdout raw collapsed every repo on such a host onto one fetch-serialization key. Refs #17828 * perf(git): make idle ref maintenance preemptible and cheaper to probe The idle veto was one-directional: it stopped a pack from starting during a create, removal, or agent work, but nothing stopped those from starting during a pack. A user-clicked Fetch, a branch delete, or a worktree removal that needed `packed-refs.lock` mid-rewrite could fail with `unable to create packed-refs.lock` -- a git error with no visible cause. Make the pack cancellable end to end. An AbortSignal now reaches the `pack-refs` child and both pre-pack probes, and `pause()` aborts what is running, waits for it to actually stop, and holds a suspension count so nothing new starts until the caller releases. Every entry point that deletes a ref takes that pause: gitFetch, gitPull, gitFastForward, removeWorktree, forceDeleteLocalBranch, prepareWorktreeCreateCheckout, addWorktree. Five more triggers close the rest of the window: battery drop, window focus, quit, the attempt deadline, and any other git command queueing for an admission slot. Judge a pack by re-probing the backlog rather than by the child's exit code. Measured in the field: another Orca session moved a branch mid-pack, git reported `cannot lock ref`, skipped that ref and packed the rest -- 36,688 loose refs down to 3. On a machine running several sessions that is the normal case, and retrying it would be wrong. Probe with one batched `readdir` per directory instead of streaming `opendir`, which issues a thread-pool round trip every 32 entries: 177ms -> 23ms on a real 36,600-ref repository, with half the event-loop lag. The walk stays strictly sequential so it can never occupy more than one of libuv's four filesystem threads. `PackRefsLockOwnership` makes a lock left by SIGKILL attributable, and only reclaims one when a marker exists, the lock is older than any pack-refs could run for, and the recorded process is gone. Refs #17828 * fix(git): wait out the packed-refs lock instead of killing the pack Measured on Git 2.55/APFS with 37k loose refs: a full `pack-refs --all --prune` takes 23-32s but holds `packed-refs.lock` for only 0.03-1.37s of it. The other ~95% is the prune phase, during which a concurrent `fetch --prune`, `branch -D` or `update-ref` succeeds every time -- per-ref locks last microseconds and git retries for `core.filesRefLockTimeout`. So the abort-on-everything design was strictly harmful. SIGTERM into the prune loop strands an empty `refs/**/*.lock` about one time in five (9/30, 5/40, 6/30 kills): `tempfile.c` opens the lock O_EXCL before `activate_tempfile()` links it into the list the signal handler walks, and a pack does ~36k lock cycles. Afterwards `update-ref -d` on that ref fails with `cannot lock ref ... File exists`, permanently. On Windows `taskkill /f` never runs git's handlers at all, so an abort inside the rewrite strands `packed-refs.lock` every time. Never signal the child. `packRefs` no longer takes an abort signal; it polls `packed-refs.lock` and reports the window through a `PackedRefsLockReporter`. `pause()` resolves when the lock is released -- bounded, and free during the prune -- while the suspension counter still blocks new attempts. Battery and window-focus become do-not-start rather than stop-what-is-running, and quit waits for the lock and lets the child finish orphaned. For strands that already exist, `PackRefsLockOwnership` now also reclaims `refs/**/*.lock` under the same three conditions plus a 0-byte check, and a lock carrying our own not-yet-reclaimable marker records `locked` with a 30min retry instead of the 6h failure cooldown -- so a Windows strand self-heals in half an hour rather than six. Reverts the git admission-scheduler event bus, which existed only to drive the abort this removes. Refs #17828 * test(git): make the ref-maintenance waits survive a loaded runner CI shard 4/8 failed on `restarts every armed countdown when the user does ref work themselves`, which passes locally. The `until()` helper spun a fixed 200 event-loop turns and then returned silently, so on a contended runner the filesystem probe had not finished and the assertion that followed failed with an unrelated message. Bound the wait by wall clock instead and throw a named error, which immediately exposed a second latent bug: the single-flight test's second wait could never succeed, because the deferred repo's retry is on a faked `setTimeout` that spinning the real loop never advances. It had been passing only because the old helper gave up quietly. Add a timer-aware variant for those, and have the countdown test await a signal the fake pack resolves rather than polling at all. Verified stable across five sequential runs and once under load average 32 with six concurrent suites. Refs #17828 --- src/main/agent-awake-service.ts | 5 + src/main/cli/cli-command-installation.ts | 8 +- src/main/cli/cli-installer.ts | 8 +- src/main/cli/wsl-cli-installer.ts | 4 +- src/main/git/canonical-repo-key.test.ts | 78 +++ src/main/git/canonical-repo-key.ts | 72 +++ .../git/local-repo-ref-maintenance.test.ts | 182 ++++++ src/main/git/local-repo-ref-maintenance.ts | 272 +++++++++ src/main/git/pack-refs-lock-ownership.test.ts | 192 +++++++ src/main/git/pack-refs-lock-ownership.ts | 202 +++++++ src/main/git/remote.ts | 56 +- .../git/repo-ref-maintenance-real-git.test.ts | 290 ++++++++++ src/main/git/worktree-add.ts | 21 +- src/main/git/worktree-branch-removal.ts | 7 +- src/main/git/worktree-create-preparation.ts | 81 +-- src/main/git/worktree-removal.ts | 10 +- src/main/ipc/repos-create.test.ts | 5 +- src/main/ipc/worktrees.ts | 7 +- .../ipc/worktrees/worktree-ipc-context.ts | 15 + src/main/repo-maintenance-idle-gate.test.ts | 134 +++++ src/main/repo-maintenance-idle-gate.ts | 67 +++ src/main/runtime/fetch-remote-cache.test.ts | 8 +- .../runtime-remote-fetch-controller.ts | 56 +- ...ntime-remote-fetch-ref-maintenance.test.ts | 145 +++++ src/main/startup/main-process-observers.ts | 5 + src/main/startup/main-process-quit.ts | 18 + src/main/startup/main-process-state.ts | 2 + src/main/worktree-create-preparation.ts | 5 + src/shared/git-binary-compatibility.test.ts | 33 ++ src/shared/loose-ref-count.test.ts | 119 ++++ src/shared/loose-ref-count.ts | 80 +++ src/shared/packed-refs-lock-gate.ts | 43 ++ src/shared/repo-ref-maintenance-policy.ts | 166 ++++++ src/shared/repo-ref-maintenance.test.ts | 530 ++++++++++++++++++ src/shared/repo-ref-maintenance.ts | 366 ++++++++++++ 35 files changed, 3197 insertions(+), 95 deletions(-) create mode 100644 src/main/git/canonical-repo-key.test.ts create mode 100644 src/main/git/canonical-repo-key.ts create mode 100644 src/main/git/local-repo-ref-maintenance.test.ts create mode 100644 src/main/git/local-repo-ref-maintenance.ts create mode 100644 src/main/git/pack-refs-lock-ownership.test.ts create mode 100644 src/main/git/pack-refs-lock-ownership.ts create mode 100644 src/main/git/repo-ref-maintenance-real-git.test.ts create mode 100644 src/main/repo-maintenance-idle-gate.test.ts create mode 100644 src/main/repo-maintenance-idle-gate.ts create mode 100644 src/main/runtime/runtime-remote-fetch-ref-maintenance.test.ts create mode 100644 src/shared/loose-ref-count.test.ts create mode 100644 src/shared/loose-ref-count.ts create mode 100644 src/shared/packed-refs-lock-gate.ts create mode 100644 src/shared/repo-ref-maintenance-policy.ts create mode 100644 src/shared/repo-ref-maintenance.test.ts create mode 100644 src/shared/repo-ref-maintenance.ts diff --git a/src/main/agent-awake-service.ts b/src/main/agent-awake-service.ts index 29e866d27f2..b79612e2b9c 100644 --- a/src/main/agent-awake-service.ts +++ b/src/main/agent-awake-service.ts @@ -117,6 +117,11 @@ export class AgentAwakeService { } } + /** Agents this runtime has seen working recently, independent of the awake setting. */ + getWorkingAgentCount(): number { + return this.getEligibleRunningStatusCount() + } + subscribe(listener: (status: ComputerAwakeStatus) => void): () => void { this.statusListeners.add(listener) return () => this.statusListeners.delete(listener) diff --git a/src/main/cli/cli-command-installation.ts b/src/main/cli/cli-command-installation.ts index 5b277bd8c62..fbabc3bf6dd 100644 --- a/src/main/cli/cli-command-installation.ts +++ b/src/main/cli/cli-command-installation.ts @@ -35,7 +35,9 @@ export class CliCommandInstallation extends CliCommandInspection { const inspected = await this.inspectStableSymlink(commandPath, launcherPath) if (inspected.status.state === 'conflict') { - throw new Error(`Refusing to replace non-Orca command at ${commandPath}. Remove it and register again if it is no longer needed.`) + throw new Error( + `Refusing to replace non-Orca command at ${commandPath}. Remove it and register again if it is no longer needed.` + ) } if (inspected.status.state === 'installed') { return @@ -54,7 +56,9 @@ export class CliCommandInstallation extends CliCommandInspection { if (!(await capturedExpectedEntry(quarantine, inspected))) { await this.restoreQuarantinedCommand(quarantine, commandPath) - throw new Error(`Refusing to replace non-Orca command at ${commandPath}. Remove it and register again if it is no longer needed.`) + throw new Error( + `Refusing to replace non-Orca command at ${commandPath}. Remove it and register again if it is no longer needed.` + ) } try { diff --git a/src/main/cli/cli-installer.ts b/src/main/cli/cli-installer.ts index 3c832df5078..d95e1649ac0 100644 --- a/src/main/cli/cli-installer.ts +++ b/src/main/cli/cli-installer.ts @@ -116,7 +116,9 @@ export class CliInstaller extends CliPathRegistration { throw new Error(initialStatus.detail ?? 'CLI registration is unavailable on this build.') } if (initialStatus.state === 'conflict') { - throw new Error(`Refusing to replace non-Orca command at ${initialStatus.commandPath}. Remove it and register again if it is no longer needed.`) + throw new Error( + `Refusing to replace non-Orca command at ${initialStatus.commandPath}. Remove it and register again if it is no longer needed.` + ) } const extractedRoot = await this.ensureLinuxAppImagePayload() const status = extractedRoot @@ -126,7 +128,9 @@ export class CliInstaller extends CliPathRegistration { throw new Error(status.detail ?? 'CLI registration is unavailable on this build.') } if (status.state === 'conflict') { - throw new Error(`Refusing to replace non-Orca command at ${status.commandPath}. Remove it and register again if it is no longer needed.`) + throw new Error( + `Refusing to replace non-Orca command at ${status.commandPath}. Remove it and register again if it is no longer needed.` + ) } // eslint-disable-next-line unicorn/prefer-ternary -- Why: the install path performs async side effects and is easier to audit as an explicit branch than as an awaited ternary. diff --git a/src/main/cli/wsl-cli-installer.ts b/src/main/cli/wsl-cli-installer.ts index f8362fddc66..484ed4f9bc3 100644 --- a/src/main/cli/wsl-cli-installer.ts +++ b/src/main/cli/wsl-cli-installer.ts @@ -207,7 +207,9 @@ export class WslCliInstaller { throw new Error(status.detail ?? 'WSL CLI registration is unavailable.') } if (status.state === 'conflict') { - throw new Error(`Refusing to replace non-Orca command at ${status.commandPath}. Remove it and register again if it is no longer needed.`) + throw new Error( + `Refusing to replace non-Orca command at ${status.commandPath}. Remove it and register again if it is no longer needed.` + ) } await this.run( diff --git a/src/main/git/canonical-repo-key.test.ts b/src/main/git/canonical-repo-key.test.ts new file mode 100644 index 00000000000..87965bcaed6 --- /dev/null +++ b/src/main/git/canonical-repo-key.test.ts @@ -0,0 +1,78 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const gitExecFileAsyncMock = vi.hoisted(() => vi.fn()) + +vi.mock('./runner', async (importOriginal) => ({ + ...((await importOriginal()) as Record), + gitExecFileAsync: gitExecFileAsyncMock +})) + +import { + _resetCanonicalRepoKeyCacheForTests, + getCanonicalRepoKey, + readGitCommonDir +} from './canonical-repo-key' + +beforeEach(() => { + _resetCanonicalRepoKeyCacheForTests() + gitExecFileAsyncMock.mockReset() +}) + +afterEach(() => { + vi.restoreAllMocks() +}) + +describe('readGitCommonDir', () => { + it('reads the absolute answer modern Git gives', () => { + expect(readGitCommonDir('/repo/.git\n', '/repo/worktrees/a')).toBe('/repo/.git') + }) + + it('drops the flag Git older than 2.31 echoes back, and resolves the relative answer', () => { + // Without this every repository on such a host would answer `.git` and collide. + expect(readGitCommonDir('--path-format=absolute\n.git\n', '/repo')).toBe('/repo/.git') + }) + + it('resolves a WSL answer in Git execution space, not against the UNC path', () => { + expect(readGitCommonDir('.git\n', '//wsl$/Ubuntu/home/dev/repo')).toBe('/home/dev/repo/.git') + }) + + it('tolerates CRLF and blank lines', () => { + expect(readGitCommonDir('\r\n/repo/.git\r\n', '/repo')).toBe('/repo/.git') + }) + + it('returns undefined when Git printed nothing usable', () => { + expect(readGitCommonDir('\n', '/repo')).toBeUndefined() + }) +}) + +describe('getCanonicalRepoKey', () => { + it('gives every worktree of one repository the same key', async () => { + gitExecFileAsyncMock.mockResolvedValue({ stdout: '/repo/.git\n', stderr: '' }) + + await expect(getCanonicalRepoKey('/repo')).resolves.toBe('local::/repo/.git') + await expect(getCanonicalRepoKey('/repo/worktrees/a')).resolves.toBe('local::/repo/.git') + }) + + it('scopes the key to the execution host', async () => { + gitExecFileAsyncMock.mockResolvedValue({ stdout: '/home/dev/repo/.git\n', stderr: '' }) + + await expect( + getCanonicalRepoKey('//wsl$/Ubuntu/home/dev/repo', { wslDistro: 'Ubuntu' }) + ).resolves.toBe('wsl:Ubuntu::/home/dev/repo/.git') + }) + + it('caches so repeated arming costs no subprocess', async () => { + gitExecFileAsyncMock.mockResolvedValue({ stdout: '/repo/.git\n', stderr: '' }) + + await getCanonicalRepoKey('/repo') + await getCanonicalRepoKey('/repo') + + expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(1) + }) + + it('falls back to the caller path when Git cannot answer', async () => { + gitExecFileAsyncMock.mockRejectedValue(new Error('not a git repository')) + + await expect(getCanonicalRepoKey('/not-a-repo')).resolves.toBe('local::/not-a-repo') + }) +}) diff --git a/src/main/git/canonical-repo-key.ts b/src/main/git/canonical-repo-key.ts new file mode 100644 index 00000000000..1b06423bdc9 --- /dev/null +++ b/src/main/git/canonical-repo-key.ts @@ -0,0 +1,72 @@ +import { toWslExecutionSpace } from '../../shared/wsl-paths' +import { gitExecFileAsync } from './runner' +import { resolveRevParsePath } from './worktree-path-comparison' + +/** + * One repository on one execution host, named by its Git common dir. + * + * Shared by the fetch controller (which serializes fetches on it) and idle ref + * maintenance (which scopes all of its state to it), so both agree on what "the + * same repo" means across every worktree that points at it. + */ + +export type CanonicalRepoKeyOptions = { wslDistro?: string } + +const CACHE_MAX = 512 +const cache = new Map() + +/** + * Git < 2.31 ignores `--path-format=absolute`: it echoes the unrecognized flag, + * exits 0, and prints a relative `.git`. Taking the raw stdout there would give + * every repository on the host the same key. + */ +export function readGitCommonDir(stdout: string, repoPath: string): string | undefined { + const commonDir = stdout + .split('\n') + .map((line) => (line.endsWith('\r') ? line.slice(0, -1) : line)) + .findLast((line) => line.length > 0 && !line.startsWith('-')) + return commonDir ? resolveRevParsePath(toWslExecutionSpace(repoPath), commonDir) : undefined +} + +function remember(cacheKey: string, value: string): string { + cache.delete(cacheKey) + cache.set(cacheKey, value) + while (cache.size > CACHE_MAX) { + const oldest = cache.keys().next() + if (oldest.done) { + break + } + cache.delete(oldest.value) + } + return value +} + +/** `${runtimeKey}::${gitCommonDir}`, falling back to the caller's path. */ +export async function getCanonicalRepoKey( + repoPath: string, + options: CanonicalRepoKeyOptions = {} +): Promise { + const runtimeKey = options.wslDistro ? `wsl:${options.wslDistro}` : 'local' + const cacheKey = `${runtimeKey}::${repoPath}` + const cached = cache.get(cacheKey) + if (cached !== undefined) { + return remember(cacheKey, cached) + } + try { + const { stdout } = await gitExecFileAsync( + ['rev-parse', '--path-format=absolute', '--git-common-dir'], + { cwd: repoPath, ...options } + ) + const commonDir = readGitCommonDir(stdout, repoPath) + if (commonDir) { + return remember(cacheKey, `${runtimeKey}::${commonDir}`) + } + } catch { + // The caller path remains a safe serialization key when canonicalization fails. + } + return remember(cacheKey, cacheKey) +} + +export function _resetCanonicalRepoKeyCacheForTests(): void { + cache.clear() +} diff --git a/src/main/git/local-repo-ref-maintenance.test.ts b/src/main/git/local-repo-ref-maintenance.test.ts new file mode 100644 index 00000000000..f6a411733c3 --- /dev/null +++ b/src/main/git/local-repo-ref-maintenance.test.ts @@ -0,0 +1,182 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const gitExecFileAsyncMock = vi.hoisted(() => vi.fn()) +const readRepoCommonDirFromGitMock = vi.hoisted(() => vi.fn()) + +vi.mock('./runner', async (importOriginal) => ({ + ...((await importOriginal()) as Record), + gitExecFileAsync: gitExecFileAsyncMock +})) + +vi.mock('./worktree-list-reader', async (importOriginal) => ({ + ...((await importOriginal()) as Record), + readRepoCommonDirFromGit: readRepoCommonDirFromGitMock +})) + +import { _resetCanonicalRepoKeyCacheForTests } from './canonical-repo-key' +import { + _resetLocalRepoRefMaintenanceForTests, + armLocalRepoRefMaintenance, + createLocalRepoRefMaintenanceTarget, + getLocalRepoRefMaintenance, + setRepoMaintenanceActivityProbe, + withRepoRefMaintenancePaused +} from './local-repo-ref-maintenance' + +const NO_ABORT = new AbortController().signal + +function target(wslDistro?: string): ReturnType { + return createLocalRepoRefMaintenanceTarget({ + key: 'local::/repo/.git', + repoPath: wslDistro ? '//wsl$/Ubuntu/home/dev/repo' : '/repo', + ...(wslDistro ? { wslDistro } : {}) + }) +} + +beforeEach(() => { + gitExecFileAsyncMock.mockReset() + readRepoCommonDirFromGitMock.mockReset() + delete process.env.ORCA_DISABLE_REPO_REF_MAINTENANCE + _resetCanonicalRepoKeyCacheForTests() + _resetLocalRepoRefMaintenanceForTests() +}) + +afterEach(() => { + delete process.env.ORCA_DISABLE_REPO_REF_MAINTENANCE + _resetLocalRepoRefMaintenanceForTests() + vi.restoreAllMocks() +}) + +describe('local repo ref maintenance target', () => { + it('never hands the pack child an abort signal', async () => { + // Killing a `pack-refs` strands a `refs/**` lock about one time in five, and + // on Windows a force-kill inside the rewrite strands `packed-refs.lock` + // every time. The child must always be allowed to finish. + readRepoCommonDirFromGitMock.mockResolvedValue('/repo/.git') + gitExecFileAsyncMock.mockResolvedValue({ stdout: '', stderr: '' }) + + await target().packRefs({ setHeld: () => {} }) + + const packCall = gitExecFileAsyncMock.mock.calls.find( + ([argv]) => (argv as string[])[0] === 'pack-refs' + ) + expect(packCall?.[1]).not.toHaveProperty('signal') + }) + + it('runs pack-refs at the background tier with a long deadline', async () => { + gitExecFileAsyncMock.mockResolvedValue({ stdout: '', stderr: '' }) + + await target().packRefs({ setHeld: () => {} }) + + expect(gitExecFileAsyncMock).toHaveBeenCalledWith( + ['pack-refs', '--all', '--prune'], + expect.objectContaining({ cwd: '/repo', admissionTier: 'background', timeout: 15 * 60_000 }) + ) + }) + + it('reads either Git auto-maintenance opt-out, and unset keys as consent', async () => { + for (const stdout of [ + 'maintenance.auto false\n', + 'gc.auto 0\n', + 'gc.auto 6700\nmaintenance.auto false\n' + ]) { + gitExecFileAsyncMock.mockResolvedValue({ stdout, stderr: '' }) + await expect(target().isOptedOut?.(NO_ABORT)).resolves.toBe(true) + } + + gitExecFileAsyncMock.mockResolvedValue({ + stdout: 'maintenance.auto true\ngc.auto 6700\n', + stderr: '' + }) + await expect(target().isOptedOut?.(NO_ABORT)).resolves.toBe(false) + + // `git config --get-regexp` exits non-zero when nothing matches. + gitExecFileAsyncMock.mockRejectedValue(new Error('exit 1')) + await expect(target().isOptedOut?.(NO_ABORT)).resolves.toBe(false) + }) + + it('walks the POSIX refs directory for a native repo', async () => { + readRepoCommonDirFromGitMock.mockResolvedValue('/repo/.git') + + await expect(target().resolveRefsDirectory(NO_ABORT)).resolves.toBe('/repo/.git/refs') + }) + + it('translates a WSL repo answer back to the UNC path the main process can open', async () => { + // Git answers in its own execution space, which for WSL is a Linux path. + readRepoCommonDirFromGitMock.mockResolvedValue('/home/dev/repo/.git') + + await expect(target('Ubuntu').resolveRefsDirectory(NO_ABORT)).resolves.toBe( + '\\\\wsl.localhost\\Ubuntu\\home\\dev\\repo\\.git\\refs' + ) + }) + + it('reports an unresolvable repository rather than guessing a path', async () => { + readRepoCommonDirFromGitMock.mockResolvedValue(undefined) + + await expect(target().resolveRefsDirectory(NO_ABORT)).resolves.toBeUndefined() + }) +}) + +describe('local repo ref maintenance scheduling', () => { + it('schedules nothing when the kill switch is set', () => { + process.env.ORCA_DISABLE_REPO_REF_MAINTENANCE = '1' + const arm = vi.spyOn(getLocalRepoRefMaintenance(), 'arm') + + armLocalRepoRefMaintenance({ key: 'local::/repo/.git', repoPath: '/repo' }) + + expect(arm).not.toHaveBeenCalled() + }) + + it('arms through the shared single-flight instance otherwise', () => { + const arm = vi.spyOn(getLocalRepoRefMaintenance(), 'arm') + + armLocalRepoRefMaintenance({ key: 'local::/repo/.git', repoPath: '/repo' }) + + expect(arm).toHaveBeenCalledTimes(1) + }) + + it('is free when nothing has ever been armed', async () => { + // The common case by far: no timers, no instance, no reason to pay anything. + await expect(withRepoRefMaintenancePaused('git-fetch', async () => 'done')).resolves.toBe( + 'done' + ) + }) + + it('holds the window shut for the duration of ref-touching work', async () => { + readRepoCommonDirFromGitMock.mockResolvedValue('/repo/.git') + _resetLocalRepoRefMaintenanceForTests({ quietPeriodMs: 1, looseRefThreshold: 0 }) + setRepoMaintenanceActivityProbe(() => false) + const maintenance = getLocalRepoRefMaintenance() + const packRefs = vi.fn(async () => {}) + maintenance.arm({ + key: 'local::/repo/.git', + resolveRefsDirectory: async () => '/repo/.git/refs', + packRefs + }) + + await withRepoRefMaintenancePaused('branch-delete', async () => { + await new Promise((resolve) => setTimeout(resolve, 25)) + expect(packRefs).not.toHaveBeenCalled() + }) + + await vi.waitFor(() => expect(packRefs).toHaveBeenCalledTimes(1)) + }) + + it('routes the app activity probe into the shared instance', async () => { + readRepoCommonDirFromGitMock.mockResolvedValue('/repo/.git') + let busy = true + setRepoMaintenanceActivityProbe(() => busy) + const maintenance = getLocalRepoRefMaintenance() + const packRefs = vi.fn(async () => {}) + + maintenance.arm({ + key: 'local::/repo/.git', + resolveRefsDirectory: async () => '/repo/.git/refs', + packRefs + }) + await maintenance.whenAttemptSettled() + + expect(packRefs).not.toHaveBeenCalled() + busy = false + }) +}) diff --git a/src/main/git/local-repo-ref-maintenance.ts b/src/main/git/local-repo-ref-maintenance.ts new file mode 100644 index 00000000000..e84f7d1ae30 --- /dev/null +++ b/src/main/git/local-repo-ref-maintenance.ts @@ -0,0 +1,272 @@ +import { posix, win32 } from 'node:path' +import { isWindowsAbsolutePathLike } from '../../shared/cross-platform-path' +import { RepoRefMaintenance } from '../../shared/repo-ref-maintenance' +import { + PACK_REFS_ARGS, + PACK_REFS_TIMEOUT_MS, + RefMaintenanceRepoLocked, + type PackedRefsLockReporter, + type RepoRefMaintenanceOptions, + type RepoRefMaintenanceTarget +} from '../../shared/repo-ref-maintenance-policy' +import { isWslUncPath, toWindowsWslPath } from '../../shared/wsl-paths' +import { withSpan } from '../observability/tracer' +import { PackRefsLockOwnership } from './pack-refs-lock-ownership' +import { gitExecFileAsync } from './runner' +import { readRepoCommonDirFromGit } from './worktree-list-reader' + +/** + * Main-process wiring for idle loose-ref packing on the local execution host + * (native and WSL). + * + * SSH-hosted repos are deliberately out of scope: the execution host owns + * anything that touches execution, so maintaining them means running host-side + * on the relay, which today has neither admission control nor spans. Keying all + * state by execution host is what keeps this path from reaching across. + */ + +export type RepoMaintenanceActivityProbe = () => boolean + +const REPO_BUSY_PROBE_MAX = 64 + +let activityProbe: RepoMaintenanceActivityProbe | null = null +let shared: RepoRefMaintenance | null = null +// Why keyed here rather than captured in the target: a repo can be armed from +// the fetch controller or from a user-initiated fetch, and every arming must see +// the same "this repo has work in flight" answer, not whichever closure was last. +const repoBusyProbes = new Map boolean>() + +/** Register the owner of "this repo has a fetch in flight" for `key`. */ +export function setRepoRefMaintenanceBusyProbe(key: string, probe: () => boolean): void { + repoBusyProbes.delete(key) + repoBusyProbes.set(key, probe) + while (repoBusyProbes.size > REPO_BUSY_PROBE_MAX) { + const oldest = repoBusyProbes.keys().next() + if (oldest.done) { + break + } + repoBusyProbes.delete(oldest.value) + } +} + +/** + * Register the app-wide "do not start maintenance now" signal. Owned by the + * main entry point because the inputs (live agents, battery, quit) are not + * visible from the git layer. + */ +export function setRepoMaintenanceActivityProbe(probe: RepoMaintenanceActivityProbe | null): void { + activityProbe = probe +} + +/** Support escape hatch: kills the sweep without touching the user's git config. */ +function isDisabled(): boolean { + return process.env.ORCA_DISABLE_REPO_REF_MAINTENANCE === '1' +} + +function localMaintenanceOptions(): RepoRefMaintenanceOptions { + return { + // Fail closed: without the app-level gate installed we cannot see agents, + // creates, or battery, and running blind is worse than not running. + isBusy: () => activityProbe?.() ?? true, + observe: (attempt) => + withSpan('repo.ref_maintenance', (span) => attempt(span), { + attributes: { kind: 'git', 'repo.maintenance_host': 'local' } + }), + onError: (error) => { + console.warn('[repo-ref-maintenance] attempt failed:', error) + } + } +} + +export function getLocalRepoRefMaintenance(): RepoRefMaintenance { + shared ??= new RepoRefMaintenance(localMaintenanceOptions()) + return shared +} + +/** + * Cancels every armed timer and waits out any `packed-refs` rewrite in progress. + * + * Deliberately does not kill the child. A pack orphaned by the app quitting + * finishes on its own; a pack signalled mid-prune strands a ref lock about one + * time in five, and on Windows a force-kill inside the rewrite strands + * `packed-refs.lock` every time -- which blocks every later ref deletion. + */ +export function disposeLocalRepoRefMaintenance(): Promise { + const settling = shared?.awaitPackedRefsLockRelease() ?? Promise.resolve() + shared?.dispose() + shared = null + repoBusyProbes.clear() + return settling +} + +/** + * Hold every repository open while `run` touches refs. + * + * A ref deletion needs `packed-refs.lock`, which a running pack holds only while + * it rewrites the file -- 0.03-1.37s of a 23-32s run. Waiting that out turns the + * collision into a short pause. Cancelling the pack instead would strand a + * `refs/**` lock about one time in five, which Git never clears, so the ref + * stays undeletable indefinitely. + */ +export async function withRepoRefMaintenancePaused( + reason: string, + run: () => Promise +): Promise { + // Taken unconditionally rather than only when something is already armed: a + // fetch inside `run` can arm the sweep, and one counter bump against an idle + // instance costs a microtask. This can rebuild the instance after the + // quit-time dispose; harmless, because a fresh one has no armed timers and its + // activity probe is gone, so it fails closed. + const release = await getLocalRepoRefMaintenance().pause(reason) + try { + return await run() + } finally { + release() + } +} + +/** Wait out a `packed-refs` rewrite without holding the window open. For shutdown. */ +export function awaitPackedRefsLockRelease(): Promise { + return shared ? shared.awaitPackedRefsLockRelease() : Promise.resolve() +} + +/** + * Count user-initiated ref work as activity and restart every armed countdown. + * + * Deliberately not keyed to a repo: resolving one would cost a `rev-parse` on a + * path the user is waiting on, and a manual fetch or pull says the user is at + * the keyboard, which is a reason to defer every repository. + */ +export function postponeRepoRefMaintenance(): void { + shared?.postponeAll() +} + +/** `overrides` preseeds the shared instance so a test can shorten the quiet period. */ +export function _resetLocalRepoRefMaintenanceForTests( + overrides?: Partial +): void { + shared?.dispose() + shared = overrides ? new RepoRefMaintenance({ ...localMaintenanceOptions(), ...overrides }) : null + activityProbe = null + repoBusyProbes.clear() +} + +/** + * Git reports the common dir in its own execution space, so a WSL repo answers + * with a Linux path the Windows main process cannot open. Translate it back to + * the UNC spelling for the dirent walk; the walk reads directories, not files, + * so the handful of round trips stays cheap even over the share. + */ +function refsDirectoryForMainProcess(commonDir: string, wslDistro: string | undefined): string { + if (wslDistro && !isWslUncPath(commonDir) && !isWindowsAbsolutePathLike(commonDir)) { + return win32.join(toWindowsWslPath(commonDir, wslDistro), 'refs') + } + // Decided by path syntax, not by platform: `win32.isAbsolute` accepts POSIX paths too. + return (isWindowsAbsolutePathLike(commonDir) ? win32 : posix).join(commonDir, 'refs') +} + +/** + * `maintenance.auto=false` and `gc.auto=0` are the two knobs a user reaches for + * to tell Git to stop maintaining a repository on its own. Orca sets both on its + * own fetches, but only as per-invocation `-c` flags, so this probe sees the + * user's persisted config and never Orca's own suppression. + */ +export function isGitAutoMaintenanceDisabled(configOutput: string): boolean { + return configOutput + .split('\n') + .map((line) => line.trim()) + .some((line) => line === 'maintenance.auto false' || line === 'gc.auto 0') +} + +/** + * The common dir in the spelling the main process can open. + * + * Derived from the converted refs path, not the raw one: a WSL answer arrives as + * a Linux path but converts to a UNC path with no `/` in it, so choosing the + * path flavour before conversion collapses the whole thing to `.`. + */ +function gitCommonDirForMainProcess(commonDir: string, wslDistro: string | undefined): string { + const refs = refsDirectoryForMainProcess(commonDir, wslDistro) + return (isWindowsAbsolutePathLike(refs) ? win32 : posix).dirname(refs) +} + +export type LocalRepoRefMaintenanceTargetArgs = { + /** `${runtimeKey}::${gitCommonDir}` -- already scoped to the execution host. */ + readonly key: string + readonly repoPath: string + readonly wslDistro?: string +} + +/** + * Record a write to this repo and restart its quiet-period countdown. The only + * entry point callers need: the kill switch is honoured before anything is + * scheduled, so a disabled build arms no timers at all. + */ +export function armLocalRepoRefMaintenance(args: LocalRepoRefMaintenanceTargetArgs): void { + if (isDisabled()) { + return + } + getLocalRepoRefMaintenance().arm(createLocalRepoRefMaintenanceTarget(args)) +} + +export function createLocalRepoRefMaintenanceTarget( + args: LocalRepoRefMaintenanceTargetArgs +): RepoRefMaintenanceTarget { + const gitOptions = args.wslDistro ? { wslDistro: args.wslDistro } : {} + // The engine always probes before it packs, so the pack reuses this answer + // rather than spending a second rev-parse on the same repository. + let commonDir: string | undefined + const resolveCommonDir = async (signal?: AbortSignal): Promise => { + commonDir ??= await readRepoCommonDirFromGit(args.repoPath, { + ...gitOptions, + ...(signal ? { signal } : {}) + }) + return commonDir + } + return { + key: args.key, + isBusy: () => repoBusyProbes.get(args.key)?.() ?? false, + async resolveRefsDirectory(signal: AbortSignal) { + const resolved = await resolveCommonDir(signal) + return resolved ? refsDirectoryForMainProcess(resolved, args.wslDistro) : undefined + }, + async isOptedOut(signal: AbortSignal) { + try { + const { stdout } = await gitExecFileAsync( + ['config', '--get-regexp', '^(maintenance\\.auto|gc\\.auto)$'], + { cwd: args.repoPath, ...gitOptions, admissionTier: 'background', signal } + ) + return isGitAutoMaintenanceDisabled(stdout) + } catch { + // Neither key set is the common case and exits non-zero; that is consent. + return false + } + }, + async packRefs(lock: PackedRefsLockReporter) { + const resolved = await resolveCommonDir() + const owner = resolved + ? new PackRefsLockOwnership(gitCommonDirForMainProcess(resolved, args.wslDistro)) + : null + const claim = owner ? await owner.claim() : { ok: true as const } + if (!claim.ok) { + throw new RefMaintenanceRepoLocked(claim.reason) + } + // Report the rewrite window rather than accepting a signal. A pack that is + // killed mid-prune strands a `refs/**` lock about one time in five, and + // Git never clears those; waiting out the window costs at most ~1.4s. + const watch = owner?.watchLock((held) => lock.setHeld(held)) + try { + await gitExecFileAsync([...PACK_REFS_ARGS], { + cwd: args.repoPath, + ...gitOptions, + admissionTier: 'background', + timeout: PACK_REFS_TIMEOUT_MS + }) + } finally { + watch?.stop() + lock.setHeld(false) + await owner?.release() + } + } + } +} diff --git a/src/main/git/pack-refs-lock-ownership.test.ts b/src/main/git/pack-refs-lock-ownership.test.ts new file mode 100644 index 00000000000..e181feb9976 --- /dev/null +++ b/src/main/git/pack-refs-lock-ownership.test.ts @@ -0,0 +1,192 @@ +import { mkdir, mkdtemp, readFile, rm, stat, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' +import { PackRefsLockOwnership } from './pack-refs-lock-ownership' + +const roots: string[] = [] + +async function gitCommonDir(): Promise { + const root = await mkdtemp(join(tmpdir(), 'orca-pack-refs-lock-')) + roots.push(root) + return root +} + +function paths(commonDir: string): { lock: string; marker: string } { + return { + lock: join(commonDir, 'packed-refs.lock'), + marker: join(commonDir, 'packed-refs.orca-owner') + } +} + +async function exists(path: string): Promise { + try { + await stat(path) + return true + } catch { + return false + } +} + +/** A pid that cannot be running: the kernel rejects it outright. */ +const DEAD_PID = 0x7fffffff +const ABANDONED_LOCK_AGE_MS = 15 * 60_000 +const PID_REUSE_HORIZON_MS = 24 * 60 * 60_000 + +/** `claim` takes `now`, so age cases need no sleeping and no mtime forgery. */ +function laterBy(ms: number): number { + return Date.now() + ms +} + +afterEach(async () => { + await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))) +}) + +describe('packed-refs lock ownership', () => { + it('claims a repository with no lock and records the owner', async () => { + const commonDir = await gitCommonDir() + const { marker } = paths(commonDir) + + await expect(new PackRefsLockOwnership(commonDir).claim()).resolves.toEqual({ ok: true }) + + await expect(readFile(marker, 'utf-8')).resolves.toContain(String(process.pid)) + }) + + it('drops the owner marker on release', async () => { + const commonDir = await gitCommonDir() + const ownership = new PackRefsLockOwnership(commonDir) + await ownership.claim() + + await ownership.release() + + await expect(exists(paths(commonDir).marker)).resolves.toBe(false) + }) + + it('refuses a lock it cannot prove is its own', async () => { + const commonDir = await gitCommonDir() + // A lock with no marker belongs to the user's own git, or to another tool. + await writeFile(paths(commonDir).lock, 'someone else') + + await expect(new PackRefsLockOwnership(commonDir).claim()).resolves.toMatchObject({ ok: false }) + await expect(exists(paths(commonDir).lock)).resolves.toBe(true) + }) + + it('refuses a lock whose recorded owner is still running', async () => { + const commonDir = await gitCommonDir() + const { lock, marker } = paths(commonDir) + await writeFile(lock, 'in progress') + await writeFile(marker, JSON.stringify({ pid: process.pid })) + + await expect( + new PackRefsLockOwnership(commonDir).claim(laterBy(ABANDONED_LOCK_AGE_MS + 1)) + ).resolves.toMatchObject({ ok: false }) + await expect(exists(lock)).resolves.toBe(true) + }) + + it('reclaims the lock its own dead process left behind', async () => { + // SIGKILL and power loss bypass git's cleanup, and git never clears this itself. + const commonDir = await gitCommonDir() + const { lock, marker } = paths(commonDir) + await writeFile(lock, 'abandoned mid-rewrite') + await writeFile(marker, JSON.stringify({ pid: DEAD_PID })) + + const claimed = await new PackRefsLockOwnership(commonDir).claim( + laterBy(ABANDONED_LOCK_AGE_MS + 1) + ) + + expect(claimed).toEqual({ ok: true }) + await expect(exists(lock)).resolves.toBe(false) + await expect(readFile(marker, 'utf-8')).resolves.toContain(String(process.pid)) + }) + + it('leaves a young lock alone even when the marker names a dead process', async () => { + // A marker outlives its lock, so a foreign lock can appear after our death. + // Age is the only thing separating our wreckage from somebody's live lock. + const commonDir = await gitCommonDir() + const { lock, marker } = paths(commonDir) + await writeFile(marker, JSON.stringify({ pid: DEAD_PID })) + await writeFile(lock, 'a different git process, started just now') + + await expect(new PackRefsLockOwnership(commonDir).claim()).resolves.toMatchObject({ ok: false }) + await expect(exists(lock)).resolves.toBe(true) + }) + + it('does not wedge a repository forever when the recorded pid was recycled', async () => { + const commonDir = await gitCommonDir() + const { lock, marker } = paths(commonDir) + await writeFile(lock, 'abandoned mid-rewrite') + // Our own pid stands in for a recycled one: alive, but not the process that wrote this. + await writeFile(marker, JSON.stringify({ pid: process.pid })) + + await expect( + new PackRefsLockOwnership(commonDir).claim(laterBy(ABANDONED_LOCK_AGE_MS + 1)) + ).resolves.toMatchObject({ ok: false }) + + await expect( + new PackRefsLockOwnership(commonDir).claim(laterBy(PID_REUSE_HORIZON_MS + 1)) + ).resolves.toEqual({ ok: true }) + await expect(exists(lock)).resolves.toBe(false) + }) + + it('refuses a lock whose marker is unreadable rather than guessing', async () => { + const commonDir = await gitCommonDir() + const { lock, marker } = paths(commonDir) + await writeFile(lock, 'in progress') + await writeFile(marker, 'not json') + + await expect( + new PackRefsLockOwnership(commonDir).claim(laterBy(PID_REUSE_HORIZON_MS + 1)) + ).resolves.toMatchObject({ ok: false }) + await expect(exists(lock)).resolves.toBe(true) + }) + + it('claims cleanly when a marker outlived its lock', async () => { + const commonDir = await gitCommonDir() + await writeFile(paths(commonDir).marker, JSON.stringify({ pid: DEAD_PID })) + + await expect(new PackRefsLockOwnership(commonDir).claim()).resolves.toEqual({ ok: true }) + }) +}) + +describe('stranded per-ref locks', () => { + it('clears the empty refs/**/*.lock files its own dead process left behind', async () => { + // `tempfile.c` opens the lock O_EXCL before linking it into the list the + // signal handler walks, so a kill in that window leaves a 0-byte file that + // Git never clears -- and `update-ref -d` on that ref then fails forever. + const commonDir = await gitCommonDir() + const namespace = join(commonDir, 'refs', 'remotes', 'origin') + await mkdir(namespace, { recursive: true }) + await writeFile(join(namespace, 'main.lock'), '') + await writeFile(join(namespace, 'main'), 'a'.repeat(40)) + await writeFile(paths(commonDir).marker, JSON.stringify({ pid: DEAD_PID })) + + await new PackRefsLockOwnership(commonDir).claim(laterBy(ABANDONED_LOCK_AGE_MS + 1)) + + await expect(exists(join(namespace, 'main.lock'))).resolves.toBe(false) + // The ref itself is untouched. + await expect(exists(join(namespace, 'main'))).resolves.toBe(true) + }) + + it('leaves a non-empty ref lock alone, because a live writer is mid-write', async () => { + const commonDir = await gitCommonDir() + const namespace = join(commonDir, 'refs', 'heads') + await mkdir(namespace, { recursive: true }) + await writeFile(join(namespace, 'busy.lock'), 'b'.repeat(40)) + await writeFile(paths(commonDir).marker, JSON.stringify({ pid: DEAD_PID })) + + await new PackRefsLockOwnership(commonDir).claim(laterBy(ABANDONED_LOCK_AGE_MS + 1)) + + await expect(exists(join(namespace, 'busy.lock'))).resolves.toBe(true) + }) + + it('leaves ref locks alone when there is no marker naming a dead process', async () => { + const commonDir = await gitCommonDir() + const namespace = join(commonDir, 'refs', 'heads') + await mkdir(namespace, { recursive: true }) + await writeFile(join(namespace, 'other.lock'), '') + + await new PackRefsLockOwnership(commonDir).claim(laterBy(ABANDONED_LOCK_AGE_MS + 1)) + + await expect(exists(join(namespace, 'other.lock'))).resolves.toBe(true) + }) +}) diff --git a/src/main/git/pack-refs-lock-ownership.ts b/src/main/git/pack-refs-lock-ownership.ts new file mode 100644 index 00000000000..9e7273b143b --- /dev/null +++ b/src/main/git/pack-refs-lock-ownership.ts @@ -0,0 +1,202 @@ +import { readFile, readdir, rm, stat, writeFile } from 'node:fs/promises' +import { posix, win32 } from 'node:path' +import { isWindowsAbsolutePathLike } from '../../shared/cross-platform-path' +import { + PACK_REFS_TIMEOUT_MS, + PACKED_REFS_LOCK_POLL_MS +} from '../../shared/repo-ref-maintenance-policy' + +/** No legitimate `pack-refs` outlives its own deadline, so an older lock is abandoned. */ +const ABANDONED_LOCK_AGE_MS = PACK_REFS_TIMEOUT_MS + +/** Beyond this a recorded pid may have been recycled, so it stops being evidence of life. */ +const PID_REUSE_HORIZON_MS = 24 * 60 * 60_000 + +/** The ref tree is wide but shallow; this only stops a pathological walk. */ +const REF_LOCK_SCAN_CEILING = 4096 + +/** + * Makes a `packed-refs.lock` Orca left behind attributable, and only that one. + * + * Git registers signal handlers that clean the lock up, but SIGKILL and power + * loss bypass them, and Git never removes a stale `packed-refs.lock` on its own + * -- every later ref deletion in that repository fails until someone deletes a + * file they have never heard of. Recording our pid beside the lock lets a later + * run recognise its own wreckage. + * + * Three independent conditions must all hold before anything is unlinked, + * because deleting a lock somebody else is holding is far worse than declining + * to pack: a marker must exist at all, the lock must be older than any + * `pack-refs` could legitimately run for, and the recorded process must be gone. + * A marker can outlive its lock, so age is what separates "our wreckage" from a + * foreign lock that happened to appear afterwards. + */ +export class PackRefsLockOwnership { + private readonly lockPath: string + private readonly markerPath: string + + constructor(gitCommonDir: string) { + const path = isWindowsAbsolutePathLike(gitCommonDir) ? win32 : posix + this.lockPath = path.join(gitCommonDir, 'packed-refs.lock') + this.markerPath = path.join(gitCommonDir, 'packed-refs.orca-owner') + } + + /** Refused when the lock belongs to something we cannot prove is our own wreckage. */ + async claim(now = Date.now()): Promise { + const reclaim = await this.reclaimAbandonedLock(now) + if (!reclaim.ok) { + return reclaim + } + // Per-ref strands outlive their pack and are invisible to Git, which never + // clears a `refs/**\/*.lock` it did not create in this process. + await this.reclaimStrandedRefLocks(now) + try { + await writeFile(this.markerPath, JSON.stringify({ pid: process.pid }), 'utf-8') + } catch { + // Losing the marker only costs attribution on the next run, never correctness. + } + return { ok: true } + } + + /** + * Poll `packed-refs.lock` so the scheduler knows when the exclusive rewrite + * window opens and closes. Cheap: one `stat` on a fixed path. + */ + watchLock(report: (held: boolean) => void): { stop: () => void } { + let stopped = false + let last = false + const tick = async (): Promise => { + if (stopped) { + return + } + const held = (await fileAgeMs(this.lockPath, Date.now())) !== null + if (!stopped && held !== last) { + last = held + report(held) + } + } + const timer = setInterval(() => void tick(), PACKED_REFS_LOCK_POLL_MS) + timer.unref?.() + void tick() + return { + stop: () => { + stopped = true + clearInterval(timer) + } + } + } + + async release(): Promise { + await rm(this.markerPath, { force: true }).catch(() => {}) + } + + private async reclaimAbandonedLock(now: number): Promise { + const lockAgeMs = await fileAgeMs(this.lockPath, now) + if (lockAgeMs === null) { + return { ok: true } + } + // No marker means the lock is not ours to reason about, let alone remove. + const marker = await readOwnerMarker(this.markerPath) + if (marker === null) { + return { ok: false, reason: 'held by another process' } + } + if (lockAgeMs < ABANDONED_LOCK_AGE_MS) { + // Ours, but too young to be certain the writer is gone. Worth retrying soon. + return { ok: false, reason: 'our own lock, not yet old enough to reclaim' } + } + // Past the pid-reuse horizon the pid proves nothing, and a lock this old is + // abandoned whoever wrote it -- otherwise a recycled pid would wedge the + // repository permanently. + if (isProcessAlive(marker.pid) && lockAgeMs < PID_REUSE_HORIZON_MS) { + return { ok: false, reason: 'the recorded owner is still running' } + } + await rm(this.lockPath, { force: true }).catch(() => {}) + await rm(this.markerPath, { force: true }).catch(() => {}) + return { ok: true } + } + + /** + * Clear `refs/**\/*.lock` files a dead pack of ours left behind. + * + * `tempfile.c` opens the lock `O_EXCL` before `activate_tempfile()` links it + * into the list the signal handler walks, so a kill inside that window leaves + * a 0-byte file. Afterwards `update-ref -d` and any fetch touching that ref + * fail with `cannot lock ref ... File exists`, forever. Same three conditions + * as the packed-refs lock, plus a size check: a live writer's lock is not empty. + */ + private async reclaimStrandedRefLocks(now: number): Promise { + const marker = await readOwnerMarker(this.markerPath) + if (marker === null || isProcessAlive(marker.pid)) { + return + } + const markerAgeMs = await fileAgeMs(this.markerPath, now) + if (markerAgeMs === null || markerAgeMs < ABANDONED_LOCK_AGE_MS) { + return + } + const path = isWindowsAbsolutePathLike(this.markerPath) ? win32 : posix + const pending = [path.join(path.dirname(this.markerPath), 'refs')] + let visited = 0 + while (pending.length > 0) { + const directory = pending.pop() + if (directory === undefined || (visited += 1) > REF_LOCK_SCAN_CEILING) { + return + } + let entries: { name: string; isDirectory: () => boolean }[] + try { + entries = await readdir(directory, { withFileTypes: true }) + } catch { + continue + } + for (const entry of entries) { + const full = path.join(directory, entry.name) + if (entry.isDirectory()) { + pending.push(full) + } else if (entry.name.endsWith('.lock') && (await isEmptyFile(full))) { + await rm(full, { force: true }).catch(() => {}) + } + } + } + } +} + +export type PackRefsLockClaim = { ok: true } | { ok: false; reason: string } + +/** A strand from the `O_EXCL` window is 0 bytes; a live writer's lock is not. */ +async function isEmptyFile(path: string): Promise { + try { + return (await stat(path)).size === 0 + } catch { + return false + } +} + +async function readOwnerMarker(path: string): Promise<{ pid: number } | null> { + try { + const raw = (await readFile(path, 'utf-8')).slice(0, 256) + const pid = (JSON.parse(raw) as { pid?: unknown }).pid + return typeof pid === 'number' && Number.isInteger(pid) && pid > 0 ? { pid } : null + } catch { + return null + } +} + +/** Null when the file does not exist. Uses stat: the lock holds a whole packed-refs. */ +async function fileAgeMs(path: string, now: number): Promise { + try { + return Math.max(0, now - (await stat(path)).mtimeMs) + } catch (error) { + return (error as NodeJS.ErrnoException).code === 'ENOENT' ? null : 0 + } +} + +function isProcessAlive(pid: number): boolean { + if (pid === process.pid) { + return true + } + try { + process.kill(pid, 0) + return true + } catch (error) { + return (error as NodeJS.ErrnoException).code !== 'ESRCH' + } +} diff --git a/src/main/git/remote.ts b/src/main/git/remote.ts index c1557bdd806..2baf3b77137 100644 --- a/src/main/git/remote.ts +++ b/src/main/git/remote.ts @@ -7,6 +7,10 @@ import { gitRefTargetsBranchOnRemote } from '../../shared/git-remote-branch-name import type { GitPushTarget } from '../../shared/worktree/types' import type { GitRuntimeOptions } from './git-runtime-options' import { gitOptionsForWorktree } from './git-runtime-options' +import { + postponeRepoRefMaintenance, + withRepoRefMaintenancePaused +} from './local-repo-ref-maintenance' import { validateGitPushTarget } from './push-target-validation' import { gitExecFileAsync } from './runner' import { fetchForkRemoteWithStaleRefspecRepair } from './fork-remote-stale-branch-refspec' @@ -260,8 +264,11 @@ export async function gitPull( // Why: plain `git pull` uses the user's configured pull strategy (merge by // default) so diverged branches reconcile instead of erroring out. Conflicts // surface through the existing conflict-resolution flow. - await runWithGitWorktreeOperationLock(worktreePath, options.signal, () => - runWithGitReadCacheInvalidation(() => gitPullWithArgs(worktreePath, [], pushTarget, options)) + postponeRepoRefMaintenance() + await withRepoRefMaintenancePaused('git-pull', () => + runWithGitWorktreeOperationLock(worktreePath, options.signal, () => + runWithGitReadCacheInvalidation(() => gitPullWithArgs(worktreePath, [], pushTarget, options)) + ) ) } @@ -270,9 +277,12 @@ export async function gitFastForward( pushTarget?: GitPushTarget, options: GitRuntimeOptions = {} ): Promise { - await runWithGitWorktreeOperationLock(worktreePath, options.signal, () => - runWithGitReadCacheInvalidation(() => - gitPullWithArgs(worktreePath, ['--ff-only'], pushTarget, options) + postponeRepoRefMaintenance() + await withRepoRefMaintenancePaused('git-fast-forward', () => + runWithGitWorktreeOperationLock(worktreePath, options.signal, () => + runWithGitReadCacheInvalidation(() => + gitPullWithArgs(worktreePath, ['--ff-only'], pushTarget, options) + ) ) ) } @@ -282,22 +292,28 @@ export async function gitFetch( pushTarget?: GitPushTarget, options: GitRuntimeOptions = {} ): Promise { + // `--prune` deletes remote-tracking refs, which needs the `packed-refs` lock a + // running idle pack holds while it rewrites -- ~1.4s at most. This is the user + // clicking Fetch, so wait that window out rather than letting it fail on the lock. + postponeRepoRefMaintenance() try { - if (pushTarget) { - const target = await validateGitPushTarget(worktreePath, pushTarget, options) - const runtimeOptions = gitOptionsForWorktree(worktreePath, options) - await fetchForkRemoteWithStaleRefspecRepair( - (args, cwd) => gitExecFileAsync(args, { ...runtimeOptions, cwd }), - worktreePath, - target.remoteName, - () => - gitExecFileAsync(['fetch', '--prune', target.remoteName], runtimeOptions).then( - () => undefined - ) - ) - return - } - await gitExecFileAsync(['fetch', '--prune'], gitOptionsForWorktree(worktreePath, options)) + await withRepoRefMaintenancePaused('git-fetch', async () => { + if (pushTarget) { + const target = await validateGitPushTarget(worktreePath, pushTarget, options) + const runtimeOptions = gitOptionsForWorktree(worktreePath, options) + await fetchForkRemoteWithStaleRefspecRepair( + (args, cwd) => gitExecFileAsync(args, { ...runtimeOptions, cwd }), + worktreePath, + target.remoteName, + () => + gitExecFileAsync(['fetch', '--prune', target.remoteName], runtimeOptions).then( + () => undefined + ) + ) + return + } + await gitExecFileAsync(['fetch', '--prune'], gitOptionsForWorktree(worktreePath, options)) + }) } catch (error) { throw new Error(normalizeGitErrorMessage(error, 'fetch')) } diff --git a/src/main/git/repo-ref-maintenance-real-git.test.ts b/src/main/git/repo-ref-maintenance-real-git.test.ts new file mode 100644 index 00000000000..30c67b0365a --- /dev/null +++ b/src/main/git/repo-ref-maintenance-real-git.test.ts @@ -0,0 +1,290 @@ +import { execFileSync } from 'node:child_process' +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' +import { countLooseRefs } from '../../shared/loose-ref-count' +import { RepoRefMaintenance } from '../../shared/repo-ref-maintenance' +import { + _resetLocalRepoRefMaintenanceForTests, + createLocalRepoRefMaintenanceTarget, + getLocalRepoRefMaintenance, + setRepoMaintenanceActivityProbe +} from './local-repo-ref-maintenance' +import { forceDeleteLocalBranch } from './worktree-branch-removal' + +const roots: string[] = [] +// Large enough that the deferral ladder (1x, 2x, 4x ... capped at 8x) outlasts +// three real `pack-refs` runs before the deferral budget is spent. +const QUIET_MS = 25 +const THRESHOLD = 20 + +function git(cwd: string, args: string[]): string { + return execFileSync('git', args, { + cwd, + encoding: 'utf8', + stdio: ['pipe', 'pipe', 'pipe'] + }).trim() +} + +/** A repo whose only loose-ref backlog is the one the test asks for. */ +async function createRepo(looseRefs: number): Promise<{ repoPath: string; refsDir: string }> { + const root = await mkdtemp(join(tmpdir(), 'orca-ref-maintenance-git-')) + roots.push(root) + const repoPath = join(root, 'repo') + execFileSync('git', ['init', '--quiet', repoPath]) + git(repoPath, ['symbolic-ref', 'HEAD', 'refs/heads/main']) + git(repoPath, ['config', 'user.email', 'test@example.com']) + git(repoPath, ['config', 'user.name', 'Test User']) + await writeFile(join(repoPath, 'file.txt'), 'one\n') + git(repoPath, ['add', 'file.txt']) + git(repoPath, ['commit', '--quiet', '-m', 'initial']) + const head = git(repoPath, ['rev-parse', 'HEAD']) + // Written directly: `update-ref` for thousands of refs is the slow part of the fixture. + const namespace = join(repoPath, '.git', 'refs', 'remotes', 'origin') + await mkdir(namespace, { recursive: true }) + for (let index = 0; index < looseRefs; index += 1) { + await writeFile(join(namespace, `branch-${index}`), `${head}\n`) + } + return { repoPath, refsDir: join(repoPath, '.git', 'refs') } +} + +function createMaintenance(onPackRefs: () => void = () => {}): { + maintenance: RepoRefMaintenance + arm: (repoPath: string) => void +} { + const maintenance = new RepoRefMaintenance({ + quietPeriodMs: QUIET_MS, + looseRefThreshold: THRESHOLD + }) + return { + maintenance, + arm: (repoPath: string) => { + const target = createLocalRepoRefMaintenanceTarget({ + key: `local::${repoPath}`, + repoPath + }) + maintenance.arm({ + ...target, + packRefs: async (signal) => { + onPackRefs() + await target.packRefs(signal) + } + }) + } + } +} + +async function settle(maintenance: RepoRefMaintenance): Promise { + await new Promise((resolve) => setTimeout(resolve, QUIET_MS * 4)) + await maintenance.whenAttemptSettled() +} + +/** Deferred repos re-arm for another quiet period, so drain rather than count rounds. */ +async function settleUntil( + maintenance: RepoRefMaintenance, + done: () => Promise +): Promise { + for (let round = 0; round < 100; round += 1) { + if (await done()) { + return + } + await settle(maintenance) + } +} + +afterEach(async () => { + _resetLocalRepoRefMaintenanceForTests() + await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))) +}) + +describe('idle ref maintenance against real Git', () => { + it('packs a backlogged repository down to zero loose refs', async () => { + const { repoPath, refsDir } = await createRepo(THRESHOLD + 30) + const { maintenance, arm } = createMaintenance() + + await expect(countLooseRefs(refsDir, 10_000)).resolves.toMatchObject({ + count: THRESHOLD + 31 + }) + + arm(repoPath) + await settle(maintenance) + maintenance.dispose() + + await expect(countLooseRefs(refsDir, 10_000)).resolves.toEqual({ count: 0, saturated: false }) + // The refs survived the move into packed-refs; nothing was lost. + expect(git(repoPath, ['for-each-ref', '--format=%(refname)']).split('\n')).toHaveLength( + THRESHOLD + 31 + ) + expect(git(repoPath, ['rev-parse', '--verify', 'refs/remotes/origin/branch-0'])).toMatch( + /^[0-9a-f]{40}$/ + ) + }, 30_000) + + it('leaves a healthy repository untouched', async () => { + const { repoPath, refsDir } = await createRepo(2) + let packed = 0 + const { maintenance, arm } = createMaintenance(() => { + packed += 1 + }) + + arm(repoPath) + await settle(maintenance) + maintenance.dispose() + + expect(packed).toBe(0) + await expect(countLooseRefs(refsDir, 10_000)).resolves.toMatchObject({ count: 3 }) + }, 30_000) + + it('honours maintenance.auto=false in the repository config', async () => { + const { repoPath, refsDir } = await createRepo(THRESHOLD + 30) + git(repoPath, ['config', 'maintenance.auto', 'false']) + let packed = 0 + const { maintenance, arm } = createMaintenance(() => { + packed += 1 + }) + + arm(repoPath) + await settle(maintenance) + maintenance.dispose() + + expect(packed).toBe(0) + await expect(countLooseRefs(refsDir, 10_000)).resolves.toMatchObject({ + count: THRESHOLD + 31 + }) + }, 30_000) + + it('runs one repository at a time even when several go quiet together', async () => { + const repos = await Promise.all([ + createRepo(THRESHOLD + 5), + createRepo(THRESHOLD + 5), + createRepo(THRESHOLD + 5) + ]) + let concurrent = 0 + let peak = 0 + const maintenance = new RepoRefMaintenance({ + quietPeriodMs: QUIET_MS, + looseRefThreshold: THRESHOLD + }) + for (const { repoPath } of repos) { + const target = createLocalRepoRefMaintenanceTarget({ + key: `local::${repoPath}`, + repoPath + }) + maintenance.arm({ + ...target, + packRefs: async (signal) => { + concurrent += 1 + peak = Math.max(peak, concurrent) + try { + await target.packRefs(signal) + } finally { + concurrent -= 1 + } + } + }) + } + + const allPacked = async (): Promise => { + const counts = await Promise.all(repos.map(({ refsDir }) => countLooseRefs(refsDir, 10_000))) + return counts.every((scan) => scan.count === 0) + } + await settleUntil(maintenance, allPacked) + maintenance.dispose() + + expect(peak).toBe(1) + for (const { refsDir } of repos) { + await expect(countLooseRefs(refsDir, 10_000)).resolves.toEqual({ + count: 0, + saturated: false + }) + } + }, 60_000) +}) + +describe('yielding the repository to work that deletes refs', () => { + it('waits for the packed-refs lock and succeeds while the prune continues', async () => { + // The pack is never killed. `packed-refs.lock` is held for ~1.4s of a 30s + // run; the rest is the prune, during which a concurrent `update-ref -d` + // succeeds on its own because per-ref locks last microseconds. Signalling + // the child there strands a `refs/**` lock Git never clears. + const { repoPath } = await createRepo(0) + git(repoPath, ['branch', 'doomed']) + const head = git(repoPath, ['rev-parse', 'refs/heads/doomed']) + + let packing = false + let releaseLock: (() => void) | undefined + _resetLocalRepoRefMaintenanceForTests({ quietPeriodMs: QUIET_MS, looseRefThreshold: 1 }) + setRepoMaintenanceActivityProbe(() => false) + getLocalRepoRefMaintenance().arm({ + key: `local::${repoPath}`, + resolveRefsDirectory: async () => join(repoPath, '.git', 'refs'), + packRefs: async (lock) => { + packing = true + lock.setHeld(true) + // Stands in for the rewrite window, then the long prune that follows it. + await new Promise((resolve) => { + releaseLock = () => { + lock.setHeld(false) + resolve() + } + }) + } + }) + for (let attempt = 0; attempt < 200 && !packing; attempt += 1) { + await new Promise((resolve) => setTimeout(resolve, QUIET_MS)) + } + expect(packing).toBe(true) + + // The real deletion path, which routes through withRepoRefMaintenancePaused. + let deleted = false + const deletion = forceDeleteLocalBranch(repoPath, 'doomed', head).then(() => { + deleted = true + }) + + // It must still be waiting: the rewrite window is open. + await new Promise((resolve) => setTimeout(resolve, QUIET_MS * 4)) + expect(deleted).toBe(false) + expect(git(repoPath, ['branch', '--list', 'doomed'])).toContain('doomed') + + // Releasing the window is enough -- the pack is never cancelled. + releaseLock?.() + await deletion + expect(deleted).toBe(true) + expect(git(repoPath, ['branch', '--list', 'doomed'])).toBe('') + }, 30_000) + + it('does not block the caller once the rewrite window has closed', async () => { + // The prune phase is concurrency-safe, so a caller arriving during it pays + // nothing at all. + const { repoPath } = await createRepo(0) + git(repoPath, ['branch', 'doomed']) + const head = git(repoPath, ['rev-parse', 'refs/heads/doomed']) + + let pruning = false + let finishPrune: (() => void) | undefined + _resetLocalRepoRefMaintenanceForTests({ quietPeriodMs: QUIET_MS, looseRefThreshold: 1 }) + setRepoMaintenanceActivityProbe(() => false) + getLocalRepoRefMaintenance().arm({ + key: `local::${repoPath}`, + resolveRefsDirectory: async () => join(repoPath, '.git', 'refs'), + packRefs: async (lock) => { + lock.setHeld(true) + lock.setHeld(false) + pruning = true + await new Promise((resolve) => { + finishPrune = resolve + }) + } + }) + for (let attempt = 0; attempt < 200 && !pruning; attempt += 1) { + await new Promise((resolve) => setTimeout(resolve, QUIET_MS)) + } + + const startedAt = Date.now() + await expect(forceDeleteLocalBranch(repoPath, 'doomed', head)).resolves.toBeUndefined() + expect(Date.now() - startedAt).toBeLessThan(2_000) + + finishPrune?.() + }, 30_000) +}) diff --git a/src/main/git/worktree-add.ts b/src/main/git/worktree-add.ts index 3cd761f4d13..ea6ec704b46 100644 --- a/src/main/git/worktree-add.ts +++ b/src/main/git/worktree-add.ts @@ -4,6 +4,7 @@ import type { LocalBaseRefUpdateSuggestion } from '../../shared/worktree/base-ref-drift-types' import { windowsLongPathGitArgs } from '../../shared/windows-long-path-git-args' +import { withRepoRefMaintenancePaused } from './local-repo-ref-maintenance' import { gitExecFileAsync } from './runner' import { runWithGitReadCacheInvalidation } from './status' import { invalidateWslLinkedWorktreeGitRouting } from './wsl-linked-worktree-git-routing' @@ -149,15 +150,17 @@ export async function addWorktree( options: AddWorktreeOptions = {} ): Promise { try { - return await runWithGitReadCacheInvalidation(() => - performAddWorktree( - repoPath, - worktreePath, - branch, - baseBranch, - refreshLocalBaseRef, - noCheckout, - options + return await withRepoRefMaintenancePaused('worktree-add', () => + runWithGitReadCacheInvalidation(() => + performAddWorktree( + repoPath, + worktreePath, + branch, + baseBranch, + refreshLocalBaseRef, + noCheckout, + options + ) ) ) } finally { diff --git a/src/main/git/worktree-branch-removal.ts b/src/main/git/worktree-branch-removal.ts index 5e64b4a582d..08b6b21a1e3 100644 --- a/src/main/git/worktree-branch-removal.ts +++ b/src/main/git/worktree-branch-removal.ts @@ -4,6 +4,7 @@ import { } from '../../shared/git-branch-cleanup' import type { RemoveWorktreeResult } from '../../shared/worktree/create-types' import { withLocalGitCapabilityCacheForExecution } from './git-capability-state' +import { withRepoRefMaintenancePaused } from './local-repo-ref-maintenance' import { gitExecFileAsync } from './runner' import { parseWorktreeList } from './worktree-list-parser' import type { GitWorktreeExecOptions, RemoveWorktreeOptions } from './worktree-operation-options' @@ -152,7 +153,11 @@ export async function forceDeleteLocalBranch( } // Why: stale toast actions must not delete a branch that moved; `update-ref -d` deletes only if the ref still == expectedHead. try { - await runGit(['update-ref', '-d', `refs/heads/${branchName}`, expectedHead], repoPath) + // `update-ref -d` needs the packed-refs lock a running idle pack holds while + // it rewrites; waits it out rather than cancelling the pack. + await withRepoRefMaintenancePaused('branch-delete', () => + runGit(['update-ref', '-d', `refs/heads/${branchName}`, expectedHead], repoPath) + ) } catch { throw new Error( `Local branch "${branchName}" changed after the workspace was deleted. Review it before deleting it.` diff --git a/src/main/git/worktree-create-preparation.ts b/src/main/git/worktree-create-preparation.ts index 3be0fee1648..b60dc01ec33 100644 --- a/src/main/git/worktree-create-preparation.ts +++ b/src/main/git/worktree-create-preparation.ts @@ -10,6 +10,7 @@ import { WORKTREE_REMOVAL_REGISTRATION_TIMEOUT_MS } from './worktree' import { hasWorktreeBaseCommitRef } from './worktree-base-ref-probe' +import { withRepoRefMaintenancePaused } from './local-repo-ref-maintenance' import { gitExecFileAsync } from './runner' import { runWithGitReadCacheInvalidation } from './status' import { invalidateWslLinkedWorktreeGitRouting } from './wsl-linked-worktree-git-routing' @@ -69,46 +70,48 @@ export async function prepareWorktreeCreateCheckout( options: GitWorktreeExecOptions = {} ): Promise { try { - await runWithGitReadCacheInvalidation(async () => { - const effectiveBase = await resolveWorktreeAddBaseRef(baseBranch, (qualifiedRef) => - hasWorktreeBaseCommitRef(repoPath, qualifiedRef, options) - ) - try { - await gitExecFileAsync( - [ - ...windowsLongPathGitArgs(repoPath), - 'worktree', - 'add', - '--detach', - '--no-checkout', - worktreePath, - effectiveBase - ], - { ...gitExecOptions(repoPath, options), timeout: resolveWorktreeAddTimeoutMs() } + await withRepoRefMaintenancePaused('worktree-prepare', () => + runWithGitReadCacheInvalidation(async () => { + const effectiveBase = await resolveWorktreeAddBaseRef(baseBranch, (qualifiedRef) => + hasWorktreeBaseCommitRef(repoPath, qualifiedRef, options) ) - // The add just wrote the marker; drop any pre-create route before the reset routes Git. - invalidateWslLinkedWorktreeGitRouting(worktreePath) - // Why: reset materializes files without running user post-checkout hooks before submit. - await gitExecFileAsync( - [...windowsLongPathGitArgs(worktreePath), 'reset', '--hard', effectiveBase], - { ...gitExecOptions(worktreePath, options), timeout: resolveWorktreeAddTimeoutMs() } - ) - await gitExecFileAsync( - [ - ...windowsLongPathGitArgs(repoPath), - 'worktree', - 'lock', - '--reason', - lockReason, - worktreePath - ], - { ...gitExecOptions(repoPath, options), timeout: resolveWorktreeAddTimeoutMs() } - ) - } catch (error) { - await performDiscardPreparedWorktree(repoPath, worktreePath, options).catch(() => {}) - throw error - } - }) + try { + await gitExecFileAsync( + [ + ...windowsLongPathGitArgs(repoPath), + 'worktree', + 'add', + '--detach', + '--no-checkout', + worktreePath, + effectiveBase + ], + { ...gitExecOptions(repoPath, options), timeout: resolveWorktreeAddTimeoutMs() } + ) + // The add just wrote the marker; drop any pre-create route before the reset routes Git. + invalidateWslLinkedWorktreeGitRouting(worktreePath) + // Why: reset materializes files without running user post-checkout hooks before submit. + await gitExecFileAsync( + [...windowsLongPathGitArgs(worktreePath), 'reset', '--hard', effectiveBase], + { ...gitExecOptions(worktreePath, options), timeout: resolveWorktreeAddTimeoutMs() } + ) + await gitExecFileAsync( + [ + ...windowsLongPathGitArgs(repoPath), + 'worktree', + 'lock', + '--reason', + lockReason, + worktreePath + ], + { ...gitExecOptions(repoPath, options), timeout: resolveWorktreeAddTimeoutMs() } + ) + } catch (error) { + await performDiscardPreparedWorktree(repoPath, worktreePath, options).catch(() => {}) + throw error + } + }) + ) } finally { notifyPreparedWorktreeMutation(repoPath) } diff --git a/src/main/git/worktree-removal.ts b/src/main/git/worktree-removal.ts index c6743a4a7e2..afe1bf2a9c1 100644 --- a/src/main/git/worktree-removal.ts +++ b/src/main/git/worktree-removal.ts @@ -22,6 +22,7 @@ import { } from './worktree-operation-options' import { areWorktreePathsEqual } from './worktree-path-comparison' import { assertWorktreeCleanForRemoval } from './worktree-removal-preflight' +import { withRepoRefMaintenancePaused } from './local-repo-ref-maintenance' import { bumpWorktreeScanGeneration, listWorktrees } from './worktree-scan-cache' import { invalidateSparseCheckoutState } from './worktree-sparse-checkout-cache' @@ -36,8 +37,13 @@ export async function removeWorktree( options: RemoveWorktreeOptions = {} ): Promise { try { - return await runWithGitReadCacheInvalidation(() => - performRemoveWorktree(repoPath, worktreePath, force, options) + // Removal deletes branches, and a ref deletion needs the packed-refs lock a + // running idle pack holds while it rewrites. Waits that window out; the + // prune phase that follows it is concurrency-safe and is left to finish. + return await withRepoRefMaintenancePaused('worktree-remove', () => + runWithGitReadCacheInvalidation(() => + performRemoveWorktree(repoPath, worktreePath, force, options) + ) ) } finally { invalidateWslLinkedWorktreeGitRouting(worktreePath) diff --git a/src/main/ipc/repos-create.test.ts b/src/main/ipc/repos-create.test.ts index 86e2dc5ee10..44a16f8e10d 100644 --- a/src/main/ipc/repos-create.test.ts +++ b/src/main/ipc/repos-create.test.ts @@ -61,8 +61,11 @@ vi.mock('fs/promises', () => ({ rm: rmMock })) +// `availableParallelism` is read at module load by the git admission scheduler, +// which this module graph reaches; a partial `os` mock breaks that import. vi.mock('os', () => ({ - homedir: homedirMock + homedir: homedirMock, + availableParallelism: () => 8 })) vi.mock('../git/runner', () => ({ diff --git a/src/main/ipc/worktrees.ts b/src/main/ipc/worktrees.ts index ff58a561ee3..3fd2fb41908 100644 --- a/src/main/ipc/worktrees.ts +++ b/src/main/ipc/worktrees.ts @@ -17,7 +17,10 @@ import { registerSparseCheckoutCacheInvalidation } from './worktrees/listing/reg import { registerWorktreeMetadataHandlers } from './worktrees/metadata/register-worktree-metadata-handlers' import { registerWorktreeForgetHandlers } from './worktrees/removal/register-worktree-forget-handlers' import { registerWorktreeRemovalHandlers } from './worktrees/removal/register-worktree-removal-handlers' -import type { WorktreeIpcContext } from './worktrees/worktree-ipc-context' +import { + createWorktreeRemovalRegistry, + type WorktreeIpcContext +} from './worktrees/worktree-ipc-context' registerDetectedWorktreeScanInvalidation() @@ -66,7 +69,7 @@ export function registerWorktreeHandlers( runtime, ...(options ? { options } : {}), detectedWorktreeCancellations: createSenderScopedRequestCancellations(), - worktreeRemovalsInFlight: new Map() + worktreeRemovalsInFlight: createWorktreeRemovalRegistry() } // Remove all stale registrations before installing any replacement handler. diff --git a/src/main/ipc/worktrees/worktree-ipc-context.ts b/src/main/ipc/worktrees/worktree-ipc-context.ts index 5455a71d06e..153e4b723ec 100644 --- a/src/main/ipc/worktrees/worktree-ipc-context.ts +++ b/src/main/ipc/worktrees/worktree-ipc-context.ts @@ -14,3 +14,18 @@ export type WorktreeIpcContext = { detectedWorktreeCancellations: SenderScopedRequestCancellations worktreeRemovalsInFlight: Map } + +// Why: removal and forget both delete refs, and a ref deletion has to take the +// `packed-refs` lock. Idle ref maintenance needs a process-wide view of that +// registry so it never packs while one is running. +let activeWorktreeRemovals: ReadonlyMap | null = null + +export function createWorktreeRemovalRegistry(): Map { + const registry = new Map() + activeWorktreeRemovals = registry + return registry +} + +export function hasWorktreeRemovalsInFlight(): boolean { + return (activeWorktreeRemovals?.size ?? 0) > 0 +} diff --git a/src/main/repo-maintenance-idle-gate.test.ts b/src/main/repo-maintenance-idle-gate.test.ts new file mode 100644 index 00000000000..78728f3a43a --- /dev/null +++ b/src/main/repo-maintenance-idle-gate.test.ts @@ -0,0 +1,134 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const isOnBatteryPowerMock = vi.hoisted(() => vi.fn(() => false)) +const hasPendingPreparationsMock = vi.hoisted(() => vi.fn(() => false)) +const hasRemovalsInFlightMock = vi.hoisted(() => vi.fn(() => false)) +const setProbeMock = vi.hoisted(() => vi.fn()) +const disposeMock = vi.hoisted(() => vi.fn(async () => {})) +const postponeMock = vi.hoisted(() => vi.fn()) +const powerListeners = vi.hoisted(() => new Map void>()) +const appListeners = vi.hoisted(() => new Map void>()) + +vi.mock('electron', () => ({ + app: { + on: (event: string, listener: () => void) => appListeners.set(event, listener), + off: (event: string) => appListeners.delete(event) + }, + powerMonitor: { + isOnBatteryPower: isOnBatteryPowerMock, + on: (event: string, listener: () => void) => powerListeners.set(event, listener), + off: (event: string) => powerListeners.delete(event) + } +})) + +vi.mock('./worktree-create-preparation', () => ({ + hasPendingWorktreeCreatePreparations: hasPendingPreparationsMock +})) + +vi.mock('./ipc/worktrees/worktree-ipc-context', () => ({ + hasWorktreeRemovalsInFlight: hasRemovalsInFlightMock +})) + +vi.mock('./git/local-repo-ref-maintenance', () => ({ + setRepoMaintenanceActivityProbe: setProbeMock, + disposeLocalRepoRefMaintenance: disposeMock, + postponeRepoRefMaintenance: postponeMock +})) + +import { installRepoMaintenanceIdleGate } from './repo-maintenance-idle-gate' + +function installProbe( + overrides: Partial<{ isQuitting: () => boolean; getWorkingAgentCount: () => number }> = {} +): { probe: () => boolean; uninstall: () => Promise } { + const uninstall = installRepoMaintenanceIdleGate({ + isQuitting: () => false, + getWorkingAgentCount: () => 0, + ...overrides + }) + return { probe: setProbeMock.mock.calls.at(-1)?.[0] as () => boolean, uninstall } +} + +beforeEach(() => { + isOnBatteryPowerMock.mockReturnValue(false) + hasPendingPreparationsMock.mockReturnValue(false) + hasRemovalsInFlightMock.mockReturnValue(false) + postponeMock.mockClear() + powerListeners.clear() + appListeners.clear() + setProbeMock.mockClear() + disposeMock.mockClear() +}) + +afterEach(() => { + vi.restoreAllMocks() +}) + +describe('repo maintenance idle gate', () => { + it('reports idle when nothing is happening', () => { + expect(installProbe().probe()).toBe(false) + }) + + it('vetoes while an agent is working', () => { + expect(installProbe({ getWorkingAgentCount: () => 1 }).probe()).toBe(true) + }) + + it('vetoes while a worktree create is prepared or in flight', () => { + hasPendingPreparationsMock.mockReturnValue(true) + + expect(installProbe().probe()).toBe(true) + }) + + it('vetoes while a worktree removal is deleting refs', () => { + // Removal deletes branches, and a ref deletion needs the same packed-refs lock. + hasRemovalsInFlightMock.mockReturnValue(true) + + expect(installProbe().probe()).toBe(true) + }) + + it('vetoes on battery power', () => { + isOnBatteryPowerMock.mockReturnValue(true) + + expect(installProbe().probe()).toBe(true) + }) + + it('vetoes during shutdown', () => { + expect(installProbe({ isQuitting: () => true }).probe()).toBe(true) + }) + + it('treats an unavailable power API as not-on-battery', () => { + isOnBatteryPowerMock.mockImplementation(() => { + throw new Error('unsupported') + }) + + expect(installProbe().probe()).toBe(false) + }) + + it('pushes the next attempt out when the machine drops onto battery', () => { + // Do-not-start, never stop-what-is-running: killing a pack to honour a + // battery change would strand a ref lock to save a little unlinking. + installProbe() + + powerListeners.get('on-battery')?.() + + expect(postponeMock).toHaveBeenCalledTimes(1) + }) + + it('pushes the next attempt out when the user comes back to the window', () => { + // A focus transition, not focus itself: a window left focused while the user + // walks away fires no event and blocks nothing. + installProbe() + + appListeners.get('browser-window-focus')?.() + + expect(postponeMock).toHaveBeenCalledTimes(1) + }) + + it('cancels armed timers, unsubscribes both sources, and clears the probe when uninstalled', async () => { + await installProbe().uninstall() + + expect(disposeMock).toHaveBeenCalledTimes(1) + expect(powerListeners.has('on-battery')).toBe(false) + expect(appListeners.has('browser-window-focus')).toBe(false) + expect(setProbeMock).toHaveBeenLastCalledWith(null) + }) +}) diff --git a/src/main/repo-maintenance-idle-gate.ts b/src/main/repo-maintenance-idle-gate.ts new file mode 100644 index 00000000000..78bb25fe68c --- /dev/null +++ b/src/main/repo-maintenance-idle-gate.ts @@ -0,0 +1,67 @@ +import { app, powerMonitor } from 'electron' +import { + disposeLocalRepoRefMaintenance, + postponeRepoRefMaintenance, + setRepoMaintenanceActivityProbe +} from './git/local-repo-ref-maintenance' +import { hasWorktreeRemovalsInFlight } from './ipc/worktrees/worktree-ipc-context' +import { hasPendingWorktreeCreatePreparations } from './worktree-create-preparation' + +/** + * The app-wide "not now" answer for idle repo maintenance. + * + * `pack-refs` holds a general git admission slot for its whole run, which on a + * large backlog is minutes, and takes the `packed-refs` lock while it writes. + * Any ref deletion needs that same lock and gives up after + * `core.packedRefsTimeout` (1s), so worktree removal in particular has to veto + * this -- as does a create in flight, an agent mid-run, and shutdown. Battery is + * a veto too: this is work the user did not ask for, and a plugged-in quiet + * window always comes along later. + */ +export type RepoMaintenanceIdleInputs = { + isQuitting: () => boolean + getWorkingAgentCount: () => number +} + +export function installRepoMaintenanceIdleGate( + inputs: RepoMaintenanceIdleInputs +): () => Promise { + setRepoMaintenanceActivityProbe( + () => + inputs.isQuitting() || + inputs.getWorkingAgentCount() > 0 || + hasPendingWorktreeCreatePreparations() || + hasWorktreeRemovalsInFlight() || + isOnBatteryPower() + ) + // Do-not-start, never stop-what-is-running. Killing a pack to honour a battery + // or focus change would strand a ref lock roughly one time in five to save at + // most a couple of minutes of background unlinking; pushing the next attempt + // out costs nothing and risks nothing. + const onBattery = (): void => { + postponeRepoRefMaintenance() + } + const onFocus = (): void => { + postponeRepoRefMaintenance() + } + powerMonitor.on('on-battery', onBattery) + app.on('browser-window-focus', onFocus) + return () => { + app.off('browser-window-focus', onFocus) + powerMonitor.off('on-battery', onBattery) + // Order matters: clearing the probe alone would leave armed timers running + // against a gate that can no longer see agents, creates, or shutdown. + const stopped = disposeLocalRepoRefMaintenance() + setRepoMaintenanceActivityProbe(null) + return stopped + } +} + +function isOnBatteryPower(): boolean { + try { + return powerMonitor.isOnBatteryPower() + } catch { + // Absence of the API is not evidence of battery; desktops answer false anyway. + return false + } +} diff --git a/src/main/runtime/fetch-remote-cache.test.ts b/src/main/runtime/fetch-remote-cache.test.ts index fc2d761c059..c97966c344e 100644 --- a/src/main/runtime/fetch-remote-cache.test.ts +++ b/src/main/runtime/fetch-remote-cache.test.ts @@ -133,9 +133,11 @@ describe('OrcaRuntimeService.fetchRemoteWithCache', () => { const first = runtime.fetchRemoteWithCache('/repo/c', 'origin') const second = runtime.fetchRemoteWithCache('/repo/c', 'origin') - // Allow both callers to register before we resolve. - await Promise.resolve() - await Promise.resolve() + // Allow both callers to register before we resolve. Each canonicalizes the + // repo key first, so the dispatch lands several microtasks in. + for (let tick = 0; tick < 8; tick += 1) { + await Promise.resolve() + } expect(fetchCallCount()).toBe(1) resolveFetch() diff --git a/src/main/runtime/runtime-remote-fetch-controller.ts b/src/main/runtime/runtime-remote-fetch-controller.ts index c48a8437a3d..b3b9df5dcba 100644 --- a/src/main/runtime/runtime-remote-fetch-controller.ts +++ b/src/main/runtime/runtime-remote-fetch-controller.ts @@ -1,4 +1,9 @@ import { GIT_FETCH_SKIP_AUTO_MAINTENANCE_CONFIG_ARGS } from '../../shared/git-fetch-auto-maintenance' +import { getCanonicalRepoKey } from '../git/canonical-repo-key' +import { + armLocalRepoRefMaintenance, + setRepoRefMaintenanceBusyProbe +} from '../git/local-repo-ref-maintenance' import { gitExecFileAsync } from '../git/runner' import { setBoundedMapEntry } from './runtime-async-boundaries' @@ -33,6 +38,11 @@ export class RuntimeRemoteFetchController { return this.fetchLastCompletedAt } + /** `${runtimeKey}::${gitCommonDir}` -- one repo on one execution host. */ + async getCanonicalRepoKey(repoPath: string, gitOptions: GitOptions = {}): Promise { + return getCanonicalRepoKey(repoPath, gitOptions) + } + async getCanonicalFetchKey( repoPath: string, remote: string, @@ -45,23 +55,41 @@ export class RuntimeRemoteFetchController { setBoundedMapEntry(this.canonicalFetchKeyCache, cacheKey, cached, REMOTE_FETCH_CACHE_MAX) return cached } - let resolved = cacheKey - try { - const { stdout } = await gitExecFileAsync( - ['rev-parse', '--path-format=absolute', '--git-common-dir'], - { cwd: repoPath, ...gitOptions } - ) - const commonDir = stdout.trim() - if (commonDir) { - resolved = `${runtimeKey}::${commonDir}::${remote}` - } - } catch { - // The caller path remains a safe serialization key when canonicalization fails. - } + const resolved = `${await this.getCanonicalRepoKey(repoPath, gitOptions)}::${remote}` setBoundedMapEntry(this.canonicalFetchKeyCache, cacheKey, resolved, REMOTE_FETCH_CACHE_MAX) return resolved } + /** + * Orca strips git's auto-maintenance off these fetches, so every one of them + * adds to a loose-ref backlog nothing else will ever pack. Arm the idle sweep + * that pays it back; each fetch pushes the attempt a further quiet period out. + */ + private armRefMaintenance(repoPath: string, gitOptions: GitOptions): void { + void this.getCanonicalRepoKey(repoPath, gitOptions) + .then((key) => { + setRepoRefMaintenanceBusyProbe(key, () => this.hasInflightFetchForRepo(key)) + armLocalRepoRefMaintenance({ + key, + repoPath, + ...(gitOptions.wslDistro ? { wslDistro: gitOptions.wslDistro } : {}) + }) + }) + .catch(() => { + // Maintenance is best effort; a repo we cannot name is a repo we skip. + }) + } + + private hasInflightFetchForRepo(repoKey: string): boolean { + const prefix = `${repoKey}::` + for (const key of this.fetchInflight.keys()) { + if (key.startsWith(prefix)) { + return true + } + } + return false + } + private enqueueRemoteFetch( remoteKey: string, runFetch: () => Promise @@ -123,6 +151,7 @@ export class RuntimeRemoteFetchController { }) ).finally(() => { this.fetchInflight.delete(key) + this.armRefMaintenance(repoPath, gitOptions) }) this.fetchInflight.set(key, promise) return promise @@ -178,6 +207,7 @@ export class RuntimeRemoteFetchController { }) }).finally(() => { this.fetchInflight.delete(key) + this.armRefMaintenance(repoPath, gitOptions) }) this.fetchInflight.set(key, promise) return promise diff --git a/src/main/runtime/runtime-remote-fetch-ref-maintenance.test.ts b/src/main/runtime/runtime-remote-fetch-ref-maintenance.test.ts new file mode 100644 index 00000000000..f577da24854 --- /dev/null +++ b/src/main/runtime/runtime-remote-fetch-ref-maintenance.test.ts @@ -0,0 +1,145 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +// Why: Orca's fetches are what create the loose-ref backlog (they suppress +// git's auto-maintenance), so the fetch controller is where the idle sweep has +// to be armed. These tests pin that wiring and the per-repo busy signal it +// hands the sweep. + +const gitExecFileAsyncMock = vi.hoisted(() => vi.fn()) +const armMock = vi.hoisted(() => vi.fn()) +const busyProbeMock = vi.hoisted(() => vi.fn()) + +vi.mock('../git/runner', async (importOriginal) => ({ + ...((await importOriginal()) as Record), + gitExecFileAsync: gitExecFileAsyncMock +})) + +vi.mock('../git/local-repo-ref-maintenance', async (importOriginal) => ({ + ...((await importOriginal()) as Record), + armLocalRepoRefMaintenance: armMock, + setRepoRefMaintenanceBusyProbe: busyProbeMock +})) + +import { _resetCanonicalRepoKeyCacheForTests } from '../git/canonical-repo-key' +import { RuntimeRemoteFetchController } from './runtime-remote-fetch-controller' + +function armedTargets(): { key: string }[] { + return armMock.mock.calls.map(([args]) => args as { key: string }) +} + +/** The per-repo "a fetch is in flight" answer the controller registers for a key. */ +function busyProbeFor(key: string): (() => boolean) | undefined { + return busyProbeMock.mock.calls.findLast(([registered]) => registered === key)?.[1] as + | (() => boolean) + | undefined +} + +beforeEach(() => { + _resetCanonicalRepoKeyCacheForTests() + gitExecFileAsyncMock.mockReset() + armMock.mockReset() + busyProbeMock.mockReset() + gitExecFileAsyncMock.mockImplementation(async (argv: string[]) => + argv[0] === 'rev-parse' ? { stdout: '/repo/.git\n', stderr: '' } : { stdout: '', stderr: '' } + ) +}) + +afterEach(() => { + vi.restoreAllMocks() +}) + +describe('fetch-armed ref maintenance', () => { + it('arms the sweep for the repo after a remote fetch, keyed by common dir', async () => { + const controller = new RuntimeRemoteFetchController() + + await controller.getOrStartRemoteFetch('/repo/worktrees/a', 'origin') + + expect(armedTargets().map((target) => target.key)).toEqual(['local::/repo/.git']) + }) + + it('gives every worktree of one repo the same maintenance key', async () => { + const controller = new RuntimeRemoteFetchController() + + await controller.getOrStartRemoteFetch('/repo/worktrees/a', 'origin') + await controller.getOrStartRemoteTrackingBaseRefresh('/repo/worktrees/b', { + remote: 'origin', + branch: 'main', + ref: 'refs/remotes/origin/main', + base: 'origin/main' + }) + + const keys = new Set(armedTargets().map((target) => target.key)) + expect(keys).toEqual(new Set(['local::/repo/.git'])) + }) + + it('scopes the key to the WSL distro that executes the repo', async () => { + const controller = new RuntimeRemoteFetchController() + + await controller.getOrStartRemoteFetch('//wsl$/Ubuntu/repo', 'origin', { + wslDistro: 'Ubuntu' + }) + + expect(armedTargets()[0]?.key).toBe('wsl:Ubuntu::/repo/.git') + }) + + it('does not collapse every repo onto one key on Git older than 2.31', async () => { + // Old Git echoes the unrecognized `--path-format` flag, exits 0, and prints a + // relative `.git`; taking that raw would name every repository identically. + gitExecFileAsyncMock.mockImplementation(async (argv: string[]) => + argv[0] === 'rev-parse' + ? { stdout: '--path-format=absolute\n.git\n', stderr: '' } + : { stdout: '', stderr: '' } + ) + const controller = new RuntimeRemoteFetchController() + + await controller.getOrStartRemoteFetch('/repo/one', 'origin') + await controller.getOrStartRemoteFetch('/repo/two', 'origin') + + expect(armedTargets().map((entry) => entry.key)).toEqual([ + 'local::/repo/one/.git', + 'local::/repo/two/.git' + ]) + }) + + it('reports the repo as busy while another fetch on it is in flight', async () => { + const controller = new RuntimeRemoteFetchController() + await controller.getOrStartRemoteFetch('/repo', 'first') + const isBusy = busyProbeFor('local::/repo/.git') + expect(isBusy?.()).toBe(false) + + let releaseFetch: (() => void) | undefined + gitExecFileAsyncMock.mockImplementation(async (argv: string[]) => { + if (argv[0] === 'rev-parse') { + return { stdout: '/repo/.git\n', stderr: '' } + } + await new Promise((resolve) => { + releaseFetch = resolve + }) + return { stdout: '', stderr: '' } + }) + const second = controller.getOrStartRemoteFetch('/repo', 'second') + await vi.waitFor(() => expect(releaseFetch).toBeDefined()) + expect(isBusy?.()).toBe(true) + + releaseFetch?.() + await second + expect(isBusy?.()).toBe(false) + }) + + it('arms even when the fetch fails, because a partial fetch still writes refs', async () => { + const controller = new RuntimeRemoteFetchController() + gitExecFileAsyncMock.mockImplementation(async (argv: string[]) => { + if (argv[0] === 'rev-parse') { + return { stdout: '/repo/.git\n', stderr: '' } + } + throw new Error('network is unreachable') + }) + vi.spyOn(console, 'warn').mockImplementation(() => {}) + + await expect(controller.getOrStartRemoteFetch('/repo', 'origin')).resolves.toEqual({ + ok: false, + errorKind: 'git_error' + }) + expect(armedTargets()).toHaveLength(1) + }) +}) diff --git a/src/main/startup/main-process-observers.ts b/src/main/startup/main-process-observers.ts index c3d83450b7c..ba37b0a312f 100644 --- a/src/main/startup/main-process-observers.ts +++ b/src/main/startup/main-process-observers.ts @@ -18,6 +18,7 @@ import { AgentSessionTransitionRecorder } from '../stats/agent-session-transitio import { ClaudeUsageStore } from '../claude-usage/store' import { CodexUsageStore } from '../codex-usage/store' import { OpenCodeUsageStore } from '../opencode-usage/store' +import { installRepoMaintenanceIdleGate } from '../repo-maintenance-idle-gate' import { mainProcessState as state } from './main-process-state' export function initializeMainProcessObservers(): void { @@ -35,6 +36,10 @@ export function initializeMainProcessObservers(): void { ) // Why: start from empty — disk-hydrated status rows are UI continuity only; only this runtime's hook events keep the computer awake. state.agentAwakeService.setStatuses([]) + state.uninstallRepoMaintenanceIdleGate = installRepoMaintenanceIdleGate({ + isQuitting: () => state.isQuitting, + getWorkingAgentCount: () => state.agentAwakeService?.getWorkingAgentCount() ?? 0 + }) const collectChangedProviderSessionWorktrees = createHookProviderSessionInvalidator() const publishProviderSessionChanges = (identities: AgentHookProviderSessionIdentity[]): void => { const ownedIdentities = identities.map((identity) => ({ diff --git a/src/main/startup/main-process-quit.ts b/src/main/startup/main-process-quit.ts index ec893456c5a..61203bc3164 100644 --- a/src/main/startup/main-process-quit.ts +++ b/src/main/startup/main-process-quit.ts @@ -14,6 +14,7 @@ import { clearRuntimeMetadataIfOwned } from '../runtime/runtime-metadata' import { shutdownPairedRuntimeBrowserClientHosts } from '../browser/paired-runtime-browser-client-host-runtime' import { browserManager } from '../browser/browser-manager' import { stopCodexStateDbBackfillRecoveries } from '../codex/codex-state-db-backfill-recovery' +import { awaitPackedRefsLockRelease } from '../git/local-repo-ref-maintenance' import { settleTeardownWithinDeadline, settleWithinMs } from '../quit-teardown-deadline' import { quitTeardownStartGate } from '../quit-teardown-start-gate' import { setUnreadDockBadgeCount } from '../dock/unread-badge' @@ -33,6 +34,8 @@ let daemonDisconnectDone = false let watcherShutdownPromise: Promise | null = null // Why 2s: a config delete is best-effort, not durable state. const GROK_HOOK_CLEANUP_DEADLINE_MS = 2_000 +// Why 2s: long enough for a `pack-refs` child to take SIGTERM and unlink its lock. +const REF_MAINTENANCE_QUIT_DEADLINE_MS = 2_000 function shutdownWatchersOnce(): Promise { if (state.watcherShutdownDone) { @@ -73,6 +76,10 @@ function installBeforeQuitHandler(): void { state.unsubscribeAgentAwakeStatusChanges = null state.agentAwakeService?.dispose() state.agentAwakeService = null + // Why wait but not uninstall: a renderer beforeunload can still veto this + // quit, and tearing the sweep down here would kill it for the rest of the + // session. `isQuitting` already vetoes new attempts; will-quit does the teardown. + 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() }) @@ -123,6 +130,16 @@ function installWillQuitHandler(): void { const structuredAgentSessionShutdown = stopStructuredAgentSessionRuntime() state.pluginService = null setUnreadDockBadgeCount(0) + // Why wait rather than kill: the child finishes fine orphaned, and signalling + // it mid-prune strands a ref lock Git never clears. The wait is only for the + // short rewrite window, and is bounded so a quit can never hang on it. + const refMaintenanceShutdown = settleWithinMs( + Promise.all([state.repoMaintenanceShutdown, state.uninstallRepoMaintenanceIdleGate?.()]).then( + () => {} + ), + REF_MAINTENANCE_QUIT_DEADLINE_MS + ).then(() => {}) + state.uninstallRepoMaintenanceIdleGate = null agentHookServer.stop() // Why Windows only: POSIX hooks short-circuit on ORCA_PANE_KEY, while Windows must register a // bare script path that cannot express the guard and would otherwise keep spawning after quit. @@ -219,6 +236,7 @@ function installWillQuitHandler(): void { { name: 'plugin-hosts', promise: pluginHostShutdown }, { name: 'skill-uploads', promise: skillUploadShutdown }, { name: 'grok-hooks', promise: grokHookCleanup }, + { name: 'ref-maintenance', promise: refMaintenanceShutdown }, { name: 'codex-backfill-recovery', promise: codexBackfillRecoveryShutdown }, { name: 'structured-agent-session', promise: structuredAgentSessionShutdown }, { name: 'usage-cache', promise: usageCacheFlush }, diff --git a/src/main/startup/main-process-state.ts b/src/main/startup/main-process-state.ts index 05c45d5b567..d48219b461e 100644 --- a/src/main/startup/main-process-state.ts +++ b/src/main/startup/main-process-state.ts @@ -71,6 +71,8 @@ export const mainProcessState = { headlessBrowserDisplayAvailable: false, starNag: null as StarNagService | null, agentAwakeService: null as AgentAwakeService | null, + uninstallRepoMaintenanceIdleGate: null as (() => Promise) | null, + repoMaintenanceShutdown: Promise.resolve() as Promise, crashReports: null as CrashReportStore | null, unsubscribeAgentAwakeStatusChanges: null as (() => void) | null, publishProviderSessionChanges: null as diff --git a/src/main/worktree-create-preparation.ts b/src/main/worktree-create-preparation.ts index 22bcf5d1e2c..b4d86923490 100644 --- a/src/main/worktree-create-preparation.ts +++ b/src/main/worktree-create-preparation.ts @@ -61,6 +61,11 @@ type ConsumePreparedWorktreeArgs = { const preparations = new Map() +/** A prepared checkout is a create that is either in flight or imminent. */ +export function hasPendingWorktreeCreatePreparations(): boolean { + return preparations.size > 0 || staleCleanupInFlight.size > 0 +} + function pathOps(path: string): Pick { return isWindowsAbsolutePathLike(path) ? win32 : posix } diff --git a/src/shared/git-binary-compatibility.test.ts b/src/shared/git-binary-compatibility.test.ts index 3a8f562dff1..2861c447788 100644 --- a/src/shared/git-binary-compatibility.test.ts +++ b/src/shared/git-binary-compatibility.test.ts @@ -277,6 +277,39 @@ describeBinaryCompatibility('real Git binary compatibility', () => { ).rejects.toMatchObject({ code: 1 }) }) + it('packs loose refs and reads the maintenance opt-out at the baseline', async () => { + // Why: idle ref maintenance runs `pack-refs --all --prune` on every supported + // Git rather than the 2.45+ `--auto` form, and reads `maintenance.auto` to + // honour a user who disabled Git's own auto-maintenance. Both must work at 2.25. + const head = (await runGit(['rev-parse', 'HEAD'])).stdout.trim() + const packedRef = 'refs/remotes/origin/compat-pack-refs' + await runGit(['update-ref', packedRef, head]) + await expect(readFile(join(repoPath, '.git', packedRef), 'utf-8')).resolves.toContain(head) + + await expect(runGit(['pack-refs', '--all', '--prune'])).resolves.toBeDefined() + + // The loose file is gone and the ref still resolves through packed-refs. + await expect(readFile(join(repoPath, '.git', packedRef), 'utf-8')).rejects.toMatchObject({ + code: 'ENOENT' + }) + await expect(runGit(['rev-parse', '--verify', packedRef])).resolves.toMatchObject({ + stdout: `${head}\n` + }) + await expect(readFile(join(repoPath, '.git', 'packed-refs'), 'utf-8')).resolves.toContain( + packedRef + ) + + // `--get` exits 1 on an unset key; that absence must read as consent, not opt-out. + await expect(runGit(['config', '--bool', '--get', 'maintenance.auto'])).rejects.toMatchObject({ + code: 1 + }) + await runGit(['config', 'maintenance.auto', 'false']) + await expect(runGit(['config', '--bool', '--get', 'maintenance.auto'])).resolves.toMatchObject({ + stdout: 'false\n' + }) + await runGit(['config', '--unset', 'maintenance.auto']) + }) + it('fetches hosted review heads into dedicated refs', async () => { const head = (await runGit(['rev-parse', 'HEAD'])).stdout.trim() await runGit(['update-ref', 'refs/pull/42/head', head]) diff --git a/src/shared/loose-ref-count.test.ts b/src/shared/loose-ref-count.test.ts new file mode 100644 index 00000000000..c69f655121a --- /dev/null +++ b/src/shared/loose-ref-count.test.ts @@ -0,0 +1,119 @@ +import { mkdir, mkdtemp, rm, symlink, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it, vi } from 'vitest' + +// Wraps the real `readdir` so the walk's concurrency is observable without +// changing what it reads. +const readdirCalls = vi.hoisted(() => ({ outstanding: 0, peak: 0, count: 0 })) + +vi.mock('node:fs/promises', async (importOriginal) => { + const actual = await importOriginal>() + const realReaddir = actual.readdir as (...args: unknown[]) => Promise + return { + ...actual, + readdir: async (...args: unknown[]) => { + readdirCalls.outstanding += 1 + readdirCalls.count += 1 + readdirCalls.peak = Math.max(readdirCalls.peak, readdirCalls.outstanding) + try { + return await realReaddir(...args) + } finally { + readdirCalls.outstanding -= 1 + } + } + } +}) + +import { countLooseRefs } from './loose-ref-count' + +const roots: string[] = [] + +async function makeRefsTree(counts: Record): Promise { + const root = await mkdtemp(join(tmpdir(), 'orca-loose-refs-')) + roots.push(root) + const refs = join(root, 'refs') + for (const [namespace, count] of Object.entries(counts)) { + const directory = join(refs, namespace) + await mkdir(directory, { recursive: true }) + for (let index = 0; index < count; index += 1) { + await writeFile(join(directory, `ref-${index}`), 'a'.repeat(40)) + } + } + await mkdir(refs, { recursive: true }) + return refs +} + +afterEach(async () => { + vi.restoreAllMocks() + await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))) +}) + +describe('countLooseRefs', () => { + it('counts files across nested namespaces', async () => { + const refs = await makeRefsTree({ heads: 3, 'remotes/origin': 4, 'remotes/fork/deep': 2 }) + + await expect(countLooseRefs(refs, 100)).resolves.toEqual({ count: 9, saturated: false }) + }) + + it('stops at the budget instead of walking the whole backlog', async () => { + const refs = await makeRefsTree({ 'remotes/origin': 500 }) + + const result = await countLooseRefs(refs, 10) + + expect(result).toEqual({ count: 10, saturated: true }) + }) + + it('reports zero for a repository with no refs directory', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-loose-refs-missing-')) + roots.push(root) + + await expect(countLooseRefs(join(root, 'refs'), 100)).resolves.toEqual({ + count: 0, + saturated: false + }) + }) + + it('never has more than one directory read outstanding', async () => { + // libuv's filesystem thread pool has four slots shared with the whole main + // process. A probe that fanned out would stall unrelated fs work, so this + // pins the walk as strictly sequential rather than merely bounded. + const refs = await makeRefsTree({ + 'remotes/a': 3, + 'remotes/b': 3, + 'remotes/c': 3, + 'remotes/d': 3, + 'remotes/e/deep': 3 + }) + readdirCalls.peak = 0 + readdirCalls.count = 0 + + await countLooseRefs(refs, 1000) + + expect(readdirCalls.count).toBeGreaterThan(1) + expect(readdirCalls.peak).toBe(1) + }) + + it('reads each directory once rather than streaming it in batches', async () => { + // One thread-pool round trip per directory is what makes the probe ~8x + // cheaper than the streaming form on a real degraded repository. + const refs = await makeRefsTree({ 'remotes/origin': 400 }) + readdirCalls.count = 0 + + await countLooseRefs(refs, 1000) + + // refs/ plus refs/remotes plus refs/remotes/origin. + expect(readdirCalls.count).toBe(3) + }) + + it('does not follow directory symlinks into a loop', async () => { + const refs = await makeRefsTree({ heads: 2 }) + await symlink(refs, join(refs, 'loop'), 'dir') + + const result = await countLooseRefs(refs, 100) + + expect(result.saturated).toBe(false) + // The symlink is one dirent, never a second traversal of the tree. + expect(result.count).toBe(3) + }) +}) diff --git a/src/shared/loose-ref-count.ts b/src/shared/loose-ref-count.ts new file mode 100644 index 00000000000..8f31d8b03a8 --- /dev/null +++ b/src/shared/loose-ref-count.ts @@ -0,0 +1,80 @@ +import { readdir } from 'node:fs/promises' +import { join } from 'node:path' + +export type LooseRefCount = { + /** Loose ref files seen, never above `budget`. */ + count: number + /** The walk stopped early, so `count` is a floor rather than the total. */ + saturated: boolean +} + +// Why: a ref tree is shallow and wide; this bounds both the directories visited +// and the queue holding those still to visit, so neither a symlink loop nor a +// pathological repo turns a gate probe into an unbounded walk. +const DIRECTORY_VISIT_CEILING = 4096 + +/** + * Count loose refs under a repository's `refs/` directory, stopping at `budget`. + * + * Deliberately budgeted: callers use this as an admission gate, so the cost has + * to be bounded by the threshold being tested and not by the size of the + * backlog it is testing for. + * + * One `readdir` per directory, dirents only -- no `stat` per entry, and no + * `opendir` streaming. Measured against a real 36,600-loose-ref repository, the + * batched form is ~8x faster (23ms vs 177ms median to reach a 1000 threshold) + * and holds the event loop for less than half as long, because streaming issues + * a thread-pool round trip every 32 entries where this issues one per + * directory. The cost is holding one directory's dirents at a time, which is + * bounded by the widest ref namespace rather than by the size of the tree. + * + * Strictly sequential on purpose: it awaits one directory before opening the + * next, so it can never occupy more than one of libuv's four thread-pool slots + * and cannot stall unrelated main-process filesystem work. + * + * `signal` stops the walk between directories. A single hung `readdir` is not + * interruptible, but it holds no Git lock, so it delays only maintenance. + */ +export async function countLooseRefs( + refsDirectory: string, + budget: number, + signal?: AbortSignal +): Promise { + const pending = [refsDirectory] + let count = 0 + let visited = 0 + while (pending.length > 0) { + const directory = pending.pop() + if (directory === undefined) { + break + } + visited += 1 + // A cancelled walk reports what it saw as a floor rather than throwing; callers + // already have to treat a saturated result as "not known to be clean". + if ( + signal?.aborted === true || + visited > DIRECTORY_VISIT_CEILING || + pending.length > DIRECTORY_VISIT_CEILING + ) { + return { count, saturated: true } + } + let entries: { name: string; isDirectory: () => boolean }[] + try { + entries = await readdir(directory, { withFileTypes: true }) + } catch { + // A missing or unreadable namespace contributes nothing to the count. + continue + } + for (const entry of entries) { + if (entry.isDirectory()) { + pending.push(join(directory, entry.name)) + continue + } + count += 1 + if (count >= budget) { + return { count, saturated: true } + } + } + } + return { count, saturated: false } +} diff --git a/src/shared/packed-refs-lock-gate.ts b/src/shared/packed-refs-lock-gate.ts new file mode 100644 index 00000000000..f5cff6b8c4a --- /dev/null +++ b/src/shared/packed-refs-lock-gate.ts @@ -0,0 +1,43 @@ +/** + * Tracks the one window in a pack that actually excludes anybody: the + * `packed-refs` rewrite. Callers about to touch refs wait this out rather than + * killing the child, because a signal delivered into the prune phase strands a + * `refs/**\/*.lock` roughly one time in five and Git never clears those. + */ +export class PackedRefsLockGate { + private held = false + private waiters: (() => void)[] = [] + + setHeld(held: boolean): void { + this.held = held + if (held) { + return + } + const waiting = this.waiters + this.waiters = [] + for (const resolve of waiting) { + resolve() + } + } + + /** Resolves on release, or on `timeoutMs` -- past which Git's own retry is the better bet. */ + whenReleased(timeoutMs: number): Promise { + if (!this.held) { + return Promise.resolve() + } + return new Promise((resolve) => { + let settled = false + const finish = (): void => { + if (settled) { + return + } + settled = true + clearTimeout(timer) + resolve() + } + const timer = setTimeout(finish, timeoutMs) + timer.unref?.() + this.waiters.push(finish) + }) + } +} diff --git a/src/shared/repo-ref-maintenance-policy.ts b/src/shared/repo-ref-maintenance-policy.ts new file mode 100644 index 00000000000..7dc8edd1de3 --- /dev/null +++ b/src/shared/repo-ref-maintenance-policy.ts @@ -0,0 +1,166 @@ +/** + * Idle-time loose-ref packing for repositories Orca itself degrades. + * + * Orca strips git's auto-maintenance off its own frequent fetches + * (`GIT_FETCH_SKIP_AUTO_MAINTENANCE_CONFIG_ARGS`) and never compensated, so an + * Orca-driven checkout accumulates loose refs forever and every ref + * enumeration -- `show-ref`, `for-each-ref`, worktree create -- pays for them. + * This is the compensation: after a repo goes quiet, probe it, and pack only + * when the backlog is real. + * + * The engine is host-agnostic on purpose. The execution host owns everything + * that touches execution, so each host supplies its own target (which git to + * run, which filesystem to walk) and all state here is keyed per host. + */ + +/** + * Below this, ref enumeration is already fast and `pack-refs` would cost more + * than it saves. + * + * Git's own files-backend auto heuristic (2.47+) packs at + * `max(16, log2(packed_refs_bytes / 100) * 5)` loose refs -- about 76 for the + * 4.1 MB `packed-refs` that motivated this work. A flat 1000 is roughly an + * order of magnitude more conservative on purpose: this runs unasked against a + * real checkout, and being late is cheap where being wrong is not. + */ +export const LOOSE_REF_PACK_THRESHOLD = 1000 + +/** No fetch, create, or other tracked write on the repo for this long. */ +export const REF_MAINTENANCE_QUIET_PERIOD_MS = 10 * 60_000 + +/** Packing empties the backlog; there is nothing to do again for a long while. */ +export const REF_MAINTENANCE_PACKED_COOLDOWN_MS = 12 * 60 * 60_000 + +/** A healthy or unresolvable repo should not be re-probed on every quiet window. */ +export const REF_MAINTENANCE_CLEAN_COOLDOWN_MS = 6 * 60 * 60_000 + +/** A failing repo (permissions, stale lock) must not be retried in a loop. */ +export const REF_MAINTENANCE_FAILURE_COOLDOWN_MS = 6 * 60 * 60_000 + +/** + * A repository whose `packed-refs.lock` is a strand from our own dead process + * becomes reclaimable at `PACK_REFS_TIMEOUT_MS`, so retry near that rather than + * serving the full failure cooldown -- otherwise a Windows force-kill leaves + * every ref deletion in that repo failing for six hours instead of thirty + * minutes. + */ +export const REF_MAINTENANCE_LOCKED_COOLDOWN_MS = 30 * 60_000 + +/** + * `pack-refs` holds `packed-refs.lock` only while it rewrites the file -- + * measured at 0.03-1.37s of a 23-32s run, the other ~95% being the prune phase + * unlinking loose refs. A caller about to touch refs waits out that window + * instead of killing the pack. + */ +export const PACKED_REFS_LOCK_POLL_MS = 50 + +/** + * Ceiling on that wait. Past this we stop blocking the user and let Git's own + * retry (`core.filesRefLockTimeout`, `core.packedRefsTimeout`) handle it, which + * is what happens today without any of this. + */ +export const PACKED_REFS_LOCK_WAIT_MS = 5_000 + +/** + * `pack-refs --prune` unlinks one file per loose ref. Paying off a 36k-ref + * backlog measured at ~83s on APFS, so the deadline has to clear a cold repo on + * a slow disk by a wide margin. A kill mid-run is safe -- git renames + * `packed-refs` into place atomically and the surviving loose refs stay + * authoritative -- but it wastes the work. + */ +export const PACK_REFS_TIMEOUT_MS = 15 * 60_000 + +/** + * Ancient, safe on the Git 2.25 baseline, and does exactly one thing. + * + * Not `pack-refs --auto`: that arrived in 2.45 and unconditionally rewrote + * `packed-refs` on the files backend until 2.47, so it is both unavailable at + * our baseline and wrong on two shipped releases. Not `git maintenance run` + * either -- newer, and it pulls in commit-graph and repack work we did not ask + * for. `--all` is required because the backlog is `refs/heads` and + * `refs/remotes`, which a bare `pack-refs` leaves alone. + */ +export const PACK_REFS_ARGS = ['pack-refs', '--all', '--prune'] as const + +/** + * Backstop on a whole attempt: aborts it, rather than abandoning it. Every Git + * child is already deadlined, but an admission wait is not, and the whole app + * shares one maintenance slot. Abandoning would release that slot while a pack + * that may still hold `packed-refs.lock` runs on, so the deadline cancels the + * work instead and the slot is held until it really stops. + */ +export const REF_MAINTENANCE_ATTEMPT_DEADLINE_MS = PACK_REFS_TIMEOUT_MS + 5 * 60_000 + +export type RefMaintenanceOutcome = + | 'packed' + | 'below_threshold' + | 'unresolved' + | 'opted_out' + | 'deferred' + | 'interrupted' + | 'locked' + | 'timed_out' + | 'failed' + +/** Structurally satisfied by the tracer's `ActiveSpan`. */ +export type RefMaintenanceSpan = { + setAttribute(key: string, value: unknown): void +} + +export type RepoRefMaintenanceTarget = { + /** Repo identity scoped to its execution host; all state here is keyed by it. */ + readonly key: string + /** Absolute `refs/` path *on the host that runs the walk*, or undefined if unresolvable. */ + resolveRefsDirectory(signal: AbortSignal): Promise + /** A user who told Git not to auto-maintain this repo has told Orca too. */ + isOptedOut?(signal: AbortSignal): Promise + /** True while work on *this repo* is in flight -- a fetch, a create, a removal. */ + isBusy?(): boolean + /** + * Runs `pack-refs` to completion. Deliberately takes no abort signal: killing + * a pack is measurably worse than waiting for it (see `PACKED_REFS_LOCK_*`). + * It must report `packed-refs.lock` transitions through `lock` so callers can + * wait for the short window that actually blocks them. + */ + packRefs(lock: PackedRefsLockReporter): Promise +} + +/** How `packRefs` tells the scheduler whether the exclusive write window is open. */ +export type PackedRefsLockReporter = { + setHeld(held: boolean): void +} + +export type RepoRefMaintenanceOptions = { + now?: () => number + /** True while app-wide work this must not race is in flight (create, live agent, battery, quit). */ + isBusy?: () => boolean + /** Wraps one attempt so a host can trace it; must invoke and await `attempt`. */ + observe?: (attempt: (span: RefMaintenanceSpan) => Promise) => Promise + quietPeriodMs?: number + looseRefThreshold?: number + onError?: (error: unknown) => void +} + +/** Marks an abort Orca asked for, so the attempt is retried rather than blamed on the repo. */ +export class RefMaintenanceInterrupted extends Error { + constructor( + reason: string, + /** True when the attempt ran out of time rather than yielding to real work. */ + readonly deadline = false + ) { + super(`Ref maintenance interrupted: ${reason}`) + this.name = 'RefMaintenanceInterrupted' + } +} + +/** + * The repository's `packed-refs.lock` is held by something we must not touch. + * Distinct from a failure so a strand our own dead process left can be retried + * once it ages into reclaimability, rather than parked for six hours. + */ +export class RefMaintenanceRepoLocked extends Error { + constructor(detail: string) { + super(`packed-refs.lock is held: ${detail}`) + this.name = 'RefMaintenanceRepoLocked' + } +} diff --git a/src/shared/repo-ref-maintenance.test.ts b/src/shared/repo-ref-maintenance.test.ts new file mode 100644 index 00000000000..f59b894748f --- /dev/null +++ b/src/shared/repo-ref-maintenance.test.ts @@ -0,0 +1,530 @@ +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { RepoRefMaintenance } from './repo-ref-maintenance' +import { + RefMaintenanceRepoLocked, + REF_MAINTENANCE_PACKED_COOLDOWN_MS, + PACKED_REFS_LOCK_WAIT_MS, + type PackedRefsLockReporter, + type RefMaintenanceSpan, + type RepoRefMaintenanceOptions, + type RepoRefMaintenanceTarget +} from './repo-ref-maintenance-policy' + +const QUIET_MS = 1000 +const THRESHOLD = 5 +const roots: string[] = [] + +async function refsDirectoryWith(looseRefs: number): Promise { + const root = await mkdtemp(join(tmpdir(), 'orca-ref-maintenance-')) + roots.push(root) + const refs = join(root, 'refs', 'remotes', 'origin') + await mkdir(refs, { recursive: true }) + for (let index = 0; index < looseRefs; index += 1) { + await writeFile(join(refs, `ref-${index}`), 'a') + } + return join(root, 'refs') +} + +/** More directories than `countLooseRefs` will visit, but very few files. */ +async function saturatingRefsDirectory(): Promise { + const root = await mkdtemp(join(tmpdir(), 'orca-ref-maintenance-wide-')) + roots.push(root) + const refs = join(root, 'refs') + for (let index = 0; index < 4200; index += 1) { + await mkdir(join(refs, `ns-${index}`), { recursive: true }) + } + return refs +} + +/** Stands in for a pack that moved the refs into packed-refs before erroring. */ +async function emptyRefsDirectory(refs: string): Promise { + await rm(refs, { recursive: true, force: true }) +} + +function attributesOf(span: RefMaintenanceSpan): Record { + return (span as unknown as { recorded: Record }).recorded +} + +function recordingSpan(): RefMaintenanceSpan { + const recorded: Record = {} + return { + recorded, + setAttribute(key: string, value: unknown) { + recorded[key] = value + } + } as unknown as RefMaintenanceSpan +} + +type Harness = { + maintenance: RepoRefMaintenance + spans: RefMaintenanceSpan[] + packRefs: ((lock: PackedRefsLockReporter) => Promise) & { mock: { calls: unknown[] } } +} + +function createHarness( + overrides: Partial & { + packRefs?: (lock: PackedRefsLockReporter) => Promise + } = {} +): Harness { + const spans: RefMaintenanceSpan[] = [] + const packRefs = vi.fn<(lock: PackedRefsLockReporter) => Promise>( + overrides.packRefs ?? (async () => {}) + ) + const maintenance = new RepoRefMaintenance({ + quietPeriodMs: QUIET_MS, + looseRefThreshold: THRESHOLD, + now: () => Date.now(), + observe: (attempt) => { + const span = recordingSpan() + spans.push(span) + return attempt(span) + }, + ...overrides + }) + return { maintenance, spans, packRefs } +} + +function target( + key: string, + refsDirectory: string, + packRefs: (lock: PackedRefsLockReporter) => Promise, + extra: Partial = {} +): RepoRefMaintenanceTarget { + return { + key, + resolveRefsDirectory: async () => refsDirectory, + packRefs, + ...extra + } +} + +/** Resolves the first time the pack starts, so tests never race real filesystem I/O. */ +function packStartSignal(): { + started: Promise + onStart: (lock: PackedRefsLockReporter) => void +} { + let onStart: (lock: PackedRefsLockReporter) => void = () => {} + const started = new Promise((resolve) => { + onStart = resolve + }) + return { started, onStart } +} + +function yieldToIo(): Promise { + return new Promise((resolve) => setImmediate(resolve)) +} + +/** + * Spins the real event loop until `predicate` holds, so filesystem completions + * can land while `setTimeout` is faked. Bounded by wall clock rather than by a + * turn count: a loaded CI runner exhausts a fixed number of turns long before + * the I/O finishes, which fails as a confusing assertion somewhere else. + */ +async function until(predicate: () => boolean, what: string): Promise { + const deadline = Date.now() + 10_000 + while (!predicate() && Date.now() < deadline) { + await yieldToIo() + } + if (!predicate()) { + throw new Error(`timed out after 10s waiting for ${what}`) + } +} + +/** + * Like `until`, but for conditions that also need a scheduled retry to fire: + * spinning the real loop alone can never satisfy them, because `setTimeout` is + * faked. Alternates advancing the fake clock with yielding to real I/O. + */ +async function untilWithTimers(predicate: () => boolean, what: string): Promise { + const deadline = Date.now() + 10_000 + while (!predicate() && Date.now() < deadline) { + await vi.advanceTimersByTimeAsync(QUIET_MS) + await yieldToIo() + } + if (!predicate()) { + throw new Error(`timed out after 10s waiting for ${what}`) + } +} + +/** Fires the quiet-period timer and waits for the attempt it starts. */ +async function elapseQuietPeriod(maintenance: RepoRefMaintenance, periods = 1): Promise { + await vi.advanceTimersByTimeAsync(QUIET_MS * periods) + await maintenance.whenAttemptSettled() +} + +/** Only the quiet-period timer is faked; real filesystem I/O still has to complete. */ +beforeEach(() => { + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }) +}) + +afterEach(async () => { + vi.useRealTimers() + await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))) +}) + +describe('RepoRefMaintenance gating', () => { + it('packs only after the repo has been quiet for the full period', async () => { + const refs = await refsDirectoryWith(THRESHOLD + 2) + const { maintenance, spans, packRefs } = createHarness() + const repo = target('local::/repo/.git', refs, packRefs) + + maintenance.arm(repo) + await vi.advanceTimersByTimeAsync(QUIET_MS - 1) + expect(packRefs).not.toHaveBeenCalled() + + // A second write restarts the countdown rather than shortening it. + maintenance.arm(repo) + await vi.advanceTimersByTimeAsync(QUIET_MS - 1) + expect(packRefs).not.toHaveBeenCalled() + + await elapseQuietPeriod(maintenance) + expect(packRefs).toHaveBeenCalledTimes(1) + expect(attributesOf(spans[0])).toMatchObject({ + 'repo.maintenance_outcome': 'packed', + 'repo.maintenance_key': 'local::/repo/.git', + 'git.loose_ref_count': THRESHOLD + 1 + }) + }) + + it('leaves a healthy repository alone', async () => { + const refs = await refsDirectoryWith(THRESHOLD - 1) + const { maintenance, spans, packRefs } = createHarness() + + maintenance.arm(target('local::/healthy/.git', refs, packRefs)) + await elapseQuietPeriod(maintenance) + + expect(packRefs).not.toHaveBeenCalled() + expect(attributesOf(spans[0])).toMatchObject({ + 'repo.maintenance_outcome': 'below_threshold', + 'git.loose_ref_count': THRESHOLD - 1 + }) + }) + + it('does not run while the app is busy', async () => { + const refs = await refsDirectoryWith(THRESHOLD + 2) + let busy = true + const { maintenance, packRefs } = createHarness({ isBusy: () => busy }) + + maintenance.arm(target('local::/busy/.git', refs, packRefs)) + await elapseQuietPeriod(maintenance) + expect(packRefs).not.toHaveBeenCalled() + + // The deferral re-arms on a backed-off delay, so the next window picks it up. + busy = false + await elapseQuietPeriod(maintenance, 2) + expect(packRefs).toHaveBeenCalledTimes(1) + }) + + it('does not run while the repo itself has work in flight', async () => { + const refs = await refsDirectoryWith(THRESHOLD + 2) + const { maintenance, packRefs } = createHarness() + + maintenance.arm(target('local::/fetching/.git', refs, packRefs, { isBusy: () => true })) + await elapseQuietPeriod(maintenance) + + expect(packRefs).not.toHaveBeenCalled() + }) + + it('honours a user who disabled Git auto-maintenance for the repo', async () => { + const refs = await refsDirectoryWith(THRESHOLD + 2) + const { maintenance, spans, packRefs } = createHarness() + + maintenance.arm( + target('local::/opted-out/.git', refs, packRefs, { isOptedOut: async () => true }) + ) + await elapseQuietPeriod(maintenance) + + expect(packRefs).not.toHaveBeenCalled() + expect(attributesOf(spans[0])['repo.maintenance_outcome']).toBe('opted_out') + }) + + it('never reads a truncated walk as a clean repository', async () => { + const { maintenance, spans, packRefs } = createHarness() + + // A walk that stopped early reports a floor, so a low count is not evidence of health. + maintenance.arm({ + key: 'local::/saturated/.git', + resolveRefsDirectory: async () => saturatingRefsDirectory(), + packRefs + }) + await elapseQuietPeriod(maintenance) + + expect(packRefs).toHaveBeenCalledTimes(1) + expect(attributesOf(spans[0])['repo.maintenance_outcome']).toBe('packed') + }) + + it('records a repo whose packed-refs lock is held, and retries sooner than a failure', async () => { + const refs = await refsDirectoryWith(THRESHOLD + 2) + const { maintenance, spans } = createHarness() + + maintenance.arm( + target('local::/locked/.git', refs, async () => { + throw new RefMaintenanceRepoLocked('our own lock, not yet old enough to reclaim') + }) + ) + await elapseQuietPeriod(maintenance) + + expect(attributesOf(spans[0])['repo.maintenance_outcome']).toBe('locked') + }) + + it('skips a repository whose common dir cannot be resolved', async () => { + const { maintenance, spans, packRefs } = createHarness() + + maintenance.arm({ + key: 'local::/gone/.git', + resolveRefsDirectory: async () => undefined, + packRefs + }) + await elapseQuietPeriod(maintenance) + + expect(packRefs).not.toHaveBeenCalled() + expect(attributesOf(spans[0])['repo.maintenance_outcome']).toBe('unresolved') + }) +}) + +describe('RepoRefMaintenance single-flight and backoff', () => { + it('runs one repository at a time', async () => { + const refs = await refsDirectoryWith(THRESHOLD + 2) + let concurrent = 0 + let peak = 0 + const releases: (() => void)[] = [] + const { maintenance } = createHarness() + const slowPack = async (): Promise => { + concurrent += 1 + peak = Math.max(peak, concurrent) + await new Promise((resolve) => releases.push(resolve)) + concurrent -= 1 + } + + maintenance.arm(target('local::/a/.git', refs, slowPack)) + maintenance.arm(target('local::/b/.git', refs, slowPack)) + await vi.advanceTimersByTimeAsync(QUIET_MS) + await until(() => concurrent === 1, 'a pack to start') + expect(concurrent).toBe(1) + + releases.shift()?.() + await maintenance.whenAttemptSettled() + // The second repo was deferred behind the first, so its retry is on a timer. + await untilWithTimers(() => concurrent === 1, 'the second repo to start') + releases.shift()?.() + await maintenance.whenAttemptSettled() + + expect(peak).toBe(1) + expect(concurrent).toBe(0) + }) + + it('waits out the rewrite window instead of killing the pack', async () => { + const refs = await refsDirectoryWith(THRESHOLD + 2) + let finished = false + let release: (() => void) | undefined + const { maintenance } = createHarness() + const started = packStartSignal() + + maintenance.arm( + target('local::/yield/.git', refs, async (lock) => { + lock.setHeld(true) + started.onStart(lock) + await new Promise((resolve) => { + release = () => { + lock.setHeld(false) + resolve() + } + }) + finished = true + }) + ) + await vi.advanceTimersByTimeAsync(QUIET_MS) + await started.started + + let paused = false + void maintenance.pause('worktree-remove').then(() => { + paused = true + }) + await vi.advanceTimersByTimeAsync(1) + // Blocked while the rewrite window is open... + expect(paused).toBe(false) + expect(finished).toBe(false) + + release?.() + await until(() => paused, 'pause() to resolve') + // ...and released without the pack ever being cancelled. + expect(paused).toBe(true) + expect(finished).toBe(true) + }) + + it('gives up waiting on the lock rather than blocking the user indefinitely', async () => { + const refs = await refsDirectoryWith(THRESHOLD + 2) + const { maintenance } = createHarness() + const started = packStartSignal() + + maintenance.arm( + target('local::/stuck-lock/.git', refs, async (lock) => { + lock.setHeld(true) + started.onStart(lock) + await new Promise(() => {}) + }) + ) + await vi.advanceTimersByTimeAsync(QUIET_MS) + await started.started + + let paused = false + void maintenance.pause('git-fetch').then(() => { + paused = true + }) + await vi.advanceTimersByTimeAsync(PACKED_REFS_LOCK_WAIT_MS) + await until(() => paused, 'pause() to resolve') + + expect(paused).toBe(true) + }) + + it('reopens the window only when the last overlapping caller releases', async () => { + const refs = await refsDirectoryWith(THRESHOLD + 2) + const { maintenance, packRefs } = createHarness() + + const outer = await maintenance.pause('worktree-add') + const inner = await maintenance.pause('git-fetch') + maintenance.arm(target('local::/nested/.git', refs, packRefs)) + + await elapseQuietPeriod(maintenance, 8) + expect(packRefs).not.toHaveBeenCalled() + + inner() + await elapseQuietPeriod(maintenance, 8) + expect(packRefs).not.toHaveBeenCalled() + + outer() + await elapseQuietPeriod(maintenance, 8) + expect(packRefs).toHaveBeenCalledTimes(1) + }) + + it('restarts every armed countdown when the user does ref work themselves', async () => { + const refs = await refsDirectoryWith(THRESHOLD + 2) + const { maintenance, packRefs } = createHarness() + + const firstPack = packStartSignal() + const observed = target('local::/a/.git', refs, async (lock) => { + firstPack.onStart(lock) + await packRefs(lock) + }) + maintenance.arm(observed) + maintenance.arm(target('local::/b/.git', refs, packRefs)) + await vi.advanceTimersByTimeAsync(QUIET_MS - 1) + + // A manual fetch says the user is at the keyboard, so nothing may fire yet. + maintenance.postponeAll() + await vi.advanceTimersByTimeAsync(QUIET_MS - 1) + expect(packRefs).not.toHaveBeenCalled() + + await vi.advanceTimersByTimeAsync(QUIET_MS) + await firstPack.started + expect(packRefs).toHaveBeenCalled() + }) + + it('costs nothing when no pack is running', async () => { + const { maintenance } = createHarness() + + const release = await maintenance.pause('git-fetch') + release() + // Releasing twice must not leave the window wedged shut. + release() + const refs = await refsDirectoryWith(THRESHOLD + 2) + const { packRefs } = createHarness() + maintenance.arm(target('local::/free/.git', refs, packRefs)) + await elapseQuietPeriod(maintenance) + + expect(packRefs).toHaveBeenCalledTimes(1) + }) + + it('does not re-pack a repository inside its cooldown', async () => { + const refs = await refsDirectoryWith(THRESHOLD + 2) + let clock = 0 + const { maintenance, packRefs } = createHarness({ now: () => clock }) + const repo = target('local::/cooldown/.git', refs, packRefs) + + maintenance.arm(repo) + await elapseQuietPeriod(maintenance) + expect(packRefs).toHaveBeenCalledTimes(1) + + clock = REF_MAINTENANCE_PACKED_COOLDOWN_MS - 1 + maintenance.arm(repo) + await elapseQuietPeriod(maintenance) + expect(packRefs).toHaveBeenCalledTimes(1) + + clock = REF_MAINTENANCE_PACKED_COOLDOWN_MS + 1 + maintenance.arm(repo) + await elapseQuietPeriod(maintenance) + expect(packRefs).toHaveBeenCalledTimes(2) + }) + + it('counts a pack that could not lock every ref as a success', async () => { + // Field-observed on a machine running several Orca sessions: a branch moved + // mid-pack, Git reported an error, and 36,688 loose refs still became 3. + // Retrying that aggressively would be wrong -- the backlog is gone. + const refs = await refsDirectoryWith(THRESHOLD + 2) + const { maintenance, spans } = createHarness() + const repo = target('local::/raced/.git', refs, async () => { + await emptyRefsDirectory(refs) + throw new Error("error: cannot lock ref 'refs/heads/moved'") + }) + + maintenance.arm(repo) + await elapseQuietPeriod(maintenance) + + expect(attributesOf(spans[0])).toMatchObject({ + 'repo.maintenance_outcome': 'packed', + 'git.pack_refs_partial': true, + 'git.loose_ref_count_after': 0 + }) + + // And it serves the full post-pack cooldown rather than retrying. + maintenance.arm(repo) + await elapseQuietPeriod(maintenance) + expect(spans).toHaveLength(1) + }) + + it('records a failure when the pack left the backlog in place', async () => { + const refs = await refsDirectoryWith(THRESHOLD + 2) + const { maintenance, spans } = createHarness({ + packRefs: async () => { + throw new Error('permission denied') + } + }) + + maintenance.arm(target('local::/denied/.git', refs, () => Promise.reject(new Error('denied')))) + await elapseQuietPeriod(maintenance) + + expect(attributesOf(spans[0])['repo.maintenance_outcome']).toBe('failed') + }) + + it('records a failure instead of throwing, and backs off', async () => { + const refs = await refsDirectoryWith(THRESHOLD + 2) + const { maintenance, spans, packRefs } = createHarness({ + packRefs: async () => { + throw new Error('packed-refs.lock exists') + } + }) + const repo = target('local::/failing/.git', refs, packRefs) + + maintenance.arm(repo) + await elapseQuietPeriod(maintenance) + expect(attributesOf(spans[0])['repo.maintenance_outcome']).toBe('failed') + + maintenance.arm(repo) + await elapseQuietPeriod(maintenance) + expect(packRefs).toHaveBeenCalledTimes(1) + }) + + it('stops scheduling once disposed', async () => { + const refs = await refsDirectoryWith(THRESHOLD + 2) + const { maintenance, packRefs } = createHarness() + + maintenance.arm(target('local::/disposed/.git', refs, packRefs)) + maintenance.dispose() + await elapseQuietPeriod(maintenance) + + expect(packRefs).not.toHaveBeenCalled() + }) +}) diff --git a/src/shared/repo-ref-maintenance.ts b/src/shared/repo-ref-maintenance.ts new file mode 100644 index 00000000000..97e228fab52 --- /dev/null +++ b/src/shared/repo-ref-maintenance.ts @@ -0,0 +1,366 @@ +import { countLooseRefs } from './loose-ref-count' +import { PackedRefsLockGate } from './packed-refs-lock-gate' +import { + LOOSE_REF_PACK_THRESHOLD, + REF_MAINTENANCE_ATTEMPT_DEADLINE_MS, + REF_MAINTENANCE_CLEAN_COOLDOWN_MS, + REF_MAINTENANCE_FAILURE_COOLDOWN_MS, + REF_MAINTENANCE_PACKED_COOLDOWN_MS, + REF_MAINTENANCE_QUIET_PERIOD_MS, + PACKED_REFS_LOCK_WAIT_MS, + REF_MAINTENANCE_LOCKED_COOLDOWN_MS, + RefMaintenanceInterrupted, + RefMaintenanceRepoLocked, + type RefMaintenanceOutcome, + type RefMaintenanceSpan, + type RepoRefMaintenanceOptions, + type RepoRefMaintenanceTarget +} from './repo-ref-maintenance-policy' + +/** + * The scheduler half of idle loose-ref packing: when to probe, when to pack, + * when to stand down. The thresholds and the host contract it works against + * live in `./repo-ref-maintenance-policy`. + */ + +/** Give up until the next real activity rather than re-arming forever. */ +const MAX_DEFERRALS = 6 +/** Each deferral doubles the wait, so a busy app is retried rarely, not hammered. */ +const MAX_DEFERRAL_BACKOFF_MULTIPLIER = 8 +/** Armed repos are evicted oldest-first past this; the next write on one re-arms it. */ +const MAX_TRACKED_REPOS = 64 + +type TrackedRepo = { + target: RepoRefMaintenanceTarget + timer: ReturnType | null + deferrals: number +} + +const noopSpan: RefMaintenanceSpan = { setAttribute: () => {} } + +/** A deadline means something is stuck: back off instead of retrying straight away. */ +function hitDeadline(signal: AbortSignal): boolean { + return signal.reason instanceof RefMaintenanceInterrupted && signal.reason.deadline +} + +export class RepoRefMaintenance { + private readonly tracked = new Map() + private readonly cooldownUntil = new Map() + private readonly now: () => number + private readonly isAppBusy: () => boolean + private readonly observe: NonNullable + private readonly quietPeriodMs: number + private readonly looseRefThreshold: number + private readonly onError: (error: unknown) => void + // Why: at most one pack-refs anywhere. It holds a general git admission slot + // for its whole run, and two at once would halve git throughput on a small host. + // The slot is never released while a pack that could hold `packed-refs.lock` + // is still running -- an interrupt cancels the work and waits for it to stop. + private inFlight: Promise | null = null + private inFlightAbort: AbortController | null = null + private readonly lockGate = new PackedRefsLockGate() + // Why a count, not a flag: several ref-touching operations overlap routinely + // (a create's fetch inside a create), and the last one out reopens the window. + private suspensions = 0 + private lastAttempt: Promise = Promise.resolve() + private disposed = false + + constructor(options: RepoRefMaintenanceOptions = {}) { + this.now = options.now ?? Date.now + this.isAppBusy = options.isBusy ?? (() => false) + this.observe = options.observe ?? ((attempt) => attempt(noopSpan)) + this.quietPeriodMs = options.quietPeriodMs ?? REF_MAINTENANCE_QUIET_PERIOD_MS + this.looseRefThreshold = options.looseRefThreshold ?? LOOSE_REF_PACK_THRESHOLD + this.onError = options.onError ?? (() => {}) + } + + /** + * Record a write to `target`'s repo and (re)start its quiet-period countdown. + * Every call pushes the attempt further out, so a burst of fetches or a + * worktree create can never be interrupted by maintenance it triggered. + */ + arm(target: RepoRefMaintenanceTarget): void { + if (this.disposed) { + return + } + const existing = this.tracked.get(target.key) + if (existing?.timer) { + clearTimeout(existing.timer) + } + const tracked: TrackedRepo = { target, timer: null, deferrals: existing?.deferrals ?? 0 } + this.tracked.delete(target.key) + this.evictOldestBeyondCap() + this.tracked.set(target.key, tracked) + this.schedule(target.key, tracked) + } + + /** Resolves once the attempt started by the most recent timer has settled. */ + whenAttemptSettled(): Promise { + return this.lastAttempt + } + + /** + * Wait out the `packed-refs` rewrite, if one is in progress. + * + * Deliberately not a kill. The lock is held for 0.03-1.37s of a 23-32s pack; + * the rest is the prune phase, during which a concurrent `fetch --prune`, + * `branch -D` or `update-ref` measurably succeeds because per-ref locks last + * microseconds and Git retries for `core.filesRefLockTimeout`. Signalling the + * child there buys nothing and strands a lock file about one time in five. + * + * Free when no pack is running, which is almost always. + */ + awaitPackedRefsLockRelease(): Promise { + return this.lockGate.whenReleased(PACKED_REFS_LOCK_WAIT_MS) + } + + /** + * Push every armed repository's attempt out by a full quiet period. + * + * User-initiated ref work is evidence the user is active in the app, not just + * in one repo, and it is free -- no key to resolve, no subprocess, nothing at + * all when nothing is armed. + */ + postponeAll(): void { + if (this.disposed) { + return + } + for (const [key, tracked] of this.tracked) { + if (tracked.timer) { + clearTimeout(tracked.timer) + } + tracked.deferrals = 0 + this.schedule(key, tracked) + } + } + + /** + * Hold the repository open for work that is about to touch refs. + * + * Two things at once: no *new* attempt can start for any repository until the + * returned release is called, and the caller waits out any `packed-refs` + * rewrite already in progress. A prune already running is left alone to + * finish -- it does not block the caller. + */ + async pause(_reason: string): Promise<() => void> { + this.suspensions += 1 + let released = false + try { + await this.awaitPackedRefsLockRelease() + } catch { + // The wait cannot reject, but a release must exist even if it did. + } + return () => { + if (!released) { + released = true + this.suspensions -= 1 + } + } + } + + dispose(): void { + this.disposed = true + this.inFlightAbort?.abort(new RefMaintenanceInterrupted('disposed')) + for (const tracked of this.tracked.values()) { + if (tracked.timer) { + clearTimeout(tracked.timer) + } + } + this.tracked.clear() + this.cooldownUntil.clear() + } + + private isBusy(tracked: TrackedRepo): boolean { + return this.isAppBusy() || (tracked.target.isBusy?.() ?? false) + } + + private schedule(key: string, tracked: TrackedRepo, delayMs = this.quietPeriodMs): void { + const timer = setTimeout(() => { + tracked.timer = null + this.lastAttempt = this.attempt(key).catch((error) => this.onError(error)) + }, delayMs) + // Never hold the process open for maintenance. + timer.unref?.() + tracked.timer = timer + } + + private evictOldestBeyondCap(): void { + while (this.tracked.size >= MAX_TRACKED_REPOS) { + const oldest = this.tracked.keys().next() + if (oldest.done) { + return + } + const evicted = this.tracked.get(oldest.value) + if (evicted?.timer) { + clearTimeout(evicted.timer) + } + this.tracked.delete(oldest.value) + } + } + + /** + * `counted` spends the give-up budget. Waiting behind another repository's + * pack, or yielding to work Orca asked us to yield to, does not: both end on + * their own, so charging for them would let a busy machine starve a repo + * until its next fetch. Only "the app is busy" is charged. + */ + private defer(key: string, tracked: TrackedRepo, counted: boolean): void { + // A fetch that landed while this attempt was probing already re-armed the + // repo; that entry is fresher, so the deferral must not overwrite it. + if (this.disposed || this.tracked.has(key)) { + return + } + if (counted) { + if (tracked.deferrals >= MAX_DEFERRALS) { + return + } + tracked.deferrals += 1 + } + this.tracked.set(key, tracked) + const multiplier = Math.min(2 ** tracked.deferrals, MAX_DEFERRAL_BACKOFF_MULTIPLIER) + this.schedule(key, tracked, this.quietPeriodMs * multiplier) + } + + private async attempt(key: string): Promise { + const tracked = this.tracked.get(key) + if (!tracked || this.disposed) { + return + } + this.tracked.delete(key) + const cooldownUntil = this.cooldownUntil.get(key) + if (cooldownUntil !== undefined && this.now() < cooldownUntil) { + return + } + if (this.inFlight !== null) { + this.defer(key, tracked, false) + return + } + if (this.suspensions > 0 || this.isBusy(tracked)) { + this.defer(key, tracked, true) + return + } + const abort = new AbortController() + const deadline = setTimeout( + () => abort.abort(new RefMaintenanceInterrupted('attempt deadline', true)), + REF_MAINTENANCE_ATTEMPT_DEADLINE_MS + ) + deadline.unref?.() + const run = this.observe((span) => this.packIfNeeded(key, tracked, span, abort.signal)) + this.inFlight = run + this.inFlightAbort = abort + try { + await run + } finally { + clearTimeout(deadline) + if (this.inFlight === run) { + this.inFlight = null + this.inFlightAbort = null + } + } + } + + private async packIfNeeded( + key: string, + tracked: TrackedRepo, + span: RefMaintenanceSpan, + signal: AbortSignal + ): Promise { + span.setAttribute('repo.maintenance_key', key) + // Every await below carries the signal, so a caller waiting in `pause()` is + // never stuck behind a probe that has already been told to stop. + if (await tracked.target.isOptedOut?.(signal)) { + this.settle(key, span, 'opted_out', REF_MAINTENANCE_CLEAN_COOLDOWN_MS) + return + } + if (signal.aborted) { + this.yieldTo(key, tracked, span, signal) + return + } + const refsDirectory = await tracked.target.resolveRefsDirectory(signal) + if (!refsDirectory) { + this.settle(key, span, 'unresolved', REF_MAINTENANCE_CLEAN_COOLDOWN_MS) + return + } + const budget = this.looseRefThreshold + 1 + const before = await countLooseRefs(refsDirectory, budget, signal) + if (signal.aborted) { + this.yieldTo(key, tracked, span, signal) + return + } + span.setAttribute('git.loose_ref_count', before.count) + span.setAttribute('git.loose_ref_threshold', this.looseRefThreshold) + // A saturated walk stopped early, so `count` is a floor -- never read it as "clean". + if (!before.saturated && before.count < this.looseRefThreshold) { + this.settle(key, span, 'below_threshold', REF_MAINTENANCE_CLEAN_COOLDOWN_MS) + return + } + // The quiet window can close while the probe walks; re-check before spending a git slot. + if (this.suspensions > 0 || this.isBusy(tracked)) { + span.setAttribute('repo.maintenance_outcome', 'deferred' satisfies RefMaintenanceOutcome) + this.defer(key, tracked, true) + return + } + const startedAt = this.now() + let partial = false + try { + // No signal: the pack runs to completion. Callers that need the refs wait + // out the rewrite window through `pause()` instead of killing it. + await tracked.target.packRefs(this.lockGate) + } catch (error) { + span.setAttribute('repo.maintenance_error', String(error)) + if (error instanceof RefMaintenanceRepoLocked) { + this.settle(key, span, 'locked', REF_MAINTENANCE_LOCKED_COOLDOWN_MS) + return + } + partial = true + } finally { + this.lockGate.setHeld(false) + } + span.setAttribute('git.pack_refs_ms', this.now() - startedAt) + // Judge by the backlog, not by the exit code. On a machine running several + // Orca sessions a branch moving mid-pack is the normal case, and Git's + // response -- leave that one ref loose, pack the rest -- is the correct one. + // Measured in the field: 36,688 loose refs down to 3, reported as an error. + const after = await countLooseRefs(refsDirectory, budget, signal) + span.setAttribute('git.loose_ref_count_after', after.count) + if (partial && (after.saturated || after.count >= this.looseRefThreshold)) { + this.settle(key, span, 'failed', REF_MAINTENANCE_FAILURE_COOLDOWN_MS) + return + } + span.setAttribute('git.pack_refs_partial', partial) + this.settle(key, span, 'packed', REF_MAINTENANCE_PACKED_COOLDOWN_MS) + } + + /** Record an aborted attempt: retry soon if Orca yielded, back off if it stalled. */ + private yieldTo( + key: string, + tracked: TrackedRepo, + span: RefMaintenanceSpan, + signal: AbortSignal + ): void { + if (hitDeadline(signal)) { + this.settle(key, span, 'timed_out', REF_MAINTENANCE_FAILURE_COOLDOWN_MS) + return + } + span.setAttribute('repo.maintenance_outcome', 'interrupted' satisfies RefMaintenanceOutcome) + this.defer(key, tracked, false) + } + + private settle( + key: string, + span: RefMaintenanceSpan, + outcome: RefMaintenanceOutcome, + cooldownMs: number + ): void { + span.setAttribute('repo.maintenance_outcome', outcome) + // Re-insert so Map order stays newest-last and the eviction below drops the oldest. + this.cooldownUntil.delete(key) + this.cooldownUntil.set(key, this.now() + cooldownMs) + if (this.cooldownUntil.size > MAX_TRACKED_REPOS * 4) { + const oldest = this.cooldownUntil.keys().next() + if (!oldest.done) { + this.cooldownUntil.delete(oldest.value) + } + } + } +} From 0352c239c249a355a26c132b9aeb071b55bc66fb Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Tue, 1 Sep 2026 19:20:48 -0700 Subject: [PATCH 3/3] Add Copy Session ID menu item to terminal tabs (#18039) * Add Copy Session ID menu item to terminal tabs Adds a menu item to copy the active pane's agent session ID when available. The item only appears when the session is still live and has reported an ID. * Add Copy Session ID i18n strings and e2e test - Add localized strings for Session ID context menu item - Add e2e test coverage for copying session ID from terminal tabs - Fix dev build permissions when copying private Electron app bundles * Drop the Electron dev-bundle fix from this branch It landed on main as 519af49a58, which restores write permission inside copyPrivateTree itself rather than at the dev runner's call site, so every caller of the private-copy contract is covered and not just this one. That commit also fixes the test that should have caught the crash: the wrapper ran with stdio: 'ignore', so a hard failure presented as a bare timeout. This branch predated that commit and carried a narrower duplicate, mixed into an i18n/e2e commit where it did not belong. * refactor: use dedicated i18n keys for copy session ID toasts Replace auto-generated translation keys with specific, dedicated keys for copy session ID success and error messages. This improves maintainability and makes the strings easier to translate across all supported languages. --- .../tab-bar/SortableTabContextMenu.test.tsx | 59 +++++++ .../tab-bar/SortableTabContextMenu.tsx | 7 + .../TabAgentSessionIdMenuItem.test.tsx | 79 +++++++++ .../tab-bar/TabAgentSessionIdMenuItem.tsx | 48 ++++++ .../tab-bar/tab-agent-session-id.test.ts | 159 ++++++++++++++++++ .../tab-bar/tab-agent-session-id.ts | 32 ++++ .../tab-context-menu-consistency.test.tsx | 1 + src/renderer/src/i18n/locales/en.json | 5 +- src/renderer/src/i18n/locales/es.json | 5 +- src/renderer/src/i18n/locales/ja.json | 5 +- src/renderer/src/i18n/locales/ko.json | 5 +- src/renderer/src/i18n/locales/zh.json | 5 +- tests/e2e/tab-context-menu-session-id.spec.ts | 76 +++++++++ 13 files changed, 481 insertions(+), 5 deletions(-) create mode 100644 src/renderer/src/components/tab-bar/TabAgentSessionIdMenuItem.test.tsx create mode 100644 src/renderer/src/components/tab-bar/TabAgentSessionIdMenuItem.tsx create mode 100644 src/renderer/src/components/tab-bar/tab-agent-session-id.test.ts create mode 100644 src/renderer/src/components/tab-bar/tab-agent-session-id.ts create mode 100644 tests/e2e/tab-context-menu-session-id.spec.ts diff --git a/src/renderer/src/components/tab-bar/SortableTabContextMenu.test.tsx b/src/renderer/src/components/tab-bar/SortableTabContextMenu.test.tsx index 76d5d8d145a..e51c599f669 100644 --- a/src/renderer/src/components/tab-bar/SortableTabContextMenu.test.tsx +++ b/src/renderer/src/components/tab-bar/SortableTabContextMenu.test.tsx @@ -55,6 +55,7 @@ vi.mock('lucide-react', () => ({ ArrowRight: () => null, ArrowUp: () => null, Columns2: () => null, + Copy: () => null, ListX: () => null, MessageSquare: () => null, PanelBottomClose: () => null, @@ -71,6 +72,8 @@ vi.mock('@/i18n/i18n', () => ({ translate: (_key: string, fallback: string) => fallback })) +vi.mock('sonner', () => ({ toast: { success: vi.fn(), error: vi.fn() } })) + vi.mock('../../store', () => ({ useAppStore: Object.assign( (selector: (state: Record) => unknown) => selector(storeMock.state), @@ -289,4 +292,60 @@ describe('SortableTabContextMenu', () => { expect(container.textContent).not.toContain('Move Tab to Split') expect(container.textContent).toContain('Split terminal right') }) + + describe('copy session id', () => { + const LEAF = '11111111-1111-4111-8111-111111111111' + + function withLiveAgent(sessionId: string | null): void { + storeMock.state = { + ...storeMock.state, + terminalLayoutsByTabId: { + 'term-1': { root: { type: 'leaf', leafId: LEAF }, activeLeafId: LEAF } + }, + agentStatusByPaneKey: { + [`term-1:${LEAF}`]: { + state: 'done', + prompt: '', + updatedAt: 1, + stateStartedAt: 1, + paneKey: `term-1:${LEAF}`, + agentType: 'claude', + stateHistory: [], + ...(sessionId ? { providerSession: { key: 'session_id', id: sessionId } } : {}) + } + }, + paneForegroundAgentByPaneKey: {} + } + } + + it('omits the item for a tab with no agent', () => { + const { container } = renderMenu() + + expect(container.textContent).not.toContain('Copy Session ID') + }) + + it('omits the item until the active agent reports a session id', () => { + withLiveAgent(null) + const { container } = renderMenu() + + expect(container.textContent).not.toContain('Copy Session ID') + }) + + it('copies the active pane session id', async () => { + const writeClipboardText = vi.fn().mockResolvedValue(undefined) + Object.assign(window, { api: { ui: { writeClipboardText } } }) + withLiveAgent('session-abc') + const { container } = renderMenu() + + act(() => getButton(container, 'Copy Session ID').click()) + await vi.waitFor(() => expect(writeClipboardText).toHaveBeenCalledWith('session-abc')) + }) + + it('does not resolve a session id while the menu is closed', () => { + withLiveAgent('session-abc') + const { container } = renderMenu({ open: false }) + + expect(container.textContent).not.toContain('Copy Session ID') + }) + }) }) diff --git a/src/renderer/src/components/tab-bar/SortableTabContextMenu.tsx b/src/renderer/src/components/tab-bar/SortableTabContextMenu.tsx index 53b31012d39..b298072f9f6 100644 --- a/src/renderer/src/components/tab-bar/SortableTabContextMenu.tsx +++ b/src/renderer/src/components/tab-bar/SortableTabContextMenu.tsx @@ -11,6 +11,8 @@ import type { TerminalTab } from '../../../../shared/terminal-tab-types' import { useAppStore } from '../../store' import { formatShortcutLabel, useOptionalShortcutLabel } from '@/hooks/useShortcutLabel' import { translate } from '@/i18n/i18n' +import { TabAgentSessionIdMenuItem } from './TabAgentSessionIdMenuItem' +import { resolveTabAgentSessionId } from './tab-agent-session-id' import { TerminalTabSplitMenuSection } from './TerminalTabSplitMenuSection' import { TAB_CONTEXT_MENU_CONTENT_CLASS } from './tab-context-menu-sizing' @@ -121,6 +123,10 @@ export function SortableTabContextMenu({ onTogglePin }: SortableTabContextMenuProps): React.JSX.Element { const keybindings = useAppStore((state) => state.keybindings) + // The id is a primitive, so unchanged sessions stay referentially stable without a cache. + const agentSessionId = useAppStore((state) => + open ? resolveTabAgentSessionId(state, tab.id) : null + ) const splitRightShortcut = formatShortcutLabel('terminal.splitRight', keybindings) const splitDownShortcut = formatShortcutLabel('terminal.splitDown', keybindings) @@ -188,6 +194,7 @@ export function SortableTabContextMenu({ {translate('auto.components.tab.bar.SortableTabContextMenu.2f697b3c31', 'Change Title')} {renameShortcut ? {renameShortcut} : null} +
{translate('auto.components.tab.bar.SortableTabContextMenu.35e8892fd0', 'Tab Color')} diff --git a/src/renderer/src/components/tab-bar/TabAgentSessionIdMenuItem.test.tsx b/src/renderer/src/components/tab-bar/TabAgentSessionIdMenuItem.test.tsx new file mode 100644 index 00000000000..da857ec57bf --- /dev/null +++ b/src/renderer/src/components/tab-bar/TabAgentSessionIdMenuItem.test.tsx @@ -0,0 +1,79 @@ +/** + * @vitest-environment happy-dom + */ +import { act, type ReactNode } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { TabAgentSessionIdMenuItem } from './TabAgentSessionIdMenuItem' + +const toastMock = vi.hoisted(() => ({ success: vi.fn(), error: vi.fn() })) + +vi.mock('@/components/ui/dropdown-menu', () => ({ + DropdownMenuItem: ({ + children, + disabled, + onSelect, + 'aria-label': ariaLabel + }: { + children?: ReactNode + disabled?: boolean + onSelect?: () => void + 'aria-label'?: string + }) => ( + + ) +})) + +vi.mock('lucide-react', () => ({ Copy: () => null })) +vi.mock('@/i18n/i18n', () => ({ translate: (_key: string, fallback: string) => fallback })) +vi.mock('sonner', () => ({ toast: toastMock })) + +const mounted: { container: HTMLDivElement; root: Root }[] = [] + +function render(sessionId: string | null): HTMLDivElement { + const container = document.createElement('div') + document.body.appendChild(container) + const root = createRoot(container) + act(() => root.render()) + mounted.push({ container, root }) + return container +} + +afterEach(() => { + for (const { container, root } of mounted.splice(0)) { + act(() => root.unmount()) + container.remove() + } + toastMock.success.mockReset() + toastMock.error.mockReset() +}) + +describe('TabAgentSessionIdMenuItem', () => { + it('renders nothing when no session id is available', () => { + expect(render(null).textContent).toBe('') + }) + + it('copies on select when an id is known', async () => { + const writeClipboardText = vi.fn().mockResolvedValue(undefined) + Object.assign(window, { api: { ui: { writeClipboardText } } }) + const container = render('abc-123') + + const button = container.querySelector('button') + expect(button?.disabled).toBe(false) + act(() => button?.click()) + await vi.waitFor(() => expect(writeClipboardText).toHaveBeenCalledWith('abc-123')) + }) + + it('reports clipboard failures', async () => { + const writeClipboardText = vi.fn().mockRejectedValue(new Error('clipboard unavailable')) + Object.assign(window, { api: { ui: { writeClipboardText } } }) + const button = render('abc-123').querySelector('button') + + act(() => button?.click()) + await vi.waitFor(() => + expect(toastMock.error).toHaveBeenCalledWith('Failed to copy Session ID') + ) + }) +}) diff --git a/src/renderer/src/components/tab-bar/TabAgentSessionIdMenuItem.tsx b/src/renderer/src/components/tab-bar/TabAgentSessionIdMenuItem.tsx new file mode 100644 index 00000000000..73f3f128e5d --- /dev/null +++ b/src/renderer/src/components/tab-bar/TabAgentSessionIdMenuItem.tsx @@ -0,0 +1,48 @@ +import { Copy } from 'lucide-react' +import { toast } from 'sonner' +import { DropdownMenuItem } from '@/components/ui/dropdown-menu' +import { translate } from '@/i18n/i18n' + +async function copySessionId(sessionId: string): Promise { + try { + await window.api.ui.writeClipboardText(sessionId) + toast.success( + translate( + 'components.tab.bar.SortableTabContextMenu.copySessionIdSuccess', + 'Session ID copied' + ) + ) + } catch { + toast.error( + translate( + 'components.tab.bar.SortableTabContextMenu.copySessionIdError', + 'Failed to copy Session ID' + ) + ) + } +} + +/** Copies the active pane's provider session id when one is available. */ +export function TabAgentSessionIdMenuItem({ + sessionId +}: { + sessionId: string | null +}): React.JSX.Element | null { + if (sessionId === null) { + return null + } + const label = translate( + 'components.tab.bar.SortableTabContextMenu.copySessionId', + 'Copy Session ID' + ) + return ( + { + void copySessionId(sessionId) + }} + > + + {label} + + ) +} diff --git a/src/renderer/src/components/tab-bar/tab-agent-session-id.test.ts b/src/renderer/src/components/tab-bar/tab-agent-session-id.test.ts new file mode 100644 index 00000000000..2f06f0a8db7 --- /dev/null +++ b/src/renderer/src/components/tab-bar/tab-agent-session-id.test.ts @@ -0,0 +1,159 @@ +import { describe, expect, it } from 'vitest' +import type { AgentStatusEntry } from '../../../../shared/agent-status-types' +import { resolveTabAgentSessionId, type TabAgentSessionIdState } from './tab-agent-session-id' + +const LEAF_A = '11111111-1111-4111-8111-111111111111' +const LEAF_B = '22222222-2222-4222-8222-222222222222' + +function entry(overrides: Partial = {}): AgentStatusEntry { + return { + state: 'done', + prompt: '', + updatedAt: 1, + stateStartedAt: 1, + paneKey: `tab-1:${LEAF_A}`, + agentType: 'claude', + stateHistory: [], + ...overrides + } +} + +function state(overrides: Partial = {}): TabAgentSessionIdState { + return { + terminalLayoutsByTabId: { + 'tab-1': { + root: { type: 'leaf', leafId: LEAF_A }, + activeLeafId: LEAF_A, + expandedLeafId: null + } + }, + agentStatusByPaneKey: {}, + paneForegroundAgentByPaneKey: {}, + ...overrides + } +} + +describe('resolveTabAgentSessionId', () => { + it('is absent when the pane has no agent row', () => { + expect(resolveTabAgentSessionId(state(), 'tab-1')).toBeNull() + }) + + it('is absent for a tab with no layout', () => { + expect(resolveTabAgentSessionId(state(), 'tab-missing')).toBeNull() + }) + + it('reads the id reported by the active pane', () => { + const resolved = resolveTabAgentSessionId( + state({ + agentStatusByPaneKey: { + [`tab-1:${LEAF_A}`]: entry({ providerSession: { key: 'session_id', id: 'abc-123' } }) + } + }), + 'tab-1' + ) + expect(resolved).toBe('abc-123') + }) + + it('is absent until the agent reports an id', () => { + const resolved = resolveTabAgentSessionId( + state({ agentStatusByPaneKey: { [`tab-1:${LEAF_A}`]: entry() } }), + 'tab-1' + ) + expect(resolved).toBeNull() + }) + + describe('liveness', () => { + it('is absent for a hydrated row with no live hook since restore', () => { + const resolved = resolveTabAgentSessionId( + state({ + agentStatusByPaneKey: { + [`tab-1:${LEAF_A}`]: entry({ + restoredUnconfirmed: true, + providerSession: { key: 'session_id', id: 'abc-123' } + }) + } + }), + 'tab-1' + ) + expect(resolved).toBeNull() + }) + + it('is absent once the pane is proven back at the shell', () => { + const resolved = resolveTabAgentSessionId( + state({ + agentStatusByPaneKey: { + [`tab-1:${LEAF_A}`]: entry({ providerSession: { key: 'session_id', id: 'abc-123' } }) + }, + paneForegroundAgentByPaneKey: { + [`tab-1:${LEAF_A}`]: { agent: null, shellForeground: true } + } + }), + 'tab-1' + ) + expect(resolved).toBeNull() + }) + + it('keeps a session whose foreground evidence is only that an agent runs', () => { + const resolved = resolveTabAgentSessionId( + state({ + agentStatusByPaneKey: { + [`tab-1:${LEAF_A}`]: entry({ providerSession: { key: 'session_id', id: 'abc-123' } }) + }, + paneForegroundAgentByPaneKey: { + [`tab-1:${LEAF_A}`]: { agent: 'claude', shellForeground: false } + } + }), + 'tab-1' + ) + expect(resolved).toBe('abc-123') + }) + + it('keeps a working session that reported a session boundary', () => { + // Why: sessionBoundary marks a resume/clear landing idle — a session start, + // not a session end, and exactly when the first id arrives. + const resolved = resolveTabAgentSessionId( + state({ + agentStatusByPaneKey: { + [`tab-1:${LEAF_A}`]: entry({ + sessionBoundary: true, + providerSession: { key: 'session_id', id: 'fresh-1' } + }) + } + }), + 'tab-1' + ) + expect(resolved).toBe('fresh-1') + }) + }) + + describe('split tabs', () => { + const splitState = (activeLeafId: string): TabAgentSessionIdState => + state({ + terminalLayoutsByTabId: { + 'tab-1': { + root: { + type: 'split', + direction: 'vertical', + first: { type: 'leaf', leafId: LEAF_A }, + second: { type: 'leaf', leafId: LEAF_B } + }, + activeLeafId, + expandedLeafId: null + } + }, + agentStatusByPaneKey: { + [`tab-1:${LEAF_A}`]: entry({ providerSession: { key: 'session_id', id: 'left' } }), + [`tab-1:${LEAF_B}`]: entry({ providerSession: { key: 'session_id', id: 'right' } }) + } + }) + + it('reads the active pane, not a sibling', () => { + expect(resolveTabAgentSessionId(splitState(LEAF_B), 'tab-1')).toBe('right') + }) + + it('is absent when the active leaf id no longer exists in the layout', () => { + const stale = '33333333-3333-4333-8333-333333333333' + expect(resolveTabAgentSessionId(splitState(stale), 'tab-1')).toBeNull() + }) + }) +}) diff --git a/src/renderer/src/components/tab-bar/tab-agent-session-id.ts b/src/renderer/src/components/tab-bar/tab-agent-session-id.ts new file mode 100644 index 00000000000..e0d804bc4c5 --- /dev/null +++ b/src/renderer/src/components/tab-bar/tab-agent-session-id.ts @@ -0,0 +1,32 @@ +import type { AgentStatusEntry } from '../../../../shared/agent-status-types' +import type { TerminalLayoutSnapshot } from '../../../../shared/terminal-tab-types' +import type { PaneForegroundAgentEntry } from '../../store/slices/pane-foreground-agent' +import { resolveNativeChatActiveLayoutLeafId } from '../native-chat/native-chat-leaf-routing' + +export type TabAgentSessionIdState = { + agentStatusByPaneKey?: Record + terminalLayoutsByTabId?: Record + paneForegroundAgentByPaneKey?: Record +} + +/** Returns the active pane's provider session id when its agent is still live. */ +export function resolveTabAgentSessionId( + state: TabAgentSessionIdState, + tabId: string +): string | null { + const leafId = resolveNativeChatActiveLayoutLeafId(state.terminalLayoutsByTabId?.[tabId]) + if (!leafId) { + return null + } + const paneKey = `${tabId}:${leafId}` + const entry = state.agentStatusByPaneKey?.[paneKey] + // Hydrated rows may describe a session that ended while no receiver was up. + if (!entry?.agentType || entry.restoredUnconfirmed === true) { + return null + } + // OSC 133;D proves the pane is back at the shell, regardless of the last hook state. + if (state.paneForegroundAgentByPaneKey?.[paneKey]?.shellForeground === true) { + return null + } + return entry.providerSession?.id ?? null +} diff --git a/src/renderer/src/components/tab-bar/tab-context-menu-consistency.test.tsx b/src/renderer/src/components/tab-bar/tab-context-menu-consistency.test.tsx index c81eae348dd..54f762d4611 100644 --- a/src/renderer/src/components/tab-bar/tab-context-menu-consistency.test.tsx +++ b/src/renderer/src/components/tab-bar/tab-context-menu-consistency.test.tsx @@ -13,6 +13,7 @@ import { const TAB_MENU_SOURCES = [ 'EditorFileTabContextMenu.tsx', 'SortableTabContextMenu.tsx', + 'TabAgentSessionIdMenuItem.tsx', 'BrowserTab.tsx', 'TabWorkspaceLayoutMenuSection.tsx', 'TerminalTabSplitMenuSection.tsx' diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 7fc9c837ebc..22b1695cc3d 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -16724,7 +16724,10 @@ "SortableTabContextMenu": { "switchToTerminalView": "Switch to terminal view", "switchToChatView": "Switch to chat view", - "closeTabsToLeft": "Close Tabs To The Left" + "closeTabsToLeft": "Close Tabs To The Left", + "copySessionId": "Copy Session ID", + "copySessionIdSuccess": "Session ID copied", + "copySessionIdError": "Failed to copy Session ID" }, "BrowserTab": { "closeOthers": "Close Others", diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index 1c8a5c7325b..be6e087060d 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -14646,7 +14646,10 @@ "SortableTabContextMenu": { "switchToTerminalView": "Cambiar a la vista de terminal", "switchToChatView": "Cambiar a vista de chat", - "closeTabsToLeft": "Cerrar pestañas a la izquierda" + "closeTabsToLeft": "Cerrar pestañas a la izquierda", + "copySessionId": "Copiar ID de sesión", + "copySessionIdSuccess": "ID de sesión copiado", + "copySessionIdError": "No se pudo copiar el ID de sesión" }, "BrowserTab": { "closeOthers": "Cerrar otras", diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index 334e9e32d27..20e2fca6553 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -14646,7 +14646,10 @@ "SortableTabContextMenu": { "switchToTerminalView": "ターミナルビューに切り替える", "switchToChatView": "チャットビューに切り替える", - "closeTabsToLeft": "左側のタブを閉じる" + "closeTabsToLeft": "左側のタブを閉じる", + "copySessionId": "セッション ID をコピー", + "copySessionIdSuccess": "セッション ID をコピーしました", + "copySessionIdError": "セッション ID のコピーに失敗しました" }, "BrowserTab": { "closeOthers": "その他を閉じる", diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index 7ef6c53ca86..e9b9c48fcdf 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -14689,7 +14689,10 @@ "SortableTabContextMenu": { "switchToTerminalView": "terminal 보기로 전환", "switchToChatView": "채팅 보기로 전환", - "closeTabsToLeft": "왼쪽으로 탭 닫기" + "closeTabsToLeft": "왼쪽으로 탭 닫기", + "copySessionId": "세션 ID 복사", + "copySessionIdSuccess": "세션 ID를 복사했습니다", + "copySessionIdError": "세션 ID를 복사하지 못했습니다" }, "BrowserTab": { "closeOthers": "다른 탭 닫기", diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index 06fae6cca89..49560989924 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -14689,7 +14689,10 @@ "SortableTabContextMenu": { "switchToTerminalView": "切换到终端视图", "switchToChatView": "切换到聊天视图", - "closeTabsToLeft": "关闭左侧的选项卡" + "closeTabsToLeft": "关闭左侧的选项卡", + "copySessionId": "复制会话 ID", + "copySessionIdSuccess": "已复制会话 ID", + "copySessionIdError": "复制会话 ID 失败" }, "BrowserTab": { "closeOthers": "关闭其他", diff --git a/tests/e2e/tab-context-menu-session-id.spec.ts b/tests/e2e/tab-context-menu-session-id.spec.ts new file mode 100644 index 00000000000..cab18bace32 --- /dev/null +++ b/tests/e2e/tab-context-menu-session-id.spec.ts @@ -0,0 +1,76 @@ +/** + * E2E coverage for copying an agent provider session ID from a terminal tab's + * context menu. + */ + +import { test, expect } from './helpers/orca-app' +import { + ensureTerminalVisible, + getActiveTabId, + waitForActiveWorktree, + waitForSessionReady +} from './helpers/store' +import { waitForPaneIdentitySnapshot } from './helpers/terminal' + +const SESSION_ID = 'e2e-terminal-tab-session' + +test('terminal tab context menu copies the active agent session ID', async ({ orcaPage }) => { + await waitForSessionReady(orcaPage) + const worktreeId = await waitForActiveWorktree(orcaPage) + await ensureTerminalVisible(orcaPage) + + const tabId = await getActiveTabId(orcaPage) + if (!tabId) { + throw new Error('No active terminal tab') + } + const snapshot = await waitForPaneIdentitySnapshot(orcaPage, 1) + const leafId = snapshot.panes[0]?.leafId + if (!leafId) { + throw new Error('No active terminal pane') + } + const paneKey = `${tabId}:${leafId}` + + // Seed the same renderer state a live agent hook produces while keeping the + // test independent of an installed provider CLI. + await orcaPage.evaluate( + ({ paneKey, tabId, worktreeId, sessionId }) => { + const state = window.__store?.getState() + if (!state) { + throw new Error('Store unavailable') + } + state.setAgentStatus( + paneKey, + { state: 'working', prompt: 'copy session id', agentType: 'claude' }, + 'Claude', + undefined, + { tabId, worktreeId }, + { providerSession: { key: 'session_id', id: sessionId } } + ) + }, + { paneKey, tabId, worktreeId, sessionId: SESSION_ID } + ) + + await expect + .poll( + () => + orcaPage.evaluate( + ({ paneKey }) => + window.__store?.getState().agentStatusByPaneKey[paneKey]?.providerSession?.id, + { paneKey } + ), + { timeout: 3_000 } + ) + .toBe(SESSION_ID) + + const tab = orcaPage.locator(`[data-testid="sortable-tab"][data-tab-id="${tabId}"]`) + await expect(tab).toBeVisible() + await tab.click({ button: 'right' }) + + const copyItem = orcaPage.getByRole('menuitem', { name: 'Copy Session ID', exact: true }) + await expect(copyItem).toBeVisible() + await copyItem.click() + + await expect + .poll(() => orcaPage.evaluate(() => window.api.ui.readClipboardText()), { timeout: 3_000 }) + .toBe(SESSION_ID) +})