fix(preload): collapse index.d.ts into type-checked api-types.ts (#1197)

Co-authored-by: Orca <help@stably.ai>

.github/workflows/pr.yml

@@ -35,6 +35,23 @@ jobs:
       - name: Lint
         run: pnpm exec oxlint --format github
 
+      # Why: project-owned type declarations must live in .ts so tsc
+      # actually checks them. TypeScript's skipLibCheck: true (inherited
+      # from @electron-toolkit/tsconfig) silently widens unresolved names
+      # in .d.ts to `any`, which is how #1186 shipped a broken IPC signature
+      # past typecheck. See docs/preload-typecheck-hole.md.
+      - name: Guard against project-owned .d.ts in preload/shared
+        run: |
+          matches=$(find src/preload src/shared -name '*.d.ts' 2>/dev/null || true)
+          if [ -n "$matches" ]; then
+            echo "::error::Project-owned .d.ts files are not allowed under src/preload or src/shared."
+            echo "Move type declarations into a .ts file so skipLibCheck does not hide errors."
+            echo "See docs/preload-typecheck-hole.md."
+            echo "Found:"
+            echo "$matches"
+            exit 1
+          fi
+
       - name: Typecheck
         run: pnpm typecheck
 

AGENTS.md

@@ -24,4 +24,8 @@ Orca targets macOS, Linux, and Windows. Keep all platform-dependent behavior beh
 
 ## GitHub CLI Usage
 
-Be mindful of the user's `gh` CLI API rate limit — batch requests where possible and avoid unnecessary calls. All code, commands, and scripts must be compatible with macOS, Linux, and Windows.
+Be mindful of the user's `gh` CLI API rate limit — batch requests where possible and avoid unnecessary calls. All code, commands, and scripts must be compatible with macOS, Linux, and Windows.
+
+## Type Declarations: Prefer `.ts` Over `.d.ts`
+
+Project-owned type declarations belong in `.ts` files. `.d.ts` is reserved for ambient shims (e.g., `env.d.ts`, `vite/client.d.ts`). TypeScript's `skipLibCheck: true` setting applies globally, including to our own `.d.ts` files, which means any unresolved type reference in a `.d.ts` silently becomes `any` at its call sites. Write your types in `.ts` files so the compiler actually checks them. CI enforces this for `src/preload/` and `src/shared/` — see `docs/preload-typecheck-hole.md`.

CONTRIBUTING.md

@@ -44,6 +44,12 @@ Add high-quality tests for behavior changes and bug fixes. Prefer tests that wou
 
 If your change affects UI or interaction behavior, verify it on the platforms it could impact.
 
+## Type Declarations: Prefer `.ts` Over `.d.ts`
+
+Project-owned type declarations belong in `.ts` files. `.d.ts` is reserved for ambient shims (e.g., `env.d.ts`, `vite/client.d.ts`). TypeScript's `skipLibCheck: true` setting applies globally, including to our own `.d.ts` files, which means any unresolved type reference in a `.d.ts` silently becomes `any` at its call sites. Write your types in `.ts` files so the compiler actually checks them.
+
+CI enforces this for `src/preload/` and `src/shared/` — see `docs/preload-typecheck-hole.md`.
+
 ## Pull Requests
 
 Each pull request should:

config/tsconfig.tc.web.json

@@ -4,7 +4,7 @@
     "../src/renderer/src/env.d.ts",
     "../src/renderer/src/**/*",
     "../src/renderer/src/**/*.tsx",
-    "../src/preload/*.d.ts",
+    "../src/preload/api-types.ts",
     "../src/shared/**/*",
     "../src/main/ipc/worktree-logic.ts",
     "../src/main/wsl.ts"

config/tsconfig.web.json

@@ -4,7 +4,7 @@
     "../src/renderer/src/env.d.ts",
     "../src/renderer/src/**/*",
     "../src/renderer/src/**/*.tsx",
-    "../src/preload/*.d.ts",
+    "../src/preload/api-types.ts",
     "../src/shared/**/*"
   ],
   "compilerOptions": {

docs/preload-typecheck-hole.md

@@ -0,0 +1,101 @@
+# Preload typecheck hole: why project-owned types live in `.ts`
+
+## The rule
+
+Project-owned type declarations under `src/preload/` and `src/shared/`
+**must live in `.ts` files, not `.d.ts`**. The CI step
+"Guard against project-owned .d.ts in preload/shared" in
+`.github/workflows/pr.yml` enforces this.
+
+## Why
+
+Orca inherits `skipLibCheck: true` from `@electron-toolkit/tsconfig`.
+That setting is the ecosystem default — it exists so a broken `.d.ts`
+in some random `node_modules` package can't block your build. TypeScript
+has no way to scope it to `node_modules`, so **`skipLibCheck`
+applies to our own `.d.ts` files too**.
+
+In a project-owned `.d.ts`, any type reference that fails to resolve
+silently becomes `any` at its call sites instead of erroring. Downstream
+assignments against that `any` are also silently accepted. The error
+never surfaces during `pnpm typecheck`.
+
+For example, with `skipLibCheck: true`:
+
+```ts
+// in src/preload/index.d.ts — Worktree is never imported
+type WorktreesApi = {
+  list: () => Promise<Worktree[]>  // silently becomes Promise<any[]>
+}
+```
+
+and at the call site:
+
+```ts
+// in any renderer file
+window.api.worktrees.list().then((arr) => {
+  setWorktreeName(arr)  // setWorktreeName expects string; accepted anyway because arr is any[]
+})
+```
+
+No compile error. Crashes at runtime.
+
+The standard TS convention that sidesteps this: put project-owned types in
+`.ts` (which are always checked), reserve `.d.ts` for ambient shims
+(`env.d.ts`, `vite/client.d.ts`, etc.). The CI guard encodes that
+convention mechanically.
+
+## Incident that forced the fix
+
+PR #1186 changed the `repos:getBaseRefDefault` IPC return shape from
+`Promise<string | null>` to `Promise<BaseRefDefaultResult>` (an envelope
+object). Two of three renderer callers were updated; the third
+(`StartFromField.tsx`) wasn't. That caller passed the envelope object
+into a `setState<string | null>` setter, which rendered as JSX and threw
+React error #31 (`Objects are not valid as a React child`).
+
+**The call site should have been a compile error.** It wasn't, because
+`src/preload/index.d.ts` (now deleted) was a 246-line project-owned
+`.d.ts` that referenced ~20 type names it never imported (`Worktree`,
+`PRInfo`, `GlobalSettings`, `BaseRefDefaultResult`, and more). Under
+`skipLibCheck`, each unresolved name became `any`, which widened the
+`.then((ref) => …)` callback parameter to `any` at the consuming call
+site. `setDefaultBaseRef(ref: any)` compiled cleanly.
+
+The crash is fixed by #1189. The typecheck hole is fixed by this PR
+(#1197), which collapses the two preload type files (`index.d.ts` +
+`api-types.d.ts`) into a single type-checked `api-types.ts`. Full design
+discussion, alternatives considered, and rollout notes live in PR #1197.
+
+## Non-obvious subtleties worth remembering
+
+- **It's not the hand-authored types that failed — it was the missing
+  imports.** The types in the old `index.d.ts` were individually fine;
+  the file only went wrong because names like `Worktree` and `PRInfo`
+  weren't imported and `skipLibCheck` swallowed the error. A future
+  contributor copy-pasting types out of a `.d.ts` into `.ts` may be
+  surprised by a wall of "Cannot find name 'X'" errors — that's the
+  flag catching its target, not a real regression.
+- **`.d.ts` is still legitimate for ambient shims.** `env.d.ts`,
+  `mermaid.d.ts`, `hosted-git-info.d.ts` all live *outside* the CI
+  guard's scan roots (`src/preload/` and `src/shared/`) and stay as
+  `.d.ts`. If a future file under those roots genuinely needs to be
+  `.d.ts` (e.g., an ambient module shim for a third-party package that
+  can't live in `.ts`), add it to an allowlist in `pr.yml` at that
+  time — don't relax the guard wholesale.
+- **Intersection types on `window.api` are what actually widened the
+  `.then` callback to `any`.** The old layout used
+  `type Api = PreloadApi & { repos: ReposApi, worktrees: WorktreesApi, … }`.
+  TypeScript's intersection-of-function-types resolution widens callback
+  parameters to `any` when one side of the intersection has unresolved
+  names, *even though static-inspection views
+  (`ReturnType<typeof fn>`) still report the correct type*. So
+  `ReturnType<typeof window.api.repos.getBaseRefDefault>` printed
+  `Promise<BaseRefDefaultResult>` during debugging while the live
+  `.then((ref) => …)` callback treated `ref` as `any`. Don't re-introduce
+  intersection typing on the preload surface for any reason.
+- **Don't try to fix this by flipping `skipLibCheck: false` globally.**
+  It would force every transitive `@types/*` package to type-check
+  cleanly, which is why the ecosystem-wide default is `true`. The
+  structural fix (project-owned types in `.ts`) removes our last
+  reason to care about the flag for our code.

src/preload/api-types.ts

@@ -2,7 +2,6 @@
 import type {
   BaseRefDefaultResult,
   BrowserCookieImportResult,
-  BrowserCookieImportSummary,
   BrowserLoadError,
   BrowserSessionProfile,
   BrowserSessionProfileScope,
@@ -51,7 +50,7 @@ import type {
   WorktreeMeta,
   WorktreeSetupLaunch,
   WorkspaceSessionState
-} from '../../shared/types'
+} from '../shared/types'
 import type {
   BrowserSetGrabModeArgs,
   BrowserSetGrabModeResult,
@@ -62,7 +61,7 @@ import type {
   BrowserCaptureSelectionScreenshotResult,
   BrowserExtractHoverArgs,
   BrowserExtractHoverResult
-} from '../../shared/browser-grab-types'
+} from '../shared/browser-grab-types'
 import type {
   BrowserContextMenuDismissedEvent,
   BrowserContextMenuRequestedEvent,
@@ -71,11 +70,13 @@ import type {
   BrowserDownloadRequestedEvent,
   BrowserPermissionDeniedEvent,
   BrowserPopupEvent
-} from '../../shared/browser-guest-events'
-import type { CliInstallStatus } from '../../shared/cli-install-types'
-import type { E2EConfig } from '../../shared/e2e-config'
-import type { AgentHookInstallStatus } from '../../shared/agent-hook-types'
-import type { RuntimeStatus, RuntimeSyncWindowGraph } from '../../shared/runtime-types'
+} from '../shared/browser-guest-events'
+import type { ElectronAPI } from '@electron-toolkit/preload'
+import type { CliInstallStatus } from '../shared/cli-install-types'
+import type { E2EConfig } from '../shared/e2e-config'
+import type { AgentHookInstallStatus } from '../shared/agent-hook-types'
+import type { AgentStatusState } from '../shared/agent-status-types'
+import type { RuntimeStatus, RuntimeSyncWindowGraph } from '../shared/runtime-types'
 import type {
   ClaudeUsageBreakdownKind,
   ClaudeUsageBreakdownRow,
@@ -85,14 +86,14 @@ import type {
   ClaudeUsageScope,
   ClaudeUsageSessionRow,
   ClaudeUsageSummary
-} from '../../shared/claude-usage-types'
-import type { RateLimitState } from '../../shared/rate-limit-types'
+} from '../shared/claude-usage-types'
+import type { RateLimitState } from '../shared/rate-limit-types'
 import type {
   SshConnectionState,
   SshTarget,
   PortForwardEntry,
   DetectedPort
-} from '../../shared/ssh-types'
+} from '../shared/ssh-types'
 import type {
   CodexUsageBreakdownKind,
   CodexUsageBreakdownRow,
@@ -102,7 +103,7 @@ import type {
   CodexUsageScope,
   CodexUsageSessionRow,
   CodexUsageSummary
-} from '../../shared/codex-usage-types'
+} from '../shared/codex-usage-types'
 
 export type BrowserApi = {
   registerGuest: (args: {
@@ -352,6 +353,10 @@ export type PreloadApi = {
       connectionId?: string | null
       worktreeId?: string
       sessionId?: string
+      // Why: lets a single tab open in a different shell than the user's default.
+      // Preserved from the deleted index.d.ts PtyApi duplicate during the
+      // single-source-of-truth collapse (see docs/preload-typecheck-hole.md §1).
+      shellOverride?: string
     }) => Promise<{
       id: string
       snapshot?: string
@@ -524,6 +529,7 @@ export type PreloadApi = {
     claudeStatus: () => Promise<AgentHookInstallStatus>
     codexStatus: () => Promise<AgentHookInstallStatus>
     geminiStatus: () => Promise<AgentHookInstallStatus>
+    cursorStatus: () => Promise<AgentHookInstallStatus>
   }
   preflight: PreflightApi
   notifications: {
@@ -860,4 +866,32 @@ export type PreloadApi = {
     onCredentialResolved: (callback: (data: { requestId: string }) => void) => () => void
     submitCredential: (args: { requestId: string; value: string | null }) => Promise<void>
   }
+  wsl: {
+    isAvailable: () => Promise<boolean>
+  }
+  agentStatus: {
+    /** Listen for agent status updates forwarded from native hook receivers. */
+    onSet: (
+      callback: (data: {
+        paneKey: string
+        tabId?: string
+        worktreeId?: string
+        state: AgentStatusState
+        prompt?: string
+        agentType?: string
+        toolName?: string
+        toolInput?: string
+        lastAssistantMessage?: string
+        interrupted?: boolean
+      }) => void
+    ) => () => void
+  }
+}
+
+declare global {
+  // oxlint-disable-next-line typescript-eslint/consistent-type-definitions -- declaration merging requires interface
+  interface Window {
+    electron: ElectronAPI
+    api: PreloadApi
+  }
 }

src/preload/index.d.ts

@@ -1,246 +0,0 @@
-import type { ElectronAPI } from '@electron-toolkit/preload'
-import type {
-  BaseRefDefaultResult,
-  CreateWorktreeResult,
-  GhosttyImportPreview,
-  GitHubPRFile,
-  GitHubPRFileContents,
-  GitHubWorkItem,
-  GitHubWorkItemDetails,
-  GitHubViewer,
-  CreateWorktreeArgs
-} from '../../shared/types'
-import type { SshTarget, SshConnectionState } from '../../shared/ssh-types'
-import type { AgentStatusState } from '../../shared/agent-status-types'
-import type { PreloadApi } from './api-types'
-
-type ReposApi = {
-  list: () => Promise<Repo[]>
-  add: (args: {
-    path: string
-    kind?: 'git' | 'folder'
-  }) => Promise<{ repo: Repo } | { error: string }>
-  addRemote: (args: {
-    connectionId: string
-    remotePath: string
-    displayName?: string
-    kind?: 'git' | 'folder'
-  }) => Promise<{ repo: Repo } | { error: string }>
-  remove: (args: { repoId: string }) => Promise<void>
-  update: (args: {
-    repoId: string
-    updates: Partial<
-      Pick<Repo, 'displayName' | 'badgeColor' | 'hookSettings' | 'worktreeBaseRef' | 'kind'>
-    >
-  }) => Promise<Repo>
-  pickFolder: () => Promise<string | null>
-  pickDirectory: () => Promise<string | null>
-  clone: (args: { url: string; destination: string }) => Promise<Repo>
-  cloneAbort: () => Promise<void>
-  onCloneProgress: (callback: (data: { phase: string; percent: number }) => void) => () => void
-  getGitUsername: (args: { repoId: string }) => Promise<string>
-  getBaseRefDefault: (args: { repoId: string }) => Promise<BaseRefDefaultResult>
-  searchBaseRefs: (args: { repoId: string; query: string; limit?: number }) => Promise<string[]>
-  onChanged: (callback: () => void) => () => void
-}
-
-type WorktreesApi = {
-  list: (args: { repoId: string }) => Promise<Worktree[]>
-  listAll: () => Promise<Worktree[]>
-  create: (args: CreateWorktreeArgs) => Promise<CreateWorktreeResult>
-  remove: (args: { worktreeId: string; force?: boolean }) => Promise<void>
-  updateMeta: (args: { worktreeId: string; updates: Partial<WorktreeMeta> }) => Promise<Worktree>
-  persistSortOrder: (args: { orderedIds: string[] }) => Promise<void>
-  onChanged: (callback: (data: { repoId: string }) => void) => () => void
-}
-
-type WslApi = {
-  isAvailable: () => Promise<boolean>
-}
-
-type PtyApi = {
-  spawn: (opts: {
-    cols: number
-    rows: number
-    cwd?: string
-    env?: Record<string, string>
-    command?: string
-    connectionId?: string | null
-    worktreeId?: string
-    sessionId?: string
-    shellOverride?: string
-  }) => Promise<{
-    id: string
-    snapshot?: string
-    snapshotCols?: number
-    snapshotRows?: number
-    isReattach?: boolean
-    isAlternateScreen?: boolean
-    replay?: string
-    sessionExpired?: boolean
-    coldRestore?: { scrollback: string; cwd: string }
-  }>
-  write: (id: string, data: string) => void
-  resize: (id: string, cols: number, rows: number) => void
-  signal: (id: string, signal: string) => void
-  kill: (id: string) => Promise<void>
-  ackColdRestore: (id: string) => void
-  hasChildProcesses: (id: string) => Promise<boolean>
-  getForegroundProcess: (id: string) => Promise<string | null>
-  listSessions: () => Promise<{ id: string; cwd: string; title: string }[]>
-  onData: (callback: (data: { id: string; data: string }) => void) => () => void
-  onExit: (callback: (data: { id: string; code: number }) => void) => () => void
-}
-
-type GhApi = {
-  viewer: () => Promise<GitHubViewer | null>
-  repoSlug: (args: { repoPath: string }) => Promise<{ owner: string; repo: string } | null>
-  prForBranch: (args: { repoPath: string; branch: string }) => Promise<PRInfo | null>
-  issue: (args: { repoPath: string; number: number }) => Promise<IssueInfo | null>
-  // Why: main-process mappers don't know the Orca Repo.id, so IPC returns
-  // items without `repoId`. The renderer stamps repoId based on the requesting
-  // repo before exposing items to UI code.
-  workItem: (args: {
-    repoPath: string
-    number: number
-  }) => Promise<Omit<GitHubWorkItem, 'repoId'> | null>
-  workItemDetails: (args: {
-    repoPath: string
-    number: number
-  }) => Promise<GitHubWorkItemDetails | null>
-  prFileContents: (args: {
-    repoPath: string
-    prNumber: number
-    path: string
-    oldPath?: string
-    status: GitHubPRFile['status']
-    headSha: string
-    baseSha: string
-  }) => Promise<GitHubPRFileContents>
-  listIssues: (args: { repoPath: string; limit?: number }) => Promise<IssueInfo[]>
-  listWorkItems: (args: {
-    repoPath: string
-    limit?: number
-    query?: string
-  }) => Promise<Omit<GitHubWorkItem, 'repoId'>[]>
-  prChecks: (args: {
-    repoPath: string
-    prNumber: number
-    headSha?: string
-    noCache?: boolean
-  }) => Promise<PRCheckDetail[]>
-  prComments: (args: {
-    repoPath: string
-    prNumber: number
-    noCache?: boolean
-  }) => Promise<PRComment[]>
-  resolveReviewThread: (args: {
-    repoPath: string
-    threadId: string
-    resolve: boolean
-  }) => Promise<boolean>
-  updatePRTitle: (args: { repoPath: string; prNumber: number; title: string }) => Promise<boolean>
-  mergePR: (args: {
-    repoPath: string
-    prNumber: number
-    method?: 'merge' | 'squash' | 'rebase'
-  }) => Promise<{ ok: true } | { ok: false; error: string }>
-  checkOrcaStarred: () => Promise<boolean | null>
-  starOrca: () => Promise<boolean>
-}
-
-type SettingsApi = {
-  get: () => Promise<GlobalSettings>
-  set: (args: Partial<GlobalSettings>) => Promise<GlobalSettings>
-  listFonts: () => Promise<string[]>
-  previewGhosttyImport: () => Promise<GhosttyImportPreview>
-}
-
-type CliApi = {
-  getInstallStatus: () => Promise<CliInstallStatus>
-  install: () => Promise<CliInstallStatus>
-  remove: () => Promise<CliInstallStatus>
-}
-
-type NotificationsApi = {
-  dispatch: (args: NotificationDispatchRequest) => Promise<NotificationDispatchResult>
-  openSystemSettings: () => Promise<void>
-}
-
-type ShellApi = {
-  openPath: (path: string) => Promise<void>
-  openUrl: (url: string) => Promise<void>
-  openFilePath: (path: string) => Promise<void>
-  openFileUri: (uri: string) => Promise<void>
-  pathExists: (path: string) => Promise<boolean>
-  pickAttachment: () => Promise<string | null>
-  pickImage: () => Promise<string | null>
-  pickDirectory: (args: { defaultPath?: string }) => Promise<string | null>
-  copyFile: (args: { srcPath: string; destPath: string }) => Promise<void>
-}
-
-type SshApi = {
-  listTargets: () => Promise<SshTarget[]>
-  addTarget: (args: { target: Omit<SshTarget, 'id'> }) => Promise<SshTarget>
-  updateTarget: (args: {
-    id: string
-    updates: Partial<Omit<SshTarget, 'id'>>
-  }) => Promise<SshTarget>
-  removeTarget: (args: { id: string }) => Promise<void>
-  importConfig: () => Promise<SshTarget[]>
-  connect: (args: { targetId: string }) => Promise<SshConnectionState>
-  disconnect: (args: { targetId: string }) => Promise<void>
-  getState: (args: { targetId: string }) => Promise<SshConnectionState | null>
-  testConnection: (args: { targetId: string }) => Promise<{ success: boolean; error?: string }>
-  onStateChanged: (
-    callback: (data: { targetId: string; state: SshConnectionState }) => void
-  ) => () => void
-  browseDir: (args: { targetId: string; dirPath: string }) => Promise<{
-    entries: { name: string; isDirectory: boolean }[]
-    resolvedPath: string
-  }>
-}
-
-type AgentStatusApi = {
-  /** Listen for agent status updates forwarded from native hook receivers. */
-  onSet: (
-    callback: (data: {
-      paneKey: string
-      tabId?: string
-      worktreeId?: string
-      state: AgentStatusState
-      prompt?: string
-      agentType?: string
-      toolName?: string
-      toolInput?: string
-      lastAssistantMessage?: string
-      interrupted?: boolean
-    }) => void
-  ) => () => void
-}
-
-// Why: Only locally-defined *Api types are listed here. Keys like preflight,
-// hooks, cache, session, updater, fs, git, ui, and runtime are inherited via
-// the PreloadApi intersection (see ./api-types), so re-declaring them would
-// reference undefined type names and risk drifting from the canonical surface.
-type Api = PreloadApi & {
-  repos: ReposApi
-  worktrees: WorktreesApi
-  pty: PtyApi
-  ssh: SshApi
-  gh: GhApi
-  settings: SettingsApi
-  cli: CliApi
-  notifications: NotificationsApi
-  shell: ShellApi
-  agentStatus: AgentStatusApi
-  wsl: WslApi
-}
-
-declare global {
-  // oxlint-disable-next-line typescript-eslint/consistent-type-definitions -- declaration merging requires interface
-  interface Window {
-    electron: ElectronAPI
-    api: Api
-  }
-}