WIP: Changes before auto-review fixes

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

00-review-context.md

@@ -0,0 +1,70 @@
+# Review Context
+
+## Branch Info
+
+- Base: origin/main
+- Current: brennanb2025/cli-created-terminals-not-showing-agent-status
+
+## Changed Files Summary
+
+- M src/main/runtime/orca-runtime.test.ts
+- M src/main/runtime/orca-runtime.ts
+- M src/main/window/attach-main-window-services.ts
+- M src/preload/api-types.ts
+- M src/preload/index.ts
+- M src/renderer/src/hooks/useIpcEvents.ts
+- M src/renderer/src/store/slices/store-cascades.test.ts
+- M src/renderer/src/store/slices/terminals.ts
+- M src/shared/telemetry-events.ts
+
+## Changed Line Ranges (PR Scope)
+
+<!-- In scope: issues on these lines OR caused by these changes. Out of scope: unrelated pre-existing issues -->
+
+| File                                                | Changed Lines                                               |
+| --------------------------------------------------- | ----------------------------------------------------------- |
+| src/main/runtime/orca-runtime.test.ts               | 452-461, 465-466, 526-527, 617-621, 1700-1706, 1716-1717, 1777-1778 |
+| src/main/runtime/orca-runtime.ts                    | 313, 4413-4427, 4433, 4450-4456                             |
+| src/main/window/attach-main-window-services.ts      | 266-270                                                     |
+| src/preload/api-types.ts                            | 1099                                                        |
+| src/preload/index.ts                                | 1860, 1872                                                  |
+| src/renderer/src/hooks/useIpcEvents.ts              | 34, 230, 251-256, 995-1005                                  |
+| src/renderer/src/store/slices/store-cascades.test.ts | 1616-1665                                                   |
+| src/renderer/src/store/slices/terminals.ts          | 140-149, 332-358                                            |
+| src/shared/telemetry-events.ts                      | 282-293, 544                                                |
+
+## Review Standards Reference
+
+- Follow /review-code standards
+- Focus on: correctness, security, performance, maintainability
+- Priority levels: Critical > High > Medium > Low
+
+## File Categories
+
+### Electron/Main
+- src/main/runtime/orca-runtime.test.ts
+- src/main/runtime/orca-runtime.ts
+- src/main/window/attach-main-window-services.ts
+- src/preload/api-types.ts
+- src/preload/index.ts
+
+### Frontend/UI
+- src/renderer/src/hooks/useIpcEvents.ts
+- src/renderer/src/store/slices/store-cascades.test.ts
+- src/renderer/src/store/slices/terminals.ts
+
+### Utility/Common
+- src/shared/telemetry-events.ts
+
+## Skipped Issues (Do Not Re-validate)
+
+<!-- Issues validated but deemed not worth fixing. Do not re-validate these in future iterations. -->
+<!-- Format: [file:line-range] | [severity] | [reason skipped] | [issue summary] -->
+
+(none)
+
+## Iteration State
+
+Current iteration: 1
+Last completed phase: Setup
+Files fixed this iteration: []

src/main/runtime/orca-runtime.test.ts

@@ -449,10 +449,21 @@ describe('OrcaRuntimeService', () => {
     })
     expect(result.handle).toMatch(/^term_/)
     expect(createTerminal).not.toHaveBeenCalled()
+    // Why: hook-based agent status keys off `${tabId}:${paneId}`, so main must
+    // pre-allocate the tabId and stamp ORCA_PANE_KEY/TAB_ID/WORKTREE_ID into
+    // the PTY env before spawn. The same tabId is then handed to the renderer
+    // via `revealTerminalSession` so adoption preserves attribution. See
+    // docs/cli-terminal-hook-pane-key.md.
+    const spawnCall = spawn.mock.calls[0]?.[0] as { env?: Record<string, string> } | undefined
+    const spawnedEnv = spawnCall?.env ?? {}
+    expect(spawnedEnv.ORCA_TAB_ID).toMatch(/^[0-9a-f-]+$/)
+    expect(spawnedEnv.ORCA_PANE_KEY).toBe(`${spawnedEnv.ORCA_TAB_ID}:1`)
+    expect(spawnedEnv.ORCA_WORKTREE_ID).toBe(TEST_WORKTREE_ID)
     expect(revealTerminalSession).toHaveBeenCalledWith(TEST_WORKTREE_ID, {
       ptyId: 'pty-bg',
       title: 'worker',
-      activate: false
+      activate: false,
+      tabId: spawnedEnv.ORCA_TAB_ID
     })
   })
 
@@ -512,7 +523,8 @@ describe('OrcaRuntimeService', () => {
       expect(revealTerminalSession).toHaveBeenCalledWith(TEST_WORKTREE_ID, {
         ptyId: 'pty-bg',
         title: null,
-        activate: false
+        activate: false,
+        tabId: expect.stringMatching(/^[0-9a-f-]+$/)
       })
       expect(warn).toHaveBeenCalledWith(
         expect.stringContaining('[terminal-create] failed to create inactive tab for pty-bg:'),
@@ -602,7 +614,11 @@ describe('OrcaRuntimeService', () => {
       tabId: 'tab-adopted',
       worktreeId: TEST_WORKTREE_ID
     })
-    expect(revealTerminalSession).toHaveBeenCalledWith(TEST_WORKTREE_ID, {
+    // Why: createTerminal first reveal stamps activate/tabId; the focus reveal
+    // is the second call and uses the simpler {ptyId, title} shape. Assert the
+    // most recent call (the focus one) so the createTerminal-side reveal does
+    // not shadow this expectation.
+    expect(revealTerminalSession).toHaveBeenLastCalledWith(TEST_WORKTREE_ID, {
       ptyId: 'pty-bg',
       title: 'worker'
     })
@@ -1681,17 +1697,24 @@ describe('OrcaRuntimeService', () => {
       expect.objectContaining({
         cwd: '/tmp/workspaces/runtime-hook-skip',
         command: 'bash /tmp/repo/.git/orca/setup-runner.sh',
-        env: {
+        // Why: createTerminal stamps ORCA_PANE_KEY/TAB_ID/WORKTREE_ID into the
+        // PTY env on top of the caller-supplied env so hook-based agent status
+        // can attribute hook events to a pane. See docs/cli-terminal-hook-pane-key.md.
+        env: expect.objectContaining({
           ORCA_ROOT_PATH: '/tmp/repo',
-          ORCA_WORKTREE_PATH: '/tmp/workspaces/runtime-hook-skip'
-        },
+          ORCA_WORKTREE_PATH: '/tmp/workspaces/runtime-hook-skip',
+          ORCA_TAB_ID: expect.stringMatching(/^[0-9a-f-]+$/),
+          ORCA_PANE_KEY: expect.stringMatching(/^[0-9a-f-]+:1$/),
+          ORCA_WORKTREE_ID: result.worktree.id
+        }),
         worktreeId: result.worktree.id
       })
     )
     expect(revealTerminalSession).toHaveBeenLastCalledWith(result.worktree.id, {
       ptyId: 'pty-setup',
       title: 'Setup',
-      activate: false
+      activate: false,
+      tabId: expect.stringMatching(/^[0-9a-f-]+$/)
     })
   })
 
@@ -1751,7 +1774,8 @@ describe('OrcaRuntimeService', () => {
     expect(revealTerminalSession).toHaveBeenCalledWith(result.worktree.id, {
       ptyId: 'pty-created-worktree',
       title: null,
-      activate: false
+      activate: false,
+      tabId: expect.stringMatching(/^[0-9a-f-]+$/)
     })
   })
 

src/main/runtime/orca-runtime.ts

@@ -310,7 +310,7 @@ type RuntimeNotifier = {
   createTerminal(worktreeId: string, opts: { command?: string; title?: string }): void
   revealTerminalSession?(
     worktreeId: string,
-    opts: { ptyId: string; title?: string | null; activate?: boolean }
+    opts: { ptyId: string; title?: string | null; activate?: boolean; tabId?: string }
   ):
     | Promise<{ tabId: string; title?: string | null }>
     | { tabId: string; title?: string | null }
@@ -4410,12 +4410,27 @@ export class OrcaRuntimeService {
       const worktree = await this.resolveWorktreeSelector(worktreeSelector)
       const repo = this.store?.getRepo(worktree.repoId)
       const preAllocatedHandle = this.createPreAllocatedTerminalHandle()
+      // Why: mint tabId in main before spawn so paneKey is known at PTY env
+      // build time. Hook-based agent status (Claude/Codex/Cursor/Gemini) keys
+      // off `${tabId}:${paneId}` — without these vars set on the PTY, the
+      // hook payload arrives with an empty paneKey and the renderer cannot
+      // attribute the event. paneId is hard-coded to 1 because this path
+      // never splits and the renderer's nextPaneId starts at 1 for a fresh
+      // tab. See docs/cli-terminal-hook-pane-key.md.
+      const tabId = randomUUID()
+      const paneKey = `${tabId}:1`
+      const env = {
+        ...opts.env,
+        ORCA_PANE_KEY: paneKey,
+        ORCA_TAB_ID: tabId,
+        ORCA_WORKTREE_ID: worktree.id
+      }
       const result = await this.ptyController.spawn({
         cols: 120,
         rows: 40,
         cwd: worktree.path,
         command: opts.command,
-        env: opts.env,
+        env,
         connectionId: repo?.connectionId ?? null,
         worktreeId: worktree.id,
         preAllocatedHandle
@@ -4432,10 +4447,13 @@ export class OrcaRuntimeService {
         try {
           // Why: after the PTY is spawned, renderer tab adoption is best-effort;
           // failing here must not strand a live process without returning a handle.
+          // Pass the pre-minted tabId so the renderer adopts under the same id
+          // already baked into the PTY env — keeps paneKey hook attribution intact.
           await this.notifier.revealTerminalSession(worktree.id, {
             ptyId: result.id,
             title: opts.title ?? null,
-            activate: false
+            activate: false,
+            tabId
           })
           surface = 'visible'
         } catch (err) {

src/main/window/attach-main-window-services.ts

@@ -263,7 +263,11 @@ function registerRuntimeWindowLifecycle(
           worktreeId,
           ptyId: opts.ptyId,
           title: opts.title ?? undefined,
-          activate: opts.activate !== false
+          activate: opts.activate !== false,
+          // Why: pre-minted tabId from main keeps the renderer's tab id aligned
+          // with the paneKey baked into the PTY env at spawn time, so hook
+          // events route to the right slot. See docs/cli-terminal-hook-pane-key.md.
+          ...(opts.tabId ? { tabId: opts.tabId } : {})
         })
       }),
     splitTerminal: (tabId, paneRuntimeId, opts) => {

src/preload/api-types.ts

@@ -1096,6 +1096,7 @@ export type PreloadApi = {
         title?: string
         ptyId?: string
         activate?: boolean
+        tabId?: string
       }) => void
     ) => () => void
     onRequestTerminalCreate: (

src/preload/index.ts

@@ -1857,6 +1857,7 @@ const api = {
         title?: string
         ptyId?: string
         activate?: boolean
+        tabId?: string
       }) => void
     ): (() => void) => {
       const listener = (
@@ -1868,6 +1869,7 @@ const api = {
           title?: string
           ptyId?: string
           activate?: boolean
+          tabId?: string
         }
       ) => callback(data)
       ipcRenderer.on('ui:createTerminal', listener)

src/renderer/src/hooks/useIpcEvents.ts

@@ -31,6 +31,7 @@ import { setDriverForPty } from '@/lib/pane-manager/mobile-driver-state'
 import { destroyPersistentWebview } from '@/components/browser-pane/webview-registry'
 import { attachMobileMarkdownBridge } from '@/runtime/mobile-markdown-bridge'
 import { detectLanguage } from '@/lib/language-detect'
+import { track } from '@/lib/telemetry'
 
 export { resolveZoomTarget } from './resolve-zoom-target'
 
@@ -226,7 +227,7 @@ export function useIpcEvents(): void {
 
     unsubs.push(
       window.api.ui.onCreateTerminal(
-        ({ requestId, worktreeId, command, title, ptyId, activate }) => {
+        ({ requestId, worktreeId, command, title, ptyId, activate, tabId }) => {
           try {
             const store = useAppStore.getState()
             const shouldActivate = activate !== false
@@ -247,7 +248,12 @@ export function useIpcEvents(): void {
                 ) ??
                 store.createTab(worktreeId, undefined, undefined, {
                   initialPtyId: ptyId,
-                  activate: shouldActivate
+                  activate: shouldActivate,
+                  // Why: tabId hint comes from CLI-spawned PTYs whose env
+                  // already has paneKey=`${tabId}:1` baked in. Adopting the
+                  // tab under the same id keeps hook-event attribution working;
+                  // see docs/cli-terminal-hook-pane-key.md.
+                  ...(tabId ? { id: tabId } : {})
                 }))
               : store.createTab(worktreeId)
             if (shouldActivate) {
@@ -986,6 +992,17 @@ export function useIpcEvents(): void {
       }
       const { exists, title } = resolvePaneKey(store, data.paneKey)
       if (!exists) {
+        // Why: regression signal — pre-fix, CLI-spawned terminals routinely
+        // shipped hook events with an empty/unknown paneKey because main
+        // could not stamp ORCA_PANE_KEY into the PTY env before the tab id
+        // existed. Post-fix this should be near-zero in normal use; a
+        // non-zero rate means a paneKey is escaping into the receiver
+        // without a matching renderer tab. Gated on workspaceSessionReady
+        // above so startup-race pings don't false-positive before tabs
+        // hydrate. See docs/cli-terminal-hook-pane-key.md.
+        track('agent_hook_unattributed', {
+          reason: data.paneKey ? 'unknown_tab_id' : 'empty_pane_key'
+        })
         return
       }
       store.setAgentStatus(data.paneKey, payload, title, {

src/renderer/src/store/slices/store-cascades.test.ts

@@ -1613,3 +1613,53 @@ describe('shutdownWorktreeTerminals (sleep) — agent status hygiene', () => {
     expect(s.acknowledgedAgentsByPaneKey['tab-1:0']).toBeUndefined()
   })
 })
+
+// Why: CLI-spawned background terminals stamp ORCA_PANE_KEY=`${tabId}:1` into
+// the PTY env at spawn time. The renderer must adopt the tab under the same
+// id so hook events route to the correct slot. See
+// docs/cli-terminal-hook-pane-key.md.
+describe('createTab tabId hint', () => {
+  it('uses the supplied id when no collision exists', () => {
+    const store = createTestStore()
+    const wt = 'repo1::/path/wt-hint'
+    seedStore(store, {
+      worktreesByRepo: {
+        repo1: [makeWorktree({ id: wt, repoId: 'repo1', path: '/path/wt-hint' })]
+      },
+      groupsByWorktree: {},
+      activeGroupIdByWorktree: {},
+      unifiedTabsByWorktree: {}
+    })
+
+    const hintedId = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
+    const tab = store.getState().createTab(wt, undefined, undefined, { id: hintedId })
+
+    expect(tab.id).toBe(hintedId)
+  })
+
+  it('falls back to a fresh id on collision and warns', () => {
+    const store = createTestStore()
+    const wt = 'repo1::/path/wt-collision'
+    const existingId = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
+    seedStore(store, {
+      worktreesByRepo: {
+        repo1: [makeWorktree({ id: wt, repoId: 'repo1', path: '/path/wt-collision' })]
+      },
+      tabsByWorktree: {
+        [wt]: [makeTab({ id: existingId, worktreeId: wt })]
+      },
+      groupsByWorktree: {},
+      activeGroupIdByWorktree: {},
+      unifiedTabsByWorktree: {}
+    })
+
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
+    try {
+      const tab = store.getState().createTab(wt, undefined, undefined, { id: existingId })
+      expect(tab.id).not.toBe(existingId)
+      expect(warn).toHaveBeenCalledWith(expect.stringContaining(existingId))
+    } finally {
+      warn.mockRestore()
+    }
+  })
+})

src/renderer/src/store/slices/terminals.ts

@@ -137,7 +137,16 @@ export type TerminalSlice = {
     worktreeId: string,
     targetGroupId?: string,
     shellOverride?: string,
-    options?: { pendingActivationSpawn?: boolean; initialPtyId?: string; activate?: boolean }
+    options?: {
+      pendingActivationSpawn?: boolean
+      initialPtyId?: string
+      activate?: boolean
+      /** Pre-allocated tab id (e.g. minted by main for CLI-spawned terminals
+       *  whose PTY env already carries `paneKey=`${tabId}:1``). Falls back to
+       *  minting a fresh id when omitted or when the supplied id collides
+       *  with an existing tab on this worktree. */
+      id?: string
+    }
   ) => TerminalTab
   closeTab: (tabId: string) => void
   reorderTabs: (worktreeId: string, tabIds: string[]) => void
@@ -321,7 +330,6 @@ export const createTerminalSlice: StateCreator<AppState, [], [], TerminalSlice>
     }),
 
   createTab: (worktreeId, targetGroupId, shellOverride, options) => {
-    const id = globalThis.crypto.randomUUID()
     let tab!: TerminalTab
     set((s) => {
       const orphanTerminalIds = getOrphanTerminalIds(s, worktreeId)
@@ -329,6 +337,25 @@ export const createTerminalSlice: StateCreator<AppState, [], [], TerminalSlice>
       const existing = (s.tabsByWorktree[worktreeId] ?? []).filter(
         (entry) => !orphanTerminalIds.has(entry.id)
       )
+      // Why: caller-supplied id (e.g. main pre-allocates the tabId for CLI
+      // background terminals so the paneKey env baked into the PTY matches
+      // the renderer's tab id). Fall back to minting if the id collides — a
+      // collision would alias two distinct PTYs to one tab id and silently
+      // corrupt agent-status routing. Hook attribution degrades for that
+      // single terminal because paneKey is already baked into PTY env, but
+      // the rest of the tab works normally. See docs/cli-terminal-hook-pane-key.md.
+      const idCollides =
+        options?.id !== undefined &&
+        Object.values(s.tabsByWorktree).some((tabs) =>
+          tabs.some((entry) => entry.id === options.id)
+        )
+      if (options?.id !== undefined && idCollides) {
+        console.warn(
+          `[createTab] tabId hint ${options.id} already exists; minting a fresh id (hook attribution will degrade for this terminal)`
+        )
+      }
+      const id =
+        options?.id !== undefined && !idCollides ? options.id : globalThis.crypto.randomUUID()
       const shouldActivate = options?.activate !== false
       const nextOrdinal = getNextTerminalOrdinal(existing)
       const defaultTitle = `Terminal ${nextOrdinal}`

src/shared/telemetry-events.ts

@@ -279,6 +279,18 @@ const agentHookInstallFailedSchema = z
   })
   .strict()
 
+// Why: regression signal for paneKey attribution. A hook event whose paneKey
+// does not correspond to any tab in `tabsByWorktree` indicates the renderer
+// could not route the event to a pane. Pre-fix this fired routinely for
+// CLI-spawned terminals (empty paneKey); post-fix it should be near-zero in
+// normal use. The lone `reason` field discriminates between the known
+// failure modes: an empty/missing paneKey on the wire (pre-fix CLI shape)
+// vs. a paneKey whose tab id no longer exists in `tabsByWorktree` (renderer
+// adopted a different id). See docs/cli-terminal-hook-pane-key.md.
+const agentHookUnattributedSchema = z
+  .object({ reason: z.enum(['empty_pane_key', 'unknown_tab_id']) })
+  .strict()
+
 // ── Onboarding ──────────────────────────────────────────────────────────
 //
 // Closed enums only — no raw paths, repo names, clone URLs, or error
@@ -529,6 +541,7 @@ export const eventSchemas = {
   agent_started: agentStartedSchema,
   agent_error: agentErrorSchema,
   agent_hook_install_failed: agentHookInstallFailedSchema,
+  agent_hook_unattributed: agentHookUnattributedSchema,
 
   settings_changed: settingsChangedSchema,