diff --git a/docs/resource-usage-merge-spec.md b/docs/resource-usage-merge-spec.md index 66fac7700..46e566125 100644 --- a/docs/resource-usage-merge-spec.md +++ b/docs/resource-usage-merge-spec.md @@ -385,4 +385,3 @@ CPU and worktrees within a repo by their CPU. - 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/src/cli/args.ts b/src/cli/args.ts index b97308b32..7f13996de 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 01d6a6e51..0fe3e151b 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/runner.test.ts b/src/main/git/runner.test.ts index 637de12f4..e83af8c11 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 1be5acc94..fa18e5918 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, @@ -406,10 +395,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 +403,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/github/project-view.test.ts b/src/main/github/project-view.test.ts index 5630b7615..383fcb1d4 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 549bc9897..32eabb523 100644 --- a/src/main/github/project-view.ts +++ b/src/main/github/project-view.ts @@ -132,7 +132,9 @@ type RawProjectV2Field = { } } -export function normalizeField(raw: RawProjectV2Field | null | undefined): GitHubProjectField | null { +export function normalizeField( + raw: RawProjectV2Field | null | undefined +): GitHubProjectField | null { if (!raw || typeof raw.id !== 'string' || typeof raw.name !== 'string') { return null } @@ -184,7 +186,9 @@ type RawUser = { } function normalizeUser(raw: RawUser | null | undefined): GitHubProjectUser | null { - if (!raw || typeof raw.login !== 'string') {return null} + if (!raw || typeof raw.login !== 'string') { + return null + } return { login: raw.login, name: raw.name ?? null, @@ -195,7 +199,9 @@ function normalizeUser(raw: RawUser | null | undefined): GitHubProjectUser | nul type RawLabel = { name?: string; color?: string } function normalizeLabel(raw: RawLabel | null | undefined): GitHubProjectLabel | null { - if (!raw || typeof raw.name !== 'string') {return null} + if (!raw || typeof raw.name !== 'string') { + return null + } return { name: raw.name, color: raw.color ?? '' } } @@ -216,12 +222,18 @@ type RawFieldValue = { users?: { nodes?: RawUser[] } } -export function normalizeFieldValue(raw: RawFieldValue | null | undefined): GitHubProjectFieldValue | null { - if (!raw || !raw.field || typeof raw.field.id !== 'string') {return null} +export function normalizeFieldValue( + raw: RawFieldValue | null | undefined +): GitHubProjectFieldValue | null { + if (!raw || !raw.field || typeof raw.field.id !== 'string') { + return null + } const fieldId = raw.field.id switch (raw.__typename) { case 'ProjectV2ItemFieldSingleSelectValue': - if (typeof raw.optionId !== 'string') {return null} + if (typeof raw.optionId !== 'string') { + return null + } return { kind: 'single-select', fieldId, @@ -230,7 +242,9 @@ export function normalizeFieldValue(raw: RawFieldValue | null | undefined): GitH color: raw.color ?? '' } case 'ProjectV2ItemFieldIterationValue': - if (typeof raw.iterationId !== 'string') {return null} + if (typeof raw.iterationId !== 'string') { + return null + } return { kind: 'iteration', fieldId, @@ -242,7 +256,9 @@ export function normalizeFieldValue(raw: RawFieldValue | null | undefined): GitH case 'ProjectV2ItemFieldTextValue': return { kind: 'text', fieldId, text: raw.text ?? '' } case 'ProjectV2ItemFieldNumberValue': - if (typeof raw.number !== 'number') {return null} + if (typeof raw.number !== 'number') { + return null + } return { kind: 'number', fieldId, number: raw.number } case 'ProjectV2ItemFieldDateValue': return { kind: 'date', fieldId, date: raw.date ?? '' } @@ -279,7 +295,12 @@ type RawContent = { assignees?: { nodes?: RawUser[] } labels?: { nodes?: RawLabel[] } parent?: { number?: number; title?: string; url?: string } | null - issueType?: { id?: string; name?: string; color?: string | null; description?: string | null } | null + issueType?: { + id?: string + name?: string + color?: string | null + description?: string | null + } | null } type RawItem = { @@ -298,10 +319,18 @@ type NormalizedItemOutcome = | { ok: false; drift: GitHubProjectViewError } function mapItemType(raw: string | undefined, hasContent: boolean): GitHubProjectRowItemType { - if (raw === 'ISSUE') {return 'ISSUE'} - if (raw === 'PULL_REQUEST') {return 'PULL_REQUEST'} - if (raw === 'DRAFT_ISSUE') {return 'DRAFT_ISSUE'} - if (raw === 'REDACTED' || !hasContent) {return 'REDACTED'} + if (raw === 'ISSUE') { + return 'ISSUE' + } + if (raw === 'PULL_REQUEST') { + return 'PULL_REQUEST' + } + if (raw === 'DRAFT_ISSUE') { + return 'DRAFT_ISSUE' + } + if (raw === 'REDACTED' || !hasContent) { + return 'REDACTED' + } // Unknown item type with content — treat as redacted rather than dropping. return 'REDACTED' } @@ -535,12 +564,16 @@ async function fetchProjectViewsPage(args: { ${FIELD_CONFIG_FRAGMENT} ` const vars: GraphqlVars = { owner: args.owner, num: args.projectNumber } - if (args.after) {vars.after = args.after} + if (args.after) { + vars.after = args.after + } const res = await runGraphql>( query, vars ) - if (!res.ok) {return res} + if (!res.ok) { + return res + } const top = res.data[root] const project = top?.projectV2 ?? null if (!project || typeof project.id !== 'string') { @@ -560,7 +593,9 @@ async function fetchProjectViewsPage(args: { async function fetchViewFieldsContinuation( viewId: string, after: string -): Promise<{ ok: true; fields: RawProjectV2Field[] } | { ok: false; error: GitHubProjectViewError }> { +): Promise< + { ok: true; fields: RawProjectV2Field[] } | { ok: false; error: GitHubProjectViewError } +> { // Why: address the view directly via `node(id:)` instead of re-fetching the // whole project + walking views every page. Previous shape paid an // unnecessary `${VIEWS_PAGE_SIZE}` views fan-out per field-continuation @@ -594,14 +629,14 @@ async function fetchViewFieldsContinuation( } } | null }>(query, { viewId, after: cursor }) - if (!res.ok) {return res} + if (!res.ok) { + return res + } const view = res.data.node ?? null if (!view) { return { ok: false, error: driftError('view disappeared during field pagination') } } - const nodes = (view.fields?.nodes ?? []).filter( - (f): f is RawProjectV2Field => f !== null - ) + const nodes = (view.fields?.nodes ?? []).filter((f): f is RawProjectV2Field => f !== null) collected.push(...nodes) const pi = view.fields?.pageInfo cursor = pi?.hasNextPage === true && typeof pi.endCursor === 'string' ? pi.endCursor : null @@ -621,18 +656,26 @@ function finalizeView( const all = [...(raw.fields?.nodes ?? []), ...extraFields.map((f) => f as RawProjectV2Field)] for (const f of all) { const n = normalizeField(f) - if (n) {fields.push(n)} + if (n) { + fields.push(n) + } } const groupByFields: GitHubProjectField[] = [] for (const f of raw.groupByFields?.nodes ?? []) { const n = normalizeField(f) - if (n) {groupByFields.push(n)} + if (n) { + groupByFields.push(n) + } } const sortByFields: GitHubProjectSort[] = [] for (const s of raw.sortByFields?.nodes ?? []) { - if (!s || (s.direction !== 'ASC' && s.direction !== 'DESC')) {continue} + if (!s || (s.direction !== 'ASC' && s.direction !== 'DESC')) { + continue + } const n = normalizeField(s.field) - if (n) {sortByFields.push({ direction: s.direction, field: n })} + if (n) { + sortByFields.push({ direction: s.direction, field: n }) + } } return { ok: true, @@ -656,9 +699,15 @@ function matchesSelector( raw: RawProjectView, sel: { viewId?: string; viewNumber?: number; viewName?: string } ): 'none' | 'id' | 'number' | 'name' | 'default' { - if (sel.viewId && raw.id === sel.viewId) {return 'id'} - if (sel.viewNumber !== undefined && raw.number === sel.viewNumber) {return 'number'} - if (sel.viewName && raw.name === sel.viewName) {return 'name'} + if (sel.viewId && raw.id === sel.viewId) { + return 'id' + } + if (sel.viewNumber !== undefined && raw.number === sel.viewNumber) { + return 'number' + } + if (sel.viewName && raw.name === sel.viewName) { + return 'name' + } if ( sel.viewId === undefined && sel.viewNumber === undefined && @@ -726,7 +775,9 @@ async function fetchItemsPageWithRaw(args: { argsArr.push('-F', `num=${args.projectNumber}`) argsArr.push('-f', `q=${args.query}`) argsArr.push('-F', `first=${args.first}`) - if (args.after) {argsArr.push('-f', `after=${args.after}`)} + if (args.after) { + argsArr.push('-f', `after=${args.after}`) + } const guard = rateLimitGuard('graphql') if (guard.blocked) { @@ -890,7 +941,9 @@ async function fetchAllItems(args: { includeParent: false }) } - if (!first.ok) {return { ok: false, error: first.error }} + if (!first.ok) { + return { ok: false, error: first.error } + } // Drift guards if (first.page.totalCount === undefined || first.page.totalCount === null) { @@ -906,23 +959,33 @@ async function fetchAllItems(args: { // Size cap if (totalCount > MAX_ITEMS) { - return { ok: false, error: { type: 'too_large', message: `View has ${totalCount} items.` }, totalCount } + return { + ok: false, + error: { type: 'too_large', message: `View has ${totalCount} items.` }, + totalCount + } } const rows: GitHubProjectRow[] = [] let position = 0 const appendNodes = (nodes: (RawItem | null)[]): GitHubProjectViewError | null => { for (const n of nodes) { - if (!n) {continue} + if (!n) { + continue + } const norm = normalizeItem(n, position) - if (!norm.ok) {return norm.drift} + if (!norm.ok) { + return norm.drift + } rows.push(norm.row) position++ } return null } const e1 = appendNodes(first.page.nodes) - if (e1) {return { ok: false, error: e1, totalCount }} + if (e1) { + return { ok: false, error: e1, totalCount } + } // Paginate let hasNext = first.page.pageInfo.hasNextPage === true @@ -944,7 +1007,9 @@ async function fetchAllItems(args: { after: cursor as string, includeParent }) - if (!next.ok) {return { ok: false, error: next.error, totalCount }} + if (!next.ok) { + return { ok: false, error: next.error, totalCount } + } if (!Array.isArray(next.page.nodes)) { return { ok: false, error: driftError('items.nodes missing on follow page'), totalCount } } @@ -956,7 +1021,9 @@ async function fetchAllItems(args: { } } const e2 = appendNodes(next.page.nodes) - if (e2) {return { ok: false, error: e2, totalCount }} + if (e2) { + return { ok: false, error: e2, totalCount } + } hasNext = next.page.pageInfo.hasNextPage === true cursor = next.page.pageInfo.endCursor if (hasNext && typeof cursor !== 'string') { @@ -991,7 +1058,9 @@ async function fetchItemsCountOnly(args: { const res = await runGraphql< Record >(query, { owner: args.owner, num: args.projectNumber, q: args.query }) - if (!res.ok) {return null} + if (!res.ok) { + return null + } const count = res.data[root]?.projectV2?.items?.totalCount return typeof count === 'number' ? count : null } @@ -1002,9 +1071,13 @@ export async function getProjectViewTable( args: GetProjectViewTableArgs ): Promise { const ownerCheck = assertSlug(args.owner, 'owner') - if (!ownerCheck.ok) {return { ok: false, error: ownerCheck.error }} + if (!ownerCheck.ok) { + return { ok: false, error: ownerCheck.error } + } const numCheck = assertPositiveInt(args.projectNumber, 'projectNumber') - if (!numCheck.ok) {return { ok: false, error: numCheck.error }} + if (!numCheck.ok) { + return { ok: false, error: numCheck.error } + } if (args.ownerType !== 'organization' && args.ownerType !== 'user') { return { ok: false, @@ -1025,7 +1098,9 @@ export async function getProjectViewTable( projectNumber: args.projectNumber, after: cursor }) - if (!page.ok) {return { ok: false, error: page.error }} + if (!page.ok) { + return { ok: false, error: page.error } + } project = page.project for (const v of page.views) { viewsSeen.push(v) @@ -1034,7 +1109,9 @@ export async function getProjectViewTable( viewNumber: args.viewNumber, viewName: args.viewName }) - if (m === 'none') {continue} + if (m === 'none') { + continue + } // Precedence: id > number > name > default. const rank: Record = { id: 4, number: 3, name: 2, default: 1 } const currentRank = matchStrength ? rank[matchStrength] : 0 @@ -1053,10 +1130,16 @@ export async function getProjectViewTable( // selector promotes a 'default' to a stronger match within the same // selector input — those ranks only matter when the caller supplied // a selector. Bail early on any non-null selectedRaw. - if (selectedRaw) {break} - if (!page.hasNextPage) {break} + if (selectedRaw) { + break + } + if (!page.hasNextPage) { + break + } cursor = page.endCursor - if (typeof cursor !== 'string') {break} + if (typeof cursor !== 'string') { + break + } } if (!project) { return { ok: false, error: { type: 'not_found', message: 'Project not found.' } } @@ -1070,12 +1153,16 @@ export async function getProjectViewTable( const fieldsPi = selectedRaw.fields?.pageInfo if (fieldsPi?.hasNextPage === true && typeof fieldsPi.endCursor === 'string' && selectedRaw.id) { const cont = await fetchViewFieldsContinuation(selectedRaw.id, fieldsPi.endCursor) - if (!cont.ok) {return { ok: false, error: cont.error }} + if (!cont.ok) { + return { ok: false, error: cont.error } + } extraFields = cont.fields } const finalized = finalizeView(selectedRaw, extraFields) - if (!finalized.ok) {return { ok: false, error: finalized.drift }} + if (!finalized.ok) { + return { ok: false, error: finalized.drift } + } const selectedView = finalized.view // Why: an explicit empty-string override means "no filter"; treat undefined @@ -1196,7 +1283,9 @@ export async function listAccessibleProjects(): Promise(query, vars) if (!res.ok) { // Why: a viewer-level failure is structural — if we can't list the @@ -1208,10 +1297,14 @@ export async function listAccessibleProjects(): Promise= DISCOVERY_PROJECTS_PER_OWNER) {break} + if (viewerFetched >= DISCOVERY_PROJECTS_PER_OWNER) { + break + } } const pi = res.data.viewer.projectsV2?.pageInfo viewerMore = pi?.hasNextPage === true && typeof pi.endCursor === 'string' @@ -1263,7 +1358,9 @@ export async function listAccessibleProjects(): Promise(query, vars) if (!res.ok) { // Why: the org-listing query itself failed (not a nested projectsV2). @@ -1276,8 +1373,12 @@ export async function listAccessibleProjects(): Promise= DISCOVERY_MAX_ORGS) {break} + if (!org || typeof org.login !== 'string') { + continue + } + if (orgsSeen >= DISCOVERY_MAX_ORGS) { + break + } orgsSeen++ const login = org.login // Cache owner → ownerType for downstream paste/resolve even when the @@ -1287,8 +1388,12 @@ export async function listAccessibleProjects(): Promise= DISCOVERY_PROJECTS_PER_OWNER) {break} + if (!n || typeof n.id !== 'string' || typeof n.number !== 'number') { + continue + } + if (ownerCount >= DISCOVERY_PROJECTS_PER_OWNER) { + break + } orgProjects.push({ id: n.id, owner: login, @@ -1306,7 +1411,9 @@ export async function listAccessibleProjects(): Promise { const tryOne = async ( ot: GitHubProjectOwnerType, num: number | null - ): Promise< - { ok: true; title: string } | { ok: false; error: GitHubProjectViewError } - > => { + ): Promise<{ ok: true; title: string } | { ok: false; error: GitHubProjectViewError }> => { const root = ownerQueryRoot(ot) // If number is provided, fetch the project title; else just confirm owner exists. const query = num @@ -1381,13 +1495,19 @@ async function resolveOwnerType( } ` const vars: GraphqlVars = { owner } - if (num) {vars.num = num} + if (num) { + vars.num = num + } const res = await runGraphql< Record >(query, vars) - if (!res.ok) {return { ok: false, error: res.error }} + if (!res.ok) { + return { ok: false, error: res.error } + } const top = res.data[root] - if (!top) {return { ok: false, error: { type: 'not_found', message: 'Owner not found.' } }} + if (!top) { + return { ok: false, error: { type: 'not_found', message: 'Owner not found.' } } + } if (num) { const p = top.projectV2 if (!p || typeof p.id !== 'string') { @@ -1407,7 +1527,9 @@ async function resolveOwnerType( const fallback: GitHubProjectOwnerType[] = preferred ? [] : cached - ? (cached === 'organization' ? ['user'] : ['organization']) + ? cached === 'organization' + ? ['user'] + : ['organization'] : [] const ordered = [...candidates, ...fallback] let lastError: GitHubProjectViewError | null = null @@ -1453,7 +1575,9 @@ export async function resolveProjectRef( parsed.kind === 'org' ? 'organization' : parsed.kind === 'user' ? 'user' : null // Verify by fetching project title. const ownerRes = await resolveOwnerType(parsed.owner, preferred) - if (!ownerRes.ok) {return { ok: false, error: ownerRes.error }} + if (!ownerRes.ok) { + return { ok: false, error: ownerRes.error } + } const ownerType = ownerRes.ownerType const root = ownerQueryRoot(ownerType) const query = ` @@ -1464,7 +1588,9 @@ export async function resolveProjectRef( const res = await runGraphql< Record >(query, { owner: parsed.owner, num: parsed.number }) - if (!res.ok) {return { ok: false, error: res.error }} + if (!res.ok) { + return { ok: false, error: res.error } + } const p = res.data[root]?.projectV2 if (!p || typeof p.id !== 'string') { return { ok: false, error: { type: 'not_found', message: 'Project not found.' } } @@ -1490,9 +1616,13 @@ export async function listProjectViews( args: ListProjectViewsArgs ): Promise { const ownerCheck = assertSlug(args.owner, 'owner') - if (!ownerCheck.ok) {return { ok: false, error: ownerCheck.error }} + if (!ownerCheck.ok) { + return { ok: false, error: ownerCheck.error } + } const numCheck = assertPositiveInt(args.projectNumber, 'projectNumber') - if (!numCheck.ok) {return { ok: false, error: numCheck.error }} + if (!numCheck.ok) { + return { ok: false, error: numCheck.error } + } if (args.ownerType !== 'organization' && args.ownerType !== 'user') { return { ok: false, error: { type: 'validation_error', message: 'Invalid ownerType.' } } } @@ -1505,9 +1635,13 @@ export async function listProjectViews( projectNumber: args.projectNumber, after: cursor }) - if (!page.ok) {return { ok: false, error: page.error }} + if (!page.ok) { + return { ok: false, error: page.error } + } for (const v of page.views) { - if (typeof v.id !== 'string' || typeof v.layout !== 'string') {continue} + if (typeof v.id !== 'string' || typeof v.layout !== 'string') { + continue + } summaries.push({ id: v.id, number: typeof v.number === 'number' ? v.number : 0, @@ -1515,9 +1649,13 @@ export async function listProjectViews( layout: v.layout as GitHubProjectViewLayout }) } - if (!page.hasNextPage) {break} + if (!page.hasNextPage) { + break + } cursor = page.endCursor - if (typeof cursor !== 'string') {break} + if (typeof cursor !== 'string') { + break + } } return { ok: true, views: summaries } } diff --git a/src/main/github/project-view/internals.ts b/src/main/github/project-view/internals.ts index 779741aa5..fd69717a6 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 { @@ -208,12 +221,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 +245,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 97cdfeb2a..cd4e0944f 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 138e078f7..1108b6bd2 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 e6d8e979b..790f4cb21 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, diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index 19e8626ff..f9a74b154 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 0fb63b5b8..3c985f4af 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 bf18216a8..15ac89ad2 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 92aecef48..30fe923b1 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/telemetry.test.ts b/src/main/ipc/telemetry.test.ts index d1b2ee363..9e1b9231c 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 ff14d27fb..1bfddc07d 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/worktrees.ts b/src/main/ipc/worktrees.ts index c7372711f..f2c0b806f 100644 --- a/src/main/ipc/worktrees.ts +++ b/src/main/ipc/worktrees.ts @@ -38,6 +38,8 @@ 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' export function registerWorktreeHandlers( mainWindow: BrowserWindow, @@ -143,11 +145,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/telemetry/classify-error.test.ts b/src/main/telemetry/classify-error.test.ts new file mode 100644 index 000000000..bad63bb06 --- /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 000000000..06b5700db --- /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 05b7ff5bf..a5361b643 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 8ed1e0880..5ee4ac4b9 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 3da5b021b..d6619448f 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 4fb6e1acb..c099c4afc 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 diff --git a/src/preload/index.ts b/src/preload/index.ts index b722f0863..8075d3daf 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 diff --git a/src/renderer/src/components/FirstLaunchBanner.tsx b/src/renderer/src/components/FirstLaunchBanner.tsx index 1372acf9d..44fa4123d 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,7 +64,7 @@ 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- diff --git a/src/renderer/src/components/GitHubItemDialog.tsx b/src/renderer/src/components/GitHubItemDialog.tsx index 2b8a86acc..6eef848bd 100644 --- a/src/renderer/src/components/GitHubItemDialog.tsx +++ b/src/renderer/src/components/GitHubItemDialog.tsx @@ -61,15 +61,8 @@ import { type PRCommentGroup } from '@/lib/pr-comment-groups' import { useAppStore } from '@/store' -import { - useRepoLabels, - useRepoAssignees, - useImmediateMutation -} from '@/hooks/useIssueMetadata' -import { - useRepoLabelsBySlug, - useRepoAssigneesBySlug -} from '@/hooks/useGitHubSlugMetadata' +import { useRepoLabels, useRepoAssignees, useImmediateMutation } from '@/hooks/useIssueMetadata' +import { useRepoLabelsBySlug, useRepoAssigneesBySlug } from '@/hooks/useGitHubSlugMetadata' import IssueSourceIndicator, { sameGitHubOwnerRepo } from '@/components/github/IssueSourceIndicator' import type { GitHubOwnerRepo, @@ -1682,14 +1675,12 @@ function MentionTextarea({ // repo. The edit IPCs return a structured `{ ok, error }` shape; we adapt // to a thrown rejection so the existing `useImmediateMutation` flow // (which expects throws on failure) continues to work unchanged. -async function runIssueUpdate( - args: { - repoPath: string | null - projectOrigin: GitHubItemDialogProjectOrigin | undefined - number: number - updates: Parameters[0]['updates'] - } -): Promise { +async function runIssueUpdate(args: { + repoPath: string | null + projectOrigin: GitHubItemDialogProjectOrigin | undefined + number: number + updates: Parameters[0]['updates'] +}): Promise { if (args.projectOrigin) { const res = await window.api.gh.updateIssueBySlug({ owner: args.projectOrigin.owner, @@ -1746,7 +1737,9 @@ function GHEditSection({ // See docs/design/github-project-view-tasks.md §Dialog editing from Project rows. const patchProjectRowIfNeeded = useCallback( (patch: Parameters[2]) => { - if (!projectOrigin) {return} + if (!projectOrigin) { + return + } patchProjectRowContent(projectOrigin.cacheKey, projectOrigin.projectItemId, patch) }, [projectOrigin, patchProjectRowContent] diff --git a/src/renderer/src/components/Landing.tsx b/src/renderer/src/components/Landing.tsx index 8d1ca8072..3e97240c3 100644 --- a/src/renderer/src/components/Landing.tsx +++ b/src/renderer/src/components/Landing.tsx @@ -252,7 +252,7 @@ export default function Landing(): React.JSX.Element { className="inline-flex items-center gap-1.5 bg-secondary/70 border border-border/80 text-foreground font-medium text-sm px-4 py-2 rounded-md transition-colors disabled:opacity-40 disabled:cursor-not-allowed enabled:cursor-pointer enabled:hover:bg-accent" disabled={!canCreateWorktree} title={!canCreateWorktree ? 'Add a Git project first' : undefined} - onClick={() => openModal('new-workspace-composer')} + onClick={() => openModal('new-workspace-composer', { telemetrySource: 'unknown' })} > Create Worktree diff --git a/src/renderer/src/components/NewWorkspaceComposerModal.tsx b/src/renderer/src/components/NewWorkspaceComposerModal.tsx index 0e0687736..a8cda39fa 100644 --- a/src/renderer/src/components/NewWorkspaceComposerModal.tsx +++ b/src/renderer/src/components/NewWorkspaceComposerModal.tsx @@ -7,13 +7,18 @@ import { useComposerState } from '@/hooks/useComposerState' import { AGENT_CATALOG } from '@/lib/agent-catalog' import type { LinkedWorkItemSummary } from '@/lib/new-workspace' import { shouldSuppressEnterSubmit } from '@/lib/new-workspace-enter-guard' -import type { TuiAgent } from '../../../shared/types' +import type { TuiAgent, WorkspaceCreateTelemetrySource } from '../../../shared/types' type ComposerModalData = { prefilledName?: string initialRepoId?: string linkedWorkItem?: LinkedWorkItemSummary | null initialBaseBranch?: string + /** Telemetry surface that opened the composer. Set by each + * `openModal('new-workspace-composer', ...)` site so + * `workspace_created.source` carries the right value. Falls back to + * `unknown` when omitted. */ + telemetrySource?: WorkspaceCreateTelemetrySource } export default function NewWorkspaceComposerModal(): React.JSX.Element | null { @@ -101,7 +106,8 @@ function QuickTabBody({ initialRepoId: modalData.initialRepoId, ...(modalData.initialBaseBranch ? { initialBaseBranch: modalData.initialBaseBranch } : {}), persistDraft: false, - onCreated: onClose + onCreated: onClose, + ...(modalData.telemetrySource ? { telemetrySource: modalData.telemetrySource } : {}) }) // Why: the composer's built-in `onOpenAgentSettings` handler navigates to // the settings page and closes the modal. For the quick-create flow we want diff --git a/src/renderer/src/components/TaskPage.tsx b/src/renderer/src/components/TaskPage.tsx index 786538466..67d653070 100644 --- a/src/renderer/src/components/TaskPage.tsx +++ b/src/renderer/src/components/TaskPage.tsx @@ -1469,7 +1469,8 @@ export default function TaskPage(): React.JSX.Element { openModal('new-workspace-composer', { linkedWorkItem, prefilledName: getLinkedWorkItemSuggestedName(item), - initialRepoId: item.repoId + initialRepoId: item.repoId, + telemetrySource: 'sidebar' }) }, [openModal] @@ -1482,7 +1483,8 @@ export default function TaskPage(): React.JSX.Element { // setup before the worktree is created. Earlier the "Use" CTA created // and activated the worktree synchronously, which was disorienting — // the worktree appeared in the sidebar before the user had a chance - // to review it. The composer already owns the prefill flow. + // to review it. The composer already owns the prefill flow. Telemetry + // attribution flows via `openComposerForItem` (sets telemetrySource). openComposerForItem(item) }, [openComposerForItem] @@ -1759,7 +1761,8 @@ export default function TaskPage(): React.JSX.Element { } openModal('new-workspace-composer', { linkedWorkItem, - prefilledName: getLinkedWorkItemSuggestedName(issue) + prefilledName: getLinkedWorkItemSuggestedName(issue), + telemetrySource: 'sidebar' }) }, [openModal] @@ -1770,7 +1773,7 @@ export default function TaskPage(): React.JSX.Element { // Why: same rationale as handleUseWorkItem — open the New Workspace // dialog pre-filled rather than yolo-creating the worktree, so the // user can confirm name / agent / setup before the worktree lands in - // the sidebar. + // the sidebar. Telemetry attribution flows via openComposerForLinearItem. openComposerForLinearItem(issue) }, [openComposerForLinearItem] diff --git a/src/renderer/src/components/WorktreeJumpPalette.tsx b/src/renderer/src/components/WorktreeJumpPalette.tsx index dd0de65d7..6eb655f98 100644 --- a/src/renderer/src/components/WorktreeJumpPalette.tsx +++ b/src/renderer/src/components/WorktreeJumpPalette.tsx @@ -601,7 +601,9 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { closeModal() // Why: defer opening so Radix fully unmounts the palette's dialog before // the composer modal mounts, avoiding focus churn between the two. - queueMicrotask(() => openModal('new-workspace-composer', data)) + queueMicrotask(() => + openModal('new-workspace-composer', { ...data, telemetrySource: 'command_palette' }) + ) } // Case 1: user pasted a GH issue/PR URL. @@ -654,11 +656,16 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { // Fallback: we couldn't resolve the URL, just seed the name. data.prefilledName = `${slug.owner}-${slug.repo}-${number}` } - queueMicrotask(() => openModal('new-workspace-composer', data)) + queueMicrotask(() => + openModal('new-workspace-composer', { ...data, telemetrySource: 'command_palette' }) + ) }) .catch(() => { queueMicrotask(() => - openModal('new-workspace-composer', { initialRepoId: repoForLookup.id }) + openModal('new-workspace-composer', { + initialRepoId: repoForLookup.id, + telemetrySource: 'command_palette' + }) ) }) return @@ -702,13 +709,16 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { } else { data.prefilledName = trimmed } - queueMicrotask(() => openModal('new-workspace-composer', data)) + queueMicrotask(() => + openModal('new-workspace-composer', { ...data, telemetrySource: 'command_palette' }) + ) }) .catch(() => { queueMicrotask(() => openModal('new-workspace-composer', { initialRepoId: repoForLookup.id, - prefilledName: trimmed + prefilledName: trimmed, + telemetrySource: 'command_palette' }) ) }) diff --git a/src/renderer/src/components/github-project/ProjectGroupHeader.tsx b/src/renderer/src/components/github-project/ProjectGroupHeader.tsx index 00367b72a..5f77a8a09 100644 --- a/src/renderer/src/components/github-project/ProjectGroupHeader.tsx +++ b/src/renderer/src/components/github-project/ProjectGroupHeader.tsx @@ -32,9 +32,7 @@ export default function ProjectGroupHeader({ {group.rows.length} - {dateRange ? ( - {dateRange} - ) : null} + {dateRange ? {dateRange} : null} {isCurrent ? ( Current @@ -46,9 +44,10 @@ export default function ProjectGroupHeader({ function formatDateRange(startDate: string, duration: number): string { const start = new Date(`${startDate}T00:00:00Z`) - if (Number.isNaN(start.getTime())) {return ''} + if (Number.isNaN(start.getTime())) { + return '' + } const end = new Date(start.getTime() + (duration - 1) * 86_400_000) - const fmt = (d: Date): string => - `${d.getUTCMonth() + 1}/${d.getUTCDate()}` + const fmt = (d: Date): string => `${d.getUTCMonth() + 1}/${d.getUTCDate()}` return `${fmt(start)} – ${fmt(end)}` } diff --git a/src/renderer/src/components/github-project/ProjectItemSlugDialog.tsx b/src/renderer/src/components/github-project/ProjectItemSlugDialog.tsx index 4b3af2f38..6cde9f00b 100644 --- a/src/renderer/src/components/github-project/ProjectItemSlugDialog.tsx +++ b/src/renderer/src/components/github-project/ProjectItemSlugDialog.tsx @@ -38,9 +38,7 @@ export default function ProjectItemSlugDialog({ Project row preview. - {projectOrigin ? ( - - ) : null} + {projectOrigin ? : null} ) diff --git a/src/renderer/src/components/github-project/ProjectPicker.tsx b/src/renderer/src/components/github-project/ProjectPicker.tsx index 33b3171d4..581f8bd92 100644 --- a/src/renderer/src/components/github-project/ProjectPicker.tsx +++ b/src/renderer/src/components/github-project/ProjectPicker.tsx @@ -27,9 +27,12 @@ export type ResolvedProjectSelection = { } type Props = { - activeProject: - | { owner: string; ownerType: GitHubProjectOwnerType; number: number; title?: string } - | null + activeProject: { + owner: string + ownerType: GitHubProjectOwnerType + number: number + title?: string + } | null onSelect: (selection: ResolvedProjectSelection) => void } @@ -61,9 +64,9 @@ export default function ProjectPicker({ activeProject, onSelect }: Props): React // popover and reopening within the 5min window doesn't flicker the // banner back. Populated only when discovery succeeded but a subset of // orgs failed (the 504 path the user reported). - const [partialFailures, setPartialFailures] = useState< - { owner: string; message: string }[] - >(() => browseCache?.partialFailures ?? []) + const [partialFailures, setPartialFailures] = useState<{ owner: string; message: string }[]>( + () => browseCache?.partialFailures ?? [] + ) const [pasteInput, setPasteInput] = useState('') const [pasteError, setPasteError] = useState(null) const [pasteBusy, setPasteBusy] = useState(false) @@ -133,10 +136,7 @@ export default function ProjectPicker({ activeProject, onSelect }: Props): React number: selection.projectNumber, lastOpenedAt: new Date().toISOString() }, - ...prev.recent.filter( - (r) => - `${r.ownerType}:${r.owner}:${r.number}` !== key - ) + ...prev.recent.filter((r) => `${r.ownerType}:${r.owner}:${r.number}` !== key) ].slice(0, 10) const lastViewByProject = { ...prev.lastViewByProject } if (selection.viewId) { @@ -163,18 +163,16 @@ export default function ProjectPicker({ activeProject, onSelect }: Props): React ) const handleChooseProject = useCallback( - async ( - selection: { - owner: string - ownerType: GitHubProjectOwnerType - number: number - title?: string - // Why: when the paste resolver parsed a /views/{n} URL, the caller - // passes the view number through so we can skip the view-pick step - // and commit directly once listProjectViews returns the matching id. - viewNumber?: number - } - ) => { + async (selection: { + owner: string + ownerType: GitHubProjectOwnerType + number: number + title?: string + // Why: when the paste resolver parsed a /views/{n} URL, the caller + // passes the view number through so we can skip the view-pick step + // and commit directly once listProjectViews returns the matching id. + viewNumber?: number + }) => { const key = `${selection.ownerType}:${selection.owner}:${selection.number}` const lastView = projectSettings.lastViewByProject[key]?.viewId // Why: an explicit viewNumber from the URL takes precedence over the @@ -236,9 +234,7 @@ export default function ProjectPicker({ activeProject, onSelect }: Props): React // step with a perpetual spinner. Treat as an empty result and toast // a transport-level message so the user can retry or paste again. setViewList([]) - toast.error( - `Failed to load views: ${err instanceof Error ? err.message : String(err)}` - ) + toast.error(`Failed to load views: ${err instanceof Error ? err.message : String(err)}`) } finally { setViewLoading(false) } @@ -285,8 +281,12 @@ export default function ProjectPicker({ activeProject, onSelect }: Props): React ) return browseProjects.filter((p) => { const key = `${p.ownerType}:${p.owner}:${p.number}` - if (pinnedKeys.has(key) || recentKeys.has(key)) {return false} - if (!q) {return true} + if (pinnedKeys.has(key) || recentKeys.has(key)) { + return false + } + if (!q) { + return true + } return ( p.title.toLowerCase().includes(q) || p.owner.toLowerCase().includes(q) || @@ -343,8 +343,7 @@ export default function ProjectPicker({ activeProject, onSelect }: Props): React
{projectSettings.pinned.map((p) => { const key = `${p.ownerType}:${p.owner}:${p.number}` - const knownGood = - projectSettings.lastViewByProject[key]?.viewId != null + const knownGood = projectSettings.lastViewByProject[key]?.viewId != null const match = browseProjects.find( (bp) => `${bp.ownerType}:${bp.owner}:${bp.number}` === key ) @@ -453,7 +452,9 @@ export default function ProjectPicker({ activeProject, onSelect }: Props): React setPasteError(null) }} onKeyDown={(e) => { - if (e.key === 'Enter') {void handlePaste()} + if (e.key === 'Enter') { + void handlePaste() + } }} placeholder="Add by URL or owner/number" className="h-8 text-xs" @@ -586,9 +587,7 @@ function ViewPickStep({ onClick={() => void onPick(v)} className={cn( 'flex w-full flex-col items-start rounded px-2 py-1 text-left', - supported - ? 'hover:bg-muted/50' - : 'cursor-not-allowed opacity-50' + supported ? 'hover:bg-muted/50' : 'cursor-not-allowed opacity-50' )} > {v.name} @@ -621,7 +620,9 @@ function PartialFailuresBanner({ failures.length === 1 && failures[0].owner !== '*' ? `Couldn't load projects from ${failures[0].owner}.` : `Some organizations didn't load (${failures.length}).` - const detail = failures.map((f) => `${f.owner === '*' ? 'orgs' : f.owner}: ${f.message}`).join('\n') + const detail = failures + .map((f) => `${f.owner === '*' ? 'orgs' : f.owner}: ${f.message}`) + .join('\n') return (
{ setHidden((prev) => { const next = new Set(prev) - if (next.has(fieldId)) {next.delete(fieldId)} - else {next.add(fieldId)} + if (next.has(fieldId)) { + next.delete(fieldId) + } else { + next.add(fieldId) + } saveHiddenColumns(scopeKey, next) return next }) } const effectiveTable = useMemo(() => { - if (!sortOverride) {return table} + if (!sortOverride) { + return table + } const field = fields.find((f) => f.id === sortOverride.fieldId) - if (!field) {return table} + if (!field) { + return table + } return { ...table, selectedView: { @@ -101,8 +104,12 @@ export default function ProjectViewList({ const handleSortClick = (fieldId: string): void => { setSortOverride((prev) => { - if (!prev || prev.fieldId !== fieldId) {return { fieldId, direction: 'ASC' }} - if (prev.direction === 'ASC') {return { fieldId, direction: 'DESC' }} + if (!prev || prev.fieldId !== fieldId) { + return { fieldId, direction: 'ASC' } + } + if (prev.direction === 'ASC') { + return { fieldId, direction: 'DESC' } + } return null }) } @@ -148,8 +155,11 @@ export default function ProjectViewList({ onToggle={() => { setCollapsed((prev) => { const next = new Set(prev) - if (next.has(g.key)) {next.delete(g.key)} - else {next.add(g.key)} + if (next.has(g.key)) { + next.delete(g.key) + } else { + next.add(g.key) + } return next }) }} diff --git a/src/renderer/src/components/github-project/ProjectViewWrapper.tsx b/src/renderer/src/components/github-project/ProjectViewWrapper.tsx index 0a67d8ec4..e627b97a3 100644 --- a/src/renderer/src/components/github-project/ProjectViewWrapper.tsx +++ b/src/renderer/src/components/github-project/ProjectViewWrapper.tsx @@ -25,9 +25,7 @@ import { DialogHeader, DialogTitle } from '@/components/ui/dialog' -import GitHubItemDialog, { - type GitHubItemDialogProjectOrigin -} from '@/components/GitHubItemDialog' +import GitHubItemDialog, { type GitHubItemDialogProjectOrigin } from '@/components/GitHubItemDialog' import { launchWorkItemDirect } from '@/lib/launch-work-item-direct' import { useRepoSlugIndex } from '@/lib/repo-slug-index' import { cn } from '@/lib/utils' @@ -90,11 +88,7 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J const [appliedQueryByView, setAppliedQueryByView] = useState>({}) const doFetch = useCallback( - async ( - selection: ResolvedProjectSelection, - force = false, - queryOverride?: string - ) => { + async (selection: ResolvedProjectSelection, force = false, queryOverride?: string) => { setLoading(true) setError(null) try { @@ -127,10 +121,14 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J // Auto-fetch when activeProject exists and we don't have cached data. useEffect(() => { - if (!activeProject) {return} + if (!activeProject) { + return + } const key = `${activeProject.ownerType}:${activeProject.owner}:${activeProject.number}` const viewId = lastViewByProject[key]?.viewId - if (!viewId) {return} + if (!viewId) { + return + } const projectViewKey = `${key}:${viewId}` const queryOverride = appliedQueryByView[projectViewKey] const cacheKey = projectViewCacheKey( @@ -140,7 +138,9 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J viewId, queryOverride ) - if (projectViewCache[cacheKey]?.data) {return} + if (projectViewCache[cacheKey]?.data) { + return + } void doFetch( { owner: activeProject.owner, @@ -157,9 +157,13 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J // tab strip can render. The list is small and rarely changes — fetched once // per project per session is fine. useEffect(() => { - if (!activeProject) {return} + if (!activeProject) { + return + } const projectKey = `${activeProject.ownerType}:${activeProject.owner}:${activeProject.number}` - if (viewListByProject[projectKey]) {return} + if (viewListByProject[projectKey]) { + return + } let cancelled = false void window.api.gh .listProjectViews({ @@ -168,7 +172,9 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J projectNumber: activeProject.number }) .then((res) => { - if (cancelled) {return} + if (cancelled) { + return + } if (res.ok) { setViewListByProject((prev) => ({ ...prev, [projectKey]: res.views })) } else { @@ -176,7 +182,9 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J } }) .catch((err) => { - if (cancelled) {return} + if (cancelled) { + return + } // Why: an IPC rejection here would surface as an unhandled rejection // and dev-tools red — log and fall back to the empty-tabs UI. console.warn('[project-view] listProjectViews threw:', err) @@ -188,10 +196,14 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J const handleSwitchView = useCallback( async (viewId: string) => { - if (!activeProject) {return} + if (!activeProject) { + return + } const projectKey = `${activeProject.ownerType}:${activeProject.owner}:${activeProject.number}` const current = lastViewByProject[projectKey]?.viewId - if (current === viewId) {return} + if (current === viewId) { + return + } // Persist the new view selection so reloads & the picker stay in sync. // Why: read the freshest settings via getState() rather than the closure- // captured `settings` — between callback creation and invocation another @@ -224,10 +236,14 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J ) const currentProjectViewKey = useMemo(() => { - if (!activeProject) {return null} + if (!activeProject) { + return null + } const key = `${activeProject.ownerType}:${activeProject.owner}:${activeProject.number}` const viewId = lastViewByProject[key]?.viewId - if (!viewId) {return null} + if (!viewId) { + return null + } return `${key}:${viewId}` }, [activeProject, lastViewByProject]) @@ -236,10 +252,14 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J : undefined const currentCacheKey = useMemo(() => { - if (!activeProject) {return null} + if (!activeProject) { + return null + } const key = `${activeProject.ownerType}:${activeProject.owner}:${activeProject.number}` const viewId = lastViewByProject[key]?.viewId - if (!viewId) {return null} + if (!viewId) { + return null + } return projectViewCacheKey( activeProject.ownerType, activeProject.owner, @@ -255,8 +275,12 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J // Parent-dropped toast, once per table. useEffect(() => { - if (!table || !currentCacheKey || !table.parentFieldDropped) {return} - if (parentDroppedToasted.has(currentCacheKey)) {return} + if (!table || !currentCacheKey || !table.parentFieldDropped) { + return + } + if (parentDroppedToasted.has(currentCacheKey)) { + return + } toast.message('Sub-issue data is unavailable for your token.') setParentDroppedToasted((prev) => { const next = new Set(prev) @@ -294,8 +318,12 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J const buildWorkItem = useCallback( (row: GitHubProjectRow, repoId: string): GitHubWorkItem | null => { - if (row.itemType !== 'ISSUE' && row.itemType !== 'PULL_REQUEST') {return null} - if (row.content.number == null || !row.content.url) {return null} + if (row.itemType !== 'ISSUE' && row.itemType !== 'PULL_REQUEST') { + return null + } + if (row.content.number == null || !row.content.url) { + return null + } return { id: `${row.itemType === 'PULL_REQUEST' ? 'pr' : 'issue'}:${row.content.number}`, type: row.itemType === 'PULL_REQUEST' ? 'pr' : 'issue', @@ -325,10 +353,16 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J cacheKey: string, table: GitHubProjectTable ): GitHubItemDialogProjectOrigin | null => { - if (row.itemType !== 'ISSUE' && row.itemType !== 'PULL_REQUEST') {return null} - if (row.content.number == null || !row.content.repository) {return null} + if (row.itemType !== 'ISSUE' && row.itemType !== 'PULL_REQUEST') { + return null + } + if (row.content.number == null || !row.content.repository) { + return null + } const [owner, repo] = row.content.repository.split('/') - if (!owner || !repo) {return null} + if (!owner || !repo) { + return null + } return { owner, repo, @@ -344,11 +378,15 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J const handleOpenDialog = useCallback( (row: GitHubProjectRow) => { - if (!currentCacheKey || !table) {return} + if (!currentCacheKey || !table) { + return + } const origin = buildOrigin(row, currentCacheKey, table) if (!origin) { // Redacted / draft / missing slug — fall back to opening GitHub. - if (row.content.url) {void window.api.shell.openUrl(row.content.url)} + if (row.content.url) { + void window.api.shell.openUrl(row.content.url) + } return } const matched = lookupSlug(`${origin.owner}/${origin.repo}`) @@ -367,9 +405,13 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J const handleStartWork = useCallback( (row: GitHubProjectRow) => { - if (!currentCacheKey || !table) {return} + if (!currentCacheKey || !table) { + return + } const origin = buildOrigin(row, currentCacheKey, table) - if (!origin) {return} + if (!origin) { + return + } const matched = lookupSlug(`${origin.owner}/${origin.repo}`) if (!matched) { setRepoNotInOrca({ @@ -380,16 +422,22 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J return } const workItem = buildWorkItem(row, matched.id) - if (!workItem) {return} + if (!workItem) { + return + } void launchWorkItemDirect({ item: workItem, repoId: matched.id, + launchSource: 'task_page', + telemetrySource: 'sidebar', openModalFallback: () => { // Why: Project mode does not own the new-workspace composer modal. // When `launchWorkItemDirect` wants user input (setupRunPolicy:'ask' // or agent detection fails), fall back to opening the URL so the // user keeps a path forward rather than a silent no-op. - if (row.content.url) {void window.api.shell.openUrl(row.content.url)} + if (row.content.url) { + void window.api.shell.openUrl(row.content.url) + } } }) }, @@ -398,33 +446,45 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J const handleEditAssignees = useCallback( async (row: GitHubProjectRow, add: string[], remove: string[]) => { - if (!currentCacheKey) {return} + if (!currentCacheKey) { + return + } const res = await patchProjectIssueOrPr(currentCacheKey, row.id, { ...(add.length ? { addAssignees: add } : {}), ...(remove.length ? { removeAssignees: remove } : {}) }) - if (!res.ok) {toast.error(res.error.message)} + if (!res.ok) { + toast.error(res.error.message) + } }, [currentCacheKey, patchProjectIssueOrPr] ) const handleEditLabels = useCallback( async (row: GitHubProjectRow, add: string[], remove: string[]) => { - if (!currentCacheKey) {return} + if (!currentCacheKey) { + return + } const res = await patchProjectIssueOrPr(currentCacheKey, row.id, { ...(add.length ? { addLabels: add } : {}), ...(remove.length ? { removeLabels: remove } : {}) }) - if (!res.ok) {toast.error(res.error.message)} + if (!res.ok) { + toast.error(res.error.message) + } }, [currentCacheKey, patchProjectIssueOrPr] ) const handleEditIssueType = useCallback( async (row: GitHubProjectRow, issueType: GitHubIssueType | null) => { - if (!currentCacheKey) {return} + if (!currentCacheKey) { + return + } const res = await patchProjectRowIssueType(currentCacheKey, row.id, issueType) - if (!res.ok) {toast.error(res.error.message)} + if (!res.ok) { + toast.error(res.error.message) + } }, [currentCacheKey, patchProjectRowIssueType] ) @@ -435,7 +495,9 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J fieldId: string, value: GitHubProjectFieldMutationValue | null ) => { - if (!currentCacheKey) {return} + if (!currentCacheKey) { + return + } const result = value === null ? await clearProjectFieldValue(currentCacheKey, row.id, fieldId) @@ -480,10 +542,14 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J viewFilter={table?.selectedView.filter ?? ''} appliedOverride={appliedQueryByView[currentProjectViewKey]} onApply={(nextOverride) => { - if (!activeProject) {return} + if (!activeProject) { + return + } const key = `${activeProject.ownerType}:${activeProject.owner}:${activeProject.number}` const viewId = lastViewByProject[key]?.viewId - if (!viewId) {return} + if (!viewId) { + return + } setAppliedQueryByView((prev) => { const next = { ...prev } if (nextOverride === undefined) { @@ -530,10 +596,14 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J size="icon" className="h-7 w-7" onClick={() => { - if (!activeProject || !currentCacheKey) {return} + if (!activeProject || !currentCacheKey) { + return + } const key = `${activeProject.ownerType}:${activeProject.owner}:${activeProject.number}` const viewId = lastViewByProject[key]?.viewId - if (!viewId) {return} + if (!viewId) { + return + } void doFetch( { owner: activeProject.owner, @@ -557,7 +627,9 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J ? (() => { const projectKey = `${activeProject.ownerType}:${activeProject.owner}:${activeProject.number}` const views = viewListByProject[projectKey] ?? [] - if (views.length === 0) {return null} + if (views.length === 0) { + return null + } const activeViewId = lastViewByProject[projectKey]?.viewId ?? null return ( { - if (selectedViewUrl) {void window.api.shell.openUrl(selectedViewUrl)} + if (selectedViewUrl) { + void window.api.shell.openUrl(selectedViewUrl) + } }} /> ) : table ? ( @@ -595,7 +669,9 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J onEditLabels={(row, add, remove) => void handleEditLabels(row, add, remove)} onEditIssueType={(row, issueType) => void handleEditIssueType(row, issueType)} onOpenInBrowser={(row) => { - if (row.content.url) {void window.api.shell.openUrl(row.content.url)} + if (row.content.url) { + void window.api.shell.openUrl(row.content.url) + } }} onStartWork={handleStartWork} /> @@ -612,12 +688,18 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J onUse={(item) => { const current = dialogRepoItem setDialogRepoItem(null) - if (!current) {return} + if (!current) { + return + } void launchWorkItemDirect({ item, repoId: current.workItem.repoId, + launchSource: 'task_page', + telemetrySource: 'sidebar', openModalFallback: () => { - if (item.url) {void window.api.shell.openUrl(item.url)} + if (item.url) { + void window.api.shell.openUrl(item.url) + } } }) }} @@ -656,7 +738,9 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J ) diff --git a/src/renderer/src/components/github-project/slug-dialog/SlugDialogBody.tsx b/src/renderer/src/components/github-project/slug-dialog/SlugDialogBody.tsx index 9eb9e5eb9..5878ab970 100644 --- a/src/renderer/src/components/github-project/slug-dialog/SlugDialogBody.tsx +++ b/src/renderer/src/components/github-project/slug-dialog/SlugDialogBody.tsx @@ -27,10 +27,16 @@ export function SlugDialogBody({ // patches applied by the table (e.g. inline assignee edits). const row = useMemo(() => { const table = projectViewCache[cacheKey]?.data - if (!table) {return null} - return table.rows.find( - (r) => r.content.number === number && r.content.repository?.toLowerCase() === `${owner}/${repo}`.toLowerCase() - ) ?? null + if (!table) { + return null + } + return ( + table.rows.find( + (r) => + r.content.number === number && + r.content.repository?.toLowerCase() === `${owner}/${repo}`.toLowerCase() + ) ?? null + ) }, [projectViewCache, cacheKey, owner, repo, number]) const [details, setDetails] = useState(null) @@ -47,7 +53,9 @@ export function SlugDialogBody({ window.api.gh .projectWorkItemDetailsBySlug({ owner, repo, number, type }) .then((res) => { - if (rid !== requestIdRef.current) {return} + if (rid !== requestIdRef.current) { + return + } if (res.ok) { setDetails(res.details) } else { @@ -55,11 +63,15 @@ export function SlugDialogBody({ } }) .catch((err) => { - if (rid !== requestIdRef.current) {return} + if (rid !== requestIdRef.current) { + return + } setError(err instanceof Error ? err.message : 'Failed to load details') }) .finally(() => { - if (rid !== requestIdRef.current) {return} + if (rid !== requestIdRef.current) { + return + } setLoading(false) }) }, [owner, repo, number, type]) @@ -74,13 +86,19 @@ export function SlugDialogBody({ const commitTitle = useCallback(async () => { const next = titleDraft.trim() setEditingTitle(false) - if (!next || next === title) {return} + if (!next || next === title) { + return + } // Why: without a row id we can't address the project item — the helper // would just return "Row not found" and toast-spam the user. The title // button is also disabled in this case (see render below). - if (!row) {return} + if (!row) { + return + } const res = await patchProjectIssueOrPr(cacheKey, row.id, { title: next }) - if (!res.ok) {toast.error(res.error.message)} + if (!res.ok) { + toast.error(res.error.message) + } }, [titleDraft, title, patchProjectIssueOrPr, cacheKey, row]) const [editingBody, setEditingBody] = useState(false) @@ -88,10 +106,14 @@ export function SlugDialogBody({ const body = details?.body ?? '' const commitBody = useCallback(async () => { setEditingBody(false) - if (bodyDraft === body) {return} + if (bodyDraft === body) { + return + } // Why: same reason as commitTitle — bail rather than ask the helper to // patch a missing row. The body button is also disabled when row is null. - if (!row) {return} + if (!row) { + return + } const res = await patchProjectIssueOrPr(cacheKey, row.id, { body: bodyDraft }) if (!res.ok) { toast.error(res.error.message) @@ -177,12 +199,16 @@ export function SlugDialogBody({ onChange={async (add, remove) => { // Why: bail rather than call the helper with an empty id — // see commitTitle above. Trigger is also disabled when !row. - if (!row) {return} + if (!row) { + return + } const res = await patchProjectIssueOrPr(cacheKey, row.id, { ...(add.length ? { addLabels: add } : {}), ...(remove.length ? { removeLabels: remove } : {}) }) - if (!res.ok) {toast.error(res.error.message)} + if (!res.ok) { + toast.error(res.error.message) + } }} /> { - if (!row) {return} + if (!row) { + return + } const res = await patchProjectIssueOrPr(cacheKey, row.id, { ...(add.length ? { addAssignees: add } : {}), ...(remove.length ? { removeAssignees: remove } : {}) }) - if (!res.ok) {toast.error(res.error.message)} + if (!res.ok) { + toast.error(res.error.message) + } }} />
@@ -269,9 +299,7 @@ export function SlugDialogBody({ owner={owner} repo={repo} number={number} - onAdded={(c) => - setDetails((d) => (d ? { ...d, comments: [...d.comments, c] } : d)) - } + onAdded={(c) => setDetails((d) => (d ? { ...d, comments: [...d.comments, c] } : d))} />
diff --git a/src/renderer/src/components/github/GitHubRateLimitPill.tsx b/src/renderer/src/components/github/GitHubRateLimitPill.tsx index 7ee26f902..51a2e5293 100644 --- a/src/renderer/src/components/github/GitHubRateLimitPill.tsx +++ b/src/renderer/src/components/github/GitHubRateLimitPill.tsx @@ -190,10 +190,7 @@ export default function GitHubRateLimitPill(): React.JSX.Element | null {
{b.description} {v.remaining} of {v.limit} left · resets in {formatReset(v.resetAt)} diff --git a/src/renderer/src/components/sidebar/AddRepoDialog.tsx b/src/renderer/src/components/sidebar/AddRepoDialog.tsx index 3f5d5a9be..b7ff3fbe9 100644 --- a/src/renderer/src/components/sidebar/AddRepoDialog.tsx +++ b/src/renderer/src/components/sidebar/AddRepoDialog.tsx @@ -205,7 +205,7 @@ const AddRepoDialog = React.memo(function AddRepoDialog() { // the first focus frame from the composer's prompt textarea. closeModal() setTimeout(() => { - openModal('new-workspace-composer', { initialRepoId: repoId }) + openModal('new-workspace-composer', { initialRepoId: repoId, telemetrySource: 'sidebar' }) }, 150) }, [closeModal, openModal, repoId]) diff --git a/src/renderer/src/components/sidebar/SidebarHeader.tsx b/src/renderer/src/components/sidebar/SidebarHeader.tsx index a943e8cd9..1458aaf36 100644 --- a/src/renderer/src/components/sidebar/SidebarHeader.tsx +++ b/src/renderer/src/components/sidebar/SidebarHeader.tsx @@ -162,7 +162,7 @@ const SidebarHeader = React.memo(function SidebarHeader() { if (!canCreateWorktree) { return } - openModal('new-workspace-composer') + openModal('new-workspace-composer', { telemetrySource: 'sidebar' }) }} aria-label="New workspace" disabled={!canCreateWorktree} diff --git a/src/renderer/src/components/sidebar/WorktreeList.tsx b/src/renderer/src/components/sidebar/WorktreeList.tsx index 0ef0a2816..0264ec458 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.tsx @@ -790,7 +790,7 @@ const WorktreeList = React.memo(function WorktreeList() { const handleCreateForRepo = useCallback( (repoId: string) => { - openModal('new-workspace-composer', { initialRepoId: repoId }) + openModal('new-workspace-composer', { initialRepoId: repoId, telemetrySource: 'sidebar' }) }, [openModal] ) diff --git a/src/renderer/src/components/terminal-pane/pty-connection-types.ts b/src/renderer/src/components/terminal-pane/pty-connection-types.ts index 2994f43b9..bb41e9699 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-types.ts @@ -1,11 +1,18 @@ import type { PtyTransport } from './pty-transport' import type { ReplayingPanesRef } from './replay-guard' +import type { EventProps } from '../../../../shared/telemetry-events' export type PtyConnectionDeps = { tabId: string worktreeId: string cwd?: string - startup?: { command: string; env?: Record } | null + startup?: { + command: string + env?: Record + /** Telemetry payload for `agent_started`. Forwarded to `pty:spawn` + * so main fires the event only after the spawn succeeds. */ + telemetry?: EventProps<'agent_started'> + } | null restoredLeafId?: string | null restoredPtyIdByLeafId?: Record paneTransportsRef: React.RefObject> diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index cfd4400ac..afc0d5d04 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -334,6 +334,7 @@ export function connectPanePty( connectionId, worktreeId: deps.worktreeId, ...(shellOverride ? { shellOverride } : {}), + ...(paneStartup?.telemetry ? { telemetry: paneStartup.telemetry } : {}), onPtyExit: onExit, onTitleChange, onPtySpawn, diff --git a/src/renderer/src/components/terminal-pane/pty-dispatcher.ts b/src/renderer/src/components/terminal-pane/pty-dispatcher.ts index 8abbe6223..f10be8baf 100644 --- a/src/renderer/src/components/terminal-pane/pty-dispatcher.ts +++ b/src/renderer/src/components/terminal-pane/pty-dispatcher.ts @@ -6,6 +6,7 @@ * and the eager-buffer reconnection logic share. */ import type { ParsedAgentStatusPayload } from '../../../../shared/agent-status-types' +import type { EventProps } from '../../../../shared/telemetry-events' // ── Singleton PTY event dispatcher ─────────────────────────────────── // One global IPC listener per channel, routes events to transports by @@ -257,6 +258,11 @@ export type IpcPtyTransportOptions = { worktreeId?: string /** Why: mirrors PtySpawnOptions.shellOverride — see types.ts for rationale. */ shellOverride?: string + /** Telemetry metadata for the `agent_started` event. Forwarded verbatim + * to `pty:spawn` so main can fire the event after confirmed launch. The + * IPC handler re-validates the schema; this type is the renderer-side + * contract. */ + telemetry?: EventProps<'agent_started'> onPtyExit?: (ptyId: string) => void onTitleChange?: (title: string, rawTitle: string) => void onPtySpawn?: (ptyId: string) => void diff --git a/src/renderer/src/components/terminal-pane/pty-transport.ts b/src/renderer/src/components/terminal-pane/pty-transport.ts index f8cb7f7f2..e81f4efc2 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport.ts @@ -150,6 +150,7 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra connectionId, worktreeId, shellOverride, + telemetry, onPtyExit, onTitleChange, onPtySpawn, @@ -353,7 +354,8 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra ...(connectionId ? { connectionId } : {}), ...(options.sessionId ? { sessionId: options.sessionId } : {}), worktreeId, - ...(shellOverride ? { shellOverride } : {}) + ...(shellOverride ? { shellOverride } : {}), + ...(telemetry ? { telemetry } : {}) }) const spawnResult = result as PtyConnectResult & { isReattach?: boolean } diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts index f045bc126..c60facc80 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts @@ -15,6 +15,7 @@ import type { SetupSplitDirection, TerminalLayoutSnapshot } from '../../../../shared/types' +import type { EventProps } from '../../../../shared/telemetry-events' import { resolveTerminalFontWeights } from '../../../../shared/terminal-fonts' import { buildFontFamily, @@ -52,7 +53,13 @@ type UseTerminalPaneLifecycleDeps = { tabId: string worktreeId: string cwd?: string - startup?: { command: string; env?: Record } | null + startup?: { + command: string + env?: Record + /** Telemetry payload for `agent_started`. Forwarded to `pty:spawn` + * so main fires the event only after the spawn succeeds. */ + telemetry?: EventProps<'agent_started'> + } | null /** When present, the initial pane boots clean and a split pane is created * (vertical or horizontal per the user setting) to run the setup command — * keeping the main terminal interactive. */ diff --git a/src/renderer/src/hooks/useComposerState.ts b/src/renderer/src/hooks/useComposerState.ts index 2da6a45d5..1045da4fd 100644 --- a/src/renderer/src/hooks/useComposerState.ts +++ b/src/renderer/src/hooks/useComposerState.ts @@ -12,9 +12,10 @@ import { parseGitHubIssueOrPRLink, normalizeGitHubLinkQuery } from '@/lib/github-links' -import { activateAndRevealWorktree } from '@/lib/worktree-activation' +import { activateAndRevealWorktree, type AgentStartedTelemetry } from '@/lib/worktree-activation' import { buildAgentDraftLaunchPlan, buildAgentStartupPlan } from '@/lib/tui-agent-startup' import { TUI_AGENT_CONFIG } from '../../../shared/tui-agent-config' +import { tuiAgentToAgentKind } from '@/lib/telemetry' import { isGitRepoKind } from '../../../shared/repo-kind' import type { GitHubWorkItem, @@ -23,7 +24,8 @@ import type { SetupDecision, SetupRunPolicy, SparsePreset, - TuiAgent + TuiAgent, + WorkspaceCreateTelemetrySource } from '../../../shared/types' import { ADD_ATTACHMENT_SHORTCUT, @@ -65,6 +67,12 @@ export type UseComposerStateOptions = { * which drives repo selection from the page header, not the card. */ repoIdOverride?: string onRepoIdOverrideChange?: (value: string) => void + /** Telemetry surface that opened this composer. Threaded into + * `createWorktree` so `workspace_created.source` reflects the actual + * entry point (Cmd+J palette → `command_palette`, sidebar buttons → + * `sidebar`, keyboard shortcut → `shortcut`). Omitted callers default + * to `unknown` at the IPC boundary. */ + telemetrySource?: WorkspaceCreateTelemetrySource } export type ComposerCardProps = { @@ -175,7 +183,8 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS persistDraft, onCreated, repoIdOverride, - onRepoIdOverrideChange + onRepoIdOverrideChange, + telemetrySource } = options // Why: each `useAppStore(s => s.someAction)` registers its own equality @@ -1247,7 +1256,8 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS directories: normalizedSparseDirectories, ...(effectivePresetId ? { presetId: effectivePresetId } : {}) } - : undefined + : undefined, + telemetrySource ) const worktree = result.worktree @@ -1273,10 +1283,27 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS platform: CLIENT_PLATFORM }) + // Why: thread agent_started telemetry through the queued startup so + // main fires the event after the spawn succeeds. The composer + // "create" path is the new-workspace surface; request_kind is + // `'new'` because this is always a fresh session (issue/PR-driven + // follow-ups go through launch-work-item-direct.ts). + const composerTelemetry: AgentStartedTelemetry = { + agent_kind: tuiAgentToAgentKind(tuiAgent), + launch_source: 'new_workspace_composer', + request_kind: 'new' + } activateAndRevealWorktree(worktree.id, { setup: result.setup, issueCommand, - ...(startupPlan ? { startup: { command: startupPlan.launchCommand } } : {}) + ...(startupPlan + ? { + startup: { + command: startupPlan.launchCommand, + telemetry: composerTelemetry + } + } + : {}) }) if (startupPlan) { void ensureAgentStartupInTerminal({ @@ -1326,6 +1353,7 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS sparseEnabled, sparseError, effectivePresetId, + telemetrySource, tuiAgent, shouldRunIssueAutomation, shouldWaitForIssueAutomationCheck, @@ -1373,7 +1401,8 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS directories: normalizedSparseDirectories, ...(effectivePresetId ? { presetId: effectivePresetId } : {}) } - : undefined + : undefined, + telemetrySource ) const worktree = result.worktree @@ -1449,9 +1478,28 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS } } + // Why: only attach telemetry when an agent was selected — the + // quick path also handles "blank shell" (agent === null) where no + // agent_started event should fire. When telemetry is present main + // emits the event after pty:spawn succeeds. + const quickTelemetry: AgentStartedTelemetry | null = + agent === null + ? null + : { + agent_kind: tuiAgentToAgentKind(agent), + launch_source: 'new_workspace_composer', + request_kind: 'new' + } activateAndRevealWorktree(worktree.id, { setup: result.setup, - ...(startupPlan ? { startup: { command: startupPlan.launchCommand } } : {}) + ...(startupPlan + ? { + startup: { + command: startupPlan.launchCommand, + ...(quickTelemetry ? { telemetry: quickTelemetry } : {}) + } + } + : {}) }) if (startupPlan) { void ensureAgentStartupInTerminal({ @@ -1483,6 +1531,7 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS createWorktree, fallbackCreatureName, effectiveLinkedPR, + linkedPR, linkedWorkItem, name, normalizedSparseDirectories, @@ -1503,6 +1552,7 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS sparseEnabled, sparseError, effectivePresetId, + telemetrySource, shouldWaitForSetupCheck ] ) diff --git a/src/renderer/src/hooks/useIpcEvents.ts b/src/renderer/src/hooks/useIpcEvents.ts index ad5ceb7a8..141cc6186 100644 --- a/src/renderer/src/hooks/useIpcEvents.ts +++ b/src/renderer/src/hooks/useIpcEvents.ts @@ -149,7 +149,7 @@ export function useIpcEvents(): void { if (store.activeModal === 'new-workspace-composer') { return } - store.openModal('new-workspace-composer') + store.openModal('new-workspace-composer', { telemetrySource: 'shortcut' }) }) ) diff --git a/src/renderer/src/lib/launch-agent-in-new-tab.ts b/src/renderer/src/lib/launch-agent-in-new-tab.ts index dd49861c8..6e959f9c8 100644 --- a/src/renderer/src/lib/launch-agent-in-new-tab.ts +++ b/src/renderer/src/lib/launch-agent-in-new-tab.ts @@ -2,6 +2,7 @@ import { useAppStore } from '@/store' import { buildAgentStartupPlan, type AgentStartupPlan } from '@/lib/tui-agent-startup' import { CLIENT_PLATFORM } from '@/lib/new-workspace' import { reconcileTabOrder } from '@/components/tab-bar/reconcile-order' +import { tuiAgentToAgentKind } from '@/lib/telemetry' import type { TuiAgent } from '../../../shared/types' export type LaunchAgentInNewTabArgs = { @@ -55,8 +56,20 @@ export function launchAgentInNewTab(args: LaunchAgentInNewTabArgs): LaunchAgentI // lands after mount the agent binary never starts; the user sees a bare shell. // Since both calls happen synchronously in the same React batch, the queue // is in place by the time the pane commits. + // + // The telemetry payload is threaded through the queue → pty-connection → + // pty-transport → pty:spawn IPC → main, where main fires `agent_started` + // only after the spawn succeeds. `request_kind: 'new'` because + // quick-launch always opens a fresh empty-prompt session. const tab = store.createTab(worktreeId, groupId) - store.queueTabStartupCommand(tab.id, { command: startupPlan.launchCommand }) + store.queueTabStartupCommand(tab.id, { + command: startupPlan.launchCommand, + telemetry: { + agent_kind: tuiAgentToAgentKind(agent), + launch_source: 'tab_bar_quick_launch', + request_kind: 'new' + } + }) // Why: match the `+` button's `createNewTerminalTab` sequence — without // `setActiveTabType('terminal')`, a worktree currently showing an editor diff --git a/src/renderer/src/lib/launch-work-item-direct.ts b/src/renderer/src/lib/launch-work-item-direct.ts index 182fc38d7..8e9078757 100644 --- a/src/renderer/src/lib/launch-work-item-direct.ts +++ b/src/renderer/src/lib/launch-work-item-direct.ts @@ -4,16 +4,23 @@ import { AGENT_CATALOG } from '@/lib/agent-catalog' import { pasteDraftWhenAgentReady } from '@/lib/agent-paste-draft' import { buildAgentDraftLaunchPlan, buildAgentStartupPlan } from '@/lib/tui-agent-startup' import { TUI_AGENT_CONFIG } from '../../../shared/tui-agent-config' -import { activateAndRevealWorktree } from '@/lib/worktree-activation' +import { activateAndRevealWorktree, type AgentStartedTelemetry } from '@/lib/worktree-activation' import { CLIENT_PLATFORM, getLinkedWorkItemSuggestedName, getSetupConfig, getWorkspaceSeedName } from '@/lib/new-workspace' -import { getSuggestedCreatureName } from '@/components/sidebar/worktree-name-suggestions' import { ensureHooksConfirmed } from '@/lib/ensure-hooks-confirmed' -import type { OrcaHooks, RepoHookSettings, SetupDecision, TuiAgent } from '../../../shared/types' +import { track, tuiAgentToAgentKind } from '@/lib/telemetry' +import type { + OrcaHooks, + RepoHookSettings, + SetupDecision, + TuiAgent, + WorkspaceCreateTelemetrySource +} from '../../../shared/types' +import type { LaunchSource } from '../../../shared/telemetry-events' export type LaunchableWorkItem = { title: string @@ -47,6 +54,15 @@ export type LaunchWorkItemDirectArgs = { * smart workspace-name PR selection to branch from the PR's head so the first * commit lands on the correct base without the user touching the UI. */ baseBranch?: string + /** Telemetry surface that initiated this agent launch. Threaded into + * the queued startup payload so `agent_started.launch_source` reflects + * the actual entry point. */ + launchSource: LaunchSource + /** Telemetry surface that initiated this launch. Threaded into + * `createWorktree` so `workspace_created.source` reflects the actual + * entry point (Tasks page row → `sidebar`, Create-from modal → + * `command_palette`). Omitted callers default to `unknown`. */ + telemetrySource?: WorkspaceCreateTelemetrySource } function pickAgent( @@ -96,20 +112,48 @@ async function resolveSetupDecision( } } +// Why: telemetry rides the queued startup so main fires `agent_started` +// only after pty:spawn confirms the launch. No agent / no plan → no event. +function buildStartupOpts( + agent: TuiAgent | null, + plan: ReturnType, + launchSource: LaunchSource +): { startup?: { command: string; telemetry?: AgentStartedTelemetry } } { + if (!plan) { + return {} + } + const telemetry: AgentStartedTelemetry | null = + agent === null + ? null + : { agent_kind: tuiAgentToAgentKind(agent), launch_source: launchSource, request_kind: 'new' } + return { + startup: { command: plan.launchCommand, ...(telemetry ? { telemetry } : {}) } + } +} + async function pasteWorkItemDraftWhenAgentReady(args: { primaryTabId: string startupPlan: NonNullable> content: string + /** Telemetry-only: which agent the renderer thinks it launched, so an + * `agent_error` on timeout can carry the right `agent_kind`. */ + agentKind?: ReturnType }): Promise { - const { primaryTabId, startupPlan, content } = args + const { primaryTabId, startupPlan, content, agentKind } = args await pasteDraftWhenAgentReady({ tabId: primaryTabId, content, agent: startupPlan.agent, - onTimeout: () => + onTimeout: () => { toast.message( 'Agent took too long to start. The workspace is ready — paste the issue URL when the agent is idle.' ) + // Why: process-startup timeout has no v1 enum slot; the `unknown` slice + // on the dashboard is the trigger to add one. + if (agentKind) { + track('agent_error', { error_class: 'unknown', agent_kind: agentKind }) + } + } }) } @@ -127,7 +171,7 @@ async function pasteWorkItemDraftWhenAgentReady(args: { * has a usable workspace and can paste the URL themselves. */ export async function launchWorkItemDirect(args: LaunchWorkItemDirectArgs): Promise { - const { item, repoId, openModalFallback, baseBranch } = args + const { item, repoId, openModalFallback, baseBranch, telemetrySource, launchSource } = args const store = useAppStore.getState() const repo = store.repos.find((r) => r.id === repoId) if (!repo) { @@ -160,14 +204,22 @@ export async function launchWorkItemDirect(args: LaunchWorkItemDirectArgs): Prom let worktreeId: string let primaryTabId: string | null let startupPlan: ReturnType = null + let effectiveAgent: TuiAgent | null = null let draftLaunchedNatively = false try { - const result = await store.createWorktree(repoId, workspaceName, baseBranch, finalSetupDecision) + const result = await store.createWorktree( + repoId, + workspaceName, + baseBranch, + finalSetupDecision, + undefined, + telemetrySource + ) worktreeId = result.worktree.id const worktreePath = result.worktree.path const detectedIds = new Set(await detectedAgentsPromise) - const effectiveAgent = pickAgent(settings?.defaultTuiAgent, detectedIds) + effectiveAgent = pickAgent(settings?.defaultTuiAgent, detectedIds) const draftContent = item.pasteContent ?? item.url // Why: agents that gate first-launch behind a "Do you trust this folder?" @@ -227,7 +279,7 @@ export async function launchWorkItemDirect(args: LaunchWorkItemDirectArgs): Prom const activation = activateAndRevealWorktree(worktreeId, { setup: result.setup, - ...(startupPlan ? { startup: { command: startupPlan.launchCommand } } : {}) + ...buildStartupOpts(effectiveAgent, startupPlan, launchSource) }) if (!activation) { // Worktree vanished between create and activate — extremely unlikely but @@ -281,93 +333,10 @@ export async function launchWorkItemDirect(args: LaunchWorkItemDirectArgs): Prom // latency on agent readiness. Run the paste in the background so the // "Use" CTA's spinner ends when the worktree is ready, not when the TUI // input buffer is ready. - void pasteWorkItemDraftWhenAgentReady({ primaryTabId, startupPlan, content }) -} - -export type LaunchFromBranchArgs = { - repoId: string - baseBranch: string - /** Called when the flow cannot proceed without user input (setup policy is - * `ask`, or the selected repo cannot resolve). */ - openModalFallback: () => void -} - -/** - * Create a workspace from a specific branch with no linked work item. Skips - * the bracketed-paste draft step — there's no URL to hand the agent, so we - * just land the user in a fresh workspace rooted at the requested branch. - */ -export async function launchFromBranch(args: LaunchFromBranchArgs): Promise { - const { repoId, baseBranch, openModalFallback } = args - const store = useAppStore.getState() - const repo = store.repos.find((r) => r.id === repoId) - if (!repo) { - openModalFallback() - return - } - - const settings = store.settings - // Why: keep agent detection off the critical path while we resolve setup - // policy. Worktree creation only needs the startup command at activation. - const detectedAgentsPromise = store.ensureDetectedAgents() - - const setupResolution = await resolveSetupDecision(repoId, repo) - if (setupResolution.kind === 'needs-modal') { - openModalFallback() - return - } - - const trustDecision = await ensureHooksConfirmed(useAppStore.getState(), repoId, 'setup') - const finalSetupDecision: SetupDecision = - trustDecision === 'skip' ? 'skip' : setupResolution.decision - - // Why: branch-based launches don't carry a title hint, so fall back to the - // repo's creature-name generator — same distinct, readable default the - // quick-composer uses when the name field is blank. - const fallbackName = getSuggestedCreatureName( - repoId, - store.worktreesByRepo, - settings?.nestWorkspaces ?? true - ) - const workspaceName = getWorkspaceSeedName({ - explicitName: '', - prompt: '', - linkedIssueNumber: null, - linkedPR: null, - fallbackName + void pasteWorkItemDraftWhenAgentReady({ + primaryTabId, + startupPlan, + content, + ...(effectiveAgent ? { agentKind: tuiAgentToAgentKind(effectiveAgent) } : {}) }) - - try { - const result = await store.createWorktree(repoId, workspaceName, baseBranch, finalSetupDecision) - const detectedIds = new Set(await detectedAgentsPromise) - const effectiveAgent = pickAgent(settings?.defaultTuiAgent, detectedIds) - const startupPlan = - effectiveAgent === null - ? null - : buildAgentStartupPlan({ - agent: effectiveAgent, - prompt: '', - cmdOverrides: settings?.agentCmdOverrides ?? {}, - platform: CLIENT_PLATFORM, - allowEmptyPromptLaunch: true - }) - const activation = activateAndRevealWorktree(result.worktree.id, { - setup: result.setup, - ...(startupPlan ? { startup: { command: startupPlan.launchCommand } } : {}) - }) - if (!activation) { - toast.error('Workspace created but could not be activated.') - return - } - } catch (error) { - const message = error instanceof Error ? error.message : 'Failed to create workspace.' - toast.error(message) - return - } - - store.setSidebarOpen(true) - if (settings?.rightSidebarOpenByDefault) { - store.setRightSidebarTab('explorer') - store.setRightSidebarOpen(true) - } } diff --git a/src/renderer/src/lib/repo-slug-index.ts b/src/renderer/src/lib/repo-slug-index.ts index 658e83b2d..6c14526b0 100644 --- a/src/renderer/src/lib/repo-slug-index.ts +++ b/src/renderer/src/lib/repo-slug-index.ts @@ -69,14 +69,18 @@ async function buildIndex(repos: Repo[]): Promise { // negative-cached null) lingers forever. const liveIds = new Set(repos.map((r) => r.id)) for (const id of slugByRepoId.keys()) { - if (!liveIds.has(id)) {slugByRepoId.delete(id)} + if (!liveIds.has(id)) { + slugByRepoId.delete(id) + } } const next: SlugIndex = new Map() const results = await Promise.all( repos.map(async (r) => ({ repo: r, slug: await resolveRepoSlug(r) })) ) for (const { repo, slug } of results) { - if (slug) {next.set(slug, repo)} + if (slug) { + next.set(slug, repo) + } } return next } @@ -94,16 +98,21 @@ export function useRepoSlugIndex(): (slug: string | null | undefined) => Repo | useEffect(() => { const gen = ++generationRef.current void buildIndex(repos).then((next) => { - if (gen !== generationRef.current) {return} + if (gen !== generationRef.current) { + return + } setIndex(next) }) }, [repos]) return useMemo( - () => (slug: string | null | undefined): Repo | null => { - if (!slug) {return null} - return index.get(slug.toLowerCase()) ?? null - }, + () => + (slug: string | null | undefined): Repo | null => { + if (!slug) { + return null + } + return index.get(slug.toLowerCase()) ?? null + }, [index] ) } diff --git a/src/renderer/src/lib/telemetry.ts b/src/renderer/src/lib/telemetry.ts index 073de2444..2bf00b879 100644 --- a/src/renderer/src/lib/telemetry.ts +++ b/src/renderer/src/lib/telemetry.ts @@ -15,6 +15,12 @@ import type { EventName, EventProps } from '../../../shared/telemetry-events' import type { TelemetryConsentState } from '../../../shared/telemetry-consent-types' +// Re-exported so renderer call sites can import the mapper from this lib +// alongside `track`. The implementation lives in `src/shared/agent-kind.ts` +// because main-process telemetry emission needs the same mapping when it +// receives a `TuiAgent`-derived agent kind through the spawn IPC. +export { tuiAgentToAgentKind } from '../../../shared/agent-kind' + // Why: single source-of-truth for the privacy doc URL linked from the two // telemetry surfaces (FirstLaunchBanner, PrivacyPane). Keeping it here — in // the shared telemetry lib — prevents the surfaces from drifting if the doc diff --git a/src/renderer/src/lib/worktree-activation.test.ts b/src/renderer/src/lib/worktree-activation.test.ts index ecb917a50..26199221f 100644 --- a/src/renderer/src/lib/worktree-activation.test.ts +++ b/src/renderer/src/lib/worktree-activation.test.ts @@ -111,6 +111,34 @@ describe('ensureWorktreeHasInitialTerminal', () => { expect(store.queueTabIssueCommandSplit).not.toHaveBeenCalled() }) + it('forwards telemetry on the queued startup so main can fire agent_started', () => { + const store = createMockStore() + + ensureWorktreeHasInitialTerminal( + store, + 'wt-1', + { + command: 'claude', + telemetry: { + agent_kind: 'claude-code', + launch_source: 'new_workspace_composer', + request_kind: 'new' + } + }, + undefined, + undefined + ) + + expect(store.queueTabStartupCommand).toHaveBeenCalledWith('tab-1', { + command: 'claude', + telemetry: { + agent_kind: 'claude-code', + launch_source: 'new_workspace_composer', + request_kind: 'new' + } + }) + }) + it('does not create a terminal just because the legacy terminal slice is empty', () => { const store = createMockStore({ tabsByWorktree: { 'wt-1': [] }, diff --git a/src/renderer/src/lib/worktree-activation.ts b/src/renderer/src/lib/worktree-activation.ts index b9228bc68..f78de0d2a 100644 --- a/src/renderer/src/lib/worktree-activation.ts +++ b/src/renderer/src/lib/worktree-activation.ts @@ -1,4 +1,5 @@ import type { SetupSplitDirection, WorktreeSetupLaunch } from '../../../shared/types' +import type { EventProps } from '../../../shared/telemetry-events' import { shouldAutoCreateInitialTerminal } from '@/components/terminal/initial-terminal' import { buildSetupRunnerCommand } from './setup-runner' import { useAppStore } from '@/store' @@ -8,6 +9,11 @@ import { setWorktreeNavViewActivator } from '@/store/slices/worktree-nav-history' +/** Telemetry payload threaded from the launch site to `pty:spawn`. Main + * fires `agent_started` only after the spawn succeeds — see + * telemetry-plan.md§Agent launch semantics. */ +export type AgentStartedTelemetry = EventProps<'agent_started'> + // Why: issue commands can originate from two sources with different shapes — // (1) a repo-level runner script generated by main (WorktreeSetupLaunch), or // (2) a user-typed command template substituted in the TaskPage flow. @@ -30,7 +36,11 @@ type WorktreeActivationStore = { reconcileWorktreeTabModel: (worktreeId: string) => { renderableTabCount: number } queueTabStartupCommand: ( tabId: string, - startup: { command: string; env?: Record } + startup: { + command: string + env?: Record + telemetry?: AgentStartedTelemetry + } ) => void queueTabSetupSplit: ( tabId: string, @@ -65,7 +75,11 @@ export type ActivateAndRevealResult = { export function activateAndRevealWorktree( worktreeId: string, opts?: { - startup?: { command: string; env?: Record } + startup?: { + command: string + env?: Record + telemetry?: AgentStartedTelemetry + } setup?: WorktreeSetupLaunch issueCommand?: IssueCommandLaunch } @@ -133,7 +147,7 @@ export function activateAndRevealWorktree( export function ensureWorktreeHasInitialTerminal( store: WorktreeActivationStore, worktreeId: string, - startup?: { command: string; env?: Record }, + startup?: { command: string; env?: Record; telemetry?: AgentStartedTelemetry }, setup?: WorktreeSetupLaunch, issueCommand?: IssueCommandLaunch ): string | null { diff --git a/src/renderer/src/store/slices/terminals.ts b/src/renderer/src/store/slices/terminals.ts index e3b068fec..9531d5687 100644 --- a/src/renderer/src/store/slices/terminals.ts +++ b/src/renderer/src/store/slices/terminals.ts @@ -8,6 +8,7 @@ import type { Worktree, WorkspaceSessionState } from '../../../../shared/types' +import type { AgentStartedTelemetry } from '../../lib/worktree-activation' import { scheduleRuntimeGraphSync } from '@/runtime/sync-runtime-graph' import { clearTransientTerminalState, emptyLayoutSnapshot } from './terminal-helpers' import { isClaudeAgent, detectAgentStatusFromTitle } from '@/lib/agent-status' @@ -80,7 +81,17 @@ export type TerminalSlice = { expandedPaneByTabId: Record canExpandPaneByTabId: Record terminalLayoutsByTabId: Record - pendingStartupByTabId: Record }> + pendingStartupByTabId: Record< + string, + { + command: string + env?: Record + /** Telemetry metadata for the `agent_started` event. Threaded all the + * way to the `pty:spawn` IPC handler in main so the event fires only + * after spawn confirms — never on click-intent. */ + telemetry?: AgentStartedTelemetry + } + > /** Queued setup-split requests — when present, TerminalPane creates the * initial pane clean, then splits (vertical or horizontal per user setting) * and runs the command in the new pane so the main terminal stays @@ -163,11 +174,15 @@ export type TerminalSlice = { setTabLayout: (tabId: string, layout: TerminalLayoutSnapshot | null) => void queueTabStartupCommand: ( tabId: string, - startup: { command: string; env?: Record } + startup: { + command: string + env?: Record + telemetry?: AgentStartedTelemetry + } ) => void consumeTabStartupCommand: ( tabId: string - ) => { command: string; env?: Record } | null + ) => { command: string; env?: Record; telemetry?: AgentStartedTelemetry } | null queueTabSetupSplit: ( tabId: string, startup: { command: string; env?: Record; direction: SetupSplitDirection } diff --git a/src/renderer/src/store/slices/worktree-helpers.ts b/src/renderer/src/store/slices/worktree-helpers.ts index adf6a35c3..1f4d45026 100644 --- a/src/renderer/src/store/slices/worktree-helpers.ts +++ b/src/renderer/src/store/slices/worktree-helpers.ts @@ -2,6 +2,7 @@ import type { CreateWorktreeResult, CreateSparseCheckoutRequest, SetupDecision, + WorkspaceCreateTelemetrySource, Worktree, WorktreeMeta } from '../../../../shared/types' @@ -59,7 +60,11 @@ export type WorktreeSlice = { name: string, baseBranch?: string, setupDecision?: SetupDecision, - sparseCheckout?: CreateSparseCheckoutRequest + sparseCheckout?: CreateSparseCheckoutRequest, + /** Telemetry-only: which renderer surface initiated this create. Optional + * so existing callers default to `unknown`; specify when the surface + * matters for the activation funnel. */ + telemetrySource?: WorkspaceCreateTelemetrySource ) => Promise removeWorktree: ( worktreeId: string, diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index e59e27756..1e8dc8092 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -162,7 +162,14 @@ export const createWorktreeSlice: StateCreator set({ hasHydratedWorktreePurge: true }) }, - createWorktree: async (repoId, name, baseBranch, setupDecision = 'inherit', sparseCheckout) => { + createWorktree: async ( + repoId, + name, + baseBranch, + setupDecision = 'inherit', + sparseCheckout, + telemetrySource + ) => { const retryableConflictPatterns = [ /already exists locally/i, /already exists on a remote/i, @@ -180,7 +187,8 @@ export const createWorktreeSlice: StateCreator name: candidateName, baseBranch, setupDecision, - sparseCheckout + sparseCheckout, + ...(telemetrySource ? { telemetrySource } : {}) }) // Why: a file watcher (worktrees.onChanged) can fire between the // backend creating the worktree and this callback running, causing diff --git a/src/shared/agent-kind.ts b/src/shared/agent-kind.ts new file mode 100644 index 000000000..04bd0aed2 --- /dev/null +++ b/src/shared/agent-kind.ts @@ -0,0 +1,34 @@ +// Mapping from the renderer's `TuiAgent` union (every agent Orca knows how +// to launch) to the closed `agentKindSchema` enum on telemetry events. The +// telemetry enum is a deliberately smaller set — anything not enumerated +// maps to `'other'` so dashboards can spot interest in an agent before its +// slot is added rather than dropping the event. +// +// Lives in `src/shared/` (not the renderer) because main-side telemetry +// emission (`agent_started` from the `pty:spawn` IPC handler) needs the +// same mapping. Centralizing here means a new TuiAgent member is one edit, +// not a sweep across renderer + main. + +import type { AgentKind } from './telemetry-events' +import type { TuiAgent } from './types' + +export function tuiAgentToAgentKind(agent: TuiAgent): AgentKind { + switch (agent) { + case 'claude': + return 'claude-code' + case 'codex': + return 'codex' + case 'copilot': + return 'copilot' + case 'gemini': + return 'gemini' + case 'cursor': + return 'cursor' + case 'opencode': + return 'opencode' + case 'aider': + return 'aider' + default: + return 'other' + } +} diff --git a/src/shared/github-project-types.ts b/src/shared/github-project-types.ts index f5d2f0eef..90ea46f9e 100644 --- a/src/shared/github-project-types.ts +++ b/src/shared/github-project-types.ts @@ -59,9 +59,7 @@ export type GitHubProjectField = kind: 'field' id: string name: string - dataType: - | Exclude - | (string & {}) + dataType: Exclude | (string & {}) } | { kind: 'single-select' diff --git a/src/shared/telemetry-events.test.ts b/src/shared/telemetry-events.test.ts index ff6849578..04bc23be3 100644 --- a/src/shared/telemetry-events.test.ts +++ b/src/shared/telemetry-events.test.ts @@ -1,12 +1,10 @@ // Schema round-trip coverage for the event map. Fail-closed invariants that // must hold: agent_error is enum-only (error_message / error_stack rejected -// by `.strict()`), error_name is whitelisted, unknown enum values fail, and -// any well-formed payload round-trips without coercion. +// by `.strict()`), unknown enum values fail, and any well-formed payload +// round-trips without coercion. import { describe, expect, it } from 'vitest' import { - AGENT_ERROR_NAME_WHITELIST, - agentErrorNameSchema, agentKindSchema, commonPropsSchema, errorClassSchema, @@ -18,38 +16,18 @@ import { describe('agent_error schema', () => { it('round-trips a minimal {error_class, agent_kind} payload', () => { const parsed = eventSchemas.agent_error.safeParse({ - error_class: 'auth_expired', + error_class: 'unknown', agent_kind: 'claude-code' }) expect(parsed.success).toBe(true) }) - it('round-trips every whitelisted error_name value', () => { - for (const name of AGENT_ERROR_NAME_WHITELIST) { - const parsed = eventSchemas.agent_error.safeParse({ - error_class: 'auth_expired', - agent_kind: 'claude-code', - error_name: name - }) - expect(parsed.success).toBe(true) - } - }) - - it('rejects error_name values outside the whitelist', () => { - const parsed = eventSchemas.agent_error.safeParse({ - error_class: 'auth_expired', - agent_kind: 'claude-code', - error_name: 'SomeNonWhitelistedName' - }) - expect(parsed.success).toBe(false) - }) - // Core invariant: `.strict()` rejects raw error strings. If this test ever // flips, the analytics lane is leaking UGC — revert the offending schema // change. it('rejects error_message via .strict()', () => { const parsed = eventSchemas.agent_error.safeParse({ - error_class: 'auth_expired', + error_class: 'unknown', agent_kind: 'claude-code', error_message: 'boom at /Users/alice/secret/path' }) @@ -58,13 +36,25 @@ describe('agent_error schema', () => { it('rejects error_stack via .strict()', () => { const parsed = eventSchemas.agent_error.safeParse({ - error_class: 'auth_expired', + error_class: 'unknown', agent_kind: 'claude-code', error_stack: 'Error: boom\n at /Users/alice/...' }) expect(parsed.success).toBe(false) }) + it('rejects error_name (deferred — schema is enum-only)', () => { + // `error_name` was part of an earlier draft. The trimmed schema is + // enum-only; if a future PR re-introduces it as additive-optional, + // this test should be replaced rather than relaxed silently. + const parsed = eventSchemas.agent_error.safeParse({ + error_class: 'unknown', + agent_kind: 'claude-code', + error_name: 'BinaryNotFound' + }) + expect(parsed.success).toBe(false) + }) + it('rejects unknown error_class enum values', () => { const parsed = eventSchemas.agent_error.safeParse({ error_class: 'made_up_class', @@ -75,7 +65,7 @@ describe('agent_error schema', () => { it('rejects unknown agent_kind enum values', () => { const parsed = eventSchemas.agent_error.safeParse({ - error_class: 'auth_expired', + error_class: 'unknown', agent_kind: 'made_up_agent' }) expect(parsed.success).toBe(false) @@ -213,10 +203,4 @@ describe('exported enum schemas', () => { expect(settingsChangedKeySchema.safeParse(key).success).toBe(true) } }) - - it('agentErrorNameSchema membership matches AGENT_ERROR_NAME_WHITELIST', () => { - for (const name of AGENT_ERROR_NAME_WHITELIST) { - expect(agentErrorNameSchema.safeParse(name).success).toBe(true) - } - }) }) diff --git a/src/shared/telemetry-events.ts b/src/shared/telemetry-events.ts index bc5dcf2d7..91751968a 100644 --- a/src/shared/telemetry-events.ts +++ b/src/shared/telemetry-events.ts @@ -35,43 +35,22 @@ export const agentKindSchema = z.enum([ ]) export type AgentKind = z.infer -export const errorClassSchema = z.enum([ - 'network_timeout', - 'auth_expired', - 'rate_limited', - 'provider_unavailable', - 'provider_error_generic', - 'binary_not_found', - 'binary_version_mismatch', - 'workspace_gone', - 'user_cancelled', - 'unknown' -]) +// Trimmed to the two values Orca's PTY-typed-command launch architecture can +// actually emit: +// - `binary_not_found` — `provider.spawn` ENOENT (the *shell* binary is +// missing). The agent CLI being missing is invisible: Orca spawns a +// healthy shell and types the command, and bash/zsh's "command not found" +// surfaces only as terminal output. +// - `unknown` — every other thrown error (paste-readiness timeout, env-build +// failures, unclassifiable shell-spawn errors). +// Provider-side errors (`auth_expired`, `rate_limited`, `network_timeout`, +// `provider_*`) happen inside the agent CLI subprocess and are not observable +// to Orca — see telemetry-plan.md §Decision: Defer per-incident error fields. +// Adding a new value is additive-safe; do it when the call site lands, not in +// anticipation. +export const errorClassSchema = z.enum(['binary_not_found', 'unknown']) export type ErrorClass = z.infer -// Closed whitelist of error `name` strings allowed on `agent_error`. This is -// the one free-ish string that can leave the machine on an agent_error event -// — the validator drops anything not in this set. -// -// A regex-shape check (e.g. `/^[A-Z][A-Za-z]{0,32}$/`) would permit -// identifier-shaped leaks like `PaymentFailedForUserAlice` or -// `TimeoutInRepoMyCompanyInternalMonorepo` — context-concatenation bugs -// under deadline pressure. A closed whitelist forces each new error name -// through review. Same pattern as `SETTINGS_CHANGED_WHITELIST`. -export const AGENT_ERROR_NAME_WHITELIST = [ - 'NetworkTimeout', - 'AuthExpired', - 'RateLimited', - 'ProviderUnavailable', - 'ProviderErrorGeneric', - 'BinaryNotFound', - 'BinaryVersionMismatch', - 'WorkspaceGone', - 'UserCancelled' -] as const -export const agentErrorNameSchema = z.enum(AGENT_ERROR_NAME_WHITELIST) -export type AgentErrorName = z.infer - export const repoMethodSchema = z.enum(['folder_picker', 'clone_url', 'drag_drop']) export type RepoMethod = z.infer @@ -161,22 +140,15 @@ const agentStartedSchema = z }) .strict() -// Enum-only by design for `error_class` + `agent_kind`. `error_name` is the -// one free-ish string that can leave the machine on this event, and it is -// drawn from the closed `AGENT_ERROR_NAME_WHITELIST` — adding a new value -// requires a PR to the whitelist, giving review a chance to catch -// context-concatenation patterns. -// -// `error_message` and `error_stack` are deliberately absent from this schema. -// `.strict()` rejects either key if a call site ever tries to attach one, -// which fails the validator and drops the event. Raw error strings carry -// arbitrary user/workspace/path content; keeping them off the wire is the -// only way to guarantee we never transmit them by accident. +// Enum-only by design for both fields. `error_message` and `error_stack` are +// deliberately absent — `.strict()` rejects either key if a call site ever +// tries to attach one, which fails the validator and drops the event. Raw +// error strings carry arbitrary user/workspace/path content; keeping them off +// the wire is the only way to guarantee we never transmit them by accident. const agentErrorSchema = z .object({ error_class: errorClassSchema, - agent_kind: agentKindSchema, - error_name: agentErrorNameSchema.optional() + agent_kind: agentKindSchema }) .strict() diff --git a/src/shared/types.ts b/src/shared/types.ts index 424f56b5d..4fea0483f 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -1,7 +1,12 @@ /* eslint-disable max-lines */ import type { SshTarget } from './ssh-types' +import type { WorkspaceSource } from './telemetry-events' import type { GitHubProjectSettings } from './github-project-types' +// Re-exported for backward compat with renderer call sites that import +// `WorkspaceCreateTelemetrySource` from '../../../shared/types'. +export type { WorkspaceSource as WorkspaceCreateTelemetrySource } from './telemetry-events' + // ─── Repo ──────────────────────────────────────────────────────────── export type RepoKind = 'git' | 'folder' @@ -832,6 +837,15 @@ export type CreateWorktreeArgs = { baseBranch?: string setupDecision?: SetupDecision sparseCheckout?: CreateSparseCheckoutRequest + /** Telemetry-only: which UI surface initiated this create. Threaded from + * the renderer entry point so main can emit `workspace_created` with the + * correct `source`. `unknown` is a valid wire value — an unrecognized + * surface emits `source: 'unknown'` rather than dropping the event, so + * dashboards surface enum-coverage gaps as a slice rather than as + * missing data. Optional on the type so older renderer code paths that + * pre-date this prop default to `unknown` at the IPC boundary instead + * of failing typecheck. */ + telemetrySource?: WorkspaceSource } export type CreateWorktreeResult = {