fix(filesystem): bound main-process filesystem reads so a stalled mount cannot freeze the app - #12149
fix(filesystem): bound main-process filesystem reads so a stalled mount cannot freeze the app#12149nwparker wants to merge 9 commits into
Conversation
|
Too many files changed for review. ( Bypass the limit by tagging |
eab5f1f to
ff3e7d5
Compare
nwparker
changed the title
|
Warning Review limit reached
Next review available in: 26 seconds Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (149)
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change adds a supervised filesystem-host child process with validated IPC, bounded filesystem operations, failure-domain scheduling, breakers, capacity limits, telemetry, and lifecycle cleanup. Credential, configuration, profile, and authentication reads now use asynchronous memory snapshots with refresh deduplication and stale-result fencing. Rate-limit providers and IPC contracts consume structured snapshot state. Packaging verifies the unpacked worker entry. New tests cover protocol validation, snapshot behavior, filesystem failures, startup ordering, and freeze-safety liveness. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (16)
tests/e2e/helpers/freeze-safety-liveness.ts-213-227 (1)
213-227: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard
JSON.parseinside themessagehandler.Line 215 parses every frame without a guard. A malformed or partial frame throws inside a
wsevent handler. That exception does not fail a single assertion. It surfaces as an uncaught exception and aborts the Playwright worker, which hides the real test result.🛡️ Proposed fix
server.on('connection', (socket) => { socket.on('message', (data) => { - const message = JSON.parse(String(data)) as WireMessage + let message: WireMessage + try { + message = JSON.parse(String(data)) as WireMessage + } catch { + return + } if (message.type === 'pong') {Apply the same guard to the renderer-side handler at line 123.
src/main/index.ts-3011-3012 (1)
3011-3012: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winClear the shared filesystem read authority before disposing it.
The module reference is reset, but
configureFilesystemHostReadAuthorityalso stores the same authority inREAD_AUTHORITY_STATE_KEYfor the read wrappers. Without clearing that state, a read arriving during quit teardown still resolves to a disposed supervisor/authority and is not rejected as unavailable.config/scripts/verify-packaged-filesystem-host-entry.cjs-33-37 (1)
33-37: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard against a null
stdout.
spawnSyncreturnsstdoutasnullwhen the child is terminated by a signal or the timeout.result.erroris not always populated in that case.result.stdout.includes(...)then throws aTypeErrorinstead of the intended failure message.🛡️ Proposed fix
- if (result.status !== 0 || !result.stdout.includes('"protocolVersion":1')) { + const stdout = result.stdout ?? '' + const stderr = result.stderr ?? '' + if (result.status !== 0 || !stdout.includes('"protocolVersion":1')) { throw new Error( - `[verify-packaged-filesystem-host-entry] self-test failed: ${result.stderr || result.stdout}` + `[verify-packaged-filesystem-host-entry] self-test failed: ${stderr || stdout || `signal ${result.signal}`}` ) }src/main/speech/openai-api-key-store.ts-139-150 (1)
139-150: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe decrypt failure path leaves the status stale-positive.
Line 147 catches the decrypt failure and throws, but it does not update the status. If the snapshot previously reported
value: truewithavailability: 'ready', it keeps reporting a configured key that cannot be decrypted. The renderer then shows the key as configured while every use fails.Mark the status unavailable before rethrowing, matching the handling in
saveOpenAiSpeechApiKeyat lines 117-119.🐛 Proposed fix
- } catch { + } catch { + markApiKeyStatusUnavailable() throw new Error('OpenAI API key could not be decrypted') }src/main/speech/openai-api-key-store.ts-93-95 (1)
93-95: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAwait the API-key snapshot before reading OpenAI model states.
registerSpeechHandlers()startshydrateOpenAiSpeechApiKeySnapshot()but does not await it, whilespeech:getModelStatesonly awaitsmigrationReadybefore callinghasOpenAiSpeechApiKey(). A configured OpenAI key can therefore remain unavailable for model-state reads until the hydration callback runs. MakemigrationReadyawait both the model cache migration andhydrateOpenAiSpeechApiKeySnapshot().src/main/orca-profiles/profile-list-snapshot-store.ts-6-11 (1)
6-11: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winCopy
profilesbefore caching it.
snapshots.setstoresindex.profilesby reference. The cached snapshot therefore aliases the array owned by the caller'sOrcaProfileIndex.
writeProfileIndexinsrc/main/orca-profiles/profile-index-store.tspublishes the exactindexobject the caller passed in. If any caller mutates that array after the write, the cache changes with it and the IPC read path serves profile state that was never written to disk. A shallow copy removes this class of bug.🛡️ Proposed fix to decouple the cached snapshot
export function publishOrcaProfileListSnapshot(indexPath: string, index: OrcaProfileIndex): void { snapshots.set(indexPath, { activeProfileId: index.activeProfileId, - profiles: index.profiles + profiles: [...index.profiles] }) }src/main/rate-limits/service.ts-357-367 (1)
357-367: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winHandle hydration rejections inside
refresh.
MemorySnapshotStore.refreshcatches loader errors and returns a snapshot, butrefreshGrokAuthSnapshotandrefreshKimiCredentialSnapshotalso invoke external functions inhydrateSnapshots. If those throw,refresh()rejects and skipsfetchAll. Catch hydration failures inrefreshand apply per-hydration failure classification before returning the state.src/main/codex-accounts/service.ts-292-325 (1)
292-325: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winInvalidate and refresh the system-default snapshot after selected account
~/.codexmutations.
CodexAccountServicenow cachessystemDefaultIdentitySnapshotand is only refreshed from the cache ingetSnapshot(). Mutation paths that can change~/.codex/auth.jsonafter selection (doAddAccount,doReauthenticateAccount,doSelectAccountwithaccountId === null, andonHostSystemDefaultSelected) still returngetSnapshot()without callinginvalidateSystemDefaultIdentity()followed byhydrateSystemDefaultIdentity(). Call the invalidation/refresher once after those mutations so the renderer receives the new system-default identity.src/main/speech/openai-api-key-store.test.ts-140-152 (1)
140-152: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRestore
isEncryptionAvailableafter this test.
safeStorageMockis a shared hoisted object. This test setsisEncryptionAvailableto returnfalseand never restores it. The test restores only theconsole.warnspy. If nobeforeEachresets the mock, the override leaks into the tests that follow in file order and makes them depend on declaration order.💚 Proposed fix
expect(safeStorageMock.encryptString).not.toHaveBeenCalled() warn.mockRestore() + safeStorageMock.isEncryptionAvailable.mockReturnValue(true) })src/main/rate-limits/gemini-usage-fetcher.ts-224-233 (1)
224-233: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
deniedreports a stale-snapshot message.Line 224 groups
snapshot.availability === 'denied'withsnapshot.staleand returns "Gemini credential snapshot is stale; refresh to retry".classifyFilesystemSnapshotFailuresetsdeniedforEACCES/EPERM, which a refresh will not fix. The user then retries a permission failure that cannot succeed. Report the permission case separately.🐛 Proposed fix
+ if (snapshot.availability === 'denied') { + return unavailableResult('Gemini credential access was denied') + } - if (snapshot.stale || snapshot.availability === 'denied') { + if (snapshot.stale) { return unavailableResult('Gemini credential snapshot is stale; refresh to retry') }src/main/startup/desktop-startup-ordering.test.ts-6-21 (1)
6-21: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winBound the
resolveranchor.
resolveris never checked against-1. IfsetInactiveCodexAccountsResolver(is renamed,resolverbecomes-1,indexOf(x, -1)searches from position 0, and every assertion still passes against an unintended region. The rest of this file already guards each anchor for this exact reason (Lines 47-51).🐛 Proposed fix
const runtimeConstruction = source.indexOf('new OrcaRuntimeService(', resolver) + expect(resolver).toBeGreaterThanOrEqual(0) expect(rateLimitHydration).toBeGreaterThan(resolver)src/main/filesystem-host/filesystem-host-process.ts-85-97 (1)
85-97: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winA failed first
kill()leaves the process permanently unkillable throughretire().If
this.child.kill()throws at Line 93, the method setsthis.retired = trueand returnsfalsewithout arming the hard-kill timer. Every laterretire()call skips theif (!this.retired)block, so no further kill signal is ever sent; the caller waitsexitDeadlineMsand receivesfalseagain. Consider retrying the signal on subsequent calls, or falling through to the timer block so theSIGKILLfallback still runs.♻️ Proposed change
if (!this.retired) { this.retired = true this.reads.rejectAll('Filesystem host was retired') - try { - this.child.kill() - } catch { - return Promise.resolve(false) - } } + try { + this.child.kill() + } catch { + // The hard-kill timer and exit deadline below own the observable outcome. + }src/main/minimax/minimax-cookie-store.ts-167-170 (1)
167-170: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winA failed
rmSyncleaves the credential file on disk while the snapshot reports cleared.
revoke()runs first and publishesmissing, sohasMiniMaxSessionCookie()and the IPC status immediately report not configured.rmSyncwithforce: truestill throws onEPERM/EBUSY, which is common on Windows when another handle holds the file. The cookie then persists on disk with no UI indication. Restore the snapshot when the removal fails.🐛 Proposed fix
export async function clearMiniMaxSessionCookie(): Promise<void> { cookieSnapshot.revoke() - rmSync(getMiniMaxCookiePath(), { force: true }) + try { + rmSync(getMiniMaxCookiePath(), { force: true }) + } catch (error) { + cookieSnapshot.invalidate() + throw error + } }src/main/ipc/worktrees.ts-3098-3102 (1)
3098-3102: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winA repository with no snapshot yet reports
status: 'error'.
orcaYamlSnapshots.readreturnsvalue: nullandavailability: 'unavailable'for a key that was never published. A local repository whose first read has not settled therefore reports an error instead of a pending state. Distinguish "never observed" from a real read failure, for example by treatinglastError === nullwithage === nullas not-yet-hydrated.src/shared/orca-yaml.ts-213-217 (1)
213-217: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClassify an empty or comment-only
orca.yamlas valid, not invalid.
document.toJS()returnsnullfor a file that is empty or contains only comments.asRecord(root)then fails and the inspection reportsvalid: false. The snapshot store maps that tocontentState: 'invalid', and the renderer surfaces the file as broken. The YAML parser reports no errors for these inputs, so treat a null or undefined root as a valid document with no top-level keys.🐛 Proposed fix
const record = asRecord(root) if (!record) { + // Empty or comment-only documents parse cleanly to a null root. + if (root === null || root === undefined) { + return { hooks: null, valid: true, topLevelKeys: [] } + } return { hooks: null, valid: false, topLevelKeys: [] } }src/main/git/orca-yaml-snapshot-store.test.ts-180-184 (1)
180-184: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMake the joined-path assertions platform independent.
refreshLocalOrcaYamlSnapshotbuilds the path withjoinfromnode:path. On Windows,join('/local/repo', 'orca.yaml')returns\local\repo\orca.yaml, so these literal comparisons fail on Windows runners. Build the expected values withjoinas well.💚 Proposed fix
+import { join } from 'node:path'expect(readOrcaYamlMock).toHaveBeenCalledTimes(2) expect(readOrcaYamlMock.mock.calls.map(([filePath]) => String(filePath))).toEqual([ - '/local/repo/orca.yaml', - '\\\\wsl.localhost\\Ubuntu\\home\\repo/orca.yaml' + join('/local/repo', 'orca.yaml'), + join(String.raw`\\wsl.localhost\Ubuntu\home\repo`, 'orca.yaml') ])
🧹 Nitpick comments (28)
config/vitest-filesystem-host-read-client.ts (1)
1-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHoist the
node:fs/promisesimport to module scope.Each operation repeats
await import('node:fs/promises'). A single static import removes the duplication and keeps the client bodies readable.♻️ Proposed refactor
+import { mkdir, readFile, realpath } from 'node:fs/promises' import { setFilesystemHostReadClientForTests } from '../src/main/filesystem-host/filesystem-host-read-authority' setFilesystemHostReadClientForTests({ - canonicalizePath: async (path) => (await import('node:fs/promises')).realpath(path), - readOrcaYaml: async (path) => (await import('node:fs/promises')).readFile(path, 'utf8'), - readKeybindings: async (path) => (await import('node:fs/promises')).readFile(path, 'utf8'), - readSnapshotFile: async (path) => (await import('node:fs/promises')).readFile(path), + canonicalizePath: (path) => realpath(path), + readOrcaYaml: (path) => readFile(path, 'utf8'), + readKeybindings: (path) => readFile(path, 'utf8'), + readSnapshotFile: (path) => readFile(path), prepareRateLimitPtyCwd: async (path) => { - const fs = await import('node:fs/promises') - await fs.mkdir(path, { recursive: true }) - return fs.realpath(path) + await mkdir(path, { recursive: true }) + return realpath(path) } })tests/e2e/freeze-safety-liveness.spec.ts (1)
24-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the fixed 31-second recovery sleep with polling.
Line 261 sleeps for
FILESYSTEM_HOST_RECOVERY_DELAY_MSbefore it checks recovery. The value encodes a breaker cooldown that lives in the supervisor. If that cooldown changes, the test fails or passes for the wrong reason. The sleep also adds 31 seconds to every run.Poll the recovered status instead, with a timeout above the expected cooldown.
♻️ Proposed refactor
- await new Promise((resolve) => setTimeout(resolve, FILESYSTEM_HOST_RECOVERY_DELAY_MS)) - const recovered = await orcaPage.evaluate(async () => { - await window.api.rateLimits.refreshGrok() - return await window.api.grokAccounts.getStatus() - }) - expect(recovered).toMatchObject({ - stale: false, - availability: 'ready', - signedIn: true, - email: 'recovered@example.invalid' - }) + await expect + .poll( + () => + orcaPage.evaluate(async () => { + await window.api.rateLimits.refreshGrok() + return await window.api.grokAccounts.getStatus() + }), + { intervalMs: 2_000, timeout: FILESYSTEM_HOST_RECOVERY_TIMEOUT_MS } + ) + .toMatchObject({ + stale: false, + availability: 'ready', + signedIn: true, + email: 'recovered@example.invalid' + })Rename the constant to
FILESYSTEM_HOST_RECOVERY_TIMEOUT_MSand keep it at a generous upper bound.Also applies to: 261-261
tests/e2e/helpers/freeze-safety-liveness.ts (1)
253-261: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the duplicated teardown sequence.
The error path at lines 254-259 and the
stoppath at lines 282-287 run the same four steps: clear the ping timer, remove both watches, terminate the client sockets, and close the server. Extract one closure so the two paths cannot drift.♻️ Proposed refactor
+ const teardown = async (): Promise<void> => { + clearInterval(pingTimer) + await Promise.allSettled([removeRendererWatch(page), removeMainLoopWatch(electronApp)]) + for (const socket of server.clients) { + socket.terminate() + } + await new Promise<void>((resolve) => server.close(() => resolve())) + } + try { await installMainLoopWatch(electronApp, intervalMs) @@ } catch (error) { - clearInterval(pingTimer) - await Promise.allSettled([removeRendererWatch(page), removeMainLoopWatch(electronApp)]) - for (const socket of server.clients) { - socket.terminate() - } - await new Promise<void>((resolve) => server.close(() => resolve())) + await teardown() throw error } @@ stop: async () => { if (stopped) { return } stopped = true - clearInterval(pingTimer) - await Promise.allSettled([removeRendererWatch(page), removeMainLoopWatch(electronApp)]) - for (const socket of server.clients) { - socket.terminate() - } - await new Promise<void>((resolve) => server.close(() => resolve())) + await teardown() }Also applies to: 277-288
src/main/filesystem-host/filesystem-host-entry.ts (1)
46-52: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unreachable
shuttingDownstate.
process.exit(0)on line 48 ends the process synchronously. The assignment on line 47 is never observed, and theshuttingDownterm on line 50 is always false. Drop the variable to make the shutdown contract explicit.♻️ Proposed refactor
- if (message.type === 'shutdown') { - shuttingDown = true - process.exit(0) - } - if (shuttingDown || message.type !== 'request') { + if (message.type === 'shutdown') { + process.exit(0) + } + if (message.type !== 'request') { return }Also remove the
let shuttingDown = falsedeclaration on line 36.src/main/filesystem-host/filesystem-host-env.ts (1)
1-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winForward
TMPDIRfor macOS and Linux parity.The allowlist forwards the Windows temp variables
TEMPandTMPbut omits the POSIX equivalentTMPDIR. On macOS and Linux the child then falls back to/tmpinstead of the per-user temp directory that the main process uses. No current operation writes to a temp directory, so this is a parity gap and not an active defect.♻️ Proposed change
'TEMP', 'TMP', + 'TMPDIR', 'LANG',Excluding
HOME,USERPROFILE,NODE_OPTIONS, and credential variables is correct and worth keeping.As per coding guidelines: "keep code, commands, and scripts compatible with macOS, Linux, and Windows".
Source: Coding guidelines
src/shared/filesystem-host-protocol.ts (1)
81-104: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse a standard UUID value for
workerId.
workerIdcomes fromcrypto.randomUUID()elsewhere in this file, which produces RFC 9562 v4 UUIDs, soz.uuid()is the right Zod 4 schema.z.string().uuid()is deprecated in Zod 4, so switch the schema toz.uuid()to avoid a future deprecation risk.♻️ Proposed refactor
type: z.literal('ready'), protocolVersion: z.literal(FILESYSTEM_HOST_PROTOCOL_VERSION), - workerId: z.string().uuid() + workerId: z.uuid() }),config/scripts/verify-packaged-filesystem-host-entry.test.mjs (1)
33-38: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign the test name with the fixture.
The fixture is a synthetic one-line script, not the real
filesystem-host-entry.js. The current name suggests the packaged entry itself is covered. Rename the test, or add a separate test that copies the built entry into the packaged layout.src/main/filesystem-host/filesystem-host-fault-injection.test.ts (1)
12-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCheck the timing budget for slow CI runners.
This test forks a real child process with
readyTimeoutMs: 1_000and waits at most1_000ms forphysicalChildrento reach 0 after a500ms exit deadline plus a hard kill. Windows and container runners can exceed these budgets under load, which makes the test flaky. Consider raisingreadyTimeoutMsand thevi.waitFortimeouts, and keep onlydeadlineMstight, because that value is what the test actually asserts.src/main/filesystem-host/filesystem-host-supervisor.ts (1)
125-144: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAwait abandoned children during disposal.
disposeretires only the processes currently attached to lanes. Handles inthis.abandonedare excluded. If an abandoned child is still running its retirement, or if its retirement never started, the process can outlive application shutdown. Includethis.abandonedin the retirement set.♻️ Proposed change
const processes = [...this.lanes.values()] .map((lane) => lane.process) .filter((process): process is FilesystemHostProcessHandle => process !== null) for (const lane of this.lanes.values()) { lane.process = null } - await Promise.all(processes.map((process) => process.retire())) + await Promise.all( + [...new Set([...processes, ...this.abandoned])].map((process) => process.retire()) + )src/main/filesystem-host/filesystem-host-read-authority.ts (2)
86-106: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPreserve the original failure for diagnostics.
failureReasoncollapsescapacity,queue-full,breaker-open,quarantined,outcome-unknown, and every non-supervisor error intounavailable. Callers then see onlyEHOSTUNREACHwith no indication of the real cause. Attach the original error ascauseonFilesystemHostReadErrorso logs keep the supervisor code.♻️ Proposed change
export class FilesystemHostReadError extends Error { readonly code: string - constructor(readonly reason: FilesystemHostReadFailureReason) { + constructor( + readonly reason: FilesystemHostReadFailureReason, + options?: { cause?: unknown } + ) { super( reason === 'deadline' ? 'Filesystem operation timed out' : reason === 'unavailable' ? 'Filesystem host is unavailable' - : `Filesystem read failed (${reason})` + : `Filesystem read failed (${reason})`, + options )Then pass the caught error at each call site, for example
throw new FilesystemHostReadError(failureReason(error), { cause: error }).
126-207: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting the repeated dispatch wrapper.
canonicalizePath,readOrcaYaml,readKeybindings,readSnapshotFile,prepareRateLimitPtyCwd, andclassifyAndPublishrepeat the same shape: buildoperationId, spreadroute, dispatch, callrequireResult, and map the error. A privaterun(operation, route, admission, deadlineMs, kind)helper would remove six copies of the try/catch and keep the deadline and admission policy in one place.src/main/rate-limits/memory-snapshot-store.ts (1)
117-122: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe EACCES/EPERM failure classifier is duplicated in three files. The shared type lives in
src/shared/memory-snapshot.ts, but the classifier that maps errno codes ontoSnapshotAvailabilitylives in a feature module. Each new snapshot owner copies it. Divergence would make the same errno report a different availability depending on which store observed it.
src/main/rate-limits/memory-snapshot-store.ts#L117-L122: moveclassifyFilesystemSnapshotFailurenext to theSnapshotAvailabilitydefinition insrc/shared/memory-snapshot.tsand re-export it here if the current import path must stay stable.src/main/agent-hooks/install-status-snapshot-store.ts#L25-L30: deletefailureAvailabilityand import the shared classifier. This also removes a dependency fromsrc/main/agent-hooksontosrc/main/rate-limits, which is the wrong direction between two feature modules.src/main/speech/openai-api-key-store.ts#L63-L71: replace the inlinecode === 'EACCES' || code === 'EPERM'check with the shared classifier.src/main/rate-limits/memory-snapshot-loader-boundedness.test.ts (2)
27-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffParenthesis counting ignores strings, comments, and regex literals.
refreshCallBodiescounts every(and)character. Parentheses inside string literals, template literals, comments, or regex literals shiftdepth. An unbalanced pair in any of those positions truncates the captured body. A truncated body can hide a realreadFile(call fromRAW_FS_CALL, so the ratchet reports a false negative.The header comment already records that only direct calls are visible. Consider extending that note to cover this lexical limitation, or parse the call arguments with the TypeScript compiler API for an exact body range.
54-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe ratchet does not cover
AgentHookInstallStatusSnapshotStore.The
ownersfilter matches only files that containnew MemorySnapshotStore.AgentHookInstallStatusSnapshotStoreinsrc/main/agent-hooks/install-status-snapshot-store.tsimplements its own single-flightrefreshwith the same unbounded-loader hazard, and itsrefreshcall sites are not scanned.Add the second store name to the owner filter so both snapshot implementations are covered.
♻️ Proposed change to widen owner detection
+const SNAPSHOT_STORE_CONSTRUCTORS = [ + 'new MemorySnapshotStore', + 'new AgentHookInstallStatusSnapshotStore' +] + describe('memory snapshot loader boundedness', () => { it('no snapshot loader reads the filesystem directly', () => { const owners = sourceFiles(MAIN_DIRECTORY) .map((path) => ({ path, contents: readFileSync(path, 'utf-8') })) - .filter(({ contents }) => contents.includes('new MemorySnapshotStore')) + .filter(({ contents }) => + SNAPSHOT_STORE_CONSTRUCTORS.some((constructor) => contents.includes(constructor)) + )src/main/agent-hooks/install-status-snapshot-store.ts (1)
103-135: 🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy lift
refreshdrops a re-read requested after invalidation.A caller that invokes
refreshafterinvalidatewhile a flight is active receives the existing in-flight promise at line 111. The generation fence at line 117 then discards that flight's result, so the caller receives the invalidated snapshot and no fresh read occurs.
MemorySnapshotStore.refreshhandles the same case atsrc/main/rate-limits/memory-snapshot-store.tslines 74-76 by starting a new flight once the obsolete one settles. The two stores in this layer behave differently for the same sequence.Also record the unbounded-reader hazard here. A
readerthat never settles pinsinFlightfor this key for the process lifetime, andsrc/main/rate-limits/memory-snapshot-loader-boundedness.test.tsdoes not scan this store.♻️ Proposed change to queue a current refresh
+ /** + * `reader` must always settle — route filesystem work through the deadline-bounded + * filesystem host. A flight is only cleared by its own completion, so an unbounded + * reader pins this key for the process lifetime and it never refreshes again. + */ refresh( agent: AgentHookTarget, reader: RefreshReader, scope = LOCAL_SCOPE ): Promise<AgentHookInstallStatusSnapshot> { const key = this.key(agent, scope) + const requestedGeneration = this.entries.get(key)?.generation ?? 0 const existing = this.inFlight.get(key) if (existing) { - return existing + return existing.promise.then(async (snapshot) => { + const current = this.entries.get(key)?.generation ?? 0 + if (requestedGeneration === current && requestedGeneration !== existing.generation) { + return await this.refresh(agent, reader, scope) + } + return snapshot + }) } - const generation = this.entries.get(key)?.generation ?? 0 + const generation = requestedGenerationThis also requires
inFlightto store{ generation, promise }instead of a bare promise.src/main/agent-hooks/install-status-snapshot-store.test.ts (1)
88-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for
clearScope.The suite covers scope isolation but never calls
clearScope. Add a test that clears a scope while a refresh is in flight and asserts the scope stays empty after the reader resolves. That case is currently broken; see the comment oninstall-status-snapshot-store.tslines 137-144.src/shared/agent-hook-types.ts (1)
43-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument why the snapshot carries both flattened and nested status.
AgentHookInstallStatusSnapshotexposes the status fields directly and again undervalue. In the unavailable case the two intentionally disagree:readspreads a synthesizedstate: 'error'status whilevaluestaysnull.getManagedAgentHookStatusesdepends on that withsnapshot.value ?? snapshot.This type crosses the preload boundary through
src/preload/api-types.ts. Add a short comment stating that the flattened fields are the fail-closed view andvalueis the last observed value.src/main/rate-limits/claude-fetcher.ts (1)
688-713: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename
attemptCliRepairThenRetryOAuthto match its behavior.The function no longer rereads credentials or retries OAuth after the CLI repair. It returns the CLI result directly. The current name states an action that no longer happens.
♻️ Proposed rename
-async function attemptCliRepairThenRetryOAuth(input: { +async function attemptCliRepair(input: { options?: FetchClaudeRateLimitsOptions attempts: ClaudeUsageAttemptState oauthCredentials: OAuthCredentialReadResult }): Promise<ProviderRateLimits | null> {Update both call sites at Line 802 and Line 855.
src/main/rate-limits/service.test.ts (1)
261-279: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the trailing
service.getState()call.Line 278 calls
service.getState()and discards the result. It asserts nothing and has no side effect that the test depends on.♻️ Proposed cleanup
expect(service.getState().grokCredentialSnapshot?.value?.signedIn).toBe(true) - service.getState() })src/main/rate-limits/grok-auth-snapshot.ts (1)
16-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated file-read-and-classify pattern into a shared helper.
refreshGrokAuthSnapshotduplicates the exactENOENT/ENOTDIR→missing, rethrow-otherwise structure already used inrefreshKimiCredentialSnapshot(src/main/rate-limits/kimi-fetcher.ts:62-88). Both callreadSnapshotFileThroughFilesystemHost, catch the same two error codes, and pass the result toclassifyFilesystemSnapshotFailure.Extract a small helper, for example in
memory-snapshot-store.ts, that takes a path, a label, and a content parser, and returns themissing/ready/throw outcome. Each provider then only supplies its parser. This reduces the risk that a future fix to the missing-file handling lands in one provider loader but not the others.src/main/filesystem-host/filesystem-host-operation.test.ts (1)
122-133: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
rejects-style assertion over manual try/catch.The guard
throw new Error('Expected canonicalization to fail')is caught by the samecatch. The test still fails, but the failure message points at thecode: 'missing'mismatch instead of the missing throw.expect(() => ...).toThrowError(expect.objectContaining({ code: 'missing' }))plus a separate message check makes the failure clearer.src/main/startup/desktop-startup-ordering.test.ts (1)
23-32: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLine 31 is tautological.
hydrationis sliced starting at the index ofreconcileLocalOrcaYamlSnapshots(store.getRepos()), sotoContainon the same string can never fail once Line 29 passes. The ordering contract is already enforced by Lines 29-30. Consider removing Line 31 or asserting something specific about the region between the two anchors.src/main/rate-limits/claude-fetcher.test.ts (1)
75-101: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftThe wrapper duplicates the production hydration rule.
Lines 88-98 re-implement the legacy-credential decision that
RateLimitService.hydrateClaudeTargetalready owns insrc/main/rate-limits/service.ts(Lines 313-355): host runtime plus a non-managed:provenance. If production changes that rule, this wrapper keeps the old rule and the suite still passes against stale semantics.Consider exporting the decision from production code (for example a
resolveClaudeLegacyHydration(preparation)helper inclaude-fetcher.ts) and calling it from bothservice.tsand this test wrapper.src/main/rate-limits/gemini-usage-fetcher.ts (1)
251-256: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winUse the snapshot returned by hydration.
hydrateGeminiOAuthPreparationSnapshotalready returns the resultingMemorySnapshot. Line 255 discards it and re-reads the store, which can observe a different value if a concurrentrevoke()orpublishOwned()lands between the two calls.♻️ Proposed change
- await hydrateGeminiOAuthPreparationSnapshot(geminiCliOAuthEnabled) - return fetchGeminiRateLimits(geminiCliOAuthEnabled, getGeminiOAuthPreparationSnapshot()) + const snapshot = await hydrateGeminiOAuthPreparationSnapshot(geminiCliOAuthEnabled) + return fetchGeminiRateLimits(geminiCliOAuthEnabled, snapshot)src/main/minimax/minimax-cookie-store.ts (1)
116-137: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
saveMiniMaxSessionCookieisasyncbut performs synchronous I/O.
writeSecureFileat Line 131 blocks the main process, and the function contains noawait. The same applies tormSyncinclearMiniMaxSessionCookie. These are write paths rather than the read paths this PR targets, so they may be intentionally out of scope. If they are, no change is needed. Otherwise, move both to the asynchronousnode:fs/promisesequivalents so theasyncsignature reflects the actual behavior.src/main/rate-limits/codex-fetcher-auth-errors.test.ts (1)
27-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated
fetchCodexRateLimitsdefault-options wrapper across three test files. Each file defines its own local wrapper to inject defaultcodexCommand,hiddenPtyCwd, andauthSnapshotvalues forFetchCodexRateLimitsOptions. The root cause is a missing shared test helper for this contract.
src/main/rate-limits/codex-fetcher-auth-errors.test.ts#L27-L41: extract this wrapper into a shared, concretely named test helper (for examplecodex-fetcher-test-defaults.ts), avoiding vague names likehelpersorutils.src/main/rate-limits/codex-fetcher-pty-settle.test.ts#L26-L40: replace this byte-for-byte duplicate wrapper with the shared helper.src/main/rate-limits/codex-fetcher-session-supplement.test.ts#L17-L30: replace this near-duplicate wrapper with the shared helper, passing its distinctauthJsonvalue as an override.src/main/ipc/filesystem-auth.test.ts (1)
34-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShared root cause: no common
FilesystemHostReadClienttest stub factory. Both test files repeat the same five-field stub object literal across multiple test cases instead of sharing one helper.
src/main/ipc/filesystem-auth.test.ts#L34-L46: replace thisbeforeEachstub with a call to a sharedcreateFilesystemHostReadClientStub()factory.src/main/ipc/filesystem-auth.test.ts#L217-L229: build this override from the shared factory, overriding onlycanonicalizePath.src/main/ipc/filesystem-auth.test.ts#L441-L452: build this override from the shared factory, overriding onlycanonicalizePath.src/main/keybindings/keybinding-service.test.ts#L68-L76: replace thisbeforeEachstub with the shared factory, overridingreadKeybindings.src/main/keybindings/keybinding-service.test.ts#L130-L138: build this override from the shared factory, overriding onlyreadKeybindings.src/main/keybindings/keybinding-service.test.ts#L154-L164: build this override from the shared factory, overriding onlyreadKeybindings.src/main/ipc/filesystem.ts (1)
879-895: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeduplicate the repeated
orca.yamlbasename check.The
getRuntimePathBasename(targetPath) === 'orca.yaml'check and the matchingorcaYamlSnapshots.publishContent(dirname(targetPath), null)call repeat three times, in the WSL branch, the trash-success branch, and the ENOENT-race branch. Compute the boolean once before the WSL check, then reuse it in all three branches. This lowers the risk that a future change to the basename check updates only some of the three call sites.♻️ Proposed refactor
+ const isOrcaYaml = getRuntimePathBasename(targetPath) === 'orca.yaml' if (await tryDeleteWslUncPath(targetPath, { recursive: args.recursive })) { - if (getRuntimePathBasename(targetPath) === 'orca.yaml') { + if (isOrcaYaml) { orcaYamlSnapshots.publishContent(dirname(targetPath), null) } return } try { await shell.trashItem(targetPath) - if (getRuntimePathBasename(targetPath) === 'orca.yaml') { + if (isOrcaYaml) { orcaYamlSnapshots.publishContent(dirname(targetPath), null) } } catch (error) { if (isENOENT(error)) { - if (getRuntimePathBasename(targetPath) === 'orca.yaml') { + if (isOrcaYaml) { orcaYamlSnapshots.publishContent(dirname(targetPath), null) } return } throw error }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2c17adc0-2783-499d-8a85-9a8b98bb6b1b
📒 Files selected for processing (139)
config/electron-builder.config.cjsconfig/scripts/electron-builder-config.test.mjsconfig/scripts/electron-vite-output-contract.test.tsconfig/scripts/verify-packaged-filesystem-host-entry.cjsconfig/scripts/verify-packaged-filesystem-host-entry.test.mjsconfig/tsconfig.cli.jsonconfig/vitest-filesystem-host-read-client.tsconfig/vitest.config.tselectron.vite.config.tspackage.jsonsrc/main/agent-hooks/install-status-snapshot-store.test.tssrc/main/agent-hooks/install-status-snapshot-store.tssrc/main/agent-hooks/managed-agent-hook-controls.test.tssrc/main/agent-hooks/managed-agent-hook-controls.tssrc/main/agent-hooks/managed-agent-hook-registry.tssrc/main/claude-accounts/runtime-auth-service.test.tssrc/main/claude-accounts/runtime-auth-service.tssrc/main/codex-accounts/service.test.tssrc/main/codex-accounts/service.tssrc/main/filesystem-host/__fixtures__/filesystem-host-hang-fixture.cjssrc/main/filesystem-host/filesystem-host-breaker-blast-radius.test.tssrc/main/filesystem-host/filesystem-host-breaker.test.tssrc/main/filesystem-host/filesystem-host-breaker.tssrc/main/filesystem-host/filesystem-host-capacity.test.tssrc/main/filesystem-host/filesystem-host-capacity.tssrc/main/filesystem-host/filesystem-host-entry-path.test.tssrc/main/filesystem-host/filesystem-host-entry-path.tssrc/main/filesystem-host/filesystem-host-entry.tssrc/main/filesystem-host/filesystem-host-env.test.tssrc/main/filesystem-host/filesystem-host-env.tssrc/main/filesystem-host/filesystem-host-failure-domain.test.tssrc/main/filesystem-host/filesystem-host-failure-domain.tssrc/main/filesystem-host/filesystem-host-fault-injection.test.tssrc/main/filesystem-host/filesystem-host-operation.test.tssrc/main/filesystem-host/filesystem-host-operation.tssrc/main/filesystem-host/filesystem-host-process-error.tssrc/main/filesystem-host/filesystem-host-process.test.tssrc/main/filesystem-host/filesystem-host-process.tssrc/main/filesystem-host/filesystem-host-read-authority.test.tssrc/main/filesystem-host/filesystem-host-read-authority.tssrc/main/filesystem-host/filesystem-host-read-requests.tssrc/main/filesystem-host/filesystem-host-supervisor-error.tssrc/main/filesystem-host/filesystem-host-supervisor-execution.tssrc/main/filesystem-host/filesystem-host-supervisor-health.tssrc/main/filesystem-host/filesystem-host-supervisor-scheduling.tssrc/main/filesystem-host/filesystem-host-supervisor-telemetry.tssrc/main/filesystem-host/filesystem-host-supervisor.test.tssrc/main/filesystem-host/filesystem-host-supervisor.tssrc/main/filesystem-host/filesystem-host-telemetry.tssrc/main/git/orca-yaml-snapshot-store.test.tssrc/main/git/orca-yaml-snapshot-store.tssrc/main/git/status.test.tssrc/main/git/status.tssrc/main/git/worktree-shared-directories.test.tssrc/main/git/worktree-shared-directories.tssrc/main/grok-accounts/status.test.tssrc/main/grok-accounts/status.tssrc/main/index.tssrc/main/ipc/agent-hooks.test.tssrc/main/ipc/agent-hooks.tssrc/main/ipc/app.tssrc/main/ipc/filesystem-auth.test.tssrc/main/ipc/filesystem-auth.tssrc/main/ipc/filesystem-import-ssh.tssrc/main/ipc/filesystem-mutations.tssrc/main/ipc/filesystem-watcher.tssrc/main/ipc/filesystem.tssrc/main/ipc/floating-workspace-directory.tssrc/main/ipc/keybindings.test.tssrc/main/ipc/keybindings.tssrc/main/ipc/minimax-credentials.test.tssrc/main/ipc/minimax-credentials.tssrc/main/ipc/orca-profiles.test.tssrc/main/ipc/orca-profiles.tssrc/main/ipc/repos.tssrc/main/ipc/speech.test.tssrc/main/ipc/speech.tssrc/main/ipc/worktrees.test.tssrc/main/ipc/worktrees.tssrc/main/keybindings/keybinding-file.tssrc/main/keybindings/keybinding-service.test.tssrc/main/keybindings/keybinding-service.tssrc/main/minimax/minimax-cookie-store.test.tssrc/main/minimax/minimax-cookie-store.tssrc/main/network/macos-tailscale-dns-diagnostic.test.tssrc/main/network/macos-tailscale-dns-diagnostic.tssrc/main/orca-profiles/profile-index-store.test.tssrc/main/orca-profiles/profile-index-store.tssrc/main/orca-profiles/profile-list-snapshot-store.tssrc/main/rate-limits/claude-fetcher.test.tssrc/main/rate-limits/claude-fetcher.tssrc/main/rate-limits/claude-pty.test.tssrc/main/rate-limits/claude-pty.tssrc/main/rate-limits/codex-fetcher-auth-errors.test.tssrc/main/rate-limits/codex-fetcher-backend.test.tssrc/main/rate-limits/codex-fetcher-pty-settle.test.tssrc/main/rate-limits/codex-fetcher-session-supplement.test.tssrc/main/rate-limits/codex-fetcher.test.tssrc/main/rate-limits/codex-fetcher.tssrc/main/rate-limits/gemini-oauth-preparation-snapshot.test.tssrc/main/rate-limits/gemini-oauth-preparation-snapshot.tssrc/main/rate-limits/gemini-oauth-sources.tssrc/main/rate-limits/gemini-usage-fetcher.fallback.test.tssrc/main/rate-limits/gemini-usage-fetcher.test.tssrc/main/rate-limits/gemini-usage-fetcher.tssrc/main/rate-limits/grok-auth-snapshot.test.tssrc/main/rate-limits/grok-auth-snapshot.tssrc/main/rate-limits/grok-auth.test.tssrc/main/rate-limits/grok-auth.tssrc/main/rate-limits/grok-fetcher.test.tssrc/main/rate-limits/grok-fetcher.tssrc/main/rate-limits/hidden-rate-limit-pty-cwd.tssrc/main/rate-limits/kimi-fetcher.test.tssrc/main/rate-limits/kimi-fetcher.tssrc/main/rate-limits/memory-snapshot-loader-boundedness.test.tssrc/main/rate-limits/memory-snapshot-store.test.tssrc/main/rate-limits/memory-snapshot-store.tssrc/main/rate-limits/service.test.tssrc/main/rate-limits/service.tssrc/main/runtime/orca-runtime.test.tssrc/main/speech/openai-api-key-store.test.tssrc/main/speech/openai-api-key-store.tssrc/main/startup/desktop-startup-ordering.test.tssrc/preload/api-types.tssrc/preload/index.tssrc/renderer/src/components/settings/VoicePane.tsxsrc/renderer/src/runtime/runtime-hooks-client.tssrc/renderer/src/web/web-preload-api.test.tssrc/renderer/src/web/web-preload-api.tssrc/shared/agent-hook-types.tssrc/shared/filesystem-host-protocol.test.tssrc/shared/filesystem-host-protocol.tssrc/shared/memory-snapshot.tssrc/shared/orca-yaml.tssrc/shared/rate-limit-types.tssrc/shared/speech-types.tssrc/shared/types.tstests/e2e/freeze-safety-liveness.spec.tstests/e2e/helpers/freeze-safety-liveness.ts
💤 Files with no reviewable changes (1)
- src/main/agent-hooks/managed-agent-hook-registry.ts
ff3e7d5 to
d940b45
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/rate-limits/gemini-usage-fetcher.ts (2)
121-131: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winSecond refresh in a fetch call is silently dropped from the snapshot store.
In
fetchViaAuthJsonandfetchViaOauthCreds, the proactive refresh (line 90 / line 140) and the reactive 401-retry refresh (line 122 / line 175) both callrefreshPreparedTokenwith the same capturedpreparationobject.
refreshPreparedTokenforwards this reference topublishGeminiOAuthTokenRefresh, which only publishes whencurrent.value === preparation(reference identity against the store's current value). After the first refresh succeeds, it already replacescurrent.valuewith a new object. IffetchQuotastill returns 401 and the code performs a second refresh using the same, now-stalepreparationreference, the identity check fails and the second (working) token is never written back to the store.The immediate fetch still succeeds locally with the second token, but the store retains the first (already-401'd) value. The next poll cycle reads that stale value, skips the proactive refresh (its cached
expires/expiry_datestill looks valid), hits another 401, and only republishes correctly on that retry. This costs one extra wasted poll cycle each time this sequence occurs, and works against the generation-fencing guarantee this snapshot architecture is meant to provide.Track the preparation reference returned after a successful refresh and reuse it for any later refresh call within the same execution, instead of the original captured
preparation.🔧 Proposed fix to track the latest preparation reference
async function fetchViaAuthJson( preparation: Extract<GeminiOAuthPreparation, { source: 'auth-json' }> ): Promise<ProviderRateLimits> { + let currentPreparation = preparation const auth = preparation.auth let accessToken = auth.access const refreshToken = (auth.refresh || '').split('|')[0] ?? '' if (auth.expires < Date.now() || !accessToken) { - const refreshResult = await refreshPreparedToken(preparation, refreshToken) + const refreshResult = await refreshPreparedToken(currentPreparation, refreshToken) if (!refreshResult?.accessToken) { ... } accessToken = refreshResult.accessToken + const refreshedSnapshot = getGeminiOAuthPreparationSnapshot() + if (refreshedSnapshot.value?.source === 'auth-json') { + currentPreparation = refreshedSnapshot.value + } } ... if (result.status === 'error' && result.error?.includes('401')) { - const refreshResult = await refreshPreparedToken(preparation, refreshToken) + const refreshResult = await refreshPreparedToken(currentPreparation, refreshToken) ... } }Apply the same pattern in
fetchViaOauthCreds, refreshingcurrentPreparationafter the first successful refresh before the 401-retry call.Also applies to: 174-186, 188-202
134-159: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPersist refreshed Gemini OAuth credentials to disk.
saveGeminiCredentialshas no call sites. After refresh, only the in-memory snapshot changes, sooauth_creds.jsonretains the expired access token and any rotated refresh token. Persist the complete refreshed credential record, includingexpiry_date, after a successful refresh.
♻️ Duplicate comments (1)
config/scripts/verify-packaged-filesystem-host-entry.cjs (1)
23-27: 🩺 Stability & Availability | 🟠 MajorRestore the required Windows environment variables.
env: {}removesSystemRootandPATH. On Windows, Node can fail beforefilesystem-host-entry.jsstarts. Keep the isolated environment on other platforms, but provide the required Windows variables.Proposed fix
+ const childEnv = + process.platform === 'win32' + ? { + SystemRoot: process.env.SystemRoot ?? '', + SYSTEMROOT: process.env.SYSTEMROOT ?? process.env.SystemRoot ?? '', + TEMP: process.env.TEMP ?? '', + PATH: process.env.PATH ?? '' + } + : {} const result = spawnSync(options.execPath || process.execPath, [entryPath, '--self-test'], { encoding: 'utf8', timeout: 10_000, - env: {} + env: childEnv })
🧹 Nitpick comments (3)
src/main/filesystem-host/filesystem-host-process.ts (1)
126-134: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the
messagelistener on the startup failure path.
finishFailureclears the timer and retires the process. It does not removeonMessage, which line 169 registers withon.onErrorandonExitareoncelisteners, so they self-remove, butonMessagestays attached. If the child survives retirement, the closure stays alive. Extract the cleanup so both paths use it.♻️ Proposed cleanup extraction
return new Promise((resolve, reject) => { let settled = false + const detach = (): void => { + clearTimeout(timer) + this.child.removeListener('error', onError) + this.child.removeListener('exit', onExit) + this.child.removeListener('message', onMessage) + } const finishFailure = (error: FilesystemHostProcessError): void => { if (settled) { return } settled = true - clearTimeout(timer) + detach() reject(error) void this.retire() }Then replace the four cleanup lines in
onMessagewith a singledetach()call.tests/e2e/freeze-safety-liveness.spec.ts (1)
24-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the fixed recovery sleep with polling.
Line 261 sleeps for the full 31 s. The value duplicates the breaker recovery window from the supervisor. If that window changes, this test fails with an unclear timeout instead of a clear signal. Poll the recovered status with
expect.polland a timeout of about 45 s, so the test finishes as soon as the breaker closes.♻️ Proposed polling replacement for lines 261-271
- await new Promise((resolve) => setTimeout(resolve, FILESYSTEM_HOST_RECOVERY_DELAY_MS)) - const recovered = await orcaPage.evaluate(async () => { - await window.api.rateLimits.refreshGrok() - return await window.api.grokAccounts.getStatus() - }) - expect(recovered).toMatchObject({ - stale: false, - availability: 'ready', - signedIn: true, - email: 'recovered@example.invalid' - }) + await expect + .poll( + () => + orcaPage.evaluate(async () => { + await window.api.rateLimits.refreshGrok() + return await window.api.grokAccounts.getStatus() + }), + { timeout: FILESYSTEM_HOST_RECOVERY_TIMEOUT_MS, intervals: [2_000] } + ) + .toMatchObject({ + stale: false, + availability: 'ready', + signedIn: true, + email: 'recovered@example.invalid' + })Rename the constant to
FILESYSTEM_HOST_RECOVERY_TIMEOUT_MSand raise it above the breaker window.src/main/rate-limits/gemini-usage-fetcher.ts (1)
204-213: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider reusing a shared error-result helper.
unavailableResultwas added for thestatus: 'unavailable'early returns.fetchViaAuthJsonandfetchViaOauthCredsstill each inline duplicatestatus: 'error'object literals for "Token refresh failed" and "Gemini project ID not found". Extracting a smallerrorResult(message)helper alongsideunavailableResultwould remove this duplication and keep both status families consistent.♻️ Proposed helper
+function errorResult(error: string): ProviderRateLimits { + return { + provider: 'gemini', + session: null, + weekly: null, + updatedAt: Date.now(), + error, + status: 'error' + } +} + function unavailableResult(error: string): ProviderRateLimits {
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c3e1a69f-7b55-45a4-ac16-f8d44826e88c
📒 Files selected for processing (141)
config/electron-builder.config.cjsconfig/scripts/electron-builder-config.test.mjsconfig/scripts/electron-vite-output-contract.test.tsconfig/scripts/verify-packaged-filesystem-host-entry.cjsconfig/scripts/verify-packaged-filesystem-host-entry.test.mjsconfig/tsconfig.cli.jsonconfig/vitest-filesystem-host-read-client.tsconfig/vitest.config.tselectron.vite.config.tspackage.jsonsrc/main/agent-hooks/install-status-snapshot-store.test.tssrc/main/agent-hooks/install-status-snapshot-store.tssrc/main/agent-hooks/managed-agent-hook-controls.test.tssrc/main/agent-hooks/managed-agent-hook-controls.tssrc/main/agent-hooks/managed-agent-hook-registry.tssrc/main/claude-accounts/runtime-auth-service.test.tssrc/main/claude-accounts/runtime-auth-service.tssrc/main/codex-accounts/service.test.tssrc/main/codex-accounts/service.tssrc/main/filesystem-host/__fixtures__/filesystem-host-hang-fixture.cjssrc/main/filesystem-host/filesystem-host-breaker-blast-radius.test.tssrc/main/filesystem-host/filesystem-host-breaker.test.tssrc/main/filesystem-host/filesystem-host-breaker.tssrc/main/filesystem-host/filesystem-host-capacity.test.tssrc/main/filesystem-host/filesystem-host-capacity.tssrc/main/filesystem-host/filesystem-host-entry-path.test.tssrc/main/filesystem-host/filesystem-host-entry-path.tssrc/main/filesystem-host/filesystem-host-entry.tssrc/main/filesystem-host/filesystem-host-env.test.tssrc/main/filesystem-host/filesystem-host-env.tssrc/main/filesystem-host/filesystem-host-failure-domain.test.tssrc/main/filesystem-host/filesystem-host-failure-domain.tssrc/main/filesystem-host/filesystem-host-fault-injection.test.tssrc/main/filesystem-host/filesystem-host-operation.test.tssrc/main/filesystem-host/filesystem-host-operation.tssrc/main/filesystem-host/filesystem-host-process-error.tssrc/main/filesystem-host/filesystem-host-process.test.tssrc/main/filesystem-host/filesystem-host-process.tssrc/main/filesystem-host/filesystem-host-read-authority.test.tssrc/main/filesystem-host/filesystem-host-read-authority.tssrc/main/filesystem-host/filesystem-host-read-requests.tssrc/main/filesystem-host/filesystem-host-supervisor-error.tssrc/main/filesystem-host/filesystem-host-supervisor-execution.tssrc/main/filesystem-host/filesystem-host-supervisor-health.tssrc/main/filesystem-host/filesystem-host-supervisor-scheduling.tssrc/main/filesystem-host/filesystem-host-supervisor-telemetry.tssrc/main/filesystem-host/filesystem-host-supervisor.test.tssrc/main/filesystem-host/filesystem-host-supervisor.tssrc/main/filesystem-host/filesystem-host-telemetry.tssrc/main/git/orca-yaml-snapshot-store.test.tssrc/main/git/orca-yaml-snapshot-store.tssrc/main/git/status.test.tssrc/main/git/status.tssrc/main/git/worktree-shared-directories.test.tssrc/main/git/worktree-shared-directories.tssrc/main/grok-accounts/status.test.tssrc/main/grok-accounts/status.tssrc/main/index.tssrc/main/ipc/agent-hooks.test.tssrc/main/ipc/agent-hooks.tssrc/main/ipc/app.tssrc/main/ipc/filesystem-auth.test.tssrc/main/ipc/filesystem-auth.tssrc/main/ipc/filesystem-import-ssh.tssrc/main/ipc/filesystem-mutations.tssrc/main/ipc/filesystem-watcher.tssrc/main/ipc/filesystem.tssrc/main/ipc/floating-workspace-directory.tssrc/main/ipc/keybindings.test.tssrc/main/ipc/keybindings.tssrc/main/ipc/minimax-credentials.test.tssrc/main/ipc/minimax-credentials.tssrc/main/ipc/orca-profiles.test.tssrc/main/ipc/orca-profiles.tssrc/main/ipc/repos.tssrc/main/ipc/speech.test.tssrc/main/ipc/speech.tssrc/main/ipc/worktrees.test.tssrc/main/ipc/worktrees.tssrc/main/keybindings/keybinding-file.tssrc/main/keybindings/keybinding-service.test.tssrc/main/keybindings/keybinding-service.tssrc/main/minimax/minimax-cookie-store.test.tssrc/main/minimax/minimax-cookie-store.tssrc/main/network/macos-tailscale-dns-diagnostic.test.tssrc/main/network/macos-tailscale-dns-diagnostic.tssrc/main/orca-profiles/profile-index-store.test.tssrc/main/orca-profiles/profile-index-store.tssrc/main/orca-profiles/profile-list-snapshot-store.tssrc/main/rate-limits/claude-fetcher.test.tssrc/main/rate-limits/claude-fetcher.tssrc/main/rate-limits/claude-pty.test.tssrc/main/rate-limits/claude-pty.tssrc/main/rate-limits/codex-fetcher-auth-errors.test.tssrc/main/rate-limits/codex-fetcher-backend.test.tssrc/main/rate-limits/codex-fetcher-probe-shutdown.test.tssrc/main/rate-limits/codex-fetcher-pty-settle.test.tssrc/main/rate-limits/codex-fetcher-session-supplement.test.tssrc/main/rate-limits/codex-fetcher.test.tssrc/main/rate-limits/codex-fetcher.tssrc/main/rate-limits/gemini-oauth-preparation-snapshot.test.tssrc/main/rate-limits/gemini-oauth-preparation-snapshot.tssrc/main/rate-limits/gemini-oauth-sources.tssrc/main/rate-limits/gemini-usage-fetcher.fallback.test.tssrc/main/rate-limits/gemini-usage-fetcher.test.tssrc/main/rate-limits/gemini-usage-fetcher.tssrc/main/rate-limits/grok-auth-snapshot.test.tssrc/main/rate-limits/grok-auth-snapshot.tssrc/main/rate-limits/grok-auth.test.tssrc/main/rate-limits/grok-auth.tssrc/main/rate-limits/grok-fetcher.test.tssrc/main/rate-limits/grok-fetcher.tssrc/main/rate-limits/hidden-rate-limit-pty-cwd.tssrc/main/rate-limits/kimi-fetcher.test.tssrc/main/rate-limits/kimi-fetcher.tssrc/main/rate-limits/memory-snapshot-loader-boundedness.test.tssrc/main/rate-limits/memory-snapshot-store.test.tssrc/main/rate-limits/memory-snapshot-store.tssrc/main/rate-limits/service.test.tssrc/main/rate-limits/service.tssrc/main/runtime/orca-runtime.test.tssrc/main/speech/openai-api-key-store.test.tssrc/main/speech/openai-api-key-store.tssrc/main/startup/desktop-startup-ordering.test.tssrc/preload/api-types.tssrc/preload/index.tssrc/renderer/src/components/settings/GrokAccountsSection.tsxsrc/renderer/src/components/settings/VoicePane.tsxsrc/renderer/src/runtime/runtime-hooks-client.tssrc/renderer/src/web/web-preload-api.test.tssrc/renderer/src/web/web-preload-api.tssrc/shared/agent-hook-types.tssrc/shared/filesystem-host-protocol.test.tssrc/shared/filesystem-host-protocol.tssrc/shared/memory-snapshot.tssrc/shared/orca-yaml.tssrc/shared/rate-limit-types.tssrc/shared/speech-types.tssrc/shared/types.tstests/e2e/freeze-safety-liveness.spec.tstests/e2e/helpers/freeze-safety-liveness.ts
💤 Files with no reviewable changes (1)
- src/main/agent-hooks/managed-agent-hook-registry.ts
🚧 Files skipped from review as they are similar to previous changes (128)
- config/tsconfig.cli.json
- src/main/git/status.test.ts
- electron.vite.config.ts
- package.json
- src/main/ipc/keybindings.test.ts
- src/main/claude-accounts/runtime-auth-service.test.ts
- src/main/rate-limits/codex-fetcher-auth-errors.test.ts
- src/main/rate-limits/grok-auth-snapshot.test.ts
- src/main/codex-accounts/service.test.ts
- config/vitest-filesystem-host-read-client.ts
- src/main/rate-limits/codex-fetcher-pty-settle.test.ts
- src/main/filesystem-host/filesystem-host-supervisor-error.ts
- config/vitest.config.ts
- src/main/filesystem-host/filesystem-host-entry-path.test.ts
- src/main/filesystem-host/filesystem-host-env.test.ts
- src/shared/filesystem-host-protocol.test.ts
- src/main/ipc/filesystem-import-ssh.ts
- src/main/filesystem-host/fixtures/filesystem-host-hang-fixture.cjs
- src/shared/memory-snapshot.ts
- src/main/filesystem-host/filesystem-host-entry-path.ts
- src/shared/speech-types.ts
- src/main/ipc/floating-workspace-directory.ts
- src/main/filesystem-host/filesystem-host-failure-domain.test.ts
- src/shared/agent-hook-types.ts
- src/main/agent-hooks/managed-agent-hook-controls.test.ts
- src/renderer/src/components/settings/VoicePane.tsx
- src/main/ipc/speech.ts
- src/main/filesystem-host/filesystem-host-telemetry.ts
- config/scripts/electron-builder-config.test.mjs
- config/scripts/electron-vite-output-contract.test.ts
- src/main/grok-accounts/status.ts
- src/main/rate-limits/grok-auth.test.ts
- src/main/rate-limits/grok-auth-snapshot.ts
- src/main/grok-accounts/status.test.ts
- src/main/ipc/filesystem-watcher.ts
- src/main/filesystem-host/filesystem-host-capacity.ts
- config/electron-builder.config.cjs
- src/renderer/src/runtime/runtime-hooks-client.ts
- src/main/filesystem-host/filesystem-host-supervisor-execution.ts
- src/main/filesystem-host/filesystem-host-process-error.ts
- src/main/rate-limits/memory-snapshot-store.test.ts
- src/main/ipc/orca-profiles.ts
- src/main/orca-profiles/profile-index-store.ts
- src/main/rate-limits/claude-pty.ts
- src/main/ipc/filesystem-auth.test.ts
- src/main/orca-profiles/profile-list-snapshot-store.ts
- src/main/rate-limits/memory-snapshot-loader-boundedness.test.ts
- src/renderer/src/web/web-preload-api.test.ts
- src/main/ipc/worktrees.ts
- src/main/startup/desktop-startup-ordering.test.ts
- config/scripts/verify-packaged-filesystem-host-entry.test.mjs
- src/main/rate-limits/claude-pty.test.ts
- src/main/git/worktree-shared-directories.ts
- src/main/ipc/keybindings.ts
- src/main/filesystem-host/filesystem-host-supervisor-health.ts
- src/main/filesystem-host/filesystem-host-breaker.test.ts
- src/main/ipc/agent-hooks.ts
- src/main/rate-limits/codex-fetcher-session-supplement.test.ts
- src/main/rate-limits/gemini-oauth-preparation-snapshot.test.ts
- src/main/filesystem-host/filesystem-host-supervisor-telemetry.ts
- src/main/filesystem-host/filesystem-host-read-requests.ts
- src/main/keybindings/keybinding-service.ts
- src/main/ipc/orca-profiles.test.ts
- src/main/filesystem-host/filesystem-host-failure-domain.ts
- src/main/filesystem-host/filesystem-host-read-authority.test.ts
- src/main/keybindings/keybinding-service.test.ts
- src/main/filesystem-host/filesystem-host-supervisor.test.ts
- src/main/rate-limits/codex-fetcher.test.ts
- src/main/speech/openai-api-key-store.test.ts
- src/main/filesystem-host/filesystem-host-breaker.ts
- src/main/git/orca-yaml-snapshot-store.test.ts
- src/shared/types.ts
- src/main/git/status.ts
- src/main/filesystem-host/filesystem-host-env.ts
- src/main/filesystem-host/filesystem-host-entry.ts
- src/main/claude-accounts/runtime-auth-service.ts
- src/main/network/macos-tailscale-dns-diagnostic.test.ts
- src/main/filesystem-host/filesystem-host-supervisor-scheduling.ts
- src/main/ipc/filesystem.ts
- src/main/keybindings/keybinding-file.ts
- src/main/ipc/minimax-credentials.ts
- src/main/rate-limits/gemini-usage-fetcher.test.ts
- src/shared/orca-yaml.ts
- src/main/ipc/speech.test.ts
- src/main/filesystem-host/filesystem-host-capacity.test.ts
- src/main/filesystem-host/filesystem-host-fault-injection.test.ts
- src/preload/index.ts
- src/main/filesystem-host/filesystem-host-operation.ts
- src/main/rate-limits/gemini-oauth-sources.ts
- src/main/ipc/agent-hooks.test.ts
- tests/e2e/helpers/freeze-safety-liveness.ts
- src/main/rate-limits/hidden-rate-limit-pty-cwd.ts
- src/main/rate-limits/gemini-usage-fetcher.fallback.test.ts
- src/main/rate-limits/claude-fetcher.test.ts
- src/main/ipc/filesystem-auth.ts
- src/shared/filesystem-host-protocol.ts
- src/preload/api-types.ts
- src/main/ipc/minimax-credentials.test.ts
- src/main/index.ts
- src/main/rate-limits/memory-snapshot-store.ts
- src/main/runtime/orca-runtime.test.ts
- src/main/rate-limits/grok-auth.ts
- src/main/agent-hooks/install-status-snapshot-store.ts
- src/main/ipc/repos.ts
- src/main/filesystem-host/filesystem-host-breaker-blast-radius.test.ts
- src/shared/rate-limit-types.ts
- src/main/rate-limits/claude-fetcher.ts
- src/main/filesystem-host/filesystem-host-supervisor.ts
- src/main/rate-limits/gemini-oauth-preparation-snapshot.ts
- src/main/orca-profiles/profile-index-store.test.ts
- src/main/git/orca-yaml-snapshot-store.ts
- src/main/agent-hooks/install-status-snapshot-store.test.ts
- src/main/speech/openai-api-key-store.ts
- src/main/agent-hooks/managed-agent-hook-controls.ts
- src/main/rate-limits/grok-fetcher.ts
- src/main/rate-limits/kimi-fetcher.test.ts
- src/main/minimax/minimax-cookie-store.test.ts
- src/main/rate-limits/kimi-fetcher.ts
- src/main/rate-limits/grok-fetcher.test.ts
- src/main/ipc/worktrees.test.ts
- src/main/ipc/filesystem-mutations.ts
- src/main/minimax/minimax-cookie-store.ts
- src/main/codex-accounts/service.ts
- src/main/rate-limits/codex-fetcher.ts
- src/main/rate-limits/codex-fetcher-backend.test.ts
- src/main/rate-limits/service.test.ts
- src/renderer/src/web/web-preload-api.ts
- src/main/rate-limits/service.ts
Release-scan readiness recordAn 8-seat pre-release audit was run against Seat matrix
Fixed before requesting reviewLiveness regressions vs
Supervisor budget and fairness.
Path authorization and protocol.
Accepted residualsListed so a scan finds them here rather than reporting them as new.
Explicitly not claimedThe forked child is a hang boundary, not a privilege boundary. It bounds how long a stalled mount can hold a read; it is not a sandbox. Its env is built from scratch with a 10-key allowlist rather than inherited — but on Windows libuv re-injects a set of variables regardless, so the allowlist is a POSIX guarantee only. Validation
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/rate-limits/service.ts (1)
503-512: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUnhandled hydration failures leave several refresh paths in a broken state.
hydrateCodexTargetandhydrateClaudeTargetreject when the hidden PTY cwd snapshot is stale or missing (Line 310-312, Line 341-343), andhydrateSnapshots()propagates that rejection throughPromise.all.runFetchAllCycleexplicitly wrapsthis.hydrateSnapshots()in a try/catch (Line 1768-1772) specifically because of this. None of the following call sites have the same guard:
- Line 503-512 (
refresh): an unguardedawait this.hydrateSnapshots()call. A hydration failure rejectsrefresh()instead of falling back to the stale snapshot the wayrunFetchAllCycledoes.- Line 521-526 (
refreshGrok):refreshGrokAuthSnapshot()is not guarded.- Line 537-570 (
refreshForCodexAccountChange) and Line 572-586 (refreshCodexForTarget):updateState(...)setscodextowithFetchingStatus(null, 'codex')(status'fetching') beforeawait this.hydrateCodexTarget(nextTarget). If hydration rejects,fetchCodexOnlynever runs, and the Codex chip is stuck showing'fetching'until an unrelated cycle succeeds.- Line 625-655 (
refreshForClaudeAccountChange) and Line 657-675 (refreshClaudeForTarget): same pattern for Claude, viawithFetchingStatus(null, 'claude')beforeawait this.hydrateClaudeTarget(nextTarget).Wrap each hydration call in the same try/catch pattern used in
runFetchAllCycleso a transient hydration failure degrades to the stale snapshot instead of stalling the UI or rejecting the caller's promise.🐛 Proposed fix pattern (apply to each listed call site)
- await this.hydrateCodexTarget(nextTarget) + try { + await this.hydrateCodexTarget(nextTarget) + } catch { + // Why: a failed hydration leaves the prior snapshot stale, which every reader handles. + } await this.fetchCodexOnly({ force: true })Also applies to: 521-526, 537-570, 572-586, 625-655, 657-675
♻️ Duplicate comments (1)
src/main/rate-limits/service.ts (1)
588-623: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winGate the Codex reset-credit auth/command/PTY data on freshness.
Line 608 reads
.get().value?.authSnapshotwithout checking.stale.MemorySnapshotStore.invalidate()/an un-hydrated store returnsstale: truewhile keeping (or lacking)value, so a stale or never-hydrated entry forcodexTarget(a caller-supplied, arbitrary target) silently passesundefinedor a credential snapshot from an earlier hydration toconsumeCodexRateLimitResetCredit, a non-idempotent, non-refundable operation.This same gap now also affects
fetchCodexResetResultState(Line 1490-1517):codexCommand,hiddenPtyCwd, andauthSnapshotare all read fromthis.getCodexHomeSnapshotStore(target).get().valuewithout checking.stale, unlikerunFetchAllCycle/runFetchCodexOnlyCycle, which gate thefetchCodexRateLimitscall behindmissingWslCodexHomecomputed from.stale(Line 1840-1844, Line 2092-2096). Here the fetch always runs, potentially mixing the caller-suppliedcodexHomePathwith acommand/authSnapshotfrom a stale or unrelated hydration, and the resultingscopedCodexis returned to the caller (Line 1542) regardless of the laterstillActivestaleness check, which only gates the internal global-state update.
.get()is also called three separate times at Line 1500-1502; cache it in a local variable.🐛 Proposed fix
try { + const codexTargetSnapshot = this.getCodexHomeSnapshotStore(codexTarget).get() const outcome = await consumeCodexRateLimitResetCredit({ codexHomePath, - authSnapshot: this.getCodexHomeSnapshotStore(codexTarget).get().value?.authSnapshot, + authSnapshot: codexTargetSnapshot.stale ? undefined : codexTargetSnapshot.value?.authSnapshot, idempotencyKey: options.idempotencyKey })const controller = this.beginFetchCycle() let fresh: ProviderRateLimits + const homeSnapshot = this.getCodexHomeSnapshotStore(target).get() try { fresh = await fetchCodexRateLimits({ codexHomePath, - codexCommand: this.getCodexHomeSnapshotStore(target).get().value?.command ?? 'codex', - hiddenPtyCwd: this.getCodexHomeSnapshotStore(target).get().value?.hiddenPtyCwd, - authSnapshot: this.getCodexHomeSnapshotStore(target).get().value?.authSnapshot, + codexCommand: homeSnapshot.stale ? 'codex' : (homeSnapshot.value?.command ?? 'codex'), + hiddenPtyCwd: homeSnapshot.stale ? undefined : homeSnapshot.value?.hiddenPtyCwd, + authSnapshot: homeSnapshot.stale ? undefined : homeSnapshot.value?.authSnapshot, allowPtyFallback: this.shouldAllowCodexPtyFallback(), signal: controller.signal })Also applies to: 1490-1543
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8f52d20f-c5e7-43ae-afbe-81e176e2830e
📒 Files selected for processing (13)
src/main/agent-hooks/managed-agent-hook-controls.test.tssrc/main/agent-hooks/managed-agent-hook-controls.tssrc/main/codex-accounts/service.tssrc/main/filesystem-host/filesystem-host-supervisor.test.tssrc/main/filesystem-host/filesystem-host-supervisor.tssrc/main/ipc/codex-accounts.tssrc/main/ipc/filesystem-auth.test.tssrc/main/ipc/filesystem-auth.tssrc/main/rate-limits/claude-fetcher.test.tssrc/main/rate-limits/claude-fetcher.tssrc/main/rate-limits/service.test.tssrc/main/rate-limits/service.tssrc/shared/filesystem-host-protocol.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- src/main/ipc/filesystem-auth.test.ts
- src/main/agent-hooks/managed-agent-hook-controls.test.ts
- src/shared/filesystem-host-protocol.ts
- src/main/ipc/filesystem-auth.ts
- src/main/filesystem-host/filesystem-host-supervisor.test.ts
- src/main/codex-accounts/service.ts
- src/main/filesystem-host/filesystem-host-supervisor.ts
- src/main/rate-limits/service.test.ts
Seat 07 P1 #1 (Windows canonicalization) — now fixed, not a residualThis was the one open P1 in the readiness comment above. It is resolved in What was wrongThe forked filesystem host canonicalized with
Those are different implementations, not two spellings. Verified on Windows hardware, not reasoned aboutTwo divergence classes, both real:
Why it matteredContainment checks are safe either way — The exposure was exact-identity comparisons against textually-recorded roots, chiefly The fixDrop Why no test caught it, and what now does
Now:
ScopeThree files, +12 / −5. No other file changed by this PR uses Revalidation
All other residuals listed in the readiness comment above stand as declared. |
0e94c32 to
372334c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
config/scripts/electron-builder-config.test.mjs (2)
639-643: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMake the fixture validate
--self-test.The fixture always prints
protocolVersionand exits successfully. A regression that stops passing--self-testcould still pass this test. Make other arguments exit with a non-zero status.
695-703: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winTest the runtime package-type mapping.
source.includes(\value === '${target}'`)can match a comment or unrelated branch. It does not prove that the mapper returns the required value. Execute the mapping used bygetLinuxRootPackageType()` and assert the results for configured targets.tests/e2e/freeze-safety-liveness.spec.ts (1)
261-271: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace the fixed 31-second recovery sleep with a poll.
Line 261 waits a fixed 31 s that encodes the filesystem-host breaker cooldown. If the cooldown changes, the test fails or wastes time.
expect.pollover the refresh-and-status sequence keeps the same intent and tolerates timing changes. The test budget is 120 s, so a poll with a 45 s timeout fits.♻️ Proposed poll-based recovery wait
- await new Promise((resolve) => setTimeout(resolve, FILESYSTEM_HOST_RECOVERY_DELAY_MS)) - const recovered = await orcaPage.evaluate(async () => { - await window.api.rateLimits.refreshGrok() - return await window.api.grokAccounts.getStatus() - }) - expect(recovered).toMatchObject({ - stale: false, - availability: 'ready', - signedIn: true, - email: 'recovered@example.invalid' - }) + await expect + .poll( + () => + orcaPage.evaluate(async () => { + await window.api.rateLimits.refreshGrok() + return await window.api.grokAccounts.getStatus() + }), + { timeout: FILESYSTEM_HOST_RECOVERY_TIMEOUT_MS, intervals: [2_000] } + ) + .toMatchObject({ + stale: false, + availability: 'ready', + signedIn: true, + email: 'recovered@example.invalid' + })Rename the constant accordingly:
-const FILESYSTEM_HOST_RECOVERY_DELAY_MS = 31_000 +const FILESYSTEM_HOST_RECOVERY_TIMEOUT_MS = 45_000
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6ea63fe5-ae46-4cab-8326-d405e49bb7e5
📒 Files selected for processing (141)
config/electron-builder.config.cjsconfig/scripts/electron-builder-config.test.mjsconfig/scripts/electron-vite-output-contract.test.tsconfig/scripts/verify-packaged-filesystem-host-entry.cjsconfig/scripts/verify-packaged-filesystem-host-entry.test.mjsconfig/tsconfig.cli.jsonconfig/vitest-filesystem-host-read-client.tsconfig/vitest.config.tselectron.vite.config.tspackage.jsonsrc/main/agent-hooks/install-status-snapshot-store.test.tssrc/main/agent-hooks/install-status-snapshot-store.tssrc/main/agent-hooks/managed-agent-hook-controls.test.tssrc/main/agent-hooks/managed-agent-hook-controls.tssrc/main/claude-accounts/runtime-auth-service.test.tssrc/main/claude-accounts/runtime-auth-service.tssrc/main/codex-accounts/service.test.tssrc/main/codex-accounts/service.tssrc/main/filesystem-host/__fixtures__/filesystem-host-hang-fixture.cjssrc/main/filesystem-host/filesystem-host-breaker-blast-radius.test.tssrc/main/filesystem-host/filesystem-host-breaker.test.tssrc/main/filesystem-host/filesystem-host-breaker.tssrc/main/filesystem-host/filesystem-host-capacity.test.tssrc/main/filesystem-host/filesystem-host-capacity.tssrc/main/filesystem-host/filesystem-host-entry-path.test.tssrc/main/filesystem-host/filesystem-host-entry-path.tssrc/main/filesystem-host/filesystem-host-entry.tssrc/main/filesystem-host/filesystem-host-env.test.tssrc/main/filesystem-host/filesystem-host-env.tssrc/main/filesystem-host/filesystem-host-failure-domain.test.tssrc/main/filesystem-host/filesystem-host-failure-domain.tssrc/main/filesystem-host/filesystem-host-fault-injection.test.tssrc/main/filesystem-host/filesystem-host-operation.test.tssrc/main/filesystem-host/filesystem-host-operation.tssrc/main/filesystem-host/filesystem-host-process-error.tssrc/main/filesystem-host/filesystem-host-process.test.tssrc/main/filesystem-host/filesystem-host-process.tssrc/main/filesystem-host/filesystem-host-read-authority.test.tssrc/main/filesystem-host/filesystem-host-read-authority.tssrc/main/filesystem-host/filesystem-host-read-requests.tssrc/main/filesystem-host/filesystem-host-supervisor-error.tssrc/main/filesystem-host/filesystem-host-supervisor-execution.tssrc/main/filesystem-host/filesystem-host-supervisor-health.tssrc/main/filesystem-host/filesystem-host-supervisor-scheduling.tssrc/main/filesystem-host/filesystem-host-supervisor-telemetry.tssrc/main/filesystem-host/filesystem-host-supervisor.test.tssrc/main/filesystem-host/filesystem-host-supervisor.tssrc/main/filesystem-host/filesystem-host-telemetry.tssrc/main/git/orca-yaml-snapshot-store.test.tssrc/main/git/orca-yaml-snapshot-store.tssrc/main/git/status.test.tssrc/main/git/status.tssrc/main/git/worktree-shared-directories.test.tssrc/main/git/worktree-shared-directories.tssrc/main/grok-accounts/status.test.tssrc/main/grok-accounts/status.tssrc/main/index.tssrc/main/ipc/agent-hooks.test.tssrc/main/ipc/agent-hooks.tssrc/main/ipc/app.tssrc/main/ipc/codex-accounts.tssrc/main/ipc/filesystem-auth.test.tssrc/main/ipc/filesystem-auth.tssrc/main/ipc/filesystem-import-ssh.tssrc/main/ipc/filesystem-mutations.tssrc/main/ipc/filesystem-watcher.tssrc/main/ipc/filesystem.tssrc/main/ipc/floating-workspace-directory.tssrc/main/ipc/keybindings.test.tssrc/main/ipc/keybindings.tssrc/main/ipc/minimax-credentials.test.tssrc/main/ipc/minimax-credentials.tssrc/main/ipc/orca-profiles.test.tssrc/main/ipc/orca-profiles.tssrc/main/ipc/repos.tssrc/main/ipc/speech.test.tssrc/main/ipc/speech.tssrc/main/ipc/worktrees.test.tssrc/main/ipc/worktrees.tssrc/main/keybindings/keybinding-file.tssrc/main/keybindings/keybinding-service.test.tssrc/main/keybindings/keybinding-service.tssrc/main/minimax/minimax-cookie-store.test.tssrc/main/minimax/minimax-cookie-store.tssrc/main/network/macos-tailscale-dns-diagnostic.test.tssrc/main/network/macos-tailscale-dns-diagnostic.tssrc/main/orca-profiles/profile-index-store.test.tssrc/main/orca-profiles/profile-index-store.tssrc/main/orca-profiles/profile-list-snapshot-store.tssrc/main/rate-limits/claude-fetcher.test.tssrc/main/rate-limits/claude-fetcher.tssrc/main/rate-limits/claude-pty.test.tssrc/main/rate-limits/claude-pty.tssrc/main/rate-limits/codex-fetcher-auth-errors.test.tssrc/main/rate-limits/codex-fetcher-backend.test.tssrc/main/rate-limits/codex-fetcher-probe-shutdown.test.tssrc/main/rate-limits/codex-fetcher-pty-settle.test.tssrc/main/rate-limits/codex-fetcher-session-supplement.test.tssrc/main/rate-limits/codex-fetcher.test.tssrc/main/rate-limits/codex-fetcher.tssrc/main/rate-limits/gemini-oauth-preparation-snapshot.test.tssrc/main/rate-limits/gemini-oauth-preparation-snapshot.tssrc/main/rate-limits/gemini-oauth-sources.tssrc/main/rate-limits/gemini-usage-fetcher.fallback.test.tssrc/main/rate-limits/gemini-usage-fetcher.test.tssrc/main/rate-limits/gemini-usage-fetcher.tssrc/main/rate-limits/grok-auth-snapshot.test.tssrc/main/rate-limits/grok-auth-snapshot.tssrc/main/rate-limits/grok-auth.test.tssrc/main/rate-limits/grok-auth.tssrc/main/rate-limits/grok-fetcher.test.tssrc/main/rate-limits/grok-fetcher.tssrc/main/rate-limits/hidden-rate-limit-pty-cwd.tssrc/main/rate-limits/kimi-fetcher.test.tssrc/main/rate-limits/kimi-fetcher.tssrc/main/rate-limits/memory-snapshot-loader-boundedness.test.tssrc/main/rate-limits/memory-snapshot-store.test.tssrc/main/rate-limits/memory-snapshot-store.tssrc/main/rate-limits/service.test.tssrc/main/rate-limits/service.tssrc/main/runtime/orca-runtime.test.tssrc/main/speech/openai-api-key-store.test.tssrc/main/speech/openai-api-key-store.tssrc/main/startup/desktop-startup-ordering.test.tssrc/preload/api-types.tssrc/preload/index.tssrc/renderer/src/components/settings/GrokAccountsSection.tsxsrc/renderer/src/components/settings/VoicePane.tsxsrc/renderer/src/runtime/runtime-hooks-client.tssrc/renderer/src/web/web-preload-api.test.tssrc/renderer/src/web/web-preload-api.tssrc/shared/agent-hook-types.tssrc/shared/filesystem-host-protocol.test.tssrc/shared/filesystem-host-protocol.tssrc/shared/memory-snapshot.tssrc/shared/orca-yaml.tssrc/shared/rate-limit-types.tssrc/shared/speech-types.tssrc/shared/types.tstests/e2e/freeze-safety-liveness.spec.tstests/e2e/helpers/freeze-safety-liveness.ts
🚧 Files skipped from review as they are similar to previous changes (132)
- electron.vite.config.ts
- config/tsconfig.cli.json
- src/shared/speech-types.ts
- src/main/claude-accounts/runtime-auth-service.test.ts
- package.json
- src/shared/types.ts
- src/renderer/src/web/web-preload-api.test.ts
- src/main/startup/desktop-startup-ordering.test.ts
- src/main/ipc/codex-accounts.ts
- src/main/filesystem-host/filesystem-host-breaker.test.ts
- src/main/filesystem-host/fixtures/filesystem-host-hang-fixture.cjs
- src/main/filesystem-host/filesystem-host-process-error.ts
- src/main/filesystem-host/filesystem-host-entry-path.test.ts
- src/main/ipc/floating-workspace-directory.ts
- src/main/filesystem-host/filesystem-host-env.test.ts
- src/main/rate-limits/codex-fetcher-probe-shutdown.test.ts
- src/main/filesystem-host/filesystem-host-breaker.ts
- src/main/filesystem-host/filesystem-host-fault-injection.test.ts
- src/main/filesystem-host/filesystem-host-supervisor-execution.ts
- src/main/filesystem-host/filesystem-host-supervisor-telemetry.ts
- src/main/ipc/filesystem-mutations.ts
- src/main/grok-accounts/status.test.ts
- src/main/filesystem-host/filesystem-host-supervisor-health.ts
- src/main/rate-limits/codex-fetcher-auth-errors.test.ts
- src/main/ipc/orca-profiles.ts
- src/main/rate-limits/codex-fetcher-session-supplement.test.ts
- src/main/codex-accounts/service.test.ts
- src/shared/agent-hook-types.ts
- src/main/filesystem-host/filesystem-host-telemetry.ts
- config/vitest-filesystem-host-read-client.ts
- src/renderer/src/runtime/runtime-hooks-client.ts
- src/shared/memory-snapshot.ts
- src/main/rate-limits/memory-snapshot-store.test.ts
- src/main/ipc/keybindings.ts
- src/main/filesystem-host/filesystem-host-env.ts
- src/main/rate-limits/gemini-oauth-preparation-snapshot.test.ts
- src/main/rate-limits/gemini-usage-fetcher.fallback.test.ts
- config/scripts/verify-packaged-filesystem-host-entry.cjs
- src/main/ipc/filesystem-watcher.ts
- src/main/agent-hooks/install-status-snapshot-store.test.ts
- config/scripts/verify-packaged-filesystem-host-entry.test.mjs
- src/shared/orca-yaml.ts
- src/main/filesystem-host/filesystem-host-operation.test.ts
- src/main/filesystem-host/filesystem-host-failure-domain.test.ts
- config/scripts/electron-vite-output-contract.test.ts
- src/renderer/src/components/settings/GrokAccountsSection.tsx
- src/main/grok-accounts/status.ts
- src/main/filesystem-host/filesystem-host-capacity.test.ts
- src/main/filesystem-host/filesystem-host-breaker-blast-radius.test.ts
- src/shared/filesystem-host-protocol.ts
- src/main/filesystem-host/filesystem-host-read-authority.test.ts
- src/main/rate-limits/memory-snapshot-loader-boundedness.test.ts
- src/main/rate-limits/hidden-rate-limit-pty-cwd.ts
- src/main/ipc/agent-hooks.ts
- src/main/git/orca-yaml-snapshot-store.test.ts
- src/main/git/status.test.ts
- src/main/rate-limits/grok-auth.ts
- src/main/filesystem-host/filesystem-host-capacity.ts
- src/main/rate-limits/memory-snapshot-store.ts
- src/main/agent-hooks/managed-agent-hook-controls.ts
- src/shared/filesystem-host-protocol.test.ts
- src/main/network/macos-tailscale-dns-diagnostic.test.ts
- tests/e2e/helpers/freeze-safety-liveness.ts
- src/main/ipc/minimax-credentials.ts
- src/main/agent-hooks/managed-agent-hook-controls.test.ts
- src/main/filesystem-host/filesystem-host-supervisor-error.ts
- src/main/rate-limits/grok-fetcher.ts
- src/main/filesystem-host/filesystem-host-read-authority.ts
- src/main/minimax/minimax-cookie-store.test.ts
- src/main/runtime/orca-runtime.test.ts
- src/main/ipc/filesystem-auth.ts
- src/main/ipc/minimax-credentials.test.ts
- src/main/rate-limits/kimi-fetcher.test.ts
- src/main/filesystem-host/filesystem-host-supervisor-scheduling.ts
- src/main/agent-hooks/install-status-snapshot-store.ts
- src/main/rate-limits/gemini-oauth-sources.ts
- src/main/keybindings/keybinding-file.ts
- src/main/rate-limits/claude-fetcher.test.ts
- src/main/filesystem-host/filesystem-host-operation.ts
- src/main/index.ts
- src/main/filesystem-host/filesystem-host-entry-path.ts
- src/main/filesystem-host/filesystem-host-failure-domain.ts
- src/main/orca-profiles/profile-index-store.test.ts
- src/renderer/src/web/web-preload-api.ts
- src/main/ipc/filesystem.ts
- src/main/keybindings/keybinding-service.test.ts
- src/main/ipc/orca-profiles.test.ts
- src/main/rate-limits/claude-pty.ts
- src/main/speech/openai-api-key-store.test.ts
- src/main/git/orca-yaml-snapshot-store.ts
- src/main/rate-limits/grok-auth-snapshot.ts
- src/main/claude-accounts/runtime-auth-service.ts
- src/main/ipc/agent-hooks.test.ts
- src/main/orca-profiles/profile-list-snapshot-store.ts
- src/main/git/worktree-shared-directories.ts
- config/vitest.config.ts
- src/main/filesystem-host/filesystem-host-supervisor.test.ts
- src/preload/api-types.ts
- src/main/ipc/repos.ts
- src/main/rate-limits/gemini-usage-fetcher.ts
- config/electron-builder.config.cjs
- src/main/rate-limits/kimi-fetcher.ts
- src/main/ipc/keybindings.test.ts
- src/main/speech/openai-api-key-store.ts
- src/preload/index.ts
- src/renderer/src/components/settings/VoicePane.tsx
- src/main/codex-accounts/service.ts
- src/shared/rate-limit-types.ts
- src/main/rate-limits/codex-fetcher-backend.test.ts
- src/main/rate-limits/grok-fetcher.test.ts
- src/main/rate-limits/grok-auth.test.ts
- src/main/rate-limits/claude-pty.test.ts
- src/main/filesystem-host/filesystem-host-entry.ts
- src/main/minimax/minimax-cookie-store.ts
- src/main/rate-limits/codex-fetcher-pty-settle.test.ts
- src/main/ipc/filesystem-auth.test.ts
- src/main/rate-limits/codex-fetcher.ts
- src/main/ipc/worktrees.ts
- src/main/git/status.ts
- src/main/ipc/worktrees.test.ts
- src/main/rate-limits/service.test.ts
- src/main/ipc/speech.test.ts
- src/main/ipc/speech.ts
- src/main/orca-profiles/profile-index-store.ts
- src/main/keybindings/keybinding-service.ts
- src/main/filesystem-host/filesystem-host-supervisor.ts
- src/main/rate-limits/claude-fetcher.ts
- src/main/rate-limits/service.ts
- src/main/ipc/filesystem-import-ssh.ts
- src/main/rate-limits/grok-auth-snapshot.test.ts
- src/main/rate-limits/codex-fetcher.test.ts
- src/main/rate-limits/gemini-usage-fetcher.test.ts
Addendum: rebase, one file split, and a flake — CI now cleanThe first push of the canonicalization fix went red. Recording what happened, since the red run is still visible in this PR's history. 1. Resolved by moving this branch's assertion to Note for whoever touches that file next: it now sits at 596 of 600 and 2. 3. Rebased onto current Final state
|
372334c to
edee189
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Final audit and verification update:
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/runtime/orca-runtime.ts (1)
25658-25681: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd a delay before Enter, and chunk the command write, in
deliverPendingStartupCommandToBareRendererPty.This method writes
commandand then immediately writes'\r'in a separate call with no delay between them. Every other write path in this file that separates text from Enter/interrupt inserts an explicit delay first. For example,writeTerminalActionwaits 500ms before writing the suffix, with this comment on the same behavior: "Claude Code treats a large PTY write as a paste and swallows a \r in the same write; send Enter separately after a delay."writeTerminalAgentPromptuses the same pattern withAGENT_PROMPT_SUBMIT_DELAY_MS.This method exists to backfill an agent startup command (including Claude) onto a PTY that spawned without it. Without the delay, it can reproduce the exact swallowed-Enter bug the other paths guard against, leaving the agent launched but not submitted.
This method also writes
commandin one call instead of chunking it throughwriteTerminalInputChunks, unlikewriteTerminalActionandwriteTerminalAgentPrompt. A long startup command (agent args, resume file paths) can exceed what a single PTY/ConPTY write reliably delivers.🐛 Proposed fix to match the delay/chunking pattern used elsewhere in this file
- private deliverPendingStartupCommandToBareRendererPty(worktreeId: string, tabId: string): void { + private async deliverPendingStartupCommandToBareRendererPty( + worktreeId: string, + tabId: string + ): Promise<void> { const pending = this.pendingMobileTerminalCreatesByKey.get(`${worktreeId}::${tabId}`) const command = pending?.startupCommand if (!command) { return } const pty = this.findLiveRegisteredPtyForRendererTab(worktreeId, tabId) if (!pty || this.terminalSpawnCommandsByPtyId.has(pty.ptyId)) { return } - if (this.ptyController?.write(pty.ptyId, command)) { - // Why: Enter rides its own write so a long command cannot swallow it. - this.ptyController.write(pty.ptyId, '\r') - this.noteTerminalSpawnCommand(pty.ptyId, command) - } + await this.writeTerminalInputChunks(pty.ptyId, command) + // Why: Claude Code treats a large PTY write as a paste and swallows a + // \r in the same write; send Enter separately after a delay. + await new Promise((resolve) => setTimeout(resolve, 500)) + if (this.ptyController?.write(pty.ptyId, '\r')) { + this.noteTerminalSpawnCommand(pty.ptyId, command) + } }Callers at Lines 25136, 25145, and the catch-block rescue path would need
void this.deliverPendingStartupCommandToBareRendererPty(...)or anawait, depending on whether the surrounding code should wait for delivery to complete.
🧹 Nitpick comments (2)
tests/e2e/freeze-safety-liveness.spec.ts (1)
261-265: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the fixed 31-second recovery sleep with a bounded poll.
Line 261 always waits the full
FILESYSTEM_HOST_RECOVERY_DELAY_MS, even when the filesystem host recovers earlier. The delay is the largest single contributor to the 120-second test timeout. If the production breaker cooldown grows, the constant silently becomes too short and the test fails at line 266 instead of at the wait.Poll the recovered status until it reports
availability: 'ready'. Keep the total bound above the cooldown.♻️ Proposed poll-based recovery wait
- await new Promise((resolve) => setTimeout(resolve, FILESYSTEM_HOST_RECOVERY_DELAY_MS)) - const recovered = await orcaPage.evaluate(async () => { - await window.api.rateLimits.refreshGrok() - return await window.api.grokAccounts.getStatus() - }) - expect(recovered).toMatchObject({ - stale: false, - availability: 'ready', - signedIn: true, - email: 'recovered@example.invalid' - }) + await expect + .poll( + () => + orcaPage.evaluate(async () => { + await window.api.rateLimits.refreshGrok() + return await window.api.grokAccounts.getStatus() + }), + { timeout: FILESYSTEM_HOST_RECOVERY_DELAY_MS + 15_000, intervals: [2_000] } + ) + .toMatchObject({ + stale: false, + availability: 'ready', + signedIn: true, + email: 'recovered@example.invalid' + })config/scripts/verify-packaged-filesystem-host-entry.cjs (1)
46-50: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGuard against a null
stdoutin the failure path.
spawnSyncreturnsnullforstdoutandstderrwhen the child is terminated before the streams are captured, for example on a signal kill withoutresult.error. Line 46 then throws aTypeErrorinstead of the intended diagnostic error. Normalize both streams first.♻️ Proposed change
- if (result.status !== 0 || !result.stdout.includes('"protocolVersion":1')) { + const stdout = result.stdout || '' + const stderr = result.stderr || '' + if (result.status !== 0 || !stdout.includes('"protocolVersion":1')) { throw new Error( - `[verify-packaged-filesystem-host-entry] self-test failed: ${result.stderr || result.stdout}` + `[verify-packaged-filesystem-host-entry] self-test failed: ${stderr || stdout}` ) }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9567d81b-7c00-44a3-aa80-6fd34c303e5a
📒 Files selected for processing (149)
config/electron-builder.config.cjsconfig/scripts/electron-builder-config.test.mjsconfig/scripts/electron-vite-output-contract.test.tsconfig/scripts/package-electron-runtime-contract.test.mjsconfig/scripts/verify-packaged-filesystem-host-entry.cjsconfig/scripts/verify-packaged-filesystem-host-entry.test.mjsconfig/tsconfig.cli.jsonconfig/vitest-filesystem-host-read-client.tsconfig/vitest.config.tselectron.vite.config.tspackage.jsonsrc/main/agent-hooks/install-status-snapshot-store.test.tssrc/main/agent-hooks/install-status-snapshot-store.tssrc/main/agent-hooks/managed-agent-hook-controls.test.tssrc/main/agent-hooks/managed-agent-hook-controls.tssrc/main/claude-accounts/runtime-auth-service.test.tssrc/main/claude-accounts/runtime-auth-service.tssrc/main/codex-accounts/service.test.tssrc/main/codex-accounts/service.tssrc/main/filesystem-host/__fixtures__/filesystem-host-hang-fixture.cjssrc/main/filesystem-host/filesystem-host-breaker-blast-radius.test.tssrc/main/filesystem-host/filesystem-host-breaker.test.tssrc/main/filesystem-host/filesystem-host-breaker.tssrc/main/filesystem-host/filesystem-host-capacity.test.tssrc/main/filesystem-host/filesystem-host-capacity.tssrc/main/filesystem-host/filesystem-host-entry-path.test.tssrc/main/filesystem-host/filesystem-host-entry-path.tssrc/main/filesystem-host/filesystem-host-entry.tssrc/main/filesystem-host/filesystem-host-env.test.tssrc/main/filesystem-host/filesystem-host-env.tssrc/main/filesystem-host/filesystem-host-failure-domain.test.tssrc/main/filesystem-host/filesystem-host-failure-domain.tssrc/main/filesystem-host/filesystem-host-fault-injection.test.tssrc/main/filesystem-host/filesystem-host-idle-process-reclamation.tssrc/main/filesystem-host/filesystem-host-operation.test.tssrc/main/filesystem-host/filesystem-host-operation.tssrc/main/filesystem-host/filesystem-host-process-error.tssrc/main/filesystem-host/filesystem-host-process-retirement.tssrc/main/filesystem-host/filesystem-host-process.test.tssrc/main/filesystem-host/filesystem-host-process.tssrc/main/filesystem-host/filesystem-host-read-authority.test.tssrc/main/filesystem-host/filesystem-host-read-authority.tssrc/main/filesystem-host/filesystem-host-read-requests.tssrc/main/filesystem-host/filesystem-host-supervisor-error.tssrc/main/filesystem-host/filesystem-host-supervisor-execution.tssrc/main/filesystem-host/filesystem-host-supervisor-health.tssrc/main/filesystem-host/filesystem-host-supervisor-lifecycle.test.tssrc/main/filesystem-host/filesystem-host-supervisor-scheduling.tssrc/main/filesystem-host/filesystem-host-supervisor-telemetry.tssrc/main/filesystem-host/filesystem-host-supervisor.test.tssrc/main/filesystem-host/filesystem-host-supervisor.tssrc/main/filesystem-host/filesystem-host-telemetry.tssrc/main/git/orca-yaml-snapshot-store.test.tssrc/main/git/orca-yaml-snapshot-store.tssrc/main/git/status.test.tssrc/main/git/status.tssrc/main/git/worktree-shared-directories.test.tssrc/main/git/worktree-shared-directories.tssrc/main/grok-accounts/status.test.tssrc/main/grok-accounts/status.tssrc/main/index.tssrc/main/ipc/agent-hooks.test.tssrc/main/ipc/agent-hooks.tssrc/main/ipc/app.tssrc/main/ipc/codex-accounts.tssrc/main/ipc/filesystem-auth.test.tssrc/main/ipc/filesystem-auth.tssrc/main/ipc/filesystem-import-ssh.tssrc/main/ipc/filesystem-mutations.tssrc/main/ipc/filesystem-watcher.tssrc/main/ipc/filesystem.tssrc/main/ipc/floating-workspace-directory.tssrc/main/ipc/keybindings.test.tssrc/main/ipc/keybindings.tssrc/main/ipc/minimax-credentials.test.tssrc/main/ipc/minimax-credentials.tssrc/main/ipc/orca-profiles.test.tssrc/main/ipc/orca-profiles.tssrc/main/ipc/repos.tssrc/main/ipc/speech.test.tssrc/main/ipc/speech.tssrc/main/ipc/worktrees.test.tssrc/main/ipc/worktrees.tssrc/main/keybindings/keybinding-file.tssrc/main/keybindings/keybinding-service.test.tssrc/main/keybindings/keybinding-service.tssrc/main/minimax/minimax-cookie-store.test.tssrc/main/minimax/minimax-cookie-store.tssrc/main/network/macos-tailscale-dns-diagnostic.test.tssrc/main/network/macos-tailscale-dns-diagnostic.tssrc/main/orca-profiles/profile-index-store.test.tssrc/main/orca-profiles/profile-index-store.tssrc/main/orca-profiles/profile-list-snapshot-store.tssrc/main/rate-limits/claude-fetcher.test.tssrc/main/rate-limits/claude-fetcher.tssrc/main/rate-limits/claude-pty.test.tssrc/main/rate-limits/claude-pty.tssrc/main/rate-limits/codex-fetcher-auth-errors.test.tssrc/main/rate-limits/codex-fetcher-backend.test.tssrc/main/rate-limits/codex-fetcher-probe-shutdown.test.tssrc/main/rate-limits/codex-fetcher-pty-settle.test.tssrc/main/rate-limits/codex-fetcher-session-supplement.test.tssrc/main/rate-limits/codex-fetcher.test.tssrc/main/rate-limits/codex-fetcher.tssrc/main/rate-limits/gemini-oauth-preparation-snapshot.test.tssrc/main/rate-limits/gemini-oauth-preparation-snapshot.tssrc/main/rate-limits/gemini-oauth-sources.tssrc/main/rate-limits/gemini-usage-fetcher.fallback.test.tssrc/main/rate-limits/gemini-usage-fetcher.test.tssrc/main/rate-limits/gemini-usage-fetcher.tssrc/main/rate-limits/grok-auth-snapshot.test.tssrc/main/rate-limits/grok-auth-snapshot.tssrc/main/rate-limits/grok-auth.test.tssrc/main/rate-limits/grok-auth.tssrc/main/rate-limits/grok-fetcher.test.tssrc/main/rate-limits/grok-fetcher.tssrc/main/rate-limits/hidden-rate-limit-pty-cwd.tssrc/main/rate-limits/kimi-fetcher.test.tssrc/main/rate-limits/kimi-fetcher.tssrc/main/rate-limits/memory-snapshot-loader-boundedness.test.tssrc/main/rate-limits/memory-snapshot-store.test.tssrc/main/rate-limits/memory-snapshot-store.tssrc/main/rate-limits/service.test.tssrc/main/rate-limits/service.tssrc/main/runtime/orca-runtime.test.tssrc/main/runtime/orca-runtime.tssrc/main/speech/openai-api-key-store.test.tssrc/main/speech/openai-api-key-store.tssrc/main/startup/desktop-startup-ordering.test.tssrc/main/updater.test.tssrc/preload/api-types.tssrc/preload/index.tssrc/renderer/src/components/settings/GrokAccountsSection.test.tsxsrc/renderer/src/components/settings/GrokAccountsSection.tsxsrc/renderer/src/components/settings/VoicePane.tsxsrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/runtime/runtime-hooks-client.tssrc/renderer/src/web/web-preload-api.test.tssrc/renderer/src/web/web-preload-api.tssrc/shared/agent-hook-types.tssrc/shared/filesystem-host-protocol.test.tssrc/shared/filesystem-host-protocol.tssrc/shared/memory-snapshot.tssrc/shared/orca-yaml.tssrc/shared/rate-limit-types.tssrc/shared/speech-types.tssrc/shared/types.tstests/e2e/freeze-safety-liveness.spec.tstests/e2e/helpers/freeze-safety-liveness.ts
🚧 Files skipped from review as they are similar to previous changes (123)
- config/tsconfig.cli.json
- package.json
- src/main/claude-accounts/runtime-auth-service.test.ts
- src/main/filesystem-host/fixtures/filesystem-host-hang-fixture.cjs
- src/main/orca-profiles/profile-index-store.test.ts
- src/main/filesystem-host/filesystem-host-env.test.ts
- config/vitest-filesystem-host-read-client.ts
- src/main/ipc/keybindings.test.ts
- config/scripts/electron-vite-output-contract.test.ts
- src/main/agent-hooks/managed-agent-hook-controls.ts
- src/main/ipc/codex-accounts.ts
- src/main/filesystem-host/filesystem-host-env.ts
- src/main/filesystem-host/filesystem-host-supervisor-telemetry.ts
- src/main/git/status.ts
- src/main/git/status.test.ts
- src/main/startup/desktop-startup-ordering.test.ts
- src/main/filesystem-host/filesystem-host-supervisor-health.ts
- src/main/runtime/orca-runtime.test.ts
- src/main/ipc/filesystem-import-ssh.ts
- src/shared/speech-types.ts
- src/main/ipc/speech.ts
- src/main/filesystem-host/filesystem-host-supervisor-error.ts
- config/scripts/electron-builder-config.test.mjs
- src/main/filesystem-host/filesystem-host-capacity.ts
- src/main/orca-profiles/profile-list-snapshot-store.ts
- electron.vite.config.ts
- src/main/rate-limits/grok-auth-snapshot.ts
- src/shared/filesystem-host-protocol.test.ts
- src/shared/agent-hook-types.ts
- src/main/agent-hooks/managed-agent-hook-controls.test.ts
- src/main/filesystem-host/filesystem-host-capacity.test.ts
- src/main/ipc/filesystem-mutations.ts
- src/renderer/src/components/settings/VoicePane.tsx
- src/main/ipc/filesystem.ts
- config/electron-builder.config.cjs
- src/main/rate-limits/grok-fetcher.ts
- src/main/rate-limits/grok-auth.ts
- src/main/filesystem-host/filesystem-host-entry-path.test.ts
- src/main/rate-limits/memory-snapshot-loader-boundedness.test.ts
- src/main/ipc/agent-hooks.test.ts
- src/main/ipc/filesystem-auth.ts
- src/main/ipc/worktrees.ts
- src/main/filesystem-host/filesystem-host-fault-injection.test.ts
- src/main/ipc/speech.test.ts
- src/main/rate-limits/grok-auth.test.ts
- src/main/rate-limits/claude-pty.ts
- src/main/ipc/floating-workspace-directory.ts
- src/main/rate-limits/grok-auth-snapshot.test.ts
- src/main/rate-limits/codex-fetcher-session-supplement.test.ts
- src/main/filesystem-host/filesystem-host-entry.ts
- src/main/minimax/minimax-cookie-store.test.ts
- src/main/filesystem-host/filesystem-host-entry-path.ts
- src/main/keybindings/keybinding-file.ts
- src/main/orca-profiles/profile-index-store.ts
- src/main/rate-limits/gemini-usage-fetcher.test.ts
- src/main/filesystem-host/filesystem-host-failure-domain.ts
- src/main/network/macos-tailscale-dns-diagnostic.test.ts
- src/main/filesystem-host/filesystem-host-telemetry.ts
- src/main/grok-accounts/status.ts
- src/main/agent-hooks/install-status-snapshot-store.test.ts
- src/renderer/src/runtime/runtime-hooks-client.ts
- src/main/claude-accounts/runtime-auth-service.ts
- src/main/ipc/orca-profiles.test.ts
- src/main/rate-limits/memory-snapshot-store.ts
- src/main/filesystem-host/filesystem-host-process-error.ts
- src/main/ipc/filesystem-watcher.ts
- src/main/codex-accounts/service.test.ts
- src/main/filesystem-host/filesystem-host-breaker-blast-radius.test.ts
- src/main/filesystem-host/filesystem-host-breaker.test.ts
- src/main/rate-limits/codex-fetcher-pty-settle.test.ts
- src/main/grok-accounts/status.test.ts
- src/main/keybindings/keybinding-service.test.ts
- src/main/ipc/repos.ts
- src/main/rate-limits/gemini-usage-fetcher.fallback.test.ts
- src/main/ipc/keybindings.ts
- src/shared/rate-limit-types.ts
- src/main/filesystem-host/filesystem-host-supervisor-execution.ts
- src/main/ipc/filesystem-auth.test.ts
- src/renderer/src/web/web-preload-api.ts
- src/main/git/orca-yaml-snapshot-store.test.ts
- src/main/rate-limits/codex-fetcher-auth-errors.test.ts
- src/main/filesystem-host/filesystem-host-operation.ts
- src/main/ipc/orca-profiles.ts
- src/shared/types.ts
- src/main/rate-limits/memory-snapshot-store.test.ts
- src/main/rate-limits/hidden-rate-limit-pty-cwd.ts
- src/main/rate-limits/gemini-oauth-preparation-snapshot.ts
- src/main/filesystem-host/filesystem-host-supervisor.test.ts
- config/vitest.config.ts
- src/shared/filesystem-host-protocol.ts
- src/main/agent-hooks/install-status-snapshot-store.ts
- src/preload/api-types.ts
- src/shared/orca-yaml.ts
- src/main/rate-limits/kimi-fetcher.test.ts
- src/main/codex-accounts/service.ts
- tests/e2e/helpers/freeze-safety-liveness.ts
- src/main/rate-limits/gemini-usage-fetcher.ts
- src/main/filesystem-host/filesystem-host-read-authority.test.ts
- src/renderer/src/web/web-preload-api.test.ts
- src/main/rate-limits/grok-fetcher.test.ts
- src/main/filesystem-host/filesystem-host-breaker.ts
- src/main/rate-limits/claude-fetcher.ts
- src/main/rate-limits/claude-pty.test.ts
- src/main/index.ts
- src/main/speech/openai-api-key-store.test.ts
- src/main/keybindings/keybinding-service.ts
- src/main/ipc/worktrees.test.ts
- src/main/git/orca-yaml-snapshot-store.ts
- src/main/filesystem-host/filesystem-host-read-authority.ts
- src/main/ipc/agent-hooks.ts
- src/main/rate-limits/gemini-oauth-sources.ts
- src/main/rate-limits/codex-fetcher.ts
- src/main/rate-limits/codex-fetcher.test.ts
- src/main/filesystem-host/filesystem-host-supervisor-scheduling.ts
- src/main/rate-limits/kimi-fetcher.ts
- src/main/rate-limits/claude-fetcher.test.ts
- src/main/filesystem-host/filesystem-host-operation.test.ts
- src/main/ipc/minimax-credentials.test.ts
- src/main/rate-limits/service.ts
- src/main/filesystem-host/filesystem-host-read-requests.ts
- src/main/filesystem-host/filesystem-host-failure-domain.test.ts
- src/main/rate-limits/codex-fetcher-probe-shutdown.test.ts
- src/preload/index.ts
edee189 to
81edd9b
Compare
A synchronous fs call against a stalled mount parks the Electron main thread in an uninterruptible wait that no main-thread timer can bound: the timeout callback queues on the very loop that is blocked. Making the call async only relocates it into the libuv threadpool, where poolSize concurrent stalls wedge every async fs caller app-wide. Route main-process reads through a forked filesystem host instead. The parent is not the stuck process, so a parent-side timer over the IPC reply is a real bound, and a wedged child is physically retired rather than left as an abandoned promise holding a threadpool slot. - Failure-domain lanes keyed by mount so one stalled share cannot starve reads against healthy paths. - Per-lane circuit breaker (closed/open/probe) with a recovery delay. - Foreground/background admission over a bounded child pool, with background capped below the physical maximum. - Packaging checks that the host entry ships and self-tests. Co-authored-by: Orca <help@stably.ai>
…reads get() is a pure memory read returning value plus staleness, age and availability, so an IPC handler never touches the filesystem to answer. Refreshes are single-flight and generation-fenced, so a slow read that lands after a newer publish is discarded rather than resurrecting stale state. Co-authored-by: Orca <help@stably.ai>
…ount Drives a real stall through a FIFO-backed path and asserts the app stays live: the event loop keeps ticking, unrelated reads still resolve, and the lane recovers once the stall clears. Co-authored-by: Orca <help@stably.ai>
…c reads Cuts the main-process status readers over to the snapshot stores and the filesystem host: agent-hook install status, orca.yaml hooks and worktree shared directories, rate-limit and account status, keybindings, profiles, MiniMax and speech credentials, and the macOS Tailscale DNS diagnostic. Handlers now answer from memory and report staleness to the renderer, so a stalled mount degrades a status chip instead of parking the UI. The scutil DNS probe moves off execFileSync for the same reason. Co-authored-by: Orca <help@stably.ai>
…dget Release-audit fixes on top of the filesystem-host cut. Liveness regressions vs main (serving status from memory is right; reading credentials once at launch was not): - re-hydrate provider snapshots at the start of each rate-limit fetch cycle, so a token the CLI rotates out-of-band is picked up on the next poll instead of stranding usage on 401 for the rest of the session - restore the read-and-retry after Claude CLI repair, which is the mechanism by which an expired token self-heals - re-hydrate the Codex system-default identity on an accounts read, so an out-of-band `codex login` stops reporting "no sign-in found for this Mac" - read agent-hook status from disk in the standalone CLI, where nothing publishes snapshots and every agent was reported as `error` Supervisor: - refuse to re-fork a failure domain that still holds an unreaped child, so a mount wedged in an uninterruptible syscall costs one slot instead of draining the process-wide budget one child per breaker probe - reserve queue headroom for foreground work, mirroring the physical-slot reservation, so a background burst can't strand the fs IPC gate Also: restore main's authorize-then-canonicalize ordering so a stalled host can't revoke a drop grant or abort a batch, and cap host text results by bytes on the parent side as well as the child.
…semantics The forked host canonicalized with realpathSync.native, but every call site that moved into it used the JS realpath. On Windows the two disagree: .native folds a path to its true on-disk casing and rewrites a virtual/mapped drive to its backing path, so results compared against textually-recorded allow-list and worktree roots could change outcome. Verified on Windows: `X:\orca` canonicalizes to `X:\orca` under the JS realpath and to `C:\Users\...\orca` under .native. Pin the operation tests to realpathSync so the divergence cannot return, and document why the vitest host client must mirror the child's implementation -- it wires the JS realpath, so a .native child would make every consumer test run different semantics than production.
81edd9b to
4804cff
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
Final current-head verificationPR #12149 is rebased onto current main, and current-head CI and review are terminal green.
Final hardening includes:
Validation:
|
1 participant
Problem
A synchronous
fscall against a stalled mount (SMB/NFS share that stops answering, a hungsshfs, a Time Machine volume mid-spindown) parks the Electron main thread in an uninterruptible wait. On macOS the process entersDstate and ignoresSIGKILL, so the app cannot even be Force-Quit. The window is frozen, the menu bar is frozen, and the only exit is a reboot or the mount timing out on its own.Two things that look like fixes are not:
setTimeoutcannot bound a main-thread block. The timeout callback is queued on the very loop that is blocked, so it fires only after the syscall it was meant to bound has already returned.sync→asyncis not a bound either. It relocates the syscall into the libuv threadpool (defaultpoolSize4). At exactlypoolSizeconcurrent stalls, every asyncfscaller in the process stops — app-wide. The event loop does keep ticking (repaint, menus, Force Quit survive), so this is strictly better than the sync case, but it is not a bound.What actually bounds it is a process that is not the stuck process. A forked child does the read; the parent runs a timer over the IPC reply. If the child does not answer, the parent gives up and physically retires the child — abandoning the promise alone would leave the libuv operation alive and its thread consumed.
What this PR does
1. Forked filesystem host for main-process reads (
src/main/filesystem-host/)AUTHORIZATION_DEADLINE_MS = 2sforeground,BACKGROUND_READ_DEADLINE_MS = 5s).${executionHost}:unknownlane.closed/open/probe) with a 30s recovery delay, so a dead mount is not re-probed on every poll.SIGTERM, thenSIGKILLafter 1s, with an exit deadline so a truly unkillable child is accounted for rather than silently leaked.2. Memory-first snapshot stores (
src/shared/memory-snapshot.ts,src/main/rate-limits/memory-snapshot-store.ts)get()is a pure memory read that returns{ value, stale, age, availability }. An IPC handler answers from memory and never touches the filesystem. Refreshes are single-flight (N concurrent pollers collapse into one read) and generation-fenced, so a slow read that lands after a newer publish is discarded instead of resurrecting stale state.3. Consumer cutover (
src/main/ipc/,src/main/agent-hooks/,src/main/git/,src/main/rate-limits/, …)Status readers that previously did sync
fswork inside an IPC handler now answer from a snapshot and report staleness to the renderer:getStatus()handlers)orca.yamlhooks andworktree.sharedDirectoriesexecFileSyncA stalled mount now degrades a status chip instead of parking the UI.
4. One e2e gate (
tests/e2e/freeze-safety-liveness.spec.ts)Drives a real stall through a FIFO-backed path and asserts the app stays live: the event loop keeps ticking, unrelated reads still resolve, and the lane recovers once the stall clears.
Follow-up work (explicitly not in this PR)
This PR was cut down from a much larger branch to the load-bearing fix, so it is reviewable and landable. The cut was deliberate, and the work below is deferred, not abandoned. Listing it here so nothing is silently lost between this merge and the follow-ups.
#12015 (
nwparker/main-thread-sync-fs-sweep) is the superseded predecessor of this PR and holds the reference implementation for every item below. It should stay open until these follow-ups exist, or be closed only with this list carried forward — 72 of its 97 files have no counterpart here.1. The out-of-process write path
Credential and config writes still use the existing synchronous
writeSecureFile/writeFileSyncprimitives, unchanged frommain. This PR does not make them worse and does not fix them. A fenced out-of-process mutation protocol is its own change, with its own crash-consistency and partial-write story.Reference implementation in #12015:
src/shared/secure-file.ts,src/shared/secure-path-hardening-snapshot.ts,src/main/agent-hooks/hooks-json-async-write.ts,src/main/orca-profiles/profile-index-async-store.ts.2. Agent hook service sweep
Roughly a dozen hook services still do sync filesystem work on the main thread:
amp,antigravity,claude,command-code,copilot,cursor,devin,droid,gemini,grok,hermes,kimi— plussrc/main/hooks.tsandsrc/main/agent-hooks/hooks-json-read.ts.These are not on a poll interval, which is why they were cut. They are still main-thread sync reads and still stall on a hung mount.
3. Remaining main-thread sync read sites
Outside the status paths this PR covers:
src/main/codex/config-toml-trust.ts,src/main/agent-trust-presets.ts,src/main/ipc/diagnostics.ts,src/main/rate-limits/grok-auth.ts,src/main/speech/openai-api-key-store.ts,src/main/codex-accounts/fs-utils.ts.This PR covers the read paths IPC handlers hit on a poll interval, which is where a stall becomes a freeze. The rest are lower frequency, not lower risk.
4. libuv threadpool sizing
src/main/libuv-threadpool-size.tsin #12015. The default pool is 4; async filesystem work that is not routed through the forked host still contends for it. Relevant once more of the above moves to async rather than out-of-process.5. Probes and reproduction harnesses
Stall probes and the live-remote freeze repro harnesses, including
config/scripts/libuv-threadpool-starvation-probe.mjs.6. Ratchets and baselines
Blocking-IO and hot-path ratchets plus their generated baselines. These need a baseline commit and a CI gate of their own; folding them in here would have made this PR unreviewable.
Related, but independent
#11841 (
nwparker/cold-face-Orca-freeze) is separate freeze work — terminal focus coalescing plus repro harnesses. It shares exactly one file with this PR (package.json). It is not superseded by this merge and is not blocked by it.Testing
npx tsc --noEmit -p config/tsconfig.node.json— clean.npx vitest run --config config/vitest.config.ts src mobile config/scripts— 44,174 passed, 78 skipped. 3 failures, all verified as pre-existing and not from this branch: the 2 insrc/relay/agent-exec-handler.test.tsand the 1 insrc/main/updater.test.tseach fail identically on a cleanorigin/maincheckout, and this PR touches no file undersrc/relayor any updater file.oxlint(default + react-doctor config) andoxfmt --checkclean over the changed files.Release-scan readiness
An 8-seat pre-release audit was run against this diff before review. Findings and their disposition are in this comment — every P0 and P1 is fixed in-branch, and the remaining P2s are listed there as accepted residuals with reasons rather than left for a scan to rediscover.
Headline fixes that came out of it:
orca agent hooks statusreads from disk again in the standalone CLI, where nothing publishes snapshots.Validation: 3/3 typechecks clean,
pnpm lintclean, 43,917 tests passing, and the packaging chain verified end to end through theafterPackself-test boot.Not claimed: the forked child is a hang boundary, not a privilege boundary. It bounds how long a stalled mount can hold a read; it is not a sandbox.