diff --git a/docs/resource-usage-merge-spec.md b/docs/resource-usage-merge-spec.md new file mode 100644 index 00000000000..46e56612557 --- /dev/null +++ b/docs/resource-usage-merge-spec.md @@ -0,0 +1,387 @@ +# Resource Usage popover: merge Resources + Sessions into one view + +## Problem + +The status-bar Resource Usage popover currently has two tabs: + +- **Resources** — per-worktree CPU/memory grouped by repo, sourced from + `MemorySnapshot.worktrees`. Local PTYs only (SSH excluded by design at + `src/main/ipc/pty.ts:832` because remote process trees are not visible + to the local `ps`/`wmic` sweep). +- **Sessions** — flat list of every PTY the daemon tracks (local **and** + SSH), sourced from `pty.listSessions()`. Adds tab-binding info, click- + to-navigate, kill-X per row, and "Kill N orphans". + +The data sources only partially overlap: Sessions can show worktrees +(e.g. `orca/Porpoise`, `orca/Stingray`) that never appear in Resources +because they're SSH-backed. Users see this as a confusing inconsistency +between two adjacent tabs in the same popover. + +The user wants one unified view that: + +1. Lists every PTY the daemon knows about (the union, not the + intersection). +2. Shows CPU/memory **when available** (i.e. for local PTYs) and `—` for + remote ones, rather than silently dropping remote sessions. +3. Keeps tab-binding affordances (click row → navigate, kill X with + confirm dialog) on every row. +4. Surfaces orphans (PTYs not bound to any tab) with the "Kill N + orphans" bulk action. + +## Direction (already decided by user) + +> Merge into one tab, drop the switcher entirely. + +Out of scope: + +- Adding new IPC. The merge uses **only** the data already available: + `MemorySnapshot` and `pty.listSessions()`. +- Changing what the memory collector tracks. SSH PTYs remain unsampled. + The unified view simply shows them with empty metric cells instead of + hiding them. +- Adding new actions (e.g. "stop all in repo", per-repo restart, etc.). + The exact action set today (kill one with confirm, kill orphans, sleep + worktree, restart daemon, kill-all-sessions) is preserved. + +## Data model + +### Inputs + +- `snapshot: MemorySnapshot | null` — local-only CPU/Mem data, grouped + by `worktreeId`. May be `null` while the daemon is unreachable. +- `sessions: DaemonSession[]` — every PTY id the daemon knows, with + `cwd`, `title`, and a sticky `isAlive`-equivalent. +- `tabsByWorktree`, `ptyIdsByTabId`, `runtimePaneTitlesByTabId`, + `workspaceSessionReady` — store-side context used to (a) compute + bound/orphan and (b) resolve human labels. + +### Output: a renderer-local view model + +The merge produces a renderer-local view-model — **not** a widening of +the shared `WorktreeMemory` shape. `src/shared/types.ts` and the +collector are untouched. New types live next to the merge helper: + +```ts +// src/renderer/src/components/status-bar/unifiedRow.ts +type Metric = number | null // null === "no local sample" (e.g. SSH) + +type UnifiedSessionRow = { + sessionId: string + label: string + bound: boolean + tabId: string | null + cpu: Metric + memory: Metric + hasLocalSamples: boolean +} + +type UnifiedWorktreeRow = { + worktreeId: string + worktreeName: string + repoId: string + repoName: string + cpu: Metric + memory: Metric + history: number[] // empty when hasLocalSamples is false + hasLocalSamples: boolean + sessions: UnifiedSessionRow[] +} + +type UnifiedRepoGroup = { + repoId: string + repoName: string + cpu: Metric // null if every child has hasLocalSamples false + memory: Metric + hasRemoteChildren: boolean // drives the "· remote" badge on the row + worktrees: UnifiedWorktreeRow[] +} +``` + +A small adapter inside `mergeSnapshotAndSessions` converts each +`WorktreeMemory` from the shared type into a `UnifiedWorktreeRow` with +numeric metrics and `hasLocalSamples: true`. Synthetic remote rows are +constructed directly with `null` metrics. The existing `MetricPair` and +`Sparkline` callsites in the popover get a thin wrapper that renders +`—` when its input is `null`; nothing in `src/shared/` or `src/main/` +changes. + +### Merge algorithm + +The merge is renderer-only and pure. Inputs in priority order: + +- **Snapshot first.** `snapshot.worktrees` is the authoritative source + for any local PTY's CPU/Mem and for the worktree's identity (it + already carries `worktreeId`, `worktreeName`, `repoId`, `repoName`, + and a `sessions[]` array with per-session metrics). +- **Sessions augments.** `sessions: DaemonSession[]` is the union over + the snapshot — it adds (a) SSH PTYs that the local memory collector + cannot see and (b) any local PTY that registered too late to make + this snapshot's sweep. + +#### Join key (snapshot session ↔ daemon session) + +Both layers identify a session by the **same string**: the PTY id. + +- `SessionMemory.sessionId` (collector) is `pty.sessionId ?? pty.ptyId` + from `pty-registry.ts`. In-process local PTYs spawn with + `args.sessionId === undefined` (`pty.ts:684`), so the registry stores + `sessionId: null` and the fallback `pty.ptyId` is used — which equals + `result.id` from the local provider's spawn, which is also what + `LocalPtyProvider.listProcesses()` returns as `id` + (`local-pty-provider.ts:518-522`). +- For daemon-hosted spawns, `mintPtySessionId(worktreeId)` produces the + `${worktreeId}@@${shortUuid}` form + (`src/main/daemon/pty-session-id.ts`); both the registry and + `pty.listSessions()` carry that exact string as the id. + +Therefore the dedup key is simply `session.id` on both sides. The +merge MUST build `Set` of all `SessionMemory.sessionId`s from +the snapshot and skip any `DaemonSession` whose `id` is already in the +set (so a local session never appears twice — once with metrics, once +as a `—` placeholder). + +#### Worktree resolution for `DaemonSession`s + +When a `DaemonSession` is **not** already accounted for by the +snapshot, we need to bucket it under a worktree group. Try in order: + +1. **Tab-store walk** (existing logic in `SessionsTabPanel`): look up + `ptyIdsByTabId` → `tabId` → `tabsByWorktree` → `worktreeId`. This + resolves any session bound to a live tab in *this* renderer. +2. **Session-id parse**: if the id contains `@@`, take + `id.slice(0, id.lastIndexOf('@@'))` as a candidate `worktreeId`. + This is the convention enforced by `mintPtySessionId` (see + `pty-session-id.ts:5-7` and the symmetric parser in + `daemon-pty-adapter.ts:328-331`). It correctly recovers the + worktreeId for SSH sessions that haven't been bound to a tab in + this Orca instance — which is the user's primary scenario + (`orca/Stingray`, `orca/Sawfish`, etc.). +3. **Unattributed**: if neither resolves, bucket under a synthetic + `unattributed` repo group at the bottom of the list. + +The `repoId` for each new worktree group is recovered the same way the +collector does it (`collector.ts:resolveWorktreeNames`): split the +worktreeId on the first `::`. Resolved repo display name comes from +the renderer-side store (`store.repos[repoId]?.displayName`) when +available; otherwise the bare `repoId` is used. + +#### Step-by-step + +1. Initialize `repos: Map` empty. +2. Insert all `snapshot.worktrees` (if any) into the map, grouped by + their `repoId`. Each worktree carries `hasLocalSamples: true` and + numeric metrics; its sessions inherit numeric metrics. +3. Build `seenSessionIds = new Set(...all session ids in + snapshot.worktrees[].sessions)`. +4. For each `DaemonSession` in `sessions: DaemonSession[]`: + - If `seenSessionIds.has(session.id)`, skip (already merged from + snapshot). + - Resolve `worktreeId` per the three-step rule above. + - If the resolved `worktreeId` is already a worktree group in the + map, append the session to its `sessions[]` with `cpu: null`, + `memory: null`, `hasLocalSamples: false`. (Reachable for SSH + sessions whose worktree happens to match a locally-active + worktree id — rare but possible.) + - Otherwise, create a new worktree group with `cpu: null`, + `memory: null`, `history: []`, `hasLocalSamples: false`, and + append the session. +5. Compute per-repo aggregates: sum `cpu`/`memory` from worktrees with + `hasLocalSamples === true`. If the repo has *any* worktree with + `hasLocalSamples === false`, set `repoHasRemoteChildren: true` (used + by the UI to render a `· remote` badge on the repo header so the + user knows the displayed totals exclude remote worktrees). +6. The Orca app section (Main / Renderer / Other) renders unchanged + below the list. + +The whole pass is O(W + S) where W = snapshot worktrees and S = +sessions. No nested scans. + +### Why this is safe with current data + +- Every existing local session continues to render with the same + numbers. The merge **adds** rows; it does not change values. +- The snapshot rebuilds every poll (10s), so SSH sessions transitioning + to local (or vice versa) flip representation on the next tick. No + reconciliation logic needed. +- Orphan detection still works: a session is "bound" iff its + `sessionId` appears in `boundPtyIds`. Sessions in repos we couldn't + resolve still get the X-kill affordance. + +## UI + +### Trigger (status-bar badge) — unchanged + +Mac/Linux/Windows safe (no platform-specific glyphs introduced). + +### Popover — single panel, fixed height + +``` +┌─────────────────────────────────────────────────────────────┐ +│ Resource Usage ⟳ 🗑 ✕ │ ← header +├─────────────────────────────────────────────────────────────┤ +│ 1.9% · 955.2 MB · 3% of system RAM │ ← summary +├─────────────────────────────────────────────────────────────┤ +│ Name CPU Memory · │ ← sort row +├─────────────────────────────────────────────────────────────┤ +│ ▾ Triton 0.0% 125.5 MB │ +│ Terminal 1 0.0% 63.2 MB ✕│ +│ Terminal 1 0.0% 1.3 MB ✕│ +│ ▾ Stingray ⓘ remote — — │ +│ orca/Stingray — — ✕│ +│ ▾ Sawfish ⓘ remote — — │ +│ orca/Sawfish — — ✕│ +│ ───────────────────────── │ +│ ▸ Orca app 0.5% 829.7 MB │ +└─────────────────────────────────────────────────────────────┘ + ↑ fixed 420px body, owns its own scroll + ↓ orphan bulk-kill pill renders here when N>0 +``` + +The header keeps the icon-only **Restart daemon** (RotateCw) and +**Kill all sessions** (Trash2) buttons in the top-right. + +### Interaction states (4 paths) + +| Scenario | Resource cells | Kill X | Click row | +|-----------------------------|----------------|-------------|--------------| +| Local PTY, bound | numeric | hover only | navigates | +| Local PTY, orphan | numeric | always | no-op | +| Remote PTY, bound | `—` | hover only | navigates | +| Remote PTY, orphan | `—` | always | no-op | + +`—` is rendered as `text-muted-foreground/50`. The worktree row carries +a small `· remote` badge when `hasLocalSamples === false`; the repo row +carries the same badge when `hasRemoteChildren === true` (so users +know the displayed repo totals exclude remote worktrees). + +**`bound` semantics are unchanged from today:** a session is bound iff +`boundPtyIds.has(session.id)` evaluated against this renderer's +`ptyIdsByTabId`. SSH sessions bound to tabs in *another* Orca process +look identical to local orphans here — same as before the merge. + +### Empty / loading / error states + +- Daemon unreachable → existing banner remains, body shows nothing. +- Sessions error AND no snapshot AND zero sessions → "Resource data + unavailable. Restart daemon." +- Snapshot present but zero worktrees AND zero sessions → "Nothing + running right now." +- Loading (no snapshot, no error) → "Loading…". + +### Sort + +Three buttons in the sort row: **Name**, **CPU**, **Memory**. CPU/Mem +sort puts `null` metrics last (stable). Name uses `localeCompare`. +Selection is repo-group aware: sorting by CPU sorts repos by aggregate +CPU and worktrees within a repo by their CPU. + +### Confirm dialogs (unchanged) + +- Per-row kill X: confirm Dialog with copy "Kill this session? Force- + quits ``. Any unsaved work in that pane is lost. This can't be + undone." +- Header Trash2: existing "Kill all sessions" Dialog from + `useDaemonActions`. +- Header RotateCw: existing "Restart daemon" Dialog from + `useDaemonActions`. +- Worktree-level Sleep / Delete: unchanged. + +## Implementation plan + +1. **New helper** `mergeSnapshotAndSessions.ts` (renderer-side, pure + function) that takes `(snapshot, sessions, storeContext)` and + returns the unified `RepoGroup[]` shape above. Unit-testable. +2. **Refactor `ResourceUsageStatusSegment.tsx`**: + - Remove the tab switcher (drop `activeTab` state, both + `'resources'` / `'sessions'` branches, the inline pill component, + and the `Trash2` import for the old switcher). + - Replace the body with a single render path that consumes the + unified `RepoGroup[]` from the helper. + - Keep the existing `WorktreeSection`/`AppSection` patterns — extend + them to render `null` metrics as `—` and to render the kill-X on + each session row (with bound-vs-orphan visibility rule). + - Promote `killConfirm` state and the confirm Dialog from + `SessionsTabPanel` to the segment level since there's only one + panel now. + - Keep `onSessionsChanged` plumbing — call it from kill / kill- + orphans handlers to trigger an immediate `refreshSessions()`. + - Delete the now-unused `SessionsTabPanel` component. +3. **Wire metric formatting** to render `—` when metric is `null`. Use + the `null`-vs-`0` distinction already present in the data model; + don't conflate them. +4. **Migration**: status-bar item id stays `'resource-usage'`. The + `migrateStatusBarItems` helper is unchanged. No persistence + migration needed for this UI-only change. + +## Testing + +- Unit: `mergeSnapshotAndSessions` with synthetic inputs covering: + - All 4 interaction-state paths (local-bound, local-orphan, + remote-bound, remote-orphan). + - **Dedup**: a local session that appears in both the snapshot and + the daemon list is rendered exactly once with numeric metrics. + - **`@@` parse**: an SSH session id `repoX::/path/wtA@@abcd` with no + matching tab resolves to repoId `repoX`, worktreeId + `repoX::/path/wtA`. + - **Tab walk wins over `@@` parse**: a session bound to a tab whose + worktreeId differs from the `@@` prefix uses the tab's worktreeId + (defensive against id-format drift). + - **Repo aggregate excludes remote children**: a repo with one + local (125 MB) and one remote (`null`) worktree reports + `cpu/memory = 125 MB` and `hasRemoteChildren = true`. + - Edge cases: snapshot null, sessions empty, both empty, sessionId + without `@@` and no tab match → falls into Unattributed. +- Manual: + - Open with mixed local + SSH worktrees → both render, SSH rows show + `—` and a `· remote` badge on their worktree and repo headers. + - Kill a remote session via the X → confirm dialog → optimistic + removal so row disappears immediately (does not wait 10s). + - Kill all (header trash) → orphans gone, bound sessions remain. + - Resize window / scroll the body → fixed 420px holds, no reflow. + - Accessibility: tab through rows → kill X focusable; after kill + confirm, focus lands back on the popover content root, not on + ``. Dialog is keyboard-dismissable except while in-flight. + +## Risks & mitigations + +- **Risk**: SSH worktrees the user never opened locally still appear in + Resources (potentially noisy if the daemon retains many idle SSH + sessions). **Mitigation**: bucketing under a `· remote` badge keeps + them visually distinct from local-active groups; sort-by-memory pushes + them to the bottom. If noise becomes a real complaint, a future PR + can add a "hide remote" toggle. +- **Risk**: Some sessions can't be resolved to any worktreeId (e.g. + daemon-internal sessions whose ids don't follow the `@@` convention, + or sessions whose worktreeId resolves to a repo store doesn't know + about). **Mitigation**: synthetic `Unattributed` repo group at the + bottom, which already exists in spirit as `ORPHAN_WORKTREE_ID` in the + memory collector. +- **Risk**: Kill-X on a remote session fires `pty.kill(id)`, which the + IPC layer routes through `ptyOwnership` to the right SSH provider. + Behavior is identical to the old Sessions tab — no new failure mode. +- **Risk: poll-rate skew between snapshot (2s) and sessions (10s).** + After killing an SSH session, the daemon-side `sessions` list can + retain the dead row for up to 10s while the snapshot has already + moved on; conversely a freshly-spawned local PTY shows up in + `sessions` first and is rendered as a remote placeholder for up to + 2s before the next snapshot promotes it. + **Mitigation**: per-row kill optimistically removes the session id + from the renderer's local `sessions` state immediately after the IPC + resolves (in addition to calling `onSessionsChanged()`), so the + killed row never lingers on screen. The new-session-flash is + bounded by `POLL_MS` and acceptable. +- **Risk: focus dropped to `` after kill confirm.** When the + killed session disappears on the next refresh, the per-row X that + had focus is unmounted. With the confirm Dialog now mounted at the + segment level, this regression is more visible. + **Mitigation**: after `runKillConfirmed` resolves, focus the + popover's content root via a stable ref so keyboard users land back + in the list, not on ``. + +## Non-goals + +- No new IPC. +- No collector changes. +- No new persisted UI prefs (the per-tab visibility split is gone, not + replaced with a per-section toggle). +- No remote memory sampling — that's a separate, much bigger project. diff --git a/docs/tasks-page-resume-state.md b/docs/tasks-page-resume-state.md new file mode 100644 index 00000000000..64589b277ac --- /dev/null +++ b/docs/tasks-page-resume-state.md @@ -0,0 +1,176 @@ +# Tasks Page Resume State + +## Goal + +When a user leaves the Tasks page and comes back later, Orca should reopen the page in the same working context instead of falling back to a generic GitHub Issues/PRs list. + +The resume behavior should cover: + +- GitHub vs Linear. +- GitHub Issues/PRs vs GitHub Project mode. +- The selected GitHub Issues/PRs preset or custom search. +- The selected GitHub Project and Project view. +- The selected repos for GitHub Issues/PRs. +- The selected Linear teams. + +This should be lightweight. It should not introduce a second project-selection model or duplicate state that is already persisted elsewhere. + +## Existing Persisted State + +The current settings model already remembers most durable task choices: + +- `settings.defaultTaskSource` remembers GitHub vs Linear. +- `settings.defaultRepoSelection` remembers GitHub repo selection for cross-repo Issues/PRs. +- `settings.defaultLinearTeamSelection` remembers Linear team selection. +- `settings.githubProjects.activeProject` remembers the active GitHub Project. +- `settings.githubProjects.lastViewByProject` remembers the last selected Project view per Project. +- `settings.defaultTaskViewPreset` remembers the user's default GitHub Issues/PRs preset. + +These should stay in place. They are already wired into the page and, for repo/team/project selection, they represent actual user preferences rather than purely transient page state. + +## Missing State + +The missing piece is the user's current page position inside Tasks: + +- Whether GitHub is showing `Issues/PRs` or `Project`. +- Which GitHub Issues/PRs preset is currently active. +- The currently applied GitHub Issues/PRs query when the user has typed a custom search. +- Which Linear preset is currently active. +- The currently applied Linear query when the user has typed a search. + +Linear presets are visible task tabs and drive the fetch path, so they must be restored too. + +## Proposed Shape + +Add a small optional field to `PersistedUIState`: + +```ts +export type TaskResumeState = { + githubMode?: 'items' | 'project' + githubItemsPreset?: TaskViewPresetId | null + githubItemsQuery?: string + linearPreset?: 'assigned' | 'created' | 'all' | 'completed' + linearQuery?: string +} +``` + +Then add: + +```ts +taskResumeState?: TaskResumeState +``` + +to `PersistedUIState`. + +Why `PersistedUIState`: this is page-position UI state, similar to sidebar filters and widths. It is not an app-wide setting and should not be presented as a configurable default. + +Do not add `source` to `TaskResumeState`. `settings.defaultTaskSource` already represents the user's last-used task source today, and duplicating it would create two persisted sources of truth. Keep source changes on the existing settings path. + +Do not reset `githubMode` just because the current source is Linear. Source selection and GitHub sub-mode are independent pieces of context: if the user was in GitHub Project mode, switched to Linear, then later switches back to GitHub, GitHub should still reopen in Project mode. + +## Restore Rules + +On Tasks page mount: + +1. If `taskPageData.taskSource` was passed by a caller, use it. Explicit navigation intent should win. +2. Otherwise fall back to `settings.defaultTaskSource`. +3. Apply these restore rules only after both settings and persisted UI state have hydrated. The current app loads those asynchronously before setting `persistedUIReady`; a `useState` initializer that reads `settings === null` or an unhydrated `taskResumeState` will capture defaults and miss the restored context. + +The restore should run once per Tasks page mount, after hydration is ready. It should not keep reapplying persisted state over local user interactions while the page remains open. + +For GitHub: + +1. Restore `githubMode` from `taskResumeState.githubMode`. +2. If mode is `items`, restore `githubItemsPreset` and `githubItemsQuery`. +3. Repo selection continues to come from `settings.defaultRepoSelection`. +4. If mode is `project`, use `settings.githubProjects.activeProject` and `settings.githubProjects.lastViewByProject`. +5. If Project mode has no active project or view, show the Project picker empty state instead of silently switching to Issues/PRs. + +For Linear: + +1. Restore `linearPreset`. +2. Restore `linearQuery`. +3. Team selection continues to come from `settings.defaultLinearTeamSelection`. + +Fallback behavior: + +- If resume state is absent, use today's defaults. +- If `githubItemsPreset` is non-null, derive the query from the preset unless `githubItemsQuery` disagrees because of a future migration. The preset should be treated as authoritative. +- If `githubItemsPreset` is null, use `githubItemsQuery` as a custom search. +- If `linearQuery` is non-empty, treat it as custom search and keep the restored `linearPreset` as the preset to return to after the search is cleared. The visible active preset should be suppressed while the search input is non-empty, matching today's UI. + +## Write Rules + +Update `taskResumeState` only when the user changes the active page context: + +- Switching GitHub mode between Issues/PRs and Project. +- Clicking a GitHub Issues/PRs preset. +- Applying the debounced GitHub Issues/PRs search. +- Clearing the GitHub Issues/PRs search. +- Clicking a Linear preset. +- Applying the debounced Linear search. +- Clearing the Linear search. + +Do not persist every raw search keystroke. The current UI applies search after a 300 ms debounce, so the persisted value should follow the debounced applied query, not the raw input. A user who types, waits for results, leaves Tasks, and comes back should see the same applied query. + +Task source, repo selection, Linear team selection, active Project, and active Project view should continue writing through their existing settings paths. + +The resume setter should merge partial updates with the existing resume state so changing one dimension does not erase the others. It should persist via `window.api.ui.set({ taskResumeState })` after updating local Zustand state. + +When writing preset selections, clear stale custom-query ambiguity explicitly: + +- GitHub preset click: write `{ githubItemsPreset: presetId, githubItemsQuery: undefined }` or the canonical preset query. Restore must still use the preset as authoritative. +- GitHub custom search apply/debounce: write `{ githubItemsPreset: null, githubItemsQuery: trimmedQuery }`. +- GitHub search clear: write `{ githubItemsPreset: null, githubItemsQuery: '' }`. +- Linear preset click: write `{ linearPreset: presetId, linearQuery: '' }`. +- Linear custom search apply/debounce: write `{ linearQuery: trimmedQuery }` without changing `linearPreset`. +- Linear search clear: write `{ linearQuery: '' }` without changing `linearPreset`. + +Do not persist mode changes caused only by hiding Project mode while another source is active. In practice, avoid an effect that changes persisted `githubMode` from `project` to `items` when `taskSource !== 'github'`. + +## Implementation Notes + +Primary files: + +- `src/shared/types.ts`: add `TaskResumeState` and `taskResumeState?: TaskResumeState`. +- `src/shared/constants.ts`: no required default beyond leaving the field absent. If a concrete default is preferred, use GitHub Issues/PRs with the existing default preset. +- `src/renderer/src/store/slices/ui.ts`: hydrate `taskResumeState` defensively, expose a setter such as `setTaskResumeState`, and use it in `openTaskPage` prefetch selection. +- `src/renderer/src/components/TaskPage.tsx`: initialize source from existing settings/page data, and initialize GitHub mode, GitHub item preset/query, Linear preset, and Linear query from the resume state. +- `src/renderer/src/App.tsx`: no new work should be needed if the field rides the existing `ui:get` hydration. + +The setter should call `window.api.ui.set({ taskResumeState })` through the same persistence path used by other persisted UI fields, or follow the existing store pattern if there is already a central UI save effect for the relevant slice. + +Hydration must sanitize the nested object because `PersistedUIState` comes from disk. Invalid `githubMode`, invalid GitHub preset ids, invalid Linear preset ids, or non-string queries should fall back field-by-field rather than entering Zustand as untrusted values. + +`TaskPage` should subscribe to `persistedUIReady`, `settings`, and `taskResumeState`, then perform a one-shot local initialization once both settings and UI hydration are available. This avoids the current class of bugs where `useState(settings?.defaultTaskViewPreset ?? 'all')` captures `'all'` before settings arrive. + +`openTaskPage` currently prefetches the settings default GitHub preset. After this change, it should prefetch the query the Tasks page will actually mount with: + +1. If explicit `taskPageData.taskSource` is `linear`, skip GitHub prefetch. +2. If explicit `taskPageData.taskSource` is `github`, or the resolved source is GitHub, prefetch only when the resolved GitHub mode is `items`. +3. If `githubItemsPreset` is non-null, prefetch that preset query. +4. If `githubItemsPreset` is null and `githubItemsQuery` is non-empty, prefetch the custom query. +5. Otherwise prefetch `settings.defaultTaskViewPreset`. + +The prefetch should also use the same repo selection the page will use: `taskPageData.preselectedRepoId` when present, otherwise `settings.defaultRepoSelection`, otherwise all eligible repos. Warming only `activeRepoId` is acceptable as an optimization fallback, but it is not equivalent to the mounted cross-repo query. + +## Non-Goals + +Do not remember scroll position, selected table row, open dialogs, pagination page, Project search overrides, or cached API results in this pass. + +Those states are more fragile because they depend on live remote data. The first version should only restore the user's broad working context. + +## Acceptance Criteria + +- Open Tasks, switch to Linear, leave Tasks, return to Tasks: Linear is selected. +- Open Tasks, switch to GitHub Project mode, switch to Linear, leave Tasks, return to Tasks, then switch back to GitHub: GitHub is still in Project mode. +- Open Tasks, switch to GitHub Project mode, select a Project view, leave Tasks, return to Tasks: Project mode opens on the same Project view. +- Open Tasks, choose GitHub Issues/PRs `My PRs`, leave Tasks, return to Tasks: GitHub Issues/PRs opens with `My PRs` active. +- Open Tasks, type and apply a custom GitHub search, leave Tasks, return to Tasks: GitHub Issues/PRs opens with that custom query and no preset selected. +- Open Tasks, type a GitHub search, wait for the debounced results, leave Tasks, return to Tasks: GitHub Issues/PRs opens with that applied query and no preset selected. +- Open Tasks, choose Linear `Completed`, leave Tasks, return to Tasks: Linear opens with `Completed` active. +- Open Tasks, type a Linear search, wait for the debounced results, leave Tasks, return to Tasks: Linear opens with that applied query. +- Open Tasks before hydration has completed, then wait for hydration: Tasks applies the persisted source/mode/preset/query exactly once and does not overwrite subsequent user changes. +- Corrupt `taskResumeState` on disk with invalid modes/preset ids/non-string queries, restart, and open Tasks: invalid fields are ignored field-by-field without breaking the page. +- Existing repo selection, Linear team selection, and GitHub Project view persistence keep working unchanged. +- Restart the app after each source/mode/preset/custom-query scenario above: the same Tasks context is restored from persisted UI/settings state. diff --git a/mobile/app.json b/mobile/app.json index 5bfa5a4b6fb..ecee00a8bfc 100644 --- a/mobile/app.json +++ b/mobile/app.json @@ -2,7 +2,7 @@ "expo": { "name": "Orca", "slug": "orca-mobile", - "version": "0.0.2", + "version": "0.0.3", "orientation": "portrait", "icon": "./assets/icon.png", "userInterfaceStyle": "automatic", @@ -16,7 +16,7 @@ "ios": { "supportsTablet": true, "bundleIdentifier": "com.stably.orca.mobile", - "buildNumber": "3", + "buildNumber": "4", "infoPlist": { "NSLocalNetworkUsageDescription": "Orca connects to the desktop app on your local network.", "NSAppTransportSecurity": { diff --git a/mobile/app/_layout.tsx b/mobile/app/_layout.tsx index 742addac1cb..bdae84320c3 100644 --- a/mobile/app/_layout.tsx +++ b/mobile/app/_layout.tsx @@ -1,9 +1,10 @@ -import { useCallback } from 'react' +import { useCallback, useEffect } from 'react' import { View, StyleSheet } from 'react-native' -import { Stack } from 'expo-router' +import { Stack, useRouter } from 'expo-router' import { StatusBar } from 'expo-status-bar' import * as SplashScreen from 'expo-splash-screen' import * as Notifications from 'expo-notifications' +import * as Linking from 'expo-linking' import { colors } from '../src/theme/mobile-theme' import { OrcaLogo } from '../src/components/OrcaLogo' @@ -26,7 +27,49 @@ Notifications.setNotificationHandler({ }) }) +// Why: extract the path+payload that follows the orca://pair anchor so we +// can route it to the confirm screen. Accept either a hash payload +// (`orca://pair#`, the QR / shared form) or a query param +// (`orca://pair?code=<...>`, future-proof for share sheets that strip +// fragments). +function extractPairCode(url: string): string | null { + if (!url.startsWith('orca://pair')) return null + const hashIndex = url.indexOf('#') + if (hashIndex !== -1) { + return url.slice(hashIndex + 1) || null + } + const queryIndex = url.indexOf('?') + if (queryIndex !== -1) { + const params = new URLSearchParams(url.slice(queryIndex + 1)) + return params.get('code') + } + return null +} + export default function RootLayout() { + const router = useRouter() + + // Why: route `orca://pair#` deep links to the confirm screen so + // the same pairing flow runs whether the link arrived via QR scan, + // paste, AirDrop, Messages, or `xcrun simctl openurl`. getInitialURL + // covers cold-start (link tapped while app was closed); the listener + // covers warm-start (link tapped while app is in memory). + useEffect(() => { + function handleUrl(url: string) { + const code = extractPairCode(url) + if (code) { + router.push({ pathname: '/pair-confirm', params: { code } }) + } + } + + void Linking.getInitialURL().then((url) => { + if (url) handleUrl(url) + }) + + const sub = Linking.addEventListener('url', ({ url }) => handleUrl(url)) + return () => sub.remove() + }, [router]) + // Why: hide the native splash only once the navigation Stack has been laid // out — this is the earliest moment the user will see actual app content. // Previously the splash hid when a placeholder View rendered, leaving a @@ -55,6 +98,7 @@ export default function RootLayout() { }} /> + diff --git a/mobile/app/about.tsx b/mobile/app/about.tsx index 088c6da211f..70a1effa44b 100644 --- a/mobile/app/about.tsx +++ b/mobile/app/about.tsx @@ -1,11 +1,25 @@ -import { View, Text, StyleSheet, Pressable, Linking } from 'react-native' +import { View, Text, StyleSheet, Pressable, Linking, Platform } from 'react-native' import { useSafeAreaInsets } from 'react-native-safe-area-context' import { useRouter } from 'expo-router' import { ChevronLeft, Globe } from 'lucide-react-native' import Svg, { Path } from 'react-native-svg' +import Constants from 'expo-constants' import { OrcaLogo } from '../src/components/OrcaLogo' import { colors, spacing, typography } from '../src/theme/mobile-theme' +// Why: read version + native build identifier from expo-constants at +// runtime so the About screen never drifts out of sync with app.json. +// nativeBuildVersion is iOS buildNumber on iOS and versionCode on +// Android — different concepts, same role (monotonic native build id). +function getVersionLabel(): string { + const version = Constants.expoConfig?.version ?? '?.?.?' + const build = + Platform.OS === 'ios' + ? Constants.expoConfig?.ios?.buildNumber + : String(Constants.expoConfig?.android?.versionCode ?? '') + return build ? `v${version} (${build})` : `v${version}` +} + function GithubIcon({ size = 16, color = colors.textSecondary }) { return ( @@ -66,6 +80,8 @@ export default function AboutScreen() { @orca_build + + {getVersionLabel()} ) } @@ -141,5 +157,11 @@ const styles = StyleSheet.create({ height: StyleSheet.hairlineWidth, backgroundColor: colors.borderSubtle, marginHorizontal: spacing.md + }, + versionText: { + marginTop: spacing.lg, + textAlign: 'center', + fontSize: typography.metaSize, + color: colors.textMuted } }) diff --git a/mobile/app/h/[hostId]/accounts.tsx b/mobile/app/h/[hostId]/accounts.tsx new file mode 100644 index 00000000000..9319b616e21 --- /dev/null +++ b/mobile/app/h/[hostId]/accounts.tsx @@ -0,0 +1,423 @@ +import { useEffect, useState, useCallback, useRef } from 'react' +import { + View, + Text, + StyleSheet, + Pressable, + ScrollView, + ActivityIndicator, + RefreshControl, + Alert +} from 'react-native' +import { SafeAreaView, useSafeAreaInsets } from 'react-native-safe-area-context' +import { useLocalSearchParams, useRouter } from 'expo-router' +import { ChevronLeft, Check, RefreshCw, User } from 'lucide-react-native' +import { connect, type RpcClient } from '../../../src/transport/rpc-client' +import { loadHosts } from '../../../src/transport/host-store' +import type { ConnectionState, RpcSuccess } from '../../../src/transport/types' +import { colors, spacing, typography, radii } from '../../../src/theme/mobile-theme' +import { ClaudeIcon, OpenAIIcon } from '../../../src/components/AgentIcons' +import { + type AccountsSnapshot, + type ProviderKey, + getActiveProviderRateLimits, + getInactiveProviderUsage, + UsageBar +} from '../../../src/components/AccountUsage' + +export default function AccountsScreen() { + const router = useRouter() + const insets = useSafeAreaInsets() + const { hostId } = useLocalSearchParams<{ hostId: string }>() + + const [client, setClient] = useState(null) + const [connState, setConnState] = useState('connecting') + const [hostName, setHostName] = useState('') + const [snapshot, setSnapshot] = useState(null) + const [error, setError] = useState(null) + const [refreshing, setRefreshing] = useState(false) + const [busyAccountId, setBusyAccountId] = useState(null) + const clientRef = useRef(null) + + // Why: connect to the host's WebSocket on mount and tear down on + // unmount. Mirrors the connection lifecycle used in + // /h/[hostId]/index.tsx so reconnect/auth-failed states are handled + // identically. + useEffect(() => { + if (!hostId) return + let cancelled = false + let rpcClient: RpcClient | null = null + + void (async () => { + const hosts = await loadHosts() + const host = hosts.find((h) => h.id === hostId) + if (!host) { + if (!cancelled) setError('Host not found') + return + } + if (!cancelled) setHostName(host.name) + rpcClient = connect(host.endpoint, host.deviceToken, host.publicKeyB64, (state) => { + if (!cancelled) setConnState(state) + }) + clientRef.current = rpcClient + if (!cancelled) setClient(rpcClient) + })() + + return () => { + cancelled = true + if (rpcClient) rpcClient.close() + clientRef.current = null + } + }, [hostId]) + + // Why: subscribe to streaming snapshot updates so usage bars refresh in + // place when the desktop's rate-limit poll completes (every 5 min) or + // when the user switches accounts. Falls back to a one-shot accounts.list + // if the subscription stream errors. + useEffect(() => { + if (!client || connState !== 'connected') return + const unsubscribe = client.subscribe('accounts.subscribe', null, (payload) => { + if (!payload || typeof payload !== 'object') return + const evt = payload as { type?: string; snapshot?: AccountsSnapshot } + if ((evt.type === 'ready' || evt.type === 'snapshot') && evt.snapshot) { + setSnapshot(evt.snapshot) + setError(null) + } + }) + return unsubscribe + }, [client, connState]) + + const refresh = useCallback(async () => { + if (!client) return + setRefreshing(true) + try { + const res = await client.sendRequest('accounts.list') + if (res.ok) { + setSnapshot((res as RpcSuccess).result as AccountsSnapshot) + setError(null) + } else { + setError(res.error.message) + } + } catch (e) { + setError(e instanceof Error ? e.message : String(e)) + } finally { + setRefreshing(false) + } + }, [client]) + + const selectAccount = useCallback( + async (provider: ProviderKey, accountId: string | null) => { + if (!client) return + setBusyAccountId(accountId ?? `${provider}:default`) + const method = provider === 'claude' ? 'accounts.selectClaude' : 'accounts.selectCodex' + try { + const res = await client.sendRequest(method, { accountId }) + if (!res.ok) { + Alert.alert('Could not switch account', res.error.message) + } else { + // Why: optimistic refresh — the streaming subscription will also + // emit, but a one-shot keeps the UI responsive even if the stream + // is temporarily disconnected. + await refresh() + } + } catch (e) { + Alert.alert('Could not switch account', e instanceof Error ? e.message : String(e)) + } finally { + setBusyAccountId(null) + } + }, + [client, refresh] + ) + + const renderProviderSection = (provider: ProviderKey, title: string) => { + if (!snapshot) return null + const state = provider === 'claude' ? snapshot.claude : snapshot.codex + const activeUsage = getActiveProviderRateLimits(snapshot, provider) + const Icon = provider === 'claude' ? ClaudeIcon : OpenAIIcon + return ( + + + + {title} + + + {/* System default row */} + [styles.row, pressed && styles.rowPressed]} + onPress={() => selectAccount(provider, null)} + disabled={busyAccountId !== null || connState !== 'connected'} + > + + System default + Use the agent's own login + + + {state.activeAccountId === null ? ( + + ) : busyAccountId === `${provider}:default` ? ( + + ) : null} + + + + {state.accounts.map((account) => { + const isActive = state.activeAccountId === account.id + const inactiveEntry = !isActive + ? getInactiveProviderUsage(snapshot, provider, account.id) + : null + const usage = isActive ? activeUsage : (inactiveEntry?.claude ?? null) + const isFetching = + (isActive && usage?.status === 'fetching') || + (!isActive && inactiveEntry?.isFetching === true) + const session = usage?.session + const weekly = usage?.weekly + return ( + + + [styles.row, pressed && styles.rowPressed]} + onPress={() => selectAccount(provider, account.id)} + disabled={busyAccountId !== null || connState !== 'connected' || isActive} + > + + + {account.email} + + + + + + {usage?.error ? ( + + {usage.error} + + ) : null} + + + {isActive ? ( + + ) : busyAccountId === account.id ? ( + + ) : null} + + + + ) + })} + + + ) + } + + return ( + + + router.back()}> + + + + Accounts + {hostName ? ( + + {hostName} + + ) : null} + + + {refreshing ? ( + + ) : ( + + )} + + + + + } + > + {connState !== 'connected' && !snapshot ? ( + + + Connecting to {hostName || 'host'}… + + ) : error && !snapshot ? ( + + {error} + + ) : !snapshot ? ( + + + Loading accounts… + + ) : ( + <> + {renderProviderSection('claude', 'Claude')} + {renderProviderSection('codex', 'Codex')} + + + + Add or re-authenticate accounts from desktop Settings → Accounts. + + + + )} + + + ) +} + +const styles = StyleSheet.create({ + container: { + flex: 1, + backgroundColor: colors.bgBase + }, + topRow: { + flexDirection: 'row', + alignItems: 'center', + paddingHorizontal: spacing.md, + paddingTop: spacing.sm, + paddingBottom: spacing.sm, + gap: spacing.sm + }, + backButton: { + width: 36, + height: 36, + borderRadius: 18, + alignItems: 'center', + justifyContent: 'center' + }, + iconButton: { + width: 36, + height: 36, + borderRadius: 18, + alignItems: 'center', + justifyContent: 'center' + }, + titleWrap: { + flex: 1 + }, + heading: { + fontSize: 20, + fontWeight: '700', + color: colors.textPrimary + }, + subheading: { + fontSize: typography.metaSize, + color: colors.textSecondary, + marginTop: 1 + }, + scroll: { + paddingHorizontal: spacing.lg, + paddingTop: spacing.sm + }, + section: { + marginBottom: spacing.xl + }, + sectionHeader: { + flexDirection: 'row', + alignItems: 'center', + gap: spacing.sm, + marginBottom: spacing.sm + }, + sectionHeading: { + fontSize: typography.metaSize, + fontWeight: '600', + color: colors.textSecondary, + textTransform: 'uppercase', + letterSpacing: 0.5 + }, + card: { + backgroundColor: colors.bgPanel, + borderRadius: radii.card, + overflow: 'hidden' + }, + row: { + flexDirection: 'row', + alignItems: 'center', + paddingVertical: spacing.md, + paddingHorizontal: spacing.md + 2 + }, + rowPressed: { + backgroundColor: colors.bgRaised + }, + rowMain: { + flex: 1, + gap: 4 + }, + // Why: fixed-width trailing slot so the usage bars in `rowMain` keep the + // same width whether or not the row is currently selected (otherwise the + // checkmark on the active account squeezes the bars narrower than the + // inactive rows above/below it). + rowTrailing: { + width: 24, + alignItems: 'flex-end', + justifyContent: 'center', + marginLeft: spacing.sm + }, + rowTitle: { + fontSize: typography.bodySize, + fontWeight: '500', + color: colors.textPrimary + }, + rowSubtitle: { + fontSize: typography.metaSize, + color: colors.textSecondary + }, + separator: { + height: StyleSheet.hairlineWidth, + backgroundColor: colors.borderSubtle, + marginHorizontal: spacing.md + }, + usageRow: { + flexDirection: 'row', + gap: spacing.md, + marginTop: 4 + }, + errorText: { + fontSize: typography.metaSize, + color: colors.statusRed + }, + placeholder: { + paddingVertical: spacing.xl * 2, + alignItems: 'center', + gap: spacing.sm + }, + placeholderText: { + fontSize: typography.bodySize, + color: colors.textSecondary + }, + footerHint: { + flexDirection: 'row', + alignItems: 'flex-start', + gap: spacing.sm, + paddingHorizontal: spacing.sm, + paddingTop: spacing.sm + }, + footerHintText: { + flex: 1, + fontSize: typography.metaSize, + color: colors.textMuted, + lineHeight: 18 + } +}) diff --git a/mobile/app/h/[hostId]/index.tsx b/mobile/app/h/[hostId]/index.tsx index e428d70e511..e5c9c318212 100644 --- a/mobile/app/h/[hostId]/index.tsx +++ b/mobile/app/h/[hostId]/index.tsx @@ -24,7 +24,8 @@ import { Plus, Moon, Filter, - Check + Check, + UserCircle } from 'lucide-react-native' import { connect, type RpcClient } from '../../../src/transport/rpc-client' import { loadHosts, updateLastConnected, removeHost } from '../../../src/transport/host-store' @@ -682,6 +683,17 @@ export default function HostScreen() { + router.push(`/h/${hostId}/accounts`)} + disabled={connState !== 'connected'} + > + + + setShowNewWorktree(true)} diff --git a/mobile/app/h/_layout.tsx b/mobile/app/h/_layout.tsx index 59bbf7eeaf4..1bd3fec4185 100644 --- a/mobile/app/h/_layout.tsx +++ b/mobile/app/h/_layout.tsx @@ -10,6 +10,7 @@ export default function HostGroupLayout() { }} > + ) diff --git a/mobile/app/index.tsx b/mobile/app/index.tsx index 7f01cecb679..59652816892 100644 --- a/mobile/app/index.tsx +++ b/mobile/app/index.tsx @@ -13,6 +13,13 @@ import { Terminal, Plus } from 'lucide-react-native' +import { ClaudeIcon, OpenAIIcon } from '../src/components/AgentIcons' +import { + type AccountsSnapshot, + type ProviderKey, + getActiveProviderRateLimits, + UsageBar +} from '../src/components/AccountUsage' import AsyncStorage from '@react-native-async-storage/async-storage' import { loadHosts, removeHost, renameHost } from '../src/transport/host-store' import { connect, type RpcClient } from '../src/transport/rpc-client' @@ -24,6 +31,7 @@ import { TextInputModal } from '../src/components/TextInputModal' import { ActionSheetModal } from '../src/components/ActionSheetModal' import { ConfirmModal } from '../src/components/ConfirmModal' import { setCachedWorktrees, getCachedWorktrees } from '../src/cache/worktree-cache' +import { loadHomeSnapshot, saveHomeSnapshot } from '../src/cache/home-snapshot-cache' import { colors, spacing, radii } from '../src/theme/mobile-theme' function endpointLabel(endpoint: string): string { @@ -102,16 +110,24 @@ function fetchWorktreeInfo( ) => void, disposed: () => boolean ) { - const markLoaded = () => { - setInfo((prev) => ({ - ...prev, - [hostId]: { - hostId, - totalWorktrees: 0, - activeCount: 0, - lastActiveWorktree: null + // Why: only seed an empty zeroed entry when this host has no prior info + // at all (e.g., first ever load before any cache hydration). On a + // transient failure for a host that already has cached data, leave the + // cached entry alone so the Resume card and host-meta line don't + // momentarily flip to "0 worktrees" / disappear during reconnects. + const markLoadedIfMissing = () => { + setInfo((prev) => { + if (prev[hostId]) return prev + return { + ...prev, + [hostId]: { + hostId, + totalWorktrees: 0, + activeCount: 0, + lastActiveWorktree: null + } } - })) + }) } client @@ -135,14 +151,34 @@ function fetchWorktreeInfo( } })) } else { - markLoaded() + markLoadedIfMissing() } }) .catch(() => { - if (!disposed()) markLoaded() + if (!disposed()) markLoadedIfMissing() }) } +function fetchAccountsSnapshot( + client: RpcClient, + hostId: string, + setSnapshots: ( + updater: (prev: Record) => Record + ) => void, + disposed: () => boolean +) { + client + .sendRequest('accounts.list') + .then((response) => { + if (disposed()) return + if (response.ok) { + const snapshot = response.result as AccountsSnapshot + setSnapshots((prev) => ({ ...prev, [hostId]: snapshot })) + } + }) + .catch(() => {}) +} + // Why: repo names get a stable color derived from hashing, matching the // host detail page's colored dots for visual consistency. const REPO_COLORS = ['#8b5cf6', '#3b82f6', '#22c55e', '#f59e0b', '#ef4444', '#ec4899', '#06b6d4'] @@ -164,11 +200,53 @@ export default function HomeScreen() { const [hostStates, setHostStates] = useState>({}) const [stats, setStats] = useState(null) const [worktreeInfo, setWorktreeInfo] = useState>({}) + const [accountsByHost, setAccountsByHost] = useState>({}) const [lastVisited, setLastVisited] = useState<{ hostId: string; worktreeId: string } | null>( null ) const clientsRef = useRef>([]) + // Why: hydrate the home page from a persisted snapshot on cold-start so + // Resume + Account-usage cards paint immediately with last-known data + // instead of flashing empty for ~1s while the WebSocket reconnects. + // Stream/list responses overwrite this seed in place when they arrive. + const hydratedRef = useRef(false) + useEffect(() => { + if (hydratedRef.current) return + hydratedRef.current = true + let cancelled = false + void loadHomeSnapshot().then((snap) => { + if (cancelled || !snap) return + setWorktreeInfo((prev) => (Object.keys(prev).length > 0 ? prev : snap.worktreeInfo)) + setAccountsByHost((prev) => (Object.keys(prev).length > 0 ? prev : snap.accountsByHost)) + for (const [hostId, info] of Object.entries(snap.worktreeInfo)) { + const wt = info.lastActiveWorktree + if (wt) { + // Why: also seed the in-memory worktree cache so resumeWorktree's + // lastVisited fast-path can find the cached worktree object. + setCachedWorktrees(hostId, [wt]) + } + } + }) + return () => { + cancelled = true + } + }, []) + + // Why: persist the merged snapshot whenever either piece updates so the + // next cold-start has fresh seed data. The cache module debounces writes + // internally so a flurry of streamed updates doesn't hammer disk. + useEffect(() => { + if (Object.keys(worktreeInfo).length === 0 && Object.keys(accountsByHost).length === 0) { + return + } + saveHomeSnapshot({ + worktreeInfo, + accountsByHost, + savedAt: Date.now() + }) + }, [worktreeInfo, accountsByHost]) + useFocusEffect( useCallback(() => { let stale = false @@ -185,6 +263,7 @@ export default function HomeScreen() { if (entry.client.getState() === 'connected') { fetchStats(entry.client, setStats, () => stale) fetchWorktreeInfo(entry.client, entry.hostId, setWorktreeInfo, () => stale) + fetchAccountsSnapshot(entry.client, entry.hostId, setAccountsByHost, () => stale) } } return () => { @@ -222,25 +301,43 @@ export default function HomeScreen() { } let unsubNotif: (() => void) | null = null + let unsubAccounts: (() => void) | null = null let statsFetched = false const unsubState = client.onStateChange((state) => { if (state === 'connected') { if (!unsubNotif) { unsubNotif = subscribeToDesktopNotifications(client) } + if (!unsubAccounts) { + unsubAccounts = client.subscribe('accounts.subscribe', null, (payload) => { + if (disposed || !payload || typeof payload !== 'object') return + const evt = payload as { type?: string; snapshot?: AccountsSnapshot } + if ((evt.type === 'ready' || evt.type === 'snapshot') && evt.snapshot) { + const snap = evt.snapshot + setAccountsByHost((prev) => ({ ...prev, [host.id]: snap })) + } + }) + } if (!statsFetched) { statsFetched = true fetchStats(client, setStats, () => disposed) fetchWorktreeInfo(client, host.id, setWorktreeInfo, () => disposed) } - } else if (unsubNotif) { - unsubNotif() - unsubNotif = null + } else { + if (unsubNotif) { + unsubNotif() + unsubNotif = null + } + if (unsubAccounts) { + unsubAccounts() + unsubAccounts = null + } } }) notifCleanups.push(() => { unsubState() unsubNotif?.() + unsubAccounts?.() }) return [{ hostId: host.id, client }] @@ -259,12 +356,20 @@ export default function HomeScreen() { // Why: prefer the worktree the user last opened on this device so the // "Resume" card reflects their mobile session history, not just the // desktop's most-recently-outputting worktree. + // Why: rendering used to be gated on hostStates === 'connected', which + // caused the Resume card to vanish for ~1s on every cold-start / + // resume-from-background while the WebSocket reconnected, even though we + // had perfectly good cached worktree data. Now the card stays visible as + // long as we have a cached lastActiveWorktree for any known host; the + // tap target is still the same and a fresher snapshot from the live RPC + // overwrites the card's contents in place when it lands. const resumeWorktree = useMemo(() => { - if (lastVisited && hostStates[lastVisited.hostId] === 'connected') { + if (lastVisited && sortedHosts.some((h) => h.id === lastVisited.hostId)) { const cached = getCachedWorktrees(lastVisited.hostId) as WorktreeSummary[] | null const match = cached?.find((w) => w.worktreeId === lastVisited.worktreeId) if (match) return { hostId: lastVisited.hostId, worktree: match } } + // Prefer a currently-connected host's data when we have it. for (const host of sortedHosts) { if (hostStates[host.id] !== 'connected') continue const info = worktreeInfo[host.id] @@ -272,6 +377,14 @@ export default function HomeScreen() { return { hostId: host.id, worktree: info.lastActiveWorktree } } } + // Fall back to whichever known host has cached data, regardless of + // current connection state. + for (const host of sortedHosts) { + const info = worktreeInfo[host.id] + if (info?.lastActiveWorktree) { + return { hostId: host.id, worktree: info.lastActiveWorktree } + } + } return null }, [sortedHosts, hostStates, worktreeInfo, lastVisited]) @@ -289,6 +402,23 @@ export default function HomeScreen() { [sortedHosts, hostStates, worktreeInfo] ) + // Why: only show the Account usage section for hosts that have at least + // one Claude or Codex account configured. Render whenever cached data + // exists, regardless of current connection state, so the cards don't + // disappear for ~1s on resume while the WebSocket reconnects. Streamed + // updates from the live RPC overwrite the snapshot in place when ready. + const accountsHosts = useMemo(() => { + const items: Array<{ host: HostProfile; snapshot: AccountsSnapshot }> = [] + for (const host of sortedHosts) { + const snap = accountsByHost[host.id] + if (!snap) continue + const hasClaude = snap.claude.accounts.length > 0 + const hasCodex = snap.codex.accounts.length > 0 + if (hasClaude || hasCodex) items.push({ host, snapshot: snap }) + } + return items + }, [sortedHosts, hostStates, accountsByHost]) + async function handleRename(newName: string) { if (!renameTarget) return try { @@ -335,8 +465,8 @@ export default function HomeScreen() { Connect your desktop - Pair with Orca on your computer to monitor worktrees, watch agents work, and manage - terminals — all from your phone. + Pair with Orca on your computer to check on your agents, jump into any terminal, and + drive work from your phone. router.push('/pair-scan')}> @@ -503,6 +633,85 @@ export default function HomeScreen() { ) : null} + {/* ─── Account usage ─── */} + {accountsHosts.length > 0 ? ( + <> + + Account usage + + {accountsHosts.map(({ host, snapshot }) => { + const claudeActiveId = snapshot.claude.activeAccountId + const claudeActive = + snapshot.claude.accounts.find((a) => a.id === claudeActiveId) ?? null + const codexActiveId = snapshot.codex.activeAccountId + const codexActive = + snapshot.codex.accounts.find((a) => a.id === codexActiveId) ?? null + const showHostName = accountsHosts.length > 1 + return ( + [ + styles.accountsCard, + pressed && styles.hostCardPressed + ]} + onPress={() => router.push(`/h/${host.id}/accounts`)} + > + {showHostName ? ( + + {host.name} + + ) : null} + {(['claude', 'codex'] as ProviderKey[]).map((provider) => { + const active = provider === 'claude' ? claudeActive : codexActive + const accounts = + provider === 'claude' + ? snapshot.claude.accounts + : snapshot.codex.accounts + if (accounts.length === 0) return null + const limits = getActiveProviderRateLimits(snapshot, provider) + const isFetching = + limits?.status === 'fetching' || limits?.status === 'idle' + const unavailable = + limits == null || + limits.status === 'unavailable' || + limits.status === 'error' + return ( + + + {provider === 'claude' ? ( + + ) : ( + + )} + + + + {active?.email ?? 'System default'} + + + + + + + + ) + })} + + ) + })} + + ) : null} + {/* ─── Quick actions ─── */} Quick Actions @@ -511,7 +720,7 @@ export default function HomeScreen() { onPress={() => router.push('/pair-scan')} > - + Pair Desktop @@ -525,7 +734,7 @@ export default function HomeScreen() { }} > - + New Worktree @@ -667,20 +876,21 @@ const styles = StyleSheet.create({ borderWidth: 1, borderColor: colors.borderSubtle, borderRadius: 10, - padding: spacing.md + paddingVertical: 10, + paddingHorizontal: spacing.md }, statIcon: { - width: 30, - height: 30, - borderRadius: 7, + width: 26, + height: 26, + borderRadius: 6, backgroundColor: 'rgba(255,255,255,0.04)', alignItems: 'center', justifyContent: 'center', - marginBottom: 10 + marginBottom: 6 }, statValue: { color: colors.textPrimary, - fontSize: 20, + fontSize: 18, fontWeight: '700', letterSpacing: -0.3 }, @@ -688,7 +898,7 @@ const styles = StyleSheet.create({ color: colors.textMuted, fontSize: 11, fontWeight: '500', - marginTop: 3 + marginTop: 2 }, /* ─── Section heading ─── */ @@ -817,6 +1027,53 @@ const styles = StyleSheet.create({ flex: 1 }, + /* ─── Account usage ─── */ + accountsCard: { + backgroundColor: colors.bgPanel, + borderWidth: 1, + borderColor: colors.borderSubtle, + borderRadius: radii.card, + paddingHorizontal: spacing.md, + paddingVertical: spacing.sm + 2, + gap: spacing.sm, + marginBottom: spacing.sm + }, + accountsHostLabel: { + fontSize: 11, + color: colors.textMuted, + fontWeight: '500', + textTransform: 'uppercase', + letterSpacing: 0.4 + }, + accountsRow: { + flexDirection: 'row', + alignItems: 'center', + gap: spacing.sm + 2 + }, + accountsIcon: { + width: 32, + height: 32, + borderRadius: 9, + backgroundColor: colors.bgRaised, + alignItems: 'center', + justifyContent: 'center' + }, + accountsInfo: { + flex: 1, + minWidth: 0, + gap: 2 + }, + accountsEmail: { + fontSize: 13, + fontWeight: '600', + color: colors.textPrimary + }, + accountsBars: { + flexDirection: 'row', + gap: spacing.md, + marginTop: 4 + }, + /* ─── Skeleton ─── */ skeletonBlock: { backgroundColor: colors.bgRaised, @@ -836,18 +1093,20 @@ const styles = StyleSheet.create({ }, quickAction: { flex: 1, + flexDirection: 'row', backgroundColor: colors.bgPanel, borderWidth: 1, borderColor: colors.borderSubtle, borderRadius: radii.card, - padding: spacing.lg, + paddingVertical: 10, + paddingHorizontal: 12, alignItems: 'center', gap: 10 }, quickActionIcon: { - width: 40, - height: 40, - borderRadius: 12, + width: 28, + height: 28, + borderRadius: 9, backgroundColor: 'rgba(255,255,255,0.04)', alignItems: 'center', justifyContent: 'center' @@ -855,8 +1114,7 @@ const styles = StyleSheet.create({ quickActionLabel: { fontSize: 12, fontWeight: '600', - color: colors.textSecondary, - textAlign: 'center' + color: colors.textSecondary }, /* ─── Empty state ─── */ diff --git a/mobile/app/pair-confirm.tsx b/mobile/app/pair-confirm.tsx new file mode 100644 index 00000000000..fefa84ee053 --- /dev/null +++ b/mobile/app/pair-confirm.tsx @@ -0,0 +1,212 @@ +import { useEffect, useState } from 'react' +import { View, Text, StyleSheet, Pressable, ActivityIndicator } from 'react-native' +import { useSafeAreaInsets } from 'react-native-safe-area-context' +import { useLocalSearchParams, useRouter } from 'expo-router' +import { ChevronLeft } from 'lucide-react-native' +import { parsePairingCode } from '../src/transport/pairing' +import { connect } from '../src/transport/rpc-client' +import { saveHost, getNextHostName } from '../src/transport/host-store' +import type { PairingOffer, RpcResponse } from '../src/transport/types' +import { colors, spacing, radii, typography } from '../src/theme/mobile-theme' + +type Status = 'awaiting-confirm' | 'connecting' | 'error' + +export default function PairConfirmScreen() { + const router = useRouter() + const insets = useSafeAreaInsets() + const params = useLocalSearchParams<{ code?: string }>() + const [offer, setOffer] = useState(null) + const [status, setStatus] = useState('awaiting-confirm') + const [errorMessage, setErrorMessage] = useState('') + + useEffect(() => { + if (!params.code) { + setStatus('error') + setErrorMessage('Missing pairing code') + return + } + const parsed = parsePairingCode(params.code) + if (!parsed) { + setStatus('error') + setErrorMessage('Not a valid pairing code') + return + } + setOffer(parsed) + }, [params.code]) + + async function confirm() { + if (!offer) return + setStatus('connecting') + let client: ReturnType | null = null + + // Why: split the try/catch around the network call vs the local save + // so a Keychain or AsyncStorage failure doesn't masquerade as a + // "Cannot connect" error. + let response: RpcResponse + try { + client = connect(offer.endpoint, offer.deviceToken, offer.publicKeyB64) + response = await client.sendRequest('status.get') + client.close() + client = null + } catch (err) { + console.warn('[pair-confirm] connect failed', err) + setStatus('error') + setErrorMessage('Cannot connect — check that your computer is on the same network') + client?.close() + return + } + + if (!response.ok) { + setStatus('error') + setErrorMessage( + response.error.code === 'unauthorized' + ? 'Authentication failed — token may be expired' + : `Server error: ${response.error.message}` + ) + return + } + + try { + const hostId = `host-${Date.now()}` + const hostName = await getNextHostName() + await saveHost({ + id: hostId, + name: hostName, + endpoint: offer.endpoint, + deviceToken: offer.deviceToken, + publicKeyB64: offer.publicKeyB64, + lastConnected: Date.now() + }) + router.replace(`/h/${hostId}`) + } catch (err) { + console.warn('[pair-confirm] save failed', err) + setStatus('error') + setErrorMessage( + `Pairing succeeded but couldn't save the host: ${err instanceof Error ? err.message : String(err)}` + ) + } + } + + function cancel() { + router.replace('/') + } + + const containerPadding = { paddingTop: insets.top + spacing.sm } + + return ( + + + + + + + {offer && status === 'awaiting-confirm' && ( + <> + Pair with this desktop? + + You opened a pairing link from your desktop. Confirm to add it to your hosts. + + void confirm()}> + Pair + + + Cancel + + + )} + + {status === 'connecting' && ( + <> + + Connecting… + + )} + + {status === 'error' && ( + <> + {errorMessage} + + Back to home + + + )} + + + ) +} + +const styles = StyleSheet.create({ + container: { + flex: 1, + backgroundColor: colors.bgBase, + padding: spacing.lg + }, + backButton: { + width: 36, + height: 36, + borderRadius: 18, + alignItems: 'center', + justifyContent: 'center', + marginBottom: spacing.sm + }, + content: { + flex: 1, + justifyContent: 'center', + paddingHorizontal: spacing.sm, + // Why: nudges the centered group slightly above the geometric + // middle so the eye reads it as visually centered above the home + // indicator / nav bar. + paddingBottom: spacing.xl * 2 + }, + title: { + fontSize: typography.titleSize, + fontWeight: '600', + color: colors.textPrimary, + marginBottom: spacing.sm, + textAlign: 'center' + }, + subtitle: { + fontSize: typography.bodySize, + color: colors.textSecondary, + lineHeight: 20, + marginBottom: spacing.xl, + textAlign: 'center' + }, + primaryButton: { + backgroundColor: colors.textPrimary, + paddingHorizontal: spacing.xl, + paddingVertical: spacing.sm + 2, + borderRadius: radii.button, + alignItems: 'center', + marginBottom: spacing.sm + }, + primaryButtonText: { + color: colors.bgBase, + fontSize: typography.bodySize, + fontWeight: '600' + }, + secondaryButton: { + paddingHorizontal: spacing.xl, + paddingVertical: spacing.sm + 2, + borderRadius: radii.button, + alignItems: 'center' + }, + secondaryButtonText: { + color: colors.textSecondary, + fontSize: typography.bodySize, + fontWeight: '500' + }, + connectingText: { + color: colors.textSecondary, + fontSize: typography.bodySize, + marginTop: spacing.lg, + textAlign: 'center' + }, + errorText: { + color: colors.statusRed, + fontSize: typography.bodySize, + textAlign: 'center', + marginBottom: spacing.xl, + lineHeight: 20 + } +}) diff --git a/mobile/app/pair-scan.tsx b/mobile/app/pair-scan.tsx index ca522e7ddae..246d3b5e3a0 100644 --- a/mobile/app/pair-scan.tsx +++ b/mobile/app/pair-scan.tsx @@ -3,12 +3,13 @@ import { View, Text, StyleSheet, Pressable, ActivityIndicator } from 'react-nati import { useSafeAreaInsets } from 'react-native-safe-area-context' import { CameraView, useCameraPermissions } from 'expo-camera' import { useRouter } from 'expo-router' -import { ChevronLeft } from 'lucide-react-native' -import { decodePairingUrl } from '../src/transport/pairing' +import { ChevronLeft, Clipboard as ClipboardIcon } from 'lucide-react-native' +import { decodePairingUrl, parsePairingCode } from '../src/transport/pairing' import { connect } from '../src/transport/rpc-client' import { saveHost, getNextHostName } from '../src/transport/host-store' -import type { PairingOffer } from '../src/transport/types' +import type { PairingOffer, RpcResponse } from '../src/transport/types' import { colors, spacing, radii, typography } from '../src/theme/mobile-theme' +import { TextInputModal } from '../src/components/TextInputModal' function Step({ number, text }: { number: number; text: string }) { return ( @@ -27,6 +28,7 @@ export default function PairScanScreen() { const [permission, requestPermission] = useCameraPermissions() const [status, setStatus] = useState<'scanning' | 'connecting' | 'error'>('scanning') const [errorMessage, setErrorMessage] = useState('') + const [pasteVisible, setPasteVisible] = useState(false) const processingRef = useRef(false) const handleBarCodeScanned = useCallback( @@ -47,32 +49,61 @@ export default function PairScanScreen() { [router] ) + const handlePasteSubmit = useCallback((input: string) => { + setPasteVisible(false) + if (processingRef.current) return + processingRef.current = true + + const offer = parsePairingCode(input) + if (!offer) { + setStatus('error') + setErrorMessage('Not a valid pairing code — copy it from your computer and paste again') + processingRef.current = false + return + } + + void testAndSave(offer) + }, []) + async function testAndSave(offer: PairingOffer) { setStatus('connecting') let client: ReturnType | null = null + // Why: split the try/catch around the network call vs the local save + // so a Keychain or AsyncStorage failure doesn't masquerade as a + // "Cannot connect — same network?" error. Pairing reached the + // desktop fine; the failure is local persistence. + let response: RpcResponse try { client = connect(offer.endpoint, offer.deviceToken, offer.publicKeyB64) - const response = await client.sendRequest('status.get') + response = await client.sendRequest('status.get') client.close() client = null + } catch (err) { + console.warn('[pair] connect failed', err) + setStatus('error') + setErrorMessage('Cannot connect — check that your computer is on the same network') + processingRef.current = false + client?.close() + return + } - if (!response.ok) { - if (response.error.code === 'unauthorized') { - setStatus('error') - setErrorMessage('Authentication failed — token may be expired') - processingRef.current = false - return - } + if (!response.ok) { + if (response.error.code === 'unauthorized') { setStatus('error') - setErrorMessage(`Server error: ${response.error.message}`) + setErrorMessage('Authentication failed — token may be expired') processingRef.current = false return } + setStatus('error') + setErrorMessage(`Server error: ${response.error.message}`) + processingRef.current = false + return + } + try { const hostId = `host-${Date.now()}` const hostName = await getNextHostName() - await saveHost({ id: hostId, name: hostName, @@ -81,14 +112,14 @@ export default function PairScanScreen() { publicKeyB64: offer.publicKeyB64, lastConnected: Date.now() }) - router.replace(`/h/${hostId}`) - } catch { + } catch (err) { + console.warn('[pair] save failed', err) setStatus('error') - setErrorMessage('Cannot connect — check that your computer is on the same network') + setErrorMessage( + `Pairing succeeded but couldn't save the host: ${err instanceof Error ? err.message : String(err)}` + ) processingRef.current = false - } finally { - client?.close() } } @@ -98,7 +129,13 @@ export default function PairScanScreen() { processingRef.current = false } - const containerPadding = { paddingTop: insets.top + spacing.sm } + // Why: bottom inset accounts for Android 3-button nav bars and iOS + // home-indicator areas that would otherwise overlap the 'Or paste + // pairing code' button at the bottom of the scan screen. + const containerPadding = { + paddingTop: insets.top + spacing.sm, + paddingBottom: insets.bottom + spacing.sm + } if (!permission) { return ( @@ -140,20 +177,37 @@ export default function PairScanScreen() { {status === 'scanning' && ( - - - - - - - - - + <> + {/* Why: unmount the camera while the paste sheet is open. The + user has clearly chosen the paste path; keeping the camera + streaming behind a sheet wastes power and looks weird if + they cancel the sheet and the QR was scanned silently in + the meantime. */} + {!pasteVisible && ( + + + + + + + + + + )} + {pasteVisible && } + [styles.pasteButton, pressed && styles.pasteButtonPressed]} + onPress={() => setPasteVisible(true)} + > + + Or paste pairing code + + )} {status === 'connecting' && ( @@ -166,11 +220,34 @@ export default function PairScanScreen() { {status === 'error' && ( {errorMessage} - - Try Again - + + + Try Again + + [ + styles.secondaryButton, + pressed && styles.pasteButtonPressed + ]} + onPress={() => { + retry() + setPasteVisible(true) + }} + > + Paste code instead + + )} + + setPasteVisible(false)} + /> ) } @@ -221,6 +298,14 @@ const styles = StyleSheet.create({ borderRadius: radii.camera, overflow: 'hidden' }, + // Why: holds the layout slot while the camera is unmounted during + // paste, so the bottom action button doesn't snap up to fill the + // empty space. + cameraPlaceholder: { + flex: 1, + backgroundColor: colors.bgPanel, + borderRadius: radii.camera + }, camera: { ...StyleSheet.absoluteFillObject }, @@ -303,5 +388,36 @@ const styles = StyleSheet.create({ color: colors.bgBase, fontSize: typography.bodySize, fontWeight: '600' + }, + pasteButton: { + flexDirection: 'row', + alignItems: 'center', + justifyContent: 'center', + gap: spacing.xs, + marginTop: spacing.md, + paddingVertical: spacing.sm, + borderRadius: radii.button + }, + pasteButtonPressed: { + opacity: 0.6 + }, + pasteButtonText: { + color: colors.textSecondary, + fontSize: typography.bodySize, + fontWeight: '500' + }, + errorActions: { + alignItems: 'center', + gap: spacing.sm + }, + secondaryButton: { + paddingHorizontal: spacing.xl, + paddingVertical: spacing.sm, + borderRadius: radii.button + }, + secondaryButtonText: { + color: colors.textSecondary, + fontSize: typography.bodySize, + fontWeight: '500' } }) diff --git a/mobile/package.json b/mobile/package.json index 6006461c802..02fa1bf57d0 100644 --- a/mobile/package.json +++ b/mobile/package.json @@ -22,6 +22,7 @@ "expo-linking": "^55.0.14", "expo-notifications": "^55.0.21", "expo-router": "^55.0.13", + "expo-secure-store": "^55.0.13", "expo-splash-screen": "^55.0.19", "expo-status-bar": "^55.0.5", "lucide-react-native": "^1.11.0", diff --git a/mobile/pnpm-lock.yaml b/mobile/pnpm-lock.yaml index 11f0e80157f..9a346120f2a 100644 --- a/mobile/pnpm-lock.yaml +++ b/mobile/pnpm-lock.yaml @@ -38,6 +38,9 @@ importers: expo-router: specifier: ^55.0.13 version: 55.0.13(121105c3b042d5e83ab3c1d3b84ed55f) + expo-secure-store: + specifier: ^55.0.13 + version: 55.0.13(expo@55.0.17) expo-splash-screen: specifier: ^55.0.19 version: 55.0.19(expo@55.0.17)(typescript@5.9.3) @@ -3234,6 +3237,11 @@ packages: react-server-dom-webpack: optional: true + expo-secure-store@55.0.13: + resolution: {integrity: sha512-I6r0JNO1Fd4o0Gu7Ixiic7s89lqgdUHq17uBH9y1f/AntoyKn71TdtYJH82RgfsBbu5qNVzrwImmvlANyOlITQ==} + peerDependencies: + expo: '*' + expo-server@55.0.8: resolution: {integrity: sha512-AoV5TKuO4biSzrhe/OVLyInfTT0pV9/OOc/g/oVq5vmCjL8SaSYTkES8PLt+67Tm7VqX+Dn0+kSx1nQcjEKaPw==} engines: {node: '>=20.16.0'} @@ -9565,6 +9573,10 @@ snapshots: - expo-font - supports-color + expo-secure-store@55.0.13(expo@55.0.17): + dependencies: + expo: 55.0.17(@babel/core@7.29.0)(@expo/dom-webview@55.0.5)(@expo/metro-runtime@55.0.10)(expo-router@55.0.13)(react-dom@19.2.5(react@19.2.0))(react-native-webview@13.16.1(react-native@0.83.6(@babel/core@7.29.0)(@react-native/metro-config@0.85.2(@babel/core@7.29.0))(@types/react@19.2.14)(react@19.2.0))(react@19.2.0))(react-native-worklets@0.7.4(@babel/core@7.29.0)(react-native@0.83.6(@babel/core@7.29.0)(@react-native/metro-config@0.85.2(@babel/core@7.29.0))(@types/react@19.2.14)(react@19.2.0))(react@19.2.0))(react-native@0.83.6(@babel/core@7.29.0)(@react-native/metro-config@0.85.2(@babel/core@7.29.0))(@types/react@19.2.14)(react@19.2.0))(react@19.2.0)(typescript@5.9.3) + expo-server@55.0.8: {} expo-splash-screen@55.0.19(expo@55.0.17)(typescript@5.9.3): diff --git a/mobile/src/cache/home-snapshot-cache.ts b/mobile/src/cache/home-snapshot-cache.ts new file mode 100644 index 00000000000..57256be8ea0 --- /dev/null +++ b/mobile/src/cache/home-snapshot-cache.ts @@ -0,0 +1,66 @@ +// Why: persist the data needed to render the home page so cold-start / +// resume-from-background paints instantly with the last known good +// values, then updates in place when fresh RPC data arrives. Without +// this, Resume and Account-usage cards flash empty for ~1s while the +// WebSocket reconnects and the first responses come back. +import AsyncStorage from '@react-native-async-storage/async-storage' +import type { AccountsSnapshot } from '../components/AccountUsage' + +const STORAGE_KEY = 'orca:home-snapshot:v1' + +type WorktreeSummary = { + worktreeId: string + repo: string + branch: string + displayName: string + liveTerminalCount: number + status?: 'working' | 'active' | 'permission' | 'done' | 'inactive' +} + +type HostWorktreeInfo = { + hostId: string + totalWorktrees: number + activeCount: number + lastActiveWorktree: WorktreeSummary | null +} + +export type HomeSnapshot = { + worktreeInfo: Record + accountsByHost: Record + savedAt: number +} + +let memoryCache: HomeSnapshot | null = null +let writeTimer: ReturnType | null = null + +export async function loadHomeSnapshot(): Promise { + if (memoryCache) return memoryCache + try { + const raw = await AsyncStorage.getItem(STORAGE_KEY) + if (!raw) return null + const parsed = JSON.parse(raw) as HomeSnapshot + if ( + typeof parsed !== 'object' || + parsed === null || + typeof parsed.worktreeInfo !== 'object' || + typeof parsed.accountsByHost !== 'object' + ) { + return null + } + memoryCache = parsed + return parsed + } catch { + return null + } +} + +// Why: throttle writes so a flurry of streamed account-snapshot updates +// (one per provider fetch finishing) doesn't hammer AsyncStorage. +export function saveHomeSnapshot(snapshot: HomeSnapshot): void { + memoryCache = snapshot + if (writeTimer) clearTimeout(writeTimer) + writeTimer = setTimeout(() => { + writeTimer = null + void AsyncStorage.setItem(STORAGE_KEY, JSON.stringify(snapshot)).catch(() => {}) + }, 250) +} diff --git a/mobile/src/components/AccountUsage.tsx b/mobile/src/components/AccountUsage.tsx new file mode 100644 index 00000000000..0ff706638b9 --- /dev/null +++ b/mobile/src/components/AccountUsage.tsx @@ -0,0 +1,155 @@ +import { View, Text, StyleSheet, ActivityIndicator } from 'react-native' +import { colors, spacing, typography } from '../theme/mobile-theme' + +// Why: keep these shapes in lockstep with src/shared/types.ts and +// src/shared/rate-limit-types.ts. We don't import from desktop here because +// the mobile bundle must not pull in Electron-coupled type files. +export type RateLimitWindow = { + usedPercent: number + windowMinutes: number + resetsAt: number | null + resetDescription: string | null +} + +export type ProviderRateLimits = { + provider: 'claude' | 'codex' | 'gemini' | 'opencode-go' + session: RateLimitWindow | null + weekly: RateLimitWindow | null + monthly?: RateLimitWindow | null + updatedAt: number + error: string | null + status: 'idle' | 'fetching' | 'ok' | 'error' | 'unavailable' +} + +export type InactiveAccountUsage = { + accountId: string + claude: ProviderRateLimits | null + updatedAt: number + isFetching: boolean +} + +export type ClaudeAccountSummary = { + id: string + email: string + organizationName?: string | null +} + +export type CodexAccountSummary = { + id: string + email: string + workspaceLabel?: string | null +} + +export type AccountsSnapshot = { + claude: { accounts: ClaudeAccountSummary[]; activeAccountId: string | null } + codex: { accounts: CodexAccountSummary[]; activeAccountId: string | null } + rateLimits: { + claude: ProviderRateLimits | null + codex: ProviderRateLimits | null + inactiveClaudeAccounts: InactiveAccountUsage[] + inactiveCodexAccounts: InactiveAccountUsage[] + } +} + +export type ProviderKey = 'claude' | 'codex' + +export function getActiveProviderRateLimits( + snapshot: AccountsSnapshot, + provider: ProviderKey +): ProviderRateLimits | null { + return provider === 'claude' ? snapshot.rateLimits.claude : snapshot.rateLimits.codex +} + +export function getInactiveProviderUsage( + snapshot: AccountsSnapshot, + provider: ProviderKey, + accountId: string +): InactiveAccountUsage | null { + const list = + provider === 'claude' + ? snapshot.rateLimits.inactiveClaudeAccounts + : snapshot.rateLimits.inactiveCodexAccounts + return list.find((u) => u.accountId === accountId) ?? null +} + +// Why: matches desktop StatusBar convention — bars show percent remaining +// (so a fresh account renders full, a depleted one renders empty), not +// percent used. Color thresholds invert accordingly. +export function UsageBar({ + label, + usedPercent, + unavailable, + loading +}: { + label: string + usedPercent: number | null + unavailable: boolean + loading?: boolean +}) { + const remaining = usedPercent == null ? null : Math.max(0, Math.min(100, 100 - usedPercent)) + const barColor = + remaining == null + ? colors.textMuted + : remaining <= 10 + ? colors.statusRed + : remaining <= 30 + ? colors.statusAmber + : colors.statusGreen + return ( + + {label} + + + + {loading ? ( + + ) : ( + + {unavailable || remaining == null ? '—' : `${Math.round(remaining)}%`} + + )} + + ) +} + +const styles = StyleSheet.create({ + usageBar: { + flexDirection: 'row', + alignItems: 'center', + gap: spacing.xs, + flex: 1 + }, + usageLabel: { + fontSize: typography.metaSize, + color: colors.textMuted, + width: 22 + }, + usageTrack: { + flex: 1, + height: 6, + borderRadius: 3, + backgroundColor: colors.bgRaised, + overflow: 'hidden' + }, + usageFill: { + height: '100%', + borderRadius: 3 + }, + usageValue: { + fontSize: typography.metaSize, + color: colors.textSecondary, + width: 36, + textAlign: 'right' + }, + usageSpinner: { + width: 36 + } +}) diff --git a/mobile/src/components/AgentIcons.tsx b/mobile/src/components/AgentIcons.tsx new file mode 100644 index 00000000000..714fe76e0f5 --- /dev/null +++ b/mobile/src/components/AgentIcons.tsx @@ -0,0 +1,30 @@ +import Svg, { Path } from 'react-native-svg' +import { colors } from '../theme/mobile-theme' + +// Why: SVG paths sourced from the desktop codebase +// (src/renderer/src/components/status-bar/icons.tsx) so mobile and desktop +// stay visually identical for Claude/Codex branding. + +export function ClaudeIcon({ size = 16 }: { size?: number }) { + return ( + + + + ) +} + +export function OpenAIIcon({ size = 16, color }: { size?: number; color?: string }) { + return ( + + + + ) +} diff --git a/mobile/src/components/AgentSpinner.tsx b/mobile/src/components/AgentSpinner.tsx index 734cb8bbc6d..673bbff049e 100644 --- a/mobile/src/components/AgentSpinner.tsx +++ b/mobile/src/components/AgentSpinner.tsx @@ -1,15 +1,20 @@ import { useEffect, useRef } from 'react' import { Animated, Easing, StyleSheet, View } from 'react-native' -import { colors } from '../theme/mobile-theme' type WorktreeStatus = 'working' | 'active' | 'permission' | 'done' | 'inactive' +// Why: colors and sizing are 1:1 with the desktop StatusIndicator +// (src/renderer/src/components/sidebar/StatusIndicator.tsx) so the mobile +// worktree list reads identically to the sidebar — same yellow spinner for +// 'working', same emerald dot for 'active'/'done', same neutral-500 @ 40% +// for 'inactive', same red for 'permission'. Diverging palettes here lose +// the design intent ('moving' vs 'alive' vs 'completed') the desktop encodes. const STATUS_COLORS: Record = { - working: colors.statusGreen, - active: colors.statusGreen, - permission: colors.statusRed, - done: '#38bdf8', - inactive: '#666666' + working: '#eab308', + active: '#10b981', + done: '#10b981', + permission: '#ef4444', + inactive: 'rgba(115,115,115,0.4)' } export function AgentSpinner({ status }: { status: WorktreeStatus }) { @@ -39,23 +44,39 @@ export function AgentSpinner({ status }: { status: WorktreeStatus }) { outputRange: ['0deg', '360deg'] }) return ( - + + + ) } - return + return ( + + + + ) } const styles = StyleSheet.create({ + // Why: 12x12 wrapper centered around an 8x8 inner glyph mirrors the + // desktop's `inline-flex h-3 w-3 ... items-center justify-center` shell + // around `size-2` indicator — keeps row height/baseline alignment stable + // across status transitions. + wrapper: { + width: 12, + height: 12, + alignItems: 'center', + justifyContent: 'center' + }, dot: { width: 8, height: 8, borderRadius: 4 }, spinner: { - width: 10, - height: 10, - borderRadius: 5, + width: 8, + height: 8, + borderRadius: 4, borderWidth: 2, borderTopColor: 'transparent' } diff --git a/mobile/src/components/NewWorktreeModal.tsx b/mobile/src/components/NewWorktreeModal.tsx index f0841054739..a7e7d2579b3 100644 --- a/mobile/src/components/NewWorktreeModal.tsx +++ b/mobile/src/components/NewWorktreeModal.tsx @@ -16,6 +16,7 @@ import type { RpcClient } from '../transport/rpc-client' import type { RpcSuccess } from '../transport/types' import { colors, spacing, radii, typography } from '../theme/mobile-theme' import { BottomDrawer } from './BottomDrawer' +import { ClaudeIcon, OpenAIIcon } from './AgentIcons' import { getSuggestedCreatureName } from './worktree-name-suggestion' type Repo = { @@ -91,34 +92,10 @@ const AGENT_COMMANDS: Record = { // ── Agent icons ───────────────────────────────────────────────────── // SVG paths sourced from the desktop codebase: -// Claude & OpenAI: src/renderer/src/components/status-bar/icons.tsx +// Claude & OpenAI: shared in ./AgentIcons.tsx // Pi & Aider: src/renderer/src/lib/agent-catalog.tsx // Agents with a faviconDomain use Google's favicon service (same as desktop). -function ClaudeIcon({ size = 16 }: { size?: number }) { - return ( - - - - ) -} - -function OpenAIIcon({ size = 16 }: { size?: number }) { - return ( - - - - ) -} - function PiIcon({ size = 16 }: { size?: number }) { return ( diff --git a/mobile/src/components/TextInputModal.tsx b/mobile/src/components/TextInputModal.tsx index 70a9f323f56..ae6f3ee3e7f 100644 --- a/mobile/src/components/TextInputModal.tsx +++ b/mobile/src/components/TextInputModal.tsx @@ -41,23 +41,19 @@ export function TextInputModal({ {message ? {message} : null} - - - - - + ? containerHeightPx : window.innerHeight; var cols = Math.floor(vpWidth / cellWidth); - // Why: subtract 2 rows after dividing. The WebView's reported height - // can slightly overstate the usable area (layout timing, subpixel - // rounding, safe-area insets). Subtracting 2 guarantees the last - // visible row plus the shell prompt (which often wraps on narrow - // screens) stays fully above the accessory bar. - var rows = Math.max(8, Math.floor(vpHeight / cellHeight) - 2); + // Why: the rows we report become the PTY's actual row count after the + // server fits to viewport, and xterm renders exactly that many lines + // anchored top-left of the WebView. Subtracting rows here would leave + // dead xterm-background space at the bottom of the container and make + // the last PTY rows visually appear above an "invisible line." Any + // safety margin between the prompt and the accessory bar must come + // from RN layout (terminalFrame's flex bounds), not from undersizing + // the PTY. + var rows = Math.max(8, Math.floor(vpHeight / cellHeight)); notify({ type: 'measure-result', cols: cols, rows: rows }); } diff --git a/mobile/src/transport/host-store.ts b/mobile/src/transport/host-store.ts index 7f35fb742a2..bf1ba57110c 100644 --- a/mobile/src/transport/host-store.ts +++ b/mobile/src/transport/host-store.ts @@ -1,16 +1,76 @@ import AsyncStorage from '@react-native-async-storage/async-storage' -import { HostProfileSchema, type HostProfile } from './types' +import * as SecureStore from 'expo-secure-store' +import { + HostProfileSchema, + StoredHostProfileSchema, + type HostProfile, + type StoredHostProfile +} from './types' const STORAGE_KEY = 'orca:hosts' +// Why: SecureStore keys must match [A-Za-z0-9._-]; colons are rejected. +// Use dots as the separator so the key shape stays readable while +// satisfying the validator. +const TOKEN_KEY_PREFIX = 'orca.host-token.' + +// Why: WHEN_UNLOCKED_THIS_DEVICE_ONLY keeps the pairing token off +// iCloud Keychain and out of iCloud/iTunes backup restores onto a +// different physical device. Reads/writes are silent (no biometric +// prompt) since we don't request access control flags. +const KEYCHAIN_OPTIONS: SecureStore.SecureStoreOptions = { + keychainAccessible: SecureStore.WHEN_UNLOCKED_THIS_DEVICE_ONLY +} + +function tokenKey(hostId: string): string { + return `${TOKEN_KEY_PREFIX}${hostId}` +} export async function loadHosts(): Promise { + const raw = await AsyncStorage.getItem(STORAGE_KEY) + if (!raw) return [] + let parsed: unknown + try { + parsed = JSON.parse(raw) + } catch { + return [] + } + if (!Array.isArray(parsed)) return [] + + const out: HostProfile[] = [] + for (const item of parsed) { + // Why: pre-v0.0.3 records carry the deviceToken in AsyncStorage. + // Drop them silently — the three pre-launch users will re-pair on + // first run rather than carry a migration shim through the auth + // path. + if (item && typeof item === 'object' && 'deviceToken' in item) { + continue + } + const stored = StoredHostProfileSchema.safeParse(item) + if (!stored.success) continue + + const token = await SecureStore.getItemAsync(tokenKey(stored.data.id), KEYCHAIN_OPTIONS) + if (!token) { + // Why: orphaned metadata with no matching keychain entry — most + // likely a stale record from a development install. Skip it + // rather than surface a half-broken host. + continue + } + out.push({ ...stored.data, deviceToken: token }) + } + return out +} + +async function loadStoredHosts(): Promise { const raw = await AsyncStorage.getItem(STORAGE_KEY) if (!raw) return [] try { const parsed = JSON.parse(raw) as unknown if (!Array.isArray(parsed)) return [] return parsed.flatMap((item) => { - const result = HostProfileSchema.safeParse(item) + // Why: same drop-old-records rule as loadHosts; keeps internal + // mutators from re-persisting pre-v0.0.3 entries. + if (item && typeof item === 'object' && 'deviceToken' in item) return [] + const result = StoredHostProfileSchema.safeParse(item) return result.success ? [result.data] : [] }) } catch { @@ -18,25 +78,39 @@ export async function loadHosts(): Promise { } } -export async function saveHost(host: HostProfile): Promise { - const hosts = await loadHosts() - const index = hosts.findIndex((h) => h.id === host.id) - if (index >= 0) { - hosts[index] = host - } else { - hosts.push(host) +function toStored(host: HostProfile): StoredHostProfile { + return { + id: host.id, + name: host.name, + endpoint: host.endpoint, + publicKeyB64: host.publicKeyB64, + lastConnected: host.lastConnected } +} + +export async function saveHost(host: HostProfile): Promise { + const validated = HostProfileSchema.parse(host) + const hosts = await loadStoredHosts() + const stored = toStored(validated) + const index = hosts.findIndex((h) => h.id === stored.id) + if (index >= 0) { + hosts[index] = stored + } else { + hosts.push(stored) + } + await SecureStore.setItemAsync(tokenKey(stored.id), validated.deviceToken, KEYCHAIN_OPTIONS) await AsyncStorage.setItem(STORAGE_KEY, JSON.stringify(hosts)) } export async function removeHost(hostId: string): Promise { - const hosts = await loadHosts() + const hosts = await loadStoredHosts() const filtered = hosts.filter((h) => h.id !== hostId) await AsyncStorage.setItem(STORAGE_KEY, JSON.stringify(filtered)) + await SecureStore.deleteItemAsync(tokenKey(hostId), KEYCHAIN_OPTIONS) } export async function renameHost(hostId: string, newName: string): Promise { - const hosts = await loadHosts() + const hosts = await loadStoredHosts() const host = hosts.find((h) => h.id === hostId) if (host) { host.name = newName @@ -45,7 +119,7 @@ export async function renameHost(hostId: string, newName: string): Promise } export async function getNextHostName(): Promise { - const hosts = await loadHosts() + const hosts = await loadStoredHosts() const existingNumbers = hosts .map((h) => { const match = h.name.match(/^Host (\d+)$/) @@ -57,7 +131,7 @@ export async function getNextHostName(): Promise { } export async function updateLastConnected(hostId: string): Promise { - const hosts = await loadHosts() + const hosts = await loadStoredHosts() const host = hosts.find((h) => h.id === hostId) if (host) { host.lastConnected = Date.now() diff --git a/mobile/src/transport/pairing.ts b/mobile/src/transport/pairing.ts index ada3e2715eb..2b54f4e58db 100644 --- a/mobile/src/transport/pairing.ts +++ b/mobile/src/transport/pairing.ts @@ -1,16 +1,38 @@ import { PairingOfferSchema, type PairingOffer } from './types' +// Why: this file mirrors src/shared/pairing.ts (which is covered by CI +// vitest) but uses atob/btoa because Metro/Hermes don't ship Node's +// Buffer. Keep the parsing semantics in sync — when one changes, update +// the other. + export function decodePairingUrl(url: string): PairingOffer | null { try { const hashIndex = url.indexOf('#') if (!url.startsWith('orca://pair') || hashIndex === -1) return null - - const base64url = url.slice(hashIndex + 1) - const base64 = base64url.replace(/-/g, '+').replace(/_/g, '/') - const json = atob(base64) - const parsed = JSON.parse(json) - return PairingOfferSchema.parse(parsed) + return decodePairingBase64(url.slice(hashIndex + 1)) } catch { return null } } + +// Why: accept either an `orca://pair#` URL or the bare base64 +// string so the paste-pair flow can take whichever the user actually +// copied from desktop. +export function parsePairingCode(input: string): PairingOffer | null { + const trimmed = input.trim() + if (!trimmed) return null + try { + if (trimmed.startsWith('orca://pair')) { + return decodePairingUrl(trimmed) + } + return decodePairingBase64(trimmed) + } catch { + return null + } +} + +function decodePairingBase64(base64url: string): PairingOffer { + const base64 = base64url.replace(/-/g, '+').replace(/_/g, '/') + const json = atob(base64) + return PairingOfferSchema.parse(JSON.parse(json)) +} diff --git a/mobile/src/transport/types.ts b/mobile/src/transport/types.ts index c73489a9751..2e9e8fcf7ff 100644 --- a/mobile/src/transport/types.ts +++ b/mobile/src/transport/types.ts @@ -60,3 +60,16 @@ export const HostProfileSchema = z.object({ publicKeyB64: z.string().min(1), lastConnected: z.number().finite() }) + +// Why: persisted host record after the v0.0.3 keychain split. The +// deviceToken is held in iOS Keychain via expo-secure-store and joined +// in at load time; it must NOT appear in AsyncStorage anymore. +export const StoredHostProfileSchema = z.object({ + id: z.string().min(1), + name: z.string().min(1), + endpoint: z.string().min(1), + publicKeyB64: z.string().min(1), + lastConnected: z.number().finite() +}) + +export type StoredHostProfile = z.infer diff --git a/package.json b/package.json index bb66a7f2a06..54e17650bc6 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "orca", - "version": "1.3.35-rc.3", + "version": "1.3.35-rc.7", "description": "Next-gen IDE for parallel agentic development", "homepage": "https://github.com/stablyai/orca", "author": "stablyai", @@ -104,7 +104,7 @@ "remark-gfm": "^4.0.1", "remark-math": "^6.0.0", "shadcn": "^4.1.0", - "simple-git": "^3.33.0", + "simple-git": "^3.36.0", "sonner": "^2.0.7", "ssh2": "^1.17.0", "tailwind-merge": "^3.5.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 7f0204d0552..0c995a70eb2 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -206,8 +206,8 @@ importers: specifier: ^4.1.0 version: 4.1.0(@types/node@25.5.0)(typescript@5.9.3) simple-git: - specifier: ^3.33.0 - version: 3.33.0 + specifier: ^3.36.0 + version: 3.36.0 sonner: specifier: ^2.0.7 version: 2.0.7(react-dom@19.2.4(react@19.2.4))(react@19.2.4) @@ -2338,6 +2338,12 @@ packages: '@sec-ant/readable-stream@0.4.1': resolution: {integrity: sha512-831qok9r2t8AlxLko40y2ebgSDhenenCatLVeW/uBtnHPyhHOvG0C7TvfgecV+wHzIm5KUICgzmVpWS+IMEAeg==} + '@simple-git/args-pathspec@1.0.3': + resolution: {integrity: sha512-ngJMaHlsWDTfjyq9F3VIQ8b7NXbBLq5j9i5bJ6XLYtD6qlDXT7fdKY2KscWWUF8t18xx052Y/PUO1K1TRc9yKA==} + + '@simple-git/argv-parser@1.1.1': + resolution: {integrity: sha512-Q9lBcfQ+VQCpQqGJFHe5yooOS5hGdLFFbJ5R+R5aDsnkPCahtn1hSkMcORX65J2Z5lxSkD0lQorMsncuBQxYUw==} + '@sindresorhus/is@4.6.0': resolution: {integrity: sha512-t09vSN3MdfsyCHoFcTRCH/iUtG7OJ0CsjzB8cjAmKc/va/kIgeDI/TxsigdncE/4be734m0cvIYwNaV4i2XqAw==} engines: {node: '>=10'} @@ -5797,8 +5803,8 @@ packages: simple-get@4.0.1: resolution: {integrity: sha512-brv7p5WgH0jmQJr1ZDDfKDOSeWWg+OVypG99A/5vYGPqJ6pxiaHLy8nxtFjBA7oMa01ebA9gfh1uMCFqOuXxvA==} - simple-git@3.33.0: - resolution: {integrity: sha512-D4V/tGC2sjsoNhoMybKyGoE+v8A60hRawKQ1iFRA1zwuDgGZCBJ4ByOzZ5J8joBbi4Oam0qiPH+GhzmSBwbJng==} + simple-git@3.36.0: + resolution: {integrity: sha512-cGQjLjK8bxJw4QuYT7gxHw3/IouVESbhahSsHrX97MzCL1gu2u7oy38W6L2ZIGECEfIBG4BabsWDPjBxJENv9Q==} simple-update-notifier@2.0.0: resolution: {integrity: sha512-a2B9Y0KlNXl9u/vsW6sTIu9vGEpfKu2wRV6l1H3XEas/0gUIzGzBoP/IouTcUQbm9JWZLH3COxyn03TYlFax6w==} @@ -8314,6 +8320,12 @@ snapshots: '@sec-ant/readable-stream@0.4.1': {} + '@simple-git/args-pathspec@1.0.3': {} + + '@simple-git/argv-parser@1.1.1': + dependencies: + '@simple-git/args-pathspec': 1.0.3 + '@sindresorhus/is@4.6.0': {} '@sindresorhus/merge-streams@4.0.0': {} @@ -12503,10 +12515,12 @@ snapshots: once: 1.4.0 simple-concat: 1.0.1 - simple-git@3.33.0: + simple-git@3.36.0: dependencies: '@kwsites/file-exists': 1.1.1 '@kwsites/promise-deferred': 1.1.1 + '@simple-git/args-pathspec': 1.0.3 + '@simple-git/argv-parser': 1.1.1 debug: 4.4.3 transitivePeerDependencies: - supports-color diff --git a/src/cli/args.ts b/src/cli/args.ts index b97308b327b..7f13996deb3 100644 --- a/src/cli/args.ts +++ b/src/cli/args.ts @@ -64,11 +64,7 @@ export function supportsBrowserPageFlag(commandPath: string[]): boolean { if (['repo', 'worktree', 'terminal'].includes(commandPath[0])) { return false } - return ![ - 'tab list', - 'tab create', - 'tab current' - ].includes(joined) + return !['tab list', 'tab create', 'tab current'].includes(joined) } export function isCommandGroup(commandPath: string[]): boolean { diff --git a/src/cli/handlers/browser-tab.ts b/src/cli/handlers/browser-tab.ts index 01d6a6e5173..0fe3e151b4f 100644 --- a/src/cli/handlers/browser-tab.ts +++ b/src/cli/handlers/browser-tab.ts @@ -5,13 +5,12 @@ import type { BrowserTabSwitchResult } from '../../shared/runtime-types' import type { CommandHandler } from '../dispatch' +import { formatTabList, formatTabListWithProfiles, formatTabShow, printResult } from '../format' import { - formatTabList, - formatTabListWithProfiles, - formatTabShow, - printResult -} from '../format' -import { getOptionalNonNegativeIntegerFlag, getOptionalStringFlag, getRequiredStringFlag } from '../flags' + getOptionalNonNegativeIntegerFlag, + getOptionalStringFlag, + getRequiredStringFlag +} from '../flags' import { RuntimeClientError } from '../runtime-client' import { getBrowserCommandTarget, getBrowserWorktreeSelector } from '../selectors' diff --git a/src/main/git/remove-worktree.test.ts b/src/main/git/remove-worktree.test.ts index 97f8eee820b..e948428fbda 100644 --- a/src/main/git/remove-worktree.test.ts +++ b/src/main/git/remove-worktree.test.ts @@ -87,7 +87,7 @@ describe('removeWorktree', () => { it('removes the worktree, prunes stale refs, and deletes its local branch', async () => { mockGitCommands({ - 'git worktree list --porcelain -z': { + 'git worktree list --porcelain': { stdout: `worktree /repo HEAD abc123 branch refs/heads/main @@ -97,7 +97,7 @@ HEAD def456 branch refs/heads/feature/test ` }, - 'git worktree list --porcelain -z#2': { + 'git worktree list --porcelain#2': { stdout: `worktree /repo HEAD abc123 branch refs/heads/main @@ -121,7 +121,7 @@ branch refs/heads/main it('skips branch deletion when another worktree still points at the branch', async () => { mockGitCommands({ - 'git worktree list --porcelain -z': { + 'git worktree list --porcelain': { stdout: `worktree /repo HEAD abc123 branch refs/heads/main @@ -135,7 +135,7 @@ HEAD def456 branch refs/heads/feature/test ` }, - 'git worktree list --porcelain -z#2': { + 'git worktree list --porcelain#2': { stdout: `worktree /repo HEAD abc123 branch refs/heads/main @@ -154,7 +154,7 @@ branch refs/heads/feature/test expect.arrayContaining([ 'git worktree remove /repo-feature', 'git worktree prune', - 'git worktree list --porcelain -z' + 'git worktree list --porcelain' ]) ) expect(calls).not.toContain('git branch -D feature/test') @@ -163,7 +163,7 @@ branch refs/heads/feature/test it('deletes the branch after prune removes stale sibling worktree entries', async () => { mockGitCommands({ - 'git worktree list --porcelain -z': { + 'git worktree list --porcelain': { stdout: `worktree /repo HEAD abc123 branch refs/heads/main @@ -178,7 +178,7 @@ branch refs/heads/feature/test prunable gitdir file points to non-existent location ` }, - 'git worktree list --porcelain -z#2': { + 'git worktree list --porcelain#2': { stdout: `worktree /repo HEAD abc123 branch refs/heads/main @@ -201,7 +201,7 @@ branch refs/heads/main it('passes --force before the worktree path when forced removal is requested', async () => { mockGitCommands({ - 'git worktree list --porcelain -z': { + 'git worktree list --porcelain': { stdout: `worktree /repo HEAD abc123 branch refs/heads/main @@ -211,7 +211,7 @@ HEAD def456 branch refs/heads/feature/test ` }, - 'git worktree list --porcelain -z#2': { + 'git worktree list --porcelain#2': { stdout: `worktree /repo HEAD abc123 branch refs/heads/main @@ -226,7 +226,7 @@ branch refs/heads/main it('matches Windows worktree paths before deleting the branch', async () => { mockGitCommands({ - 'git worktree list --porcelain -z': { + 'git worktree list --porcelain': { stdout: `worktree C:/repo HEAD abc123 branch refs/heads/main @@ -236,7 +236,7 @@ HEAD def456 branch refs/heads/feature/test ` }, - 'git worktree list --porcelain -z#2': { + 'git worktree list --porcelain#2': { stdout: `worktree C:/repo HEAD abc123 branch refs/heads/main @@ -259,7 +259,7 @@ branch refs/heads/main it('keeps removal successful when branch cleanup fails', async () => { const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) mockGitCommands({ - 'git worktree list --porcelain -z': { + 'git worktree list --porcelain': { stdout: `worktree /repo HEAD abc123 branch refs/heads/main @@ -269,7 +269,7 @@ HEAD def456 branch refs/heads/feature/test ` }, - 'git worktree list --porcelain -z#2': { + 'git worktree list --porcelain#2': { stdout: `worktree /repo HEAD abc123 branch refs/heads/main @@ -306,16 +306,15 @@ describe('listWorktrees', () => { resolveGitDirMock.mockImplementation(async (worktreePath: string) => `${worktreePath}/.git`) }) - it('translates parsed path fields from NUL-delimited porcelain output', async () => { + it('translates parsed path fields from line-block porcelain output', async () => { gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: - 'worktree /home/me/repo\0HEAD abc123\0branch refs/heads/main\0\0' + - 'worktree /home/me/repo-feature\0HEAD def456\0branch refs/heads/feature/test\0sparse\0\0' - }) - translateWslOutputPathsMock.mockImplementation((output: string) => { - expect(output).not.toContain('\0') - return output.replace('/home/me/', '\\\\wsl.localhost\\Ubuntu\\home\\me\\') + 'worktree /home/me/repo\nHEAD abc123\nbranch refs/heads/main\n\n' + + 'worktree /home/me/repo-feature\nHEAD def456\nbranch refs/heads/feature/test\nsparse\n\n' }) + translateWslOutputPathsMock.mockImplementation((output: string) => + output.replace('/home/me/', '\\\\wsl.localhost\\Ubuntu\\home\\me\\') + ) await expect(listWorktrees('\\\\wsl.localhost\\Ubuntu\\home\\me\\repo')).resolves.toEqual([ { @@ -337,27 +336,26 @@ describe('listWorktrees', () => { // Why: the non-sparse main worktree gets an fs probe of its sparse config // file; the linked worktree short-circuits on the parsed `sparse` token and // does not. Only one git subprocess runs regardless of worktree count. - expect(getGitCalls()).toEqual(['git worktree list --porcelain -z']) + expect(getGitCalls()).toEqual(['git worktree list --porcelain']) expect(statMock).toHaveBeenCalledTimes(1) expect(translateWslOutputPathsMock).toHaveBeenCalledTimes(2) }) it('detects sparse checkout after translating paths when porcelain omits sparse token', async () => { gitExecFileAsyncMock.mockImplementation((args: string[]) => { - if (args.join(' ') === 'worktree list --porcelain -z') { + if (args.join(' ') === 'worktree list --porcelain') { return { stdout: - 'worktree /home/me/repo\0HEAD abc123\0branch refs/heads/main\0\0' + - 'worktree /home/me/repo-feature\0HEAD def456\0branch refs/heads/feature/test\0\0', + 'worktree /home/me/repo\nHEAD abc123\nbranch refs/heads/main\n\n' + + 'worktree /home/me/repo-feature\nHEAD def456\nbranch refs/heads/feature/test\n\n', stderr: '' } } throw new Error(`Unexpected git call: ${args.join(' ')}`) }) - translateWslOutputPathsMock.mockImplementation((output: string) => { - expect(output).not.toContain('\0') - return output.replace('/home/me/', '\\\\wsl.localhost\\Ubuntu\\home\\me\\') - }) + translateWslOutputPathsMock.mockImplementation((output: string) => + output.replace('/home/me/', '\\\\wsl.localhost\\Ubuntu\\home\\me\\') + ) const featureWorktreePath = '\\\\wsl.localhost\\Ubuntu\\home\\me\\repo-feature' resolveGitDirMock.mockImplementation(async (worktreePath: string) => worktreePath === featureWorktreePath @@ -394,7 +392,7 @@ describe('listWorktrees', () => { // Why: the detection path must not spawn a git subprocess per worktree — // the perf regression in #1131 came from `git sparse-checkout list` firing // on every poll. - expect(getGitCalls()).toEqual(['git worktree list --porcelain -z']) + expect(getGitCalls()).toEqual(['git worktree list --porcelain']) }) }) @@ -428,7 +426,7 @@ describe('addSparseWorktree', () => { 'git sparse-checkout set -- packages/web': { error: new Error('sparse setup failed') }, - 'git worktree list --porcelain -z': { + 'git worktree list --porcelain': { stdout: `worktree /repo HEAD abc123 branch refs/heads/main @@ -438,7 +436,7 @@ HEAD def456 branch refs/heads/feature/test ` }, - 'git worktree list --porcelain -z#2': { + 'git worktree list --porcelain#2': { stdout: `worktree /repo HEAD abc123 branch refs/heads/main diff --git a/src/main/git/runner.test.ts b/src/main/git/runner.test.ts index 637de12f42d..e83af8c115f 100644 --- a/src/main/git/runner.test.ts +++ b/src/main/git/runner.test.ts @@ -41,12 +41,10 @@ describe('isTransientGhError', () => { // Why: when GitHub returns Retry-After, the server is telling us how long // to wait. Retrying on our 250ms cadence just earns another 429 and burns // the retry budget. - expect( - isTransientGhError('HTTP 429 Too Many Requests\nRetry-After: 60\n') - ).toBe(false) + expect(isTransientGhError('HTTP 429 Too Many Requests\nRetry-After: 60\n')).toBe(false) }) - it('does NOT retry 4xx that aren\'t 429', () => { + it("does NOT retry 4xx that aren't 429", () => { expect(isTransientGhError('HTTP 401 Unauthorized')).toBe(false) expect(isTransientGhError('HTTP 404 Not Found')).toBe(false) expect(isTransientGhError('HTTP 422 Unprocessable Entity')).toBe(false) diff --git a/src/main/git/runner.ts b/src/main/git/runner.ts index 1be5acc9415..c43058504b4 100644 --- a/src/main/git/runner.ts +++ b/src/main/git/runner.ts @@ -6,13 +6,7 @@ * This module detects WSL paths and routes command execution through `wsl.exe -d ` * with translated Linux paths, so every call site gets WSL support for free. */ -import { - execFile, - execFileSync, - spawn, - type ChildProcess, - type SpawnOptions -} from 'child_process' +import { execFile, execFileSync, spawn, type ChildProcess, type SpawnOptions } from 'child_process' import { promisify } from 'util' import { parseWslPath, toWindowsWslPath, type WslPathInfo } from '../wsl' @@ -99,9 +93,7 @@ function resolveCommand( // inside the bash -c string. Single quotes are safe for all chars except // single quotes themselves, which we escape as '\'' (end quote, escaped // literal, reopen quote). - const escapedArgs = translatedArgs.map( - (a) => `'${a.replace(/'/g, "'\\''")}'` - ) + const escapedArgs = translatedArgs.map((a) => `'${a.replace(/'/g, "'\\''")}'`) // Why: when cwd is supplied as a WSL UNC path, prepend `cd &&` // so the command runs in the expected directory. When the caller only // supplied a distro override (no cwd), skip the cd entirely — the gh CLI @@ -193,10 +185,7 @@ export function gitExecFileSync( * Spawn a git child process. Drop-in replacement for * `spawn('git', args, { cwd, stdio, ... })`. */ -export function gitSpawn( - args: string[], - options: SpawnOptions & { cwd: string } -): ChildProcess { +export function gitSpawn(args: string[], options: SpawnOptions & { cwd: string }): ChildProcess { const resolved = resolveCommand('git', args, options.cwd) return spawn(resolved.binary, resolved.args, { ...options, @@ -216,7 +205,69 @@ export function gitSpawn( // resolveCommand routes the spawn through `wsl.exe -d -- gh ...` // even without a UNC cwd to parse a distro from. Repo-scoped callers should // keep using cwd — the distro derives from the path automatically there. -type GhExecOptions = Omit & { cwd?: string; wslDistro?: string } +// Why: `idempotent` gates the transient-error retry. When undefined we +// auto-detect from argv (writes are detected by `-X POST/PATCH/PUT/DELETE` +// or a `query=mutation …` arg); callers can also pass an explicit override. +// A 5xx/socket reset after the request reaches GitHub but before the +// response returns is the canonical case where the server-side write +// succeeded; retrying would create a duplicate comment/issue/label addition. +// See bug-scan finding 1. +type GhExecOptions = Omit & { + cwd?: string + wslDistro?: string + idempotent?: boolean +} + +const NON_IDEMPOTENT_METHODS = new Set(['POST', 'PATCH', 'PUT', 'DELETE']) +// `gh ` write subcommands. Reads (view/list/status/checks) +// are absent on purpose so the default of "retry" stays for them. +const NON_IDEMPOTENT_GH_VERBS = new Set([ + 'create', + 'edit', + 'delete', + 'close', + 'reopen', + 'merge', + 'comment', + 'review', + 'ready', + 'lock', + 'unlock', + 'pin', + 'unpin', + 'transfer', + 'develop' +]) + +function argsLookIdempotent(args: string[]): boolean { + for (let i = 0; i < args.length; i++) { + const a = args[i] + if (a === '-X' || a === '--method') { + const next = args[i + 1] + if (typeof next === 'string' && NON_IDEMPOTENT_METHODS.has(next.toUpperCase())) { + return false + } + } + // `gh api graphql -f query=mutation(...){ ... }` — detect mutation queries + // so writes via the GraphQL endpoint also fail fast on transient errors. + if (a.startsWith('query=')) { + const trimmed = a.slice('query='.length).trimStart().toLowerCase() + if (trimmed.startsWith('mutation')) { + return false + } + } + } + // `gh issue close`, `gh pr edit`, `gh pr merge`, etc. The first arg is the + // noun (issue/pr/repo/label/...) and the second is the verb. Defaulting + // `gh api` calls without an explicit -X to GET-equivalent (idempotent) is + // intentional: callers that POST through `gh api` set `-X POST`. + if (args.length >= 2 && args[0] !== 'api') { + if (NON_IDEMPOTENT_GH_VERBS.has(args[1])) { + return false + } + } + return true +} /** * Extract stderr from an execFile rejection. @@ -355,7 +406,14 @@ export async function ghExecFileAsync( lastError = err const { stderr } = extractExecError(err) const isLastAttempt = attempt >= GH_RETRY_DELAYS_MS.length - if (!isLastAttempt && isTransientGhError(stderr)) { + // Why: only retry idempotent calls. A 5xx/socket reset can arrive + // after the server already applied a POST/PATCH/PUT/DELETE; retrying + // would duplicate the write (e.g. double-post a comment, double-add + // a label). When the caller doesn't say, we auto-detect from argv — + // explicit `-X ` and GraphQL `query=mutation …` are treated + // as non-idempotent. See bug-scan finding 1. + const idempotent = options.idempotent ?? argsLookIdempotent(args) + if (idempotent && !isLastAttempt && isTransientGhError(stderr)) { // Why: when the upstream surfaced a Retry-After (e.g. on a transient // 5xx that GitHub explicitly recommends backing off for), honor it // instead of using our default backoff — sleeping less than the @@ -406,10 +464,7 @@ export function wslAwareSpawn( * are Linux-native (/home/user/repo). The rest of Orca needs Windows UNC * paths (\\wsl.localhost\Ubuntu\home\user\repo) to read files via Node fs. */ -export function translateWslOutputPaths( - output: string, - originalCwd: string -): string { +export function translateWslOutputPaths(output: string, originalCwd: string): string { const wsl = parseWslPath(originalCwd) if (!wsl) { return output @@ -417,9 +472,8 @@ export function translateWslOutputPaths( // Replace absolute Linux paths that start with / and look like filesystem // paths in structured git output (e.g. "worktree /home/user/repo/feature") - return output.replace( - /(?<=worktree )(\/.+)$/gm, - (_match, linuxPath: string) => toWindowsWslPath(linuxPath, wsl.distro) + return output.replace(/(?<=worktree )(\/.+)$/gm, (_match, linuxPath: string) => + toWindowsWslPath(linuxPath, wsl.distro) ) } diff --git a/src/main/git/worktree.ts b/src/main/git/worktree.ts index b77b89e33f6..1c8ac49bf75 100644 --- a/src/main/git/worktree.ts +++ b/src/main/git/worktree.ts @@ -35,12 +35,10 @@ function looksLikeWindowsPath(pathValue: string): boolean { */ export function parseWorktreeList(output: string): GitWorktreeInfo[] { const worktrees: GitWorktreeInfo[] = [] - const blocks = output.includes('\0') - ? parseNullDelimitedWorktreeBlocks(output) - : output - .trim() - .split(/\r?\n\r?\n/) - .map((block) => block.trim().split(/\r?\n/)) + const blocks = output + .trim() + .split(/\r?\n\r?\n/) + .map((block) => block.trim().split(/\r?\n/)) for (const lines of blocks) { if (lines.length === 0) { @@ -88,11 +86,12 @@ export function parseWorktreeList(output: string): GitWorktreeInfo[] { */ export async function listWorktrees(repoPath: string): Promise { try { - const { stdout } = await gitExecFileAsync(['worktree', 'list', '--porcelain', '-z'], { + // Why: do not pass `-z` here. `-z` requires Git ≥ 2.36; older Git rejects + // it, listWorktrees returns [], and every create flow throws "Worktree + // created but not found in listing" (issue #1453). + const { stdout } = await gitExecFileAsync(['worktree', 'list', '--porcelain'], { cwd: repoPath }) - // Why: WSL path translation is line-oriented, but `-z` porcelain output is - // NUL-delimited. Parse first so only complete path fields are translated. const worktrees = parseWorktreeList(stdout).map((worktree) => { const translatedPath = translateWorktreePath(worktree.path, repoPath) return translatedPath === worktree.path ? worktree : { ...worktree, path: translatedPath } @@ -106,7 +105,11 @@ export async function listWorktrees(repoPath: string): Promise 0) { - blocks.push(current) - current = [] - } - continue - } - current.push(token) - } - - if (current.length > 0) { - blocks.push(current) - } - - return blocks -} - function translateWorktreePath(worktreePath: string, repoPath: string): string { const prefix = 'worktree ' const translated = translateWslOutputPaths(`${prefix}${worktreePath}`, repoPath) diff --git a/src/main/github/client.ts b/src/main/github/client.ts index 81d427efb70..033998d85ac 100644 --- a/src/main/github/client.ts +++ b/src/main/github/client.ts @@ -851,17 +851,21 @@ export async function getWorkItemByOwnerRepo( /** * Get PR info for a given branch using gh CLI. * Returns null if gh is not installed, or no PR exists for the branch. + * + * When `linkedPRNumber` is provided and the branch lookup yields nothing, + * falls back to looking up the PR by number. This handles "create from PR" + * worktrees, whose branch is a fresh local branch (not the PR's head ref) — + * the branch-keyed lookup misses, but the user still expects the linked PR + * to surface on the worktree card. */ -export async function getPRForBranch(repoPath: string, branch: string): Promise { +export async function getPRForBranch( + repoPath: string, + branch: string, + linkedPRNumber?: number | null +): Promise { // Strip refs/heads/ prefix if present const branchName = branch.replace(/^refs\/heads\//, '') - // During a rebase the worktree is in detached HEAD and branch is empty. - // An empty --head filter causes gh to return an arbitrary PR — bail early. - if (!branchName) { - return null - } - await acquire() try { const ownerRepo = await getOwnerRepo(repoPath) @@ -880,38 +884,73 @@ export async function getPRForBranch(repoPath: string, branch: string): Promise< headRefOid?: string } | null = null - if (ownerRepo) { - const { stdout } = await ghExecFileAsync( - [ - 'pr', - 'list', - '--repo', - `${ownerRepo.owner}/${ownerRepo.repo}`, - '--head', - branchName, - '--state', - 'all', - '--limit', - '1', - '--json', - 'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid' - ], - { cwd: repoPath } - ) - const list = JSON.parse(stdout) as NonNullable[] - data = list[0] ?? null - } else { - const { stdout } = await ghExecFileAsync( - [ - 'pr', - 'view', - branchName, - '--json', - 'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid' - ], - { cwd: repoPath } - ) - data = JSON.parse(stdout) + // During a rebase the worktree is in detached HEAD and branch is empty. + // An empty --head filter causes gh to return an arbitrary PR — skip the + // branch lookup and rely on the linkedPR fallback below if available. + if (branchName) { + if (ownerRepo) { + const { stdout } = await ghExecFileAsync( + [ + 'pr', + 'list', + '--repo', + `${ownerRepo.owner}/${ownerRepo.repo}`, + '--head', + branchName, + '--state', + 'all', + '--limit', + '1', + '--json', + 'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid' + ], + { cwd: repoPath } + ) + const list = JSON.parse(stdout) as NonNullable[] + data = list[0] ?? null + } else { + const { stdout } = await ghExecFileAsync( + [ + 'pr', + 'view', + branchName, + '--json', + 'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid' + ], + { cwd: repoPath } + ) + data = JSON.parse(stdout) + } + } + + if (!data && typeof linkedPRNumber === 'number') { + const args = ownerRepo + ? [ + 'pr', + 'view', + String(linkedPRNumber), + '--repo', + `${ownerRepo.owner}/${ownerRepo.repo}`, + '--json', + 'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid' + ] + : [ + 'pr', + 'view', + String(linkedPRNumber), + '--json', + 'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid' + ] + try { + const { stdout } = await ghExecFileAsync(args, { cwd: repoPath }) + data = JSON.parse(stdout) + } catch { + // Why: a stale linkedPRNumber (PR deleted, wrong repo, …) makes + // `gh pr view ` reject. Treat that as the no-PR case so + // callers see the historical `null` semantics instead of a thrown + // error every poll cycle. + data = null + } } if (!data) { diff --git a/src/main/github/project-view.test.ts b/src/main/github/project-view.test.ts index 5630b76156f..383fcb1d412 100644 --- a/src/main/github/project-view.test.ts +++ b/src/main/github/project-view.test.ts @@ -20,9 +20,9 @@ describe('classifyProjectError', () => { }) it('classifies "Could not resolve to a User" as not_found', () => { - expect( - classifyProjectError('Could not resolve to a User with the login of foo', '').type - ).toBe('not_found') + expect(classifyProjectError('Could not resolve to a User with the login of foo', '').type).toBe( + 'not_found' + ) }) it('classifies "could not resolve host" as network_error, NOT not_found', () => { @@ -117,9 +117,12 @@ describe('parseProjectPaste', () => { }) it('parses org URL with view number', () => { - expect( - parseProjectPaste('https://github.com/orgs/acme/projects/42/views/3') - ).toEqual({ kind: 'org', owner: 'acme', number: 42, viewNumber: 3 }) + expect(parseProjectPaste('https://github.com/orgs/acme/projects/42/views/3')).toEqual({ + kind: 'org', + owner: 'acme', + number: 42, + viewNumber: 3 + }) }) it('parses user URL', () => { diff --git a/src/main/github/project-view.ts b/src/main/github/project-view.ts index 549bc989741..1a06723e90c 100644 Binary files a/src/main/github/project-view.ts and b/src/main/github/project-view.ts differ diff --git a/src/main/github/project-view/internals.ts b/src/main/github/project-view/internals.ts index 779741aa568..52509c30652 100644 --- a/src/main/github/project-view/internals.ts +++ b/src/main/github/project-view/internals.ts @@ -82,9 +82,13 @@ export function validateSlugArgs( repo: unknown ): { ok: true } | { ok: false; error: GitHubProjectViewError } { const o = assertSlug(owner, 'owner') - if (!o.ok) {return { ok: false, error: o.error }} + if (!o.ok) { + return { ok: false, error: o.error } + } const r = assertSlug(repo, 'repo') - if (!r.ok) {return { ok: false, error: r.error }} + if (!r.ok) { + return { ok: false, error: r.error } + } return { ok: true } } @@ -103,7 +107,9 @@ export function extractGraphqlErrors(stderr: string, stdout: string): GhGraphqlE // fails, fall back to stderr. const sources = [stdout, stderr] for (const src of sources) { - if (!src) {continue} + if (!src) { + continue + } try { const parsed = JSON.parse(src) as { errors?: GhGraphqlErrorShape[] } if (parsed.errors && parsed.errors.length > 0) { @@ -116,20 +122,23 @@ export function extractGraphqlErrors(stderr: string, stdout: string): GhGraphqlE return [] } -export function errorsIndicateParentField( - errors: GhGraphqlErrorShape[], - stderr: string -): boolean { +export function errorsIndicateParentField(errors: GhGraphqlErrorShape[], stderr: string): boolean { const lower = stderr.toLowerCase() // Preview-header shape: gh returns a 4xx with "preview" in the message. - if (lower.includes('preview') && lower.includes('parent')) {return true} + if (lower.includes('preview') && lower.includes('parent')) { + return true + } return errors.some((e) => { const type = (e.type ?? '').toUpperCase() if (type === 'FIELD_NOT_FOUND' || type === 'UNDEFINED_FIELD' || type === 'FIELD_ERRORS') { const tail = e.path?.at(-1) - if (tail === 'parent') {return true} + if (tail === 'parent') { + return true + } // FIELD_ERRORS often omits `path`; match on message for the parent field. - if ((e.message ?? '').toLowerCase().includes('parent')) {return true} + if ((e.message ?? '').toLowerCase().includes('parent')) { + return true + } } return false }) @@ -140,7 +149,11 @@ export function classifyProjectError(stderr: string, stdout: string): GitHubProj const s = stderr.toLowerCase() // Auth - if (s.includes('authentication required') || s.includes('not logged in') || s.includes('gh auth login')) { + if ( + s.includes('authentication required') || + s.includes('not logged in') || + s.includes('gh auth login') + ) { return { type: 'auth_required', message: 'Sign in to GitHub to load project tasks. Run `gh auth login`.' @@ -149,7 +162,7 @@ export function classifyProjectError(stderr: string, stdout: string): GitHubProj // Scope if ( s.includes('missing required scope') || - s.includes("your token has not been granted") || + s.includes('your token has not been granted') || (s.includes('resource not accessible') && (s.includes('project') || s.includes('scope'))) ) { return { @@ -181,7 +194,11 @@ export function classifyProjectError(stderr: string, stdout: string): GitHubProj if ( s.includes('http 404') || errors.some((e) => (e.type ?? '').toUpperCase() === 'NOT_FOUND') || - s.includes('could not resolve to a ') + // Why: GitHub uses "to an" for vowel-leading types ("to an Issue", "to + // an Organization") and "to a" otherwise. The previous singular-only + // check missed the "an" variants when gh emits only the stderr summary + // without a structured GraphQL error array. See bug-scan finding 3. + /could not resolve to an? /.test(s) ) { const firstNotFound = errors.find((e) => (e.type ?? '').toUpperCase() === 'NOT_FOUND') return { @@ -208,12 +225,16 @@ export function classifyProjectError(stderr: string, stdout: string): GitHubProj // Why: don't leak full stderr to the UI — it can include verbose request // dumps with header diagnostics. Truncate to the first non-empty line and // cap length so unexpected diagnostics stay readable but bounded. - const firstLine = stderr - .split('\n') - .map((l) => l.trim()) - .find((l) => l.length > 0) ?? '' + const firstLine = + stderr + .split('\n') + .map((l) => l.trim()) + .find((l) => l.length > 0) ?? '' const safe = firstLine.length > 200 ? `${firstLine.slice(0, 200)}…` : firstLine - return { type: 'unknown', message: safe ? `GitHub request failed: ${safe}` : 'GitHub request failed.' } + return { + type: 'unknown', + message: safe ? `GitHub request failed: ${safe}` : 'GitHub request failed.' + } } export function driftError( @@ -228,9 +249,11 @@ export function driftError( // same `rate_limited` error shape as the post-hoc classifier so the UI path // is unchanged. We DO NOT fail open here when there's no cached snapshot — // rateLimitGuard already handles that case (returns `blocked:false`). -export function rateLimitedError( - blocked: { remaining: number; limit: number; resetAt: number } -): GitHubProjectViewError { +export function rateLimitedError(blocked: { + remaining: number + limit: number + resetAt: number +}): GitHubProjectViewError { const resetIn = Math.max(0, blocked.resetAt - Math.floor(Date.now() / 1000)) const mins = Math.ceil(resetIn / 60) return { diff --git a/src/main/github/project-view/mutations.ts b/src/main/github/project-view/mutations.ts index 97cdfeb2ad8..cd4e0944fc9 100644 --- a/src/main/github/project-view/mutations.ts +++ b/src/main/github/project-view/mutations.ts @@ -127,7 +127,9 @@ export async function updateProjectItemFieldValue( value: valVar.val } const res = await runGraphql(query, vars) - if (!res.ok) {return { ok: false, error: res.error }} + if (!res.ok) { + return { ok: false, error: res.error } + } return { ok: true } } @@ -151,7 +153,9 @@ export async function clearProjectItemFieldValue( itemId: args.itemId, fieldId: args.fieldId }) - if (!res.ok) {return { ok: false, error: res.error }} + if (!res.ok) { + return { ok: false, error: res.error } + } return { ok: true } } @@ -161,13 +165,18 @@ export async function updateIssueBySlug( args: UpdateIssueBySlugArgs ): Promise { const v = validateSlugArgs(args.owner, args.repo) - if (!v.ok) {return v} + if (!v.ok) { + return v + } const n = assertPositiveInt(args.number, 'number') - if (!n.ok) {return { ok: false, error: n.error }} + if (!n.ok) { + return { ok: false, error: n.error } + } if (!args.updates || typeof args.updates !== 'object') { return { ok: false, error: { type: 'validation_error', message: 'Updates required.' } } } - const { title, body, state, addLabels, removeLabels, addAssignees, removeAssignees } = args.updates + const { title, body, state, addLabels, removeLabels, addAssignees, removeAssignees } = + args.updates // Title / body / state go through PATCH /repos/{owner}/{repo}/issues/{n}. // Labels/assignees go through their dedicated endpoints. @@ -176,11 +185,19 @@ export async function updateIssueBySlug( // 1) PATCH body if (title !== undefined || body !== undefined || state !== undefined) { const patchArgs: string[] = ['-X', 'PATCH', base] - if (title !== undefined) {patchArgs.push('--raw-field', `title=${title}`)} - if (body !== undefined) {patchArgs.push('--raw-field', `body=${body}`)} - if (state !== undefined) {patchArgs.push('--raw-field', `state=${state}`)} + if (title !== undefined) { + patchArgs.push('--raw-field', `title=${title}`) + } + if (body !== undefined) { + patchArgs.push('--raw-field', `body=${body}`) + } + if (state !== undefined) { + patchArgs.push('--raw-field', `state=${state}`) + } const r = await runRest(patchArgs) - if (!r.ok) {return { ok: false, error: r.error }} + if (!r.ok) { + return { ok: false, error: r.error } + } } // 2) Labels — collapse multi-delete fan-out into a single PUT when removing @@ -194,36 +211,49 @@ export async function updateIssueBySlug( if (removeCount > 1) { type RawLabelResp = { name?: string }[] const fetched = await runRest(['-X', 'GET', `${base}/labels`]) - if (!fetched.ok) {return { ok: false, error: fetched.error }} + if (!fetched.ok) { + return { ok: false, error: fetched.error } + } const currentNames = new Set( fetched.data.map((l) => l.name).filter((n): n is string => typeof n === 'string') ) - for (const l of removeLabels ?? []) {currentNames.delete(l)} - for (const l of addLabels ?? []) {currentNames.add(l)} + for (const l of removeLabels ?? []) { + currentNames.delete(l) + } + for (const l of addLabels ?? []) { + currentNames.add(l) + } if (currentNames.size === 0) { // Why: `gh api -X PUT` with no `--raw-field` arguments sends an empty // body — GitHub does NOT interpret that as "clear labels". The // dedicated DELETE endpoint is the documented way to remove all // labels in a single call. - const r = await runRest( - ['-X', 'DELETE', `${base}/labels`], - undefined, - 'core', - { expectEmpty: true } - ) - if (!r.ok && r.error.type !== 'not_found') {return { ok: false, error: r.error }} + const r = await runRest(['-X', 'DELETE', `${base}/labels`], undefined, 'core', { + expectEmpty: true + }) + if (!r.ok && r.error.type !== 'not_found') { + return { ok: false, error: r.error } + } } else { const putArgs = ['-X', 'PUT', `${base}/labels`] - for (const name of currentNames) {putArgs.push('--raw-field', `labels[]=${name}`)} + for (const name of currentNames) { + putArgs.push('--raw-field', `labels[]=${name}`) + } const r = await runRest(putArgs) - if (!r.ok) {return { ok: false, error: r.error }} + if (!r.ok) { + return { ok: false, error: r.error } + } } } else { if (addCount > 0) { const restArgs = ['-X', 'POST', `${base}/labels`] - for (const l of addLabels ?? []) {restArgs.push('--raw-field', `labels[]=${l}`)} + for (const l of addLabels ?? []) { + restArgs.push('--raw-field', `labels[]=${l}`) + } const r = await runRest(restArgs) - if (!r.ok) {return { ok: false, error: r.error }} + if (!r.ok) { + return { ok: false, error: r.error } + } } if (removeCount === 1) { const r = await runRest( @@ -232,7 +262,9 @@ export async function updateIssueBySlug( 'core', { expectEmpty: true } ) - if (!r.ok && r.error.type !== 'not_found') {return { ok: false, error: r.error }} + if (!r.ok && r.error.type !== 'not_found') { + return { ok: false, error: r.error } + } } } @@ -240,15 +272,23 @@ export async function updateIssueBySlug( // add/remove are at most 2 calls regardless of array size. if (addAssignees && addAssignees.length > 0) { const restArgs = ['-X', 'POST', `${base}/assignees`] - for (const u of addAssignees) {restArgs.push('--raw-field', `assignees[]=${u}`)} + for (const u of addAssignees) { + restArgs.push('--raw-field', `assignees[]=${u}`) + } const r = await runRest(restArgs) - if (!r.ok) {return { ok: false, error: r.error }} + if (!r.ok) { + return { ok: false, error: r.error } + } } if (removeAssignees && removeAssignees.length > 0) { const restArgs = ['-X', 'DELETE', `${base}/assignees`] - for (const u of removeAssignees) {restArgs.push('--raw-field', `assignees[]=${u}`)} + for (const u of removeAssignees) { + restArgs.push('--raw-field', `assignees[]=${u}`) + } const r = await runRest(restArgs) - if (!r.ok) {return { ok: false, error: r.error }} + if (!r.ok) { + return { ok: false, error: r.error } + } } return { ok: true } } @@ -257,13 +297,21 @@ export async function updatePullRequestBySlug( args: UpdatePullRequestBySlugArgs ): Promise { const v = validateSlugArgs(args.owner, args.repo) - if (!v.ok) {return v} + if (!v.ok) { + return v + } const n = assertPositiveInt(args.number, 'number') - if (!n.ok) {return { ok: false, error: n.error }} + if (!n.ok) { + return { ok: false, error: n.error } + } if (!args.updates || typeof args.updates !== 'object') { return { ok: false, error: { type: 'validation_error', message: 'Updates required.' } } } - const patchArgs: string[] = ['-X', 'PATCH', `repos/${args.owner}/${args.repo}/pulls/${args.number}`] + const patchArgs: string[] = [ + '-X', + 'PATCH', + `repos/${args.owner}/${args.repo}/pulls/${args.number}` + ] // Why: count fields explicitly rather than inferring from patchArgs.length — // adding a future header/flag arg silently breaks an array-length check. let fieldCount = 0 @@ -280,7 +328,9 @@ export async function updatePullRequestBySlug( return { ok: true } } const r = await runRest(patchArgs) - if (!r.ok) {return { ok: false, error: r.error }} + if (!r.ok) { + return { ok: false, error: r.error } + } return { ok: true } } @@ -308,9 +358,13 @@ export async function addIssueCommentBySlug( args: AddIssueCommentBySlugArgs ): Promise { const v = validateSlugArgs(args.owner, args.repo) - if (!v.ok) {return v} + if (!v.ok) { + return v + } const n = assertPositiveInt(args.number, 'number') - if (!n.ok) {return { ok: false, error: n.error }} + if (!n.ok) { + return { ok: false, error: n.error } + } if (typeof args.body !== 'string' || !args.body.trim()) { return { ok: false, error: { type: 'validation_error', message: 'Comment body required.' } } } @@ -321,7 +375,9 @@ export async function addIssueCommentBySlug( '--raw-field', `body=${args.body}` ]) - if (!r.ok) {return { ok: false, error: r.error }} + if (!r.ok) { + return { ok: false, error: r.error } + } return { ok: true, comment: mapIssueComment(r.data, args.body) } } @@ -329,9 +385,13 @@ export async function updateIssueCommentBySlug( args: UpdateIssueCommentBySlugArgs ): Promise { const v = validateSlugArgs(args.owner, args.repo) - if (!v.ok) {return v} + if (!v.ok) { + return v + } const n = assertPositiveInt(args.commentId, 'commentId') - if (!n.ok) {return { ok: false, error: n.error }} + if (!n.ok) { + return { ok: false, error: n.error } + } if (typeof args.body !== 'string' || !args.body.trim()) { return { ok: false, error: { type: 'validation_error', message: 'Comment body required.' } } } @@ -342,7 +402,9 @@ export async function updateIssueCommentBySlug( '--raw-field', `body=${args.body}` ]) - if (!r.ok) {return { ok: false, error: r.error }} + if (!r.ok) { + return { ok: false, error: r.error } + } return { ok: true } } @@ -350,16 +412,22 @@ export async function deleteIssueCommentBySlug( args: DeleteIssueCommentBySlugArgs ): Promise { const v = validateSlugArgs(args.owner, args.repo) - if (!v.ok) {return v} + if (!v.ok) { + return v + } const n = assertPositiveInt(args.commentId, 'commentId') - if (!n.ok) {return { ok: false, error: n.error }} + if (!n.ok) { + return { ok: false, error: n.error } + } const r = await runRest( ['-X', 'DELETE', `repos/${args.owner}/${args.repo}/issues/comments/${args.commentId}`], undefined, 'core', { expectEmpty: true } ) - if (!r.ok) {return { ok: false, error: r.error }} + if (!r.ok) { + return { ok: false, error: r.error } + } return { ok: true } } @@ -369,9 +437,13 @@ export async function listLabelsBySlug( args: ListLabelsBySlugArgs ): Promise { const v = validateSlugArgs(args.owner, args.repo) - if (!v.ok) {return v} + if (!v.ok) { + return v + } const guard = rateLimitGuard('core') - if (guard.blocked) {return { ok: false, error: rateLimitedError(guard) }} + if (guard.blocked) { + return { ok: false, error: rateLimitedError(guard) } + } await acquire() // Why: `--paginate` may fan out to multiple pages; we can only reasonably // estimate a 1-call spend up front. The next probe will reconcile. @@ -400,12 +472,16 @@ export async function listAssignableUsersBySlug( args: ListAssignableUsersBySlugArgs ): Promise { const v = validateSlugArgs(args.owner, args.repo) - if (!v.ok) {return v} + if (!v.ok) { + return v + } // Seed logins merge after the fetch so callers can include currently-visible // assignees even if the repo participant search is sparse. const result: GitHubAssignableUser[] = [] const guard = rateLimitGuard('core') - if (guard.blocked) {return { ok: false, error: rateLimitedError(guard) }} + if (guard.blocked) { + return { ok: false, error: rateLimitedError(guard) } + } await acquire() noteRateLimitSpend('core') try { @@ -419,7 +495,10 @@ export async function listAssignableUsersBySlug( ], { encoding: 'utf-8' } ) - for (const line of stdout.trim().split('\n').filter((l) => l.length > 0)) { + for (const line of stdout + .trim() + .split('\n') + .filter((l) => l.length > 0)) { try { const u = JSON.parse(line) as { login?: string; avatarUrl?: string; name?: string | null } if (typeof u.login === 'string') { @@ -455,7 +534,9 @@ export async function listIssueTypesBySlug( args: ListIssueTypesBySlugArgs ): Promise { const v = validateSlugArgs(args.owner, args.repo) - if (!v.ok) {return v} + if (!v.ok) { + return v + } const query = ` query($owner:String!, $repo:String!) { repository(owner:$owner, name:$repo) { @@ -488,7 +569,10 @@ export async function listIssueTypesBySlug( } const nodes = res.data.repository?.issueTypes?.nodes ?? [] const types = nodes - .filter((n): n is NonNullable => n !== null && typeof n.id === 'string' && typeof n.name === 'string') + .filter( + (n): n is NonNullable => + n !== null && typeof n.id === 'string' && typeof n.name === 'string' + ) .map((n) => ({ id: n.id as string, name: n.name as string, @@ -502,9 +586,13 @@ export async function updateIssueTypeBySlug( args: UpdateIssueTypeBySlugArgs ): Promise { const v = validateSlugArgs(args.owner, args.repo) - if (!v.ok) {return v} + if (!v.ok) { + return v + } const n = assertPositiveInt(args.number, 'number') - if (!n.ok) {return { ok: false, error: n.error }} + if (!n.ok) { + return { ok: false, error: n.error } + } // Why: `updateIssueIssueType` is the dedicated mutation; passing null for // `issueTypeId` clears the type. We resolve the issue id via a lightweight // GraphQL lookup because the REST endpoint doesn't accept issue types. @@ -516,7 +604,9 @@ export async function updateIssueTypeBySlug( }`, { owner: args.owner, repo: args.repo, num: args.number } ) - if (!lookup.ok) {return { ok: false, error: lookup.error }} + if (!lookup.ok) { + return { ok: false, error: lookup.error } + } const issueId = lookup.data.repository?.issue?.id if (!issueId) { return { ok: false, error: { type: 'not_found', message: 'Issue not found.' } } @@ -543,7 +633,9 @@ export async function updateIssueTypeBySlug( ? { issueId, issueTypeId: args.issueTypeId } : { issueId } const res = await runGraphql(query, vars) - if (!res.ok) {return { ok: false, error: res.error }} + if (!res.ok) { + return { ok: false, error: res.error } + } return { ok: true } } @@ -567,9 +659,13 @@ export async function getWorkItemDetailsBySlug( args: ProjectWorkItemDetailsBySlugArgs ): Promise { const v = validateSlugArgs(args.owner, args.repo) - if (!v.ok) {return v} + if (!v.ok) { + return v + } const n = assertPositiveInt(args.number, 'number') - if (!n.ok) {return { ok: false, error: n.error }} + if (!n.ok) { + return { ok: false, error: n.error } + } if (args.type !== 'issue' && args.type !== 'pr') { return { ok: false, error: { type: 'validation_error', message: 'Invalid type.' } } } @@ -620,41 +716,47 @@ export async function getWorkItemDetailsBySlug( ` const res = await runGraphql<{ repository?: { - issue?: RawWorkItemContent & { - updatedAt?: string - body?: string - author?: { login?: string } | null - participants?: { nodes?: RawUser[] } - comments?: { - nodes?: ({ - databaseId?: number - author?: { login?: string; avatarUrl?: string; __typename?: string } | null + issue?: + | (RawWorkItemContent & { + updatedAt?: string body?: string - createdAt?: string - url?: string - } | null)[] - } - } | null - pullRequest?: RawWorkItemContent & { - updatedAt?: string - body?: string - headRefName?: string - baseRefName?: string - author?: { login?: string } | null - participants?: { nodes?: RawUser[] } - comments?: { - nodes?: ({ - databaseId?: number - author?: { login?: string; avatarUrl?: string; __typename?: string } | null + author?: { login?: string } | null + participants?: { nodes?: RawUser[] } + comments?: { + nodes?: ({ + databaseId?: number + author?: { login?: string; avatarUrl?: string; __typename?: string } | null + body?: string + createdAt?: string + url?: string + } | null)[] + } + }) + | null + pullRequest?: + | (RawWorkItemContent & { + updatedAt?: string body?: string - createdAt?: string - url?: string - } | null)[] - } - } | null + headRefName?: string + baseRefName?: string + author?: { login?: string } | null + participants?: { nodes?: RawUser[] } + comments?: { + nodes?: ({ + databaseId?: number + author?: { login?: string; avatarUrl?: string; __typename?: string } | null + body?: string + createdAt?: string + url?: string + } | null)[] + } + }) + | null } | null }>(query, { owner: args.owner, repo: args.repo, num: args.number }) - if (!res.ok) {return { ok: false, error: res.error }} + if (!res.ok) { + return { ok: false, error: res.error } + } const raw = args.type === 'issue' ? res.data.repository?.issue : res.data.repository?.pullRequest if (!raw) { return { ok: false, error: { type: 'not_found', message: 'Item not found.' } } @@ -668,7 +770,9 @@ export async function getWorkItemDetailsBySlug( .filter((l): l is string => typeof l === 'string') const comments: PRComment[] = [] for (const c of raw.comments?.nodes ?? []) { - if (!c || typeof c.body !== 'string') {continue} + if (!c || typeof c.body !== 'string') { + continue + } comments.push({ id: typeof c.databaseId === 'number' ? c.databaseId : Date.now(), author: c.author?.login ?? '', @@ -735,4 +839,3 @@ export async function getWorkItemDetailsBySlug( } return { ok: true, details } } - diff --git a/src/main/github/rate-limit.ts b/src/main/github/rate-limit.ts index 138e078f78f..1108b6bd219 100644 --- a/src/main/github/rate-limit.ts +++ b/src/main/github/rate-limit.ts @@ -37,11 +37,15 @@ type GhRateLimitPayload = { } } -function parseBucket(raw: { - limit?: number - remaining?: number - reset?: number -} | undefined): GitHubRateLimitBucket { +function parseBucket( + raw: + | { + limit?: number + remaining?: number + reset?: number + } + | undefined +): GitHubRateLimitBucket { // Why: if a bucket is absent from the response (old gh, partial response), // return 0/0/now so the UI shows a clear "unknown" state (0/0 is // unambiguous) rather than a misleading "plenty left" fallback. @@ -81,12 +85,14 @@ export type RateLimitBucketKind = 'core' | 'graphql' | 'search' * it advisory (returns a reason, doesn't throw) so callers can format the * envelope/error in their own shape. */ -export function rateLimitGuard(bucket: RateLimitBucketKind): { blocked: false } | { - blocked: true - remaining: number - limit: number - resetAt: number -} { +export function rateLimitGuard(bucket: RateLimitBucketKind): + | { blocked: false } + | { + blocked: true + remaining: number + limit: number + resetAt: number + } { if (!cached) { return { blocked: false } } diff --git a/src/main/index.ts b/src/main/index.ts index e6d8e979bf0..84553104e93 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -16,7 +16,8 @@ import { setAppRuntimeFlags } from './ipc/app' import { closeAllWatchers } from './ipc/filesystem-watcher' import { registerCoreHandlers } from './ipc/register-core-handlers' import { registerMobileHandlers } from './ipc/mobile' -import { initTelemetry, shutdownTelemetry } from './telemetry/client' +import { initTelemetry, shutdownTelemetry, trackAppOpenedOnce } from './telemetry/client' +import { resolveConsent } from './telemetry/consent' import { triggerStartupNotificationRegistration } from './ipc/notifications' import { OrcaRuntimeService } from './runtime/orca-runtime' import { OrcaRuntimeRpcServer } from './runtime/runtime-rpc' @@ -216,6 +217,22 @@ function openMainWindow(): BrowserWindow { } }) + // Why: telemetry-plan.md§First-launch experience anchors default-on + // `app_opened` to the first main-window load. Existing users in the + // pending-banner cohort resolve through telemetry/client.ts; this load + // path only fires once consent is already enabled. + const onFirstWindowLoad = (): void => { + if (!store) { + return + } + const consent = resolveConsent(store.getSettings()) + if (consent.effective !== 'enabled') { + return + } + trackAppOpenedOnce() + } + window.webContents.on('did-finish-load', onFirstWindowLoad) + registerCoreHandlers( store, runtime, @@ -422,6 +439,7 @@ app.whenReady().then(async () => { // and defeat the teardown helper's prefix sweep (design §4.3 wire-up). getLocalProvider: () => getLocalPtyProvider() }) + runtime.setAccountServices({ claudeAccounts, codexAccounts, rateLimits }) starNag = new StarNagService(store, stats) starNag.start() starNag.registerIpcHandlers() diff --git a/src/main/ipc/github.test.ts b/src/main/ipc/github.test.ts index 8e09575309c..8a74e3b741c 100644 --- a/src/main/ipc/github.test.ts +++ b/src/main/ipc/github.test.ts @@ -93,7 +93,7 @@ describe('registerGitHubHandlers', () => { branch: 'feature/test' }) - expect(getPRForBranchMock).toHaveBeenCalledWith('/workspace/repo', 'feature/test') + expect(getPRForBranchMock).toHaveBeenCalledWith('/workspace/repo', 'feature/test', null) }) it('rejects unknown repository paths', async () => { diff --git a/src/main/ipc/github.ts b/src/main/ipc/github.ts index d0da43341c3..0926093bbde 100644 --- a/src/main/ipc/github.ts +++ b/src/main/ipc/github.ts @@ -84,22 +84,25 @@ function assertRegisteredRepo(repoPath: string, store: Store): Repo { } export function registerGitHubHandlers(store: Store, stats: StatsCollector): void { - ipcMain.handle('gh:prForBranch', async (_event, args: { repoPath: string; branch: string }) => { - const repo = assertRegisteredRepo(args.repoPath, store) - const pr = await getPRForBranch(repo.path, args.branch) - // Emit pr_created when a PR is first detected for a branch. - // Why here: the renderer polls gh:prForBranch to check PR status per worktree. - // This captures PRs opened from any workflow (Orca UI, gh CLI, github.com). - if (pr && !stats.hasCountedPR(pr.url)) { - stats.record({ - type: 'pr_created', - at: Date.now(), - repoId: repo.id, - meta: { prNumber: pr.number, prUrl: pr.url } - }) + ipcMain.handle( + 'gh:prForBranch', + async (_event, args: { repoPath: string; branch: string; linkedPRNumber?: number | null }) => { + const repo = assertRegisteredRepo(args.repoPath, store) + const pr = await getPRForBranch(repo.path, args.branch, args.linkedPRNumber ?? null) + // Emit pr_created when a PR is first detected for a branch. + // Why here: the renderer polls gh:prForBranch to check PR status per worktree. + // This captures PRs opened from any workflow (Orca UI, gh CLI, github.com). + if (pr && !stats.hasCountedPR(pr.url)) { + stats.record({ + type: 'pr_created', + at: Date.now(), + repoId: repo.id, + meta: { prNumber: pr.number, prUrl: pr.url } + }) + } + return pr } - return pr - }) + ) ipcMain.handle('gh:issue', (_event, args: { repoPath: string; number: number }) => { const repo = assertRegisteredRepo(args.repoPath, store) diff --git a/src/main/ipc/mobile.ts b/src/main/ipc/mobile.ts index c7ee87d07ee..528749c29b5 100644 --- a/src/main/ipc/mobile.ts +++ b/src/main/ipc/mobile.ts @@ -82,6 +82,7 @@ export function registerMobileHandlers(rpcServer: OrcaRuntimeRpcServer): void { return { available: true as const, qrDataUrl, + pairingUrl: url, endpoint, deviceId: device.deviceId } diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index 19e8626ff31..f9a74b15441 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -21,7 +21,9 @@ const { buildAgentHookEnvMock, piBuildPtyEnvMock, piClearPtyMock, - isPwshAvailableMock + isPwshAvailableMock, + trackMock, + classifyErrorMock } = vi.hoisted(() => ({ handleMock: vi.fn(), onMock: vi.fn(), @@ -40,7 +42,9 @@ const { openCodeClearPtyMock: vi.fn(), buildAgentHookEnvMock: vi.fn(), piBuildPtyEnvMock: vi.fn(), - piClearPtyMock: vi.fn() + piClearPtyMock: vi.fn(), + trackMock: vi.fn(), + classifyErrorMock: vi.fn() })) vi.mock('electron', () => ({ @@ -95,6 +99,14 @@ vi.mock('../pi/titlebar-extension-service', () => ({ vi.mock('../pwsh', () => ({ isPwshAvailable: isPwshAvailableMock })) + +vi.mock('../telemetry/client', () => ({ + track: trackMock +})) + +vi.mock('../telemetry/classify-error', () => ({ + classifyError: classifyErrorMock +})) import { LocalPtyProvider } from '../providers/local-pty-provider' import { registerPtyHandlers, @@ -142,6 +154,8 @@ describe('registerPtyHandlers', () => { piBuildPtyEnvMock.mockReset() piClearPtyMock.mockReset() isPwshAvailableMock.mockReset() + trackMock.mockReset() + classifyErrorMock.mockReset() mainWindow.webContents.on.mockReset() mainWindow.webContents.send.mockReset() @@ -1629,4 +1643,86 @@ describe('registerPtyHandlers', () => { expect(openCodeClearPtyMock).toHaveBeenCalledWith(spawnResult.id) expect(piClearPtyMock).toHaveBeenCalledWith(spawnResult.id) }) + + describe('agent_started telemetry', () => { + // Why: telemetry-plan.md§Agent launch semantics — agent_started must + // fire only after provider.spawn resolves. The renderer threads + // launch metadata through `pty:spawn`; a missing or malformed + // payload must not produce a silently-malformed event. + it('emits agent_started after a successful spawn when telemetry is supplied', async () => { + handlers.clear() + registerPtyHandlers(mainWindow as never) + await handlers.get('pty:spawn')!(null, { + cols: 80, + rows: 24, + telemetry: { + agent_kind: 'claude-code', + launch_source: 'new_workspace_composer', + request_kind: 'new' + } + }) + expect(trackMock).toHaveBeenCalledWith('agent_started', { + agent_kind: 'claude-code', + launch_source: 'new_workspace_composer', + request_kind: 'new' + }) + }) + + it('does not emit agent_started when telemetry is omitted (bare-shell tab)', async () => { + handlers.clear() + registerPtyHandlers(mainWindow as never) + await handlers.get('pty:spawn')!(null, { cols: 80, rows: 24 }) + expect(trackMock).not.toHaveBeenCalled() + }) + + it('drops the event when any telemetry field is outside its closed enum', async () => { + handlers.clear() + registerPtyHandlers(mainWindow as never) + await handlers.get('pty:spawn')!(null, { + cols: 80, + rows: 24, + telemetry: { + agent_kind: 'claude-code', + launch_source: 'not_a_real_surface', + request_kind: 'new' + } + }) + expect(trackMock).not.toHaveBeenCalledWith('agent_started', expect.anything()) + }) + + it('does not emit agent_started when provider.spawn throws', async () => { + // Why: telemetry-plan contract is that agent_started fires only on + // confirmed launch. Inject a provider whose spawn throws so we hit + // the catch path with no race against the real LocalPtyProvider. + setLocalPtyProvider({ + spawn: vi.fn(async () => { + throw new Error('spawn boom') + }), + write: vi.fn(), + resize: vi.fn(), + kill: vi.fn(), + shutdown: vi.fn(), + onData: vi.fn(() => vi.fn()), + onExit: vi.fn(() => vi.fn()), + listProcesses: vi.fn(async () => []), + getForegroundProcess: vi.fn(async () => null) + } as never) + classifyErrorMock.mockReturnValue({ error_class: 'unknown' }) + handlers.clear() + registerPtyHandlers(mainWindow as never) + await expect( + handlers.get('pty:spawn')!(null, { + cols: 80, + rows: 24, + command: 'claude', + telemetry: { + agent_kind: 'claude-code', + launch_source: 'new_workspace_composer', + request_kind: 'new' + } + }) + ).rejects.toThrow(/spawn boom/) + expect(trackMock).not.toHaveBeenCalledWith('agent_started', expect.anything()) + }) + }) }) diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index 0fb63b5b800..3c985f4afb0 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -25,6 +25,13 @@ import { } from '../claude-accounts/live-pty-gate' import { applyTerminalAttributionEnv } from '../attribution/terminal-attribution' import { registerPty, unregisterPty } from '../memory/pty-registry' +import { track } from '../telemetry/client' +import { classifyError } from '../telemetry/classify-error' +import { + agentKindSchema, + launchSourceSchema, + requestKindSchema +} from '../../shared/telemetry-events' // ─── Provider Registry ────────────────────────────────────────────── // Routes PTY operations by connectionId. null = local provider. @@ -715,6 +722,17 @@ export function registerPtyHandlers( worktreeId?: string sessionId?: string shellOverride?: string + // Why: telemetry-plan.md§Agent launch semantics. The renderer + // threads what Orca was *asked* to launch through this field; main + // fires `agent_started` only after `provider.spawn` resolves. Loose + // typing on the IPC boundary because the main-side schema + // validator is the single enforcement point — `track()` will drop + // the event if any field is outside its closed enum. + telemetry?: { + agent_kind?: unknown + launch_source?: unknown + request_kind?: unknown + } } ) => { const provider = getProvider(args.connectionId) @@ -879,6 +897,30 @@ export function registerPtyHandlers( if (isMintedSessionId && effectiveSessionId !== undefined) { clearProviderPtyState(effectiveSessionId) } + // Why: telemetry-plan.md§agent_error — when the renderer threaded + // agent_kind through args.telemetry, attribute the error to that agent. + // Otherwise fall back to sniffing the command for `claude` (the one + // agent the main process can identify on its own via the existing + // `isClaudeLaunchCommand` regex used for auth gating). Bare-shell + // catches and unknown-agent catches without renderer telemetry remain + // unattributed. The event still emits with a classified `error_class`; + // raw error messages are dropped at the telemetry validator boundary. + const rendererAgentKindParse = + args.telemetry?.agent_kind !== undefined + ? agentKindSchema.safeParse(args.telemetry.agent_kind) + : null + const errorAgentKind = rendererAgentKindParse?.success + ? rendererAgentKindParse.data + : isClaudeLaunch + ? ('claude-code' as const) + : null + if (errorAgentKind) { + const classified = classifyError(err) + track('agent_error', { + agent_kind: errorAgentKind, + error_class: classified.error_class + }) + } throw err } ptyOwnership.set(result.id, args.connectionId ?? null) @@ -993,6 +1035,28 @@ export function registerPtyHandlers( : null }) } + // Why: telemetry-plan.md§Agent launch semantics — fire `agent_started` + // only after `provider.spawn` resolved. The renderer threads + // `args.telemetry` through the spawn IPC for every launch we want to + // attribute; bare-shell tabs (no agent) leave the field undefined and + // do not produce an event. Each field is parsed against its closed + // enum here so a malformed renderer payload (or a spoofed IPC) does + // not poison the event — `safeParse` failure drops that field, and + // if any required field is missing we skip the event entirely. The + // main-side `track()` validator re-runs the schema on the full + // payload as a second defense-in-depth check. + if (args.telemetry) { + const agentKindParse = agentKindSchema.safeParse(args.telemetry.agent_kind) + const launchSourceParse = launchSourceSchema.safeParse(args.telemetry.launch_source) + const requestKindParse = requestKindSchema.safeParse(args.telemetry.request_kind) + if (agentKindParse.success && launchSourceParse.success && requestKindParse.success) { + track('agent_started', { + agent_kind: agentKindParse.data, + launch_source: launchSourceParse.data, + request_kind: requestKindParse.data + }) + } + } return result } ) diff --git a/src/main/ipc/repos.ts b/src/main/ipc/repos.ts index bf18216a863..15ac89ad25c 100644 --- a/src/main/ipc/repos.ts +++ b/src/main/ipc/repos.ts @@ -29,6 +29,25 @@ import { import { getSshGitProvider } from '../providers/ssh-git-dispatch' import { getActiveMultiplexer } from './ssh' import { normalizeSparseDirectories } from './sparse-checkout-directories' +import { track } from '../telemetry/client' +import type { RepoMethod } from '../../shared/telemetry-events' + +// Why: `method` answers "which entry point did the user take?", not "what did +// they add?" — so the IPC the renderer invoked IS the method. We never send +// the path, URL, or display name. `repos:create` collapses into +// `folder_picker` because the user's entry was the folder picker, even +// though main also `git init`s. `drag_drop` is reserved for a future call +// site; no current renderer surface produces it. +function emitRepoAdded(method: RepoMethod, alreadyExisted: boolean): void { + // Why: re-adding an existing repo (matched by path inside the handler) + // is not a new activation event. Suppressing the duplicate keeps the + // funnel honest and avoids inflating `repo_added` for users who + // re-pick the same folder. + if (alreadyExisted) { + return + } + track('repo_added', { method }) +} // Why: module-scoped so the abort handle survives window re-creation on macOS. // registerRepoHandlers is called again when a new BrowserWindow is created, @@ -74,6 +93,7 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v // Check if already added const existing = store.getRepos().find((r) => r.path === args.path) if (existing) { + emitRepoAdded('folder_picker', true) return { repo: existing } } @@ -89,6 +109,7 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v store.addRepo(repo) await rebuildAuthorizedRootsCache(store) notifyReposChanged(mainWindow) + emitRepoAdded('folder_picker', false) return { repo } } ) @@ -135,6 +156,7 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v .getRepos() .find((r) => r.connectionId === args.connectionId && r.path === resolvedPath) if (existing) { + emitRepoAdded('folder_picker', true) return { repo: existing } } @@ -195,6 +217,7 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v mux.notify('session.registerRoot', { rootPath: resolvedPath }) } + emitRepoAdded('folder_picker', false) return { repo } } ) @@ -241,6 +264,7 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v // the race matters even after this one passes. const existing = store.getRepos().find((r) => r.path === targetPath) if (existing) { + emitRepoAdded('folder_picker', true) return { repo: existing } } @@ -382,6 +406,7 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v store.addRepo(repo) await rebuildAuthorizedRootsCache(store) notifyReposChanged(mainWindow) + emitRepoAdded('folder_picker', false) return { repo } } ) @@ -621,9 +646,12 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v const updated = store.updateRepo(existing.id, { kind: 'git' }) if (updated) { notifyReposChanged(mainWindow) + // Why: folder→git upgrade is a real new git repo provisioning event. + emitRepoAdded('clone_url', false) return updated } } + emitRepoAdded('clone_url', true) return existing } @@ -639,6 +667,7 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v store.addRepo(repo) await rebuildAuthorizedRootsCache(store) notifyReposChanged(mainWindow) + emitRepoAdded('clone_url', false) return repo } ) diff --git a/src/main/ipc/settings.ts b/src/main/ipc/settings.ts index 92aecef4868..30fe923b1d4 100644 --- a/src/main/ipc/settings.ts +++ b/src/main/ipc/settings.ts @@ -4,6 +4,13 @@ import type { GlobalSettings, PersistedState } from '../../shared/types' import { listSystemFontFamilies } from '../system-fonts' import { previewGhosttyImport } from '../ghostty/index' import { rebuildAppMenu } from '../menu/register-app-menu' +import { track } from '../telemetry/client' +import { SETTINGS_CHANGED_WHITELIST, type SettingsChangedKey } from '../../shared/telemetry-events' + +// Why: the whitelist is the source-of-truth for which keys we emit on. Casting +// to a Set once at module load lets the IPC handler's per-key membership +// check stay O(1) without re-coercing the readonly tuple on every call. +const SETTINGS_CHANGED_WHITELIST_SET = new Set(SETTINGS_CHANGED_WHITELIST) // Why: fields that appear in the View > Appearance submenu need the menu // rebuilt after any update so the checkbox `checked` state stays in sync @@ -23,10 +30,43 @@ export function registerSettingsHandlers(store: Store): void { if (args.theme) { nativeTheme.themeSource = args.theme } + // Why: capture the pre-update value so we only emit when the value + // actually changes. The settings UI sometimes re-saves the same value + // (e.g. blur after a no-op edit), and a `settings_changed` event for a + // no-op flip would inflate the experimental-feature-adoption signal. + const before = store.getSettings() const result = store.updateSettings(args) if (APPEARANCE_MENU_KEYS.some((key) => key in args)) { rebuildAppMenu() } + + // Why: telemetry-plan.md§Settings — fire `settings_changed` only for + // whitelisted keys, with `value_kind` distinguishing booleans from + // string-enum settings. We deliberately do NOT send the raw value for + // non-enum settings; the whitelist is currently scoped to experimental + // toggles, all of which are booleans, so `value_kind === 'bool'` is + // the path the v1 enum has a slot for. If a non-bool whitelisted + // setting is ever added, extend the discriminator here at the same + // time the schema's `value_kind` enum gains the new value. + for (const key of Object.keys(args)) { + if (!SETTINGS_CHANGED_WHITELIST_SET.has(key)) { + continue + } + const beforeValue = (before as Record)[key] + const afterValue = (result as Record)[key] + if (beforeValue === afterValue) { + continue + } + if (typeof afterValue !== 'boolean') { + // No non-bool whitelist entries today; skip rather than guess. + continue + } + track('settings_changed', { + setting_key: key as SettingsChangedKey, + value_kind: 'bool' + }) + } + return result }) diff --git a/src/main/ipc/sidekick.ts b/src/main/ipc/sidekick.ts index ee8e97b6852..f7de7f10e45 100644 --- a/src/main/ipc/sidekick.ts +++ b/src/main/ipc/sidekick.ts @@ -1,7 +1,9 @@ -import { app, BrowserWindow, dialog, ipcMain } from 'electron' -import { copyFile, mkdir, readFile, rm, stat } from 'node:fs/promises' +/* eslint-disable max-lines */ +import { app, BrowserWindow, dialog, ipcMain, nativeImage } from 'electron' +import { copyFile, mkdir, readFile, rename, rm, stat, lstat } from 'node:fs/promises' import { randomUUID } from 'node:crypto' -import { basename, extname, join, normalize, sep } from 'node:path' +import { basename, dirname, extname, isAbsolute, join, normalize, resolve, sep } from 'node:path' +import { z } from 'zod' import type { CustomSidekick } from '../../shared/types' // Why: image-only sidekick uploads. Static + animated variants render natively @@ -35,36 +37,125 @@ function getSidekicksDir(): string { } const MAX_BYTES = 64 * 1024 * 1024 // 64 MB — generous but bounded so a user can't point at a multi-GB file and OOM the renderer when it builds a Blob URL. +const MAX_MANIFEST_BYTES = 64 * 1024 // pet.json is tiny by spec; cap to defend against a malicious bundle stuffing megabytes into the manifest. function isSafeId(id: string): boolean { - // UUIDs only; blocks path traversal and unexpected characters. + // UUIDs only; blocks path traversal and unexpected characters. Storage ids + // are always generated in main (never sourced from the bundle's manifest.id), + // so this regex is the canonical gate for any fs path that includes the id. return /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i.test(id) } -function resolveSidekickFile(id: string, fileName: string): string | null { +function resolveSidekickFile( + id: string, + fileName: string, + kind: 'image' | 'bundle' +): string | null { if (!isSafeId(id)) { return null } - // Why: the renderer hands back the persisted fileName (which includes the - // original extension so Blob MIME detection works). We still normalize and - // prefix-check against the sidekicks dir to defend against any edge case that - // slipped the id regex. const safeName = basename(fileName) + const root = normalize(getSidekicksDir()) + let filePath: string + if (kind === 'bundle') { + // Bundle layout: custom//. fileName is just the spritesheet + // basename (e.g. "spritesheet.webp"); pet.json is read by main only and + // never served to the renderer. + filePath = normalize(join(root, id, safeName)) + const bundleDir = normalize(join(root, id)) + sep + if (!filePath.startsWith(bundleDir)) { + return null + } + return filePath + } + // Legacy image layout: custom/.. Filename must start with the id + // so the prefix check catches any edge case that slipped the regex. if (!safeName.startsWith(`${id}.`)) { return null } - const filePath = normalize(join(getSidekicksDir(), safeName)) - if (!filePath.startsWith(normalize(getSidekicksDir()) + sep)) { + filePath = normalize(join(root, safeName)) + if (!filePath.startsWith(root + sep)) { return null } return filePath } +const PetManifestSchema = z + .object({ + id: z.string().min(1).max(128).optional(), + displayName: z.string().min(1).max(120).optional(), + description: z.string().max(2000).optional(), + spritesheetPath: z + .string() + .min(1) + .max(255) + // Why: belt-and-suspenders against malicious manifests — downstream + // resolve+prefix check still runs as defense in depth. + .refine( + (p) => !p.includes('\0') && !p.startsWith('/') && !p.startsWith('\\') && !p.includes('..'), + 'invalid spritesheetPath' + ), + frame: z + .object({ + width: z.number().int().positive().max(1024), + height: z.number().int().positive().max(1024) + }) + .optional(), + fps: z.number().positive().max(60).optional(), + defaultAnimation: z.string().min(1).max(64).optional(), + animations: z + .record( + z.string().min(1).max(64), + z.object({ + row: z.number().int().min(0).max(256), + frames: z.number().int().positive().max(512) + }) + ) + .optional() + }) + // Why: tolerate unknown top-level fields. Pet-bundle generators may emit + // descriptive metadata (e.g. "kind": "person") we don't consume yet, and + // strict mode would reject those imports with a confusing "unrecognized + // key" error instead of just ignoring the extras. + .loose() + +type PetManifest = z.infer + +// Why: renderer-supplied IPC inputs are untrusted — validate shape before any +// path resolution. resolveSidekickFile still gates the actual filesystem path. +const SidekickFileRequestSchema = z.object({ + id: z.string(), + fileName: z.string(), + kind: z.enum(['image', 'bundle']).optional() +}) + +async function readSheetDimensions( + buffer: Buffer +): Promise<{ width: number; height: number } | null> { + // Why: nativeImage decodes PNG/JPEG/GIF/WebP/BMP. SVG isn't supported here + // (vector → no integer pixel grid), so pet bundles must use a raster sheet. + const image = nativeImage.createFromBuffer(buffer) + if (image.isEmpty()) { + return null + } + const size = image.getSize() + if (size.width <= 0 || size.height <= 0) { + return null + } + return { width: size.width, height: size.height } +} + +async function isSymlink(path: string): Promise { + try { + const s = await lstat(path) + return s.isSymbolicLink() + } catch { + return false + } +} + export function registerSidekickHandlers(): void { ipcMain.handle('sidekick:import', async (event): Promise => { - // Why: parent the file picker to the sender window so the dialog opens as - // a sheet attached to the main window. Without a parent, on macOS the - // dialog can land behind the main window. const senderWindow = BrowserWindow.fromWebContents(event.sender) ?? BrowserWindow.getFocusedWindow() const options: Electron.OpenDialogOptions = { @@ -129,14 +220,215 @@ export function registerSidekickHandlers(): void { id, label, fileName, - mimeType: classified.mimeType + mimeType: classified.mimeType, + kind: 'image' + } + }) + + ipcMain.handle('sidekick:importPetBundle', async (event): Promise => { + const senderWindow = + BrowserWindow.fromWebContents(event.sender) ?? BrowserWindow.getFocusedWindow() + // Why: the bundle is a folder. macOS users may also pick `pet.json` itself + // when Finder is set to show package contents — the post-pick logic walks + // up to the parent directory in that case. + const options: Electron.OpenDialogOptions = { + title: 'Pick a .codex-pet bundle', + properties: ['openFile', 'openDirectory', 'treatPackageAsDirectory'] + } + const result = senderWindow + ? await dialog.showOpenDialog(senderWindow, options) + : await dialog.showOpenDialog(options) + if (result.canceled || result.filePaths.length === 0) { + return null + } + const picked = result.filePaths[0] + let bundleDir: string + try { + const pickedStat = await stat(picked) + bundleDir = pickedStat.isDirectory() ? picked : dirname(picked) + } catch { + throw new Error('Could not read the selected path.') + } + + const manifestPath = join(bundleDir, 'pet.json') + let manifestStat: Awaited> + try { + manifestStat = await stat(manifestPath) + } catch { + throw new Error('Bundle is missing pet.json.') + } + if (!manifestStat.isFile() || manifestStat.size > MAX_MANIFEST_BYTES) { + throw new Error('pet.json is invalid.') + } + if (await isSymlink(manifestPath)) { + throw new Error('pet.json must not be a symlink.') + } + + let manifest: PetManifest + try { + const raw = await readFile(manifestPath, 'utf8') + // Why: defend against TOCTOU between stat and read — the file could have + // grown after the stat check. + if (Buffer.byteLength(raw, 'utf8') > MAX_MANIFEST_BYTES) { + throw new Error('pet.json exceeded the manifest size limit.') + } + manifest = PetManifestSchema.parse(JSON.parse(raw)) + } catch (error) { + throw new Error(`Invalid pet.json: ${error instanceof Error ? error.message : 'parse error'}`) + } + + // Why: spritesheetPath is bundle-relative. Reject absolute paths and any + // resolved path that escapes the bundle directory. Also reject symlinks so + // a malicious bundle can't reach outside via a sibling link. + if (isAbsolute(manifest.spritesheetPath)) { + throw new Error('spritesheetPath must be relative to the bundle.') + } + const sheetSrc = resolve(bundleDir, manifest.spritesheetPath) + const bundleResolved = resolve(bundleDir) + if (sheetSrc === bundleResolved) { + throw new Error('spritesheetPath must point to a file, not the bundle root.') + } + const bundleRoot = bundleResolved + sep + // Why: NTFS/macOS HFS+ default volumes are case-insensitive — a path like + // `BUNDLE\sheet.png` is still inside `bundle\`. Compare lowercased on + // Windows so the prefix check isn't bypassed by case differences. + const cmp = process.platform === 'win32' ? (s: string) => s.toLowerCase() : (s: string) => s + if (!cmp(sheetSrc + sep).startsWith(cmp(bundleRoot))) { + throw new Error('spritesheetPath escapes the bundle.') + } + if (await isSymlink(sheetSrc)) { + throw new Error('spritesheet must not be a symlink.') + } + const sheetClass = classifyFile(sheetSrc) + if (!sheetClass || sheetClass.ext === '.svg') { + // SVG can't be used as a sprite sheet (no pixel grid). + throw new Error('Spritesheet must be a PNG, APNG, JPG, GIF, or WebP.') + } + let sheetStat: Awaited> + try { + sheetStat = await stat(sheetSrc) + } catch { + throw new Error('Spritesheet file not found.') + } + if (!sheetStat.isFile()) { + throw new Error('Spritesheet path is not a file.') + } + if (sheetStat.size > MAX_BYTES) { + throw new Error( + `Spritesheet is too large (${(sheetStat.size / (1024 * 1024)).toFixed(1)} MB).` + ) + } + + let sprite: NonNullable | undefined + if (manifest.frame) { + // Why: only decode the sheet when we need to validate frame layout. + // nativeImage may fail on some WebP variants in headless contexts, and + // bundles without `frame` render as a static image where dimensions + // don't matter. + const sheetBuf = await readFile(sheetSrc) + // Why: defend against TOCTOU — file may have grown between stat and read. + if (sheetBuf.byteLength > MAX_BYTES) { + throw new Error('Spritesheet exceeded the size limit.') + } + const dims = await readSheetDimensions(sheetBuf) + if (!dims) { + throw new Error('Could not decode the spritesheet image.') + } + const { width: fw, height: fh } = manifest.frame + if (dims.width % fw !== 0 || dims.height % fh !== 0) { + throw new Error( + `Spritesheet ${dims.width}×${dims.height} is not a clean multiple of frame ${fw}×${fh}.` + ) + } + const columns = dims.width / fw + const rows = dims.height / fh + if (manifest.animations) { + for (const [name, anim] of Object.entries(manifest.animations)) { + if (anim.row >= rows) { + throw new Error(`Animation "${name}" references row ${anim.row} but sheet has ${rows}.`) + } + if (anim.frames > columns) { + throw new Error( + `Animation "${name}" has ${anim.frames} frames but sheet only has ${columns} columns.` + ) + } + } + if (manifest.defaultAnimation && !manifest.animations[manifest.defaultAnimation]) { + throw new Error(`defaultAnimation "${manifest.defaultAnimation}" not in animations.`) + } + } + sprite = { + frameWidth: fw, + frameHeight: fh, + columns, + rows, + sheetWidth: dims.width, + sheetHeight: dims.height, + fps: manifest.fps ?? 8, + defaultAnimation: manifest.defaultAnimation, + animations: manifest.animations + } + } + + // Why: storage id is always a fresh UUID — the manifest's `id` is purely + // a display hint. This guards against (a) collisions with bundled + // sidekick ids, (b) failing isSafeId, and (c) re-import clobbering an + // earlier copy of the same bundle. + const id = randomUUID() + const root = getSidekicksDir() + await mkdir(root, { recursive: true }) + const destDir = join(root, id) + const sheetExt = sheetClass.ext + const sheetFileName = `spritesheet${sheetExt}` + // Why: stage the bundle into a sibling .tmp directory and atomically rename + // into place so destDir only appears once both files are written. Avoids + // half-imported bundles if a copy fails midway. + const tmpDir = `${destDir}.tmp` + try { + await rm(tmpDir, { recursive: true, force: true }).catch(() => {}) + await mkdir(tmpDir, { recursive: true }) + await copyFile(sheetSrc, join(tmpDir, sheetFileName)) + await copyFile(manifestPath, join(tmpDir, 'pet.json')) + await rename(tmpDir, destDir) + } catch { + await rm(tmpDir, { recursive: true, force: true }).catch(() => {}) + throw new Error('Could not save the pet bundle.') + } + + const rawLabel = (manifest.displayName ?? manifest.id ?? basename(bundleDir)).trim() + const label = rawLabel.length > 0 ? rawLabel.slice(0, 40) : 'Pet bundle' + return { + id, + label, + fileName: sheetFileName, + mimeType: sheetClass.mimeType, + kind: 'bundle', + sprite, + // Why: carries manifest.fps for detected-frame bundles where `sprite` + // is undefined — renderer falls back to this when sprite is absent. + ...(manifest.fps !== undefined ? { spriteFps: manifest.fps } : {}) } }) ipcMain.handle( 'sidekick:read', - async (_event, id: string, fileName: string): Promise => { - const filePath = resolveSidekickFile(id, fileName) + async ( + _event, + id: string, + fileName: string, + kind?: 'image' | 'bundle' + ): Promise => { + // Why: validate IPC inputs before any path logic — renderer is not + // trusted to send strings of the right shape. + let parsed: z.infer + try { + parsed = SidekickFileRequestSchema.parse({ id, fileName, kind }) + } catch { + throw new Error('Invalid sidekick:read arguments') + } + // Why: missing kind defaults to 'image' for backwards compatibility with + // pre-bundle persisted state. + const filePath = resolveSidekickFile(parsed.id, parsed.fileName, parsed.kind ?? 'image') if (!filePath) { return null } @@ -150,15 +442,44 @@ export function registerSidekickHandlers(): void { } ) - ipcMain.handle('sidekick:delete', async (_event, id: string, fileName: string): Promise => { - const filePath = resolveSidekickFile(id, fileName) - if (!filePath) { - return + ipcMain.handle( + 'sidekick:delete', + async (_event, id: string, fileName: string, kind?: 'image' | 'bundle'): Promise => { + // Why: validate IPC inputs before any path logic. + let parsed: z.infer + try { + parsed = SidekickFileRequestSchema.parse({ id, fileName, kind }) + } catch { + throw new Error('Invalid sidekick:delete arguments') + } + if (!isSafeId(parsed.id)) { + return + } + if ((parsed.kind ?? 'image') === 'bundle') { + // Why: bundle imports own a whole directory. isSafeId already gates id; + // we still build the path under the sidekicks root and verify the + // prefix before recursive removal as defense in depth. + const root = normalize(getSidekicksDir()) + const target = normalize(join(root, parsed.id)) + if (!target.startsWith(root + sep)) { + return + } + try { + await rm(target, { recursive: true, force: true }) + } catch (error) { + console.warn('[sidekick-overlay] sidekick:delete (bundle) failed', error) + } + return + } + const filePath = resolveSidekickFile(parsed.id, parsed.fileName, 'image') + if (!filePath) { + return + } + try { + await rm(filePath, { force: true }) + } catch (error) { + console.warn('[sidekick-overlay] sidekick:delete failed', error) + } } - try { - await rm(filePath, { force: true }) - } catch (error) { - console.warn('[sidekick-overlay] sidekick:delete failed', error) - } - }) + ) } diff --git a/src/main/ipc/telemetry.test.ts b/src/main/ipc/telemetry.test.ts index d1b2ee36377..9e1b9231cc5 100644 --- a/src/main/ipc/telemetry.test.ts +++ b/src/main/ipc/telemetry.test.ts @@ -245,9 +245,8 @@ describe('telemetry IPC handlers', () => { it('routes banner ✕ through persistBannerAcknowledgeWithoutEmitting without invoking setOptIn', () => { // This is the whole point of the separate channel: the silent-persist // path MUST NOT reach setOptIn, which would derive a `via` and fire - // `telemetry_opted_in`. The ✕-as-silent-acknowledge semantics are - // explicit: the user did not opt in, they declined to intervene, so - // no event transmits. + // `telemetry_opted_in`. The client primitive may unlock `app_opened`, + // but the acknowledge channel itself must not emit an opt-in event. registerWith({ installId: 'x', existedBeforeTelemetryRelease: true, diff --git a/src/main/ipc/telemetry.ts b/src/main/ipc/telemetry.ts index ff14d27fbed..1bfddc07d41 100644 --- a/src/main/ipc/telemetry.ts +++ b/src/main/ipc/telemetry.ts @@ -165,13 +165,11 @@ export function registerTelemetryHandlers(store: Store): void { return resolveConsent(storeRef.getSettings()) }) - ipcMain.handle('telemetry:acknowledgeBanner', (_event): void => { - // Banner ✕ — persist `optedIn = true`, emit nothing. The ✕-as-silent- - // acknowledge semantics are explicit: the user did not explicitly opt - // in, they declined to intervene, so no event transmits. This outcome - // cannot route through `telemetry:setOptIn` because the derivation - // above would tag it `first_launch_banner` and fire - // `telemetry_opted_in`. + ipcMain.handle('telemetry:acknowledgeBanner', (_event): Promise | void => { + // Banner ✕ — persist `optedIn = true` without emitting a telemetry opt-in + // event. The acknowledge still unlocks `app_opened`, but this outcome + // cannot route through `telemetry:setOptIn` because the derivation above + // would tag it `first_launch_banner` and fire `telemetry_opted_in`. // // Check storeRef BEFORE consuming a consent-mutation token, mirroring // the setOptIn handler's guard above — see that comment for why @@ -205,7 +203,7 @@ export function registerTelemetryHandlers(store: Store): void { if (!consumeConsentMutationToken()) { return } - persistBannerAcknowledgeWithoutEmitting() + return persistBannerAcknowledgeWithoutEmitting() }) } diff --git a/src/main/ipc/worktree-logic.ts b/src/main/ipc/worktree-logic.ts index f4fc0e292b8..d6472413bf3 100644 --- a/src/main/ipc/worktree-logic.ts +++ b/src/main/ipc/worktree-logic.ts @@ -172,6 +172,7 @@ export function mergeWorktree( isPinned: meta?.isPinned ?? false, sortOrder: meta?.sortOrder ?? 0, lastActivityAt: meta?.lastActivityAt ?? 0, + ...(meta?.createdAt !== undefined ? { createdAt: meta.createdAt } : {}), ...(git.isSparse === true ? { sparseDirectories: meta?.sparseDirectories, diff --git a/src/main/ipc/worktree-remote.ts b/src/main/ipc/worktree-remote.ts index cb37a482ae4..416206d2687 100644 --- a/src/main/ipc/worktree-remote.ts +++ b/src/main/ipc/worktree-remote.ts @@ -204,8 +204,16 @@ export async function createRemoteWorktree( } const worktreeId = `${repo.id}::${created.path}` + const now = Date.now() const metaUpdates: Partial = { - lastActivityAt: Date.now(), + lastActivityAt: now, + // Why: grants the new worktree a short grace window at the top of the + // Recent sort. During worktree creation (git fetch + add can take several + // seconds) other worktrees get ambient PTY bumps that would otherwise + // leave the newly-created one below them; the Recent comparator uses + // max(lastActivityAt, createdAt + GRACE_MS) to keep it on top until the + // window elapses. See smart-sort.ts `CREATE_GRACE_MS`. + createdAt: now, ...(shouldSetDisplayName(requestedName, branchName, sanitizedName) ? { displayName: requestedName } : {}) @@ -434,11 +442,15 @@ export async function createLocalWorktree( } const worktreeId = `${repo.id}::${created.path}` + const now = Date.now() const metaUpdates: Partial = { // Stamp activity so the worktree sorts into its final position // immediately — prevents scroll-to-reveal racing with a later // bumpWorktreeActivity that would re-sort the list. - lastActivityAt: Date.now(), + lastActivityAt: now, + // See createRemoteWorktree above: createdAt protects the newly-created + // worktree from ambient PTY bumps in other worktrees for CREATE_GRACE_MS. + createdAt: now, ...(shouldSetDisplayName(effectiveRequestedName, branchName, effectiveSanitizedName) ? { displayName: effectiveRequestedName } : {}), diff --git a/src/main/ipc/worktrees.test.ts b/src/main/ipc/worktrees.test.ts index 12547caecde..e2e055f0866 100644 --- a/src/main/ipc/worktrees.test.ts +++ b/src/main/ipc/worktrees.test.ts @@ -378,6 +378,136 @@ describe('registerWorktreeHandlers', () => { expect(listWorktreesMock).not.toHaveBeenCalled() }) + it('stamps lastActivityAt on first discovery so newly-added worktrees sort to the top of Recent', async () => { + // Why: a worktree that exists on disk but has no persisted WorktreeMeta + // (e.g. a folder repo just added, or a pre-existing worktree in a + // newly-added git repo) would otherwise fall back to `lastActivityAt: 0` + // and rank dead last in the Recent sort. + listWorktreesMock.mockResolvedValue([ + { + path: '/workspace/discovered-wt', + head: 'abc123', + branch: 'refs/heads/feature', + isBare: false, + isMainWorktree: false + } + ]) + store.getWorktreeMeta.mockReturnValue(undefined) + const stampedMeta = { lastActivityAt: 1_700_000_000_000 } + store.setWorktreeMeta.mockReturnValue(stampedMeta) + + const listed = (await handlers['worktrees:list'](null, { repoId: 'repo-1' })) as { + id: string + lastActivityAt: number + }[] + + expect(store.setWorktreeMeta).toHaveBeenCalledWith( + 'repo-1::/workspace/discovered-wt', + expect.objectContaining({ lastActivityAt: expect.any(Number) }) + ) + expect(listed[0]).toMatchObject({ + id: 'repo-1::/workspace/discovered-wt', + lastActivityAt: 1_700_000_000_000 + }) + }) + + it('does not re-stamp lastActivityAt when a worktree already has persisted meta', async () => { + // Why: only the *first* discovery should stamp. Re-stamping on every list + // would overwrite real activity and reshuffle the sidebar on refresh. + listWorktreesMock.mockResolvedValue([ + { + path: '/workspace/existing-wt', + head: 'abc123', + branch: 'refs/heads/feature', + isBare: false, + isMainWorktree: false + } + ]) + store.getWorktreeMeta.mockReturnValue({ + displayName: '', + comment: '', + linkedIssue: null, + linkedPR: null, + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 0, + lastActivityAt: 42 + }) + + const listed = (await handlers['worktrees:list'](null, { repoId: 'repo-1' })) as { + id: string + lastActivityAt: number + }[] + + expect(store.setWorktreeMeta).not.toHaveBeenCalled() + expect(listed[0].lastActivityAt).toBe(42) + }) + + it('stamps lastActivityAt on first discovery for folder-mode repos', async () => { + // Why: folder repos produce a synthetic worktree that flows through the + // same list path. Without the stamp, adding a folder puts its card at the + // bottom of Recent even though the user just added it. + store.getRepos.mockReturnValue([ + { + id: 'repo-1', + path: '/workspace/folder', + displayName: 'folder', + badgeColor: '#000', + addedAt: 0, + kind: 'folder' + } + ]) + store.getRepo.mockReturnValue({ + id: 'repo-1', + path: '/workspace/folder', + displayName: 'folder', + badgeColor: '#000', + addedAt: 0, + kind: 'folder' + }) + store.getWorktreeMeta.mockReturnValue(undefined) + store.setWorktreeMeta.mockReturnValue({ lastActivityAt: 1_700_000_000_000 }) + + await handlers['worktrees:list'](null, { repoId: 'repo-1' }) + + expect(store.setWorktreeMeta).toHaveBeenCalledWith( + 'repo-1::/workspace/folder', + expect.objectContaining({ lastActivityAt: expect.any(Number) }) + ) + }) + + it('stamps lastActivityAt on first discovery via worktrees:listAll', async () => { + // Why: the stamping logic lives in both worktrees:list and worktrees:listAll. + // Without a dedicated test, a regression in the listAll loop would silently + // bury newly-discovered worktrees from the multi-repo sidebar view. + listWorktreesMock.mockResolvedValue([ + { + path: '/workspace/discovered-wt', + head: 'abc123', + branch: 'refs/heads/feature', + isBare: false, + isMainWorktree: false + } + ]) + store.getWorktreeMeta.mockReturnValue(undefined) + store.setWorktreeMeta.mockReturnValue({ lastActivityAt: 1_700_000_000_000 }) + + const listed = (await handlers['worktrees:listAll'](null, undefined)) as { + id: string + lastActivityAt: number + }[] + + expect(store.setWorktreeMeta).toHaveBeenCalledWith( + 'repo-1::/workspace/discovered-wt', + expect.objectContaining({ lastActivityAt: expect.any(Number) }) + ) + expect(listed[0]).toMatchObject({ + id: 'repo-1::/workspace/discovered-wt', + lastActivityAt: 1_700_000_000_000 + }) + }) + it('skips past a suffix that already belongs to a PR after an initial branch conflict', async () => { // Why: `gh pr list` is network-bound and previously fired on every single // create, adding 1–3s to the happy path. We now only probe PR conflicts diff --git a/src/main/ipc/worktrees.ts b/src/main/ipc/worktrees.ts index c7372711fb1..77274212631 100644 --- a/src/main/ipc/worktrees.ts +++ b/src/main/ipc/worktrees.ts @@ -38,6 +38,22 @@ import type { OrcaRuntimeService } from '../runtime/orca-runtime' import { killAllProcessesForWorktree } from '../runtime/worktree-teardown' import { getLocalPtyProvider } from './pty' import { removeWorktreeSymlinks } from './worktree-symlinks' +import { track } from '../telemetry/client' +import { workspaceSourceSchema, type WorkspaceSource } from '../../shared/telemetry-events' + +// Why: worktrees discovered on disk (not created via Orca's UI) have no +// persisted WorktreeMeta, so mergeWorktree falls back to `lastActivityAt: 0`. +// That makes them sort to the bottom of "Recent" even though the user just +// added the repo / folder. Stamp discovery time the first time we see a +// worktree so its very existence counts as a recency signal. Subsequent +// list calls find the persisted meta and skip the stamp. +function resolveWorktreeMetaWithDiscoveryStamp(store: Store, worktreeId: string): WorktreeMeta { + const existing = store.getWorktreeMeta(worktreeId) + if (existing) { + return existing + } + return store.setWorktreeMeta(worktreeId, { lastActivityAt: Date.now() }) +} export function registerWorktreeHandlers( mainWindow: BrowserWindow, @@ -84,7 +100,7 @@ export function registerWorktreeHandlers( } return gitWorktrees.map((gw) => { const worktreeId = `${repo.id}::${gw.path}` - const meta = store.getWorktreeMeta(worktreeId) + const meta = resolveWorktreeMetaWithDiscoveryStamp(store, worktreeId) return mergeWorktree(repo.id, gw, meta, repo.displayName) }) } catch { @@ -126,7 +142,7 @@ export function registerWorktreeHandlers( } return gitWorktrees.map((gw) => { const worktreeId = `${repo.id}::${gw.path}` - const meta = store.getWorktreeMeta(worktreeId) + const meta = resolveWorktreeMetaWithDiscoveryStamp(store, worktreeId) return mergeWorktree(repo.id, gw, meta, repo.displayName) }) }) @@ -143,11 +159,26 @@ export function registerWorktreeHandlers( } // Remote repos route all git operations through the relay - if (repo.connectionId) { - return createRemoteWorktree(args, repo, store, mainWindow) - } + const result = repo.connectionId + ? await createRemoteWorktree(args, repo, store, mainWindow) + : await createLocalWorktree(args, repo, store, mainWindow, runtime) - return createLocalWorktree(args, repo, store, mainWindow, runtime) + // Why: emit `workspace_created` only after the underlying create has + // resolved (the helpers throw on failure, so reaching this line means + // git-add succeeded — we deliberately do not also emit a separate + // `workspace_initialized`, see telemetry-plan.md§Deferred events). + // `from_existing_branch` is true iff the caller specified a non-empty + // baseBranch; an unspecified baseBranch means "branch from default + // HEAD", which is the not-from-existing-branch case. We never send + // the branch name itself. + const sourceParse = workspaceSourceSchema.safeParse(args.telemetrySource) + const source: WorkspaceSource = sourceParse.success ? sourceParse.data : 'unknown' + track('workspace_created', { + source, + from_existing_branch: typeof args.baseBranch === 'string' && args.baseBranch.length > 0 + }) + + return result } ) diff --git a/src/main/rate-limits/service.ts b/src/main/rate-limits/service.ts index ef827a820b4..b90568567ff 100644 --- a/src/main/rate-limits/service.ts +++ b/src/main/rate-limits/service.ts @@ -74,9 +74,17 @@ export class RateLimitService { private inactiveCodexFetching = new Set() private lastInactiveClaudeFetchAt = 0 private lastInactiveCodexFetchAt = 0 + private stateListeners = new Set<(state: RateLimitState) => void>() constructor() {} + onStateChange(listener: (state: RateLimitState) => void): () => void { + this.stateListeners.add(listener) + return () => { + this.stateListeners.delete(listener) + } + } + setCodexHomePathResolver(resolver: () => string | null): void { this.codexHomePathResolver = resolver } @@ -757,9 +765,17 @@ export class RateLimitService { } private pushToRenderer(): void { + const state = this.getState() + for (const listener of this.stateListeners) { + try { + listener(state) + } catch { + // ignore — one bad listener must not break the others + } + } if (!this.mainWindow || this.mainWindow.isDestroyed()) { return } - this.mainWindow.webContents.send('rateLimits:update', this.getState()) + this.mainWindow.webContents.send('rateLimits:update', state) } } diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 56e5c1029c5..20d66814f9c 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -793,6 +793,46 @@ describe('OrcaRuntimeService', () => { }) }) + it('clears stale working status after the agent exits and the shell takes over the title', async () => { + // Why: regression test for issue #1437 — the mobile worktree-list spinner + // kept playing forever because lastAgentStatus was sticky on 'working' + // once an agent exited without emitting an idle/agent-shaped final OSC + // title. worktree.ps must recompute from the live OSC title each call. + const runtime = new OrcaRuntimeService(store) + + runtime.attachWindow(1) + runtime.syncWindowGraph(1, { + tabs: [ + { + tabId: 'tab-1', + worktreeId: 'repo-1::/tmp/worktree-a', + title: 'Codex working', + activeLeafId: 'pane:1', + layout: null + } + ], + leaves: [ + { + tabId: 'tab-1', + worktreeId: 'repo-1::/tmp/worktree-a', + leafId: 'pane:1', + paneRuntimeId: 1, + ptyId: 'pty-1' + } + ] + }) + + runtime.onPtyData('pty-1', '\x1b]0;Codex working\x07', 100) + const working = await runtime.getWorktreePs() + expect(working.worktrees[0].status).toBe('working') + + // Agent exits, shell title takes over — desktop's getWorktreeStatus would + // immediately flip back to 'active'. Mobile must do the same. + runtime.onPtyData('pty-1', '\x1b]0;bash\x07', 200) + const afterExit = await runtime.getWorktreePs() + expect(afterExit.worktrees[0].status).toBe('active') + }) + it('fails terminal stop closed while the renderer graph is reloading', async () => { const runtime = new OrcaRuntimeService(store) let killed = false @@ -1064,6 +1104,54 @@ describe('OrcaRuntimeService', () => { expect(activateWorktree).toHaveBeenCalledWith('repo-1', expect.any(String), undefined) }) + it('stamps createdAt alongside lastActivityAt so CLI-created worktrees get the Recent-sort grace window', async () => { + // Why: parity with createLocalWorktree / createRemoteWorktree. Without + // createdAt, ambient PTY bumps in OTHER worktrees during the few seconds + // after creation can push the new worktree below them in Recent sort. + const runtime = new OrcaRuntimeService(store) + runtime.setNotifier({ + worktreesChanged: vi.fn(), + reposChanged: vi.fn(), + activateWorktree: vi.fn(), + createTerminal: vi.fn(), + splitTerminal: vi.fn(), + renameTerminal: vi.fn(), + focusTerminal: vi.fn(), + closeTerminal: vi.fn(), + sleepWorktree: vi.fn(), + terminalFitOverrideChanged: vi.fn(), + terminalDriverChanged: vi.fn() + }) + runtime.attachWindow(1) + + computeWorktreePathMock.mockReturnValue('/tmp/workspaces/runtime-grace') + ensurePathWithinWorkspaceMock.mockReturnValue('/tmp/workspaces/runtime-grace') + vi.mocked(getEffectiveHooks).mockReturnValue({ scripts: {} }) + vi.mocked(listWorktrees).mockResolvedValueOnce([ + { + path: '/tmp/workspaces/runtime-grace', + head: 'def', + branch: 'runtime-grace', + isBare: false, + isMainWorktree: false + } + ]) + + const before = Date.now() + const result = await runtime.createManagedWorktree({ + repoSelector: 'id:repo-1', + name: 'runtime-grace' + }) + const after = Date.now() + + expect(result.worktree.createdAt).toBeDefined() + expect(result.worktree.createdAt).toBeGreaterThanOrEqual(before) + expect(result.worktree.createdAt).toBeLessThanOrEqual(after) + // Both fields must be stamped from the same `now` so the grace-window + // math (max(lastActivityAt, createdAt + GRACE_MS)) is well-defined. + expect(result.worktree.createdAt).toBe(result.worktree.lastActivityAt) + }) + it('skips archive hooks for CLI worktree removal by default', async () => { const runtime = new OrcaRuntimeService(store) vi.mocked(getEffectiveHooks).mockReturnValue({ diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 8c8ce9e4262..9e50759173c 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -134,6 +134,23 @@ import { HeadlessEmulator } from '../daemon/headless-emulator' import { killAllProcessesForWorktree } from './worktree-teardown' import { MOBILE_SUBSCRIBE_SCROLLBACK_ROWS } from './scrollback-limits' import type { IPtyProvider } from '../providers/types' +import type { ClaudeAccountService } from '../claude-accounts/service' +import type { CodexAccountService } from '../codex-accounts/service' +import type { RateLimitService } from '../rate-limits/service' +import type { ClaudeRateLimitAccountsState, CodexRateLimitAccountsState } from '../../shared/types' +import type { RateLimitState } from '../../shared/rate-limit-types' + +type RuntimeAccountServices = { + claudeAccounts: ClaudeAccountService + codexAccounts: CodexAccountService + rateLimits: RateLimitService +} + +export type AccountsSnapshot = { + claude: ClaudeRateLimitAccountsState + codex: CodexRateLimitAccountsState + rateLimits: RateLimitState +} type RuntimeStore = { getRepos: Store['getRepos'] @@ -167,6 +184,12 @@ type RuntimeLeafRecord = RuntimeSyncedLeaf & { tailLinesTotal: number preview: string lastAgentStatus: AgentStatus | null + // Why: the most recent OSC title observed on this leaf's PTY data. Used by + // worktree.ps so daemon-hosted terminals (no renderer pushing pane titles) + // still recompute working/idle from the live title each call instead of + // serving a stale `lastAgentStatus` after the agent process exits and the + // shell takes over the title — the bug behind issue #1437. + lastOscTitle: string | null } type RuntimePtyWorktreeRecord = { @@ -351,6 +374,12 @@ export class OrcaRuntimeService { Set<(event: { mode: 'mobile-fit' | 'desktop-fit'; cols: number; rows: number }) => void> >() private subscriptionCleanups = new Map void>() + // Why: index of subscriptionIds by per-WebSocket connectionId so the + // server can sweep all subscriptions for a closing socket without + // touching subscriptions on other live sockets that share the same + // deviceToken (multi-screen mobile). + private subscriptionsByConnection = new Map>() + private subscriptionConnectionByEntry = new Map() // Why: mobile clients subscribe to desktop notifications via // notifications.subscribe. This set enables fan-out — each connected // mobile client gets its own listener, and dispatchMobileNotification @@ -505,6 +534,7 @@ export class OrcaRuntimeService { private fetchInflight = new Map>() private fetchLastCompletedAt = new Map() private readonly getLocalProviderFn: (() => IPtyProvider) | null + private accountServices: RuntimeAccountServices | null = null constructor( store: RuntimeStore | null = null, @@ -632,7 +662,8 @@ export class OrcaRuntimeService { tailTruncated: existing?.ptyId === ptyId ? existing.tailTruncated : false, tailLinesTotal: existing?.ptyId === ptyId ? existing.tailLinesTotal : 0, preview: existing?.ptyId === ptyId ? existing.preview : '', - lastAgentStatus: existing?.ptyId === ptyId ? existing.lastAgentStatus : null + lastAgentStatus: existing?.ptyId === ptyId ? existing.lastAgentStatus : null, + lastOscTitle: existing?.ptyId === ptyId ? existing.lastOscTitle : null }) if (leaf.ptyId) { @@ -790,8 +821,21 @@ export class OrcaRuntimeService { leaf.tailLinesTotal += nextTail.newCompleteLines leaf.preview = buildPreview(leaf.tailBuffer, leaf.tailPartialLine) - if (agentStatus !== null) { + if (oscTitle !== null) { + // Why: keep the latest OSC title on the leaf so worktree.ps can + // recompute status from the live title each call. Without this, + // daemon-hosted terminals (no renderer pushing pane titles) had no + // way to clear a stale 'working' status after the agent exited and + // the shell took over the title — the stuck-spinner bug in #1437. + leaf.lastOscTitle = oscTitle const prevStatus = leaf.lastAgentStatus + // Why: when a new OSC title doesn't classify as an agent state (e.g. + // bare shell title after the agent exits), clear lastAgentStatus so + // it is no longer sticky. Tui-idle waiters that needed the previous + // 'idle' transition were already resolved at the moment of the + // transition below; only fresh waiters registered after the agent + // exits would observe the cleared value, and they correctly fall + // back to title-based detection / polling. leaf.lastAgentStatus = agentStatus // Why: resolve tui-idle on any transition TO idle (not just working→idle). // Claude Code may skip "working" entirely on fast tasks, going null→idle, @@ -986,12 +1030,15 @@ export class OrcaRuntimeService { return } const status = detectAgentStatusFromTitle(title) - if (status === null) { - return - } for (const leaf of this.leaves.values()) { if (leaf.ptyId === ptyId) { - leaf.lastAgentStatus = status + // Why: seed lastOscTitle even when the seeded title doesn't classify + // as an agent state, so worktree.ps recomputes status from the live + // title rather than treating the leaf as agentless. + leaf.lastOscTitle = title + if (status !== null) { + leaf.lastAgentStatus = status + } } } } @@ -1107,7 +1154,11 @@ export class OrcaRuntimeService { return { ptyId: leaf.ptyId } } - registerSubscriptionCleanup(subscriptionId: string, cleanup: () => void): void { + registerSubscriptionCleanup( + subscriptionId: string, + cleanup: () => void, + connectionId?: string + ): void { // Why: mobile clients reconnect frequently (phone lock, network switch). // The RPC client re-sends terminal.subscribe on reconnect, creating a new // handler before the old one is cleaned up. Without this, the old data @@ -1115,18 +1166,57 @@ export class OrcaRuntimeService { const existing = this.subscriptionCleanups.get(subscriptionId) if (existing) { existing() + // Why: existing() already evicts itself from the per-connection index + // via cleanupSubscription, so no extra bookkeeping is needed here. } this.subscriptionCleanups.set(subscriptionId, cleanup) + if (connectionId) { + let set = this.subscriptionsByConnection.get(connectionId) + if (!set) { + set = new Set() + this.subscriptionsByConnection.set(connectionId, set) + } + set.add(subscriptionId) + this.subscriptionConnectionByEntry.set(subscriptionId, connectionId) + } } cleanupSubscription(subscriptionId: string): void { const cleanup = this.subscriptionCleanups.get(subscriptionId) if (cleanup) { this.subscriptionCleanups.delete(subscriptionId) + const connectionId = this.subscriptionConnectionByEntry.get(subscriptionId) + if (connectionId) { + this.subscriptionConnectionByEntry.delete(subscriptionId) + const set = this.subscriptionsByConnection.get(connectionId) + if (set) { + set.delete(subscriptionId) + if (set.size === 0) { + this.subscriptionsByConnection.delete(connectionId) + } + } + } cleanup() } } + // Why: invoked from the WebSocket transport's on-close hook so streaming + // listeners registered for this exact socket get torn down even when other + // sockets sharing the same deviceToken are still alive (multi-screen + // mobile). Without this sweep, listeners leak across every reconnect. + cleanupSubscriptionsForConnection(connectionId: string): void { + const set = this.subscriptionsByConnection.get(connectionId) + if (!set) { + return + } + // Why: snapshot the ids before iterating because cleanupSubscription + // mutates both the set and the index map. + const ids = Array.from(set) + for (const id of ids) { + this.cleanupSubscription(id) + } + } + // Why: mobile clients subscribe via notifications.subscribe streaming RPC. // Each subscriber gets its own listener. Returns an unsubscribe function // that the subscription cleanup mechanism calls on disconnect. @@ -1147,6 +1237,73 @@ export class OrcaRuntimeService { } } + // ─── Account Services (mobile RPC bridge) ───────────────────── + + setAccountServices(services: RuntimeAccountServices): void { + this.accountServices = services + } + + private requireAccountServices(): RuntimeAccountServices { + if (!this.accountServices) { + throw new Error('Account services are not configured on this runtime') + } + return this.accountServices + } + + getAccountsSnapshot(): AccountsSnapshot { + const { claudeAccounts, codexAccounts, rateLimits } = this.requireAccountServices() + return { + claude: claudeAccounts.listAccounts(), + codex: codexAccounts.listAccounts(), + rateLimits: rateLimits.getState() + } + } + + // Why: RateLimitService polls only when the Electron window is visible AND + // focused, and the inactive-account caches fill lazily when the user opens + // the desktop AccountsPane. Mobile has neither trigger, so without this the + // phone shows 0% / "—" against a backgrounded desktop. Errors swallowed + // because partial usage is still useful for the rest of the snapshot. + async refreshAccountsForMobile(): Promise { + const { rateLimits } = this.requireAccountServices() + await Promise.allSettled([ + rateLimits.refresh(), + rateLimits.fetchInactiveClaudeAccountsOnOpen(), + rateLimits.fetchInactiveCodexAccountsOnOpen() + ]) + } + + selectClaudeAccount(accountId: string | null): Promise { + return this.requireAccountServices().claudeAccounts.selectAccount(accountId) + } + + selectCodexAccount(accountId: string | null): Promise { + return this.requireAccountServices().codexAccounts.selectAccount(accountId) + } + + removeClaudeAccount(accountId: string): Promise { + return this.requireAccountServices().claudeAccounts.removeAccount(accountId) + } + + removeCodexAccount(accountId: string): Promise { + return this.requireAccountServices().codexAccounts.removeAccount(accountId) + } + + // Why: rate-limit polling fires every 5 minutes and on account switch. + // Mobile clients subscribe to receive a fresh AccountsSnapshot whenever + // RateLimitService pushes new usage data, mirroring the existing + // `rateLimits:update` IPC channel desktop already uses. + onAccountsChanged(listener: (snapshot: AccountsSnapshot) => void): () => void { + const services = this.requireAccountServices() + return services.rateLimits.onStateChange(() => { + listener({ + claude: services.claudeAccounts.listAccounts(), + codex: services.codexAccounts.listAccounts(), + rateLimits: services.rateLimits.getState() + }) + }) + } + // ─── Mobile Fit Override Management ───────────────────────── resizeForClient( @@ -2810,8 +2967,14 @@ export class OrcaRuntimeService { } const worktreeId = `${repo.id}::${created.path}` + const now = Date.now() const meta = this.store.setWorktreeMeta(worktreeId, { - lastActivityAt: Date.now(), + lastActivityAt: now, + // See createRemoteWorktree: createdAt grants the new worktree a grace + // window in Recent sort so ambient PTY bumps in OTHER worktrees can't + // push it down before the user has had a chance to notice it. Smart-sort + // uses max(lastActivityAt, createdAt + CREATE_GRACE_MS). + createdAt: now, ...(shouldSetDisplayName(requestedName, branchName, sanitizedName) ? { displayName: requestedName } : {}), @@ -5695,7 +5858,14 @@ function getLeafWorktreeStatus( leaf: RuntimeLeafRecord, tabTitle: string | null ): RuntimeWorktreeStatus { - const detected = leaf.lastAgentStatus ?? detectAgentStatusFromTitle(leaf.title ?? tabTitle ?? '') + // Why: recompute from the live title each call so worktree.ps mirrors what + // the desktop sidebar's getWorktreeStatus does (no sticky state). Prefer + // the runtime-tracked OSC title (covers daemon-hosted terminals) over the + // renderer-pushed leaf.title and the tab title. Falling back to + // lastAgentStatus only when no title is available preserves a sensible + // signal for very fresh leaves before any title has been observed. + const liveTitle = leaf.lastOscTitle ?? leaf.title ?? tabTitle ?? '' + const detected = liveTitle ? detectAgentStatusFromTitle(liveTitle) : leaf.lastAgentStatus if (detected === 'permission') { return 'permission' } diff --git a/src/main/runtime/rpc/core.ts b/src/main/runtime/rpc/core.ts index 9c6eab7212a..0340561938e 100644 --- a/src/main/runtime/rpc/core.ts +++ b/src/main/runtime/rpc/core.ts @@ -46,6 +46,12 @@ export type RpcContext = { // runtime-rpc transport (direct in-process callers don't need it). // See design doc §3.1 counter-lifecycle. signal?: AbortSignal + // Why: streaming handlers (notifications/accounts/terminal subscribe) + // register cleanup callbacks against the runtime so reconnects don't leak + // listeners. Keying those cleanups by per-WebSocket connectionId lets the + // server reap all subscriptions for a closing socket, even when other + // sockets for the same deviceToken stay alive (multi-screen mobile). + connectionId?: string } export type RpcHandler = (params: TParams, ctx: RpcContext) => Promise | unknown diff --git a/src/main/runtime/rpc/dispatcher.ts b/src/main/runtime/rpc/dispatcher.ts index faa8c57e33d..bdd2f181b30 100644 --- a/src/main/runtime/rpc/dispatcher.ts +++ b/src/main/runtime/rpc/dispatcher.ts @@ -74,7 +74,11 @@ export class RpcDispatcher { // Why: streaming dispatch sends multiple responses through the reply callback // instead of returning a single Promise. This enables terminal.subscribe and // other subscription-style methods that push data over time. - async dispatchStreaming(request: RpcRequest, reply: (response: string) => void): Promise { + async dispatchStreaming( + request: RpcRequest, + reply: (response: string) => void, + options?: { connectionId?: string } + ): Promise { const meta = this.meta() const method = this.registry.get(request.method) if (!method) { @@ -94,7 +98,10 @@ export class RpcDispatcher { if (!isStreamingMethod(method)) { try { - const result = await method.handler(parsedParams.value, { runtime: this.runtime }) + const result = await method.handler(parsedParams.value, { + runtime: this.runtime, + connectionId: options?.connectionId + }) reply(JSON.stringify(successResponse(request.id, meta, result))) } catch (error) { reply(JSON.stringify(this.mapError(request, meta, error))) @@ -109,7 +116,11 @@ export class RpcDispatcher { } try { - await method.handler(parsedParams.value, { runtime: this.runtime }, emit) + await method.handler( + parsedParams.value, + { runtime: this.runtime, connectionId: options?.connectionId }, + emit + ) } catch (error) { reply(JSON.stringify(this.mapError(request, meta, error))) } diff --git a/src/main/runtime/rpc/methods/accounts.ts b/src/main/runtime/rpc/methods/accounts.ts new file mode 100644 index 00000000000..21612a707f0 --- /dev/null +++ b/src/main/runtime/rpc/methods/accounts.ts @@ -0,0 +1,111 @@ +import { z } from 'zod' +import { defineMethod, defineStreamingMethod, type RpcAnyMethod } from '../core' + +// Why: monotonically increasing per-process counter avoids the Date.now() +// collision that fired when two near-simultaneous accounts.subscribe calls +// collided on the same millisecond and one evicted the other through +// registerSubscriptionCleanup's existing-key eviction path. +let accountsSubscriptionSeq = 0 + +const SelectAccountParams = z.object({ + accountId: z + .union([z.string().min(1, 'Missing accountId'), z.null()]) + .transform((v) => (v === null ? null : v)) +}) + +const RemoveAccountParams = z.object({ + accountId: z.string().min(1, 'Missing accountId') +}) + +const AccountsUnsubscribeParams = z.object({ + subscriptionId: z + .unknown() + .transform((value) => (typeof value === 'string' && value.length > 0 ? value : '')) + .pipe(z.string().min(1, 'Missing subscriptionId')) +}) + +// Why: bridges the desktop ClaudeAccountService / CodexAccountService / +// RateLimitService into the mobile WebSocket RPC. Read + switch + remove +// only — interactive add/re-auth flows spawn `claude login` / `codex login` +// PTYs that need a desktop browser, so they intentionally remain +// desktop-only. See plan in spec doc for issue #1438. +export const ACCOUNT_METHODS: readonly RpcAnyMethod[] = [ + defineMethod({ + name: 'accounts.list', + params: null, + handler: async (_params, { runtime }) => { + // Why: ensure the snapshot reflects the latest provider state before + // returning. Desktop polling pauses when the window is unfocused and + // inactive-account caches only fill on AccountsPane open, so without + // this the mobile UI would render stale nulls / zeroes. + await runtime.refreshAccountsForMobile() + return runtime.getAccountsSnapshot() + } + }), + defineMethod({ + name: 'accounts.selectClaude', + params: SelectAccountParams, + handler: async (params, { runtime }) => runtime.selectClaudeAccount(params.accountId) + }), + defineMethod({ + name: 'accounts.selectCodex', + params: SelectAccountParams, + handler: async (params, { runtime }) => runtime.selectCodexAccount(params.accountId) + }), + defineMethod({ + name: 'accounts.removeClaude', + params: RemoveAccountParams, + handler: async (params, { runtime }) => runtime.removeClaudeAccount(params.accountId) + }), + defineMethod({ + name: 'accounts.removeCodex', + params: RemoveAccountParams, + handler: async (params, { runtime }) => runtime.removeCodexAccount(params.accountId) + }), + // Why: streaming counterpart so mobile usage bars refresh in place when the + // desktop's 5-minute rate-limit poll completes or when the user switches + // accounts on either side. Mirrors the notifications.subscribe pattern. + defineStreamingMethod({ + name: 'accounts.subscribe', + params: null, + handler: async (_params, { runtime, connectionId }, emit) => { + await new Promise((resolve) => { + const unsubscribe = runtime.onAccountsChanged((snapshot) => { + emit({ type: 'snapshot', snapshot }) + }) + + // Why: scope the id by connectionId so two sockets from the same + // device (host + accounts screen) cannot evict each other through + // registerSubscriptionCleanup's "existing key" branch, and append a + // per-process counter so two concurrent subscribes on the same + // socket also can't collide. + const seq = ++accountsSubscriptionSeq + const subscriptionId = `accounts-${connectionId ?? 'inproc'}-${seq}` + runtime.registerSubscriptionCleanup( + subscriptionId, + () => { + unsubscribe() + emit({ type: 'end' }) + resolve() + }, + connectionId + ) + + // Why: emit the current snapshot synchronously so the phone has + // something to render immediately, then kick a forced refresh that + // will broadcast a fresh snapshot through the listener once each + // provider fetch completes. + emit({ type: 'ready', subscriptionId, snapshot: runtime.getAccountsSnapshot() }) + void runtime.refreshAccountsForMobile() + }) + } + }), + defineMethod({ + name: 'accounts.unsubscribe', + params: AccountsUnsubscribeParams, + handler: async (params, { runtime }) => { + runtime.cleanupSubscription(params.subscriptionId) + return { unsubscribed: true } + } + }) +] diff --git a/src/main/runtime/rpc/methods/index.ts b/src/main/runtime/rpc/methods/index.ts index cf3fbf34f86..bce5a50c4e0 100644 --- a/src/main/runtime/rpc/methods/index.ts +++ b/src/main/runtime/rpc/methods/index.ts @@ -8,6 +8,7 @@ import { BROWSER_EXTRA_METHODS } from './browser-extras' import { ORCHESTRATION_METHODS } from './orchestration' import { NOTIFICATION_METHODS } from './notifications' import { STATS_METHODS } from './stats' +import { ACCOUNT_METHODS } from './accounts' // Why: a flat manifest keeps registration order explicit and provides one // grep-point for "what methods does the RPC server expose?" — useful when @@ -21,5 +22,6 @@ export const ALL_RPC_METHODS: readonly RpcAnyMethod[] = [ ...BROWSER_EXTRA_METHODS, ...ORCHESTRATION_METHODS, ...NOTIFICATION_METHODS, - ...STATS_METHODS + ...STATS_METHODS, + ...ACCOUNT_METHODS ] diff --git a/src/main/runtime/rpc/methods/notifications.ts b/src/main/runtime/rpc/methods/notifications.ts index 607474bb941..1d63f26d785 100644 --- a/src/main/runtime/rpc/methods/notifications.ts +++ b/src/main/runtime/rpc/methods/notifications.ts @@ -1,6 +1,11 @@ import { z } from 'zod' import { defineStreamingMethod, defineMethod, type RpcAnyMethod } from '../core' +// Why: monotonically increasing per-process counter eliminates the +// Date.now() collision that could fire when two near-simultaneous +// notifications.subscribe calls landed on the same millisecond. +let notificationsSubscriptionSeq = 0 + const NotificationUnsubscribeParams = z.object({ subscriptionId: z .unknown() @@ -16,18 +21,25 @@ export const NOTIFICATION_METHODS: readonly RpcAnyMethod[] = [ defineStreamingMethod({ name: 'notifications.subscribe', params: null, - handler: async (_params, { runtime }, emit) => { + handler: async (_params, { runtime, connectionId }, emit) => { await new Promise((resolve) => { const unsubscribe = runtime.onNotificationDispatched((event) => { emit({ type: 'notification', ...event }) }) - const subscriptionId = `notifications-${Date.now()}` - runtime.registerSubscriptionCleanup(subscriptionId, () => { - unsubscribe() - emit({ type: 'end' }) - resolve() - }) + // Why: scope by per-ws connectionId + per-process counter so + // concurrent subscribes never collide on the cleanup map. + const seq = ++notificationsSubscriptionSeq + const subscriptionId = `notifications-${connectionId ?? 'inproc'}-${seq}` + runtime.registerSubscriptionCleanup( + subscriptionId, + () => { + unsubscribe() + emit({ type: 'end' }) + resolve() + }, + connectionId + ) emit({ type: 'ready', subscriptionId }) }) diff --git a/src/main/runtime/rpc/ws-transport.ts b/src/main/runtime/rpc/ws-transport.ts index 218bdfe2851..2d5881324eb 100644 --- a/src/main/runtime/rpc/ws-transport.ts +++ b/src/main/runtime/rpc/ws-transport.ts @@ -28,7 +28,9 @@ export class WebSocketTransport implements RpcTransport { private messageHandler: | ((msg: string, reply: (response: string) => void, ws: WebSocket) => void) | null = null - private connectionCloseHandler: ((clientId: string) => void) | null = null + private connectionCloseHandler: + | ((clientId: string, ws: WebSocket, hasOtherConnections: boolean) => void) + | null = null // Why: maps each WebSocket to the clientId (deviceToken) that authenticated it, // so ws.on('close') can notify the runtime which mobile client disconnected. private wsClientIds = new Map() @@ -46,7 +48,15 @@ export class WebSocketTransport implements RpcTransport { this.messageHandler = handler } - onConnectionClose(handler: (clientId: string) => void): void { + // Why: handlers receive the closing `ws` so per-connection state can be + // targeted exactly (one paired device may hold multiple concurrent sockets, + // e.g. host screen + accounts screen). `hasOtherConnections` tells the + // runtime whether other sockets for the same deviceToken are still alive, + // so client-scoped teardown (mobile-fit overrides, etc.) only fires on the + // last disconnect. + onConnectionClose( + handler: (clientId: string, ws: WebSocket, hasOtherConnections: boolean) => void + ): void { this.connectionCloseHandler = handler } @@ -178,7 +188,13 @@ export class WebSocketTransport implements RpcTransport { const clientId = this.wsClientIds.get(ws) this.wsClientIds.delete(ws) if (clientId) { - this.connectionCloseHandler?.(clientId) + // Why: a paired device may have multiple concurrent sockets open + // (e.g. one per app screen). Per-client teardown must only fire when + // the last socket for this token closes — otherwise closing the + // accounts-screen socket would clobber the host-screen socket's + // state and strand it in a non-functional state until re-paired. + const hasOtherConnections = Array.from(this.wsClientIds.values()).includes(clientId) + this.connectionCloseHandler?.(clientId, ws, hasOtherConnections) } }) diff --git a/src/main/runtime/runtime-rpc.ts b/src/main/runtime/runtime-rpc.ts index b2e4cf27660..c1f946e6990 100644 --- a/src/main/runtime/runtime-rpc.ts +++ b/src/main/runtime/runtime-rpc.ts @@ -88,6 +88,10 @@ export class OrcaRuntimeRpcServer { // Why: each WebSocket connection has its own E2EE channel that manages the // handshake and encrypt/decrypt lifecycle. Keyed by WebSocket instance. private e2eeChannels = new Map() + // Why: stable per-WebSocket id used as the cleanup key for streaming + // subscriptions, so the server can reap a closing socket's subscriptions + // without affecting other live sockets that share the same deviceToken. + private wsConnectionIds = new Map() // Why: separate from Node's server.maxConnections because we need to count // only long-running dispatches, not every in-flight short RPC. See §3.1 + // §7 risk #2. @@ -220,6 +224,11 @@ export class OrcaRuntimeRpcServer { wsTransport.onMessage((msg, _reply, ws) => { let channel = this.e2eeChannels.get(ws) if (!channel) { + // Why: stable per-ws id used as the cleanup-index key for + // streaming subscriptions, so the server can reap them exactly + // when this socket closes (without affecting other live sockets + // that share the same deviceToken). + this.wsConnectionIds.set(ws, randomBytes(8).toString('hex')) channel = new E2EEChannel(ws, { serverSecretKey: this.e2eeKeypair!.secretKey, validateToken: (token) => this.deviceRegistry?.validateToken(token) != null, @@ -251,16 +260,28 @@ export class OrcaRuntimeRpcServer { // Why: when a mobile client disconnects, the runtime must clean up // connection-scoped state like mobile-fit overrides and the E2EE - // channel to prevent orphaned state. - wsTransport.onConnectionClose((clientId) => { - for (const [ws, channel] of this.e2eeChannels) { - if (channel.deviceToken === clientId) { - channel.destroy() - this.e2eeChannels.delete(ws) - break - } + // channel to prevent orphaned state. A single paired device can hold + // multiple concurrent sockets (host screen + accounts screen, etc.), + // so destroy the channel for THIS exact ws and skip the per-client + // teardown when other sockets for the same token are still alive. + wsTransport.onConnectionClose((clientId, ws, hasOtherConnections) => { + // Why: sweep streaming subscriptions for THIS ws regardless of + // hasOtherConnections, so per-ws listeners (notifications, + // accounts, terminal) don't leak across reconnects. This is + // independent of the deviceToken-scoped onClientDisconnected. + const connectionId = this.wsConnectionIds.get(ws) + if (connectionId) { + this.runtime.cleanupSubscriptionsForConnection(connectionId) + this.wsConnectionIds.delete(ws) + } + const channel = this.e2eeChannels.get(ws) + if (channel) { + channel.destroy() + this.e2eeChannels.delete(ws) + } + if (!hasOtherConnections) { + this.runtime.onClientDisconnected(clientId) } - this.runtime.onClientDisconnected(clientId) }) await wsTransport.start() @@ -429,7 +450,8 @@ export class OrcaRuntimeRpcServer { wsTransport.setClientId(ws, token) } - await this.dispatcher.dispatchStreaming(request, reply) + const connectionId = ws ? this.wsConnectionIds.get(ws) : undefined + await this.dispatcher.dispatchStreaming(request, reply, { connectionId }) } private buildError(id: string, code: string, message: string): RpcResponse { diff --git a/src/main/telemetry/classify-error.test.ts b/src/main/telemetry/classify-error.test.ts new file mode 100644 index 00000000000..bad63bb06aa --- /dev/null +++ b/src/main/telemetry/classify-error.test.ts @@ -0,0 +1,43 @@ +import { describe, it, expect } from 'vitest' +import { classifyError } from './classify-error' + +describe('classifyError', () => { + it('classifies ENOENT as binary_not_found', () => { + expect(classifyError(new Error('spawn zsh ENOENT'))).toEqual({ + error_class: 'binary_not_found' + }) + expect(classifyError(Object.assign(new Error('missing'), { code: 'ENOENT' }))).toEqual({ + error_class: 'binary_not_found' + }) + }) + + it('keeps shell-output-shaped not-found messages unknown', () => { + expect(classifyError(new Error('command not found: claude'))).toEqual({ + error_class: 'unknown' + }) + expect(classifyError(new Error('workspace not found'))).toEqual({ + error_class: 'unknown' + }) + expect( + classifyError( + Object.assign(new Error('socket missing'), { code: 'ENOENT', syscall: 'connect' }) + ) + ).toEqual({ error_class: 'unknown' }) + }) + + it('returns unknown for null/undefined input', () => { + expect(classifyError(null)).toEqual({ error_class: 'unknown' }) + expect(classifyError(undefined)).toEqual({ error_class: 'unknown' }) + }) + + it('returns unknown for an unrecognized message', () => { + expect(classifyError(new Error('some unique failure mode we do not know'))).toEqual({ + error_class: 'unknown' + }) + }) + + it('returns unknown for non-Error throws', () => { + expect(classifyError('a bare string')).toEqual({ error_class: 'unknown' }) + expect(classifyError({ random: 'object' })).toEqual({ error_class: 'unknown' }) + }) +}) diff --git a/src/main/telemetry/classify-error.ts b/src/main/telemetry/classify-error.ts new file mode 100644 index 00000000000..06b5700dbe4 --- /dev/null +++ b/src/main/telemetry/classify-error.ts @@ -0,0 +1,44 @@ +// Best-effort classifier from a thrown value to the closed `error_class` +// enum on `agent_error`. The wire never carries a raw error message or stack +// — every transmitted error is bucketed into one of the two enum members +// (`binary_not_found` for ENOENT-shaped failures, `unknown` for everything +// else). A non-zero `unknown` slice on the dashboard is the trigger to add a +// new enum value alongside the call site that would emit it. + +import type { ErrorClass } from '../../shared/telemetry-events' + +export type ClassifiedError = { + error_class: ErrorClass +} + +export function classifyError(err: unknown): ClassifiedError { + if (err === null || err === undefined) { + return { error_class: 'unknown' } + } + + const code = + typeof err === 'object' && err !== null && 'code' in err + ? (err as { code?: unknown }).code + : undefined + const syscall = + typeof err === 'object' && err !== null && 'syscall' in err + ? (err as { syscall?: unknown }).syscall + : undefined + const message = + typeof err === 'object' && err !== null && 'message' in err + ? (err as { message?: unknown }).message + : undefined + + if ( + code === 'ENOENT' && + (typeof syscall !== 'string' || syscall.toLowerCase().startsWith('spawn')) + ) { + return { error_class: 'binary_not_found' } + } + + if (typeof message === 'string' && /\bspawn\b/i.test(message) && /\benoent\b/i.test(message)) { + return { error_class: 'binary_not_found' } + } + + return { error_class: 'unknown' } +} diff --git a/src/main/telemetry/client.test.ts b/src/main/telemetry/client.test.ts index 05b7ff5bfaa..a5361b64395 100644 --- a/src/main/telemetry/client.test.ts +++ b/src/main/telemetry/client.test.ts @@ -26,10 +26,13 @@ import { _setPostHogClientForTests, _setShuttingDownForTests, _setStoreForTests, + _resetFirstAppOpenedFiredForTests, + persistBannerAcknowledgeWithoutEmitting, setOptIn, shouldOptOutSdkAtInit, shutdownTelemetry, - track + track, + trackAppOpenedOnce } from './client' // Minimal mock of the PostHog client surface the wrapper actually calls. @@ -170,12 +173,14 @@ describe('track()', () => { _setStoreForTests(store) _setShuttingDownForTests(false) _enableTransportForTests(true) + _resetFirstAppOpenedFiredForTests() }) afterEach(() => { _enableTransportForTests(false) _setPostHogClientForTests(null) _setCommonPropsForTests(null) _setStoreForTests(null) + _resetFirstAppOpenedFiredForTests() vi.restoreAllMocks() restoreConsentEnv(envStash) }) @@ -270,12 +275,19 @@ describe('track()', () => { it('drops invalid events before calling capture', () => { // Raw error strings on agent_error are rejected by `.strict()`. track('agent_error', { - error_class: 'auth_expired', + error_class: 'unknown', agent_kind: 'claude-code', error_message: 'leaked message' // rejected by .strict() } as never) expect(mock.capture).not.toHaveBeenCalled() }) + + it('trackAppOpenedOnce emits app_opened at most once per session', () => { + trackAppOpenedOnce() + trackAppOpenedOnce() + expect(mock.capture).toHaveBeenCalledTimes(1) + expect(mock.capture.mock.calls[0]![0].event).toBe('app_opened') + }) }) describe('setOptIn()', () => { @@ -301,12 +313,14 @@ describe('setOptIn()', () => { _setStoreForTests(store) _setShuttingDownForTests(false) _enableTransportForTests(true) + _resetFirstAppOpenedFiredForTests() }) afterEach(() => { _enableTransportForTests(false) _setPostHogClientForTests(null) _setCommonPropsForTests(null) _setStoreForTests(null) + _resetFirstAppOpenedFiredForTests() vi.restoreAllMocks() restoreConsentEnv(envStash) }) @@ -333,15 +347,97 @@ describe('setOptIn()', () => { expect(order).toEqual(['capture called', 'sdk enqueue', 'optOut']) }) - it('fires telemetry_opted_in AFTER posthog.optIn()', async () => { + it('fires telemetry_opted_in after posthog.optIn without app_opened for settings opt-in', async () => { // Flip settings to currently-opted-out so the flip to true exercises - // the opt-in branch cleanly. + // the opt-in branch cleanly. This is not the pending-banner path, so + // it must not replay the once-per-session app_opened event. settings.telemetry!.optedIn = false const order: string[] = [] mock.optIn.mockImplementation(async () => order.push('optIn')) - mock.capture.mockImplementation(() => order.push('capture')) + mock.capture.mockImplementation((message: { event?: string }) => { + order.push(`capture:${message.event}`) + }) await setOptIn('settings', true) - expect(order).toEqual(['optIn', 'capture']) + expect(order).toEqual(['optIn', 'capture:telemetry_opted_in']) + }) + + it('console-mirrors telemetry_opted_in when no PostHog client is initialized', async () => { + settings.telemetry!.optedIn = false + _setPostHogClientForTests(null) + _enableTransportForTests(false) + + await setOptIn('settings', true) + + expect(console.debug).toHaveBeenCalledWith('[telemetry]', 'telemetry_opted_in', { + via: 'settings' + }) + }) + + it('fires app_opened once after pending-banner opt-in enables the SDK', async () => { + settings.telemetry = { + optedIn: null, + installId: BASE_COMMON.install_id, + existedBeforeTelemetryRelease: true + } + const order: string[] = [] + mock.optIn.mockImplementation(async () => order.push('optIn')) + mock.capture.mockImplementation((message: { event?: string }) => { + order.push(`capture:${message.event}`) + }) + + await setOptIn('settings', true) + + expect(order).toEqual(['optIn', 'capture:app_opened', 'capture:telemetry_opted_in']) + }) +}) + +describe('persistBannerAcknowledgeWithoutEmitting()', () => { + let mock: MockPostHog + let store: Store + let settings: GlobalSettings + let envStash: Record + + beforeEach(() => { + envStash = stashAndClearConsentEnv() + vi.spyOn(console, 'debug').mockImplementation(() => {}) + vi.spyOn(console, 'warn').mockImplementation(() => {}) + resetBurstCapsForSession() + mock = makeMockPostHog() + settings = makeFakeSettings({ + optedIn: null, + installId: BASE_COMMON.install_id, + existedBeforeTelemetryRelease: true + }) + store = makeFakeStore(settings) + _setPostHogClientForTests(mock as unknown as PostHog) + _setCommonPropsForTests(BASE_COMMON) + _setStoreForTests(store) + _setShuttingDownForTests(false) + _enableTransportForTests(true) + _resetFirstAppOpenedFiredForTests() + }) + + afterEach(() => { + _enableTransportForTests(false) + _setPostHogClientForTests(null) + _setCommonPropsForTests(null) + _setStoreForTests(null) + _resetFirstAppOpenedFiredForTests() + vi.restoreAllMocks() + restoreConsentEnv(envStash) + }) + + it('fires app_opened after re-enabling the SDK and does not emit telemetry_opted_in', async () => { + const order: string[] = [] + mock.optIn.mockImplementation(async () => order.push('optIn')) + mock.capture.mockImplementation((message: { event?: string }) => { + order.push(`capture:${message.event}`) + }) + + await persistBannerAcknowledgeWithoutEmitting() + + expect(order).toEqual(['optIn', 'capture:app_opened']) + expect(settings.telemetry?.optedIn).toBe(true) }) }) diff --git a/src/main/telemetry/client.ts b/src/main/telemetry/client.ts index 8ed1e0880b1..5ee4ac4b956 100644 --- a/src/main/telemetry/client.ts +++ b/src/main/telemetry/client.ts @@ -92,29 +92,10 @@ const OPT_OUT_CAPTURE_ENQUEUE_TIMEOUT_MS = 1_000 // be bounded by `resolveConsent` + the validator. let testTransportEnabled = false -// First-launch `app_opened` gate. The existing-user banner contract in -// telemetry-plan.md:177 is: zero events — *not even* `app_opened` — transmit -// until the user clicks "Sure". The mechanism is: PR 4's `app_opened` call -// site checks `hasFirstAppOpenedFired()` before firing, and `setOptIn(_, -// true)` flips the gate via `markFirstAppOpenedFired()`. -// -// Why this gate exists at all, and why it lives in the client module: -// - The consent resolver already returns `pending_banner` for an existing -// user with `optedIn === null`, so `track()` would drop `app_opened` -// during the pre-consent window anyway. The gate is a belt-and-suspenders -// guard for the transition second: between the moment `setOptIn(_, true)` -// writes `optedIn = true` to disk and the moment the PR 4 call site fires -// `app_opened`, nothing else must sneak a bare `app_opened` onto the wire -// using the now-enabled consent. Keeping the gate here — next to -// `setOptIn` — means the "flip" is one module-local assignment that no -// call-site can bypass accidentally. -// - The gate resets to `false` per session by `initTelemetry()` (via the -// `resetSessionState()` call below). New users who kept default-on do -// not go through `setOptIn` on launch, so PR 4 will also call -// `markFirstAppOpenedFired()` once for non-banner cohorts immediately -// before firing the session's `app_opened`. PR 3 ships only the state -// machinery; PR 4 wires both call sites. -let firstAppOpenedFired = false +// First-launch `app_opened` session gate. The existing-user banner contract is: +// no events transmit until the notice resolves. Keep "mark" and "emit" +// atomic so no path can accidentally suppress the event without firing it. +let appOpenedTrackedThisSession = false function buildCommonProps(installId: string, sid: string, channel: 'stable' | 'rc'): CommonProps { // `.max(64)` on every free-form string field in `commonPropsSchema` is the @@ -140,10 +121,9 @@ export function initTelemetry(store: Store): void { storeRef = store resetBurstCapsForSession() shuttingDown = false - // Gate reset per session: the "no app_opened until Sure" invariant is - // per-launch, not across the lifetime of the install. See the comment on - // `firstAppOpenedFired` above. - firstAppOpenedFired = false + // Gate reset per session: the "no app_opened until banner resolution" + // invariant is per-launch, not across the lifetime of the install. + appOpenedTrackedThisSession = false if (!TELEMETRY_ENABLED || !IS_OFFICIAL_BUILD) { return @@ -335,6 +315,10 @@ export async function setOptIn(via: OptInVia, optedIn: boolean): Promise { return } const settings = storeRef.getSettings() + const telemetryBeforeUpdate = settings.telemetry + const wasPendingBanner = + telemetryBeforeUpdate?.existedBeforeTelemetryRelease === true && + telemetryBeforeUpdate.optedIn === null // `updateSettings` is a partial-merge (see persistence.ts:552). The Store's // `telemetry` field is deep-merged there specifically so an `optedIn` flip // from the Privacy pane / consent flow does not clobber `installId` or @@ -346,26 +330,19 @@ export async function setOptIn(via: OptInVia, optedIn: boolean): Promise { } }) - // Unlock the first-session `app_opened` after a successful opt-in. The - // PR 4 call site is responsible for consulting `hasFirstAppOpenedFired()` - // before firing; flipping the gate here is what makes the existing-user - // "Sure, help improve Orca" path work — telemetry-plan.md:177 is explicit - // that `app_opened` fires only after that click, and this assignment is - // the "completes" half of that invariant. Flipping before the - // `!posthog` early-return keeps console-mirror builds consistent with - // transmitting builds (the gate is semantic, not transport-gated). - if (optedIn) { - firstAppOpenedFired = true - } - const client = posthog - if (!client) { - return - } if (optedIn) { - await client.optIn() + if (client) { + await client.optIn() + } + if (wasPendingBanner) { + trackAppOpenedOnce() + } track('telemetry_opted_in', { via }) } else { + if (!client) { + return + } // Fire opt-out event BEFORE disabling the SDK. This is the one event // that transmits against the user's new preference — the user chose to // tell us they are opting out, and that single signal is what tells us @@ -412,24 +389,26 @@ export async function setOptIn(via: OptInVia, optedIn: boolean): Promise { } } -// Banner ✕ path. Writes `optedIn = true` permanently and emits NO event. +// Banner ✕ path. Writes `optedIn = true` permanently without emitting a +// telemetry opt-in event. `app_opened` still fires because resolving the +// banner is the first point where this session is eligible to transmit. // That outcome cannot route through `setOptIn()` — `setOptIn()` always // fires a `telemetry_opted_in/out` event and the IPC handler always // derives a non-`null` `via` value, which would tag a ✕ click as // `first_launch_banner` + `telemetry_opted_in`. The ✕-as-silent- // acknowledge contract is explicit: the user did not explicitly opt in, -// they declined to intervene, so no event transmits. +// they declined to intervene, so no opt-in event transmits. // // So this primitive exists as a named, non-overloaded code path: persist -// the opt-in, flip the first-app-opened gate, unlock the SDK, and emit -// nothing. The corresponding `telemetry:acknowledgeBanner` IPC channel +// the opt-in, unlock the SDK, and fire the once-per-session app-opened event. +// The corresponding `telemetry:acknowledgeBanner` IPC channel // routes renderer ✕ clicks here instead of through `telemetry:setOptIn`. // // Do NOT extend this with a `via` parameter or emission flag. If a future // surface also needs a silent persisted opt-in, give it its own named // function rather than overloading this one — the grep'ability of // `persistBannerAcknowledgeWithoutEmitting` is the whole point. -export function persistBannerAcknowledgeWithoutEmitting(): void { +export async function persistBannerAcknowledgeWithoutEmitting(): Promise { if (!storeRef) { return } @@ -445,31 +424,20 @@ export function persistBannerAcknowledgeWithoutEmitting(): void { optedIn: true } }) - // Mirror the opt-in half of `setOptIn`: flip the first-app-opened gate - // and re-enable the SDK. Without these, a ✕ acknowledge would persist - // `optedIn: true` on disk but leave the in-memory SDK flag opted-out - // (seeded by `initTelemetry` for pre-banner users whose consent resolver - // returned `pending_banner`), so the next `track()` would drop despite - // the persisted opt-in. The first-app-opened gate is the "no events - // until the user resolves the banner" half of the contract — flipping - // it here unlocks PR 4's `app_opened` call site the same way `setOptIn` - // does for the Turn-off path. - firstAppOpenedFired = true if (posthog) { - posthog.optIn() + await posthog.optIn() } + // Why: resolving the banner is the first eligible moment for app_opened. + // Re-enable the SDK first so capture sees the new consent state. + trackAppOpenedOnce() } -// First-launch `app_opened` gate accessors. Used by PR 4's `app_opened` -// call site to enforce the "no events transmit for existing users until -// Sure" invariant in telemetry-plan.md:177. PR 3 ships the state machinery -// only — the call site itself lands in PR 4. -export function hasFirstAppOpenedFired(): boolean { - return firstAppOpenedFired -} - -export function markFirstAppOpenedFired(): void { - firstAppOpenedFired = true +export function trackAppOpenedOnce(): void { + if (appOpenedTrackedThisSession) { + return + } + appOpenedTrackedThisSession = true + track('app_opened', {}) } export async function shutdownTelemetry(): Promise { @@ -521,5 +489,5 @@ export function _enableTransportForTests(enabled: boolean): void { } export function _resetFirstAppOpenedFiredForTests(): void { - firstAppOpenedFired = false + appOpenedTrackedThisSession = false } diff --git a/src/main/telemetry/validator.test.ts b/src/main/telemetry/validator.test.ts index 3da5b021ba2..d6619448f34 100644 --- a/src/main/telemetry/validator.test.ts +++ b/src/main/telemetry/validator.test.ts @@ -49,7 +49,7 @@ describe('validate', () => { // transmits. it('rejects error_message on agent_error', () => { const result = validate('agent_error', { - error_class: 'auth_expired', + error_class: 'unknown', agent_kind: 'claude-code', error_message: 'at /Users/alice/secret/path/index.ts:42' } as never) @@ -58,7 +58,7 @@ describe('validate', () => { it('rejects error_stack on agent_error', () => { const result = validate('agent_error', { - error_class: 'auth_expired', + error_class: 'unknown', agent_kind: 'claude-code', error_stack: 'Error: boom\n at /Users/alice/...' } as never) @@ -83,36 +83,21 @@ describe('validate', () => { expect(result.ok).toBe(false) }) - it('drops overlength strings past the .max() cap', () => { - // commonPropsSchema's `.max(64)` caps don't live on per-event schemas — - // the per-event schemas use enum-only strings — so we exercise the cap - // via a whitelisted `error_name` that is *one specific enum value* - // meaning any attempt to smuggle a long string fails the enum check - // regardless. For explicit string-length cap coverage we also confirm - // the agent_error schema does not accept an overlength error_name - // even if it matches the prefix of a whitelisted value. + it('accepts a well-formed agent_error payload', () => { const result = validate('agent_error', { - error_class: 'auth_expired', - agent_kind: 'claude-code', - error_name: 'AuthExpiredButAlsoWithExtraGarbageThatMakesItTooLong' - } as never) - expect(result.ok).toBe(false) - }) - - it('accepts whitelisted error_name on agent_error', () => { - const result = validate('agent_error', { - error_class: 'auth_expired', - agent_kind: 'claude-code', - error_name: 'AuthExpired' + error_class: 'binary_not_found', + agent_kind: 'claude-code' }) expect(result.ok).toBe(true) }) - it('rejects error_name outside the whitelist', () => { + it('rejects agent_error with a deferred enum value', () => { + // `auth_expired` was in an earlier draft; the trimmed enum is + // ['binary_not_found', 'unknown']. Pin that contract so re-introducing + // a deferred value silently is impossible. const result = validate('agent_error', { error_class: 'auth_expired', - agent_kind: 'claude-code', - error_name: 'CustomErrorNotInList' + agent_kind: 'claude-code' } as never) expect(result.ok).toBe(false) }) diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index e5aaf567b87..446ff39bd81 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -149,6 +149,7 @@ import type { CodexUsageSummary } from '../shared/codex-usage-types' import type { TelemetryConsentState } from '../shared/telemetry-consent-types' +import type { AgentKind, LaunchSource, RequestKind } from '../shared/telemetry-events' export type BrowserApi = { registerGuest: (args: { @@ -449,6 +450,11 @@ export type PreloadApi = { // 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 + // Why: telemetry-plan.md§Agent launch semantics — main emits + // `agent_started` only after the PTY/session is created successfully, + // so the renderer threads the launch metadata through this field and + // the IPC handler fires the event from the spawn-success branch. + telemetry?: { agent_kind: AgentKind; launch_source: LaunchSource; request_kind: RequestKind } }) => Promise<{ id: string snapshot?: string @@ -499,7 +505,11 @@ export type PreloadApi = { gh: { viewer: () => Promise repoSlug: (args: { repoPath: string }) => Promise<{ owner: string; repo: string } | null> - prForBranch: (args: { repoPath: string; branch: string }) => Promise + prForBranch: (args: { + repoPath: string + branch: string + linkedPRNumber?: number | null + }) => Promise issue: (args: { repoPath: string; number: number }) => Promise workItem: (args: { repoPath: string @@ -603,9 +613,7 @@ export type PreloadApi = { updateProjectItemField: ( args: UpdateProjectItemFieldArgs ) => Promise - clearProjectItemField: ( - args: ClearProjectItemFieldArgs - ) => Promise + clearProjectItemField: (args: ClearProjectItemFieldArgs) => Promise updateIssueBySlug: (args: UpdateIssueBySlugArgs) => Promise updatePullRequestBySlug: ( args: UpdatePullRequestBySlugArgs @@ -624,9 +632,7 @@ export type PreloadApi = { args: ListAssignableUsersBySlugArgs ) => Promise listIssueTypesBySlug: (args: ListIssueTypesBySlugArgs) => Promise - updateIssueTypeBySlug: ( - args: UpdateIssueTypeBySlugArgs - ) => Promise + updateIssueTypeBySlug: (args: UpdateIssueTypeBySlugArgs) => Promise } linear: { connect: (args: { @@ -752,8 +758,9 @@ export type PreloadApi = { } sidekick: { import: () => Promise - read: (id: string, fileName: string) => Promise - delete: (id: string, fileName: string) => Promise + importPetBundle: () => Promise + read: (id: string, fileName: string, kind?: 'image' | 'bundle') => Promise + delete: (id: string, fileName: string, kind?: 'image' | 'bundle') => Promise } browser: BrowserApi hooks: { @@ -1161,11 +1168,15 @@ export type PreloadApi = { listNetworkInterfaces: () => Promise<{ interfaces: { name: string; address: string }[] }> - getPairingQR: (args?: { - address?: string - }) => Promise< + getPairingQR: (args?: { address?: string }) => Promise< | { available: false } - | { available: true; qrDataUrl: string; endpoint: string; deviceId: string } + | { + available: true + qrDataUrl: string + pairingUrl: string + endpoint: string + deviceId: string + } > listDevices: () => Promise<{ devices: { deviceId: string; name: string; pairedAt: number; lastSeenAt: number }[] diff --git a/src/preload/index.ts b/src/preload/index.ts index 2a62a9e4332..db9f0d01664 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -63,6 +63,7 @@ import type { } from '../shared/ssh-types' import type { AgentStatusState } from '../shared/agent-status-types' import type { TelemetryConsentState } from '../shared/telemetry-consent-types' +import type { AgentKind, LaunchSource, RequestKind } from '../shared/telemetry-events' import { ORCA_EDITOR_SAVE_DIRTY_FILES_EVENT, type EditorSaveDirtyFilesDetail @@ -383,6 +384,12 @@ const api = { worktreeId?: string sessionId?: string shellOverride?: string + // Why: telemetry-plan.md§Agent launch semantics — main fires + // `agent_started` only after the spawn succeeds. The renderer is the + // source of truth for the launch metadata; main is the source of + // truth for whether the launch happened. Loose typing here on + // purpose: validation lives at the main-side schema validator. + telemetry?: { agent_kind: AgentKind; launch_source: LaunchSource; request_kind: RequestKind } }): Promise<{ id: string snapshot?: string @@ -522,8 +529,11 @@ const api = { repoSlug: (args: { repoPath: string }): Promise => ipcRenderer.invoke('gh:repoSlug', args), - prForBranch: (args: { repoPath: string; branch: string }): Promise => - ipcRenderer.invoke('gh:prForBranch', args), + prForBranch: (args: { + repoPath: string + branch: string + linkedPRNumber?: number | null + }): Promise => ipcRenderer.invoke('gh:prForBranch', args), issue: (args: { repoPath: string; number: number }): Promise => ipcRenderer.invoke('gh:issue', args), @@ -678,8 +688,7 @@ const api = { ipcRenderer.invoke('gh:updateProjectItemField', args), clearProjectItemField: ( args: ClearProjectItemFieldArgs - ): Promise => - ipcRenderer.invoke('gh:clearProjectItemField', args), + ): Promise => ipcRenderer.invoke('gh:clearProjectItemField', args), updateIssueBySlug: (args: UpdateIssueBySlugArgs): Promise => ipcRenderer.invoke('gh:updateIssueBySlug', args), updatePullRequestBySlug: ( @@ -708,8 +717,7 @@ const api = { ipcRenderer.invoke('gh:listIssueTypesBySlug', args), updateIssueTypeBySlug: ( args: UpdateIssueTypeBySlugArgs - ): Promise => - ipcRenderer.invoke('gh:updateIssueTypeBySlug', args) + ): Promise => ipcRenderer.invoke('gh:updateIssueTypeBySlug', args) }, linear: { @@ -978,10 +986,12 @@ const api = { sidekick: { import: (): Promise => ipcRenderer.invoke('sidekick:import'), - read: (id: string, fileName: string): Promise => - ipcRenderer.invoke('sidekick:read', id, fileName), - delete: (id: string, fileName: string): Promise => - ipcRenderer.invoke('sidekick:delete', id, fileName) + importPetBundle: (): Promise => + ipcRenderer.invoke('sidekick:importPetBundle'), + read: (id: string, fileName: string, kind?: 'image' | 'bundle'): Promise => + ipcRenderer.invoke('sidekick:read', id, fileName, kind), + delete: (id: string, fileName: string, kind?: 'image' | 'bundle'): Promise => + ipcRenderer.invoke('sidekick:delete', id, fileName, kind) }, browser: { @@ -2125,7 +2135,13 @@ const api = { address?: string }): Promise< | { available: false } - | { available: true; qrDataUrl: string; endpoint: string; deviceId: string } + | { + available: true + qrDataUrl: string + pairingUrl: string + endpoint: string + deviceId: string + } > => ipcRenderer.invoke('mobile:getPairingQR', args), listDevices: (): Promise<{ diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index 7316a026b37..30560b8dab3 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -957,118 +957,130 @@ function App(): React.JSX.Element { The hooks' internal early-returns remain as defense-in-depth (see the comment above useIpcEvents()). */} {agentDashboardEnabled ? : null} - {/* Why: in workspace view (split groups always enabled), the full-width - titlebar is removed so tab groups + terminal extend to the top of - the window. Left titlebar controls move to a header above the sidebar. - Settings, landing, and the tasks page keep the full-width titlebar. */} - {!workspaceActive ? ( -
-
- {titlebarLeftControls} -
-
- {showTitlebarExpandButton && ( - - - - - - Collapse pane - - - )} - {rightSidebarToggle} -
- ) : null}
- {showSidebar ? ( - workspaceActive ? ( - /* Why: left column wraps the sidebar with a titlebar-height - header above it. The header holds the same controls - (traffic lights, sidebar toggle, "Orca" title, agent badge) - that the full-width titlebar held while the center and right - columns keep their own top strips at the same 42px height. - When the sidebar is collapsed, take this header out of flex - layout so the terminal/editor reclaim the left edge instead of - leaving behind a content-width blank strip. */ -
+ {/* Why: the non-workspace titlebar lives inside this left+center + wrapper so it does not span over the right-sidebar column — + when the right sidebar is open, its own header anchors at the + top alongside the titlebar instead of being pushed below it. */} +
+ {/* Why: in workspace view (split groups always enabled), the + full-width titlebar is removed so tab groups + terminal extend + to the top of the window. Left titlebar controls move to a + header above the sidebar. Settings, landing, and the tasks + page keep the titlebar. */} + {!workspaceActive ? ( +
{titlebarLeftControls}
-
- {/* Why: the workspace-view wrapper adds a fixed 42px header - above the sidebar. Without a flex-1/min-h-0 slot here, - the sidebar falls back to its content height, so the - worktree list loses its scroll viewport and the fixed - bottom toolbar (including Add Project) gets pushed offscreen. */} +
+ {showTitlebarExpandButton && ( + + + + + + Collapse pane + + + )} + {/* Why: when the right sidebar is open, its own header renders + an identical close button — hide this copy so only one is + visible at a time. */} + {!rightSidebarOpen && rightSidebarToggle} +
+ ) : null} +
+ {showSidebar ? ( + workspaceActive ? ( + /* Why: left column wraps the sidebar with a titlebar-height + header above it. The header holds the same controls + (traffic lights, sidebar toggle, "Orca" title, agent badge) + that the full-width titlebar held while the center and right + columns keep their own top strips at the same 36px height. + When the sidebar is collapsed, take this header out of flex + layout so the terminal/editor reclaim the left edge instead of + leaving behind a content-width blank strip. */ +
+
+ {titlebarLeftControls} +
+
+ {/* Why: the workspace-view wrapper adds a fixed 36px header + above the sidebar. Without a flex-1/min-h-0 slot here, + the sidebar falls back to its content height, so the + worktree list loses its scroll viewport and the fixed + bottom toolbar (including Add Project) gets pushed offscreen. */} + +
+
+ ) : ( + ) + ) : null} +
+ {/* Why: right sidebar toggle floats at the top-right of the center + column so it's always accessible whether the right sidebar is + open or closed. Match the RightSidebar header's 36px height and + top-0 anchor so the icon's vertical center is identical between + open and closed states — otherwise toggling makes the icon jump + a few pixels, which reads as layout jitter. */} + {workspaceActive && !rightSidebarOpen && ( +
+ {rightSidebarToggle} +
+ )} +
+
+ +
+ + {activeView === 'settings' ? : null} + {activeView === 'tasks' ? : null} + {activeView === 'terminal' && !activeWorktreeId ? : null} +
- ) : ( - - ) - ) : null} -
- {/* Why: right sidebar toggle floats at the top-right of the center - column so it's always accessible whether the right sidebar is - open or closed. Match the RightSidebar header's 42px height and - top-0 anchor so the icon's vertical center is identical between - open and closed states — otherwise toggling makes the icon jump - a few pixels, which reads as layout jitter. */} - {workspaceActive && !rightSidebarOpen && ( -
- {rightSidebarToggle} -
- )} -
-
- -
- - {activeView === 'settings' ? : null} - {activeView === 'tasks' ? : null} - {activeView === 'terminal' && !activeWorktreeId ? : null} -
{/* Why: keep RightSidebar mounted even when closed so that its diff --git a/src/renderer/src/assets/main.css b/src/renderer/src/assets/main.css index ebc86413453..5ddadaebf4f 100644 --- a/src/renderer/src/assets/main.css +++ b/src/renderer/src/assets/main.css @@ -359,8 +359,8 @@ } .titlebar { - height: 42px; - min-height: 42px; + height: 36px; + min-height: 36px; display: flex; flex-direction: row; align-items: center; diff --git a/src/renderer/src/components/FirstLaunchBanner.tsx b/src/renderer/src/components/FirstLaunchBanner.tsx index 7335c197ded..8a03379786e 100644 --- a/src/renderer/src/components/FirstLaunchBanner.tsx +++ b/src/renderer/src/components/FirstLaunchBanner.tsx @@ -11,7 +11,7 @@ // // Three actions, two semantics: // - "Got it" and the ✕ in the corner → silent acknowledge. Both persist -// `optedIn: true`, fire nothing, route through +// `optedIn: true`, fire no opt-in event, route through // `window.api.telemetryAcknowledgeBanner()` to a dedicated main-side // channel so no `via` derivation can tag this path. Two surfaces for // the same action because the ✕ alone is easy to miss; "Got it" is @@ -64,16 +64,24 @@ export function FirstLaunchBanner({ } setInFlight(true) // Main's `telemetry:acknowledgeBanner` handler persists `optedIn: true` - // silently (no event) and intentionally does NOT broadcast + // without an opt-in event and intentionally does NOT broadcast // `settings:changed` (see src/main/ipc/telemetry.ts). Without an // explicit `fetchSettings()` refresh, the renderer store would retain // `optedIn: null` and PrivacyPane would keep rendering its pending- // banner helper text until the next full relaunch. Mirror // PrivacyPane's handleToggle pattern which refetches for the same // reason before surfacing UI changes. - await acknowledgeBanner() - await fetchSettings() - onResolve() + try { + await acknowledgeBanner() + await fetchSettings() + onResolve() + } finally { + // Why: if `fetchSettings` rejects (IPC error during shutdown, + // settings file lock, etc.), `onResolve` never runs and the banner + // stays mounted. Without resetting `inFlight`, every button stays + // permanently disabled for the rest of the session. + setInFlight(false) + } } const handleTurnOff = async (): Promise => { @@ -86,9 +94,13 @@ export function FirstLaunchBanner({ // `setOptIn` (client.ts). The renderer just needs to route through // `telemetrySetOptIn(false)` so the IPC handler derives the correct // `via` and fires the event. - await telemetrySetOptIn(false) - await fetchSettings() - onResolve() + try { + await telemetrySetOptIn(false) + await fetchSettings() + onResolve() + } finally { + setInFlight(false) + } } return ( @@ -103,7 +115,7 @@ export function FirstLaunchBanner({ // `relative` is load-bearing: the absolutely-positioned ✕ anchors to // this container.
-

Help us figure out what to build next

+

Help us decide what to build next

- We'd like to start sending anonymous counts of which features you use and where - things break — no file contents, prompts, terminal output, or anything that identifies - you. Change this anytime in Settings → Privacy & Telemetry.{' '} + Anonymous counts of which features you use help us prioritize what to build. No file + contents, prompts, terminal output, or anything that identifies you. Change anytime in + Settings → Privacy & Telemetry.{' '}