[comment audit] Shorten top-level main process H–K comments (#32290)

* 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

---------

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

src/main/hook-archive-timeout-observation.test.ts

@@ -10,7 +10,6 @@ vi.mock('./effective-hook-config', () => ({
 
 const REPO: Repo = { id: 'r', path: '/repo', displayName: 'r', badgeColor: '#000', addedAt: 0 }
 
-/** Run a real archive script in a real shell, under a deadline short enough to test. */
 async function runArchive(script: string, timeoutMs = 400) {
   const { runHook } = await import('./hooks')
   const dir = mkdtempSync(join(tmpdir(), 'orca-hook-deadline-'))
@@ -22,14 +21,12 @@ async function runArchive(script: string, timeoutMs = 400) {
   }
 }
 
-// Why a real shell (#19334): this bug is invisible to a mock. Node's `exec({ timeout })` SIGTERMs
-// the child and reports whatever it chose to do, so a hook that traps SIGTERM and exits 0 came
-// back as a PASS — a hook cut off mid-archive, indistinguishable from one that finished its work.
+// Real shells expose SIGTERM traps that exit 0 after an archive timeout (#19334).
 describe.skipIf(process.platform === 'win32')('archive hook deadline', () => {
   it('fails a hook that traps SIGTERM and exits zero, despite its zero exit', async () => {
     const result = await runArchive("trap 'exit 0' TERM; sleep 30")
     expect(result.success).toBe(false)
-    // Withheld, so the removal gate reads `unverifiable` rather than a pass.
+    // Missing exitCode keeps the removal verdict unverifiable.
     expect(result.exitCode).toBeUndefined()
     expect(result.output).toContain('timed out')
   }, 20_000)
@@ -38,7 +35,6 @@ describe.skipIf(process.platform === 'win32')('archive hook deadline', () => {
     const started = Date.now()
     const result = await runArchive("trap '' TERM; sleep 30")
     expect(result.success).toBe(false)
-    // A hook that ignores the signal must not hold a removal open until it finishes.
     expect(Date.now() - started).toBeLessThan(10_000)
   }, 20_000)
 

src/main/hook-runtime-target.ts

@@ -40,7 +40,7 @@ export function getHookWslContext(
     return null
   }
 
-  // Why: project runtime can route a Windows checkout through WSL, so hooks need the Linux view of the path.
+  // Project runtime may route a Windows checkout through WSL; hooks need its Linux path.
   return {
     distro: wslDistro,
     linuxPath: toLinuxPath(cwd)

src/main/hook-termination-real-process.test.ts

@@ -15,7 +15,6 @@ const alive = (pid: number): boolean => {
   }
 }
 
-/** Run a hook past its deadline and report which of its real processes survived. */
 async function survivorsAfterDeadline(
   script: string
 ): Promise<{ shell: boolean; child: boolean; output: string; pids: number[] }> {
@@ -32,8 +31,7 @@ async function survivorsAfterDeadline(
     expect(result.success).toBe(false)
     expect(existsSync(pidFile)).toBe(true)
     pids = readFileSync(pidFile, 'utf8').trim().split(/\s+/).map(Number)
-    // Without this, a script that recorded only the shell leaves `pids[1]` undefined, `alive`
-    // throws, and the missing descendant reads as dead — a test that passes on nothing.
+    // Require both PIDs so a missing descendant cannot make the liveness assertion pass vacuously.
     expect(pids).toHaveLength(2)
     expect(pids.every((pid) => Number.isSafeInteger(pid) && pid > 0)).toBe(true)
     // SIGTERM lands at the deadline; resistant descendants need the real SIGKILL grace.
@@ -43,25 +41,19 @@ async function survivorsAfterDeadline(
     for (const pid of pids) {
       try {
         process.kill(pid, 'SIGKILL')
-      } catch {
-        /* already gone */
-      }
+      } catch {}
     }
     rmSync(dir, { recursive: true, force: true })
   }
 }
 
-// NO `process.kill` mock, deliberately. The defect this file exists for — `exec` silently ignoring
-// `detached`, so the shell was never a group leader and the group signal reached nothing — is
-// invisible to a mocked `process.kill`, because the mock makes the signal-0 probe succeed whether
-// or not a real group exists. That is the precise condition the bug turns on.
+// Mocked kill probes cannot prove a process group exists; exercise real signals here.
 describe.skipIf(process.platform === 'win32')('hook termination against real processes', () => {
   it('kills the shell and its child when the deadline expires', async () => {
     const { shell, child, output } = await survivorsAfterDeadline(
       'echo "archive step 3 of 7"\nsleep 120 &\necho "$$ $!" > "$PWD/pids"\nwait'
     )
     expect({ shell, child }).toEqual({ shell: false, child: false })
-    // The gate reports this run as `unverifiable`; what the hook printed is the only clue why.
     expect(output).toContain('archive step 3 of 7')
   }, 30_000)
 

src/main/hooks-archive-exit-observation.test.ts

@@ -10,10 +10,7 @@ vi.mock('./effective-hook-config', () => ({
 
 const REPO: Repo = { id: 'r', path: '/repo', displayName: 'r', badgeColor: '#000', addedAt: 0 }
 
-/**
- * A real EventEmitter, so an `error` with no listener throws exactly as Node's would — which is the
- * whole point of the stream-error row below. Replays its chunks to whoever subscribes to `data`.
- */
+/** EventEmitter preserves Node’s fatal unhandled-error behavior while replaying chunks. */
 class FakeStream extends EventEmitter {
   constructor(private readonly chunks: string[]) {
     super()
@@ -30,7 +27,6 @@ class FakeStream extends EventEmitter {
   }
 }
 
-/** Minimal ChildProcess stand-in: runHook reads the streams and waits for close/error. */
 function fakeChild(
   outcome: { code?: number | null; signal?: NodeJS.Signals | null } | Error,
   stdoutChunks: string[] = [],
@@ -79,10 +75,7 @@ async function runArchiveWith(
   return result
 }
 
-// Why (#19334): an ABSENT exitCode is what the removal gate reads as `unverifiable`. Every row here
-// is a way a hook can fail to deliver one. The timeout and termination arms of the same contract
-// are covered against REAL processes in hook-termination-real-process.test.ts — deliberately not
-// here, because a mocked child cannot show whether a process group exists.
+// Real-process timeout/group tests live in hook-termination-real-process.test.ts (#19334).
 describe('archive hook exit observation', () => {
   it('passes a clean run through without an exit code', async () => {
     await expect(runArchiveWith({ code: 0 })).resolves.toEqual({ success: true, output: '' })
@@ -99,8 +92,6 @@ describe('archive hook exit observation', () => {
   })
 
   it('caps what it retains from a hook that floods stdout', async () => {
-    // `exec`'s 1 MiB maxBuffer is gone with `spawn`; without a cap a flooding hook grows the main
-    // process's heap for the whole 120 s deadline.
     const megabyte = 'x'.repeat(1024 * 1024)
     const result = await runArchiveWith(
       { code: 0 },
@@ -111,8 +102,6 @@ describe('archive hook exit observation', () => {
   })
 
   it('survives an error on the output stream', async () => {
-    // An `error` with no listener is an uncaught exception, and in the main process that is the
-    // app. `exec` never covered this either — its only `error` listener is on the child.
     await expect(
       runArchiveWith({ code: 0 }, ['partial'], new Error('EIO: read failed'))
     ).resolves.toMatchObject({ success: true })
@@ -125,8 +114,7 @@ describe('archive hook exit observation', () => {
 
   it.each([
     ['was killed by a signal', { code: null, signal: 'SIGKILL' as const }],
-    // A real spawn failure carries a STRING code; the guard under test is `typeof code ===
-    // 'number'`, so a bare Error would pass even if that guard regressed.
+    // Spawn errors carry string codes; a bare Error would not exercise the numeric-code guard.
     ['never started', Object.assign(new Error('spawn /bin/bash ENOENT'), { code: 'ENOENT' })]
   ])('withholds the exit code when the hook %s', async (_label, outcome) => {
     const result = await runArchiveWith(outcome)

src/main/hooks-effective-hook-resolution.test.ts

@@ -7,7 +7,6 @@ import {
   TEST_WORKTREE_PATH
 } from './hooks-test-fixtures'
 
-// Mock fs used by loadHooks
 vi.mock('fs', () => ({
   readFileSync: vi.fn(),
   existsSync: vi.fn(),
@@ -18,7 +17,6 @@ vi.mock('fs', () => ({
 }))
 
 describe('getEffectiveHooks', () => {
-  // We need to dynamically import after mocking
   const makeRepo = (hookSettings?: {
     mode?: 'auto' | 'override'
     setupRunPolicy?: 'ask' | 'run-by-default' | 'skip-by-default'
@@ -31,7 +29,6 @@ describe('getEffectiveHooks', () => {
     vi.mocked(fs.existsSync).mockReturnValue(true)
     vi.mocked(fs.readFileSync).mockReturnValue('scripts:\n  setup: |\n    echo "yaml setup"\n')
 
-    // Re-import to pick up mocks
     const { getEffectiveHooks } = await import('./hooks')
     const repo = makeRepo()
     const result = getEffectiveHooks(repo)

src/main/hooks-issue-command.test.ts

@@ -10,7 +10,6 @@ import {
   TEST_REPO_PATH
 } from './hooks-test-fixtures'
 
-// Mock fs used by loadHooks
 vi.mock('fs', () => ({
   readFileSync: vi.fn(),
   existsSync: vi.fn(),
@@ -28,7 +27,6 @@ vi.mock('child_process', () => ({
   exec: vi.fn(),
   execFile: vi.fn(),
   execFileSync: vi.fn(),
-  // runner.ts imports spawn from child_process transitively.
   spawn: vi.fn()
 }))
 
@@ -203,7 +201,7 @@ describe('createIssueCommandRunnerScript', () => {
         ['rev-parse', '--git-path', 'orca/issue-command-runner.cmd'],
         { cwd: 'C:\\repo-worktree' }
       )
-      // Why: the runner file is batch (.cmd) while the pane that launches it is still Git Bash.
+      // The pane remains Git Bash even when it launches a batch runner.
       expect(result.shell).toEqual({ family: 'posix' })
     } finally {
       Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform })

src/main/hooks-orca-yaml-parsing.test.ts

@@ -1,7 +1,6 @@
 import { describe, expect, it, vi } from 'vitest'
 import { parseOrcaYaml } from './hooks'
 
-// Mock fs used by loadHooks
 vi.mock('fs', () => ({
   readFileSync: vi.fn(),
   existsSync: vi.fn(),
@@ -338,9 +337,7 @@ describe('parseOrcaYaml', () => {
     expect(result?.worktree?.sharedDirectories).toEqual(['node_modules'])
   })
 
-  // Why: `resolve()` collapses `.` when the link is created, but Git reports the
-  // collapsed path — keeping the raw entry would leave a link that every later
-  // comparison misses, which is the permanently-dirty worktree this feature fixes.
+  // Resolve collapses path segments; raw entries would never match Git’s normalized paths.
   it('drops sharedDirectories entries that still need path collapsing', () => {
     const result = parseOrcaYaml(
       [

src/main/hooks-runner.test.ts

@@ -22,8 +22,6 @@ vi.mock('fs', () => ({
 vi.mock('child_process', () => ({
   exec: vi.fn(),
   execFileSync: execFileSyncMock,
-  // runner.ts imports these from child_process; stubs prevent
-  // "missing export" errors when the mock is resolved transitively.
   execFile: vi.fn(),
   spawn: vi.fn()
 }))
@@ -63,7 +61,6 @@ describe('createSetupRunnerScript', () => {
           ORCA_WORKTREE_PATH: 'C:\\repo\\feature\\',
           ORCA_WORKSPACE_NAME: 'feature'
         }),
-        // Why: native Windows worktrees without a configured setup shell keep the cmd runner.
         shell: { family: 'cmd' }
       })
       expect(vi.mocked(fs.writeFileSync)).toHaveBeenCalledWith(
@@ -111,7 +108,7 @@ describe('createSetupRunnerScript', () => {
           ORCA_WORKTREE_PATH: '/c/repo/feature',
           CONDUCTOR_ROOT_PATH: '/c/Users/jinwo/git/orca',
           GHOSTX_ROOT_PATH: '/c/Users/jinwo/git/orca',
-          // Why: a display name, never a path — it must survive the conversion untouched.
+          // Workspace names are labels and must not undergo path conversion.
           ORCA_WORKSPACE_NAME: 'feature'
         }),
         shell: { family: 'posix' }
@@ -121,7 +118,7 @@ describe('createSetupRunnerScript', () => {
         '#!/usr/bin/env bash\nset -e\npnpm install\n',
         'utf-8'
       )
-      // Why: chmod over a native Windows path is meaningless; only the WSL branch sets the bit.
+      // Only guest Linux paths need chmod; native Windows paths do not.
       expect(vi.mocked(fs.chmodSync)).not.toHaveBeenCalledWith(
         'C:\\repo\\.git\\worktrees\\feature\\orca\\setup-runner.sh',
         0o755
@@ -185,8 +182,7 @@ describe('createSetupRunnerScript', () => {
 
     const runner = buildWindowsRunnerScript('echo hello!world!')
 
-    // Why: launchers invoke the runner under cmd /v:on, so the runner must disable delayed
-    // expansion itself or `!world!` is consumed as a variable reference.
+    // Disable inherited cmd delayed expansion so !world! remains literal.
     expect(runner).toContain('setlocal EnableExtensions DisableDelayedExpansion')
     expect(runner).toContain('call echo hello!world!')
   })
@@ -194,10 +190,7 @@ describe('createSetupRunnerScript', () => {
   it('refuses to run a shebang script as a batch command', async () => {
     const { buildWindowsRunnerScript } = await import('./setup-runner-script-text')
 
-    // Regression: a POSIX script reaching the cmd runner (no Git Bash terminal, or an SSH
-    // Windows host) must not have its interpreter-agnostic prefix executed under cmd —
-    // `pnpm install` would run and only a later bash-only line would fail, leaving the
-    // worktree half set up.
+    // Reject POSIX scripts before cmd runs even their portable prefix and leaves setup incomplete.
     const runner = buildWindowsRunnerScript('#!/usr/bin/env bash\npnpm install\nsource .env')
 
     expect(runner).not.toContain('call pnpm install')
@@ -218,8 +211,7 @@ describe('createSetupRunnerScript', () => {
   it.skipIf(process.platform === 'win32')(
     'runs a script whose shebang carries invocation-only bash flags',
     async () => {
-      // Regression: `set` rejects `-l`/`-s`/`-i` with exit 2, and the runner's own `set -e` turns
-      // that into an aborted setup before the first user line.
+      // Reapplying login/interactive flags through set would abort before the first user line.
       const { buildPosixRunnerScript } = await import('./setup-runner-script-text')
       const { mkdtempSync, rmSync, writeFileSync } = await vi.importActual<typeof NodeFs>('node:fs')
       const { execFileSync } = await vi.importActual<typeof NodeChildProcess>('node:child_process')
@@ -435,7 +427,7 @@ describe('createIssueCommandRunnerScript', () => {
         { wslDistro: 'Ubuntu' }
       )
 
-      // Why: the runner path is written back in native Windows form, so the launch needs /mnt again.
+      // Runner paths are stored in Windows form and need /mnt translation again at launch.
       expect(result.runnerScriptPath).toBe('C:\\repo\\.git\\orca\\issue-command-runner.sh')
       expect(result.shell).toEqual({ family: 'posix', executable: 'wsl.exe' })
     } finally {

src/main/hooks-setup-runner-script.test.ts

@@ -3,7 +3,6 @@ import type * as GitRunner from './git/runner'
 import { describe, expect, it, vi } from 'vitest'
 import { makeHookTestRepo } from './hooks-test-fixtures'
 
-// Mock fs used by the runner-script writers
 vi.mock('fs', () => ({
   readFileSync: vi.fn(),
   existsSync: vi.fn(),
@@ -21,7 +20,6 @@ vi.mock('child_process', () => ({
   exec: vi.fn(),
   execFile: vi.fn(),
   execFileSync: vi.fn(),
-  // runner.ts imports spawn from child_process transitively.
   spawn: vi.fn()
 }))
 
@@ -131,8 +129,7 @@ describe('createSetupRunnerScript', () => {
   })
 
   it('keeps batch setup scripts on cmd.exe when the terminal is Git Bash', async () => {
-    // Regression (#6967): a Git Bash terminal preference used to hand pre-existing
-    // batch setup scripts to bash, where `copy`/`xcopy`/`if errorlevel` do not exist.
+    // Batch scripts must retain cmd semantics even when the pane uses Git Bash (#6967).
     gitExecFileSyncMock.mockReset()
     gitExecFileSyncMock.mockReturnValue('C:\\repo\\.git\\orca\\setup-runner.cmd\n')
     const fs = await import('node:fs')
@@ -162,8 +159,7 @@ describe('createSetupRunnerScript', () => {
       )
       expect(result).toMatchObject({
         runnerScriptPath: 'C:\\repo\\.git\\orca\\setup-runner.cmd',
-        // Why: `shell` is the pane that types the launch command — still Git Bash here. The
-        // runner's own .cmd extension is what says the file is batch.
+        // shell describes the pane; the .cmd extension selects the runner’s interpreter.
         shell: { family: 'posix' }
       })
     } finally {
@@ -172,8 +168,7 @@ describe('createSetupRunnerScript', () => {
   })
 
   it('hands a Git Bash pane a launch command MSYS cannot rewrite for a cmd runner', async () => {
-    // Regression (#6896): `cmd.exe /c "C:\...\setup-runner.cmd"` typed into Git Bash has its
-    // `/c` switch rewritten into a drive path, so cmd opens interactively and setup never runs.
+    // MSYS rewrites cmd.exe /c into a drive path, leaving setup stuck at an interactive prompt (#6896).
     gitExecFileSyncMock.mockReset()
     gitExecFileSyncMock.mockReturnValue('C:\\repo\\.git\\orca\\setup-runner.cmd\n')
     const originalPlatform = process.platform
@@ -193,7 +188,6 @@ describe('createSetupRunnerScript', () => {
       const command = buildSetupRunnerCommand(result.runnerScriptPath, 'windows', result.shell)
 
       expect(command).not.toContain('cmd.exe /c')
-      // Why: the batch runner must never be handed to bash either.
       expect(command).not.toContain('bash ')
       expect(command).toContain('powershell.exe')
     } finally {
@@ -202,8 +196,7 @@ describe('createSetupRunnerScript', () => {
   })
 
   it('replays interpreter flags declared on the shebang line', async () => {
-    // Regression: the runner is launched as `bash <path>`, so `-euo pipefail` on the script's
-    // own `#!` line never reaches the interpreter unless the runner re-applies it.
+    // bash <path> ignores the script’s shebang options unless the runner reapplies them.
     gitExecFileSyncMock.mockReset()
     gitExecFileSyncMock.mockReturnValue('C:\\repo\\.git\\orca\\setup-runner.sh\n')
     const fs = await import('node:fs')
@@ -463,8 +456,7 @@ describe('resolveSetupRunnerShell', () => {
     const { resolveSetupRunnerShell } = await import('./worktree-runner-script')
     const { resolveWindowsGitBashShellPath } = await import('./git-bash')
 
-    // Regression: a stale configured path used to commit setup to a .sh runner the
-    // PTY could never spawn, hanging wait-for-setup until the 2h timeout.
+    // Reject stale shells before setup waits on a runner the PTY cannot spawn.
     expect(
       resolveSetupRunnerShell(
         { terminalWindowsShell: 'C:\\Program Files\\Git\\bin\\bash.exe' },

src/main/hooks-test-fixtures.ts

@@ -9,7 +9,6 @@ export const TEST_WORKTREE_ORCA_YAML_PATH = join(TEST_WORKTREE_PATH, 'orca.yaml'
 export const TEST_ISSUE_COMMAND_PATH = join(TEST_REPO_PATH, '.orca', 'issue-command')
 export const TEST_GITIGNORE_PATH = join(TEST_REPO_PATH, '.gitignore')
 
-/** Minimal repo record the hooks module reads; hookSettings shape varies per suite. */
 export const makeHookTestRepo = (hookSettings?: unknown): Repo =>
   ({
     id: 'test-id',

src/main/hooks.test.ts

@@ -3,7 +3,6 @@ import type * as GitRunner from './git/runner'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 import { makeHookTestRepo } from './hooks-test-fixtures'
 
-// Mock fs used by loadHooks
 vi.mock('fs', () => ({
   readFileSync: vi.fn(),
   existsSync: vi.fn(),
@@ -20,8 +19,6 @@ const { spawnMock, runWslProcessMock, gitExecFileSyncMock } = vi.hoisted(() => (
 }))
 
 vi.mock('child_process', () => ({
-  // One `spawn` for both: hooks.ts runs the script through it, and runner.ts imports it
-  // transitively. A second key here silently shadowed the first.
   spawn: spawnMock,
   execFileSync: vi.fn()
 }))
@@ -35,7 +32,6 @@ vi.mock('./git/runner', async () => ({
   gitExecFileSync: gitExecFileSyncMock
 }))
 
-/** Minimal ChildProcess stand-in: hooks.ts reads the streams and waits for close/error. */
 function fakeChild(exit: { code?: number | null; signal?: NodeJS.Signals | null } = { code: 0 }) {
   const listeners: Record<string, ((...args: unknown[]) => void)[]> = {}
   const stream = { setEncoding: () => {}, on: () => {} }
@@ -59,7 +55,6 @@ function fakeChild(exit: { code?: number | null; signal?: NodeJS.Signals | null
 }
 
 beforeEach(() => {
-  // Clear as well as re-arm: these assertions are order-sensitive and calls otherwise accumulate.
   spawnMock.mockClear()
   spawnMock.mockImplementation(() => fakeChild())
 })
@@ -113,8 +108,7 @@ describe('runHook', () => {
   })
 
   it('does not run setup scripts with a half-activated conda env', async () => {
-    // Why: setup scripts source conda exactly like a shell rc does, so the
-    // orphaned CONDA_SHLVL sentinel surfaces as an opaque hook failure (#14195).
+    // An orphaned CONDA_SHLVL sentinel breaks hook environment activation (#14195).
     let capturedEnv: Record<string, string> | undefined
     spawnMock.mockImplementation((_script, options) => {
       capturedEnv = (options as { env: Record<string, string> }).env
@@ -180,8 +174,7 @@ describe('runHook', () => {
         expect.objectContaining({
           cwd: '/repo/worktree',
           shell: '/bin/bash',
-          // Setup hooks run unattended: git in them must not pop the OS
-          // credential helper's OAuth window and loop it (issue #7652).
+          // Unattended Git must not open credential-helper OAuth windows (#7652).
           env: expect.objectContaining({
             GIT_TERMINAL_PROMPT: '0',
             GCM_INTERACTIVE: 'never'
@@ -237,8 +230,7 @@ describe('runHook', () => {
           loginPath: 'preferred',
           script: 'echo hello',
           cwd: '/home/jin/feature',
-          // #7652 regression: the unattended WSL hook branch must carry the
-          // credential guard into the guest env.
+          // WSL hooks must receive the credential guard inside the guest (#7652).
           env: expect.objectContaining({
             ORCA_ROOT_PATH: '/mnt/c/Users/jinwo/git/orca',
             ORCA_WORKTREE_PATH: '/home/jin/feature',
@@ -369,8 +361,7 @@ describe('runHook', () => {
   })
 
   it('settles WSL hooks when wsl.exe never reports completion', async () => {
-    // Why no fake timers: the timeout is now runProcess's own, internal to the
-    // mocked runWslProcess -- there is nothing left in hooks.ts to advance.
+    // The mocked runWslProcess owns the timeout; hooks.ts has no timer to advance.
     spawnMock.mockReset()
     spawnMock.mockImplementation(() => fakeChild())
     runWslProcessMock.mockReset()

src/main/hooks.ts

@@ -21,18 +21,11 @@ import {
 } from '@orca/process-host/process-tree-termination'
 import { createOutputSink } from '@orca/process-host/bounded-output-sink'
 
-const HOOK_TIMEOUT = 120_000 // 2 minutes
+const HOOK_TIMEOUT = 120_000
 
 type HookProcessOutcome = { success: boolean; output: string; exitCode?: number }
 
-/**
- * Turn a finished process into a hook verdict.
- *
- * Why `timedOut` decides before `code` (#19334): a hook that traps SIGTERM and exits 0 reports a
- * zero exit for a run we cut off mid-archive. The exit code of something we stopped is not
- * evidence it finished, so a timeout withholds the code and the removal gate reads that as
- * `unverifiable` rather than as a pass.
- */
+/** Timed-out hooks cannot prove completion, even if their SIGTERM handler exits 0 (#19334). */
 function classifyHookProcessResult(
   result: { code: number | null; stdout: string; stderr: string; timedOut: boolean },
   context: { hookName: string; cwd: string; timeoutMs: number }
@@ -44,8 +37,7 @@ function classifyHookProcessResult(
     return { success: false, output: `${streams}\n${message}`.trim() }
   }
   if (result.code !== 0) {
-    // `null` means signalled: there is no exit code, and saying "exit code null" reads as a
-    // reporting glitch rather than the `unverifiable` verdict the gate is about to give it.
+    // A signaled exit has no code for the removal gate to verify.
     const message =
       result.code === null
         ? 'Command was terminated without reporting an exit code.'
@@ -63,12 +55,7 @@ function classifyHookProcessResult(
 
 const SIGTERM_GRACE_MS = 2_000
 
-/**
- * `exec` capped output at 1 MiB and killed the hook on overflow; `spawn` has no cap at all, and a
- * hook flooding stdout for the full deadline can take the main process's heap with it. Truncation
- * is reported in the output rather than as a failure — a chatty hook that exits 0 did succeed, and
- * failing it for being chatty is the `exec` behaviour this is replacing.
- */
+/** Bound retained output without failing chatty hooks that otherwise succeed. */
 const HOOK_OUTPUT_LIMIT_BYTES = 10 * 1024 * 1024
 
 function readSink(sink: ReturnType<typeof createOutputSink>): string {
@@ -77,7 +64,6 @@ function readSink(sink: ReturnType<typeof createOutputSink>): string {
     : sink.text()
 }
 
-/** A spawn failure: the process never started, so no exit was ever observed. */
 function hookProcessError(
   error: Error,
   stdout: string,
@@ -93,7 +79,6 @@ function hookProcessError(
   }
 }
 
-/** A hook that never started reported no exit, so the code stays withheld. */
 function hookSpawnFailure(
   error: unknown,
   context: { hookName: string; cwd: string }
@@ -113,9 +98,6 @@ function getHookShell(): string | undefined {
 
 export { parseOrcaYaml }
 
-/**
- * Load hooks from orca.yaml in the given repo root.
- */
 export function loadHooks(repoPath: string): OrcaHooks | null {
   const yamlPath = join(repoPath, 'orca.yaml')
   if (!existsSync(yamlPath)) {
@@ -130,14 +112,11 @@ export function loadHooks(repoPath: string): OrcaHooks | null {
   }
 }
 
-/**
- * Check whether an orca.yaml exists for a repo.
- */
 export function hasHooksFile(repoPath: string): boolean {
   return existsSync(join(repoPath, 'orca.yaml'))
 }
 
-// Why: detect unrecognised keys so the UI can suggest an update instead of showing a "could not be parsed" error.
+// Unknown keys suggest a newer config format; let the UI offer an update.
 const RECOGNIZED_ORCA_YAML_KEYS = new Set([
   'scripts',
   'setupAgentStartupPolicy',
@@ -147,12 +126,11 @@ const RECOGNIZED_ORCA_YAML_KEYS = new Set([
   'worktree'
 ])
 
-/** True when `orca.yaml` has a top-level key this version of Orca does not handle. */
 export function hasUnrecognizedOrcaYamlKeys(repoPath: string): boolean {
   try {
     const content = readFileSync(join(repoPath, 'orca.yaml'), 'utf-8')
     for (const line of iterateLfScriptLines(content)) {
-      // Why: match bare `key:` at end-of-line too, since a mapping with a block value on the next line is valid YAML.
+      // A bare key: is valid YAML when its block value follows on the next line.
       const m = line.match(/^([A-Za-z][A-Za-z0-9_-]*):(\s|$)/)
       if (m != null && !RECOGNIZED_ORCA_YAML_KEYS.has(m[1])) {
         return true
@@ -197,19 +175,14 @@ export function getSetupCommandSource(
   return null
 }
 
-/**
- * Run a named hook script in the given working directory.
- */
 export function runHook(
   hookName: 'setup' | 'archive',
   cwd: string,
   repo: Repo,
   hooksPath?: string,
   projectRuntime?: ProjectExecutionRuntimeResolution | HookRuntimeTarget,
-  /** Deadline override. Production uses HOOK_TIMEOUT; tests use it to exercise the timeout path. */
   timeoutMs: number = HOOK_TIMEOUT
-  // Why (#19334): an absent exitCode means no exit was ever observed. The archive-hook removal
-  // gate reads that as `unverifiable` rather than folding it into a zero.
+  // Failed hooks without an observed exit are unverifiable to the removal gate (#19334).
 ): Promise<{ success: boolean; output: string; exitCode?: number }> {
   const hooks = getEffectiveHooks(repo, hooksPath)
   const script = hooks?.scripts[hookName]
@@ -222,15 +195,13 @@ export function runHook(
   const wslInfo = getHookWslContext(cwd, runtimeTarget)
 
   if (wslInfo) {
-    // Why: hook scripts run inside WSL, so translate the ORCA_* Windows UNC paths to Linux paths.
+    // WSL hooks need Linux paths for ORCA_* values, including Windows UNC paths.
     const envVars = getSetupEnvVars(repo, cwd)
     const wslEnv: Record<string, string> = {}
     for (const [key, value] of Object.entries(envVars)) {
       wslEnv[key] = toLinuxPath(value)
     }
-    // Why: same unattended-git guard as the non-WSL branch below (issue
-    // #7652) — only the guard flags and any indexed git-config protocol are
-    // meant to reach the guest; askpass stays host-side, same as before.
+    // Forward unattended Git flags and indexed config to WSL; keep host askpass out of the guest.
     const guardedEnv = promptGuardShellEnv(wslEnv)
     const guestEnv: Record<string, string> = { ...wslEnv }
     for (const [key, value] of Object.entries(guardedEnv)) {
@@ -248,9 +219,7 @@ export function runHook(
       distro: wslInfo.distro ?? undefined,
       loginPath: 'preferred',
       script,
-      // Why pinned: these are user-authored orca.yaml scripts and the native
-      // path runs /bin/bash. Defaulting to sh would fail bash-only hooks on WSL
-      // only -- a downgrade the user never asked for.
+      // Match native Bash hook semantics; sh would reject Bash-only scripts inside WSL.
       shell: 'bash',
       cwd: wslInfo.linuxPath,
       env: guestEnv,
@@ -264,11 +233,7 @@ export function runHook(
   dropIncoherentCondaActivationEnv(shellHookEnv)
 
   return new Promise<HookProcessOutcome>((resolve) => {
-    // Why we own the deadline (#19334): Node's `exec({ timeout })` SIGTERMs the child and then
-    // reports whatever it chose to do, so a hook that traps SIGTERM and exits 0 came back as a
-    // PASS — a hook cut off mid-archive, indistinguishable from one that finished. Settle on the
-    // deadline instead, and settle AT it, so a hook that traps and keeps running cannot hold a
-    // removal open.
+    // Settle at the deadline even if the hook traps SIGTERM or keeps running (#19334).
     let settled = false
     let deadline: NodeJS.Timeout | undefined
     const settle = (result: HookProcessOutcome): void => {
@@ -281,32 +246,23 @@ export function runHook(
       }
       resolve(result)
     }
-    // Why `spawn` and not `exec` (#19334 follow-up): `detached` is a spawn-only option — `exec`
-    // accepts and ignores it, so the shell never became a group leader and the group signal below
-    // had nothing to reach. Passing `shell` as a string keeps Node's own platform invocation, which
-    // is what `exec` was being kept for: `cmd.exe /d /s /c` on Windows rather than a bare `-c`.
+    // exec ignores detached; spawn creates the process group needed for tree termination.
     const child = spawn(script, {
       cwd,
       shell: getHookShell(),
-      // Why: hooks run unattended; block Git Credential Manager's interactive prompt while keeping cached auth (issue #7652).
+      // Unattended hooks must suppress interactive Git prompts while retaining cached auth (#7652).
       env: promptGuardShellEnv(shellHookEnv),
       stdio: ['ignore', 'pipe', 'pipe'],
-      // Pinned, not left to Node's default, for the same reason `runProcess` pins it: a `cmd.exe`
-      // hook otherwise flashes a console window and takes focus. Pre-existing — `exec` did not set
-      // it either — but AGENTS.md asks for it pinned on every Windows spawn.
+      // Hide Windows hook consoles to prevent flashes and focus theft.
       windowsHide: true,
-      // Make the shell a group leader so its children can be reached. Not on Windows, which has no
-      // process groups in this sense and where `detached` means a new console instead.
+      // POSIX groups let signals reach descendants; Windows detached would create a new console.
       ...(process.platform === 'win32' ? {} : { detached: true })
     })
     const stdout = createOutputSink(HOOK_OUTPUT_LIMIT_BYTES)
     const stderr = createOutputSink(HOOK_OUTPUT_LIMIT_BYTES)
     child.stdout?.on('data', (chunk: Buffer | string) => stdout.write(chunk))
     child.stderr?.on('data', (chunk: Buffer | string) => stderr.write(chunk))
-    // Why listeners that do nothing: an unhandled `error` on a stream is an uncaught exception, and
-    // in the Electron main process that is the whole app. `exec` never covered this either — its
-    // only `error` listener is on the child — so this is a pre-existing gap, closed the way
-    // `runProcess` closes it. Losing output is not worth a crash; the exit code still gets through.
+    // Unhandled stream errors crash main; retain exit reporting even when output fails.
     for (const stream of [child.stdin, child.stdout, child.stderr]) {
       stream?.on('error', () => {})
     }
@@ -316,7 +272,6 @@ export function runHook(
     child.on('close', (code, signal) => {
       settle(
         classifyHookProcessResult(
-          // A signalled exit reports no code, which stays `unverifiable` rather than becoming a 0.
           {
             code: signal ? null : code,
             stdout: readSink(stdout),
@@ -327,21 +282,16 @@ export function runHook(
         )
       )
     })
-    // Why guarded: a spawn failure can settle before the deadline is armed, and arming one on a
-    // finished run would later signal a pid that is gone — and may by then belong to something else.
+    // Arming after settlement could later signal a reused PID.
     if (!settled) {
       deadline = setTimeout(() => {
         settle(
           classifyHookProcessResult(
-            // Keep what the hook printed: it is the only clue to why the removal gate says
-            // `unverifiable`.
             { code: null, stdout: readSink(stdout), stderr: readSink(stderr), timedOut: true },
             { hookName, cwd, timeoutMs }
           )
         )
-        // Orca's own tree terminator: POSIX process groups, `taskkill /t /f` on Windows (where a
-        // bare `child.kill` reaches only the shell and leaves its descendants running), and the
-        // recycled-pid guard that hazard needs. SIGTERM first so a well-behaved hook can clean up.
+        // Terminate descendants too, with a SIGTERM grace period for cooperative hooks to clean up.
         void signalProcessTree(child, 'SIGTERM')
         setTimeout(() => {
           void forceTerminateProcessTree(child)

src/main/host-tree-removal-asar.electron.test.ts

@@ -13,14 +13,7 @@ import { dirname, join } from 'node:path'
 import { afterAll, describe, expect, it } from 'vitest'
 import { removeTreeSync } from '../shared/windows-transient-lock-removal'
 
-/**
- * Why the real binary: Electron patches `fs` so a `*.asar` file reports `isDirectory() === true`, so
- * a recursive `rm` descends into the archive, `rmdir`s a real file, and fails the parent with
- * ENOTEMPTY. Plain Node has no such shim, so no in-process unit test can reproduce it — and every
- * worktree that has run `pnpm install` carries a `default_app.asar`, which is what stranded 267
- * trash entries on the reporting machine. This runs the shipped `removeHostTree` against a real
- * archive under the real binary.
- */
+/** Electron’s asar fs shim makes recursive removal fail; plain Node cannot reproduce it. */
 const requireFromTest = createRequire(import.meta.url)
 const electronBinary = requireFromTest('electron') as string
 const electronDist = join(dirname(requireFromTest.resolve('electron/package.json')), 'dist')
@@ -29,7 +22,6 @@ const FIXTURE_ASAR = [
   join(electronDist, 'resources/default_app.asar')
 ].find((candidate) => existsSync(candidate))
 
-// Mirrors the residue reported on the failing machine, down to the depth of the blocking leaf.
 const ENTRY_NAME = 'wt-1700000000000-abcdef01'
 const ASAR_PARENT = 'node_modules/.pnpm/electron/node_modules/electron/dist/App/Contents/Resources'
 
@@ -40,7 +32,7 @@ afterAll(() => {
     try {
       removeTreeSync(root)
     } catch {
-      // A fixture the shim strands is exactly what this file is about; never fail teardown on it.
+      // Failed cases can strand archives; cleanup must not obscure the assertion.
     }
   }
 })
@@ -51,8 +43,7 @@ function buildDriver(bundlePath: string, target: string, resultPath: string): st
   return [
     `const fs = require('node:fs')`,
     `const { removeHostTree } = require(${JSON.stringify(bundlePath)})`,
-    // Why noAsar for the read-back: the shim would report the stranded archive as a directory here
-    // too, so the residue listing has to be taken with real filesystem semantics.
+    // Inspect residue without the shim treating remaining archives as directories.
     `const withoutAsar = (fn) => { const prev = process.noAsar; process.noAsar = true; try { return fn() } finally { process.noAsar = prev } }`,
     `;(async () => {`,
     `  let failure = null`,
@@ -77,11 +68,7 @@ async function bundleHostTreeRemoval(outFile: string): Promise<void> {
       ssr: true,
       rollupOptions: {
         input: 'src/main/host-tree-removal.ts',
-        // Why mirror `isExternalMainModule` from electron.vite.config.ts exactly — CJS, and
-        // `original-fs` deliberately *not* externalized: the shipped bundle does not list it either,
-        // so if the archive-aware `rm` ever became a static import (or the bundler learned to fold
-        // `createRequire(...)('original-fs')`) production would silently degrade to the shimmed `fs`
-        // while a test that pre-externalized it kept passing.
+        // Match production externals: externalizing original-fs here could hide a bundling regression.
         output: { format: 'cjs' },
         external: (id: string) => isBuiltin(id) || id === 'electron' || id.startsWith('electron/')
       }
@@ -123,8 +110,6 @@ describe('removeHostTree against a tree holding an asar archive', () => {
       expect(run.status, run.stderr?.slice(-2000)).toBe(0)
 
       const probe = JSON.parse(readFileSync(resultPath, 'utf8')) as ProbeResult
-      // Without an asar-transparent `rm` this is `ENOTEMPTY` and the residue stops at the archive,
-      // on every attempt, forever — it is not a race a retry can win.
       expect(probe).toEqual({ failure: null, residue: [] })
     },
     120_000

src/main/host-tree-removal.ts

@@ -1,8 +1,4 @@
-// Why: every recursive host delete Orca performs (worktrees, terminal history, quarantined recovery
-// generations) hits the same two hazards, so one helper exists so no call site forgets either.
-// Windows stickiness — AV/indexers/late handle releases surface transient EBUSY/ENOTEMPTY/EPERM on a
-// tree Node just emptied — and Electron's asar shim, which strands any tree holding a `*.asar`
-// (see `asar-transparent-fs`).
+// Host deletion needs Windows lock retries and unpatched fs for Electron’s .asar handling.
 
 import { win32 } from 'node:path'
 import { setTimeout as delay } from 'node:timers/promises'
@@ -15,8 +11,7 @@ const WINDOWS_REMOVE_RETRY_DELAYS_MS = [250, 500, 1_000, 2_000]
 
 /** Convert a native host filesystem path to the Win32 long-path namespace. */
 export function toHostFilesystemPath(targetPath: string): string {
-  // POSIX paths are used by WSL callers even while the Electron process runs
-  // on Windows; do not reinterpret those as drive-relative Win32 paths.
+  // WSL callers supply POSIX paths even on Windows; do not interpret them as drive-relative paths.
   return process.platform === 'win32' &&
     isWindowsAbsolutePathLike(targetPath) &&
     !isWslUncPath(targetPath)
@@ -25,8 +20,7 @@ export function toHostFilesystemPath(targetPath: string): string {
 }
 
 export function toHostRemovalPath(targetPath: string): string {
-  // Why: Git for Windows can fail long recursive deletes even after Orca has
-  // proven the worktree target; Node's host deletion should use Win32 long paths.
+  // Node deletion uses Win32 long paths even after Git has verified the target.
   return toHostFilesystemPath(targetPath)
 }
 
@@ -46,8 +40,7 @@ function isTransientWindowsRemovalError(error: unknown): boolean {
 export async function removeHostTree(targetPath: string): Promise<void> {
   const removalPath = toHostRemovalPath(targetPath)
   const retryDelays = process.platform === 'win32' ? WINDOWS_REMOVE_RETRY_DELAYS_MS : []
-  // Why: large Windows trees commonly surface transient ENOTEMPTY/EPERM while Node walks and
-  // removes nested directories; Node's own retries absorb that before the loop below has to.
+  // Node’s retries absorb transient directory locks before the outer retry loop.
   const rmOptions = transientLockRemovalOptions()
   let attempt = 0
 
@@ -59,8 +52,7 @@ export async function removeHostTree(targetPath: string): Promise<void> {
       if (attempt >= retryDelays.length || !isTransientWindowsRemovalError(error)) {
         throw error
       }
-      // Why: Git/Node recursive deletes on Windows can observe a just-emptied
-      // directory before antivirus/indexers/handles release it.
+      // Antivirus, indexers, and late handle release can keep just-emptied Windows directories locked.
       await delay(retryDelays[attempt])
       attempt += 1
     }

src/main/image-file-signature.ts

@@ -3,7 +3,6 @@ export type ImageFileSignature = {
   extension: 'png' | 'jpg' | 'gif' | 'webp'
 }
 
-/** Leading bytes needed to recognize every signature below. */
 export const IMAGE_SIGNATURE_BYTES = 12
 
 const PNG_SIGNATURE = [0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]
@@ -24,7 +23,7 @@ function startsWithAscii(buffer: Uint8Array, text: string, offset = 0): boolean
   )
 }
 
-// Why: magic bytes, not a decoder — enough to name a file or reject a non-image.
+// Signature checks identify file types without proving the image decodes.
 export function detectImageFileSignature(buffer: Uint8Array): ImageFileSignature | null {
   if (startsWith(buffer, PNG_SIGNATURE)) {
     return { mimeType: 'image/png', extension: 'png' }

src/main/in-flight-run-dedupe.ts

@@ -1,12 +1,4 @@
-/**
- * Shares one in-flight promise per key so concurrent callers that want the same
- * result run the work once. Purely a concurrency collapse, never a cache: the
- * entry is dropped the moment the run settles, so the next caller starts fresh
- * and re-reads whatever state the run depends on.
- *
- * Callers own the map, which keeps each lane's exact result type (including
- * nullable ones) without a cast at every read.
- */
+/** Share concurrent work only; settled results are never cached. */
 export function dedupeInFlightRun<T>(
   runs: Map<string, Promise<T>>,
   key: string,
@@ -18,9 +10,7 @@ export function dedupeInFlightRun<T>(
   }
   const run = start()
   runs.set(key, run)
-  // Why: clear on rejection too, or one failure would wedge every later caller
-  // onto the same rejected promise. The identity check keeps a late settle from
-  // evicting a newer entry for the same key.
+  // Clear rejected runs too, without letting a late settlement evict a newer run.
   const clear = (): void => {
     if (runs.get(key) === run) {
       runs.delete(key)

src/main/index.ts

@@ -51,11 +51,11 @@ function requestDesktopActivation(argv: readonly string[] = []): void {
     state.mainWindow?.webContents.send('ui:openSkillShare', shareId)
   })
   state.osOpenedDocuments.capture(argv, publishOsOpenedDocuments)
-  // Why: a duplicate `orca serve` must not drag a headless server into opening a desktop window (#11935).
+  // Duplicate serve launches must not activate a desktop on a headless server (#11935).
   if (!shouldActivateDesktopForSecondInstance(argv)) {
     return
   }
-  // Why: the sidecar's throwaway profile must never become the user's window.
+  // The sidecar’s throwaway profile must never become the user’s window.
   if (isOrcadSidecarLaunch()) {
     requestDesktopHandoff()
     return
@@ -63,19 +63,13 @@ function requestDesktopActivation(argv: readonly string[] = []): void {
   state.desktopActivationGate?.requestActivation()
 }
 
-/**
- * Hands buffered OS-opened document paths to a renderer that has proven it is listening.
- *
- * Until that proof arrives the paths stay buffered, because `webContents.send` to a renderer
- * with no listener attached is dropped silently and the queue would be gone.
- */
+/** Buffer documents until listener readiness; webContents.send silently drops unhandled messages. */
 function publishOsOpenedDocuments(): void {
   const targetWindow = state.mainWindow
   if (!state.osDocumentOpenListenerReady || !targetWindow || targetWindow.isDestroyed()) {
     return
   }
-  // Why consumed before the await: a renderer pull racing this resolve must not take the same
-  // batch again. The restore() calls hand it back if delivery turns out to be impossible.
+  // Consume before awaiting to prevent duplicate renderer pulls; restore if delivery fails.
   const filePaths = state.osOpenedDocuments.consume()
   if (filePaths.length === 0) {
     return
@@ -106,7 +100,7 @@ const preflightReady = runMainProcessPreflight({
   requestDesktopActivation
 })
 
-// Why: when another process holds the lock we've already exited; skip file-writing side effects so this transient process never touches userData.
+// Skip userData writes after losing the single-instance lock.
 if (preflightReady) {
   void leaseAppImageGenerationForCurrentProcess()
   app.on('open-url', (event, url) => {
@@ -116,21 +110,19 @@ if (preflightReady) {
     event.preventDefault()
     requestDesktopActivation([url])
   })
-  // Why: macOS delivers "Open With" as open-file, often before `ready`, and only to a handler
-  // that claims the event. Unsupported paths stay unclaimed.
+  // macOS can deliver Open With before ready; claim only supported paths.
   app.on('open-file', (event, filePath) => {
     if (!state.osOpenedDocuments.captureFilePaths([filePath], publishOsOpenedDocuments)) {
       return
     }
     event.preventDefault()
-    // Why gated on isReady: pre-ready the cold-start window is already on its way, and
-    // activating the gate here would try to open one before Electron can. The sidecar has none.
+    // Cold-start activation must wait for Electron readiness; sidecars have no pending window.
     if (app.isReady() || isOrcadSidecarLaunch()) {
       requestDesktopActivation()
     }
   })
   state.skillShareDeepLinks.capture(process.argv)
-  // Why no publish: nothing is listening this early, so the first renderer pulls these on mount.
+  // The first renderer pulls on mount; nothing listens this early.
   state.osOpenedDocuments.capture(process.argv)
   registerMainProcessIpcHandlers()
   installMainProcessQuitHandlers()

src/main/integration-credential-file.ts

@@ -17,8 +17,7 @@ import {
   type IntegrationCredentialService
 } from '../shared/integration-credential-errors'
 
-// Why: connection status treats a token file as a saved credential; empty
-// files read as "missing", so counting them would split-brain getStatus.
+// Empty files mean no saved credential; keep status consistent with token reads.
 export function credentialFileHasContent(path: string): boolean {
   try {
     return statSync(path).size > 0
@@ -27,8 +26,7 @@ export function credentialFileHasContent(path: string): boolean {
   }
 }
 
-// Falls back to 0600 plaintext when the OS keyring is unavailable, matching the
-// Linear/Jira token writers so a keyring-less box still connects.
+// Owner-only plaintext permits connections when the OS keyring is unavailable.
 export function writeEncryptedCredential(
   service: IntegrationCredentialService,
   path: string,
@@ -44,29 +42,24 @@ export function writeEncryptedCredential(
   writeCredentialFileAtomic(path, Buffer.from(value, 'utf-8'))
 }
 
-// Why (STA-3941): a direct write can truncate the previous credential if it
-// fails or the app dies mid-write, leaving nothing to authenticate with. Write
-// a sibling temp file, fsync it, then rename — readers only ever observe the
-// old bytes or the complete new ones.
+// Stage and fsync before rename so failed writes cannot truncate the saved credential (STA-3941).
 export function writeCredentialFileAtomic(
   path: string,
   data: Buffer,
   shouldPublish?: () => boolean
 ): boolean {
-  // Why unique: a concurrent writer sharing one temp name could truncate the bytes we publish.
+  // Unique temp names prevent concurrent writers from truncating staged bytes.
   const tempPath = `${path}.${process.pid}.${randomBytes(4).toString('hex')}.tmp`
   let handle: number | null = null
   try {
     if (process.platform === 'win32') {
-      // Why born restricted: a sibling would otherwise inherit the directory's DACL and replace
-      // the previous credential's owner-only ACL.
+      // Create owner-only staging files so replacement cannot inherit a permissive Windows DACL.
       writeRestrictedWindowsFile(tempPath, data)
       fsyncFileSync(tempPath)
       return publishStagedCredential(tempPath, path, shouldPublish)
     }
     handle = openSync(tempPath, 'w', 0o600)
-    // Why: write(2) is allowed to return a short count, so a single writeSync
-    // could publish a truncated credential — the very corruption this avoids.
+    // writeSync can return a short count; publish only after all bytes are written.
     let written = 0
     while (written < data.length) {
       const bytes = writeSync(handle, data, written, data.length - written)
@@ -83,15 +76,11 @@ export function writeCredentialFileAtomic(
     if (handle !== null) {
       try {
         closeSync(handle)
-      } catch {
-        // Already closed or invalid; the unlink below is what matters.
-      }
+      } catch {}
     }
     try {
       unlinkSync(tempPath)
-    } catch {
-      // Nothing to clean up.
-    }
+    } catch {}
     throw error
   }
 }
@@ -110,9 +99,7 @@ function publishStagedCredential(
   return true
 }
 
-// Why: writeFileSync's `mode` only applies when it creates the file, so
-// rewriting an existing credential would silently keep looser permissions.
-// Best-effort: Windows has no POSIX modes; its staging file is created owner-only instead.
+// Creation mode does not tighten existing files; Windows hardens the staging ACL instead.
 export function restrictCredentialFileToOwner(path: string): void {
   try {
     chmodSync(path, 0o600)
@@ -128,9 +115,7 @@ export class CredentialDecryptionError extends Error {
   }
 }
 
-// Returns the stored token, null when the file is empty, and throws
-// CredentialDecryptionError when the file holds ciphertext we cannot decrypt
-// (e.g. the user denied the OS keychain prompt after an app re-sign).
+// Undecryptable ciphertext throws CredentialDecryptionError; an empty file returns null.
 export function readStoredCredentialToken(
   service: IntegrationCredentialService,
   raw: Buffer
@@ -155,8 +140,7 @@ function readPlaintextLegacyCredential(
   raw: Buffer
 ): string | null {
   const plaintext = decodeUtf8(raw)
-  // Why: legacy plaintext tokens are printable UTF-8; sealed ciphertext
-  // such as macOS v10 blobs must not be decoded into auth-header junk.
+  // Legacy plaintext is printable UTF-8; reject ciphertext masquerading as an auth-header value.
   if (plaintext === null || hasControlCharacter(plaintext)) {
     throw new CredentialDecryptionError(service)
   }

src/main/integration-state-file-cache.test.ts

@@ -90,7 +90,6 @@ describe('createIntegrationStateFileCache', () => {
   })
 
   it('reloads when a file appears after an absent first read', () => {
-    // The desktop app commonly starts before any integration is connected.
     const path = tempFile()
     const cache = createIntegrationStateFileCache({
       filePath: () => path,
@@ -133,7 +132,6 @@ describe('createIntegrationStateFileCache', () => {
   })
 
   it('recovers when another process writes between the stamp and the read', () => {
-    // A concurrent write must force another read instead of pinning stale content.
     const path = tempFile()
     writeFileSync(path, 'v1')
     let reads = 0
@@ -155,12 +153,11 @@ describe('createIntegrationStateFileCache', () => {
     cache.invalidate()
     writeDuringRead = true
 
-    // Reads 'theirs' but stamped against 'ours', so the next look must reload.
+    // Content read after a concurrent write still carries the earlier stamp, forcing one more read.
     expect(cache.get()).toEqual({ body: 'theirs' })
     expect(cache.get()).toEqual({ body: 'theirs' })
     expect(reads).toBe(3)
 
-    // Stamp and content now agree, so a further look is served from memory.
     expect(cache.get()).toEqual({ body: 'theirs' })
     expect(reads).toBe(3)
   })

src/main/integration-state-file-cache.ts

@@ -14,7 +14,6 @@ function readStamp(path: string): string | null {
   }
 }
 
-// Every Orca process on a host reads the same integration state files.
 export function createIntegrationStateFileCache<T>(args: {
   filePath: () => string
   readFromDisk: () => T

src/main/issue-command-file.ts

@@ -1,4 +1,4 @@
-// Why: `.orca/issue-command` is the per-user override; `orca.yaml` is the tracked project default.
+// .orca/issue-command is local; orca.yaml supplies the tracked project default.
 import { readFileSync, existsSync, mkdirSync, writeFileSync, rmSync } from 'node:fs'
 import { join } from 'node:path'
 import { loadHooks } from './hooks'
@@ -26,9 +26,6 @@ export type ResolvedIssueCommand = {
   source: 'local' | 'shared' | 'none'
 }
 
-/**
- * Resolve the GitHub issue command using local override first, then tracked repo config.
- */
 export function readIssueCommand(repoPath: string): ResolvedIssueCommand {
   const filePath = getIssueCommandFilePath(repoPath)
   let localContent: string | null = null
@@ -54,10 +51,7 @@ export function readIssueCommand(repoPath: string): ResolvedIssueCommand {
   }
 }
 
-/**
- * Write the per-user issue command override to `{repoRoot}/.orca/issue-command`.
- * Empty content deletes the override so the shared `orca.yaml` command applies again.
- */
+/** Empty content removes the local override so the shared orca.yaml command applies. */
 export async function writeIssueCommand(
   repoPath: string,
   content: string,
@@ -82,7 +76,6 @@ export async function writeIssueCommand(
     writeFileSync(filePath, `${trimmed}\n`, 'utf-8')
   } catch (err) {
     console.error('[hooks] Failed to write issue command:', err)
-    // Why: re-throw so the IPC handler surfaces the write failure to the renderer's .catch().
     throw err
   }
 }
@@ -107,7 +100,6 @@ async function isIssueCommandIgnoredByGit(
   }
 }
 
-/** Ensure `.orca` is in `.gitignore` so the per-user directory is never committed. */
 function ensureOrcaDirIgnored(repoPath: string): void {
   const gitignorePath = join(repoPath, '.gitignore')
   try {

src/main/jetbrains-windows-gui-launchers.ts

@@ -5,11 +5,7 @@ import {
   stripMatchingQuotes
 } from './editor-launcher-name'
 
-// Why: JetBrains installers ship `*64.exe` GUI-subsystem binaries. The
-// Toolbox/installer `.cmd`/`.bat` shims (and short-name console `idea.exe`
-// stubs) chain through helpers that allocate a visible Command Prompt even
-// when the parent spawn uses windowsHide — the STA-3040 prompt-window loop.
-// Maps each short console launcher name to its GUI sibling.
+// JetBrains console shims can spawn visible helpers despite windowsHide; prefer GUI siblings.
 const JETBRAINS_WINDOWS_CONSOLE_TO_GUI: Readonly<Record<string, string>> = {
   idea: 'idea64',
   webstorm: 'webstorm64',
@@ -26,7 +22,6 @@ const JETBRAINS_WINDOWS_CONSOLE_TO_GUI: Readonly<Record<string, string>> = {
 
 const JETBRAINS_WINDOWS_GUI_NAMES = new Set(Object.values(JETBRAINS_WINDOWS_CONSOLE_TO_GUI))
 
-/** GUI binary basename for a console or GUI launcher name, or null if unknown. */
 function getGuiExecutableName(launcherBaseName: string): string | null {
   if (JETBRAINS_WINDOWS_GUI_NAMES.has(launcherBaseName)) {
     return launcherBaseName
@@ -34,19 +29,7 @@ function getGuiExecutableName(launcherBaseName: string): string | null {
   return JETBRAINS_WINDOWS_CONSOLE_TO_GUI[launcherBaseName] ?? null
 }
 
-/**
- * The GUI `*64.exe` beside an already-resolved console entry point, or null.
- *
- * Why sibling-only: a bare PATH lookup for `idea64` can land in a different,
- * stale install, and it costs a second full PATH walk on Electron's main
- * thread. Toolbox script directories ship no exe, so those keep the shim and
- * rely on the detached-GUI spawn instead.
- *
- * Upgrades:
- * - `idea.cmd` / `idea.bat` / `idea64.cmd` → colocated `idea64.exe` when present
- * - short-name console stubs `idea.exe` → colocated `idea64.exe` when present
- * - leaves `idea64.exe` and non-JetBrains launchers unchanged
- */
+/** Search only beside the resolved launcher; PATH could select another install or block main again. */
 export function resolveColocatedJetBrainsGuiExecutable(
   resolvedCommand: string,
   fileExists: (path: string) => boolean
@@ -59,7 +42,6 @@ export function resolveColocatedJetBrainsGuiExecutable(
     return null
   }
 
-  // .exe: only short console names (idea.exe), never the GUI binary itself.
   if (isExe) {
     const guiName = JETBRAINS_WINDOWS_CONSOLE_TO_GUI[baseName]
     if (!guiName) {
@@ -69,7 +51,6 @@ export function resolveColocatedJetBrainsGuiExecutable(
     return fileExists(candidate) ? candidate : null
   }
 
-  // .cmd/.bat: idea.cmd and idea64.cmd both prefer idea64.exe when colocated.
   const guiName = getGuiExecutableName(baseName)
   if (!guiName) {
     return null
@@ -78,10 +59,7 @@ export function resolveColocatedJetBrainsGuiExecutable(
   return fileExists(candidate) ? candidate : null
 }
 
-// Why: only JetBrains batch shims chain through console helpers that outlive
-// the batch and re-open a prompt. Other GUI shims (code.cmd, cursor.cmd) exit
-// on their own, and running them under `start` would re-parse their quoted
-// WSL/SSH remote argv. detachedGui only rewrites batch targets.
+// Detach JetBrains shims with persistent console helpers; other GUI shims must retain quoted argv.
 export function isJetBrainsConsoleShim(command: string, platform: NodeJS.Platform): boolean {
   return (
     platform === 'win32' &&