[comment audit] Shorten top-level main process L–O comments (#32295)

* chore: shorten process-host comments

* chore: audit CLI parsing and host-routing comments

* chore: remove stale host-selector restriction comment

* chore: shorten remaining CLI root comments

* Shorten CLI handler comments

* Shorten CLI runtime and command specification comments

* Shorten top-level main process A–G comments

* [comment audit] Shorten main process H–K comments

* [comment audit] Shorten main process L–O comments

---------

Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>

src/main/lazy-worker-thread-host.ts

@@ -8,13 +8,7 @@ export type WorkerRequestTransport = Pick<Worker, 'unref' | 'postMessage' | 'ter
 
 export type WorkerThreadFactory = () => WorkerRequestTransport
 
-/**
- * Owns the lifetime of one lazily-spawned worker thread: spawn on demand,
- * listener wiring, teardown, and idle expiry. It holds no request state, so
- * every decision about which call a message belongs to stays with its client,
- * and a failed spawn is reported rather than thrown so the client can fail its
- * queued calls closed instead of moving the work back onto the main thread.
- */
+/** Clients own request state and must fail queued calls when spawning fails. */
 export class LazyWorkerThreadHost<TResponse> {
   private worker: WorkerRequestTransport | null = null
   private idleTimer: NodeJS.Timeout | null = null
@@ -29,14 +23,11 @@ export class LazyWorkerThreadHost<TResponse> {
       onMessage: (response: TResponse) => void
       onError: (error: Error) => void
       onExit: (code: number) => void
-      /** Nothing active and nothing queued, checked again when the idle timer fires. */
+      /** No active or queued requests; checked again at idle expiry. */
       isIdle: () => boolean
-      /** First spawn failure only: a repeating one must not repeat the log. */
+      /** Called only for the first spawn failure. */
       onUnavailable: (error: unknown) => void
-      /**
-       * No replacement until a terminated worker has exited (#24572): terminate()
-       * cannot interrupt a native call, so respawning would stack threads on it.
-       */
+      /** Native calls can delay termination; wait for exit before spawning another worker (#24572). */
       awaitRetirement?: boolean
     }
   ) {}
@@ -45,12 +36,11 @@ export class LazyWorkerThreadHost<TResponse> {
     return this.worker
   }
 
-  /** A terminated worker has not exited yet, so `ensure` refuses to spawn. */
   get isRetiring(): boolean {
     return this.retiring !== null
   }
 
-  /** The live worker, spawning one if needed; null when no worker can be had. */
+  /** Returns null if spawning fails or a previous worker is still retiring. */
   ensure(): WorkerRequestTransport | null {
     if (this.worker) {
       return this.worker
@@ -110,8 +100,7 @@ export class LazyWorkerThreadHost<TResponse> {
     }
     this.idleTimer = setTimeout(() => {
       this.idleTimer = null
-      // Re-checked here: a request arriving as the timer fires must never be
-      // lost to a self-exiting worker.
+      // Recheck idle state so requests arriving during the delay survive.
       if (this.options.isIdle()) {
         this.destroy()
       }

src/main/linux-lid-sleep-assertion.ts

@@ -72,8 +72,7 @@ export class LinuxLidSleepAssertion {
 
     let child: SystemdInhibitProcess
     try {
-      // logind's lid switch handling ignores ordinary sleep inhibitors on many systems,
-      // so Linux needs both a sleep lock and a handle-lid-switch lock.
+      // logind may ignore sleep inhibitors for lid events; acquire both locks.
       child = this.spawn(
         'systemd-inhibit',
         [

src/main/linux-package-install-command.test.ts

@@ -5,7 +5,6 @@ const { statSyncMock } = vi.hoisted(() => ({ statSyncMock: vi.fn() }))
 
 vi.mock('node:fs', () => ({ statSync: statSyncMock }))
 
-// Executables the fake filesystem exposes, keyed by absolute path.
 let executables: Map<string, { file: boolean; mode: number }>
 
 function install(absolutePath: string, options?: { file?: boolean; mode?: number }): void {
@@ -122,7 +121,7 @@ describe('resolveTrustedExecutable', () => {
 
 describe('buildLinuxPackageInstallCommand', () => {
   it('refuses a non-absolute package path before looking for anything', async () => {
-    // Why: zypper/dnf/yum/rpm take no `--`, so a relative or dash-leading path would become an option.
+    // Managers without -- support require absolute paths to prevent option injection.
     install('/usr/bin/sudo')
     install('/usr/bin/apt')
     const { buildLinuxPackageInstallCommand } = await loadCommandModule()
@@ -265,7 +264,7 @@ describe('hasTrustedPackageManagerFor', () => {
     expect(hasTrustedPackageManagerFor('deb')).toBe(true)
   })
 
-  // The #17702 case: an Arch rebuild of the .deb inherits the marker but has no deb tooling.
+  // Repackagers can inherit deb/rpm markers without the corresponding tooling (#17702).
   it('rejects a deb marker on a host with only pacman', async () => {
     install('/usr/bin/pacman')
     install('/usr/bin/sudo')

src/main/linux-package-install-command.ts

@@ -3,7 +3,7 @@ import { statSync } from 'node:fs'
 import path from 'node:path'
 import type { LinuxRootPackageType } from '../shared/update-status-types'
 
-// Why: an absolute but user-writable PATH entry must never be treated as a trusted package manager.
+// User-writable PATH entries cannot be trusted for privileged package installation.
 const TRUSTED_EXECUTABLE_DIRECTORIES = ['/usr/bin', '/bin', '/usr/sbin', '/sbin']
 
 const DEB_PACKAGE_MANAGERS: { name: string; args: string[] }[] = [
@@ -11,8 +11,7 @@ const DEB_PACKAGE_MANAGERS: { name: string; args: string[] }[] = [
   { name: 'dpkg', args: ['-i', '--'] }
 ]
 
-// No `--` terminator: these tools do not accept one. Safe because capture requires an absolute path,
-// so the argument can never be read as an option.
+// These tools lack -- support; absolute package paths prevent option injection.
 const RPM_PACKAGE_MANAGERS: { name: string; args: string[] }[] = [
   {
     name: 'zypper',
@@ -27,47 +26,31 @@ export type LinuxPackageInstallCommandResult =
   | { ok: true; command: string }
   | { ok: false; reason: 'no-sudo' | 'no-package-manager' | 'invalid-package-path' }
 
-/**
- * Resolves an executable strictly from the trusted system directories. A symlink inside those
- * directories is fine — its target is what `statSync` checks — but nothing outside them is consulted
- * and no shell is ever invoked for discovery.
- */
 export function resolveTrustedExecutable(name: string): string | null {
   for (const directory of TRUSTED_EXECUTABLE_DIRECTORIES) {
-    // posix.join: these are POSIX paths, and this module only ever runs on Linux.
     const candidate = path.posix.join(directory, name)
     try {
       const stats = statSync(candidate)
       if (stats.isFile() && (stats.mode & 0o111) !== 0) {
         return candidate
       }
-    } catch {
-      // Absent here; keep looking in the remaining trusted directories.
-    }
+    } catch {}
   }
   return null
 }
 
-/**
- * Whether this host has any package manager able to install the marker's format. A repackaged
- * install (AUR, Nix, a container rebuild) inherits the `package-type` marker from the .deb/.rpm it
- * was built from, so the marker alone never proves the host can act on it.
- */
+/** Repackaged installs can inherit a deb/rpm marker without a matching package manager. */
 export function hasTrustedPackageManagerFor(packageType: LinuxRootPackageType): boolean {
   const candidates = packageType === 'deb' ? DEB_PACKAGE_MANAGERS : RPM_PACKAGE_MANAGERS
   return candidates.some((candidate) => resolveTrustedExecutable(candidate.name) !== null)
 }
 
-/**
- * Builds the interactive command the user pastes into their own terminal. Every token except the
- * package path is a fixed literal, and the path is POSIX-single-quoted — Orca never runs this.
- */
+/** Produces a command for the user to run and confirm in their terminal. */
 export function buildLinuxPackageInstallCommand(
   packageType: LinuxRootPackageType,
   packagePath: string
 ): LinuxPackageInstallCommandResult {
-  // Why: several package managers accept no `--` terminator, so a relative or dash-leading path would
-  // be read as an option. Hold that property here rather than relying on a caller two modules away.
+  // Absolute paths prevent option injection in managers that lack -- support.
   if (!path.isAbsolute(packagePath)) {
     return { ok: false, reason: 'invalid-package-path' }
   }

src/main/linux-package-install-diagnostic.ts

@@ -62,7 +62,7 @@ export function redactLinuxPackageInstallText(
   if (homeDir) {
     text = replaceAllLiteral(text, homeDir, '<home>')
   }
-  // Why: privilege tools can name the user without including their home directory.
+  // Privilege tools can name the user without including their home directory.
   const userName = readUserName()
   if (userName && userName.length >= MIN_REDACTED_USERNAME_LENGTH) {
     text = replaceAllLiteral(text, userName, '<user>')

src/main/linux-package-update-recovery.test.ts

@@ -20,7 +20,6 @@ vi.mock('./linux-package-install-command', () => ({
   buildLinuxPackageInstallCommand: buildCommandMock
 }))
 
-// Counts hash passes without changing read behavior.
 vi.mock('node:fs', async (importOriginal) => {
   const actual = await importOriginal<typeof NodeFs>()
   return {
@@ -32,7 +31,7 @@ vi.mock('node:fs', async (importOriginal) => {
   }
 })
 
-// Real symlinks, POSIX modes, and the XDG cache layout make these cases meaningless on Windows.
+// These fixtures require POSIX modes, symlinks, and XDG cache paths.
 const describePosix = describe.skipIf(process.platform === 'win32')
 
 const VERSION = '1.2.3'
@@ -62,7 +61,6 @@ async function writePackage(name: string, contents = PAYLOAD): Promise<string> {
   return filePath
 }
 
-/** Captures a well-formed downloaded event unless a field is overridden. */
 function capture(overrides: Record<string, unknown> = {}): LinuxPackageArtifact | null {
   const downloadedFile = (overrides.downloadedFile ?? path.join(downloadDir, 'orca.deb')) as string
   return recovery.captureLinuxPackageArtifact({
@@ -81,7 +79,6 @@ beforeEach(async () => {
   tempRoot = await fsp.mkdtemp(path.join(os.tmpdir(), 'orca-recovery-'))
   cacheRoot = path.join(tempRoot, 'cache')
   updaterDir = path.join(cacheRoot, 'orca-updater')
-  // The only shape electron-updater downloads into: <cacheRoot>/<updaterCacheDirName>/pending.
   downloadDir = path.join(updaterDir, 'pending')
   outsideDir = path.join(tempRoot, 'outside')
   await fsp.mkdir(downloadDir, { recursive: true })
@@ -197,7 +194,7 @@ describe('captureLinuxPackageArtifact', () => {
   })
 
   it('refuses a digest that cannot decode to 64 bytes', () => {
-    // Why: an undecodable digest is a metadata problem — arming here would blame the user's file.
+    // Invalid release metadata must not arm recovery that blames the downloaded file.
     capture({
       files: [{ url: 'orca.deb', sha512: createHash('sha256').update(PAYLOAD).digest('base64') }]
     })
@@ -205,7 +202,7 @@ describe('captureLinuxPackageArtifact', () => {
   })
 
   it('refuses a digest with characters outside the base64 alphabet', () => {
-    // Buffer.from would silently drop the '!', so the round-trip check must reject this.
+    // Buffer.from silently drops invalid characters unless checked by a round trip.
     capture({ files: [{ url: 'orca.deb', sha512: `${SHA512.slice(0, 4)}!${SHA512.slice(4)}` }] })
     expect(recovery.getTrackedLinuxPackageArtifact()).toBeNull()
   })
@@ -491,7 +488,6 @@ describePosix('cache containment', () => {
   })
 
   it('uses the home cache when XDG_CACHE_HOME is unset', async () => {
-    // Why: the fallback is the only reason an unset XDG_CACHE_HOME still finds the real download.
     const homeCache = path.join(tempRoot, 'home')
     const homeDownloadDir = path.join(homeCache, '.cache', 'orca-updater', 'pending')
     await fsp.mkdir(homeDownloadDir, { recursive: true })
@@ -508,11 +504,7 @@ describePosix('cache containment', () => {
   })
 })
 
-// electron-updater builds `downloadedFile` from the cache's own writable update-info.json without
-// calling basename, so these pin the accepted shape to the one directory it actually downloads into.
-// Defence in depth only — a same-uid process can create its own `pending` directory, and per the
-// design's trust model it can already overwrite the real cached package. The load-bearing check is the
-// cache-root anchor covered by the `cache containment` describe above.
+// Pending-directory shape is defense in depth; cache-root containment excludes other users’ swap locations.
 describePosix('pending-directory anchoring', () => {
   it('rejects a package in an attacker-owned directory under the cache root', async () => {
     const evilDir = path.join(cacheRoot, 'evil')
@@ -561,7 +553,6 @@ describePosix('pending-directory anchoring', () => {
   })
 
   it('accepts any updater directory name that holds the pending directory', async () => {
-    // The cache directory name comes from the app, so only its position is fixed.
     const otherPending = path.join(cacheRoot, 'orca-updater-next', 'pending')
     await fsp.mkdir(otherPending, { recursive: true })
     const filePath = path.join(otherPending, 'orca.deb')

src/main/linux-package-update-recovery.ts

@@ -11,7 +11,6 @@ import { getLinuxRootPackageType } from './linux-update-package-type'
 import { buildLinuxPackageInstallCommand } from './linux-package-install-command'
 
 const SHA512_BYTE_LENGTH = 64
-// electron-updater downloads into <cacheRoot>/<updaterCacheDirName>/pending; nothing else is trusted.
 const PENDING_DIRECTORY_NAME = 'pending'
 
 export type LinuxPackageArtifact = {
@@ -43,7 +42,7 @@ type ValidationResult =
   | { ok: false; reason: 'missing' | 'not-regular' | 'hash-mismatch' | 'read-failed' }
 
 let trackedArtifact: LinuxPackageArtifact | null = null
-// Why: the renderer debounces clicks, but the IPC boundary must not allow parallel hashing of a 160 MB package.
+// Deduplicate hashing at the IPC boundary even if clients debounce clicks.
 const inFlightValidations = new WeakMap<LinuxPackageArtifact, Promise<ValidationResult>>()
 
 export function getTrackedLinuxPackageArtifact(): LinuxPackageArtifact | null {
@@ -54,12 +53,11 @@ export function clearTrackedLinuxPackageArtifact(): void {
   trackedArtifact = null
 }
 
-/** Clears the artifact only when a different version demonstrably takes over the update cycle. */
 export function clearTrackedLinuxPackageArtifactForOtherVersion(version: unknown): void {
   if (!trackedArtifact) {
     return
   }
-  // Why: an unknown/empty version is not evidence of another cycle and must not destroy recovery.
+  // Missing version metadata does not prove another update cycle has started.
   if (typeof version !== 'string' || version.length === 0) {
     return
   }
@@ -69,12 +67,7 @@ export function clearTrackedLinuxPackageArtifactForOtherVersion(version: unknown
   trackedArtifact = null
 }
 
-/**
- * Mirrors electron-updater's cache-name rule: resolve the manifest URL against a dummy base, decode
- * the pathname, require the package extension, and match the basename of the downloaded file.
- * A malformed encoding or an ambiguous file/hash pairing disables cached-package recovery rather
- * than guessing a digest.
- */
+/** Match electron-updater’s decoded cache filename; reject malformed or conflicting digests. */
 function resolveExpectedSha512(
   files: unknown,
   downloadedFile: string,
@@ -115,10 +108,7 @@ function resolveExpectedSha512(
   return resolved
 }
 
-/**
- * Retains the downloaded package and its release digest so manual actions do not repeat the 160 MB
- * transfer. Only the in-memory event metadata is trusted for the digest.
- */
+/** Trust only the release event’s digest; retain the download for manual recovery. */
 export function captureLinuxPackageArtifact(event: unknown): LinuxPackageArtifact | null {
   const packageType = getLinuxRootPackageType()
   if (!packageType) {
@@ -140,12 +130,9 @@ export function captureLinuxPackageArtifact(event: unknown): LinuxPackageArtifac
     downloadedFile,
     packageType
   )
-  // Why: a malformed digest can never validate. Arming recovery on it would send the user to the
-  // alarming "no longer matches the verified release" path for what is a release-metadata problem.
+  // Reject malformed release metadata before offering recovery that cannot validate.
   if (!sha512 || !decodeExpectedDigest(sha512)) {
-    // Why: an unresolvable digest only means THIS event cannot arm recovery. A previously retained
-    // artifact carries its own digest and is revalidated on every use, so dropping it would force a
-    // needless 160 MB redownload of a file that is still on disk and still verifiable.
+    // Keep the prior artifact: it has its own digest and is revalidated on use.
     return null
   }
   const artifact = { packageType, version, path: downloadedFile, sha512 }
@@ -158,27 +145,12 @@ function isInsideDirectory(root: string, target: string): boolean {
   return relative.length > 0 && !relative.startsWith('..') && !path.isAbsolute(relative)
 }
 
-/**
- * The cache root electron-updater downloads into. Mirrors its own `getAppCacheDir()` rule, which on
- * Linux resolves to the same directory Electron reports as the `cache` path.
- */
 function getUpdaterCacheRoot(): string {
-  // Mirrors upstream exactly, including its lack of an absoluteness check — diverging here would put
-  // the real download outside the directory this module treats as the cache.
+  // Match electron-updater’s XDG_CACHE_HOME handling, including relative paths.
   return process.env.XDG_CACHE_HOME || path.join(os.homedir(), '.cache')
 }
 
-/**
- * Rejects traversal and a symlinked parent that escapes the updater cache.
- *
- * The security boundary is the cache-root anchor below: electron-updater builds `downloadedFile` from
- * the `fileName` in the cache's own writable `update-info.json` without calling `basename`, so without
- * it a manipulated entry could point at `/tmp` or `/dev/shm` where a DIFFERENT principal can win the
- * swap race. The `<cacheRoot>/<updaterCacheDirName>/pending` shape check narrows the accepted paths to
- * the one place electron-updater actually downloads into; it is defence in depth, not a boundary — a
- * same-uid process can create its own `pending` directory, and per the design's trust model such a
- * process can already overwrite the real cached package anyway.
- */
+/** Anchor paths under the real cache root to exclude other users’ swap locations; same-user writes remain possible. */
 export async function isInUpdaterPendingCache(filePath: string): Promise<boolean> {
   const cacheRoot = path.resolve(getUpdaterCacheRoot())
   if (!isInsideDirectory(cacheRoot, path.resolve(filePath))) {
@@ -202,9 +174,7 @@ function decodeExpectedDigest(sha512: string): Buffer | null {
   if (decoded.byteLength !== SHA512_BYTE_LENGTH) {
     return null
   }
-  // Why: Buffer.from silently drops invalid base64 characters; round-tripping rejects malformed input.
-  // The round-trip emits standard base64, so a URL-safe digest would be rejected — electron-updater's
-  // latest-linux.yml is standard base64, and failing closed on an unrecognized encoding is correct.
+  // Round-trip standard base64: Buffer.from silently accepts malformed input.
   return decoded.toString('base64') === trimmed ? decoded : null
 }
 
@@ -245,7 +215,6 @@ async function validateArtifact(artifact: LinuxPackageArtifact): Promise<Validat
   }
 }
 
-/** Hashes the exact captured artifact, joining only that capture's in-flight proof. */
 function runValidation(artifact: LinuxPackageArtifact): Promise<ValidationResult> {
   const inFlight = inFlightValidations.get(artifact)
   if (inFlight) {
@@ -260,11 +229,7 @@ function runValidation(artifact: LinuxPackageArtifact): Promise<ValidationResult
   return promise
 }
 
-/**
- * Revalidates the retained package against the digest captured from the release metadata. This is
- * an integrity check against that HTTPS metadata, not a package signature and not a privilege
- * boundary — a same-user process can still replace the file after this returns.
- */
+/** Release-metadata integrity only; a same-user process can still replace the file after validation. */
 async function validateTrackedArtifact(
   recovery: LinuxPackageInstallRecovery
 ): Promise<ValidationResult> {

src/main/linux-update-package-type.test.ts

@@ -221,7 +221,6 @@ describe('getLinuxRootPackageType', () => {
   })
 
   it('returns unusable when the marker is unreadable', async () => {
-    // A directory in the marker's place makes fail with EISDIR.
     await fsp.mkdir(path.join(resourcesDir, 'package-type'))
     const module = await loadPackageType()
     expect(module.getLinuxPackageType()).toBe('unusable')
@@ -287,7 +286,6 @@ describe('getLinuxRootPackageType', () => {
 
   it('reads the marker once and warns once per process', async () => {
     const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
-    // A directory in place of the marker is readable-but-unusable, which is the case worth reporting.
     await fsp.mkdir(path.join(resourcesDir, 'package-type'))
     const module = await loadPackageType()
     module.getLinuxPackageType()
@@ -315,7 +313,6 @@ describe('getLinuxRootPackageType', () => {
 })
 
 describe('isExternallyManagedLinuxInstall', () => {
-  // The #17702 case: an AUR/Nix/container rebuild of the .deb inherits `package-type` verbatim.
   it('reports a deb marker with no deb package manager as externally managed', async () => {
     hasTrustedPackageManagerForMock.mockReturnValue(false)
     await writeMarker('deb')

src/main/linux-update-package-type.ts

@@ -6,14 +6,11 @@ import type { LinuxRootPackageType } from '../shared/update-status-types'
 
 export type { LinuxRootPackageType }
 
-/** The packaged Linux format that controls how updates may be installed. */
 export type LinuxPackageType = LinuxRootPackageType | 'non-root' | 'unusable'
 
-// Why: `undefined` means "not resolved yet"; every other value is stable for this process.
 let cachedPackageType: LinuxPackageType | undefined
 let cachedExternallyManaged: boolean | undefined
 
-// Bounded by construction: the marker is read at most once per process.
 function warnMarkerUnusable(detail: string): void {
   console.warn(`[updater] linux package-type marker unusable: ${detail}`)
 }
@@ -98,11 +95,7 @@ function readPackageTypeMarker(): LinuxPackageType {
   return 'unusable'
 }
 
-/**
- * Resolves the installed Linux package format. Packaged builds fail closed when their marker is
- * missing or unusable unless the legacy APPIMAGE runtime identity is valid. Unpackaged, non-Linux,
- * and identified AppImage runs are non-root.
- */
+/** Packaged Linux builds require a valid marker, except legacy AppImages with a missing marker. */
 export function getLinuxPackageType(): LinuxPackageType {
   if (cachedPackageType === undefined) {
     cachedPackageType = readPackageTypeMarker()
@@ -110,23 +103,12 @@ export function getLinuxPackageType(): LinuxPackageType {
   return cachedPackageType
 }
 
-/** Returns a root package type when this build supports manual package recovery. */
 export function getLinuxRootPackageType(): LinuxRootPackageType | null {
   const packageType = getLinuxPackageType()
   return packageType === 'deb' || packageType === 'rpm' ? packageType : null
 }
 
-/**
- * Whether the marker claims a root package format this host cannot install. Repackagers (AUR, Nix,
- * container rebuilds) unpack Orca's .deb and inherit its `package-type` verbatim, so the marker
- * describes the artifact Orca was built as, never the system that now owns the install. Without a
- * matching package manager no downloaded package can ever be applied here.
- *
- * A false positive is impossible by construction: this reuses the exact manager lists and resolver
- * that `buildLinuxPackageInstallCommand` already loops over, so any host flagged here would have
- * failed with `no-package-manager` after the download anyway. The gate only moves that verdict
- * earlier — it never refuses a host that could have installed the update.
- */
+/** Repackagers can inherit deb/rpm markers; require the same manager discovery used for installation. */
 export function isExternallyManagedLinuxInstall(): boolean {
   if (cachedExternallyManaged === undefined) {
     const packageType = getLinuxRootPackageType()

src/main/local-project-runtime-resolution.ts

@@ -29,15 +29,7 @@ import {
 import { resolveManagedSshHostLoginShell } from './pty/managed-ssh-host-login-shell'
 import { getRepoExecutionHostId, LOCAL_EXECUTION_HOST_ID } from '../shared/execution-host'
 
-/**
- * The slice of the store runtime resolution actually reads. Structural rather
- * than the full `Store` so narrowed stores -- the Orca runtime's `RuntimeStore`,
- * worktree root preparation -- resolve the same runtime the create path does
- * instead of silently falling back to host placement.
- *
- * Members stay optional because those stores declare them optional; the
- * `typeof` guards below are what actually decide whether resolution can run.
- */
+// Structural typing lets narrowed runtime stores use the same resolution as workspace creation.
 export type ProjectRuntimeResolutionStore = {
   getProjects?: Store['getProjects']
   getRepo?: Store['getRepo']
@@ -99,8 +91,7 @@ export function resolveLocalProjectRuntimeForRepo(
   return resolveLocalProjectRuntime(store, project)
 }
 
-/** Same choice the renderer makes for a workspace: the project's runtime, else the distro of a
- *  WSL-share path, else the global default. Works for a repo with no project too. */
+/** Match renderer precedence: project preference, WSL-share distro, then global default. */
 export function resolveLocalWorkspaceRuntime(
   store: ProjectRuntimeResolutionStore,
   repo: Repo,
@@ -117,8 +108,7 @@ export function resolveLocalWorkspaceRuntime(
     project?.localWindowsRuntimePreference,
     workspacePath
   )
-  // A `\\wsl$` path proves its distro exists, and main's distro cache can be stale; the window
-  // checks such a path against no cache either.
+  // WSL-share paths supply their distro; ignore potentially stale discovery caches for inferred preferences.
   return resolveLocalProjectRuntime(
     store,
     { id: project?.id ?? repo.id, localWindowsRuntimePreference: preference },
@@ -127,8 +117,7 @@ export function resolveLocalWorkspaceRuntime(
   )
 }
 
-/** The one local repo whose project runs a folder workspace on this machine; none for a folder
- *  pinned to another host or spanning several repos. */
+/** A folder inherits a runtime only from a single repo on the local host. */
 export function findLocalFolderWorkspaceRuntimeRepo(
   state: FolderWorkspaceHostState,
   folderWorkspaceId: string
@@ -155,8 +144,7 @@ export type LaunchScopeRuntimeStore = ProjectRuntimeResolutionStore & {
   getProjectGroups?: () => ProjectGroup[]
 }
 
-/** A launch scope's runtime, as the window reads it for a workspace. A remote scope has none: its
- *  host plans for itself. */
+/** Remote scopes resolve their runtime on their own host. */
 export function resolveLocalLaunchScopeRuntime(
   store: LaunchScopeRuntimeStore,
   scope: {
@@ -210,8 +198,7 @@ export function resolveLocalProjectRuntimesForRepos(
     if (matchingRepoIds.length === 0) {
       continue
     }
-    // Why: one project runtime applies to every source repo in that project;
-    // resolving it once prevents mobile polls from rescanning project settings.
+    // Resolve once per project to avoid repeated settings scans during mobile polls.
     const runtime = resolveLocalProjectRuntime(store, project, settings)
     for (const repoId of matchingRepoIds) {
       runtimeByRepoId.set(repoId, runtime)
@@ -238,7 +225,7 @@ type LocalPtySettings = Partial<
   Pick<GlobalSettings, 'terminalWindowsShell' | 'terminalWindowsWslDistro' | 'terminalDefaultShell'>
 >
 
-/** A local PTY's shell and distro. Throws for a runtime that needs repair. */
+/** Throws for a Windows runtime that needs repair. */
 export function resolveLocalPtyOptions(
   settings: LocalPtySettings | undefined,
   projectRuntime: ProjectExecutionRuntimeResolution | undefined,
@@ -260,8 +247,7 @@ export function resolveLocalPtyOptions(
       }
 }
 
-/** A plain terminal's PTY: the project-only runtime and `COMSPEC`, as the spawn picks when no
- *  agent plan brought its own. */
+/** Plain terminals use the project runtime and fall back to COMSPEC for the Windows host shell. */
 export function resolvePlainTerminalPtyOptions(
   store: ProjectRuntimeResolutionStore | undefined,
   settings: LocalPtySettings | undefined,

src/main/local-worktree-filesystem-wsl-banner.wsl.test.ts

@@ -23,7 +23,6 @@ async function wsl(command: string, ...args: string[]): Promise<string> {
   return stdout.trim()
 }
 
-/** What an unfenced login-shell read returns — the shape this suite exists to rule out. */
 async function readThroughRawLoginShell(linuxPath: string): Promise<string> {
   const { stdout } = await execFileAsync(
     'wsl.exe',
@@ -57,9 +56,7 @@ describe.skipIf(!runRealWsl)('WSL worktree reads carry no shell chatter', () =>
     const { readPath } = getLocalWorktreePathAccess({ wslDistro: DISTRO })
     const raw = await readThroughRawLoginShell(`${fixtureRoot}/file.txt`)
 
-    // Contrast: routed through a login shell the read carries whatever the rc
-    // files printed (stock Ubuntu ships a sudo hint). These reads use a plain
-    // `sh -c`, which runs no rc at all, so they are exactly the file.
+    // A login shell may print rc banners; plain sh must return only the file’s bytes.
     expect(raw.endsWith(FILE_CONTENTS)).toBe(true)
     expect(await readPath(unc(`${fixtureRoot}/file.txt`))).toBe(FILE_CONTENTS)
   }, 60_000)

src/main/local-worktree-filesystem.test.ts

@@ -7,8 +7,7 @@ const { runProcessMock, lstatMock, readFileMock, rmMock } = vi.hoisted(() => ({
   rmMock: vi.fn()
 }))
 
-// Why mock the chokepoint: encoding, timeout and the hidden console are its
-// contract now, so this suite asserts what the distro is asked to run.
+// Process-host tests own encoding, timeouts, and console hiding; assert guest commands here.
 vi.mock('@orca/process-host', () => ({
   runProcess: runProcessMock
 }))
@@ -190,9 +189,7 @@ describe('local worktree filesystem runtime access', () => {
         expect.objectContaining({
           program: 'wsl.exe',
           args: expect.arrayContaining(['-d', 'Ubuntu']),
-          // Why a concrete directory (#16463): the guest path is inside the
-          // command, and these run while a worktree is being removed -- which is
-          // the cwd an omitted one would inherit.
+          // The inherited cwd may be the worktree being removed (#16463).
           cwd: expect.any(String)
         })
       )
@@ -208,9 +205,7 @@ describe('local worktree filesystem runtime access', () => {
       completeExecFile('file')
       await getLocalWorktreePathAccess({ wslDistro: 'Ubuntu' }).statPath('/home/me/repo/.git')
 
-      // Why: a login/interactive shell is what puts the distro's rc banner on the
-      // stdout these callers parse. cat/stat/rm need nothing from the user's PATH,
-      // so the shell mode is the fix rather than filtering what it prints.
+      // Avoid login-shell banners instead of filtering potentially valid file content.
       const args = runProcessMock.mock.calls[0]?.[0].args as string[]
       expect(args).toContain('-c')
       expect(args).not.toContain('-lc')
@@ -250,7 +245,6 @@ describe('local worktree filesystem runtime access', () => {
         const access = getLocalWorktreePathAccess({ wslDistro: 'Ubuntu' })
 
         await expect(access.statPath('/mnt/c/repo/locked')).rejects.toThrow('Permission denied')
-        // A delete that did not happen still has to look like a failure, or cleanup silently "succeeds".
         await expect(isWorktreePathMissing('/mnt/c/repo/locked', access.statPath)).resolves.toBe(
           false
         )
@@ -270,8 +264,7 @@ describe('local worktree filesystem runtime access', () => {
 
   it('reports a translated stat failure as a failure rather than as absence', async () => {
     await withPlatform('win32', async () => {
-      // Synthetic: no host available here ships a non-English locale, and the probe pins LC_ALL=C
-      // anyway. The point is the direction it errs if that pin ever comes off.
+      // Synthetic localization verifies that unknown errors cannot prove absence if LC_ALL=C regresses.
       failExecFile(1, "stat: <translated verb> '/mnt/c/repo/missing': <translated errno>\n")
       const access = getLocalWorktreePathAccess({ wslDistro: 'Ubuntu' })
 

src/main/local-worktree-filesystem.ts

@@ -24,21 +24,12 @@ function shouldUseWslFilesystem(options: LocalWorktreeFilesystemOptions): boolea
   return process.platform === 'win32' && !!options.wslDistro?.trim()
 }
 
-/**
- * Run a filesystem command inside the distro.
- *
- * Why no login shell: these are coreutils at standard paths plus shell builtins,
- * and need nothing from the user's PATH. A login shell would only add its rc/motd
- * output to the stdout these callers parse -- the banner problem -- so the fix is
- * to not start one rather than to fence what it prints.
- */
+// Coreutils need no login shell; rc/motd output would corrupt parsed stdout.
 async function runWslCommand(distro: string, command: string): Promise<string> {
   const result = await runProcess({
     program: 'wsl.exe',
     args: buildWslExecArgs(distro, ['sh', '-c', command]),
-    // Why explicit (#16463): the guest path is inside `command`, so this only
-    // decides whether CreateProcessW succeeds -- and these calls run while a
-    // worktree is being removed, which is the cwd an inherited one would be.
+    // An inherited cwd may be the worktree currently being deleted (#16463).
     cwd: resolveWslInteropSpawnCwd(),
     timeoutMs: WSL_FILE_OPERATION_TIMEOUT_MS
   })
@@ -54,12 +45,7 @@ async function runWslCommand(distro: string, command: string): Promise<string> {
   return result.stdout
 }
 
-/**
- * Only stat's trailing strerror text is portable: GNU coreutils says `cannot statx`,
- * BusyBox says `can't stat`, so matching the verb made a BusyBox distro report a
- * successful cleanup as a permanent failure. The tail stays English because the probe
- * pins LC_ALL=C, and stat is the only thing in that probe that writes to stderr.
- */
+// LC_ALL=C fixes strerror text; match only its tail because GNU and BusyBox use different verbs.
 const WSL_MISSING_PATH_STDERR = /: (?:No such file or directory|Not a directory)\r?\n?$/
 
 function isWslMissingPathError(error: unknown): boolean {
@@ -130,7 +116,6 @@ export async function removeLocalWorktreePath(
     return
   }
 
-  // Why: WSL-owned worktree directories may be POSIX paths that Node on
-  // Windows cannot delete safely. Run the deletion inside the selected distro.
+  // Delete guest paths inside their distro; Windows Node cannot safely remove them.
   await runWslCommand(distro, `rm -rf -- ${quotePosixShell(toLinuxPath(targetPath))}`)
 }

src/main/local-worktree-metadata-prune-gate.test.ts

@@ -80,7 +80,6 @@ describe('local worktree metadata prune gate', () => {
     forgetLocalWorktreeMetadataPruneGate('repo-1')
 
     expect(isLocalWorktreeMetadataPruneDue('repo-1')).toBe(true)
-    // The forgotten fingerprint must not later read as a change against a stale entry.
     recordLocalWorktreeListingForPruneGate('repo-1', ['/a'])
     markLocalWorktreeMetadataPruneStarted('repo-1')
     expect(isLocalWorktreeMetadataPruneDue('repo-1')).toBe(false)

src/main/local-worktree-metadata-prune-gate.ts

@@ -1,66 +1,30 @@
-/**
- * Decides whether a detected-worktree scan owes the store a metadata-hygiene pass.
- *
- * The pass captures a prune expectation over the repo's whole metadata table, `stat`s every
- * path-missing candidate, then prunes metadata and lineage. That is O(all rows) and destructive, so
- * it must not ride a polled read path: on a store whose dangling rows outnumber live worktrees
- * ~100:1 it re-derives an answer it already has, forever, pinning the main process in filesystem
- * completion callbacks (#17775). Rows pinned by a persisted session are never removable, so the
- * repetition cannot even make progress.
- *
- * Nothing the pass reads changes on its own, so it is gated on evidence rather than a clock:
- *  - a worktree lifecycle event for the repo (the existing worktree-change invalidator hub),
- *  - a mutation that can make some row *more* removable (see `invalidate…PruneInputs`),
- *  - the repo's git listing differing from the one the last pass ran against.
- * With none of those, the pass is a provable repeat and is skipped outright.
- *
- * Why only "more removable" mutations count: the listing path itself writes worktree metadata
- * (discovery backfill, host-ownership stamping), so bumping on every metadata write would re-arm
- * the gate from the very scan it gates and restore the storm. Additions and updates can only
- * preserve more rows, so staleness there is harmless.
- *
- * The failure direction is deliberate. A signal we miss leaves a dangling row in place until the
- * next one arrives — hygiene lags, and nothing is deleted that would not have been deleted anyway.
- */
+// Pruning scans all metadata: rerun on removal evidence, never on every listing poll (#17775).
+// Ignore additions/updates: discovery writes metadata itself, and retaining extra rows is safe.
 
-/** Bumped by evidence that some row may have become removable; repos re-run the pass once each. */
 let pruneInputsGeneration = 0
 const observedGenerationByRepo = new Map<string, number>()
 const listingFingerprintByRepo = new Map<string, string>()
 
-/** True until the repo has run a pass against the current generation — so also on the first scan. */
+/** Unseen repos are due on their first scan. */
 export function isLocalWorktreeMetadataPruneDue(repoId: string): boolean {
   return observedGenerationByRepo.get(repoId) !== pruneInputsGeneration
 }
 
-/**
- * Claim the pending pass. Recorded when the expectation is captured rather than when the prune
- * finishes: a scan that is invalidated or fails mid-flight must not re-arm the full capture on the
- * very next listing poll, and any real change re-bumps the generation anyway.
- */
+/** Claim before pruning so failures do not retry on every poll; new evidence rearms the pass. */
 export function markLocalWorktreeMetadataPruneStarted(repoId: string): void {
   observedGenerationByRepo.set(repoId, pruneInputsGeneration)
 }
 
-/** One repo's worktrees changed (create/remove/rename). */
 export function requireLocalWorktreeMetadataPrune(repoId: string): void {
   observedGenerationByRepo.delete(repoId)
 }
 
-/**
- * Some metadata row may have become removable — a worktree-meta/identity/lineage row was deleted,
- * or a session, lease, automation or selection released its claim on a workspace. Repo-agnostic
- * because ownership is global state: a session release can unpin a row in any repo.
- */
+/** Deletions and released claims can unpin rows in any repo; invalidate globally. */
 export function invalidateLocalWorktreeMetadataPruneInputs(): void {
   pruneInputsGeneration += 1
 }
 
-/**
- * Backstop for changes no event reported: if Git now lists a different set of worktrees than the
- * last pass ran against, that pass's conclusions no longer describe this repo. Costs one join over
- * a list we already hold.
- */
+/** Changed Git listings catch worktree mutations that emitted no event. */
 export function recordLocalWorktreeListingForPruneGate(
   repoId: string,
   worktreePaths: readonly string[]
@@ -71,14 +35,12 @@ export function recordLocalWorktreeListingForPruneGate(
     return
   }
   listingFingerprintByRepo.set(repoId, fingerprint)
-  // Why not on the first listing: a repo we have never scanned is already due by default, and the
-  // scan that produced this listing is the pass that covers it. Re-arming here would double it.
+  // First listings are already due; rearming would duplicate their pass.
   if (previous !== undefined) {
     requireLocalWorktreeMetadataPrune(repoId)
   }
 }
 
-/** A deregistered repo leaves no reason to retain its gate state. */
 export function forgetLocalWorktreeMetadataPruneGate(repoId: string): void {
   observedGenerationByRepo.delete(repoId)
   listingFingerprintByRepo.delete(repoId)
@@ -90,8 +52,7 @@ export function __resetLocalWorktreeMetadataPruneGateForTests(): void {
   listingFingerprintByRepo.clear()
 }
 
-/** Repo teardown: a full removal retires this repo's gate, and either shape can unpin rows in
- *  other repos, so the shared inputs are always re-armed. */
+/** Removing either local or remote ownership can unpin metadata in other repos. */
 export function retireLocalWorktreeMetadataPruneStateForRepo(
   repoId: string,
   hostId: string | null

src/main/local-worktree-path-presence.ts

@@ -3,7 +3,7 @@ import { mapWithConcurrency } from '../shared/map-with-concurrency'
 import { PrioritySemaphore } from '../shared/priority-semaphore'
 import { abortSignalWithTimeout } from './lib/abort-signal-with-timeout'
 
-// Why: share one bound across polling callers so slow mounts cannot multiply libuv work.
+// Share a concurrency bound across polling callers so slow mounts cannot multiply libuv work.
 const LOCAL_WORKTREE_PATH_PROBE_CONCURRENCY = 2
 export const LOCAL_WORKTREE_PATH_PROBE_TIMEOUT_MS = 2_000
 const localWorktreePathProbeSemaphore = new PrioritySemaphore(LOCAL_WORKTREE_PATH_PROBE_CONCURRENCY)

src/main/local-worktree-removal-recovery-live.test.ts

@@ -70,8 +70,7 @@ describe('local Windows worktree removal recovery (live Git)', () => {
       git(repoPath, ['worktree', 'add', '--quiet', '-b', 'feature/churn', worktreePath])
       const head = git(worktreePath, ['rev-parse', 'HEAD'])
 
-      // Why: a producer racing Git's recursive delete deterministically exposes
-      // the Windows ENOTEMPTY state reported by real workspace deletions.
+      // Race Git’s recursive deletion to reproduce Windows ENOTEMPTY deterministically.
       const churn = spawn(
         process.execPath,
         [

src/main/local-worktree-removal-recovery.ts

@@ -90,8 +90,7 @@ async function removeRequiredGitWorktreeRegistration(
     removalError = error
   }
 
-  // Why: Git prune exits successfully while retaining locked registrations;
-  // a failed remove can also have detached the row before filesystem cleanup.
+  // Successful prune can retain locked registrations; verify the row is gone.
   try {
     await verifyGitWorktreeRegistrationRemoved(
       args.repoPath,
@@ -104,26 +103,21 @@ async function removeRequiredGitWorktreeRegistration(
       ? staleRegistrationRecoveryError(removalError, args.canonicalWorktreePath, forceForError)
       : verificationError
   }
-  // Why: if Git detached the row before reporting its filesystem error, keep
-  // the branch rather than guessing whether the normal branch cleanup ran.
+  // Preserve the branch when Git may have detached the row before reporting a filesystem error.
   return result ?? preservedBranchResult(args.registeredWorktree, args.deleteBranch)
 }
 
 export async function recoverLocalWindowsWorktreeRemoval(
   args: LocalWindowsRemovalRecoveryArgs
 ): Promise<RemoveWorktreeResult | undefined> {
-  // Why: recovery can recursively delete the remaining directory, so a Git
-  // lock must reject the attempt before classification or any side effects.
+  // Reject Git locks before recovery can recursively delete files.
   assertWorktreeUnlockedForRemoval(args.registeredWorktree)
 
   if (!(await isRecoverableWindowsFilesystemRemovalFailure(args))) {
     return undefined
   }
 
-  // Why: this error means Git accepted removal and started deleting the path;
-  // finish that partial success with Windows retries, then verify Git metadata.
-  // Why: Windows recovery recursively deletes the remaining directory, so it
-  // must fail closed while a watcher process may still own a native handle.
+  // Release watcher handles before recursively finishing Git’s partial removal; abort if closing fails.
   await args.closeWatcher(args.canonicalWorktreePath)
   try {
     await removeLocalWorktreePath(args.canonicalWorktreePath, args.localWorktreeGitOptions)
@@ -142,8 +136,7 @@ async function isRecoverableWindowsFilesystemRemovalFailure(
 
   try {
     const worktrees = await listWorktreesStrict(args.repoPath, args.localWorktreeGitOptions)
-    // Why: error prose can be localized or ambiguous. Only a missing Git row
-    // proves removal started and makes recursive Windows cleanup safe.
+    // Localized error text cannot prove removal started; require the Git row to be absent.
     return !worktrees.some((worktree) =>
       areWorktreePathsEqual(worktree.path, args.canonicalWorktreePath)
     )

src/main/local-worktree-scan-generation.ts

@@ -4,16 +4,13 @@ import type { WorktreeCatalogVersion } from '../shared/worktree/catalog-version'
 const generationByRepoId = new Map<string, number>()
 let generationSequence = 0
 let mutationRevision = 0
-// Why per process: generations restart with the process, so a client ordering catalog
-// publications across a host restart must see the restart as a new catalog, not an older one.
+// A new epoch lets clients distinguish a host restart from older catalog publications.
 const LOCAL_WORKTREE_CATALOG_EPOCH = randomUUID()
 
-/** The catalog version a scan that began at `generation` describes. */
 export function localWorktreeCatalogVersionAt(generation: number): WorktreeCatalogVersion {
   return { epoch: LOCAL_WORKTREE_CATALOG_EPOCH, sequence: generation }
 }
 
-/** The repo's current catalog version: what a mutation reply issued now describes. */
 export function getLocalWorktreeCatalogVersion(repoId: string): WorktreeCatalogVersion {
   return localWorktreeCatalogVersionAt(getLocalWorktreeScanGeneration(repoId))
 }
@@ -42,17 +39,7 @@ export function retireLocalWorktreeScanGeneration(repoId: string): void {
   forgetLocalWorktreeScanGeneration(repoId)
 }
 
-/**
- * Advances on every event above that can change what a worktree scan would find — repo add,
- * removal, update, and scan-cache invalidation — and on nothing else. A cache that must not answer
- * for repos it never saw compares this in O(1) instead of walking the repo list.
- *
- * Why not `generationSequence`: that also advances when `getLocalWorktreeScanGeneration` mints a key
- * for a repo id nothing has scanned yet, which is a read. Keying a snapshot on it would let a read
- * path discard a snapshot that is still perfectly valid.
- *
- * Ordering-only: the value means nothing outside a same-process comparison.
- */
+/** Same-process mutation counter; unlike generationSequence, reads cannot advance it. */
 export function getWorktreeScanMutationRevision(): number {
   return mutationRevision
 }

src/main/localhost-worktree-label-proxy.test.ts

@@ -27,8 +27,7 @@ function fetchThroughProxy(
 ): Promise<{ status: number; body: string; headers: http.IncomingHttpHeaders }> {
   const url = new URL(labeledUrl)
   return new Promise((resolve, reject) => {
-    // Why: *.orca.localhost is not resolvable DNS; connect to the proxy on
-    // loopback and carry the label through the Host header instead.
+    // Bypass DNS with loopback; route the worktree label through Host.
     const request = http.request(
       {
         host: '127.0.0.1',
@@ -84,8 +83,6 @@ describe('localhost worktree label proxy', () => {
     const result = await fetchThroughProxy(url)
 
     expect(result.status).toBe(200)
-    // The proxy only relabels the hostname; the page is delivered verbatim
-    // with no injected favicon/title and the CSP header intact.
     expect(result.body).toBe('<html><head></head><body>hello</body></html>')
     expect(result.headers['content-security-policy']).toBe("default-src 'self'")
   })
@@ -149,7 +146,6 @@ describe('localhost worktree label proxy', () => {
 
     proxy.unregisterWorktree('wt-a')
 
-    // The removed worktree's label no longer routes; the other worktree is unaffected.
     expect((await fetchThroughProxy(a.url)).status).toBe(404)
     expect((await fetchThroughProxy(b.url)).status).toBe(200)
   })

src/main/localhost-worktree-label-proxy.ts

@@ -49,9 +49,6 @@ export class LocalhostWorktreeLabelProxy {
     }
   }
 
-  // Why: routes are added per (worktreeId, targetUrl) but were never removed, so
-  // labels for deleted worktrees accumulated in both maps for the whole session.
-  // Drop them when the worktree is torn down. The shared http.Server stays up.
   unregisterWorktree(worktreeId: string): void {
     for (const [label, route] of this.routes) {
       if (route.worktreeId === worktreeId) {
@@ -135,22 +132,17 @@ export class LocalhostWorktreeLabelProxy {
       headers: requestHeadersForTarget(request, route.target)
     })
 
-    // Why: a client abort or downstream socket error must tear down the
-    // upstream request instead of surfacing as an uncaught exception/leak.
+    // Downstream errors must release upstream work rather than leak requests or crash main.
     request.on('error', () => proxyRequest.destroy())
     response.on('error', () => proxyRequest.destroy())
 
-    // Why: the proxy only relabels the hostname; responses are streamed
-    // through untouched so app headers (CSP, cookies) and bodies are
-    // preserved exactly as the dev server sent them.
     proxyRequest.on('response', (proxyResponse) => {
       proxyResponse.on('error', () => response.destroy())
       response.writeHead(proxyResponse.statusCode ?? 502, proxyResponse.headers)
       proxyResponse.pipe(response)
     })
     proxyRequest.on('error', (error) => {
-      // Why: once headers/bytes are flushed we can't write a 502, so tear the
-      // socket down to avoid an ERR_HTTP_HEADERS_SENT crash.
+      // After headers are sent, destroy the response instead of attempting a 502.
       if (response.headersSent) {
         response.destroy(error)
         return

src/main/macos-press-and-hold-default.defaults-domain.test.ts

@@ -15,22 +15,10 @@ import {
   type PressAndHoldRecord
 } from './macos-press-and-hold-default'
 
-/**
- * Runs against the real `/usr/bin/defaults` on a throwaway Orca-owned domain.
- *
- * The whole design rests on one claim the mocks cannot make: a domain that has never been written
- * is distinguishable from one explicitly set to `false`. Electron's `systemPreferences` cannot tell
- * them apart, so if `defaults` could not either, "only write when unset" would be unimplementable.
- *
- * Every domain used here is a throwaway UUID under Orca's own prefix, deleted along with its plist
- * in `afterEach`; the real `com.stablyai.orca` domain is never read or written.
- */
+// Verify real defaults distinguishes absence from false using disposable Orca-prefixed domains.
 
-// Why a real Orca-owned domain shape: the ownership guard rejects anything else, so a fake prefix
-// would exercise a different branch than production.
 const domains: string[] = []
 
-/** Pinned against the real binary below, then reused by the platform-agnostic unit tests. */
 const DEFAULTS_MISSING_EXIT_CODE = 1
 
 function throwawayDomain(): string {
@@ -71,11 +59,8 @@ describe.skipIf(process.platform !== 'darwin')(
       for (const domain of domains.splice(0)) {
         try {
           execFileSync('/usr/bin/defaults', ['delete', domain], { stdio: 'ignore' })
-        } catch {
-          // The domain may never have been created; nothing to clean up.
-        }
-        // Why the file too: `defaults delete` empties the domain but leaves its plist behind, so
-        // without this every run litters the developer's ~/Library/Preferences.
+        } catch {}
+        // defaults delete empties the domain but leaves its plist behind.
         rmSync(join(homedir(), 'Library', 'Preferences', `${domain}.plist`), { force: true })
       }
     })
@@ -116,8 +101,6 @@ describe.skipIf(process.platform !== 'darwin')(
     })
 
     it('refuses to call a failed probe "unset"', () => {
-      // Real runs, not hand-built results: a binary that is not there must come back 'unknown'
-      // through the same path production takes, not be mistaken for "the key does not exist".
       const spawnFailure = (): ReturnType<typeof runProcessSync> =>
         runProcessSync({
           program: '/usr/bin/defaults-does-not-exist',
@@ -132,16 +115,13 @@ describe.skipIf(process.platform !== 'darwin')(
         args: ['read', throwawayDomain(), PRESS_AND_HOLD_KEY],
         stdio: ['ignore', 'ignore', 'ignore']
       })
-      // The one exit code the whole design reads as unset; every other outcome is 'unknown'.
       expect(missingKey.code).toBe(DEFAULTS_MISSING_EXIT_CODE)
       expect(missingKey.timedOut).toBe(false)
       expect(interpretDefaultsRead(() => missingKey)).toBe('unset')
     })
 
-    // Skipped as root, where the read-only directory below would still be writable.
+    // Root could still write the read-only fixture directory.
     it.skipIf(process.getuid?.() === 0)('reports a write the binary refused', () => {
-      // Why a real refusal: `defaults write` exits non-zero instead of throwing, so a caller that
-      // only caught exceptions would record 'applied' for a value that never reached the plist.
       const readOnly = join(mkdtempSync(join(tmpdir(), 'orca-press-hold-ro-')), 'locked')
       mkdirSync(readOnly)
       chmodSync(readOnly, 0o500)
@@ -159,7 +139,6 @@ describe.skipIf(process.platform !== 'darwin')(
       execFileSync('/usr/bin/defaults', ['delete', domain, PRESS_AND_HOLD_KEY], { stdio: 'ignore' })
 
       expect(readDomainPressAndHoldPreference(domain)).toBe('unset')
-      // A launch that already decided must not rewrite it, which is what makes the delete stick.
       const decided: PressAndHoldRecord = {
         version: 1,
         decision: 'applied',

src/main/macos-press-and-hold-default.test.ts

@@ -18,7 +18,6 @@ const ORCA_DOMAIN = 'com.stablyai.orca'
 
 type HostOverrides = Partial<PressAndHoldHost> & { record?: PressAndHoldRecord | null }
 
-/** A macOS host whose domain has no explicit value — the state every affected user is in. */
 function createHost(overrides: HostOverrides = {}): {
   host: PressAndHoldHost
   writes: { domain: string; value: boolean }[]
@@ -71,8 +70,6 @@ describe('ensureMacPressAndHoldDefault', () => {
         })
 
         expect(ensureMacPressAndHoldDefault(host)).toBe('not-macos')
-        // Why assert the reads too: "inert" means no `defaults` subprocess and no userData I/O,
-        // not merely no write.
         expect(probe).not.toHaveBeenCalled()
         expect(readRecord).not.toHaveBeenCalled()
         expect(writes).toEqual([])
@@ -83,9 +80,7 @@ describe('ensureMacPressAndHoldDefault', () => {
 
   describe('respects an explicit user setting', () => {
     it('leaves the domain alone when the user already set a value', () => {
-      // One arm covers both `true` and `false`: the probe deliberately reports only presence, so
-      // whichever way the user's value points it is theirs. That the probe really does report
-      // presence for both is pinned against the real binary in the .defaults-domain test.
+      // The real-binary suite verifies that both true and false report set.
       const { host, writes, records } = createHost({ readDomainPreference: () => 'set' })
 
       expect(ensureMacPressAndHoldDefault(host)).toBe('kept-user-preference')
@@ -108,8 +103,7 @@ describe('ensureMacPressAndHoldDefault', () => {
     })
 
     it('stays out of the way after the user deletes the key we wrote', () => {
-      // The documented way back to the accent picker is `defaults delete`, which returns the
-      // domain to "unset". Without the recorded decision the next launch would rewrite it.
+      // Deleting the preference restores the accent picker; the recorded decision must prevent rewriting it.
       const { host, writes } = createHost({
         record: terminalRecord('applied'),
         readDomainPreference: () => 'unset'
@@ -145,7 +139,6 @@ describe('ensureMacPressAndHoldDefault', () => {
       expect(isOrcaPreferencesDomain('com.stablyai.orca')).toBe(true)
       expect(isOrcaPreferencesDomain('com.stablyai.orca.dev')).toBe(true)
       expect(isOrcaPreferencesDomain('com.github.Electron')).toBe(false)
-      // Why: a prefix test without the dot would accept a lookalike bundle id.
       expect(isOrcaPreferencesDomain('com.stablyai.orcafake')).toBe(false)
     })
   })
@@ -182,11 +175,6 @@ describe('ensureMacPressAndHoldDefault', () => {
   })
 })
 
-/**
- * Why here and not only in the .defaults-domain file: that file is macOS-only and CI has no macOS
- * runner, so this three-way rule would otherwise be enforced nowhere in CI. These cases need no
- * `defaults` binary — the sibling file pins that the real binary produces them.
- */
 describe('interpretDefaultsRead', () => {
   function probe(overrides: Partial<ProcessResult>): () => ProcessResult {
     return () => ({ code: 0, signal: null, stdout: '', stderr: '', timedOut: false, ...overrides })
@@ -201,7 +189,6 @@ describe('interpretDefaultsRead', () => {
   })
 
   describe('refuses to call a failed probe unset', () => {
-    // Each of these would otherwise overwrite a value the user deliberately set.
     const failures: [string, () => ProcessResult][] = [
       ['killed before it exited', probe({ code: null, signal: 'SIGTERM' })],
       ['timed out', probe({ code: null, timedOut: true })],
@@ -285,8 +272,7 @@ describe('startup wiring', () => {
     expect(callIndex).toBeGreaterThanOrEqual(0)
     expect(readyIndex).toBeGreaterThanOrEqual(0)
     expect(preflightCall).toBeGreaterThanOrEqual(0)
-    // Why after initDataPath: the record lives beside orca-data.json, and the canonical userData
-    // path is only captured there.
+    // initDataPath captures the canonical userData path where the decision record belongs.
     expect(callIndex).toBeGreaterThan(initDataPathIndex)
     expect(preflightCall).toBeLessThan(readyIndex)
   })

src/main/macos-press-and-hold-default.ts

@@ -4,64 +4,32 @@ import { runProcessSync } from '@orca/process-host'
 import type { ProcessResult } from '@orca/process-host/process-spec'
 import { writeFileAtomically } from './codex-accounts/fs-utils'
 
-/**
- * Turns off the macOS accent picker for Orca's own preferences domain (#14746).
- *
- * macOS routes press-and-hold to the accent popup unless an app opts out via
- * `ApplePressAndHoldEnabled`, so holding `j` in vim inserts one character instead of repeating.
- * The key is unset by default, which is why every terminal-hosting Mac app ships this opt-out.
- *
- * Written once and never again: a user who wants the accent picker back sets
- * `defaults write com.stablyai.orca ApplePressAndHoldEnabled -bool true` (or deletes the key), and
- * the recorded decision below keeps a later launch from overwriting that choice.
- *
- * A fresh write is assumed to land for the *next* launch, not the current one: it goes out through
- * a separate `defaults` process, so this app's own cached copy may not observe it. Whether AppKit
- * re-reads sooner is untested, so the conservative assumption is the one encoded here.
- *
- * If this is ever reverted, delete the key as well — AppKit reads the plist, not this file, so
- * removing the code alone leaves press-and-hold disabled forever for everyone who ran an affected
- * build, with nothing left in the tree to explain it.
- *
- * The `defaults`-binary behaviour this relies on is pinned by the sibling test file, which runs
- * only on a developer Mac: CI has no macOS test runner, so those tests never execute there.
- */
+// Opt out of the accent picker once so terminal keys repeat (#14746); later user choices win.
+// The separate defaults process may affect only the next launch. Reverting this also requires deleting the key.
 
 export const PRESS_AND_HOLD_KEY = 'ApplePressAndHoldEnabled'
 export const PRESS_AND_HOLD_RECORD_FILE = 'macos-press-and-hold-default.json'
 export const PRESS_AND_HOLD_RECORD_VERSION = 1
 
-/** Why not `systemPreferences.getUserDefault`: it reads through the whole NSUserDefaults search
- *  list and is typed non-nullable, so an unset key and an explicit `false` both come back `false` —
- *  and the system default is unset, so it reports `false` on a Mac where press-and-hold is on.
- *  `defaults read <domain> <key>` is domain-scoped and exits 1 when the key is absent. */
+// defaults read distinguishes an absent key from explicit false; systemPreferences.getUserDefault cannot.
 const DEFAULTS_BINARY = '/usr/bin/defaults'
-/** A hang cutoff, not a latency budget: the probe measures ~6ms, and cutting a slow-but-working
- *  `defaults` short costs the user the feature for that launch. The shared 30s default is what
- *  must not apply — this runs before `ready`, so a 30s block is a hung app. */
+// This runs before ready; bound a wedged defaults process more tightly than the shared 30s timeout.
 const DEFAULTS_TIMEOUT_MS = 5_000
-/** Why: `defaults` exits 1 for "does not exist"; anything else means the probe itself failed. */
+// defaults uses exit 1 for an absent key; other failures leave its presence unknown.
 const DEFAULTS_MISSING_STATUS = 1
 
 const ORCA_BUNDLE_ID = 'com.stablyai.orca'
 
 export type PressAndHoldDecision =
-  /** Not macOS — nothing is read or written. */
   | 'not-macos'
-  /** A previous launch already decided; the domain is never touched again. */
   | 'already-decided'
-  /** The running bundle is not Orca's (e.g. a bare `Electron.app`), whose domain we do not own. */
   | 'foreign-bundle'
-  /** `defaults read` could not answer, so we cannot tell an unset key from a user's choice. */
   | 'probe-failed'
-  /** `defaults write` failed. */
   | 'write-failed'
-  /** The domain already carries an explicit value; the user's choice stands. */
   | 'kept-user-preference'
-  /** The key was unset and we wrote `false`. */
   | 'applied'
 
-/** Decisions about the domain itself. Anything else is a condition that can change by next launch. */
+// Transient failures are retried on later launches; completed decisions preserve user changes.
 const TERMINAL_DECISIONS = new Set<PressAndHoldDecision>(['applied', 'kept-user-preference'])
 
 export type PressAndHoldRecord = {
@@ -81,13 +49,11 @@ export type PressAndHoldHost = {
   now: () => string
 }
 
-/** Only Orca's own bundle: an unpackaged run is `com.github.Electron`, shared with every other
- *  unpackaged Electron app on the machine. */
+// Unpackaged Electron apps share com.github.Electron; do not change that domain.
 export function isOrcaPreferencesDomain(domain: string): boolean {
   return domain === ORCA_BUNDLE_ID || domain.startsWith(`${ORCA_BUNDLE_ID}.`)
 }
 
-/** `<bundle>/Contents/MacOS/<exe>` → `<bundle>/Contents/Info.plist`. */
 export function readBundleIdentifierFromExecutablePath(execPath: string): string | null {
   try {
     const plist = readFileSync(join(dirname(dirname(execPath)), 'Info.plist'), 'utf8')
@@ -99,20 +65,15 @@ export function readBundleIdentifierFromExecutablePath(execPath: string): string
   }
 }
 
-/**
- * Why this distinction carries the whole design: only a real "does not exist" answer may be read as
- * unset. A killed or timed-out probe never answered, and calling that unset would let a broken
- * probe overwrite a value the user chose.
- */
+/** Failed probes cannot establish absence and must never authorize overwriting a preference. */
 export function interpretDefaultsRead(probe: () => ProcessResult): 'set' | 'unset' | 'unknown' {
   let result: ProcessResult
   try {
     result = probe()
   } catch {
-    // `runProcessSync` throws only when the child never started, which answers nothing at all.
     return 'unknown'
   }
-  // Why before the code: a timeout kills mid-run, so any exit it leaves behind is not an answer.
+  // A timeout invalidates any exit status from the interrupted probe.
   if (result.timedOut) {
     return 'unknown'
   }
@@ -123,8 +84,7 @@ export function interpretDefaultsRead(probe: () => ProcessResult): 'set' | 'unse
 }
 
 export function readDomainPressAndHoldPreference(domain: string): 'set' | 'unset' | 'unknown' {
-  // Why no captured output: the exit code is the whole answer, and reading only presence is what
-  // makes an explicit `true` and an explicit `false` both count as the user's.
+  // Presence alone preserves both explicit true and explicit false as user choices.
   return interpretDefaultsRead(() =>
     runProcessSync({
       program: DEFAULTS_BINARY,
@@ -137,8 +97,6 @@ export function readDomainPressAndHoldPreference(domain: string): 'set' | 'unset
 
 export function writeDomainPressAndHoldPreference(domain: string, value: boolean): boolean {
   try {
-    // Why the code check: a non-zero `defaults write` returns rather than throwing, so treating a
-    // completed run as success would record 'applied' for a write that never landed.
     return (
       runProcessSync({
         program: DEFAULTS_BINARY,
@@ -169,13 +127,7 @@ function parseRecord(raw: string): PressAndHoldRecord | null {
   }
 }
 
-/**
- * Apply Orca's press-and-hold default at most once, leaving any explicit user value alone.
- *
- * Returns the decision so startup can log it; the same value is persisted for support triage.
- */
 export function ensureMacPressAndHoldDefault(host: PressAndHoldHost): PressAndHoldDecision {
-  // Why first and without touching disk: Windows and Linux must do no work at all here.
   if (host.platform !== 'darwin') {
     return 'not-macos'
   }
@@ -186,7 +138,7 @@ export function ensureMacPressAndHoldDefault(host: PressAndHoldHost): PressAndHo
   }
 
   const record = (decision: PressAndHoldDecision, domain: string | null): PressAndHoldDecision => {
-    // Why skip an unchanged rewrite: the non-terminal decisions are re-evaluated every launch.
+    // Transient decisions recur on every launch; persist only changed results.
     if (!previous || previous.decision !== decision || previous.domain !== domain) {
       host.writeRecord({
         version: PRESS_AND_HOLD_RECORD_VERSION,
@@ -216,15 +168,12 @@ export function ensureMacPressAndHoldDefault(host: PressAndHoldHost): PressAndHo
   return record('applied', domain)
 }
 
-/** Why log: silently rewriting a macOS default for the user's login account should leave a trace,
- *  and the failure paths are otherwise invisible. The quiet decisions are the steady state. */
 const REPORTED_DECISIONS: Partial<Record<PressAndHoldDecision, string>> = {
   applied: `set ${PRESS_AND_HOLD_KEY}=false so held keys repeat; takes effect next launch`,
   'probe-failed': `could not read ${PRESS_AND_HOLD_KEY}; leaving it alone`,
   'write-failed': `could not write ${PRESS_AND_HOLD_KEY}; held keys will not repeat`
 }
 
-/** Wires {@link ensureMacPressAndHoldDefault} to the real bundle, `defaults`, and userData. */
 export function applyMacPressAndHoldDefaultAtStartup(userDataPath: string): PressAndHoldDecision {
   const recordPath = pressAndHoldRecordPath(userDataPath)
   const decision = ensureMacPressAndHoldDefault({
@@ -239,11 +188,11 @@ export function applyMacPressAndHoldDefaultAtStartup(userDataPath: string): Pres
     },
     writeRecord: (record) => {
       try {
-        // Why: this runs before `ready`, and Electron has not necessarily created userData yet.
+        // Before ready, Electron may not have created userData yet.
         mkdirSync(userDataPath, { recursive: true })
         writeFileAtomically(recordPath, `${JSON.stringify(record, null, 2)}\n`)
       } catch {
-        // Best-effort: an unwritten record only costs one repeated probe next launch.
+        // If persistence fails, a later launch safely probes the preference again.
       }
     },
     readDomainPreference: readDomainPressAndHoldPreference,

src/main/macos-tcc-log-stream-orphans.test.ts

@@ -30,7 +30,7 @@ describe('findOrphanedTccLogStreams', () => {
         row(11, 4321),
         row(12, 1, '/usr/bin/log stream --predicate subsystem == "com.apple.TCC" --style compact'),
         row(13, 1, '/usr/bin/log show --last 1m'),
-        // A pid recycled between the args and comm sweeps, or an unreadable comm.
+        // Simulate PID reuse between argv and executable snapshots, plus unreadable executable identity.
         row(14, 1, LEGACY_ARGS, '/usr/bin/yes'),
         row(15, 1, LEGACY_ARGS, '')
       ]).map((match) => match.pid)

src/main/macos-tcc-log-stream-orphans.ts

@@ -7,31 +7,25 @@ import {
 } from './opencode/opencode-client-sweep'
 import { LOG_PREDICATE } from './macos-tcc-prompt-watch'
 
-/** The exact argv that Orca builds before the sh wrapper spawned `log stream` directly. */
+// Match legacy builds that spawned log stream without a lifetime wrapper.
 const LEGACY_LOG_STREAM_COMMAND = splitCommandLineArgv(
   ['/usr/bin/log', 'stream', '--predicate', LOG_PREDICATE, '--info', '--style', 'compact'].join(' ')
 ).join(' ')
 
-// Why: `etime` has 1 s resolution and two reads of it can straddle a tick.
+// etime has one-second resolution; separate snapshots may straddle a tick.
 const START_TIME_TOLERANCE_MS = 2_000
 
 function isLegacyOrphan(row: ProcessIdentityRow): boolean {
   return row.ppid === 1 && row.argv.join(' ') === LEGACY_LOG_STREAM_COMMAND
 }
 
-/**
- * Why: older builds left this `log stream` running under launchd whenever Orca died
- * uncleanly, and those orphans outlive every later launch. A live watcher's
- * `log stream` is parented by its wrapper, so only launchd's children (ppid 1) match.
- * `executable` comes from a later `ps` than argv, so a pid recycled between them fails.
- */
+/** Only legacy streams reparented to launchd qualify; live streams belong to their wrapper. */
 export function findOrphanedTccLogStreams(
   rows: readonly ProcessIdentityRow[]
 ): ProcessIdentityRow[] {
   return rows.filter((row) => row.executable === '/usr/bin/log' && isLegacyOrphan(row))
 }
 
-/** One-snapshot re-read of a single pid, so its ppid, start time and argv agree. */
 async function readProcessIdentity(pid: number): Promise<ProcessIdentityRow | null> {
   const result = await runProcess({
     program: 'ps',
@@ -59,8 +53,7 @@ export async function reclaimOrphanedTccLogStreams(
     await (deps.sweep ?? sweepProcessIdentities)()
   )) {
     try {
-      // Why: the sweep joins two `ps` snapshots, so re-read right before signalling
-      // and skip anything whose identity changed or can no longer be confirmed.
+      // Recheck identity immediately before signalling; the sweep joins separate ps snapshots.
       const current = await readProcess(candidate.pid)
       if (
         !current ||
@@ -71,9 +64,7 @@ export async function reclaimOrphanedTccLogStreams(
       }
       kill(candidate.pid, 'SIGTERM')
       reclaimed++
-    } catch {
-      // Already gone, or another user's process.
-    }
+    } catch {}
   }
   if (reclaimed > 0) {
     console.info(`[tcc-prompt-watch] reclaimed ${reclaimed} orphaned log stream process(es)`)

src/main/macos-tcc-prompt-notice.ts

@@ -6,19 +6,12 @@ import { getCanonicalUserDataPath } from './persistence'
 import { MacosTccPromptWatch } from './macos-tcc-prompt-watch'
 import { reclaimOrphanedTccLogStreams } from './macos-tcc-log-stream-orphans'
 
-/**
- * Surfaces Full Disk Access guidance only to users macOS is actually prompting
- * (#9756), instead of nudging every Mac user. Counts Orca-attributed TCC
- * dialogs across launches and tells the renderer when the first one lands.
- */
-
-/** Why: the first detected dialog identifies an affected user; the notice remains one-time. */
 export const TCC_PROMPT_NOTICE_THRESHOLD = 1
 
-/** Why: re-arm users who may have trusted the former false-positive Full Disk Access badge. */
+// Re-arm users who may have trusted the former false-positive Full Disk Access badge.
 export const TCC_PROMPT_NOTICE_VERSION = 2
 
-/** Why: preserve detection when Electron never emits `ready-to-show` without competing with startup. */
+// Fall back when ready-to-show never fires, while keeping detection out of startup’s critical path.
 export const TCC_PROMPT_WATCH_START_FALLBACK_MS = 10_000
 
 export const TCC_PROMPT_NOTICE_CHANNEL = 'macosTccPrompts:threshold'
@@ -77,7 +70,7 @@ function loadTally(): TccPromptTally {
       return { ...EMPTY_TALLY }
     }
     if (typeof parsed.acknowledgedAfterClose !== 'boolean') {
-      // Why: an incomplete tally proves only a past prompt, not that access is still missing.
+      // An incomplete tally proves a past prompt, not that access is still missing.
       return { ...EMPTY_TALLY }
     }
     const acknowledgedAfterClose = parsed.acknowledgedAfterClose === true
@@ -97,7 +90,7 @@ function saveTally(): void {
   try {
     writeFileAtomically(tallyPath(), `${JSON.stringify(tally, null, 2)}\n`)
   } catch {
-    // Best-effort: losing the count only means the notice arrives a launch later.
+    // Failed persistence only delays the notice until another prompt.
   }
 }
 
@@ -154,7 +147,6 @@ export function releasePendingTccPromptNotice(ownerToken: number, claimId?: numb
   }
 }
 
-/** Permanently stops the notice for this user; the watcher shuts down with it. */
 export function dismissTccPromptNotice(): void {
   pendingClaim = null
   tally = { ...tally, dismissed: true, notified: true, acknowledgedAfterClose: true }
@@ -178,7 +170,7 @@ function sendTccPromptNotice(mainWindow: BrowserWindow, payload: TccPromptNotice
   try {
     mainWindow.webContents.send(TCC_PROMPT_NOTICE_CHANNEL, payload)
   } catch {
-    // Why: the durable renderer pull recovers a send lost during renderer teardown.
+    // The durable renderer pull recovers sends lost during teardown.
   }
 }
 
@@ -227,7 +219,7 @@ export function initTccPromptNotice(
   }
   if (!orphanReclaimScheduled) {
     orphanReclaimScheduled = true
-    // Why: runs even after the notice is dismissed, since older builds' orphans are still alive.
+    // Legacy orphan cleanup remains necessary after the notice is dismissed.
     setTimeout(
       () => void reclaimOrphanedTccLogStreams(),
       TCC_PROMPT_WATCH_START_FALLBACK_MS
@@ -263,7 +255,7 @@ export function initTccPromptNotice(
       if (target) {
         sendTccPromptNotice(target, payload)
       }
-      // Why: pending state is renderer-acknowledged, so the log child can stop at threshold.
+      // The persisted pending notice survives until renderer acknowledgement, so logging can stop now.
       stopTccPromptNotice()
     }
   })

src/main/macos-tcc-prompt-watch.test.ts

@@ -8,12 +8,11 @@ import {
   parseTccPromptEvent
 } from './macos-tcc-prompt-watch'
 
-// Captured verbatim from `log stream --predicate 'subsystem == "com.apple.TCC"'`
-// on macOS 26.5 while a real consent dialog was displayed and denied.
+// Captured from log stream on macOS 26.5 while displaying and denying a real consent dialog.
 const REAL_PROMPT_LINE =
   '2026-07-27 15:35:26.136 Df tccd[79149:c81551c] [com.apple.TCC:access] AUTHREQ_PROMPTING: msgID=80871.81, service=kTCCServiceSystemPolicyDocumentsFolder, subject=Sub:{com.orca.tccprobe.shapecapture}Resp:{TCCDProcess: identifier=com.orca.tccprobe.shapecapture, pid=74171, auid=501, euid=501, binary_path=/private/tmp/tccprobe/TccProbe.app/Contents/MacOS/TccProbe},'
 
-// Same shape, but the #9756 case: an agent CLI accesses, Orca is held responsible.
+// Derived fixture: an agent accesses files while Orca is held responsible (#9756).
 const ORCA_APPDATA_LINE =
   '2026-07-27 15:40:02.001 Df tccd[79149:c81551c] [com.apple.TCC:access] AUTHREQ_PROMPTING: msgID=80871.99, service=kTCCServiceSystemPolicyAppData, subject=Sub:{node-5555494487fbc7467d473fd8b0a397018cbf954b}Resp:{TCCDProcess: identifier=com.stablyai.orca, pid=47548, auid=501, euid=501, binary_path=/opt/homebrew/Cellar/node/26.5.0/bin/node},'
 
@@ -33,7 +32,6 @@ describe('parseTccPromptEvent', () => {
 
   it('separates the accessing binary from the responsible app', () => {
     const event = parseTccPromptEvent(ORCA_APPDATA_LINE)
-    // The whole point of #9756: the dialog says Orca, but node did the access.
     expect(event?.responsibleIdentifier).toBe('com.stablyai.orca')
     expect(event?.accessingIdentifier).toBe('node-5555494487fbc7467d473fd8b0a397018cbf954b')
     expect(event?.binaryPath).toBe('/opt/homebrew/Cellar/node/26.5.0/bin/node')
@@ -43,7 +41,6 @@ describe('parseTccPromptEvent', () => {
     expect(parseTccPromptEvent(PREFLIGHT_LINE)).toBeNull()
     expect(parseTccPromptEvent('Filtering the log data using "subsystem == ..."')).toBeNull()
     expect(parseTccPromptEvent('')).toBeNull()
-    // Has the marker but no parseable identities — must not yield a partial event.
     expect(parseTccPromptEvent('AUTHREQ_PROMPTING: msgID=1.2,')).toBeNull()
   })
 })
@@ -111,9 +108,7 @@ function createFakeLogStream(): {
   return { child, stdin, stdout, killed }
 }
 
-// Why: the watcher is darwin-gated, so CI (Linux) would silently no-op every
-// assertion below unless the platform is pinned. The gate itself is covered by
-// the non-darwin case at the end of this block.
+// Pin Darwin to exercise the watcher on every host; test the platform guard separately.
 const originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform')
 
 function setPlatform(value: NodeJS.Platform): void {
@@ -167,7 +162,7 @@ describe('MacosTccPromptWatch', () => {
     watch.start()
     watch.stop()
     expect(killed).toEqual(['SIGTERM'])
-    // Why: closing the parent-death pipe is the backstop if the wrapper missed the signal.
+    // Closing stdin is the parent-death backstop if the wrapper missed SIGTERM.
     expect(stdin.destroyed).toBe(true)
   })
 
@@ -239,7 +234,6 @@ describe('MacosTccPromptWatch', () => {
     })
     afterStop.stop()
     afterStop.start()
-    // Why: quit ordering can call stop() before start(); it must not leave a live child behind.
     expect(afterStop['child']).toBeNull()
   })
 })

src/main/macos-tcc-prompt-watch.ts

@@ -2,22 +2,12 @@ import { spawn, type ChildProcessByStdio } from 'node:child_process'
 import { createInterface, type Interface } from 'node:readline'
 import type { Readable, Writable } from 'node:stream'
 
-/** Why: stdin is a never-written pipe whose EOF tells the wrapper that Orca died. */
+/** Stdin EOF tells the wrapper that Orca died. */
 export type LogStreamChild = ChildProcessByStdio<Writable, Readable, Readable>
 
-/**
- * Counts the macOS TCC consent dialogs that name Orca as the responsible
- * process (#9756). Terminal children — agent CLIs and anything else the user
- * runs — perform the access, but TCC walks the responsibility chain back to
- * Orca and puts Orca's name on the dialog, so users read it as Orca snooping.
- *
- * tccd emits one `AUTHREQ_PROMPTING` line per dialog it actually displays,
- * carrying the service and both identities, so this never has to correlate
- * across lines or guess whether a dialog was shown. Routine preflight checks
- * (the overwhelming majority of TCC log traffic) do not emit it.
- */
+// AUTHREQ_PROMPTING identifies actual dialogs, including access attributed to Orca’s terminal children (#9756).
 
-/** Why: terminals run from the detached helper, which TCC can hold responsible independently. */
+// TCC can attribute terminal access to the detached helper independently of the main app.
 const ORCA_RESPONSIBLE_IDENTIFIERS = new Set([
   'com.stablyai.orca',
   'com.stablyai.orca.helper',
@@ -27,7 +17,6 @@ const ORCA_RESPONSIBLE_IDENTIFIERS = new Set([
   'com.stablyai.orca.local.helper'
 ])
 
-/** Why: the prompt classes #9756 is about — other-apps' data plus the protected home folders agents sweep. */
 const WATCHED_SERVICES = new Set([
   'kTCCServiceSystemPolicyAppData',
   'kTCCServiceSystemPolicyAllFiles',
@@ -40,20 +29,14 @@ export const LOG_PREDICATE =
   'subsystem == "com.apple.TCC" AND eventMessage CONTAINS "AUTHREQ_PROMPTING"'
 
 export type TccPromptEvent = {
-  /** TCC service the dialog was raised for, e.g. `kTCCServiceSystemPolicyAppData`. */
   service: string
-  /** Bundle id or hashed identity of the process performing the access (often an agent CLI). */
+  /** Bundle ID or hashed identity of the process performing the access. */
   accessingIdentifier: string
-  /** Bundle id macOS holds responsible, and therefore names in the dialog. */
+  /** Bundle ID named as responsible in the dialog. */
   responsibleIdentifier: string
-  /** Absolute path of the accessing binary, when tccd reports one. */
   binaryPath?: string
 }
 
-/**
- * Parses one `AUTHREQ_PROMPTING` log line. Returns null for anything else, so
- * callers can feed the raw stream (which includes a header line) straight in.
- */
 export function parseTccPromptEvent(line: string): TccPromptEvent | null {
   if (!line.includes('AUTHREQ_PROMPTING')) {
     return null
@@ -73,7 +56,6 @@ export function parseTccPromptEvent(line: string): TccPromptEvent | null {
   }
 }
 
-/** True when this dialog is one macOS raised in Orca's name for a watched file-access service. */
 export function isOrcaAttributedPrompt(event: TccPromptEvent): boolean {
   return (
     ORCA_RESPONSIBLE_IDENTIFIERS.has(event.responsibleIdentifier) &&
@@ -83,18 +65,13 @@ export function isOrcaAttributedPrompt(event: TccPromptEvent): boolean {
 
 export type TccPromptWatchOptions = {
   onPrompt: (event: TccPromptEvent) => void
-  /** Injected in tests; defaults to spawning `log stream`. */
   spawnLogStream?: () => LogStreamChild
-  /** Injected in tests; production waits before its single recovery attempt. */
   restartDelayMs?: number
 }
 
-// Why: `log stream` never notices its parent dying (it only writes on a rare
-// match), so a crashed or SIGKILLed Orca orphaned it forever. The wrapper kills
-// it when our stdin pipe hits EOF, and exits when it exits. fd 3 is needed
-// because async lists in a non-interactive sh get stdin=/dev/null. The trap is
-// armed before the fork; `term` covers a signal that lands before `log_pid` is set.
-// `set +eu` because an inherited SHELLOPTS=errexit would end the reader on read's EOF status.
+// log stream ignores parent death; stdin EOF makes the wrapper terminate it.
+// Preserve stdin on fd 3 because sh backgrounds async lists with /dev/null; arm the trap before forking.
+// Disable inherited errexit/nounset so EOF and early signals still reach cleanup.
 const LOG_STREAM_WRAPPER_SCRIPT = `set +eu
 exec 3<&0
 log_pid=
@@ -111,17 +88,13 @@ kill "$reader_pid" 2>/dev/null
 exit "$status"`
 
 function spawnDefaultLogStream(): LogStreamChild {
-  // Why: the predicate is evaluated inside the logging system, so this delivers
-  // ~1 line per real dialog instead of the ~30/s the TCC subsystem emits raw.
+  // Filter within logd to avoid streaming routine TCC traffic into main.
   return spawn('/bin/sh', ['-c', LOG_STREAM_WRAPPER_SCRIPT, 'sh', LOG_PREDICATE], {
     stdio: ['pipe', 'pipe', 'pipe']
   })
 }
 
-/**
- * Watches for Orca-attributed TCC dialogs. macOS-only; `start()` is a no-op
- * elsewhere so callers don't need their own platform guard.
- */
+/** start() is a no-op outside macOS. */
 export class MacosTccPromptWatch {
   private child: LogStreamChild | null = null
   private reader: Interface | null = null
@@ -140,8 +113,7 @@ export class MacosTccPromptWatch {
     try {
       child = spawnLogStream()
     } catch {
-      // Why: diagnostics only — a missing or restricted `log` binary must never
-      // break startup, and there is nothing actionable to tell the user.
+      // Optional diagnostics must not prevent startup if log is unavailable.
       return
     }
     this.child = child
@@ -158,13 +130,12 @@ export class MacosTccPromptWatch {
     this.reader?.close()
     this.reader = null
     this.child = null
-    // Why: EOF on stdin makes any surviving wrapper reader kill its log stream.
+    // EOF makes any surviving wrapper reader terminate its log stream.
     child.stdin.destroy()
     if (this.stopped || this.restartAttempted) {
       return
     }
     this.restartAttempted = true
-    // Why: recover one transient logd failure without respawning forever when logging is unavailable.
     this.restartTimer = setTimeout(() => {
       this.restartTimer = null
       this.start()
@@ -187,7 +158,7 @@ export class MacosTccPromptWatch {
     }
     this.reader?.close()
     this.reader = null
-    // Why: log stream ignores a closed stdout, so the child needs an explicit kill or it outlives quit.
+    // Closing stdout does not stop log stream; terminate it explicitly.
     this.child?.kill('SIGTERM')
     this.child?.stdin.destroy()
     this.child = null

src/main/macos-tcc-reset.ts

@@ -1,20 +1,12 @@
-// Clearing a macOS TCC row, plus the bundle id every row is keyed by. Shared because two remedies
-// must issue the identical `tccutil reset`: the computer-use helper's stale-grant reset and the
-// daemon folder-access fix (STA-7948), where clearing the row is what makes macOS ask again.
-
 import { join } from 'node:path'
 import { runProcess } from '@orca/process-host'
 import type { ProcessResult } from '@orca/process-host/process-spec'
 
-/** Bounded so a wedged helper cannot hold a caller: neither binary prompts, so neither lingers. */
 const TCC_COMMAND_TIMEOUT_MS = 10_000
 
 export type MacosTccResetResult = { ok: true } | { ok: false; detail: string }
 
-/**
- * The bundle's `CFBundleIdentifier`, or null when it cannot be read — Info.plist is usually a
- * binary plist, so PlistBuddy is the only reader that works on both encodings.
- */
+/** PlistBuddy handles both binary and XML Info.plist encodings. */
 export async function readMacosBundleId(appBundlePath: string): Promise<string | null> {
   try {
     const result = await runProcess({
@@ -31,13 +23,7 @@ export async function readMacosBundleId(appBundlePath: string): Promise<string |
   }
 }
 
-/**
- * Clears the TCC row for one service and bundle id, so the next access re-prompts.
- *
- * Failure is data, not an exception: `tccutil` exits 64 and explains itself on stderr when
- * LaunchServices does not know the bundle id, which is the ordinary outcome for an app running
- * from an unregistered location.
- */
+/** Return tccutil failures as data; unregistered bundles commonly exit 64. */
 export async function resetMacosTccPermission(
   service: string,
   bundleId: string

src/main/macos-tree-clone.ts

@@ -2,20 +2,16 @@ import { basename } from 'node:path'
 import { runProcess } from '@orca/process-host'
 import { rm } from './asar-transparent-fs'
 
-/**
- * Copies `source` to `destination`, whose parent must exist, keeping symlinks as links. One
- * `/bin/cp` per tree is several times faster than Node's file-by-file `fs.cp` on APFS.
- */
+/** Requires an existing destination parent; whole-tree cp avoids Node’s file-by-file APFS overhead. */
 export async function cloneMacTree(
   source: string,
   destination: string,
   signal?: AbortSignal
 ): Promise<void> {
-  // APFS clone first; a failed clone can leave a partial tree on other filesystems.
-  // Clearing also keeps `cp -R` from nesting `source` inside an existing `destination`.
+  // Clear partial clones before fallback, and prevent cp -R from nesting into an existing destination.
   for (const flags of ['-cR', '-R']) {
     await rm(destination, { recursive: true, force: true })
-    // Why no timeout: a large tree at background I/O priority can outlast the 30 s default.
+    // Large trees copied at background I/O priority can exceed the default timeout.
     const result = await runProcess({
       program: '/bin/cp',
       args: [flags, source, destination],

src/main/orca-chromium-process-pids.ts

@@ -2,38 +2,9 @@ import { getAppEnvironment, hasAppEnvironment } from '../shared/app-environment'
 import { recordCoalescedDurableCrashBreadcrumb } from './crash-reporting/durable-crash-breadcrumb'
 
 /**
- * PIDs of Orca's own Chromium processes — browser, renderers, GPU, utilities.
- *
- * Why: `taskkill /T /F` aimed at one of these kills a renderer we depend on, and
- * the `render-process-gone` it produces is indistinguishable from an external
- * kill in every field Orca records (#10680). A pid in this set is proof the
- * target is ours to keep, not ours to tear down.
- *
- * Empty on a Node host and empty on failure: that is "no refusal proven", never
- * "safe to kill" — callers must keep every other guard they already have.
- *
- * The other direction is real too, and bounded by design: `getAppMetrics()` can
- * still list a renderer Electron has not finished reaping, so on Windows a pid
- * already recycled onto an unrelated child of ours reads as `own` and its tree
- * walk is refused. That is why a refusal only blocks the pid-addressed walk and
- * every gated site still kills its own root through the child handle.
- *
- * Why failure stays open rather than refusing everything: a refusal is not free.
- * `terminateWindowsProcessTree` resolves without killing, and
- * `killSourceControlAgentProcess` returns that straight to a caller that then
- * releases the managed-home lock, so failing closed would trade one unreadable
- * metrics table for every PTY, git, codex and notebook tree in main leaking at
- * once. The `own_chromium_pids_unreadable` crumb is the price of that choice:
- * without it a throw is byte-identical to "no Chromium on this host".
- *
- * Host coverage: only Electron main installs a Chromium-backed AppEnvironment
- * (main-process-preflight). The standalone daemon installs none and `orcad`
- * installs a Node one whose `getAppMetrics()` is `[]`, so this set is empty in
- * both — and that is sound, not a hole: the pid-addressed kills those hosts
- * issue go through `classifyWindowsTreeKillTarget`, which walks ancestry back to
- * the *killing* process's own pid. Orca's Chromium processes are children of
- * Electron main, so they never classify `own` from a daemon or orcad host, and
- * on an SSH/serve host there is no Chromium on the machine at all.
+ * Guard Windows PID-based kills; stale metrics must not prevent root termination through its handle.
+ * Missing metrics leave other kill guards responsible: refusing all kills would leak managed processes.
+ * Daemon/Node hosts have no Chromium metrics and instead classify targets against their own ancestry.
  */
 export function readOrcaChromiumProcessPids(): ReadonlySet<number> {
   if (!hasAppEnvironment()) {
@@ -51,8 +22,7 @@ export function readOrcaChromiumProcessPids(): ReadonlySet<number> {
   }
 }
 
-// Why coalesced: the gate reads this set on every tree kill, so a persistently
-// broken metrics table would otherwise flood the 30-slot ring it shares.
+// Repeated kill attempts must not flood the shared breadcrumb ring when metrics remain unreadable.
 const UNREADABLE_METRICS_COALESCE_MS = 60_000
 
 function recordUnreadableOwnChromiumMetrics(error: unknown): void {
@@ -64,7 +34,6 @@ function recordUnreadableOwnChromiumMetrics(error: unknown): void {
       minIntervalMs: UNREADABLE_METRICS_COALESCE_MS
     })
   } catch {
-    // Diagnostics must never turn an admitted kill into a thrown one: callers
-    // read this set outside their own try.
+    // Diagnostic failures must not interrupt callers’ termination paths.
   }
 }

src/main/own-chromium-tree-kill-guard.test.ts

@@ -28,10 +28,8 @@ import { _resetTracerForTests, setActiveSink } from './observability/tracer'
 
 const ORCA_MAIN_PID = 1000
 const RENDERER_PID = 1001
-/** The standalone daemon is a sibling of the renderers, spawned by main. */
 const DAEMON_PID = 1500
 
-/** Orca's renderer is a direct child of the main process, so the ppid walk says `own`. */
 const PROCESS_ROWS = [
   { pid: RENDERER_PID, ppid: ORCA_MAIN_PID },
   { pid: ORCA_MAIN_PID, ppid: 900 }
@@ -90,9 +88,7 @@ describe('refusing to tree-kill our own Chromium processes', () => {
   ])(
     'refuses an Orca renderer from a daemon host with %s, because no Chromium descends from it',
     (_case, ownChromiumPids) => {
-      // The standalone daemon and orcad install no Chromium-backed AppEnvironment,
-      // so this set is empty there. The ancestry walk is what refuses instead: it
-      // ends at the *killing* process's pid, and the renderer's chain reaches main.
+      // Daemon ancestry stops at the daemon PID; Chromium siblings fail that check without metrics.
       const rows = [...PROCESS_ROWS, { pid: DAEMON_PID, ppid: ORCA_MAIN_PID }]
 
       expect(classifyWindowsTreeKillTarget(RENDERER_PID, rows, DAEMON_PID, ownChromiumPids)).toBe(
@@ -102,8 +98,7 @@ describe('refusing to tree-kill our own Chromium processes', () => {
   )
 
   it('is the only thing standing between Electron main and its own renderer', () => {
-    // Falsifiable counterpart to the daemon case above: in main the ancestry walk
-    // says `own`, so the pid set is load-bearing here and nowhere else.
+    // In main, renderer ancestry passes; Chromium metrics provide the refusal.
     expect(
       classifyWindowsTreeKillTarget(RENDERER_PID, PROCESS_ROWS, ORCA_MAIN_PID, new Set())
     ).toBe('own')
@@ -159,8 +154,7 @@ describe('refusing to tree-kill our own Chromium processes', () => {
       spawnImpl: spawnImpl as never
     })
 
-    // The deadline timer fires on `child.pid` alone; a reaped-then-recycled pid
-    // is the stale-pid mechanism this gate exists to stop.
+    // Deadline kills have only a PID, which may have been reused after the child exited.
     expect(spawnImpl).not.toHaveBeenCalled()
     expect(getCrashBreadcrumbSnapshot()).toEqual([
       expect.objectContaining({ name: 'self_tree_kill_refused_own_chromium' })
@@ -182,19 +176,13 @@ describe('refusing to tree-kill our own Chromium processes', () => {
     })
   })
 
-  /**
-   * Fail-open is the deliberate choice — see `orca-chromium-process-pids.ts` for
-   * why refusing everything is worse — so the crumb is the only thing that keeps
-   * an unreadable metrics table distinguishable from a host that has no Chromium.
-   */
   it('leaves proof, and still admits the kill, when the Chromium metrics cannot be read', () => {
     appMetricsMock.mockImplementation(() => {
       throw new Error('getAppMetrics unavailable')
     })
 
     expect([...readOrcaChromiumProcessPids()]).toEqual([])
-    // Coalesced: the gate reads this set on every kill, so a broken table must
-    // not evict the ring it shares with the refusal crumb.
+    // Coalesce repeated failures so they cannot evict refusal evidence from the shared ring.
     expect([...readOrcaChromiumProcessPids()]).toEqual([])
     expect(
       admitSelfInitiatedTreeKill({

src/main/own-chromium-tree-kill-guard.ts

@@ -6,40 +6,13 @@ import {
 import { readOrcaChromiumProcessPids } from './orca-chromium-process-pids'
 import { setProcessTreeKillGate } from '@orca/process-host/process-tree-kill-gate'
 
-/**
- * Gate every main-process tree-kill through one decision: refuse a pid-addressed
- * walk when Electron is currently accounting for the pid, otherwise put the
- * kill on the record.
- *
- * Why a shared gate rather than a check inside `terminateWindowsProcessTree`:
- * five other families in main run their own `taskkill /T /F` with different
- * lifetimes (sync, fire-and-forget, timeout ladder), and three more live in
- * `src/shared` and reach this through `process-tree-kill-gate`, so a guard that
- * only lived in the tree-kill helper would cover one of nine.
- * Returns false
- * when the caller must not walk that pid's tree; the caller still kills its own
- * root through the child handle (`refused-tree-kill-root-termination.test.ts`),
- * so a refusal is never a process leak.
- *
- * Electron main only, by construction. `terminateWindowsProcessTree` also runs
- * in the standalone daemon (the `pty-descendant-sweep` site), where
- * `readOrcaChromiumProcessPids()` is empty and this always admits. That is not
- * the gap it looks like: the daemon reaches that taskkill only through
- * `classifyWindowsTreeKillTarget`, whose ancestry walk ends at the daemon's own
- * pid, and no Chromium process descends from the daemon. See
- * `orca-chromium-process-pids.ts`.
- */
+/** Refusal forbids only the PID-based tree walk; callers must still kill the root through its handle. */
 export function admitSelfInitiatedTreeKill(target: {
   pid: number
   site: string
   scope: SelfInitiatedTreeKillScope
 }): boolean {
-  // Why: no PTY root, codex root or git child is ever one of our own Chromium
-  // processes, so a pid that is means the caller is about to kill a renderer,
-  // the GPU or the browser itself (#10680). Only the pid-addressed scope can
-  // land there: a POSIX group holds only what Orca put in it, so that arm is
-  // recorded and admitted like every other group kill in main, and a stale
-  // `getAppMetrics()` entry cannot orphan a macOS/Linux tree.
+  // Windows PID reuse can target Chromium; POSIX groups contain only processes assigned to them (#10680).
   const isOwnChromiumPid =
     target.scope === 'win-taskkill-tree' && readOrcaChromiumProcessPids().has(target.pid)
   try {
@@ -49,13 +22,12 @@ export function admitSelfInitiatedTreeKill(target: {
       recordSelfInitiatedTreeKill(target)
     }
   } catch {
-    // Recording must never turn a successful termination into a failed one, and
-    // never flip the decision: it is taken above, before anything can throw.
+    // Recording failures must neither interrupt termination nor change the guard’s decision.
   }
   return !isOwnChromiumPid
 }
 
-/** Hands the gate to the shared choke points, which cannot import main. */
+/** Shared kill sites cannot import main; install the same guard through their hook. */
 export function installMainProcessTreeKillGate(): void {
   setProcessTreeKillGate((kill) => admitSelfInitiatedTreeKill(kill))
 }