From 83117f2860cf06254f363e23cc38d9aa1fbb742e Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 15 Aug 2026 18:17:20 -0700 Subject: [PATCH] refactor(integrations): split issue-tracker clients under the max-lines budget (#14704) The GitLab, GitHub, Jira and Linear integration modules, their two IPC registrars, and the shared GitHub project types each carried a file-level `eslint-disable max-lines` and ran 351-614 counted lines against a 300-line budget. AGENTS.md calls for splitting rather than suppressing, and config/max-lines-baseline.txt is a shrink-only ratchet, so this removes all eight suppressions and prunes their entries (341 -> 333). Pure move, no behavior change. Each client is cut along the seam it already had: per-operation modules for the issue APIs (create / update / comment / field options), and for Jira the request queue, site credential store, authenticated request, and site identity. The two IPC registrars keep their own handlers and delegate the rest to per-domain sub-registrars, so they remain real entry points rather than re-export shims. The IPC surface is proved intact rather than assumed: comparing (method, channel) multisets between HEAD and the split gives 52 registrations across 52 distinct channels on both sides. Provider-neutrality is preserved -- GitLab and GitHub keep separate, parallel module layouts rather than being merged behind a shared abstraction. Verified: oxlint clean, ratchet passes, typecheck clean, full unit suite green (the one remaining failure is a pre-existing load flake in an untouched file, green when re-run serially), no new runtime import cycles among 744 modules, and no lint suppression added anywhere. --- config/max-lines-baseline.txt | 8 - src/main/github/client.ts | 14 +- src/main/github/issue-comment.ts | 77 +++ src/main/github/issue-create.ts | 133 ++++ src/main/github/issue-field-options.ts | 122 ++++ src/main/github/issue-update.ts | 130 ++++ src/main/github/issues.test.ts | 14 +- src/main/github/issues.ts | 451 +------------- src/main/github/project-view.ts | 24 +- src/main/github/project-view/internals.ts | 2 +- src/main/github/project-view/mutations.ts | 18 +- .../project-error-classification.ts | 2 +- src/main/gitlab/client.ts | 12 +- .../gitlab/gitlab-assignable-user-mapping.ts | 22 + src/main/gitlab/issue-update.ts | 155 +++++ src/main/gitlab/issues.test.ts | 12 +- src/main/gitlab/issues.ts | 271 +------- src/main/gitlab/mr-discussion-notes.ts | 83 +++ src/main/gitlab/mr-file-diffs.ts | 94 +++ src/main/gitlab/mr-reviewers-and-approvals.ts | 94 +++ src/main/gitlab/pipeline-job-graph.ts | 241 ++++++++ .../gitlab/project-label-and-member-lookup.ts | 124 ++++ src/main/gitlab/project-path-encoding.ts | 6 + src/main/gitlab/work-item-details.test.ts | 3 +- src/main/gitlab/work-item-details.ts | 513 +-------------- src/main/ipc/github.ts | 2 +- src/main/ipc/gitlab-ci-job-handlers.ts | 50 ++ src/main/ipc/gitlab-issue-handlers.ts | 150 +++++ .../gitlab-merge-request-mutation-handlers.ts | 172 +++++ .../gitlab-merge-request-query-handlers.ts | 76 +++ src/main/ipc/gitlab-repo-access.ts | 65 ++ src/main/ipc/gitlab-work-item-handlers.ts | 100 +++ src/main/ipc/gitlab.ts | 585 +----------------- src/main/ipc/linear-custom-view-handlers.ts | 99 +++ src/main/ipc/linear-ipc-args.ts | 49 ++ src/main/ipc/linear-issue-handlers.ts | 200 ++++++ src/main/ipc/linear-project-handlers.ts | 133 ++++ src/main/ipc/linear-team-handlers.ts | 43 ++ src/main/ipc/linear.ts | 504 +-------------- src/main/jira/attachment-images.test.ts | 4 +- src/main/jira/attachment-images.ts | 4 +- src/main/jira/authenticated-request.ts | 192 ++++++ src/main/jira/client.test.ts | 9 +- src/main/jira/client.ts | 533 +--------------- src/main/jira/issues.test.ts | 17 +- src/main/jira/issues.ts | 13 +- .../jira/jira-issue-summary-timeout.test.ts | 18 +- src/main/jira/jira-search-abort.test.ts | 18 +- src/main/jira/request-queue.ts | 57 ++ src/main/jira/site-credential-store.ts | 210 +++++++ src/main/jira/site-identity.ts | 54 ++ src/main/linear/client.ts | 517 +--------------- src/main/linear/issue-context-client.test.ts | 12 +- src/main/linear/issue-context-client.ts | 5 +- src/main/linear/issue-context.test.ts | 12 +- src/main/linear/issue-relation-write.test.ts | 14 +- src/main/linear/issue-relation-write.ts | 4 +- src/main/linear/issues.test.ts | 14 +- src/main/linear/issues.ts | 11 +- src/main/linear/linear-credential-paths.ts | 46 ++ src/main/linear/linear-legacy-viewer-store.ts | 58 ++ src/main/linear/linear-request-concurrency.ts | 25 + src/main/linear/linear-token-store.ts | 145 +++++ src/main/linear/linear-workspace-record.ts | 63 ++ src/main/linear/linear-workspace-registry.ts | 240 +++++++ src/main/linear/mcp-issue-list.test.ts | 12 +- src/main/linear/projects.test.ts | 14 +- src/main/linear/projects.ts | 11 +- src/main/linear/teams.test.ts | 14 +- src/main/linear/teams.ts | 4 +- src/main/runtime/orca-runtime.ts | 2 +- src/preload/api/github-work-item-api.ts | 24 +- src/preload/index.ts | 24 +- .../github-project/GhAuthErrorHelp.tsx | 2 +- .../components/github-project/ProjectCell.tsx | 4 +- .../github-project/ProjectPicker.tsx | 6 +- .../github-project/ProjectViewWrapper.tsx | 10 +- .../github-project/slug-dialog/Comments.tsx | 2 +- .../src/hooks/useGitHubSlugMetadata.ts | 2 +- src/renderer/src/store/slices/github.ts | 12 +- src/shared/github/project-request-types.ts | 132 ++++ src/shared/github/project-result-types.ts | 103 +++ src/shared/github/project-types.ts | 222 ------- 83 files changed, 4051 insertions(+), 3697 deletions(-) create mode 100644 src/main/github/issue-comment.ts create mode 100644 src/main/github/issue-create.ts create mode 100644 src/main/github/issue-field-options.ts create mode 100644 src/main/github/issue-update.ts create mode 100644 src/main/gitlab/gitlab-assignable-user-mapping.ts create mode 100644 src/main/gitlab/issue-update.ts create mode 100644 src/main/gitlab/mr-discussion-notes.ts create mode 100644 src/main/gitlab/mr-file-diffs.ts create mode 100644 src/main/gitlab/mr-reviewers-and-approvals.ts create mode 100644 src/main/gitlab/pipeline-job-graph.ts create mode 100644 src/main/gitlab/project-label-and-member-lookup.ts create mode 100644 src/main/gitlab/project-path-encoding.ts create mode 100644 src/main/ipc/gitlab-ci-job-handlers.ts create mode 100644 src/main/ipc/gitlab-issue-handlers.ts create mode 100644 src/main/ipc/gitlab-merge-request-mutation-handlers.ts create mode 100644 src/main/ipc/gitlab-merge-request-query-handlers.ts create mode 100644 src/main/ipc/gitlab-repo-access.ts create mode 100644 src/main/ipc/gitlab-work-item-handlers.ts create mode 100644 src/main/ipc/linear-custom-view-handlers.ts create mode 100644 src/main/ipc/linear-ipc-args.ts create mode 100644 src/main/ipc/linear-issue-handlers.ts create mode 100644 src/main/ipc/linear-project-handlers.ts create mode 100644 src/main/ipc/linear-team-handlers.ts create mode 100644 src/main/jira/authenticated-request.ts create mode 100644 src/main/jira/request-queue.ts create mode 100644 src/main/jira/site-credential-store.ts create mode 100644 src/main/jira/site-identity.ts create mode 100644 src/main/linear/linear-credential-paths.ts create mode 100644 src/main/linear/linear-legacy-viewer-store.ts create mode 100644 src/main/linear/linear-request-concurrency.ts create mode 100644 src/main/linear/linear-token-store.ts create mode 100644 src/main/linear/linear-workspace-record.ts create mode 100644 src/main/linear/linear-workspace-registry.ts create mode 100644 src/shared/github/project-request-types.ts create mode 100644 src/shared/github/project-result-types.ts diff --git a/config/max-lines-baseline.txt b/config/max-lines-baseline.txt index 216a081c84a..a1f6c29c3df 100644 --- a/config/max-lines-baseline.txt +++ b/config/max-lines-baseline.txt @@ -41,31 +41,24 @@ inline src/main/git/runner.ts inline src/main/git/status.ts inline src/main/git/worktree.ts inline src/main/github/client.ts -inline src/main/github/issues.ts inline src/main/github/pr-refresh-coordinator.ts inline src/main/github/project-view.ts inline src/main/github/project-view/mutations.ts inline src/main/github/work-item-details.ts inline src/main/gitlab/client.ts -inline src/main/gitlab/issues.ts -inline src/main/gitlab/work-item-details.ts inline src/main/hermes/hook-service.ts inline src/main/hooks.ts inline src/main/index.ts inline src/main/ipc/filesystem-watcher.ts inline src/main/ipc/filesystem.ts inline src/main/ipc/github.ts -inline src/main/ipc/gitlab.ts -inline src/main/ipc/linear.ts inline src/main/ipc/pty.ts inline src/main/ipc/repos.ts inline src/main/ipc/ssh.ts inline src/main/ipc/worktree-remote.ts inline src/main/ipc/worktrees.ts -inline src/main/jira/client.ts inline src/main/jira/issues.ts inline src/main/keybindings/keybinding-file.ts -inline src/main/linear/client.ts inline src/main/linear/issues.ts inline src/main/linear/projects.ts inline src/main/memory/collector.ts @@ -218,7 +211,6 @@ inline src/shared/agent-hook-listener.ts inline src/shared/automation-schedules.ts inline src/shared/commit-message-agent-spec.ts inline src/shared/constants.ts -inline src/shared/github/project-types.ts inline src/shared/keybindings.ts inline src/shared/marine-creatures.ts inline src/shared/remote-runtime-client.ts diff --git a/src/main/github/client.ts b/src/main/github/client.ts index 5391539d353..c3cae80520f 100644 --- a/src/main/github/client.ts +++ b/src/main/github/client.ts @@ -96,15 +96,11 @@ import { } from './github-api-repository' import { githubRepoIdentityKey } from '../../shared/github/repository-identity-key' export { _resetOwnerRepoCache } from './gh-utils' -export { - getIssue, - listIssues, - createIssue, - updateIssue, - addIssueComment, - listLabels, - listAssignableUsers -} from './issues' +export { getIssue, listIssues } from './issues' +export { createIssue } from './issue-create' +export { updateIssue } from './issue-update' +export { addIssueComment } from './issue-comment' +export { listLabels, listAssignableUsers } from './issue-field-options' import { mapCheckRunRESTStatus, mapCheckRunRESTConclusion, diff --git a/src/main/github/issue-comment.ts b/src/main/github/issue-comment.ts new file mode 100644 index 00000000000..ac3f6a205de --- /dev/null +++ b/src/main/github/issue-comment.ts @@ -0,0 +1,77 @@ +import type { GitHubCommentResult, PRComment } from '../../shared/github/comment-types' +import type { LocalGitExecOptions, OwnerRepo } from './gh-utils' +import { getIssueGitHubApiRepository, resolveGitHubRepoExecution } from './github-api-repository' +import { acquire, classifyGhError, ghExecFileAsync, release } from './gh-utils' + +/** + * Add a comment to an existing GitHub issue. + * + * Why this path doesn't take a preference (mirrors `getIssue` / `updateIssue`): + * a comment is posted against an issue number already bound to a worktree or + * surfaced from a prior read. Routing through the live per-repo preference + * would let a user read upstream#N, toggle the selector to origin, and have + * their reply silently post on origin#N — a different issue entirely. That + * is the same silent-source-switch class of wrongness #1186 / the parent + * design doc guard against. List and create paths honor preference; + * mutations stay on the heuristic `getIssueOwnerRepo`. + */ +export async function addIssueComment( + repoPath: string, + issueNumber: number, + body: string, + connectionId?: string | null, + ownerRepoOverride?: OwnerRepo | null, + localGitOptions: LocalGitExecOptions = {} +): Promise { + const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution( + repoPath, + ownerRepoOverride ?? + (() => getIssueGitHubApiRepository(repoPath, connectionId, localGitOptions)), + connectionId, + localGitOptions + ) + if (!ownerRepo) { + return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' } + } + await acquire() + try { + const { stdout } = await ghExecFileAsync( + [ + 'api', + '-X', + 'POST', + `repos/${ownerRepo.owner}/${ownerRepo.repo}/issues/${issueNumber}/comments`, + '--raw-field', + `body=${body}` + ], + ghOptions + ) + const data = JSON.parse(stdout) as { + id?: number + node_id?: string | null + user: { login: string; avatar_url: string; type?: string } | null + body?: string + created_at?: string + html_url?: string + } + if (typeof data.id !== 'number' || !Number.isSafeInteger(data.id) || data.id < 1) { + return { ok: false, error: 'Unexpected response from GitHub' } + } + const comment: PRComment = { + id: data.id, + reactionSubjectId: data.node_id?.trim() || undefined, + author: data.user?.login ?? 'You', + authorAvatarUrl: data.user?.avatar_url ?? '', + body: data.body ?? body, + createdAt: data.created_at ?? new Date().toISOString(), + url: data.html_url ?? '', + isBot: data.user?.type === 'Bot' + } + return { ok: true, comment } + } catch (err) { + const stderr = err instanceof Error ? err.message : String(err) + return { ok: false, error: classifyGhError(stderr).message } + } finally { + release() + } +} diff --git a/src/main/github/issue-create.ts b/src/main/github/issue-create.ts new file mode 100644 index 00000000000..3b8af028706 --- /dev/null +++ b/src/main/github/issue-create.ts @@ -0,0 +1,133 @@ +import type { + GitHubCreateIssueFields, + GitHubCreateIssueResult +} from '../../shared/issue-mutation-types' +import type { IssueSourcePreference } from '../../shared/repo-types' +import type { LocalGitExecOptions } from './gh-utils' +import { + resolveGitHubRepoExecution, + resolveIssueGitHubApiRepositorySource +} from './github-api-repository' +import { acquire, extractExecError, ghExecFileAsync, release } from './gh-utils' + +function githubIssueErrorMessage(error: unknown): string { + const { stderr, stdout } = extractExecError(error) + return stderr.trim() || stdout.trim() +} + +/** + * Create a new GitHub issue. Uses `gh api` with explicit owner/repo so the + * call does not depend on the current working directory having a remote that + * matches the repo the user picked in the tasks page. + */ +export async function createIssue( + repoPath: string, + title: string, + body: string, + preference?: IssueSourcePreference, + connectionId?: string | null, + fields?: GitHubCreateIssueFields, + localGitOptions: LocalGitExecOptions = {} +): Promise { + const trimmedTitle = title.trim() + if (!trimmedTitle) { + return { ok: false, error: 'Title is required' } + } + const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution( + repoPath, + async () => + ( + await resolveIssueGitHubApiRepositorySource( + repoPath, + preference, + connectionId, + localGitOptions + ) + ).source, + connectionId, + localGitOptions + ) + if (!ownerRepo) { + return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' } + } + await acquire() + try { + const createArgs = (issueBody: string) => { + const args = [ + 'api', + '-X', + 'POST', + `repos/${ownerRepo.owner}/${ownerRepo.repo}/issues`, + '--raw-field', + `title=${trimmedTitle}`, + '--raw-field', + `body=${issueBody}` + ] + for (const label of fields?.labels ?? []) { + args.push('--raw-field', `labels[]=${label}`) + } + for (const assignee of fields?.assignees ?? []) { + args.push('--raw-field', `assignees[]=${assignee}`) + } + return args + } + + const parseIssue = (stdout: string) => + JSON.parse(stdout) as { number?: number; html_url?: string; url?: string } + + let data: { number?: number; html_url?: string; url?: string } + try { + const { stdout } = await ghExecFileAsync(createArgs(body), ghOptions) + data = parseIssue(stdout) + } catch (err) { + const message = githubIssueErrorMessage(err) + if (!/body is too long \(maximum is \d+ characters\)/i.test(message)) { + return { ok: false, error: message } + } + + // Why: GitHub rejects oversized bodies on create but accepts the same body + // on update, so establish the issue before attaching its body. + const { stdout } = await ghExecFileAsync(createArgs(''), ghOptions) + data = parseIssue(stdout) + if (typeof data.number !== 'number') { + return { ok: false, error: 'Unexpected response from GitHub' } + } + + try { + await ghExecFileAsync( + [ + 'api', + '-X', + 'PATCH', + `repos/${ownerRepo.owner}/${ownerRepo.repo}/issues/${data.number}`, + '--raw-field', + `body=${body}` + ], + ghOptions + ) + } catch (patchErr) { + const patchMessage = githubIssueErrorMessage(patchErr) + const identity = data.html_url ?? data.url ?? `#${data.number}` + return { + ok: true, + number: data.number, + url: String(data.html_url ?? data.url ?? ''), + bodySaveWarning: `Issue ${identity} was created, but saving its body failed: ${patchMessage}` + } + } + } + + if (typeof data.number !== 'number') { + return { ok: false, error: 'Unexpected response from GitHub' } + } + return { + ok: true, + number: data.number, + url: String(data.html_url ?? data.url ?? '') + } + } catch (err) { + return { ok: false, error: githubIssueErrorMessage(err) } + } finally { + release() + } +} diff --git a/src/main/github/issue-field-options.ts b/src/main/github/issue-field-options.ts new file mode 100644 index 00000000000..a4d2aae32f4 --- /dev/null +++ b/src/main/github/issue-field-options.ts @@ -0,0 +1,122 @@ +import type { GitHubAssignableUser } from '../../shared/github/pull-request-types' +import type { IssueSourcePreference } from '../../shared/repo-types' +import type { LocalGitExecOptions } from './gh-utils' +import { + resolveGitHubRepoExecution, + resolveIssueGitHubApiRepositorySource +} from './github-api-repository' +import { acquire, ghExecFileAsync, release } from './gh-utils' + +export async function listLabels( + repoPath: string, + preference?: IssueSourcePreference, + connectionId?: string | null, + localGitOptions: LocalGitExecOptions = {} +): Promise { + const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution( + repoPath, + async () => + ( + await resolveIssueGitHubApiRepositorySource( + repoPath, + preference, + connectionId, + localGitOptions + ) + ).source, + connectionId, + localGitOptions + ) + if (!ownerRepo) { + return [] + } + await acquire() + try { + const { stdout } = await ghExecFileAsync( + [ + 'api', + '--paginate', + `repos/${ownerRepo.owner}/${ownerRepo.repo}/labels`, + '--jq', + '.[].name' + ], + ghOptions + ) + return stdout + .trim() + .split('\n') + .filter((l) => l.length > 0) + } catch { + return [] + } finally { + release() + } +} + +export async function listAssignableUsers( + repoPath: string, + preference?: IssueSourcePreference, + connectionId?: string | null, + localGitOptions: LocalGitExecOptions = {} +): Promise { + const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution( + repoPath, + async () => + ( + await resolveIssueGitHubApiRepositorySource( + repoPath, + preference, + connectionId, + localGitOptions + ) + ).source, + connectionId, + localGitOptions + ) + if (!ownerRepo) { + return [] + } + await acquire() + try { + // Why: paginate through all assignable users — GraphQL's assignableUsers + // maxes out at 100 per page and large orgs/repos silently lose assignees + // beyond the first page. REST /assignees with --paginate walks every page; + // --jq collapses per-page arrays into NDJSON so we don't have to stitch + // JSON arrays that gh concatenates back-to-back. + const { stdout } = await ghExecFileAsync( + [ + 'api', + '--paginate', + `repos/${ownerRepo.owner}/${ownerRepo.repo}/assignees?per_page=100`, + '--jq', + '.[] | {login, avatar_url}' + ], + ghOptions + ) + type RESTAssignee = { login?: string; avatar_url?: string | null } + const users: GitHubAssignableUser[] = [] + for (const line of stdout.split('\n')) { + const trimmed = line.trim() + if (!trimmed) { + continue + } + try { + const user = JSON.parse(trimmed) as RESTAssignee + if (user.login) { + users.push({ + login: user.login, + name: null, + avatarUrl: user.avatar_url ?? '' + }) + } + } catch { + // Skip malformed NDJSON lines defensively. + } + } + return users + } catch { + return [] + } finally { + release() + } +} diff --git a/src/main/github/issue-update.ts b/src/main/github/issue-update.ts new file mode 100644 index 00000000000..801c12b79c5 --- /dev/null +++ b/src/main/github/issue-update.ts @@ -0,0 +1,130 @@ +import type { GitHubIssueUpdate } from '../../shared/issue-mutation-types' +import type { LocalGitExecOptions } from './gh-utils' +import { getIssueGitHubApiRepository, resolveGitHubRepoExecution } from './github-api-repository' +import { acquire, classifyGhError, ghExecFileAsync, release } from './gh-utils' + +/** + * Update an existing GitHub issue. Fans out to separate gh commands for + * state changes vs field edits since `gh issue edit` does not support state. + * + * Why this path doesn't take a preference (mirrors `getIssue`): mutations + * target an issue number already bound to a worktree / linked elsewhere in + * the UI. Routing an update through the live per-repo preference would let + * a user open upstream#N, toggle the selector to origin, save, and silently + * write to origin#N — a different issue (or 404). That is the exact + * silent-source-switch class of wrongness #1186 / the parent design doc + * guard against. List and create paths honor preference; mutations stay on + * the heuristic `getIssueOwnerRepo`. + */ +export async function updateIssue( + repoPath: string, + issueNumber: number, + updates: GitHubIssueUpdate, + connectionId?: string | null, + localGitOptions: LocalGitExecOptions = {} +): Promise<{ ok: true } | { ok: false; error: string }> { + const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution( + repoPath, + () => getIssueGitHubApiRepository(repoPath, connectionId, localGitOptions), + connectionId, + localGitOptions + ) + if (!ownerRepo) { + return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' } + } + + const repo = `${ownerRepo.owner}/${ownerRepo.repo}` + const errors: string[] = [] + + // State change requires a separate command + if (updates.state) { + await acquire() + try { + if (updates.state === 'closed') { + const closeArgs = ['issue', 'close', String(issueNumber), '--repo', repo] + if (updates.stateReason === 'completed') { + closeArgs.push('--reason', 'completed') + } else if (updates.stateReason === 'not_planned') { + closeArgs.push('--reason', 'not planned') + } else if (updates.stateReason === 'duplicate' && updates.duplicateOf) { + closeArgs.push('--duplicate-of', String(updates.duplicateOf)) + } + await ghExecFileAsync(closeArgs, ghOptions) + } else { + await ghExecFileAsync(['issue', 'reopen', String(issueNumber), '--repo', repo], ghOptions) + } + } catch (err) { + const stderr = err instanceof Error ? err.message : String(err) + // Treat "already closed/open" as a no-op + if (!stderr.toLowerCase().includes('already')) { + errors.push(classifyGhError(stderr).message) + } + } finally { + release() + } + } + + if (updates.body !== undefined) { + await acquire() + try { + await ghExecFileAsync( + [ + 'api', + '-X', + 'PATCH', + `repos/${ownerRepo.owner}/${ownerRepo.repo}/issues/${issueNumber}`, + '--raw-field', + `body=${updates.body}` + ], + ghOptions + ) + } catch (err) { + const stderr = err instanceof Error ? err.message : String(err) + errors.push(classifyGhError(stderr).message) + } finally { + release() + } + } + + // Field edits (labels, assignees, title) via gh issue edit + const editArgs: string[] = ['issue', 'edit', String(issueNumber), '--repo', repo] + let hasEditArgs = false + + if (updates.title) { + editArgs.push('--title', updates.title) + hasEditArgs = true + } + for (const label of updates.addLabels ?? []) { + editArgs.push('--add-label', label) + hasEditArgs = true + } + for (const label of updates.removeLabels ?? []) { + editArgs.push('--remove-label', label) + hasEditArgs = true + } + for (const assignee of updates.addAssignees ?? []) { + editArgs.push('--add-assignee', assignee) + hasEditArgs = true + } + for (const assignee of updates.removeAssignees ?? []) { + editArgs.push('--remove-assignee', assignee) + hasEditArgs = true + } + + if (hasEditArgs) { + await acquire() + try { + await ghExecFileAsync(editArgs, ghOptions) + } catch (err) { + const stderr = err instanceof Error ? err.message : String(err) + errors.push(classifyGhError(stderr).message) + } finally { + release() + } + } + + if (errors.length > 0) { + return { ok: false, error: errors.join('; ') } + } + return { ok: true } +} diff --git a/src/main/github/issues.test.ts b/src/main/github/issues.test.ts index 23a7242810c..f70443dea0e 100644 --- a/src/main/github/issues.test.ts +++ b/src/main/github/issues.test.ts @@ -68,15 +68,11 @@ vi.mock('./github-api-repository', async (importOriginal) => { } }) -import { - addIssueComment, - createIssue, - getIssue, - listAssignableUsers, - listIssues, - listLabels, - updateIssue -} from './issues' +import { getIssue, listIssues } from './issues' +import { createIssue } from './issue-create' +import { updateIssue } from './issue-update' +import { addIssueComment } from './issue-comment' +import { listAssignableUsers, listLabels } from './issue-field-options' import { _resetOriginGitHubApiRepositoryCache } from './github-api-repository' diff --git a/src/main/github/issues.ts b/src/main/github/issues.ts index 40cf4e5fa26..a9f24107393 100644 --- a/src/main/github/issues.ts +++ b/src/main/github/issues.ts @@ -1,25 +1,15 @@ -/* eslint-disable max-lines -- Why: co-locating issue list/create/update/ -comment operations keeps the shared acquire/release + error-classification -pattern obvious. Each function is short; the file is long because the -surface is broad. */ import type { ClassifiedError } from '../../shared/classified-error' -import type { GitHubCommentResult, PRComment } from '../../shared/github/comment-types' -import type { GitHubAssignableUser, IssueInfo } from '../../shared/github/pull-request-types' -import type { - GitHubCreateIssueFields, - GitHubCreateIssueResult, - GitHubIssueUpdate -} from '../../shared/issue-mutation-types' +import type { IssueInfo } from '../../shared/github/pull-request-types' import type { IssueSourcePreference } from '../../shared/repo-types' import { mapIssueInfo } from './mappers' -import type { LocalGitExecOptions, OwnerRepo } from './gh-utils' +import type { LocalGitExecOptions } from './gh-utils' import { getIssueGitHubApiRepository, resolveGitHubRepoExecution, resolveIssueGitHubApiRepositorySource } from './github-api-repository' // prettier-ignore -import { ghExecFileAsync, acquire, release, classifyGhError, classifyListIssuesError, extractExecError } from './gh-utils' +import { ghExecFileAsync, acquire, release, classifyListIssuesError } from './gh-utils' // Why: distinguishes a successful-empty listing from a failed fetch. The // previous `catch { return [] }` conflated a 403 on a private upstream with an @@ -35,11 +25,6 @@ export type IssueListResult = { error?: ClassifiedError } -function githubIssueErrorMessage(error: unknown): string { - const { stderr, stdout } = extractExecError(error) - return stderr.trim() || stdout.trim() -} - /** * Get a single issue by number. * Uses gh api --cache so 304 Not Modified responses don't count against the rate limit. @@ -182,433 +167,3 @@ export async function listIssues( release() } } - -/** - * Create a new GitHub issue. Uses `gh api` with explicit owner/repo so the - * call does not depend on the current working directory having a remote that - * matches the repo the user picked in the tasks page. - */ -export async function createIssue( - repoPath: string, - title: string, - body: string, - preference?: IssueSourcePreference, - connectionId?: string | null, - fields?: GitHubCreateIssueFields, - localGitOptions: LocalGitExecOptions = {} -): Promise { - const trimmedTitle = title.trim() - if (!trimmedTitle) { - return { ok: false, error: 'Title is required' } - } - const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution( - repoPath, - async () => - ( - await resolveIssueGitHubApiRepositorySource( - repoPath, - preference, - connectionId, - localGitOptions - ) - ).source, - connectionId, - localGitOptions - ) - if (!ownerRepo) { - return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' } - } - await acquire() - try { - const createArgs = (issueBody: string) => { - const args = [ - 'api', - '-X', - 'POST', - `repos/${ownerRepo.owner}/${ownerRepo.repo}/issues`, - '--raw-field', - `title=${trimmedTitle}`, - '--raw-field', - `body=${issueBody}` - ] - for (const label of fields?.labels ?? []) { - args.push('--raw-field', `labels[]=${label}`) - } - for (const assignee of fields?.assignees ?? []) { - args.push('--raw-field', `assignees[]=${assignee}`) - } - return args - } - - const parseIssue = (stdout: string) => - JSON.parse(stdout) as { number?: number; html_url?: string; url?: string } - - let data: { number?: number; html_url?: string; url?: string } - try { - const { stdout } = await ghExecFileAsync(createArgs(body), ghOptions) - data = parseIssue(stdout) - } catch (err) { - const message = githubIssueErrorMessage(err) - if (!/body is too long \(maximum is \d+ characters\)/i.test(message)) { - return { ok: false, error: message } - } - - // Why: GitHub rejects oversized bodies on create but accepts the same body - // on update, so establish the issue before attaching its body. - const { stdout } = await ghExecFileAsync(createArgs(''), ghOptions) - data = parseIssue(stdout) - if (typeof data.number !== 'number') { - return { ok: false, error: 'Unexpected response from GitHub' } - } - - try { - await ghExecFileAsync( - [ - 'api', - '-X', - 'PATCH', - `repos/${ownerRepo.owner}/${ownerRepo.repo}/issues/${data.number}`, - '--raw-field', - `body=${body}` - ], - ghOptions - ) - } catch (patchErr) { - const patchMessage = githubIssueErrorMessage(patchErr) - const identity = data.html_url ?? data.url ?? `#${data.number}` - return { - ok: true, - number: data.number, - url: String(data.html_url ?? data.url ?? ''), - bodySaveWarning: `Issue ${identity} was created, but saving its body failed: ${patchMessage}` - } - } - } - - if (typeof data.number !== 'number') { - return { ok: false, error: 'Unexpected response from GitHub' } - } - return { - ok: true, - number: data.number, - url: String(data.html_url ?? data.url ?? '') - } - } catch (err) { - return { ok: false, error: githubIssueErrorMessage(err) } - } finally { - release() - } -} - -/** - * Update an existing GitHub issue. Fans out to separate gh commands for - * state changes vs field edits since `gh issue edit` does not support state. - * - * Why this path doesn't take a preference (mirrors `getIssue`): mutations - * target an issue number already bound to a worktree / linked elsewhere in - * the UI. Routing an update through the live per-repo preference would let - * a user open upstream#N, toggle the selector to origin, save, and silently - * write to origin#N — a different issue (or 404). That is the exact - * silent-source-switch class of wrongness #1186 / the parent design doc - * guard against. List and create paths honor preference; mutations stay on - * the heuristic `getIssueOwnerRepo`. - */ -export async function updateIssue( - repoPath: string, - issueNumber: number, - updates: GitHubIssueUpdate, - connectionId?: string | null, - localGitOptions: LocalGitExecOptions = {} -): Promise<{ ok: true } | { ok: false; error: string }> { - const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution( - repoPath, - () => getIssueGitHubApiRepository(repoPath, connectionId, localGitOptions), - connectionId, - localGitOptions - ) - if (!ownerRepo) { - return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' } - } - - const repo = `${ownerRepo.owner}/${ownerRepo.repo}` - const errors: string[] = [] - - // State change requires a separate command - if (updates.state) { - await acquire() - try { - if (updates.state === 'closed') { - const closeArgs = ['issue', 'close', String(issueNumber), '--repo', repo] - if (updates.stateReason === 'completed') { - closeArgs.push('--reason', 'completed') - } else if (updates.stateReason === 'not_planned') { - closeArgs.push('--reason', 'not planned') - } else if (updates.stateReason === 'duplicate' && updates.duplicateOf) { - closeArgs.push('--duplicate-of', String(updates.duplicateOf)) - } - await ghExecFileAsync(closeArgs, ghOptions) - } else { - await ghExecFileAsync(['issue', 'reopen', String(issueNumber), '--repo', repo], ghOptions) - } - } catch (err) { - const stderr = err instanceof Error ? err.message : String(err) - // Treat "already closed/open" as a no-op - if (!stderr.toLowerCase().includes('already')) { - errors.push(classifyGhError(stderr).message) - } - } finally { - release() - } - } - - if (updates.body !== undefined) { - await acquire() - try { - await ghExecFileAsync( - [ - 'api', - '-X', - 'PATCH', - `repos/${ownerRepo.owner}/${ownerRepo.repo}/issues/${issueNumber}`, - '--raw-field', - `body=${updates.body}` - ], - ghOptions - ) - } catch (err) { - const stderr = err instanceof Error ? err.message : String(err) - errors.push(classifyGhError(stderr).message) - } finally { - release() - } - } - - // Field edits (labels, assignees, title) via gh issue edit - const editArgs: string[] = ['issue', 'edit', String(issueNumber), '--repo', repo] - let hasEditArgs = false - - if (updates.title) { - editArgs.push('--title', updates.title) - hasEditArgs = true - } - for (const label of updates.addLabels ?? []) { - editArgs.push('--add-label', label) - hasEditArgs = true - } - for (const label of updates.removeLabels ?? []) { - editArgs.push('--remove-label', label) - hasEditArgs = true - } - for (const assignee of updates.addAssignees ?? []) { - editArgs.push('--add-assignee', assignee) - hasEditArgs = true - } - for (const assignee of updates.removeAssignees ?? []) { - editArgs.push('--remove-assignee', assignee) - hasEditArgs = true - } - - if (hasEditArgs) { - await acquire() - try { - await ghExecFileAsync(editArgs, ghOptions) - } catch (err) { - const stderr = err instanceof Error ? err.message : String(err) - errors.push(classifyGhError(stderr).message) - } finally { - release() - } - } - - if (errors.length > 0) { - return { ok: false, error: errors.join('; ') } - } - return { ok: true } -} - -/** - * Add a comment to an existing GitHub issue. - * - * Why this path doesn't take a preference (mirrors `getIssue` / `updateIssue`): - * a comment is posted against an issue number already bound to a worktree or - * surfaced from a prior read. Routing through the live per-repo preference - * would let a user read upstream#N, toggle the selector to origin, and have - * their reply silently post on origin#N — a different issue entirely. That - * is the same silent-source-switch class of wrongness #1186 / the parent - * design doc guard against. List and create paths honor preference; - * mutations stay on the heuristic `getIssueOwnerRepo`. - */ -export async function addIssueComment( - repoPath: string, - issueNumber: number, - body: string, - connectionId?: string | null, - ownerRepoOverride?: OwnerRepo | null, - localGitOptions: LocalGitExecOptions = {} -): Promise { - const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution( - repoPath, - ownerRepoOverride ?? - (() => getIssueGitHubApiRepository(repoPath, connectionId, localGitOptions)), - connectionId, - localGitOptions - ) - if (!ownerRepo) { - return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' } - } - await acquire() - try { - const { stdout } = await ghExecFileAsync( - [ - 'api', - '-X', - 'POST', - `repos/${ownerRepo.owner}/${ownerRepo.repo}/issues/${issueNumber}/comments`, - '--raw-field', - `body=${body}` - ], - ghOptions - ) - const data = JSON.parse(stdout) as { - id?: number - node_id?: string | null - user: { login: string; avatar_url: string; type?: string } | null - body?: string - created_at?: string - html_url?: string - } - if (typeof data.id !== 'number' || !Number.isSafeInteger(data.id) || data.id < 1) { - return { ok: false, error: 'Unexpected response from GitHub' } - } - const comment: PRComment = { - id: data.id, - reactionSubjectId: data.node_id?.trim() || undefined, - author: data.user?.login ?? 'You', - authorAvatarUrl: data.user?.avatar_url ?? '', - body: data.body ?? body, - createdAt: data.created_at ?? new Date().toISOString(), - url: data.html_url ?? '', - isBot: data.user?.type === 'Bot' - } - return { ok: true, comment } - } catch (err) { - const stderr = err instanceof Error ? err.message : String(err) - return { ok: false, error: classifyGhError(stderr).message } - } finally { - release() - } -} - -export async function listLabels( - repoPath: string, - preference?: IssueSourcePreference, - connectionId?: string | null, - localGitOptions: LocalGitExecOptions = {} -): Promise { - const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution( - repoPath, - async () => - ( - await resolveIssueGitHubApiRepositorySource( - repoPath, - preference, - connectionId, - localGitOptions - ) - ).source, - connectionId, - localGitOptions - ) - if (!ownerRepo) { - return [] - } - await acquire() - try { - const { stdout } = await ghExecFileAsync( - [ - 'api', - '--paginate', - `repos/${ownerRepo.owner}/${ownerRepo.repo}/labels`, - '--jq', - '.[].name' - ], - ghOptions - ) - return stdout - .trim() - .split('\n') - .filter((l) => l.length > 0) - } catch { - return [] - } finally { - release() - } -} - -export async function listAssignableUsers( - repoPath: string, - preference?: IssueSourcePreference, - connectionId?: string | null, - localGitOptions: LocalGitExecOptions = {} -): Promise { - const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution( - repoPath, - async () => - ( - await resolveIssueGitHubApiRepositorySource( - repoPath, - preference, - connectionId, - localGitOptions - ) - ).source, - connectionId, - localGitOptions - ) - if (!ownerRepo) { - return [] - } - await acquire() - try { - // Why: paginate through all assignable users — GraphQL's assignableUsers - // maxes out at 100 per page and large orgs/repos silently lose assignees - // beyond the first page. REST /assignees with --paginate walks every page; - // --jq collapses per-page arrays into NDJSON so we don't have to stitch - // JSON arrays that gh concatenates back-to-back. - const { stdout } = await ghExecFileAsync( - [ - 'api', - '--paginate', - `repos/${ownerRepo.owner}/${ownerRepo.repo}/assignees?per_page=100`, - '--jq', - '.[] | {login, avatar_url}' - ], - ghOptions - ) - type RESTAssignee = { login?: string; avatar_url?: string | null } - const users: GitHubAssignableUser[] = [] - for (const line of stdout.split('\n')) { - const trimmed = line.trim() - if (!trimmed) { - continue - } - try { - const user = JSON.parse(trimmed) as RESTAssignee - if (user.login) { - users.push({ - login: user.login, - name: null, - avatarUrl: user.avatar_url ?? '' - }) - } - } catch { - // Skip malformed NDJSON lines defensively. - } - } - return users - } catch { - return [] - } finally { - release() - } -} diff --git a/src/main/github/project-view.ts b/src/main/github/project-view.ts index cd763b2542e..f9320a3b10e 100644 --- a/src/main/github/project-view.ts +++ b/src/main/github/project-view.ts @@ -22,8 +22,6 @@ import { type GhGraphqlErrorShape } from './project-view/project-error-classification' import type { - GetProjectViewTableArgs, - GetProjectViewTableResult, GitHubProjectField, GitHubProjectFieldValue, GitHubProjectIteration, @@ -37,16 +35,22 @@ import type { GitHubProjectTable, GitHubProjectUser, GitHubProjectView, - GitHubProjectViewError, GitHubProjectViewLayout, - GitHubProjectViewSummary, - ListAccessibleProjectsArgs, - ListAccessibleProjectsResult, - ListProjectViewsArgs, - ListProjectViewsResult, - ResolveProjectRefArgs, - ResolveProjectRefResult + GitHubProjectViewSummary } from '../../shared/github/project-types' +import type { + GetProjectViewTableResult, + GitHubProjectViewError, + ListAccessibleProjectsResult, + ListProjectViewsResult, + ResolveProjectRefResult +} from '../../shared/github/project-result-types' +import type { + GetProjectViewTableArgs, + ListAccessibleProjectsArgs, + ListProjectViewsArgs, + ResolveProjectRefArgs +} from '../../shared/github/project-request-types' import { GITHUB_PROJECT_REF_INPUT_TOO_LARGE_ERROR, isGitHubProjectRefInputTooLarge diff --git a/src/main/github/project-view/internals.ts b/src/main/github/project-view/internals.ts index 06f0f7d69cf..1c828909669 100644 --- a/src/main/github/project-view/internals.ts +++ b/src/main/github/project-view/internals.ts @@ -9,7 +9,7 @@ import { noteRepositoryRateLimitSpend, type RateLimitBucketKind } from '../rate-limit' -import type { GitHubProjectViewError } from '../../../shared/github/project-types' +import type { GitHubProjectViewError } from '../../../shared/github/project-result-types' import { githubProjectHost } from '../../../shared/github/project-identity' import { isDefaultGitHubHost } from '../../../shared/github/repository-identity-key' import { isGitHubHostAuthenticatedForGlobalCli } from '../github-enterprise-repository' diff --git a/src/main/github/project-view/mutations.ts b/src/main/github/project-view/mutations.ts index efe27d452e2..8e06899b512 100644 --- a/src/main/github/project-view/mutations.ts +++ b/src/main/github/project-view/mutations.ts @@ -21,27 +21,29 @@ import { githubProjectHost } from '../../../shared/github/project-identity' import type { PRComment } from '../../../shared/github/comment-types' import type { GitHubAssignableUser } from '../../../shared/github/pull-request-types' import type { GitHubWorkItemDetails } from '../../../shared/github/work-item-types' +import type { GitHubProjectFieldMutationValue } from '../../../shared/github/project-types' +import type { + GitHubProjectCommentMutationResult, + GitHubProjectMutationResult, + ListAssignableUsersBySlugResult, + ListIssueTypesBySlugResult, + ListLabelsBySlugResult, + ProjectWorkItemDetailsBySlugResult +} from '../../../shared/github/project-result-types' import type { AddIssueCommentBySlugArgs, ClearProjectItemFieldArgs, DeleteIssueCommentBySlugArgs, - GitHubProjectCommentMutationResult, - GitHubProjectFieldMutationValue, - GitHubProjectMutationResult, ListAssignableUsersBySlugArgs, - ListAssignableUsersBySlugResult, ListIssueTypesBySlugArgs, - ListIssueTypesBySlugResult, ListLabelsBySlugArgs, - ListLabelsBySlugResult, ProjectWorkItemDetailsBySlugArgs, - ProjectWorkItemDetailsBySlugResult, UpdateIssueBySlugArgs, UpdateIssueCommentBySlugArgs, UpdateIssueTypeBySlugArgs, UpdatePullRequestBySlugArgs, UpdateProjectItemFieldArgs -} from '../../../shared/github/project-types' +} from '../../../shared/github/project-request-types' function githubHostExecOptions(args: { host?: string }): { host: string } { return { host: githubProjectHost(args.host) } diff --git a/src/main/github/project-view/project-error-classification.ts b/src/main/github/project-view/project-error-classification.ts index 5db90f4305d..6b3d9a3fe09 100644 --- a/src/main/github/project-view/project-error-classification.ts +++ b/src/main/github/project-view/project-error-classification.ts @@ -1,7 +1,7 @@ // Why: turns raw gh stderr/stdout into the typed GitHubProjectViewError the // renderer can act on (auth vs scope vs rate limit vs drift), shared by the // project-view read and mutation paths. -import type { GitHubProjectViewError } from '../../../shared/github/project-types' +import type { GitHubProjectViewError } from '../../../shared/github/project-result-types' import { githubProjectHost } from '../../../shared/github/project-identity' export type GhGraphqlErrorShape = { diff --git a/src/main/gitlab/client.ts b/src/main/gitlab/client.ts index 9e7d03c01b2..50c541c0213 100644 --- a/src/main/gitlab/client.ts +++ b/src/main/gitlab/client.ts @@ -1370,15 +1370,9 @@ export async function updateMR( /** Re-export so callers don't need to know the gl-utils module split. */ export { _resetProjectRefCache } from './gl-utils' -export { - addIssueComment, - createIssue, - getIssue, - listAssignableUsers, - listIssues, - listLabels, - updateIssue -} from './issues' +export { addIssueComment, createIssue, getIssue, listIssues } from './issues' +export { updateIssue } from './issue-update' +export { listAssignableUsers, listLabels } from './project-label-and-member-lookup' // Re-exported so paste-URL call sites don't import getProjectRefForRemote from gl-utils directly. export { getProjectRefForRemote } diff --git a/src/main/gitlab/gitlab-assignable-user-mapping.ts b/src/main/gitlab/gitlab-assignable-user-mapping.ts new file mode 100644 index 00000000000..57936ecb9a9 --- /dev/null +++ b/src/main/gitlab/gitlab-assignable-user-mapping.ts @@ -0,0 +1,22 @@ +import type { GitLabAssignableUser } from '../../shared/gitlab-types' + +export type GitLabRawUser = { + id?: number + username?: string | null + name?: string | null + avatar_url?: string | null + state?: string | null +} + +export function mapGitLabUser(raw: GitLabRawUser | null | undefined): GitLabAssignableUser | null { + if (!raw?.username) { + return null + } + return { + ...(typeof raw.id === 'number' ? { id: raw.id } : {}), + username: raw.username, + name: raw.name ?? null, + avatarUrl: raw.avatar_url ?? '', + ...(raw.state !== undefined ? { state: raw.state } : {}) + } +} diff --git a/src/main/gitlab/issue-update.ts b/src/main/gitlab/issue-update.ts new file mode 100644 index 00000000000..369e889ff09 --- /dev/null +++ b/src/main/gitlab/issue-update.ts @@ -0,0 +1,155 @@ +import type { GitLabIssueUpdate } from '../../shared/gitlab-types' +import type { IssueSourcePreference } from '../../shared/repo-types' +import { + acquire, + classifyGlabError, + getGlabKnownHosts, + glabExecFileAsync, + glabHostnameArgs, + glabRepoExecOptions, + release, + resolveIssueSource, + type LocalGitExecOptions, + type ProjectRef +} from './gl-utils' +import { encodedProject } from './project-path-encoding' + +/** + * Update an existing GitLab issue. + * + * Why: callers that list through a per-repo issue source preference must + * mutate the same GitLab project, or identical IIDs on origin/upstream can + * silently edit the wrong issue. + */ +export async function updateIssue( + repoPath: string, + issueNumber: number, + updates: GitLabIssueUpdate, + preference?: IssueSourcePreference, + connectionId?: string | null, + projectRefOverride?: ProjectRef | null, + localGitOptions: LocalGitExecOptions = {} +): Promise<{ ok: true } | { ok: false; error: string }> { + const projectRef = + projectRefOverride ?? + ( + await resolveIssueSource( + repoPath, + preference, + await getGlabKnownHosts(connectionId, localGitOptions), + connectionId, + localGitOptions + ) + ).source + if (!projectRef) { + return { + ok: false, + error: 'Could not resolve GitLab project for this repository' + } + } + + const repoFlag = projectRef.path + const errors: string[] = [] + + // State change requires a separate command (parallel to github's split). + if (updates.state) { + await acquire() + try { + const cmd = updates.state === 'closed' ? 'close' : 'reopen' + await glabExecFileAsync( + [ + 'issue', + cmd, + String(issueNumber), + '-R', + repoFlag, + ...glabHostnameArgs(projectRef, connectionId) + ], + glabRepoExecOptions(repoPath, connectionId, localGitOptions) + ) + } catch (err) { + const stderr = err instanceof Error ? err.message : String(err) + // Treat "already closed/reopened" as a no-op (matches gh path). + if (!stderr.toLowerCase().includes('already')) { + errors.push(classifyGlabError(stderr).message) + } + } finally { + release() + } + } + + if (updates.body !== undefined) { + await acquire() + try { + await glabExecFileAsync( + [ + 'api', + ...glabHostnameArgs(projectRef, connectionId), + '-X', + 'PUT', + `projects/${encodedProject(repoFlag)}/issues/${issueNumber}`, + '-f', + `description=${updates.body}` + ], + glabRepoExecOptions(repoPath, connectionId, localGitOptions) + ) + } catch (err) { + const stderr = err instanceof Error ? err.message : String(err) + errors.push(classifyGlabError(stderr).message) + } finally { + release() + } + } + + // Field edits via `glab issue update`. + const editArgs: string[] = [ + 'issue', + 'update', + String(issueNumber), + '-R', + repoFlag, + ...glabHostnameArgs(projectRef, connectionId) + ] + let hasEditArgs = false + + if (updates.title) { + editArgs.push('--title', updates.title) + hasEditArgs = true + } + for (const label of updates.addLabels ?? []) { + editArgs.push('--label', label) + hasEditArgs = true + } + for (const label of updates.removeLabels ?? []) { + editArgs.push('--unlabel', label) + hasEditArgs = true + } + for (const assignee of updates.addAssignees ?? []) { + editArgs.push('--assignee', assignee) + hasEditArgs = true + } + for (const assignee of updates.removeAssignees ?? []) { + editArgs.push('--unassignee', assignee) + hasEditArgs = true + } + + if (hasEditArgs) { + await acquire() + try { + await glabExecFileAsync( + editArgs, + glabRepoExecOptions(repoPath, connectionId, localGitOptions) + ) + } catch (err) { + const stderr = err instanceof Error ? err.message : String(err) + errors.push(classifyGlabError(stderr).message) + } finally { + release() + } + } + + if (errors.length > 0) { + return { ok: false, error: errors.join('; ') } + } + return { ok: true } +} diff --git a/src/main/gitlab/issues.test.ts b/src/main/gitlab/issues.test.ts index 221041356ec..c86ee077fdf 100644 --- a/src/main/gitlab/issues.test.ts +++ b/src/main/gitlab/issues.test.ts @@ -30,15 +30,9 @@ vi.mock('./gl-utils', async () => { } }) -import { - addIssueComment, - createIssue, - getIssue, - listAssignableUsers, - listIssues, - listLabels, - updateIssue -} from './issues' +import { addIssueComment, createIssue, getIssue, listIssues } from './issues' +import { updateIssue } from './issue-update' +import { listAssignableUsers, listLabels } from './project-label-and-member-lookup' describe('gitlab issue operations', () => { beforeEach(() => { diff --git a/src/main/gitlab/issues.ts b/src/main/gitlab/issues.ts index 9bf57d6beb9..bfe19ef411b 100644 --- a/src/main/gitlab/issues.ts +++ b/src/main/gitlab/issues.ts @@ -1,19 +1,10 @@ -/* eslint-disable max-lines -- Why: parallel to src/main/github/issues.ts — -co-locating issue list/create/update/comment operations keeps the shared -acquire/release + error-classification pattern obvious. Each function is -short; the file is long because the surface is broad. */ import type { ClassifiedError } from '../../shared/classified-error' -import type { - GitLabAssignableUser, - GitLabCommentResult, - GitLabIssueInfo, - GitLabIssueUpdate, - MRComment -} from '../../shared/gitlab-types' +import type { GitLabCommentResult, GitLabIssueInfo, MRComment } from '../../shared/gitlab-types' import type { IssueSourcePreference } from '../../shared/repo-types' import { mapGitLabIssueInfo } from './mappers' // prettier-ignore import { glabExecFileAsync, acquire, release, getIssueProjectRef, resolveIssueSource, classifyGlabError, classifyListFetchError, getGlabKnownHosts, glabRepoExecOptions, glabHostnameArgs, parseGlabJsonList, type LocalGitExecOptions, type ProjectRef } from './gl-utils' +import { encodedProject } from './project-path-encoding' // Why: parallel to GitHub's IssueListResult — distinguishes a successful- // empty listing from a failed fetch. @@ -22,13 +13,6 @@ export type IssueListResult = { error?: ClassifiedError } -// Why: GitLab REST API addresses projects by URL-encoded path. Centralize -// the encoding so a future call site can't forget it (the slash escapes -// are easy to miss). -function encodedProject(projectPath: string): string { - return encodeURIComponent(projectPath) -} - /** * Get a single issue by number. * @@ -209,146 +193,6 @@ export async function createIssue( } } -/** - * Update an existing GitLab issue. - * - * Why: callers that list through a per-repo issue source preference must - * mutate the same GitLab project, or identical IIDs on origin/upstream can - * silently edit the wrong issue. - */ -export async function updateIssue( - repoPath: string, - issueNumber: number, - updates: GitLabIssueUpdate, - preference?: IssueSourcePreference, - connectionId?: string | null, - projectRefOverride?: ProjectRef | null, - localGitOptions: LocalGitExecOptions = {} -): Promise<{ ok: true } | { ok: false; error: string }> { - const projectRef = - projectRefOverride ?? - ( - await resolveIssueSource( - repoPath, - preference, - await getGlabKnownHosts(connectionId, localGitOptions), - connectionId, - localGitOptions - ) - ).source - if (!projectRef) { - return { - ok: false, - error: 'Could not resolve GitLab project for this repository' - } - } - - const repoFlag = projectRef.path - const errors: string[] = [] - - // State change requires a separate command (parallel to github's split). - if (updates.state) { - await acquire() - try { - const cmd = updates.state === 'closed' ? 'close' : 'reopen' - await glabExecFileAsync( - [ - 'issue', - cmd, - String(issueNumber), - '-R', - repoFlag, - ...glabHostnameArgs(projectRef, connectionId) - ], - glabRepoExecOptions(repoPath, connectionId, localGitOptions) - ) - } catch (err) { - const stderr = err instanceof Error ? err.message : String(err) - // Treat "already closed/reopened" as a no-op (matches gh path). - if (!stderr.toLowerCase().includes('already')) { - errors.push(classifyGlabError(stderr).message) - } - } finally { - release() - } - } - - if (updates.body !== undefined) { - await acquire() - try { - await glabExecFileAsync( - [ - 'api', - ...glabHostnameArgs(projectRef, connectionId), - '-X', - 'PUT', - `projects/${encodedProject(repoFlag)}/issues/${issueNumber}`, - '-f', - `description=${updates.body}` - ], - glabRepoExecOptions(repoPath, connectionId, localGitOptions) - ) - } catch (err) { - const stderr = err instanceof Error ? err.message : String(err) - errors.push(classifyGlabError(stderr).message) - } finally { - release() - } - } - - // Field edits via `glab issue update`. - const editArgs: string[] = [ - 'issue', - 'update', - String(issueNumber), - '-R', - repoFlag, - ...glabHostnameArgs(projectRef, connectionId) - ] - let hasEditArgs = false - - if (updates.title) { - editArgs.push('--title', updates.title) - hasEditArgs = true - } - for (const label of updates.addLabels ?? []) { - editArgs.push('--label', label) - hasEditArgs = true - } - for (const label of updates.removeLabels ?? []) { - editArgs.push('--unlabel', label) - hasEditArgs = true - } - for (const assignee of updates.addAssignees ?? []) { - editArgs.push('--assignee', assignee) - hasEditArgs = true - } - for (const assignee of updates.removeAssignees ?? []) { - editArgs.push('--unassignee', assignee) - hasEditArgs = true - } - - if (hasEditArgs) { - await acquire() - try { - await glabExecFileAsync( - editArgs, - glabRepoExecOptions(repoPath, connectionId, localGitOptions) - ) - } catch (err) { - const stderr = err instanceof Error ? err.message : String(err) - errors.push(classifyGlabError(stderr).message) - } finally { - release() - } - } - - if (errors.length > 0) { - return { ok: false, error: errors.join('; ') } - } - return { ok: true } -} - /** * Add a comment (note) to an existing GitLab issue. Mirrors * github/addIssueComment. @@ -419,114 +263,3 @@ export async function addIssueComment( release() } } - -export async function listLabels( - repoPath: string, - preference?: IssueSourcePreference, - connectionId?: string | null, - localGitOptions: LocalGitExecOptions = {} -): Promise { - const knownHosts = await getGlabKnownHosts(connectionId, localGitOptions) - const { source: projectRef } = await resolveIssueSource( - repoPath, - preference, - knownHosts, - connectionId, - localGitOptions - ) - if (!projectRef) { - return [] - } - await acquire() - try { - const { stdout } = await glabExecFileAsync( - [ - 'api', - ...glabHostnameArgs(projectRef, connectionId), - '--paginate', - `projects/${encodedProject(projectRef.path)}/labels`, - '--jq', - '.[].name' - ], - glabRepoExecOptions(repoPath, connectionId, localGitOptions) - ) - return stdout - .trim() - .split('\n') - .filter((l) => l.length > 0) - } catch { - return [] - } finally { - release() - } -} - -export async function listAssignableUsers( - repoPath: string, - preference?: IssueSourcePreference, - connectionId?: string | null, - localGitOptions: LocalGitExecOptions = {} -): Promise { - const knownHosts = await getGlabKnownHosts(connectionId, localGitOptions) - const { source: projectRef } = await resolveIssueSource( - repoPath, - preference, - knownHosts, - connectionId, - localGitOptions - ) - if (!projectRef) { - return [] - } - await acquire() - try { - // Why: `members/all` returns project members including those inherited - // from parent groups — important for projects under a top-level group - // where assignable users typically come from the group, not the project. - // --paginate walks every page; --jq emits NDJSON. - const { stdout } = await glabExecFileAsync( - [ - 'api', - ...glabHostnameArgs(projectRef, connectionId), - '--paginate', - `projects/${encodedProject(projectRef.path)}/members/all?per_page=100`, - '--jq', - '.[] | {id, username, name, avatar_url, state}' - ], - glabRepoExecOptions(repoPath, connectionId, localGitOptions) - ) - type RESTMember = { - id?: number - username?: string - name?: string | null - avatar_url?: string | null - state?: string | null - } - const users: GitLabAssignableUser[] = [] - for (const line of stdout.split('\n')) { - const trimmed = line.trim() - if (!trimmed) { - continue - } - try { - const user = JSON.parse(trimmed) as RESTMember - if (user.username) { - users.push({ - ...(typeof user.id === 'number' ? { id: user.id } : {}), - username: user.username, - name: user.name ?? null, - avatarUrl: user.avatar_url ?? '', - ...(user.state !== undefined ? { state: user.state } : {}) - }) - } - } catch { - // Skip malformed NDJSON lines defensively. - } - } - return users - } catch { - return [] - } finally { - release() - } -} diff --git a/src/main/gitlab/mr-discussion-notes.ts b/src/main/gitlab/mr-discussion-notes.ts new file mode 100644 index 00000000000..fa3aac2dc6a --- /dev/null +++ b/src/main/gitlab/mr-discussion-notes.ts @@ -0,0 +1,83 @@ +import type { MRComment } from '../../shared/gitlab-types' +import { encodedProject } from './project-path-encoding' +import { + glabHostnameArgs, + glabRepoExecOptions, + glabExecFileAsync, + type LocalGitExecOptions, + type ProjectRef +} from './gl-utils' + +// ── Discussion → MRComment flattening ────────────────────────────── +// GitLab returns discussions with nested notes; the dialog renders a +// flat conversation. We drop system notes ("X assigned the MR", auto- +// generated changelog entries) since they aren't user-authored content. + +type GitLabRawNote = { + id?: number + body?: string + author?: { username?: string | null; avatar_url?: string | null; state?: string } | null + created_at?: string + system?: boolean + resolvable?: boolean + resolved?: boolean + position?: { new_path?: string; new_line?: number; old_line?: number } | null +} + +export type GitLabRawDiscussion = { + id?: string + individual_note?: boolean + notes?: GitLabRawNote[] +} + +export function flattenDiscussions(discussions: GitLabRawDiscussion[]): MRComment[] { + const out: MRComment[] = [] + for (const discussion of discussions) { + const notes = discussion.notes ?? [] + for (const note of notes) { + if (note.system === true) { + // Why: skip GitLab's auto-generated activity entries — they + // would dominate a busy MR's conversation tab if rendered. + continue + } + out.push({ + id: note.id ?? 0, + author: note.author?.username ?? 'unknown', + authorAvatarUrl: note.author?.avatar_url ?? '', + body: note.body ?? '', + createdAt: note.created_at ?? '', + url: '', + isBot: note.author?.state === 'bot', + ...(discussion.id ? { threadId: discussion.id } : {}), + ...(note.resolvable === true ? { isResolved: note.resolved === true } : {}), + ...(note.position?.new_path ? { path: note.position.new_path } : {}), + ...(typeof note.position?.new_line === 'number' ? { line: note.position.new_line } : {}) + }) + } + } + // Why: oldest-first matches gitlab.com's conversation rendering and + // makes "what's new" intuitive when polling for updates later. + return out.sort((a, b) => (a.createdAt ?? '').localeCompare(b.createdAt ?? '')) +} + +export async function fetchDiscussions( + repoPath: string, + projectRef: ProjectRef, + type: 'issue' | 'mr', + iid: number, + connectionId?: string | null, + localGitOptions: LocalGitExecOptions = {} +): Promise { + const resource = type === 'mr' ? 'merge_requests' : 'issues' + const { stdout } = await glabExecFileAsync( + [ + 'api', + ...glabHostnameArgs(projectRef, connectionId), + // Why: detail drawers need a bounded recent conversation snapshot. + // Walking every historic discussion can retain and render huge note sets. + `projects/${encodedProject(projectRef.path)}/${resource}/${iid}/discussions?per_page=100` + ], + glabRepoExecOptions(repoPath, connectionId, localGitOptions) + ) + return JSON.parse(stdout) as GitLabRawDiscussion[] +} diff --git a/src/main/gitlab/mr-file-diffs.ts b/src/main/gitlab/mr-file-diffs.ts new file mode 100644 index 00000000000..0e16567a294 --- /dev/null +++ b/src/main/gitlab/mr-file-diffs.ts @@ -0,0 +1,94 @@ +import type { GitLabMRFile } from '../../shared/gitlab-types' +import { encodedProject } from './project-path-encoding' +import { + glabHostnameArgs, + glabRepoExecOptions, + glabExecFileAsync, + type LocalGitExecOptions, + type ProjectRef +} from './gl-utils' + +/** + * Counts the added/removed lines in a single GitLab MR file's unified diff, + * feeding the +N/-N shown in the MR file list. `---`/`+++` are file headers + * only before the first `@@` hunk; every `+`/`-` line inside a hunk is content. + * Requires hunk headers: a diff with no `@@` counts zero, so do not reuse this + * for header-less agent-tool diffs (see `diffFromText` in shared/native-chat-diff). + * + * @internal - exposed for tests only. + */ +export function countDiffLines(diff: string): { additions: number; deletions: number } { + let additions = 0 + let deletions = 0 + // Why: `---`/`+++` are file headers only before the first hunk. A removed line + // whose original text began with `--` (SQL/Lua/Haskell `-- comment`) becomes a + // diff line `---`, colliding with the `--- a/file` header — so it must + // be counted once inside a hunk, not skipped. + let inHunk = false + for (const line of diff.split('\n')) { + if (line.startsWith('@@')) { + inHunk = true + continue + } + if (!inHunk) { + continue + } + if (line.startsWith('+')) { + additions += 1 + } else if (line.startsWith('-')) { + deletions += 1 + } + } + return { additions, deletions } +} + +function mapMRFile(raw: { + new_path?: string + old_path?: string + diff?: string + new_file?: boolean + deleted_file?: boolean + renamed_file?: boolean + binary?: boolean + too_large?: boolean +}): GitLabMRFile { + const diff = raw.diff ?? '' + const counts = countDiffLines(diff) + const status = raw.new_file + ? 'added' + : raw.deleted_file + ? 'removed' + : raw.renamed_file + ? 'renamed' + : 'modified' + return { + path: raw.new_path ?? raw.old_path ?? '', + ...(raw.old_path && raw.old_path !== raw.new_path ? { oldPath: raw.old_path } : {}), + status, + additions: counts.additions, + deletions: counts.deletions, + isBinary: Boolean(raw.binary || raw.too_large || !diff), + ...(diff ? { diff } : {}) + } +} + +export async function fetchMRFiles( + repoPath: string, + projectRef: ProjectRef, + iid: number, + connectionId?: string | null, + localGitOptions: LocalGitExecOptions = {} +): Promise { + const { stdout } = await glabExecFileAsync( + [ + 'api', + ...glabHostnameArgs(projectRef, connectionId), + // Why: GitLab deprecated the all-in-one `changes` endpoint in favor of + // the paginated diffs endpoint; cap the file snapshot at one visible page. + `projects/${encodedProject(projectRef.path)}/merge_requests/${iid}/diffs?per_page=100` + ], + glabRepoExecOptions(repoPath, connectionId, localGitOptions) + ) + const data = JSON.parse(stdout) as Parameters[0][] + return data.map(mapMRFile).filter((file) => file.path) +} diff --git a/src/main/gitlab/mr-reviewers-and-approvals.ts b/src/main/gitlab/mr-reviewers-and-approvals.ts new file mode 100644 index 00000000000..cdbe9870e37 --- /dev/null +++ b/src/main/gitlab/mr-reviewers-and-approvals.ts @@ -0,0 +1,94 @@ +import type { GitLabAssignableUser, GitLabMRApprovalState } from '../../shared/gitlab-types' +import { mapGitLabUser, type GitLabRawUser } from './gitlab-assignable-user-mapping' +import { encodedProject } from './project-path-encoding' +import { + glabHostnameArgs, + glabRepoExecOptions, + glabExecFileAsync, + type LocalGitExecOptions, + type ProjectRef +} from './gl-utils' + +export async function fetchMRReviewers( + repoPath: string, + projectRef: ProjectRef, + iid: number, + connectionId?: string | null, + localGitOptions: LocalGitExecOptions = {} +): Promise { + const { stdout } = await glabExecFileAsync( + [ + 'api', + ...glabHostnameArgs(projectRef, connectionId), + `projects/${encodedProject(projectRef.path)}/merge_requests/${iid}/reviewers` + ], + glabRepoExecOptions(repoPath, connectionId, localGitOptions) + ) + const data = JSON.parse(stdout) as { user?: GitLabRawUser | null }[] + return data + .map((entry) => mapGitLabUser(entry.user)) + .filter((u): u is GitLabAssignableUser => !!u) +} + +export async function fetchMRApprovalState( + repoPath: string, + projectRef: ProjectRef, + iid: number, + connectionId?: string | null, + localGitOptions: LocalGitExecOptions = {} +): Promise { + const [approvalsRes, stateRes] = await Promise.allSettled([ + glabExecFileAsync( + [ + 'api', + ...glabHostnameArgs(projectRef, connectionId), + `projects/${encodedProject(projectRef.path)}/merge_requests/${iid}/approvals` + ], + glabRepoExecOptions(repoPath, connectionId, localGitOptions) + ), + glabExecFileAsync( + [ + 'api', + ...glabHostnameArgs(projectRef, connectionId), + `projects/${encodedProject(projectRef.path)}/merge_requests/${iid}/approval_state` + ], + glabRepoExecOptions(repoPath, connectionId, localGitOptions) + ) + ]) + if (approvalsRes.status === 'rejected' && stateRes.status === 'rejected') { + return undefined + } + const approvals = + approvalsRes.status === 'fulfilled' + ? (JSON.parse(approvalsRes.value.stdout) as { + approvals_required?: number | null + approvals_left?: number | null + approved_by?: { user?: GitLabRawUser | null }[] + }) + : null + const state = + stateRes.status === 'fulfilled' + ? (JSON.parse(stateRes.value.stdout) as { + rules?: { + id?: number + name?: string + approvals_required?: number + approved?: boolean + }[] + }) + : null + return { + approvalsRequired: + typeof approvals?.approvals_required === 'number' ? approvals.approvals_required : null, + approvalsLeft: typeof approvals?.approvals_left === 'number' ? approvals.approvals_left : null, + approvedBy: (approvals?.approved_by ?? []) + .map((entry) => mapGitLabUser(entry.user)) + .filter((u): u is GitLabAssignableUser => !!u), + rules: (state?.rules ?? []).map((rule) => ({ + id: rule.id ?? 0, + name: rule.name ?? 'Approval rule', + approvalsRequired: rule.approvals_required ?? 0, + approved: Boolean(rule.approved) + })) + } +} diff --git a/src/main/gitlab/pipeline-job-graph.ts b/src/main/gitlab/pipeline-job-graph.ts new file mode 100644 index 00000000000..46de918f49a --- /dev/null +++ b/src/main/gitlab/pipeline-job-graph.ts @@ -0,0 +1,241 @@ +import type { GitLabPipelineJob } from '../../shared/gitlab-types' +import { encodedProject } from './project-path-encoding' +import { + glabHostnameArgs, + glabRepoExecOptions, + glabExecFileAsync, + type LocalGitExecOptions, + type ProjectRef +} from './gl-utils' + +// ── Pipeline jobs ────────────────────────────────────────────────── +// Why: GitLab's `/pipelines/:id/jobs` only returns jobs owned by that pipeline. +// Trigger/include bridges live under `/bridges` and their real CI jobs live on +// the child pipeline — Orca used to show only the parent (often just SAST), so +// Checks looked empty next to gitlab.com's full graph. + +const PIPELINE_JOB_PAGE_SIZE = 100 +/** Cap expanded child-pipeline fan-out so one MR details load stays bounded. */ +const MAX_CHILD_PIPELINES_TO_EXPAND = 20 +// Why: every fetch spawns a `glab` binary (a remote exec over SSH), and this runs on +// the Checks poll timer. Match gl-utils' MAX_CONCURRENT so a bridge-heavy MR trickles +// its children instead of bursting 20 processes at once. +const MAX_CONCURRENT_CHILD_FETCHES = 4 + +type GitLabRawJob = { + id?: number + name?: string + stage?: string + status?: string + web_url?: string + duration?: number | null +} + +type GitLabRawBridge = { + id?: number + name?: string + stage?: string + status?: string + web_url?: string + duration?: number | null + downstream_pipeline?: { + id?: number + project_id?: number + status?: string + web_url?: string + } | null +} + +function mapPipelineJob(raw: GitLabRawJob, pipelineId: number): GitLabPipelineJob { + return { + id: raw.id ?? 0, + pipelineId, + name: raw.name ?? '', + stage: raw.stage ?? '', + status: raw.status ?? '', + webUrl: raw.web_url ?? '', + duration: typeof raw.duration === 'number' ? raw.duration : null + } +} + +function mapBridgeAsJob(raw: GitLabRawBridge, pipelineId: number): GitLabPipelineJob { + const childStatus = raw.downstream_pipeline?.status + return { + // Why: bridges are not real jobs — omit a positive id so Checks won't try + // job-trace/retry APIs on them. Rows still render via name + webUrl. + id: 0, + pipelineId, + name: raw.name ?? 'bridge', + stage: raw.stage ?? '', + // Prefer the child pipeline's rollup when present — the bridge job itself + // often stays `success` while the downstream graph is still running/failed. + status: childStatus || raw.status || '', + webUrl: raw.downstream_pipeline?.web_url ?? raw.web_url ?? '', + duration: typeof raw.duration === 'number' ? raw.duration : null + } +} + +async function fetchPipelineJobPage( + repoPath: string, + projectRef: ProjectRef, + pipelineId: number, + connectionId: string | null | undefined, + localGitOptions: LocalGitExecOptions +): Promise { + const { stdout } = await glabExecFileAsync( + [ + 'api', + ...glabHostnameArgs(projectRef, connectionId), + `projects/${encodedProject(projectRef.path)}/pipelines/${pipelineId}/jobs?per_page=${PIPELINE_JOB_PAGE_SIZE}` + ], + glabRepoExecOptions(repoPath, connectionId, localGitOptions) + ) + const data = JSON.parse(stdout) as GitLabRawJob[] + if (!Array.isArray(data)) { + return [] + } + return data.map((job) => mapPipelineJob(job, pipelineId)) +} + +async function fetchPipelineBridges( + repoPath: string, + projectRef: ProjectRef, + pipelineId: number, + connectionId: string | null | undefined, + localGitOptions: LocalGitExecOptions +): Promise { + const { stdout } = await glabExecFileAsync( + [ + 'api', + ...glabHostnameArgs(projectRef, connectionId), + `projects/${encodedProject(projectRef.path)}/pipelines/${pipelineId}/bridges?per_page=${PIPELINE_JOB_PAGE_SIZE}` + ], + glabRepoExecOptions(repoPath, connectionId, localGitOptions) + ) + const data = JSON.parse(stdout) as GitLabRawBridge[] + return Array.isArray(data) ? data : [] +} + +function childPipelineTarget( + bridge: GitLabRawBridge, + parentProjectRef: ProjectRef +): { projectRef: ProjectRef; pipelineId: number } | null { + const childId = bridge.downstream_pipeline?.id + if (typeof childId !== 'number') { + return null + } + // Why: prefer path from web_url so same- and cross-project children both work + // without a project-id lookup. If the URL is missing/unparseable, fall back to + // the parent project (same-project triggers); wrong-project calls fail soft. + const webUrl = bridge.downstream_pipeline?.web_url + if (webUrl) { + try { + const url = new URL(webUrl) + // web_url shape: https://host/group/project/-/pipelines/123 + const marker = url.pathname.indexOf('/-/pipelines/') + if (marker > 0) { + const path = url.pathname.slice(1, marker).replace(/\/$/, '') + if (path) { + return { + projectRef: { host: url.host || parentProjectRef.host, path }, + pipelineId: childId + } + } + } + } catch { + // fall through to parent project + } + } + return { projectRef: parentProjectRef, pipelineId: childId } +} + +/** Results stay in input order; a shared cursor keeps fast workers from idling behind a slow batch. */ +async function mapWithConcurrencyLimit( + items: T[], + limit: number, + run: (item: T) => Promise +): Promise { + const out = Array.from({ length: items.length }) as R[] + let cursor = 0 + await Promise.all( + Array.from({ length: Math.min(limit, items.length) }, async () => { + while (cursor < items.length) { + const index = cursor + cursor += 1 + out[index] = await run(items[index]) + } + }) + ) + return out +} + +export async function fetchPipelineJobs( + repoPath: string, + projectRef: ProjectRef, + pipelineId: number, + connectionId?: string | null, + localGitOptions: LocalGitExecOptions = {} +): Promise { + const [parentJobs, bridges] = await Promise.all([ + fetchPipelineJobPage(repoPath, projectRef, pipelineId, connectionId, localGitOptions), + fetchPipelineBridges(repoPath, projectRef, pipelineId, connectionId, localGitOptions).catch( + () => [] as GitLabRawBridge[] + ) + ]) + + const bridgeRows = bridges.map((bridge) => mapBridgeAsJob(bridge, pipelineId)) + const childTargets: { projectRef: ProjectRef; pipelineId: number }[] = [] + const seenChildIds = new Set() + for (const bridge of bridges) { + const target = childPipelineTarget(bridge, projectRef) + if (!target || seenChildIds.has(target.pipelineId)) { + continue + } + seenChildIds.add(target.pipelineId) + childTargets.push(target) + if (childTargets.length >= MAX_CHILD_PIPELINES_TO_EXPAND) { + break + } + } + + const childJobBatches = await mapWithConcurrencyLimit( + childTargets, + MAX_CONCURRENT_CHILD_FETCHES, + (target) => + fetchPipelineJobPage( + repoPath, + target.projectRef, + target.pipelineId, + connectionId, + localGitOptions + ).catch(() => [] as GitLabPipelineJob[]) + ) + + // Parent jobs first, then each child's jobs. Bridge rollup rows last and only + // when no expanded child job already carries the same name (avoid duplicates). + const seenJobIds = new Set() + const seenNames = new Set() + const out: GitLabPipelineJob[] = [] + for (const job of [...parentJobs, ...childJobBatches.flat()]) { + if (job.id) { + if (seenJobIds.has(job.id)) { + continue + } + seenJobIds.add(job.id) + } + out.push(job) + if (job.name) { + seenNames.add(job.name) + } + } + for (const bridge of bridgeRows) { + if (bridge.name && seenNames.has(bridge.name)) { + continue + } + out.push(bridge) + if (bridge.name) { + seenNames.add(bridge.name) + } + } + return out +} diff --git a/src/main/gitlab/project-label-and-member-lookup.ts b/src/main/gitlab/project-label-and-member-lookup.ts new file mode 100644 index 00000000000..fcad19b9fdf --- /dev/null +++ b/src/main/gitlab/project-label-and-member-lookup.ts @@ -0,0 +1,124 @@ +import type { GitLabAssignableUser } from '../../shared/gitlab-types' +import type { IssueSourcePreference } from '../../shared/repo-types' +import { + acquire, + getGlabKnownHosts, + glabExecFileAsync, + glabHostnameArgs, + glabRepoExecOptions, + release, + resolveIssueSource, + type LocalGitExecOptions +} from './gl-utils' +import { encodedProject } from './project-path-encoding' + +export async function listLabels( + repoPath: string, + preference?: IssueSourcePreference, + connectionId?: string | null, + localGitOptions: LocalGitExecOptions = {} +): Promise { + const knownHosts = await getGlabKnownHosts(connectionId, localGitOptions) + const { source: projectRef } = await resolveIssueSource( + repoPath, + preference, + knownHosts, + connectionId, + localGitOptions + ) + if (!projectRef) { + return [] + } + await acquire() + try { + const { stdout } = await glabExecFileAsync( + [ + 'api', + ...glabHostnameArgs(projectRef, connectionId), + '--paginate', + `projects/${encodedProject(projectRef.path)}/labels`, + '--jq', + '.[].name' + ], + glabRepoExecOptions(repoPath, connectionId, localGitOptions) + ) + return stdout + .trim() + .split('\n') + .filter((l) => l.length > 0) + } catch { + return [] + } finally { + release() + } +} + +export async function listAssignableUsers( + repoPath: string, + preference?: IssueSourcePreference, + connectionId?: string | null, + localGitOptions: LocalGitExecOptions = {} +): Promise { + const knownHosts = await getGlabKnownHosts(connectionId, localGitOptions) + const { source: projectRef } = await resolveIssueSource( + repoPath, + preference, + knownHosts, + connectionId, + localGitOptions + ) + if (!projectRef) { + return [] + } + await acquire() + try { + // Why: `members/all` returns project members including those inherited + // from parent groups — important for projects under a top-level group + // where assignable users typically come from the group, not the project. + // --paginate walks every page; --jq emits NDJSON. + const { stdout } = await glabExecFileAsync( + [ + 'api', + ...glabHostnameArgs(projectRef, connectionId), + '--paginate', + `projects/${encodedProject(projectRef.path)}/members/all?per_page=100`, + '--jq', + '.[] | {id, username, name, avatar_url, state}' + ], + glabRepoExecOptions(repoPath, connectionId, localGitOptions) + ) + type RESTMember = { + id?: number + username?: string + name?: string | null + avatar_url?: string | null + state?: string | null + } + const users: GitLabAssignableUser[] = [] + for (const line of stdout.split('\n')) { + const trimmed = line.trim() + if (!trimmed) { + continue + } + try { + const user = JSON.parse(trimmed) as RESTMember + if (user.username) { + users.push({ + ...(typeof user.id === 'number' ? { id: user.id } : {}), + username: user.username, + name: user.name ?? null, + avatarUrl: user.avatar_url ?? '', + ...(user.state !== undefined ? { state: user.state } : {}) + }) + } + } catch { + // Skip malformed NDJSON lines defensively. + } + } + return users + } catch { + return [] + } finally { + release() + } +} diff --git a/src/main/gitlab/project-path-encoding.ts b/src/main/gitlab/project-path-encoding.ts new file mode 100644 index 00000000000..da6700273f6 --- /dev/null +++ b/src/main/gitlab/project-path-encoding.ts @@ -0,0 +1,6 @@ +// Why: GitLab REST API addresses projects by URL-encoded path. Centralize +// the encoding so a future call site can't forget it (the slash escapes +// are easy to miss). +export function encodedProject(projectPath: string): string { + return encodeURIComponent(projectPath) +} diff --git a/src/main/gitlab/work-item-details.test.ts b/src/main/gitlab/work-item-details.test.ts index b5bbef40e56..f1e2297e14b 100644 --- a/src/main/gitlab/work-item-details.test.ts +++ b/src/main/gitlab/work-item-details.test.ts @@ -32,7 +32,8 @@ vi.mock('./gl-utils', () => ({ glabRepoExecOptions: glabRepoExecOptionsMock })) -import { countDiffLines, getWorkItemDetails } from './work-item-details' +import { countDiffLines } from './mr-file-diffs' +import { getWorkItemDetails } from './work-item-details' describe('getWorkItemDetails', () => { beforeEach(() => { diff --git a/src/main/gitlab/work-item-details.ts b/src/main/gitlab/work-item-details.ts index 3155727eeaf..637baa0c78b 100644 --- a/src/main/gitlab/work-item-details.ts +++ b/src/main/gitlab/work-item-details.ts @@ -1,19 +1,20 @@ -/* eslint-disable max-lines -- Why: aggregated detail-fetch for GitLabItemDialog spans issues, MRs, comments, pipelines, reviewers, approvals, and changed files; splitting would obscure the shared fetch context. */ // Why: aggregated detail-fetch for GitLabItemDialog. Parallel of // src/main/github/work-item-details.ts but scoped to v1 surface — // description body, flattened discussion notes, MR pipeline jobs/reviewers. // Files / inline review-comment positioning are deferred. import type { GitLabAssignableUser, - GitLabMRApprovalState, - GitLabMRFile, - GitLabPipelineJob, GitLabWorkItem, - GitLabWorkItemDetails, - MRComment + GitLabWorkItemDetails } from '../../shared/gitlab-types' import type { IssueSourcePreference } from '../../shared/repo-types' import { mapIssueToWorkItem, mapMRToWorkItem } from './mappers' +import { mapGitLabUser, type GitLabRawUser } from './gitlab-assignable-user-mapping' +import { encodedProject } from './project-path-encoding' +import { fetchDiscussions, flattenDiscussions } from './mr-discussion-notes' +import { fetchMRFiles } from './mr-file-diffs' +import { fetchMRApprovalState, fetchMRReviewers } from './mr-reviewers-and-approvals' +import { fetchPipelineJobs } from './pipeline-job-graph' import { acquire, getGlabKnownHosts, @@ -26,422 +27,6 @@ import { type ProjectRef } from './gl-utils' -function encodedProject(projectPath: string): string { - return encodeURIComponent(projectPath) -} - -// ── Discussion → MRComment flattening ────────────────────────────── -// GitLab returns discussions with nested notes; the dialog renders a -// flat conversation. We drop system notes ("X assigned the MR", auto- -// generated changelog entries) since they aren't user-authored content. - -type GitLabRawNote = { - id?: number - body?: string - author?: { username?: string | null; avatar_url?: string | null; state?: string } | null - created_at?: string - system?: boolean - resolvable?: boolean - resolved?: boolean - position?: { new_path?: string; new_line?: number; old_line?: number } | null -} - -type GitLabRawDiscussion = { - id?: string - individual_note?: boolean - notes?: GitLabRawNote[] -} - -function flattenDiscussions(discussions: GitLabRawDiscussion[]): MRComment[] { - const out: MRComment[] = [] - for (const discussion of discussions) { - const notes = discussion.notes ?? [] - for (const note of notes) { - if (note.system === true) { - // Why: skip GitLab's auto-generated activity entries — they - // would dominate a busy MR's conversation tab if rendered. - continue - } - out.push({ - id: note.id ?? 0, - author: note.author?.username ?? 'unknown', - authorAvatarUrl: note.author?.avatar_url ?? '', - body: note.body ?? '', - createdAt: note.created_at ?? '', - url: '', - isBot: note.author?.state === 'bot', - ...(discussion.id ? { threadId: discussion.id } : {}), - ...(note.resolvable === true ? { isResolved: note.resolved === true } : {}), - ...(note.position?.new_path ? { path: note.position.new_path } : {}), - ...(typeof note.position?.new_line === 'number' ? { line: note.position.new_line } : {}) - }) - } - } - // Why: oldest-first matches gitlab.com's conversation rendering and - // makes "what's new" intuitive when polling for updates later. - return out.sort((a, b) => (a.createdAt ?? '').localeCompare(b.createdAt ?? '')) -} - -async function fetchDiscussions( - repoPath: string, - projectRef: ProjectRef, - type: 'issue' | 'mr', - iid: number, - connectionId?: string | null, - localGitOptions: LocalGitExecOptions = {} -): Promise { - const resource = type === 'mr' ? 'merge_requests' : 'issues' - const { stdout } = await glabExecFileAsync( - [ - 'api', - ...glabHostnameArgs(projectRef, connectionId), - // Why: detail drawers need a bounded recent conversation snapshot. - // Walking every historic discussion can retain and render huge note sets. - `projects/${encodedProject(projectRef.path)}/${resource}/${iid}/discussions?per_page=100` - ], - glabRepoExecOptions(repoPath, connectionId, localGitOptions) - ) - return JSON.parse(stdout) as GitLabRawDiscussion[] -} - -// ── Pipeline jobs ────────────────────────────────────────────────── -// Why: GitLab's `/pipelines/:id/jobs` only returns jobs owned by that pipeline. -// Trigger/include bridges live under `/bridges` and their real CI jobs live on -// the child pipeline — Orca used to show only the parent (often just SAST), so -// Checks looked empty next to gitlab.com's full graph. - -const PIPELINE_JOB_PAGE_SIZE = 100 -/** Cap expanded child-pipeline fan-out so one MR details load stays bounded. */ -const MAX_CHILD_PIPELINES_TO_EXPAND = 20 -// Why: every fetch spawns a `glab` binary (a remote exec over SSH), and this runs on -// the Checks poll timer. Match gl-utils' MAX_CONCURRENT so a bridge-heavy MR trickles -// its children instead of bursting 20 processes at once. -const MAX_CONCURRENT_CHILD_FETCHES = 4 - -type GitLabRawJob = { - id?: number - name?: string - stage?: string - status?: string - web_url?: string - duration?: number | null -} - -type GitLabRawBridge = { - id?: number - name?: string - stage?: string - status?: string - web_url?: string - duration?: number | null - downstream_pipeline?: { - id?: number - project_id?: number - status?: string - web_url?: string - } | null -} - -type GitLabRawUser = { - id?: number - username?: string | null - name?: string | null - avatar_url?: string | null - state?: string | null -} - -function mapGitLabUser(raw: GitLabRawUser | null | undefined): GitLabAssignableUser | null { - if (!raw?.username) { - return null - } - return { - ...(typeof raw.id === 'number' ? { id: raw.id } : {}), - username: raw.username, - name: raw.name ?? null, - avatarUrl: raw.avatar_url ?? '', - ...(raw.state !== undefined ? { state: raw.state } : {}) - } -} - -function mapPipelineJob(raw: GitLabRawJob, pipelineId: number): GitLabPipelineJob { - return { - id: raw.id ?? 0, - pipelineId, - name: raw.name ?? '', - stage: raw.stage ?? '', - status: raw.status ?? '', - webUrl: raw.web_url ?? '', - duration: typeof raw.duration === 'number' ? raw.duration : null - } -} - -function mapBridgeAsJob(raw: GitLabRawBridge, pipelineId: number): GitLabPipelineJob { - const childStatus = raw.downstream_pipeline?.status - return { - // Why: bridges are not real jobs — omit a positive id so Checks won't try - // job-trace/retry APIs on them. Rows still render via name + webUrl. - id: 0, - pipelineId, - name: raw.name ?? 'bridge', - stage: raw.stage ?? '', - // Prefer the child pipeline's rollup when present — the bridge job itself - // often stays `success` while the downstream graph is still running/failed. - status: childStatus || raw.status || '', - webUrl: raw.downstream_pipeline?.web_url ?? raw.web_url ?? '', - duration: typeof raw.duration === 'number' ? raw.duration : null - } -} - -async function fetchPipelineJobPage( - repoPath: string, - projectRef: ProjectRef, - pipelineId: number, - connectionId: string | null | undefined, - localGitOptions: LocalGitExecOptions -): Promise { - const { stdout } = await glabExecFileAsync( - [ - 'api', - ...glabHostnameArgs(projectRef, connectionId), - `projects/${encodedProject(projectRef.path)}/pipelines/${pipelineId}/jobs?per_page=${PIPELINE_JOB_PAGE_SIZE}` - ], - glabRepoExecOptions(repoPath, connectionId, localGitOptions) - ) - const data = JSON.parse(stdout) as GitLabRawJob[] - if (!Array.isArray(data)) { - return [] - } - return data.map((job) => mapPipelineJob(job, pipelineId)) -} - -async function fetchPipelineBridges( - repoPath: string, - projectRef: ProjectRef, - pipelineId: number, - connectionId: string | null | undefined, - localGitOptions: LocalGitExecOptions -): Promise { - const { stdout } = await glabExecFileAsync( - [ - 'api', - ...glabHostnameArgs(projectRef, connectionId), - `projects/${encodedProject(projectRef.path)}/pipelines/${pipelineId}/bridges?per_page=${PIPELINE_JOB_PAGE_SIZE}` - ], - glabRepoExecOptions(repoPath, connectionId, localGitOptions) - ) - const data = JSON.parse(stdout) as GitLabRawBridge[] - return Array.isArray(data) ? data : [] -} - -function childPipelineTarget( - bridge: GitLabRawBridge, - parentProjectRef: ProjectRef -): { projectRef: ProjectRef; pipelineId: number } | null { - const childId = bridge.downstream_pipeline?.id - if (typeof childId !== 'number') { - return null - } - // Why: prefer path from web_url so same- and cross-project children both work - // without a project-id lookup. If the URL is missing/unparseable, fall back to - // the parent project (same-project triggers); wrong-project calls fail soft. - const webUrl = bridge.downstream_pipeline?.web_url - if (webUrl) { - try { - const url = new URL(webUrl) - // web_url shape: https://host/group/project/-/pipelines/123 - const marker = url.pathname.indexOf('/-/pipelines/') - if (marker > 0) { - const path = url.pathname.slice(1, marker).replace(/\/$/, '') - if (path) { - return { - projectRef: { host: url.host || parentProjectRef.host, path }, - pipelineId: childId - } - } - } - } catch { - // fall through to parent project - } - } - return { projectRef: parentProjectRef, pipelineId: childId } -} - -/** Results stay in input order; a shared cursor keeps fast workers from idling behind a slow batch. */ -async function mapWithConcurrencyLimit( - items: T[], - limit: number, - run: (item: T) => Promise -): Promise { - const out = Array.from({ length: items.length }) as R[] - let cursor = 0 - await Promise.all( - Array.from({ length: Math.min(limit, items.length) }, async () => { - while (cursor < items.length) { - const index = cursor - cursor += 1 - out[index] = await run(items[index]) - } - }) - ) - return out -} - -async function fetchPipelineJobs( - repoPath: string, - projectRef: ProjectRef, - pipelineId: number, - connectionId?: string | null, - localGitOptions: LocalGitExecOptions = {} -): Promise { - const [parentJobs, bridges] = await Promise.all([ - fetchPipelineJobPage(repoPath, projectRef, pipelineId, connectionId, localGitOptions), - fetchPipelineBridges(repoPath, projectRef, pipelineId, connectionId, localGitOptions).catch( - () => [] as GitLabRawBridge[] - ) - ]) - - const bridgeRows = bridges.map((bridge) => mapBridgeAsJob(bridge, pipelineId)) - const childTargets: { projectRef: ProjectRef; pipelineId: number }[] = [] - const seenChildIds = new Set() - for (const bridge of bridges) { - const target = childPipelineTarget(bridge, projectRef) - if (!target || seenChildIds.has(target.pipelineId)) { - continue - } - seenChildIds.add(target.pipelineId) - childTargets.push(target) - if (childTargets.length >= MAX_CHILD_PIPELINES_TO_EXPAND) { - break - } - } - - const childJobBatches = await mapWithConcurrencyLimit( - childTargets, - MAX_CONCURRENT_CHILD_FETCHES, - (target) => - fetchPipelineJobPage( - repoPath, - target.projectRef, - target.pipelineId, - connectionId, - localGitOptions - ).catch(() => [] as GitLabPipelineJob[]) - ) - - // Parent jobs first, then each child's jobs. Bridge rollup rows last and only - // when no expanded child job already carries the same name (avoid duplicates). - const seenJobIds = new Set() - const seenNames = new Set() - const out: GitLabPipelineJob[] = [] - for (const job of [...parentJobs, ...childJobBatches.flat()]) { - if (job.id) { - if (seenJobIds.has(job.id)) { - continue - } - seenJobIds.add(job.id) - } - out.push(job) - if (job.name) { - seenNames.add(job.name) - } - } - for (const bridge of bridgeRows) { - if (bridge.name && seenNames.has(bridge.name)) { - continue - } - out.push(bridge) - if (bridge.name) { - seenNames.add(bridge.name) - } - } - return out -} - -/** - * Counts the added/removed lines in a single GitLab MR file's unified diff, - * feeding the +N/-N shown in the MR file list. `---`/`+++` are file headers - * only before the first `@@` hunk; every `+`/`-` line inside a hunk is content. - * Requires hunk headers: a diff with no `@@` counts zero, so do not reuse this - * for header-less agent-tool diffs (see `diffFromText` in shared/native-chat-diff). - * - * @internal - exposed for tests only. - */ -export function countDiffLines(diff: string): { additions: number; deletions: number } { - let additions = 0 - let deletions = 0 - // Why: `---`/`+++` are file headers only before the first hunk. A removed line - // whose original text began with `--` (SQL/Lua/Haskell `-- comment`) becomes a - // diff line `---`, colliding with the `--- a/file` header — so it must - // be counted once inside a hunk, not skipped. - let inHunk = false - for (const line of diff.split('\n')) { - if (line.startsWith('@@')) { - inHunk = true - continue - } - if (!inHunk) { - continue - } - if (line.startsWith('+')) { - additions += 1 - } else if (line.startsWith('-')) { - deletions += 1 - } - } - return { additions, deletions } -} - -function mapMRFile(raw: { - new_path?: string - old_path?: string - diff?: string - new_file?: boolean - deleted_file?: boolean - renamed_file?: boolean - binary?: boolean - too_large?: boolean -}): GitLabMRFile { - const diff = raw.diff ?? '' - const counts = countDiffLines(diff) - const status = raw.new_file - ? 'added' - : raw.deleted_file - ? 'removed' - : raw.renamed_file - ? 'renamed' - : 'modified' - return { - path: raw.new_path ?? raw.old_path ?? '', - ...(raw.old_path && raw.old_path !== raw.new_path ? { oldPath: raw.old_path } : {}), - status, - additions: counts.additions, - deletions: counts.deletions, - isBinary: Boolean(raw.binary || raw.too_large || !diff), - ...(diff ? { diff } : {}) - } -} - -async function fetchMRFiles( - repoPath: string, - projectRef: ProjectRef, - iid: number, - connectionId?: string | null, - localGitOptions: LocalGitExecOptions = {} -): Promise { - const { stdout } = await glabExecFileAsync( - [ - 'api', - ...glabHostnameArgs(projectRef, connectionId), - // Why: GitLab deprecated the all-in-one `changes` endpoint in favor of - // the paginated diffs endpoint; cap the file snapshot at one visible page. - `projects/${encodedProject(projectRef.path)}/merge_requests/${iid}/diffs?per_page=100` - ], - glabRepoExecOptions(repoPath, connectionId, localGitOptions) - ) - const data = JSON.parse(stdout) as Parameters[0][] - return data.map(mapMRFile).filter((file) => file.path) -} - // ── Top-level aggregator ─────────────────────────────────────────── type GitLabRawIssue = Parameters[0] & { @@ -457,90 +42,6 @@ type GitLabRawMR = Parameters[0] & { reviewers?: GitLabRawUser[] | null } -async function fetchMRReviewers( - repoPath: string, - projectRef: ProjectRef, - iid: number, - connectionId?: string | null, - localGitOptions: LocalGitExecOptions = {} -): Promise { - const { stdout } = await glabExecFileAsync( - [ - 'api', - ...glabHostnameArgs(projectRef, connectionId), - `projects/${encodedProject(projectRef.path)}/merge_requests/${iid}/reviewers` - ], - glabRepoExecOptions(repoPath, connectionId, localGitOptions) - ) - const data = JSON.parse(stdout) as { user?: GitLabRawUser | null }[] - return data - .map((entry) => mapGitLabUser(entry.user)) - .filter((u): u is GitLabAssignableUser => !!u) -} - -async function fetchMRApprovalState( - repoPath: string, - projectRef: ProjectRef, - iid: number, - connectionId?: string | null, - localGitOptions: LocalGitExecOptions = {} -): Promise { - const [approvalsRes, stateRes] = await Promise.allSettled([ - glabExecFileAsync( - [ - 'api', - ...glabHostnameArgs(projectRef, connectionId), - `projects/${encodedProject(projectRef.path)}/merge_requests/${iid}/approvals` - ], - glabRepoExecOptions(repoPath, connectionId, localGitOptions) - ), - glabExecFileAsync( - [ - 'api', - ...glabHostnameArgs(projectRef, connectionId), - `projects/${encodedProject(projectRef.path)}/merge_requests/${iid}/approval_state` - ], - glabRepoExecOptions(repoPath, connectionId, localGitOptions) - ) - ]) - if (approvalsRes.status === 'rejected' && stateRes.status === 'rejected') { - return undefined - } - const approvals = - approvalsRes.status === 'fulfilled' - ? (JSON.parse(approvalsRes.value.stdout) as { - approvals_required?: number | null - approvals_left?: number | null - approved_by?: { user?: GitLabRawUser | null }[] - }) - : null - const state = - stateRes.status === 'fulfilled' - ? (JSON.parse(stateRes.value.stdout) as { - rules?: { - id?: number - name?: string - approvals_required?: number - approved?: boolean - }[] - }) - : null - return { - approvalsRequired: - typeof approvals?.approvals_required === 'number' ? approvals.approvals_required : null, - approvalsLeft: typeof approvals?.approvals_left === 'number' ? approvals.approvals_left : null, - approvedBy: (approvals?.approved_by ?? []) - .map((entry) => mapGitLabUser(entry.user)) - .filter((u): u is GitLabAssignableUser => !!u), - rules: (state?.rules ?? []).map((rule) => ({ - id: rule.id ?? 0, - name: rule.name ?? 'Approval rule', - approvalsRequired: rule.approvals_required ?? 0, - approved: Boolean(rule.approved) - })) - } -} - /** * Fetch full details for a GitLab MR or issue: the work item itself, * description body, discussion notes flattened to MRComment[], and (for diff --git a/src/main/ipc/github.ts b/src/main/ipc/github.ts index 36516790446..7673aff06ce 100644 --- a/src/main/ipc/github.ts +++ b/src/main/ipc/github.ts @@ -108,7 +108,7 @@ import type { UpdateIssueTypeBySlugArgs, UpdateProjectItemFieldArgs, UpdatePullRequestBySlugArgs -} from '../../shared/github/project-types' +} from '../../shared/github/project-request-types' import { appStarSourceSchema } from '../../shared/gh-star-source' import { track } from '../telemetry/client' import { getCohortAtEmit } from '../telemetry/cohort-classifier' diff --git a/src/main/ipc/gitlab-ci-job-handlers.ts b/src/main/ipc/gitlab-ci-job-handlers.ts new file mode 100644 index 00000000000..f80eda00c2a --- /dev/null +++ b/src/main/ipc/gitlab-ci-job-handlers.ts @@ -0,0 +1,50 @@ +import { ipcMain } from 'electron' +import { toGitLabJobLogExcerptResult } from '../../shared/gitlab-job-log-excerpt' +import type { Store } from '../persistence' +import { getJobTrace, retryJob } from '../gitlab/client' +import type { ProjectRef } from '../gitlab/gl-utils' +import type { GitLabRepoSelectorArgs } from './gitlab-repo-access' +import { assertRegisteredRepo, localGitOptionArgs, repoConnectionId } from './gitlab-repo-access' + +export function registerGitLabCiJobHandlers(store: Store): void { + ipcMain.handle( + 'gitlab:jobTrace', + async ( + _event, + args: GitLabRepoSelectorArgs & { + jobId: number + projectRef?: ProjectRef | null + logExcerpt?: boolean + } + ) => { + const repo = assertRegisteredRepo(args, store) + const result = await getJobTrace( + repo.path, + args.jobId, + repo.issueSourcePreference, + repoConnectionId(repo), + args.projectRef, + ...localGitOptionArgs(store, repo) + ) + return args.logExcerpt ? toGitLabJobLogExcerptResult(result) : result + } + ) + + ipcMain.handle( + 'gitlab:retryJob', + async ( + _event, + args: GitLabRepoSelectorArgs & { jobId: number; projectRef?: ProjectRef | null } + ) => { + const repo = assertRegisteredRepo(args, store) + return retryJob( + repo.path, + args.jobId, + repo.issueSourcePreference, + repoConnectionId(repo), + args.projectRef, + ...localGitOptionArgs(store, repo) + ) + } + ) +} diff --git a/src/main/ipc/gitlab-issue-handlers.ts b/src/main/ipc/gitlab-issue-handlers.ts new file mode 100644 index 00000000000..7ae8edf6ceb --- /dev/null +++ b/src/main/ipc/gitlab-issue-handlers.ts @@ -0,0 +1,150 @@ +import { ipcMain } from 'electron' +import type { GitLabIssueUpdate, GitLabWorkItem } from '../../shared/gitlab-types' +import type { TaskSourceContext } from '../../shared/task-source-context' +import type { Store } from '../persistence' +import { + normalizeGitLabIssueAssignee, + normalizeGitLabIssueListState, + normalizeGitLabPositiveInteger +} from '../gitlab/gitlab-preload-args' +import { + addIssueComment, + createIssue, + getIssue, + listAssignableUsers, + listIssues, + listLabels, + updateIssue +} from '../gitlab/client' +import type { GitLabRepoSelectorArgs } from './gitlab-repo-access' +import { assertRegisteredRepo, localGitOptionArgs, repoConnectionId } from './gitlab-repo-access' + +export function registerGitLabIssueHandlers(store: Store): void { + ipcMain.handle( + 'gitlab:issue', + async (_event, args: GitLabRepoSelectorArgs & { number: number }) => { + const repo = assertRegisteredRepo(args, store) + return getIssue( + repo.path, + args.number, + repoConnectionId(repo), + ...localGitOptionArgs(store, repo) + ) + } + ) + + ipcMain.handle( + 'gitlab:listIssues', + async ( + _event, + args: { + repoPath: string + repoId?: string | null + sourceContext?: TaskSourceContext | null + state?: 'opened' | 'closed' | 'all' + assignee?: string + limit?: number + } + ) => { + const repo = assertRegisteredRepo(args, store) + const limit = normalizeGitLabPositiveInteger(args.limit, 20, 100) + const state = normalizeGitLabIssueListState(args.state) + const assignee = normalizeGitLabIssueAssignee(args.assignee) + const result = await listIssues( + repo.path, + limit, + repo.issueSourcePreference, + state, + assignee, + repoConnectionId(repo), + ...localGitOptionArgs(store, repo) + ) + // Why: Tasks page expects GitLabWorkItem[] so it can share row + // rendering with MRs. Map IssueInfo → WorkItem here so the renderer + // doesn't need a separate code path. + const workItems: GitLabWorkItem[] = result.items.map((issue) => ({ + id: `gitlab-issue-${repo.id}-${issue.number}`, + type: 'issue' as const, + number: issue.number, + title: issue.title, + state: issue.state, + url: issue.url, + labels: issue.labels, + updatedAt: issue.updatedAt ?? '', + author: issue.author ?? null, + repoId: repo.id + })) + return { items: workItems, ...(result.error ? { error: result.error } : {}) } + } + ) + + ipcMain.handle( + 'gitlab:createIssue', + async (_event, args: GitLabRepoSelectorArgs & { title: string; body: string }) => { + const repo = assertRegisteredRepo(args, store) + return createIssue( + repo.path, + args.title, + args.body, + repo.issueSourcePreference, + repoConnectionId(repo), + ...localGitOptionArgs(store, repo) + ) + } + ) + + ipcMain.handle( + 'gitlab:updateIssue', + async ( + _event, + args: GitLabRepoSelectorArgs & { number: number; updates: GitLabIssueUpdate } + ) => { + const repo = assertRegisteredRepo(args, store) + return updateIssue( + repo.path, + args.number, + args.updates, + repo.issueSourcePreference, + repoConnectionId(repo), + undefined, + ...localGitOptionArgs(store, repo) + ) + } + ) + + ipcMain.handle( + 'gitlab:addIssueComment', + async (_event, args: GitLabRepoSelectorArgs & { number: number; body: string }) => { + const repo = assertRegisteredRepo(args, store) + return addIssueComment( + repo.path, + args.number, + args.body, + repo.issueSourcePreference, + repoConnectionId(repo), + undefined, + ...localGitOptionArgs(store, repo) + ) + } + ) + + ipcMain.handle('gitlab:listLabels', async (_event, args: GitLabRepoSelectorArgs) => { + const repo = assertRegisteredRepo(args, store) + return listLabels( + repo.path, + repo.issueSourcePreference, + repoConnectionId(repo), + ...localGitOptionArgs(store, repo) + ) + }) + + ipcMain.handle('gitlab:listAssignableUsers', async (_event, args: GitLabRepoSelectorArgs) => { + const repo = assertRegisteredRepo(args, store) + return listAssignableUsers( + repo.path, + repo.issueSourcePreference, + repoConnectionId(repo), + ...localGitOptionArgs(store, repo) + ) + }) +} diff --git a/src/main/ipc/gitlab-merge-request-mutation-handlers.ts b/src/main/ipc/gitlab-merge-request-mutation-handlers.ts new file mode 100644 index 00000000000..b47b9523e46 --- /dev/null +++ b/src/main/ipc/gitlab-merge-request-mutation-handlers.ts @@ -0,0 +1,172 @@ +import { ipcMain } from 'electron' +import type { GitLabMRInlineCommentInput, GitLabMRUpdate } from '../../shared/gitlab-types' +import type { TaskSourceContext } from '../../shared/task-source-context' +import type { Store } from '../persistence' +import { + addMRComment, + addMRInlineComment, + closeMR, + mergeMR, + reopenMR, + resolveMRDiscussion, + updateMR, + updateMRReviewers +} from '../gitlab/client' +import type { ProjectRef } from '../gitlab/gl-utils' +import type { GitLabRepoSelectorArgs } from './gitlab-repo-access' +import { assertRegisteredRepo, localGitOptionArgs, repoConnectionId } from './gitlab-repo-access' + +export function registerGitLabMergeRequestMutationHandlers(store: Store): void { + ipcMain.handle( + 'gitlab:closeMR', + async (_event, args: GitLabRepoSelectorArgs & { iid: number }) => { + const repo = assertRegisteredRepo(args, store) + return closeMR( + repo.path, + args.iid, + repo.issueSourcePreference, + repoConnectionId(repo), + undefined, + ...localGitOptionArgs(store, repo) + ) + } + ) + + ipcMain.handle( + 'gitlab:reopenMR', + async (_event, args: GitLabRepoSelectorArgs & { iid: number }) => { + const repo = assertRegisteredRepo(args, store) + return reopenMR( + repo.path, + args.iid, + repo.issueSourcePreference, + repoConnectionId(repo), + undefined, + ...localGitOptionArgs(store, repo) + ) + } + ) + + ipcMain.handle( + 'gitlab:mergeMR', + async ( + _event, + args: GitLabRepoSelectorArgs & { iid: number; method?: 'merge' | 'squash' | 'rebase' } + ) => { + const repo = assertRegisteredRepo(args, store) + return mergeMR( + repo.path, + args.iid, + args.method ?? 'merge', + repo.issueSourcePreference, + repoConnectionId(repo), + undefined, + ...localGitOptionArgs(store, repo) + ) + } + ) + + ipcMain.handle( + 'gitlab:updateMR', + async (_event, args: GitLabRepoSelectorArgs & { iid: number; updates: GitLabMRUpdate }) => { + const repo = assertRegisteredRepo(args, store) + return updateMR( + repo.path, + args.iid, + args.updates, + repo.issueSourcePreference, + repoConnectionId(repo), + undefined, + ...localGitOptionArgs(store, repo) + ) + } + ) + + ipcMain.handle( + 'gitlab:updateMRReviewers', + async ( + _event, + args: { + repoPath: string + repoId?: string | null + sourceContext?: TaskSourceContext | null + iid: number + reviewerIds: number[] + projectRef?: ProjectRef | null + } + ) => { + const repo = assertRegisteredRepo(args, store) + return updateMRReviewers( + repo.path, + args.iid, + args.reviewerIds, + repo.issueSourcePreference, + repoConnectionId(repo), + args.projectRef, + ...localGitOptionArgs(store, repo) + ) + } + ) + + ipcMain.handle( + 'gitlab:addMRComment', + async (_event, args: GitLabRepoSelectorArgs & { iid: number; body: string }) => { + const repo = assertRegisteredRepo(args, store) + return addMRComment( + repo.path, + args.iid, + args.body, + repo.issueSourcePreference, + repoConnectionId(repo), + undefined, + ...localGitOptionArgs(store, repo) + ) + } + ) + + ipcMain.handle( + 'gitlab:addMRInlineComment', + async ( + _event, + args: { + repoPath: string + repoId?: string | null + sourceContext?: TaskSourceContext | null + iid: number + input: GitLabMRInlineCommentInput + projectRef?: ProjectRef | null + } + ) => { + const repo = assertRegisteredRepo(args, store) + return addMRInlineComment( + repo.path, + args.iid, + args.input, + repo.issueSourcePreference, + repoConnectionId(repo), + args.projectRef, + ...localGitOptionArgs(store, repo) + ) + } + ) + + ipcMain.handle( + 'gitlab:resolveMRDiscussion', + async ( + _event, + args: GitLabRepoSelectorArgs & { iid: number; discussionId: string; resolved: boolean } + ) => { + const repo = assertRegisteredRepo(args, store) + return resolveMRDiscussion( + repo.path, + args.iid, + args.discussionId, + args.resolved, + repo.issueSourcePreference, + repoConnectionId(repo), + undefined, + ...localGitOptionArgs(store, repo) + ) + } + ) +} diff --git a/src/main/ipc/gitlab-merge-request-query-handlers.ts b/src/main/ipc/gitlab-merge-request-query-handlers.ts new file mode 100644 index 00000000000..2d5e3bb88c7 --- /dev/null +++ b/src/main/ipc/gitlab-merge-request-query-handlers.ts @@ -0,0 +1,76 @@ +import { ipcMain } from 'electron' +import type { TaskSourceContext } from '../../shared/task-source-context' +import type { Store } from '../persistence' +import { + normalizeGitLabMRListState, + normalizeGitLabPositiveInteger, + normalizeGitLabSearchQuery +} from '../gitlab/gitlab-preload-args' +import { getMergeRequest, getMergeRequestForBranch, listMergeRequests } from '../gitlab/client' +import type { GitLabRepoSelectorArgs } from './gitlab-repo-access' +import { + assertRegisteredRepo, + hostedReviewOptionArgs, + localGitOptionArgs, + repoConnectionId +} from './gitlab-repo-access' + +export function registerGitLabMergeRequestQueryHandlers(store: Store): void { + ipcMain.handle( + 'gitlab:mrForBranch', + async ( + _event, + args: GitLabRepoSelectorArgs & { branch: string; linkedMRIid?: number | null } + ) => { + const repo = assertRegisteredRepo(args, store) + return getMergeRequestForBranch( + repo.path, + args.branch, + args.linkedMRIid ?? null, + repoConnectionId(repo), + ...hostedReviewOptionArgs(store, repo) + ) + } + ) + + ipcMain.handle('gitlab:mr', async (_event, args: GitLabRepoSelectorArgs & { iid: number }) => { + const repo = assertRegisteredRepo(args, store) + return getMergeRequest( + repo.path, + args.iid, + repoConnectionId(repo), + ...hostedReviewOptionArgs(store, repo) + ) + }) + + ipcMain.handle( + 'gitlab:listMRs', + async ( + _event, + args: { + repoPath: string + repoId?: string | null + sourceContext?: TaskSourceContext | null + state?: 'opened' | 'merged' | 'closed' | 'all' + page?: number + perPage?: number + query?: string + } + ) => { + const repo = assertRegisteredRepo(args, store) + const state = normalizeGitLabMRListState(args.state) + const page = normalizeGitLabPositiveInteger(args.page, 1, 10_000) + const perPage = normalizeGitLabPositiveInteger(args.perPage, 20, 100) + return listMergeRequests( + repo.path, + state, + page, + perPage, + repo.issueSourcePreference, + normalizeGitLabSearchQuery(args.query), + repoConnectionId(repo), + ...localGitOptionArgs(store, repo) + ) + } + ) +} diff --git a/src/main/ipc/gitlab-repo-access.ts b/src/main/ipc/gitlab-repo-access.ts new file mode 100644 index 00000000000..5bf70e7ff2b --- /dev/null +++ b/src/main/ipc/gitlab-repo-access.ts @@ -0,0 +1,65 @@ +import { resolve } from 'node:path' +import type { Repo } from '../../shared/repo-types' +import { getRepoExecutionHostId } from '../../shared/execution-host' +import type { TaskSourceContext } from '../../shared/task-source-context' +import type { Store } from '../persistence' +import type { LocalGitExecOptions } from '../gitlab/gitlab-project-ref-resolution' +import { getLocalProjectWorktreeGitOptions } from '../project-runtime-git-options' +import type { HostedReviewExecutionOptions } from '../source-control/hosted-review-git-options' + +export type GitLabRepoSelectorArgs = { + repoPath: string + repoId?: string | null + sourceContext?: TaskSourceContext | null +} + +function findRegisteredGitLabRepo(args: GitLabRepoSelectorArgs, store: Store): Repo | undefined { + const sourceRepoId = + args.sourceContext?.provider === 'gitlab' ? args.sourceContext.repoId?.trim() : null + const repoId = args.repoId?.trim() || sourceRepoId || null + if (repoId) { + const repo = store.getRepo(repoId) + if (repo) { + return repo + } + } + const resolvedRepoPath = resolve(args.repoPath) + return store.getRepos().find((r) => resolve(r.path) === resolvedRepoPath) +} + +// Why: mirror github.ts assertRegisteredRepo — main-process handlers +// must never operate on a path the user hasn't explicitly registered as +// a repo (filesystem-auth boundary). Source context adds a host check so a +// task fetched from one machine cannot mutate a same-path repo on another. +export function assertRegisteredRepo(args: GitLabRepoSelectorArgs, store: Store): Repo { + const repo = findRegisteredGitLabRepo(args, store) + if (!repo) { + throw new Error('Access denied: unknown repository path') + } + if ( + args.sourceContext?.provider === 'gitlab' && + args.sourceContext.hostId !== getRepoExecutionHostId(repo) + ) { + throw new Error('Access denied: GitLab source host does not match repository host') + } + return repo +} + +export function repoConnectionId(repo: Repo): string | null { + return repo.connectionId ?? null +} + +export function localGitOptionArgs(store: Store, repo: Repo): [] | [LocalGitExecOptions] { + const localGitOptions = getLocalProjectWorktreeGitOptions(store, repo) + return localGitOptions.wslDistro ? [{ wslDistro: localGitOptions.wslDistro }] : [] +} + +export function hostedReviewOptionArgs( + store: Store, + repo: Repo +): [] | [HostedReviewExecutionOptions] { + const localGitOptions = getLocalProjectWorktreeGitOptions(store, repo) + return localGitOptions.wslDistro + ? [{ localGitExecOptions: { wslDistro: localGitOptions.wslDistro } }] + : [] +} diff --git a/src/main/ipc/gitlab-work-item-handlers.ts b/src/main/ipc/gitlab-work-item-handlers.ts new file mode 100644 index 00000000000..4f98680d3f1 --- /dev/null +++ b/src/main/ipc/gitlab-work-item-handlers.ts @@ -0,0 +1,100 @@ +import { ipcMain } from 'electron' +import type { TaskSourceContext } from '../../shared/task-source-context' +import type { Store } from '../persistence' +import { + normalizeGitLabMRListState, + normalizeGitLabPositiveInteger, + normalizeGitLabSearchQuery +} from '../gitlab/gitlab-preload-args' +import { recordGitLabProjectRecent } from '../gitlab/gitlab-project-recents' +import { getWorkItemByProjectRef, listWorkItems } from '../gitlab/client' +import { getWorkItemDetails } from '../gitlab/work-item-details' +import type { ProjectRef } from '../gitlab/gl-utils' +import type { GitLabRepoSelectorArgs } from './gitlab-repo-access' +import { assertRegisteredRepo, localGitOptionArgs, repoConnectionId } from './gitlab-repo-access' + +export function registerGitLabWorkItemHandlers(store: Store): void { + // Why: combined MR + issue list — Tasks screen and any future picker + // that wants a unified view. Centralizes the merge / sort logic so + // callers don't have to re-implement it. + ipcMain.handle( + 'gitlab:listWorkItems', + async ( + _event, + args: { + repoPath: string + repoId?: string | null + sourceContext?: TaskSourceContext | null + state?: 'opened' | 'merged' | 'closed' | 'all' + page?: number + perPage?: number + query?: string + } + ) => { + const repo = assertRegisteredRepo(args, store) + return listWorkItems( + repo.path, + normalizeGitLabMRListState(args.state), + normalizeGitLabPositiveInteger(args.page, 1, 10_000), + normalizeGitLabPositiveInteger(args.perPage, 20, 100), + repo.issueSourcePreference, + normalizeGitLabSearchQuery(args.query), + repoConnectionId(repo), + ...localGitOptionArgs(store, repo) + ) + } + ) + + // Why: aggregated dialog payload — body + discussions + pipeline jobs. + // Powers GitLabItemDialog's tabs. + ipcMain.handle( + 'gitlab:workItemDetails', + async (_event, args: GitLabRepoSelectorArgs & { iid: number; type: 'issue' | 'mr' }) => { + const repo = assertRegisteredRepo(args, store) + return getWorkItemDetails( + repo.path, + args.iid, + args.type, + repo.issueSourcePreference, + repoConnectionId(repo), + undefined, + ...localGitOptionArgs(store, repo) + ) + } + ) + + // Why: paste-URL flow in the picker. The user pastes a GitLab URL that + // may target a project different from the local checkout's remote, so + // the call carries the parsed project path explicitly rather than + // resolving from cwd. + ipcMain.handle( + 'gitlab:workItemByPath', + async ( + _event, + args: GitLabRepoSelectorArgs & { + host: string + path: string + iid: number + type: 'issue' | 'mr' + } + ) => { + const repo = assertRegisteredRepo(args, store) + const projectRef: ProjectRef = { host: args.host, path: args.path } + const result = await getWorkItemByProjectRef( + repo.path, + projectRef, + args.iid, + args.type, + repoConnectionId(repo), + ...localGitOptionArgs(store, repo) + ) + // Why: only persist a recent entry when the lookup actually + // produced an item. A 404 / auth failure shouldn't pollute the + // user's recents list with project paths they can't read. + if (result) { + recordGitLabProjectRecent(store, args.host, args.path) + } + return result + } + ) +} diff --git a/src/main/ipc/gitlab.ts b/src/main/ipc/gitlab.ts index cc668841038..b492e66ef7a 100644 --- a/src/main/ipc/gitlab.ts +++ b/src/main/ipc/gitlab.ts @@ -1,115 +1,24 @@ -/* eslint-disable max-lines -- Why: parallel to ipc/github.ts — keeping all -GitLab IPC handlers co-located keeps the repo-path validation pattern -reviewable as one surface. */ import { ipcMain } from 'electron' -import { resolve } from 'node:path' -import { toGitLabJobLogExcerptResult } from '../../shared/gitlab-job-log-excerpt' -import type { - GitLabIssueUpdate, - GitLabMRInlineCommentInput, - GitLabMRUpdate, - GitLabWorkItem -} from '../../shared/gitlab-types' -import type { Repo } from '../../shared/repo-types' -import { getRepoExecutionHostId } from '../../shared/execution-host' -import type { TaskSourceContext } from '../../shared/task-source-context' import type { Store } from '../persistence' import { - normalizeGitLabIssueAssignee, - normalizeGitLabIssueListState, - normalizeGitLabMRListState, - normalizeGitLabPositiveInteger, - normalizeGitLabSearchQuery -} from '../gitlab/gitlab-preload-args' -import { recordGitLabProjectRecent } from '../gitlab/gitlab-project-recents' -import { - addIssueComment, - addMRInlineComment, - addMRComment, - closeMR, - createIssue, diagnoseAuth, getAuthenticatedViewer, - getJobTrace, - getIssue, - getMergeRequest, - getMergeRequestForBranch, getProjectSlug, getRateLimit, - getWorkItemByProjectRef, - listAssignableUsers, - listIssues, - listLabels, - listMergeRequests, - listTodos, - listWorkItems, - mergeMR, - reopenMR, - resolveMRDiscussion, - retryJob, - updateIssue, - updateMR, - updateMRReviewers + listTodos } from '../gitlab/client' -import { getWorkItemDetails } from '../gitlab/work-item-details' -import type { ProjectRef } from '../gitlab/gl-utils' -import type { LocalGitExecOptions } from '../gitlab/gitlab-project-ref-resolution' -import { getLocalProjectWorktreeGitOptions } from '../project-runtime-git-options' -import type { HostedReviewExecutionOptions } from '../source-control/hosted-review-git-options' - -type GitLabRepoSelectorArgs = { - repoPath: string - repoId?: string | null - sourceContext?: TaskSourceContext | null -} - -function findRegisteredGitLabRepo(args: GitLabRepoSelectorArgs, store: Store): Repo | undefined { - const sourceRepoId = - args.sourceContext?.provider === 'gitlab' ? args.sourceContext.repoId?.trim() : null - const repoId = args.repoId?.trim() || sourceRepoId || null - if (repoId) { - const repo = store.getRepo(repoId) - if (repo) { - return repo - } - } - const resolvedRepoPath = resolve(args.repoPath) - return store.getRepos().find((r) => resolve(r.path) === resolvedRepoPath) -} - -// Why: mirror github.ts assertRegisteredRepo — main-process handlers -// must never operate on a path the user hasn't explicitly registered as -// a repo (filesystem-auth boundary). Source context adds a host check so a -// task fetched from one machine cannot mutate a same-path repo on another. -function assertRegisteredRepo(args: GitLabRepoSelectorArgs, store: Store): Repo { - const repo = findRegisteredGitLabRepo(args, store) - if (!repo) { - throw new Error('Access denied: unknown repository path') - } - if ( - args.sourceContext?.provider === 'gitlab' && - args.sourceContext.hostId !== getRepoExecutionHostId(repo) - ) { - throw new Error('Access denied: GitLab source host does not match repository host') - } - return repo -} - -function repoConnectionId(repo: Repo): string | null { - return repo.connectionId ?? null -} - -function localGitOptionArgs(store: Store, repo: Repo): [] | [LocalGitExecOptions] { - const localGitOptions = getLocalProjectWorktreeGitOptions(store, repo) - return localGitOptions.wslDistro ? [{ wslDistro: localGitOptions.wslDistro }] : [] -} - -function hostedReviewOptionArgs(store: Store, repo: Repo): [] | [HostedReviewExecutionOptions] { - const localGitOptions = getLocalProjectWorktreeGitOptions(store, repo) - return localGitOptions.wslDistro - ? [{ localGitExecOptions: { wslDistro: localGitOptions.wslDistro } }] - : [] -} +import { registerGitLabCiJobHandlers } from './gitlab-ci-job-handlers' +import { registerGitLabIssueHandlers } from './gitlab-issue-handlers' +import { registerGitLabMergeRequestMutationHandlers } from './gitlab-merge-request-mutation-handlers' +import { registerGitLabMergeRequestQueryHandlers } from './gitlab-merge-request-query-handlers' +import type { GitLabRepoSelectorArgs } from './gitlab-repo-access' +import { + assertRegisteredRepo, + hostedReviewOptionArgs, + localGitOptionArgs, + repoConnectionId +} from './gitlab-repo-access' +import { registerGitLabWorkItemHandlers } from './gitlab-work-item-handlers' export function registerGitLabHandlers(store: Store): void { ipcMain.handle('gitlab:viewer', async () => { @@ -129,434 +38,11 @@ export function registerGitLabHandlers(store: Store): void { return getProjectSlug(repo.path, repoConnectionId(repo), ...hostedReviewOptionArgs(store, repo)) }) - ipcMain.handle( - 'gitlab:mrForBranch', - async ( - _event, - args: GitLabRepoSelectorArgs & { branch: string; linkedMRIid?: number | null } - ) => { - const repo = assertRegisteredRepo(args, store) - return getMergeRequestForBranch( - repo.path, - args.branch, - args.linkedMRIid ?? null, - repoConnectionId(repo), - ...hostedReviewOptionArgs(store, repo) - ) - } - ) - - ipcMain.handle('gitlab:mr', async (_event, args: GitLabRepoSelectorArgs & { iid: number }) => { - const repo = assertRegisteredRepo(args, store) - return getMergeRequest( - repo.path, - args.iid, - repoConnectionId(repo), - ...hostedReviewOptionArgs(store, repo) - ) - }) - - ipcMain.handle( - 'gitlab:listMRs', - async ( - _event, - args: { - repoPath: string - repoId?: string | null - sourceContext?: TaskSourceContext | null - state?: 'opened' | 'merged' | 'closed' | 'all' - page?: number - perPage?: number - query?: string - } - ) => { - const repo = assertRegisteredRepo(args, store) - const state = normalizeGitLabMRListState(args.state) - const page = normalizeGitLabPositiveInteger(args.page, 1, 10_000) - const perPage = normalizeGitLabPositiveInteger(args.perPage, 20, 100) - return listMergeRequests( - repo.path, - state, - page, - perPage, - repo.issueSourcePreference, - normalizeGitLabSearchQuery(args.query), - repoConnectionId(repo), - ...localGitOptionArgs(store, repo) - ) - } - ) - - ipcMain.handle( - 'gitlab:issue', - async (_event, args: GitLabRepoSelectorArgs & { number: number }) => { - const repo = assertRegisteredRepo(args, store) - return getIssue( - repo.path, - args.number, - repoConnectionId(repo), - ...localGitOptionArgs(store, repo) - ) - } - ) - - ipcMain.handle( - 'gitlab:listIssues', - async ( - _event, - args: { - repoPath: string - repoId?: string | null - sourceContext?: TaskSourceContext | null - state?: 'opened' | 'closed' | 'all' - assignee?: string - limit?: number - } - ) => { - const repo = assertRegisteredRepo(args, store) - const limit = normalizeGitLabPositiveInteger(args.limit, 20, 100) - const state = normalizeGitLabIssueListState(args.state) - const assignee = normalizeGitLabIssueAssignee(args.assignee) - const result = await listIssues( - repo.path, - limit, - repo.issueSourcePreference, - state, - assignee, - repoConnectionId(repo), - ...localGitOptionArgs(store, repo) - ) - // Why: Tasks page expects GitLabWorkItem[] so it can share row - // rendering with MRs. Map IssueInfo → WorkItem here so the renderer - // doesn't need a separate code path. - const workItems: GitLabWorkItem[] = result.items.map((issue) => ({ - id: `gitlab-issue-${repo.id}-${issue.number}`, - type: 'issue' as const, - number: issue.number, - title: issue.title, - state: issue.state, - url: issue.url, - labels: issue.labels, - updatedAt: issue.updatedAt ?? '', - author: issue.author ?? null, - repoId: repo.id - })) - return { items: workItems, ...(result.error ? { error: result.error } : {}) } - } - ) - - ipcMain.handle( - 'gitlab:createIssue', - async (_event, args: GitLabRepoSelectorArgs & { title: string; body: string }) => { - const repo = assertRegisteredRepo(args, store) - return createIssue( - repo.path, - args.title, - args.body, - repo.issueSourcePreference, - repoConnectionId(repo), - ...localGitOptionArgs(store, repo) - ) - } - ) - - ipcMain.handle( - 'gitlab:updateIssue', - async ( - _event, - args: GitLabRepoSelectorArgs & { number: number; updates: GitLabIssueUpdate } - ) => { - const repo = assertRegisteredRepo(args, store) - return updateIssue( - repo.path, - args.number, - args.updates, - repo.issueSourcePreference, - repoConnectionId(repo), - undefined, - ...localGitOptionArgs(store, repo) - ) - } - ) - - ipcMain.handle( - 'gitlab:addIssueComment', - async (_event, args: GitLabRepoSelectorArgs & { number: number; body: string }) => { - const repo = assertRegisteredRepo(args, store) - return addIssueComment( - repo.path, - args.number, - args.body, - repo.issueSourcePreference, - repoConnectionId(repo), - undefined, - ...localGitOptionArgs(store, repo) - ) - } - ) - - ipcMain.handle('gitlab:listLabels', async (_event, args: GitLabRepoSelectorArgs) => { - const repo = assertRegisteredRepo(args, store) - return listLabels( - repo.path, - repo.issueSourcePreference, - repoConnectionId(repo), - ...localGitOptionArgs(store, repo) - ) - }) - - ipcMain.handle('gitlab:listAssignableUsers', async (_event, args: GitLabRepoSelectorArgs) => { - const repo = assertRegisteredRepo(args, store) - return listAssignableUsers( - repo.path, - repo.issueSourcePreference, - repoConnectionId(repo), - ...localGitOptionArgs(store, repo) - ) - }) - - // Why: combined MR + issue list — Tasks screen and any future picker - // that wants a unified view. Centralizes the merge / sort logic so - // callers don't have to re-implement it. - ipcMain.handle( - 'gitlab:listWorkItems', - async ( - _event, - args: { - repoPath: string - repoId?: string | null - sourceContext?: TaskSourceContext | null - state?: 'opened' | 'merged' | 'closed' | 'all' - page?: number - perPage?: number - query?: string - } - ) => { - const repo = assertRegisteredRepo(args, store) - return listWorkItems( - repo.path, - normalizeGitLabMRListState(args.state), - normalizeGitLabPositiveInteger(args.page, 1, 10_000), - normalizeGitLabPositiveInteger(args.perPage, 20, 100), - repo.issueSourcePreference, - normalizeGitLabSearchQuery(args.query), - repoConnectionId(repo), - ...localGitOptionArgs(store, repo) - ) - } - ) - - // Why: aggregated dialog payload — body + discussions + pipeline jobs. - // Powers GitLabItemDialog's tabs. - ipcMain.handle( - 'gitlab:workItemDetails', - async (_event, args: GitLabRepoSelectorArgs & { iid: number; type: 'issue' | 'mr' }) => { - const repo = assertRegisteredRepo(args, store) - return getWorkItemDetails( - repo.path, - args.iid, - args.type, - repo.issueSourcePreference, - repoConnectionId(repo), - undefined, - ...localGitOptionArgs(store, repo) - ) - } - ) - - ipcMain.handle( - 'gitlab:closeMR', - async (_event, args: GitLabRepoSelectorArgs & { iid: number }) => { - const repo = assertRegisteredRepo(args, store) - return closeMR( - repo.path, - args.iid, - repo.issueSourcePreference, - repoConnectionId(repo), - undefined, - ...localGitOptionArgs(store, repo) - ) - } - ) - - ipcMain.handle( - 'gitlab:reopenMR', - async (_event, args: GitLabRepoSelectorArgs & { iid: number }) => { - const repo = assertRegisteredRepo(args, store) - return reopenMR( - repo.path, - args.iid, - repo.issueSourcePreference, - repoConnectionId(repo), - undefined, - ...localGitOptionArgs(store, repo) - ) - } - ) - - ipcMain.handle( - 'gitlab:mergeMR', - async ( - _event, - args: GitLabRepoSelectorArgs & { iid: number; method?: 'merge' | 'squash' | 'rebase' } - ) => { - const repo = assertRegisteredRepo(args, store) - return mergeMR( - repo.path, - args.iid, - args.method ?? 'merge', - repo.issueSourcePreference, - repoConnectionId(repo), - undefined, - ...localGitOptionArgs(store, repo) - ) - } - ) - - ipcMain.handle( - 'gitlab:updateMR', - async (_event, args: GitLabRepoSelectorArgs & { iid: number; updates: GitLabMRUpdate }) => { - const repo = assertRegisteredRepo(args, store) - return updateMR( - repo.path, - args.iid, - args.updates, - repo.issueSourcePreference, - repoConnectionId(repo), - undefined, - ...localGitOptionArgs(store, repo) - ) - } - ) - - ipcMain.handle( - 'gitlab:updateMRReviewers', - async ( - _event, - args: { - repoPath: string - repoId?: string | null - sourceContext?: TaskSourceContext | null - iid: number - reviewerIds: number[] - projectRef?: ProjectRef | null - } - ) => { - const repo = assertRegisteredRepo(args, store) - return updateMRReviewers( - repo.path, - args.iid, - args.reviewerIds, - repo.issueSourcePreference, - repoConnectionId(repo), - args.projectRef, - ...localGitOptionArgs(store, repo) - ) - } - ) - - ipcMain.handle( - 'gitlab:addMRComment', - async (_event, args: GitLabRepoSelectorArgs & { iid: number; body: string }) => { - const repo = assertRegisteredRepo(args, store) - return addMRComment( - repo.path, - args.iid, - args.body, - repo.issueSourcePreference, - repoConnectionId(repo), - undefined, - ...localGitOptionArgs(store, repo) - ) - } - ) - - ipcMain.handle( - 'gitlab:addMRInlineComment', - async ( - _event, - args: { - repoPath: string - repoId?: string | null - sourceContext?: TaskSourceContext | null - iid: number - input: GitLabMRInlineCommentInput - projectRef?: ProjectRef | null - } - ) => { - const repo = assertRegisteredRepo(args, store) - return addMRInlineComment( - repo.path, - args.iid, - args.input, - repo.issueSourcePreference, - repoConnectionId(repo), - args.projectRef, - ...localGitOptionArgs(store, repo) - ) - } - ) - - ipcMain.handle( - 'gitlab:resolveMRDiscussion', - async ( - _event, - args: GitLabRepoSelectorArgs & { iid: number; discussionId: string; resolved: boolean } - ) => { - const repo = assertRegisteredRepo(args, store) - return resolveMRDiscussion( - repo.path, - args.iid, - args.discussionId, - args.resolved, - repo.issueSourcePreference, - repoConnectionId(repo), - undefined, - ...localGitOptionArgs(store, repo) - ) - } - ) - - ipcMain.handle( - 'gitlab:jobTrace', - async ( - _event, - args: GitLabRepoSelectorArgs & { - jobId: number - projectRef?: ProjectRef | null - logExcerpt?: boolean - } - ) => { - const repo = assertRegisteredRepo(args, store) - const result = await getJobTrace( - repo.path, - args.jobId, - repo.issueSourcePreference, - repoConnectionId(repo), - args.projectRef, - ...localGitOptionArgs(store, repo) - ) - return args.logExcerpt ? toGitLabJobLogExcerptResult(result) : result - } - ) - - ipcMain.handle( - 'gitlab:retryJob', - async ( - _event, - args: GitLabRepoSelectorArgs & { jobId: number; projectRef?: ProjectRef | null } - ) => { - const repo = assertRegisteredRepo(args, store) - return retryJob( - repo.path, - args.jobId, - repo.issueSourcePreference, - repoConnectionId(repo), - args.projectRef, - ...localGitOptionArgs(store, repo) - ) - } - ) + registerGitLabMergeRequestQueryHandlers(store) + registerGitLabIssueHandlers(store) + registerGitLabWorkItemHandlers(store) + registerGitLabMergeRequestMutationHandlers(store) + registerGitLabCiJobHandlers(store) // Why: My Todos surface — cross-project, user-scoped. The repoPath is // only used for the registered-repo guard; `glab api todos` doesn't @@ -565,39 +51,4 @@ export function registerGitLabHandlers(store: Store): void { const repo = assertRegisteredRepo(args, store) return listTodos(repo.path, repoConnectionId(repo), ...localGitOptionArgs(store, repo)) }) - - // Why: paste-URL flow in the picker. The user pastes a GitLab URL that - // may target a project different from the local checkout's remote, so - // the call carries the parsed project path explicitly rather than - // resolving from cwd. - ipcMain.handle( - 'gitlab:workItemByPath', - async ( - _event, - args: GitLabRepoSelectorArgs & { - host: string - path: string - iid: number - type: 'issue' | 'mr' - } - ) => { - const repo = assertRegisteredRepo(args, store) - const projectRef: ProjectRef = { host: args.host, path: args.path } - const result = await getWorkItemByProjectRef( - repo.path, - projectRef, - args.iid, - args.type, - repoConnectionId(repo), - ...localGitOptionArgs(store, repo) - ) - // Why: only persist a recent entry when the lookup actually - // produced an item. A 404 / auth failure shouldn't pollute the - // user's recents list with project paths they can't read. - if (result) { - recordGitLabProjectRecent(store, args.host, args.path) - } - return result - } - ) } diff --git a/src/main/ipc/linear-custom-view-handlers.ts b/src/main/ipc/linear-custom-view-handlers.ts new file mode 100644 index 00000000000..623e7958ce5 --- /dev/null +++ b/src/main/ipc/linear-custom-view-handlers.ts @@ -0,0 +1,99 @@ +import { ipcMain } from 'electron' +import { + getCustomView, + listCustomViewIssues, + listCustomViewProjects, + listCustomViews +} from '../linear/projects' +import { + normalizeConcreteWorkspaceId, + normalizeCustomViewModel, + normalizeWorkspaceSelection +} from './linear-ipc-args' +import { clampLinearIssueListLimit } from '../../shared/linear/issue-read-limits' +import type { LinearCustomViewModel } from '../../shared/linear/project-types' +import type { LinearWorkspaceSelection } from '../../shared/linear/workspace-types' + +export function registerLinearCustomViewHandlers(): void { + ipcMain.handle( + 'linear:listCustomViews', + async ( + _event, + args?: { + model?: LinearCustomViewModel + limit?: number + workspaceId?: LinearWorkspaceSelection + force?: boolean + } + ) => { + const limit = Math.min(Math.max(1, args?.limit ?? 20), 50) + return listCustomViews( + normalizeCustomViewModel(args?.model), + limit, + normalizeWorkspaceSelection(args?.workspaceId), + args?.force === true + ) + } + ) + + ipcMain.handle( + 'linear:getCustomView', + async ( + _event, + args: { + viewId: string + model?: LinearCustomViewModel + workspaceId?: string + force?: boolean + } + ) => { + if (typeof args?.viewId !== 'string' || !args.viewId.trim()) { + throw new Error('Custom view ID is required') + } + return getCustomView( + args.viewId.trim(), + normalizeCustomViewModel(args.model), + normalizeConcreteWorkspaceId(args.workspaceId), + args.force === true + ) + } + ) + + ipcMain.handle( + 'linear:listCustomViewIssues', + async ( + _event, + args: { viewId: string; limit?: number; workspaceId?: string; force?: boolean } + ) => { + if (typeof args?.viewId !== 'string' || !args.viewId.trim()) { + throw new Error('Custom view ID is required') + } + const limit = clampLinearIssueListLimit(args?.limit) + return listCustomViewIssues( + args.viewId.trim(), + limit, + normalizeConcreteWorkspaceId(args.workspaceId), + args.force === true + ) + } + ) + + ipcMain.handle( + 'linear:listCustomViewProjects', + async ( + _event, + args: { viewId: string; limit?: number; workspaceId?: string; force?: boolean } + ) => { + if (typeof args?.viewId !== 'string' || !args.viewId.trim()) { + throw new Error('Custom view ID is required') + } + const limit = Math.min(Math.max(1, args?.limit ?? 20), 50) + return listCustomViewProjects( + args.viewId.trim(), + limit, + normalizeConcreteWorkspaceId(args.workspaceId), + args.force === true + ) + } + ) +} diff --git a/src/main/ipc/linear-ipc-args.ts b/src/main/ipc/linear-ipc-args.ts new file mode 100644 index 00000000000..c7654c8ffe0 --- /dev/null +++ b/src/main/ipc/linear-ipc-args.ts @@ -0,0 +1,49 @@ +import type { LinearCustomViewModel } from '../../shared/linear/project-types' +import type { LinearWorkspaceSelection } from '../../shared/linear/workspace-types' + +export function normalizeWorkspaceId(value: unknown): string | undefined { + return typeof value === 'string' && value.trim() ? value.trim() : undefined +} + +export function normalizeWorkspaceSelection(value: unknown): LinearWorkspaceSelection | undefined { + const workspaceId = normalizeWorkspaceId(value) + return workspaceId as LinearWorkspaceSelection | undefined +} + +export function normalizeConcreteWorkspaceId(value: unknown): string { + const workspaceId = normalizeWorkspaceId(value) + if (!workspaceId || workspaceId === 'all') { + throw new Error('Concrete Linear workspace ID is required') + } + return workspaceId +} + +export function normalizeCustomViewModel(value: unknown): LinearCustomViewModel { + if (value !== 'issue' && value !== 'project') { + throw new Error('Custom view model is required') + } + return value +} + +export function normalizeIdList(value: unknown, fieldName: string): string[] | undefined { + if (value === undefined) { + return undefined + } + if ( + !Array.isArray(value) || + !value.every((id): id is string => typeof id === 'string' && Boolean(id.trim())) + ) { + throw new Error(`Invalid ${fieldName}`) + } + return value.map((id) => id.trim()) +} + +export function normalizeOptionalDate(value: unknown, fieldName: string): string | undefined { + if (value === undefined || value === null || value === '') { + return undefined + } + if (typeof value !== 'string' || !/^\d{4}-\d{2}-\d{2}$/.test(value.trim())) { + throw new Error(`Invalid ${fieldName}`) + } + return value.trim() +} diff --git a/src/main/ipc/linear-issue-handlers.ts b/src/main/ipc/linear-issue-handlers.ts new file mode 100644 index 00000000000..c358746c6f5 --- /dev/null +++ b/src/main/ipc/linear-issue-handlers.ts @@ -0,0 +1,200 @@ +import { ipcMain } from 'electron' +import { + getIssue, + searchIssues, + listIssues, + createIssue, + updateIssue, + addIssueComment, + getIssueComments +} from '../linear/issues' +import { normalizeWorkspaceId, normalizeWorkspaceSelection } from './linear-ipc-args' +import type { LinearListFilter } from '../linear/issues' +import { clampLinearIssueListLimit } from '../../shared/linear/issue-read-limits' +import { optionalParsedLinearIssueAttributeFilter } from '../../shared/linear/issue-attribute-filter' +import type { LinearIssueUpdate } from '../../shared/issue-mutation-types' +import type { LinearWorkspaceSelection } from '../../shared/linear/workspace-types' + +const VALID_FILTERS = new Set(['assigned', 'created', 'all', 'completed']) + +export function registerLinearIssueHandlers(): void { + ipcMain.handle( + 'linear:searchIssues', + async ( + _event, + args: { query: string; limit?: number; workspaceId?: LinearWorkspaceSelection } + ) => { + if (typeof args?.query !== 'string') { + return [] + } + const limit = Math.min(Math.max(1, args.limit ?? 20), 50) + return searchIssues(args.query, limit, normalizeWorkspaceSelection(args.workspaceId)) + } + ) + + ipcMain.handle( + 'linear:listIssues', + async ( + _event, + args?: { + filter?: LinearListFilter + limit?: number + workspaceId?: LinearWorkspaceSelection + attributeFilter?: unknown + } + ) => { + const filter = VALID_FILTERS.has(args?.filter as LinearListFilter) + ? (args!.filter as LinearListFilter) + : undefined + const limit = clampLinearIssueListLimit(args?.limit) + // Why: reject malformed filters at the trust boundary instead of + // normalizing them to empty (which would silently broaden results). + const attributeFilter = + args && 'attributeFilter' in args && args.attributeFilter !== undefined + ? optionalParsedLinearIssueAttributeFilter(args.attributeFilter) + : undefined + return listIssues(filter, limit, normalizeWorkspaceSelection(args?.workspaceId), { + attributeFilter + }) + } + ) + + ipcMain.handle( + 'linear:createIssue', + async ( + _event, + args: { + teamId: string + title: string + description?: string + workspaceId?: string + parentIssueId?: string + projectId?: string | null + stateId?: string + priority?: number + assigneeId?: string | null + labelIds?: string[] + } + ) => { + if (typeof args?.teamId !== 'string' || !args.teamId.trim()) { + return { ok: false, error: 'Team ID is required' } + } + if (typeof args?.title !== 'string' || !args.title.trim()) { + return { ok: false, error: 'Title is required' } + } + if ( + args.priority !== undefined && + (!Number.isInteger(args.priority) || args.priority < 0 || args.priority > 4) + ) { + return { ok: false, error: 'Invalid priority' } + } + if ( + args.labelIds !== undefined && + (!Array.isArray(args.labelIds) || + !args.labelIds.every((id) => typeof id === 'string' && id.trim())) + ) { + return { ok: false, error: 'Invalid label IDs' } + } + return createIssue( + args.teamId.trim(), + args.title.trim(), + args.description?.trim() || undefined, + normalizeWorkspaceId(args.workspaceId), + { + parentId: typeof args.parentIssueId === 'string' ? args.parentIssueId.trim() : undefined, + projectId: typeof args.projectId === 'string' ? args.projectId.trim() : null, + stateId: typeof args.stateId === 'string' ? args.stateId.trim() : undefined, + priority: typeof args.priority === 'number' ? args.priority : undefined, + assigneeId: typeof args.assigneeId === 'string' ? args.assigneeId.trim() : null, + labelIds: Array.isArray(args.labelIds) ? args.labelIds.map((id) => id.trim()) : undefined + } + ) + } + ) + + ipcMain.handle('linear:getIssue', async (_event, args: { id: string; workspaceId?: string }) => { + if (typeof args?.id !== 'string' || !args.id.trim()) { + return null + } + return getIssue(args.id.trim(), normalizeWorkspaceId(args.workspaceId)) + }) + + ipcMain.handle( + 'linear:updateIssue', + async (_event, args: { id: string; updates: LinearIssueUpdate; workspaceId?: string }) => { + if (typeof args?.id !== 'string' || !args.id.trim()) { + return { ok: false, error: 'Issue ID is required' } + } + // Why: IPC args are untyped at runtime — validate the updates object and + // individual fields to prevent the Linear SDK from receiving unexpected + // primitives that would produce confusing API errors. + if (!args.updates || typeof args.updates !== 'object') { + return { ok: false, error: 'Updates object is required' } + } + const u = args.updates + if (u.stateId !== undefined && (typeof u.stateId !== 'string' || !u.stateId.trim())) { + return { ok: false, error: 'Invalid state ID' } + } + if (u.title !== undefined && (typeof u.title !== 'string' || !u.title.trim())) { + return { ok: false, error: 'Title is required' } + } + if (u.description !== undefined && typeof u.description !== 'string') { + return { ok: false, error: 'Description must be a string' } + } + if ( + u.priority !== undefined && + (!Number.isInteger(u.priority) || u.priority < 0 || u.priority > 4) + ) { + return { ok: false, error: 'Priority must be an integer 0-4' } + } + if ( + u.estimate !== undefined && + u.estimate !== null && + (!Number.isInteger(u.estimate) || u.estimate < 0) + ) { + return { ok: false, error: 'Estimate must be a non-negative integer' } + } + if ( + u.labelIds !== undefined && + (!Array.isArray(u.labelIds) || !u.labelIds.every((id: unknown) => typeof id === 'string')) + ) { + return { ok: false, error: 'Label IDs must be an array of strings' } + } + if ( + u.projectId !== undefined && + u.projectId !== null && + (typeof u.projectId !== 'string' || !u.projectId.trim()) + ) { + return { ok: false, error: 'Invalid project ID' } + } + return updateIssue(args.id.trim(), args.updates, normalizeWorkspaceId(args.workspaceId)) + } + ) + + ipcMain.handle( + 'linear:addIssueComment', + async (_event, args: { issueId: string; body: string; workspaceId?: string }) => { + if (typeof args?.issueId !== 'string' || !args.issueId.trim()) { + return { ok: false, error: 'Issue ID is required' } + } + if (!args.body?.trim()) { + return { ok: false, error: 'Comment body is required' } + } + return addIssueComment( + args.issueId.trim(), + args.body.trim(), + normalizeWorkspaceId(args.workspaceId) + ) + } + ) + + ipcMain.handle( + 'linear:issueComments', + async (_event, args: { issueId: string; workspaceId?: string }) => { + if (typeof args?.issueId !== 'string' || !args.issueId.trim()) { + return [] + } + return getIssueComments(args.issueId.trim(), normalizeWorkspaceId(args.workspaceId)) + } + ) +} diff --git a/src/main/ipc/linear-project-handlers.ts b/src/main/ipc/linear-project-handlers.ts new file mode 100644 index 00000000000..3c683ff3892 --- /dev/null +++ b/src/main/ipc/linear-project-handlers.ts @@ -0,0 +1,133 @@ +import { ipcMain } from 'electron' +import { createProject, getProject, listProjectIssues, listProjects } from '../linear/projects' +import { + normalizeConcreteWorkspaceId, + normalizeIdList, + normalizeOptionalDate, + normalizeWorkspaceId, + normalizeWorkspaceSelection +} from './linear-ipc-args' +import { clampLinearIssueListLimit } from '../../shared/linear/issue-read-limits' +import type { LinearWorkspaceSelection } from '../../shared/linear/workspace-types' + +export function registerLinearProjectHandlers(): void { + ipcMain.handle( + 'linear:listProjects', + async ( + _event, + args?: { + query?: string + limit?: number + workspaceId?: LinearWorkspaceSelection + force?: boolean + } + ) => { + const limit = Math.min(Math.max(1, args?.limit ?? 20), 50) + return listProjects( + args?.query, + limit, + normalizeWorkspaceSelection(args?.workspaceId), + args?.force === true + ) + } + ) + + ipcMain.handle( + 'linear:createProject', + async ( + _event, + args: { + name: string + description?: string + content?: string + teamIds?: string[] + leadId?: string | null + memberIds?: string[] + labelIds?: string[] + priority?: number + startDate?: string + targetDate?: string + workspaceId?: string + } + ) => { + if (typeof args?.name !== 'string' || !args.name.trim()) { + return { ok: false, error: 'Project name is required' } + } + let teamIds: string[] + try { + teamIds = normalizeIdList(args.teamIds, 'team IDs') ?? [] + } catch (error) { + return { ok: false, error: error instanceof Error ? error.message : 'Invalid team IDs' } + } + if (teamIds.length === 0) { + return { ok: false, error: 'At least one team is required' } + } + if ( + args.priority !== undefined && + (!Number.isInteger(args.priority) || args.priority < 0 || args.priority > 4) + ) { + return { ok: false, error: 'Invalid priority' } + } + let memberIds: string[] | undefined + let labelIds: string[] | undefined + let startDate: string | undefined + let targetDate: string | undefined + try { + memberIds = normalizeIdList(args.memberIds, 'member IDs') + labelIds = normalizeIdList(args.labelIds, 'label IDs') + startDate = normalizeOptionalDate(args.startDate, 'start date') + targetDate = normalizeOptionalDate(args.targetDate, 'target date') + } catch (error) { + return { ok: false, error: error instanceof Error ? error.message : 'Invalid project' } + } + return createProject( + { + name: args.name.trim(), + description: args.description?.trim() || undefined, + content: args.content?.trim() || undefined, + teamIds, + leadId: normalizeWorkspaceId(args.leadId), + memberIds, + labelIds, + priority: typeof args.priority === 'number' ? args.priority : undefined, + startDate, + targetDate + }, + normalizeWorkspaceId(args.workspaceId) + ) + } + ) + + ipcMain.handle( + 'linear:getProject', + async (_event, args: { id: string; workspaceId?: string; force?: boolean }) => { + if (typeof args?.id !== 'string' || !args.id.trim()) { + throw new Error('Project ID is required') + } + return getProject( + args.id.trim(), + normalizeConcreteWorkspaceId(args.workspaceId), + args.force === true + ) + } + ) + + ipcMain.handle( + 'linear:listProjectIssues', + async ( + _event, + args: { projectId: string; limit?: number; workspaceId?: string; force?: boolean } + ) => { + if (typeof args?.projectId !== 'string' || !args.projectId.trim()) { + throw new Error('Project ID is required') + } + const limit = clampLinearIssueListLimit(args?.limit) + return listProjectIssues( + args.projectId.trim(), + limit, + normalizeConcreteWorkspaceId(args.workspaceId), + args.force === true + ) + } + ) +} diff --git a/src/main/ipc/linear-team-handlers.ts b/src/main/ipc/linear-team-handlers.ts new file mode 100644 index 00000000000..dc37ebad6d3 --- /dev/null +++ b/src/main/ipc/linear-team-handlers.ts @@ -0,0 +1,43 @@ +import { ipcMain } from 'electron' +import { listTeams, getTeamStates, getTeamLabels, getTeamMembers } from '../linear/teams' +import { normalizeWorkspaceId, normalizeWorkspaceSelection } from './linear-ipc-args' +import type { LinearWorkspaceSelection } from '../../shared/linear/workspace-types' + +export function registerLinearTeamHandlers(): void { + ipcMain.handle( + 'linear:listTeams', + async (_event, args?: { workspaceId?: LinearWorkspaceSelection }) => { + return listTeams(normalizeWorkspaceSelection(args?.workspaceId)) + } + ) + + ipcMain.handle( + 'linear:teamStates', + async (_event, args: { teamId: string; workspaceId?: string }) => { + if (typeof args?.teamId !== 'string' || !args.teamId.trim()) { + return [] + } + return getTeamStates(args.teamId.trim(), normalizeWorkspaceId(args.workspaceId)) + } + ) + + ipcMain.handle( + 'linear:teamLabels', + async (_event, args: { teamId: string; workspaceId?: string }) => { + if (typeof args?.teamId !== 'string' || !args.teamId.trim()) { + return [] + } + return getTeamLabels(args.teamId.trim(), normalizeWorkspaceId(args.workspaceId)) + } + ) + + ipcMain.handle( + 'linear:teamMembers', + async (_event, args: { teamId: string; workspaceId?: string }) => { + if (typeof args?.teamId !== 'string' || !args.teamId.trim()) { + return [] + } + return getTeamMembers(args.teamId.trim(), normalizeWorkspaceId(args.workspaceId)) + } + ) +} diff --git a/src/main/ipc/linear.ts b/src/main/ipc/linear.ts index dd64a777ab4..b3e61d96d10 100644 --- a/src/main/ipc/linear.ts +++ b/src/main/ipc/linear.ts @@ -1,83 +1,11 @@ -/* eslint-disable max-lines -- Why: Linear IPC validates one namespace in one - registration boundary so local and SSH runtime schemas can stay mirrored. */ import { ipcMain } from 'electron' import { connect, disconnect, getStatus, selectWorkspace, testConnection } from '../linear/client' import { _resetPreflightCache } from './preflight' -import { - getIssue, - searchIssues, - listIssues, - createIssue, - updateIssue, - addIssueComment, - getIssueComments -} from '../linear/issues' -import { - createProject, - getCustomView, - getProject, - listCustomViewIssues, - listCustomViewProjects, - listCustomViews, - listProjectIssues, - listProjects -} from '../linear/projects' -import { listTeams, getTeamStates, getTeamLabels, getTeamMembers } from '../linear/teams' -import type { LinearListFilter } from '../linear/issues' -import { clampLinearIssueListLimit } from '../../shared/linear/issue-read-limits' -import { optionalParsedLinearIssueAttributeFilter } from '../../shared/linear/issue-attribute-filter' -import type { LinearIssueUpdate } from '../../shared/issue-mutation-types' -import type { LinearCustomViewModel } from '../../shared/linear/project-types' -import type { LinearWorkspaceSelection } from '../../shared/linear/workspace-types' - -const VALID_FILTERS = new Set(['assigned', 'created', 'all', 'completed']) - -function normalizeWorkspaceId(value: unknown): string | undefined { - return typeof value === 'string' && value.trim() ? value.trim() : undefined -} - -function normalizeWorkspaceSelection(value: unknown): LinearWorkspaceSelection | undefined { - const workspaceId = normalizeWorkspaceId(value) - return workspaceId as LinearWorkspaceSelection | undefined -} - -function normalizeConcreteWorkspaceId(value: unknown): string { - const workspaceId = normalizeWorkspaceId(value) - if (!workspaceId || workspaceId === 'all') { - throw new Error('Concrete Linear workspace ID is required') - } - return workspaceId -} - -function normalizeCustomViewModel(value: unknown): LinearCustomViewModel { - if (value !== 'issue' && value !== 'project') { - throw new Error('Custom view model is required') - } - return value -} - -function normalizeIdList(value: unknown, fieldName: string): string[] | undefined { - if (value === undefined) { - return undefined - } - if ( - !Array.isArray(value) || - !value.every((id): id is string => typeof id === 'string' && Boolean(id.trim())) - ) { - throw new Error(`Invalid ${fieldName}`) - } - return value.map((id) => id.trim()) -} - -function normalizeOptionalDate(value: unknown, fieldName: string): string | undefined { - if (value === undefined || value === null || value === '') { - return undefined - } - if (typeof value !== 'string' || !/^\d{4}-\d{2}-\d{2}$/.test(value.trim())) { - throw new Error(`Invalid ${fieldName}`) - } - return value.trim() -} +import { normalizeWorkspaceId, normalizeWorkspaceSelection } from './linear-ipc-args' +import { registerLinearIssueHandlers } from './linear-issue-handlers' +import { registerLinearProjectHandlers } from './linear-project-handlers' +import { registerLinearCustomViewHandlers } from './linear-custom-view-handlers' +import { registerLinearTeamHandlers } from './linear-team-handlers' export function registerLinearHandlers(): void { ipcMain.handle('linear:connect', async (_event, args: { apiKey: string }) => { @@ -112,422 +40,8 @@ export function registerLinearHandlers(): void { return testConnection(normalizeWorkspaceId(args?.workspaceId)) }) - ipcMain.handle( - 'linear:searchIssues', - async ( - _event, - args: { query: string; limit?: number; workspaceId?: LinearWorkspaceSelection } - ) => { - if (typeof args?.query !== 'string') { - return [] - } - const limit = Math.min(Math.max(1, args.limit ?? 20), 50) - return searchIssues(args.query, limit, normalizeWorkspaceSelection(args.workspaceId)) - } - ) - - ipcMain.handle( - 'linear:listIssues', - async ( - _event, - args?: { - filter?: LinearListFilter - limit?: number - workspaceId?: LinearWorkspaceSelection - attributeFilter?: unknown - } - ) => { - const filter = VALID_FILTERS.has(args?.filter as LinearListFilter) - ? (args!.filter as LinearListFilter) - : undefined - const limit = clampLinearIssueListLimit(args?.limit) - // Why: reject malformed filters at the trust boundary instead of - // normalizing them to empty (which would silently broaden results). - const attributeFilter = - args && 'attributeFilter' in args && args.attributeFilter !== undefined - ? optionalParsedLinearIssueAttributeFilter(args.attributeFilter) - : undefined - return listIssues(filter, limit, normalizeWorkspaceSelection(args?.workspaceId), { - attributeFilter - }) - } - ) - - ipcMain.handle( - 'linear:createIssue', - async ( - _event, - args: { - teamId: string - title: string - description?: string - workspaceId?: string - parentIssueId?: string - projectId?: string | null - stateId?: string - priority?: number - assigneeId?: string | null - labelIds?: string[] - } - ) => { - if (typeof args?.teamId !== 'string' || !args.teamId.trim()) { - return { ok: false, error: 'Team ID is required' } - } - if (typeof args?.title !== 'string' || !args.title.trim()) { - return { ok: false, error: 'Title is required' } - } - if ( - args.priority !== undefined && - (!Number.isInteger(args.priority) || args.priority < 0 || args.priority > 4) - ) { - return { ok: false, error: 'Invalid priority' } - } - if ( - args.labelIds !== undefined && - (!Array.isArray(args.labelIds) || - !args.labelIds.every((id) => typeof id === 'string' && id.trim())) - ) { - return { ok: false, error: 'Invalid label IDs' } - } - return createIssue( - args.teamId.trim(), - args.title.trim(), - args.description?.trim() || undefined, - normalizeWorkspaceId(args.workspaceId), - { - parentId: typeof args.parentIssueId === 'string' ? args.parentIssueId.trim() : undefined, - projectId: typeof args.projectId === 'string' ? args.projectId.trim() : null, - stateId: typeof args.stateId === 'string' ? args.stateId.trim() : undefined, - priority: typeof args.priority === 'number' ? args.priority : undefined, - assigneeId: typeof args.assigneeId === 'string' ? args.assigneeId.trim() : null, - labelIds: Array.isArray(args.labelIds) ? args.labelIds.map((id) => id.trim()) : undefined - } - ) - } - ) - - ipcMain.handle('linear:getIssue', async (_event, args: { id: string; workspaceId?: string }) => { - if (typeof args?.id !== 'string' || !args.id.trim()) { - return null - } - return getIssue(args.id.trim(), normalizeWorkspaceId(args.workspaceId)) - }) - - ipcMain.handle( - 'linear:updateIssue', - async (_event, args: { id: string; updates: LinearIssueUpdate; workspaceId?: string }) => { - if (typeof args?.id !== 'string' || !args.id.trim()) { - return { ok: false, error: 'Issue ID is required' } - } - // Why: IPC args are untyped at runtime — validate the updates object and - // individual fields to prevent the Linear SDK from receiving unexpected - // primitives that would produce confusing API errors. - if (!args.updates || typeof args.updates !== 'object') { - return { ok: false, error: 'Updates object is required' } - } - const u = args.updates - if (u.stateId !== undefined && (typeof u.stateId !== 'string' || !u.stateId.trim())) { - return { ok: false, error: 'Invalid state ID' } - } - if (u.title !== undefined && (typeof u.title !== 'string' || !u.title.trim())) { - return { ok: false, error: 'Title is required' } - } - if (u.description !== undefined && typeof u.description !== 'string') { - return { ok: false, error: 'Description must be a string' } - } - if ( - u.priority !== undefined && - (!Number.isInteger(u.priority) || u.priority < 0 || u.priority > 4) - ) { - return { ok: false, error: 'Priority must be an integer 0-4' } - } - if ( - u.estimate !== undefined && - u.estimate !== null && - (!Number.isInteger(u.estimate) || u.estimate < 0) - ) { - return { ok: false, error: 'Estimate must be a non-negative integer' } - } - if ( - u.labelIds !== undefined && - (!Array.isArray(u.labelIds) || !u.labelIds.every((id: unknown) => typeof id === 'string')) - ) { - return { ok: false, error: 'Label IDs must be an array of strings' } - } - if ( - u.projectId !== undefined && - u.projectId !== null && - (typeof u.projectId !== 'string' || !u.projectId.trim()) - ) { - return { ok: false, error: 'Invalid project ID' } - } - return updateIssue(args.id.trim(), args.updates, normalizeWorkspaceId(args.workspaceId)) - } - ) - - ipcMain.handle( - 'linear:addIssueComment', - async (_event, args: { issueId: string; body: string; workspaceId?: string }) => { - if (typeof args?.issueId !== 'string' || !args.issueId.trim()) { - return { ok: false, error: 'Issue ID is required' } - } - if (!args.body?.trim()) { - return { ok: false, error: 'Comment body is required' } - } - return addIssueComment( - args.issueId.trim(), - args.body.trim(), - normalizeWorkspaceId(args.workspaceId) - ) - } - ) - - ipcMain.handle( - 'linear:issueComments', - async (_event, args: { issueId: string; workspaceId?: string }) => { - if (typeof args?.issueId !== 'string' || !args.issueId.trim()) { - return [] - } - return getIssueComments(args.issueId.trim(), normalizeWorkspaceId(args.workspaceId)) - } - ) - - ipcMain.handle( - 'linear:listTeams', - async (_event, args?: { workspaceId?: LinearWorkspaceSelection }) => { - return listTeams(normalizeWorkspaceSelection(args?.workspaceId)) - } - ) - - ipcMain.handle( - 'linear:listProjects', - async ( - _event, - args?: { - query?: string - limit?: number - workspaceId?: LinearWorkspaceSelection - force?: boolean - } - ) => { - const limit = Math.min(Math.max(1, args?.limit ?? 20), 50) - return listProjects( - args?.query, - limit, - normalizeWorkspaceSelection(args?.workspaceId), - args?.force === true - ) - } - ) - - ipcMain.handle( - 'linear:createProject', - async ( - _event, - args: { - name: string - description?: string - content?: string - teamIds?: string[] - leadId?: string | null - memberIds?: string[] - labelIds?: string[] - priority?: number - startDate?: string - targetDate?: string - workspaceId?: string - } - ) => { - if (typeof args?.name !== 'string' || !args.name.trim()) { - return { ok: false, error: 'Project name is required' } - } - let teamIds: string[] - try { - teamIds = normalizeIdList(args.teamIds, 'team IDs') ?? [] - } catch (error) { - return { ok: false, error: error instanceof Error ? error.message : 'Invalid team IDs' } - } - if (teamIds.length === 0) { - return { ok: false, error: 'At least one team is required' } - } - if ( - args.priority !== undefined && - (!Number.isInteger(args.priority) || args.priority < 0 || args.priority > 4) - ) { - return { ok: false, error: 'Invalid priority' } - } - let memberIds: string[] | undefined - let labelIds: string[] | undefined - let startDate: string | undefined - let targetDate: string | undefined - try { - memberIds = normalizeIdList(args.memberIds, 'member IDs') - labelIds = normalizeIdList(args.labelIds, 'label IDs') - startDate = normalizeOptionalDate(args.startDate, 'start date') - targetDate = normalizeOptionalDate(args.targetDate, 'target date') - } catch (error) { - return { ok: false, error: error instanceof Error ? error.message : 'Invalid project' } - } - return createProject( - { - name: args.name.trim(), - description: args.description?.trim() || undefined, - content: args.content?.trim() || undefined, - teamIds, - leadId: normalizeWorkspaceId(args.leadId), - memberIds, - labelIds, - priority: typeof args.priority === 'number' ? args.priority : undefined, - startDate, - targetDate - }, - normalizeWorkspaceId(args.workspaceId) - ) - } - ) - - ipcMain.handle( - 'linear:getProject', - async (_event, args: { id: string; workspaceId?: string; force?: boolean }) => { - if (typeof args?.id !== 'string' || !args.id.trim()) { - throw new Error('Project ID is required') - } - return getProject( - args.id.trim(), - normalizeConcreteWorkspaceId(args.workspaceId), - args.force === true - ) - } - ) - - ipcMain.handle( - 'linear:listProjectIssues', - async ( - _event, - args: { projectId: string; limit?: number; workspaceId?: string; force?: boolean } - ) => { - if (typeof args?.projectId !== 'string' || !args.projectId.trim()) { - throw new Error('Project ID is required') - } - const limit = clampLinearIssueListLimit(args?.limit) - return listProjectIssues( - args.projectId.trim(), - limit, - normalizeConcreteWorkspaceId(args.workspaceId), - args.force === true - ) - } - ) - - ipcMain.handle( - 'linear:listCustomViews', - async ( - _event, - args?: { - model?: LinearCustomViewModel - limit?: number - workspaceId?: LinearWorkspaceSelection - force?: boolean - } - ) => { - const limit = Math.min(Math.max(1, args?.limit ?? 20), 50) - return listCustomViews( - normalizeCustomViewModel(args?.model), - limit, - normalizeWorkspaceSelection(args?.workspaceId), - args?.force === true - ) - } - ) - - ipcMain.handle( - 'linear:getCustomView', - async ( - _event, - args: { - viewId: string - model?: LinearCustomViewModel - workspaceId?: string - force?: boolean - } - ) => { - if (typeof args?.viewId !== 'string' || !args.viewId.trim()) { - throw new Error('Custom view ID is required') - } - return getCustomView( - args.viewId.trim(), - normalizeCustomViewModel(args.model), - normalizeConcreteWorkspaceId(args.workspaceId), - args.force === true - ) - } - ) - - ipcMain.handle( - 'linear:listCustomViewIssues', - async ( - _event, - args: { viewId: string; limit?: number; workspaceId?: string; force?: boolean } - ) => { - if (typeof args?.viewId !== 'string' || !args.viewId.trim()) { - throw new Error('Custom view ID is required') - } - const limit = clampLinearIssueListLimit(args?.limit) - return listCustomViewIssues( - args.viewId.trim(), - limit, - normalizeConcreteWorkspaceId(args.workspaceId), - args.force === true - ) - } - ) - - ipcMain.handle( - 'linear:listCustomViewProjects', - async ( - _event, - args: { viewId: string; limit?: number; workspaceId?: string; force?: boolean } - ) => { - if (typeof args?.viewId !== 'string' || !args.viewId.trim()) { - throw new Error('Custom view ID is required') - } - const limit = Math.min(Math.max(1, args?.limit ?? 20), 50) - return listCustomViewProjects( - args.viewId.trim(), - limit, - normalizeConcreteWorkspaceId(args.workspaceId), - args.force === true - ) - } - ) - - ipcMain.handle( - 'linear:teamStates', - async (_event, args: { teamId: string; workspaceId?: string }) => { - if (typeof args?.teamId !== 'string' || !args.teamId.trim()) { - return [] - } - return getTeamStates(args.teamId.trim(), normalizeWorkspaceId(args.workspaceId)) - } - ) - - ipcMain.handle( - 'linear:teamLabels', - async (_event, args: { teamId: string; workspaceId?: string }) => { - if (typeof args?.teamId !== 'string' || !args.teamId.trim()) { - return [] - } - return getTeamLabels(args.teamId.trim(), normalizeWorkspaceId(args.workspaceId)) - } - ) - - ipcMain.handle( - 'linear:teamMembers', - async (_event, args: { teamId: string; workspaceId?: string }) => { - if (typeof args?.teamId !== 'string' || !args.teamId.trim()) { - return [] - } - return getTeamMembers(args.teamId.trim(), normalizeWorkspaceId(args.workspaceId)) - } - ) + registerLinearIssueHandlers() + registerLinearProjectHandlers() + registerLinearCustomViewHandlers() + registerLinearTeamHandlers() } diff --git a/src/main/jira/attachment-images.test.ts b/src/main/jira/attachment-images.test.ts index e160f69093b..efd3b8b42ec 100644 --- a/src/main/jira/attachment-images.test.ts +++ b/src/main/jira/attachment-images.test.ts @@ -1,11 +1,11 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -import type { JiraClientForSite } from './client' +import type { JiraClientForSite } from './authenticated-request' const { jiraRequestBinaryMock } = vi.hoisted(() => ({ jiraRequestBinaryMock: vi.fn() })) -vi.mock('./client', () => ({ +vi.mock('./authenticated-request', () => ({ jiraRequestBinary: (...args: unknown[]) => jiraRequestBinaryMock(...args), apiBasePath: (site: { authType?: string }) => site.authType === 'server' ? '/rest/api/2' : '/rest/api/3', diff --git a/src/main/jira/attachment-images.ts b/src/main/jira/attachment-images.ts index d1cfb2ab079..bc0f8f6f523 100644 --- a/src/main/jira/attachment-images.ts +++ b/src/main/jira/attachment-images.ts @@ -1,5 +1,5 @@ -import type { JiraClientForSite } from './client' -import { JiraApiError, apiBasePath, jiraRequestBinary } from './client' +import type { JiraClientForSite } from './authenticated-request' +import { JiraApiError, apiBasePath, jiraRequestBinary } from './authenticated-request' import type { JiraAdfMediaAttrs, JiraAdfMediaResolver } from './adf-markdown' import { escapeMarkdownAlt, unresolvedMediaPlaceholder } from './adf-markdown' import { escapeMarkdownLinkDestination } from './adf-media-destination' diff --git a/src/main/jira/authenticated-request.ts b/src/main/jira/authenticated-request.ts new file mode 100644 index 00000000000..6da47706476 --- /dev/null +++ b/src/main/jira/authenticated-request.ts @@ -0,0 +1,192 @@ +import { net, session } from 'electron' +import { ensureElectronProxyFromEnvironment } from '../network/proxy-settings' +import { withSpan } from '../observability/tracer' +import type { JiraAuthType, JiraSite } from '../../shared/jira-types' + +// Why: Atlassian's XSRF filter rejects POST/PUT REST calls that carry a browser +// User-Agent, failing them with "XSRF check failed" even under API-token auth. +// Electron's net.fetch sends a Chrome UA, so issue search/create/update/comment +// all 403'd while GET calls (connect, /myself) passed. A non-browser UA is the +// reliable fix; X-Atlassian-Token: no-check is not honored for this case. +const JIRA_API_USER_AGENT = 'Orca' + +export type JiraClientForSite = { + site: JiraSite + authorization: string +} + +// Self-hosted Jira Server/Data Center only exposes REST v2; Cloud endpoints +// in this codebase are written against v3. Callers build paths with this +// prefix so one code path serves both deployments. +export function apiBasePath(site: JiraSite): string { + return site.authType === 'server' ? '/rest/api/2' : '/rest/api/3' +} + +export class JiraApiError extends Error { + status: number | null + + constructor(message: string, status: number | null = null) { + super(message) + this.status = status + } +} + +export function authHeader(email: string, apiToken: string, authType?: JiraAuthType): string { + // Self-hosted with no username = a personal access token (Bearer); Basic auth + // with a PAT in the password slot is what produces the 401s users report. + // Self-hosted WITH a username is classic username+password Basic auth, which + // older Server/DC instances (predating PATs) require. Cloud is always Basic. + if (authType === 'server' && !email) { + return `Bearer ${apiToken}` + } + return `Basic ${Buffer.from(`${email}:${apiToken}`).toString('base64')}` +} + +function describeErrorCause(error: unknown): string | undefined { + if (!error || typeof error !== 'object' || !('cause' in error)) { + return undefined + } + const cause = (error as { cause?: unknown }).cause + if (cause instanceof Error) { + return `${cause.name}: ${cause.message}` + } + return cause === undefined ? undefined : String(cause) +} + +async function jiraFetch(url: string, init: RequestInit): Promise { + return withSpan( + 'jira.request', + async (span) => { + span.setAttribute('jira.siteUrl', new URL(url).origin) + await ensureElectronProxyFromEnvironment({ + proxySession: session.defaultSession, + probeUrl: url + }).catch((error) => { + span.addEvent('jira.proxySetupFailed', { + errorName: error instanceof Error ? error.name : typeof error, + errorMessage: error instanceof Error ? error.message : String(error) + }) + }) + try { + // Why: Electron's network stack follows Chromium proxy/session state, + // avoiding undici's stale keep-alive sockets after VPN path changes. + return await net.fetch(url, init) + } catch (error) { + span.setAttribute( + 'jira.transportErrorName', + error instanceof Error ? error.name : typeof error + ) + span.setAttribute( + 'jira.transportErrorMessage', + error instanceof Error ? error.message : String(error) + ) + const cause = describeErrorCause(error) + if (cause) { + span.setAttribute('jira.transportErrorCause', cause) + } + throw error + } + }, + { kind: 'client' } + ) +} + +export async function requestWithCredentials( + siteUrl: string, + email: string, + apiToken: string, + path: string, + init?: RequestInit, + authType?: JiraAuthType +): Promise { + const headers = new Headers(init?.headers) + headers.set('Accept', 'application/json') + headers.set('Content-Type', 'application/json') + headers.set('User-Agent', JIRA_API_USER_AGENT) + headers.set('Authorization', authHeader(email, apiToken, authType)) + const response = await jiraFetch(`${siteUrl}${path}`, { + ...init, + headers + }) + if (!response.ok) { + throw new JiraApiError(await readJiraError(response), response.status) + } + if (response.status === 204) { + return null + } + return response.json() +} + +async function readJiraError(response: Response): Promise { + try { + const data = (await response.json()) as { + errorMessages?: string[] + errors?: Record + message?: string + } + const messages = [ + ...(Array.isArray(data.errorMessages) ? data.errorMessages : []), + ...Object.values(data.errors ?? {}), + ...(data.message ? [data.message] : []) + ].filter(Boolean) + if (messages.length > 0) { + return messages.join('; ') + } + } catch { + // Fall through to status text. + } + return response.statusText || `Jira request failed (${response.status})` +} + +export async function jiraRequest( + client: JiraClientForSite, + path: string, + init?: RequestInit +): Promise { + const headers = new Headers(init?.headers) + headers.set('Accept', 'application/json') + headers.set('Content-Type', 'application/json') + headers.set('User-Agent', JIRA_API_USER_AGENT) + headers.set('Authorization', client.authorization) + const response = await jiraFetch(`${client.site.siteUrl}${path}`, { + ...init, + headers + }) + if (!response.ok) { + throw new JiraApiError(await readJiraError(response), response.status) + } + if (response.status === 204) { + return null as T + } + return (await response.json()) as T +} + +export async function jiraRequestBinary( + client: JiraClientForSite, + pathOrUrl: string +): Promise<{ data: ArrayBuffer; contentType: string }> { + const siteUrl = new URL(client.site.siteUrl) + const requestUrl = /^https?:\/\//i.test(pathOrUrl) + ? new URL(pathOrUrl) + : new URL(`${client.site.siteUrl}${pathOrUrl}`) + if (requestUrl.origin !== siteUrl.origin) { + // Why: attachment metadata is provider-controlled; never forward Jira + // credentials if a malformed response points at another origin. + throw new JiraApiError('Jira attachment URL must use the configured site origin.', null) + } + const headers = new Headers() + // Why: attachment content is binary; forcing JSON Accept/Content-Type can + // break downloads and confuses some Atlassian edge responses. + headers.set('Accept', '*/*') + headers.set('User-Agent', JIRA_API_USER_AGENT) + headers.set('Authorization', client.authorization) + const response = await jiraFetch(requestUrl.toString(), { headers }) + if (!response.ok) { + throw new JiraApiError(await readJiraError(response), response.status) + } + const contentType = response.headers.get('content-type') || 'application/octet-stream' + return { + data: await response.arrayBuffer(), + contentType + } +} diff --git a/src/main/jira/client.test.ts b/src/main/jira/client.test.ts index 9e0c8f4482c..faee0d2eaa4 100644 --- a/src/main/jira/client.test.ts +++ b/src/main/jira/client.test.ts @@ -111,7 +111,14 @@ async function loadClientModule(options: SafeStorageMockOptions = {}) { return { ...actual, homedir: () => tempHome } }) - return import('./client') + // One import call per reset so the split modules share a single graph (and + // thus one copy of the request queue / credential caches) per test. + const [client, queue, api] = await Promise.all([ + import('./client'), + import('./request-queue'), + import('./authenticated-request') + ]) + return { ...client, ...queue, ...api } } beforeEach(() => { diff --git a/src/main/jira/client.ts b/src/main/jira/client.ts index f9c4e8943c0..3e4dcaf6e92 100644 --- a/src/main/jira/client.ts +++ b/src/main/jira/client.ts @@ -1,18 +1,4 @@ -/* eslint-disable max-lines -- Why: Jira credential storage and authenticated -request plumbing share one boundary so encrypted token lifecycle and -multi-site selection cannot drift between task operations. */ -import { createHash } from 'node:crypto' -import { existsSync, mkdirSync, readFileSync, unlinkSync, writeFileSync } from 'node:fs' -import { homedir } from 'node:os' -import { join } from 'node:path' -import { net, safeStorage, session } from 'electron' -import { - CredentialDecryptionError, - credentialFileHasContent, - readStoredCredentialToken -} from '../integration-credential-file' -import { ensureElectronProxyFromEnvironment } from '../network/proxy-settings' -import { withSpan } from '../observability/tracer' +import { CredentialDecryptionError } from '../integration-credential-file' import type { JiraAuthType, JiraConnectArgs, @@ -22,504 +8,25 @@ import type { JiraViewer } from '../../shared/jira-types' import { clearAttachmentImagesForSite } from './attachment-image-cache' - -// Why: Atlassian's XSRF filter rejects POST/PUT REST calls that carry a browser -// User-Agent, failing them with "XSRF check failed" even under API-token auth. -// Electron's net.fetch sends a Chrome UA, so issue search/create/update/comment -// all 403'd while GET calls (connect, /myself) passed. A non-browser UA is the -// reliable fix; X-Atlassian-Token: no-check is not honored for this case. -const JIRA_API_USER_AGENT = 'Orca' - -const MAX_CONCURRENT = 4 -let running = 0 -type QueuedJiraRequest = { - resolve: () => void - reject: (error: Error) => void - signal?: AbortSignal - onAbort: () => void -} -const queue: QueuedJiraRequest[] = [] - -function createJiraRequestAbortError(): Error { - const error = new Error('Jira request aborted') - error.name = 'AbortError' - return error -} - -export function acquire(signal?: AbortSignal): Promise { - if (signal?.aborted) { - return Promise.reject(createJiraRequestAbortError()) - } - if (running < MAX_CONCURRENT) { - running += 1 - return Promise.resolve() - } - return new Promise((resolve, reject) => { - const entry: QueuedJiraRequest = { - resolve, - reject, - signal, - onAbort: () => { - const index = queue.indexOf(entry) - if (index === -1) { - return - } - queue.splice(index, 1) - reject(createJiraRequestAbortError()) - } - } - signal?.addEventListener('abort', entry.onAbort, { once: true }) - queue.push(entry) - }) -} - -export function release(): void { - running -= 1 - let next = queue.shift() - while (next) { - next.signal?.removeEventListener('abort', next.onAbort) - if (!next.signal?.aborted) { - running += 1 - next.resolve() - return - } - next.reject(createJiraRequestAbortError()) - next = queue.shift() - } -} - -type JiraSiteFile = { - version: 1 - activeSiteId: string | null - selectedSiteId: JiraSiteSelection | null - sites: JiraSite[] -} - -export type JiraClientForSite = { - site: JiraSite - authorization: string -} - -// Self-hosted Jira Server/Data Center only exposes REST v2; Cloud endpoints -// in this codebase are written against v3. Callers build paths with this -// prefix so one code path serves both deployments. -export function apiBasePath(site: JiraSite): string { - return site.authType === 'server' ? '/rest/api/2' : '/rest/api/3' -} - -export class JiraApiError extends Error { - status: number | null - - constructor(message: string, status: number | null = null) { - super(message) - this.status = status - } -} - -let cachedSiteFile: JiraSiteFile | null = null -let siteFileLoaded = false -const cachedTokens = new Map() -// Why: decrypt failures are recorded per site so getStatus can explain -// failing reads without re-touching the keychain on every status poll. -const credentialErrors = new Map() - -function getOrcaDir(): string { - return join(homedir(), '.orca') -} - -function getSiteFilePath(): string { - return join(getOrcaDir(), 'jira-sites.json') -} - -function getTokenDir(): string { - return join(getOrcaDir(), 'jira-tokens') -} - -function getTokenPath(siteId: string): string { - return join(getTokenDir(), `${Buffer.from(siteId).toString('base64url')}.enc`) -} - -function ensureOrcaDir(): void { - const dir = getOrcaDir() - if (!existsSync(dir)) { - mkdirSync(dir, { recursive: true }) - } -} - -function ensureTokenDir(): void { - const dir = getTokenDir() - if (!existsSync(dir)) { - mkdirSync(dir, { recursive: true }) - } -} - -function emptySiteFile(): JiraSiteFile { - return { - version: 1, - activeSiteId: null, - selectedSiteId: null, - sites: [] - } -} - -function hasStoredToken(siteId: string): boolean { - return cachedTokens.has(siteId) || credentialFileHasContent(getTokenPath(siteId)) -} - -function normalizeSite(input: unknown): JiraSite | null { - if (!input || typeof input !== 'object') { - return null - } - const record = input as Record - if ( - typeof record.id !== 'string' || - typeof record.siteUrl !== 'string' || - typeof record.email !== 'string' || - typeof record.displayName !== 'string' || - typeof record.accountId !== 'string' - ) { - return null - } - return { - id: record.id, - siteUrl: record.siteUrl, - email: record.email, - displayName: record.displayName, - accountId: record.accountId, - // Sites saved before self-hosted support have no authType; they are Cloud. - authType: record.authType === 'server' ? 'server' : 'cloud' - } -} - -function readSiteFileFromDisk(): JiraSiteFile { - const path = getSiteFilePath() - if (!existsSync(path)) { - return emptySiteFile() - } - try { - const parsed = JSON.parse(readFileSync(path, { encoding: 'utf-8' })) as Partial - const sites = Array.isArray(parsed.sites) - ? parsed.sites - .map((site) => normalizeSite(site)) - .filter((site): site is JiraSite => site !== null) - .filter((site) => hasStoredToken(site.id)) - : [] - const activeSiteId = - typeof parsed.activeSiteId === 'string' && - sites.some((site) => site.id === parsed.activeSiteId) - ? parsed.activeSiteId - : (sites[0]?.id ?? null) - const selectedSiteId = - parsed.selectedSiteId === 'all' || - (typeof parsed.selectedSiteId === 'string' && - sites.some((site) => site.id === parsed.selectedSiteId)) - ? parsed.selectedSiteId - : activeSiteId - return { version: 1, activeSiteId, selectedSiteId, sites } - } catch { - return emptySiteFile() - } -} - -function getSiteFile(): JiraSiteFile { - if (!siteFileLoaded || !cachedSiteFile) { - cachedSiteFile = readSiteFileFromDisk() - siteFileLoaded = true - } - return cachedSiteFile -} - -function writeSiteFile(file: JiraSiteFile): void { - ensureOrcaDir() - const sites = file.sites.filter((site) => hasStoredToken(site.id)) - const activeSiteId = - file.activeSiteId && sites.some((site) => site.id === file.activeSiteId) - ? file.activeSiteId - : (sites[0]?.id ?? null) - const selectedSiteId = - file.selectedSiteId === 'all' - ? 'all' - : file.selectedSiteId && sites.some((site) => site.id === file.selectedSiteId) - ? file.selectedSiteId - : activeSiteId - - cachedSiteFile = { - version: 1, - activeSiteId, - selectedSiteId, - sites - } - siteFileLoaded = true - writeFileSync(getSiteFilePath(), JSON.stringify(cachedSiteFile, null, 2), { - encoding: 'utf-8', - mode: 0o600 - }) -} - -function writeEncryptedToken(path: string, apiToken: string): void { - if (safeStorage.isEncryptionAvailable()) { - writeFileSync(path, safeStorage.encryptString(apiToken), { mode: 0o600 }) - return - } - console.warn('[jira] safeStorage encryption unavailable — storing token in plaintext') - writeFileSync(path, apiToken, { encoding: 'utf-8', mode: 0o600 }) -} - -function readToken(siteId: string): string | null { - const cached = cachedTokens.get(siteId) - if (cached !== undefined) { - return cached - } - const path = getTokenPath(siteId) - if (!existsSync(path)) { - return null - } - try { - const raw = readFileSync(path) - const token = readStoredCredentialToken('Jira', raw) - if (token) { - cachedTokens.set(siteId, token) - } - credentialErrors.delete(siteId) - return token - } catch (error) { - if (error instanceof CredentialDecryptionError) { - credentialErrors.set(siteId, error.message) - throw error - } - return null - } -} - -function saveToken(siteId: string, apiToken: string): void { - ensureOrcaDir() - ensureTokenDir() - writeEncryptedToken(getTokenPath(siteId), apiToken) - cachedTokens.set(siteId, apiToken) - credentialErrors.delete(siteId) -} - -function deleteToken(siteId: string): void { - cachedTokens.delete(siteId) - credentialErrors.delete(siteId) - try { - unlinkSync(getTokenPath(siteId)) - } catch { - // Token may not exist — safe to ignore. - } -} - -export function normalizeJiraSiteUrl(siteUrl: string): string { - const trimmed = siteUrl.trim() - const withProtocol = /^[a-z][a-z0-9+.-]*:\/\//i.test(trimmed) ? trimmed : `https://${trimmed}` - const url = new URL(withProtocol) - url.pathname = url.pathname.replace(/\/+$/, '') - url.search = '' - url.hash = '' - return url.toString().replace(/\/$/, '') -} - -function getSiteId(siteUrl: string, email: string): string { - return createHash('sha256') - .update(`${siteUrl}\n${email.toLowerCase()}`) - .digest('base64url') - .slice(0, 24) -} - -function toViewer(data: Record, fallbackEmail: string): JiraViewer { - const avatarUrls = data.avatarUrls as Record | undefined - // Server/DC /myself has no accountId; its stable identifiers are name/key. - const accountId = - typeof data.accountId === 'string' - ? data.accountId - : typeof data.name === 'string' - ? data.name - : typeof data.key === 'string' - ? data.key - : '' - return { - accountId, - displayName: typeof data.displayName === 'string' ? data.displayName : fallbackEmail, - email: typeof data.emailAddress === 'string' ? data.emailAddress : fallbackEmail, - avatarUrl: - typeof avatarUrls?.['48x48'] === 'string' - ? avatarUrls['48x48'] - : typeof avatarUrls?.['32x32'] === 'string' - ? avatarUrls['32x32'] - : undefined - } -} - -function siteToViewer(site: JiraSite | null): JiraViewer | null { - if (!site) { - return null - } - return { - accountId: site.accountId, - displayName: site.displayName, - email: site.email - } -} - -function authHeader(email: string, apiToken: string, authType?: JiraAuthType): string { - // Self-hosted with no username = a personal access token (Bearer); Basic auth - // with a PAT in the password slot is what produces the 401s users report. - // Self-hosted WITH a username is classic username+password Basic auth, which - // older Server/DC instances (predating PATs) require. Cloud is always Basic. - if (authType === 'server' && !email) { - return `Bearer ${apiToken}` - } - return `Basic ${Buffer.from(`${email}:${apiToken}`).toString('base64')}` -} - -function describeErrorCause(error: unknown): string | undefined { - if (!error || typeof error !== 'object' || !('cause' in error)) { - return undefined - } - const cause = (error as { cause?: unknown }).cause - if (cause instanceof Error) { - return `${cause.name}: ${cause.message}` - } - return cause === undefined ? undefined : String(cause) -} - -async function jiraFetch(url: string, init: RequestInit): Promise { - return withSpan( - 'jira.request', - async (span) => { - span.setAttribute('jira.siteUrl', new URL(url).origin) - await ensureElectronProxyFromEnvironment({ - proxySession: session.defaultSession, - probeUrl: url - }).catch((error) => { - span.addEvent('jira.proxySetupFailed', { - errorName: error instanceof Error ? error.name : typeof error, - errorMessage: error instanceof Error ? error.message : String(error) - }) - }) - try { - // Why: Electron's network stack follows Chromium proxy/session state, - // avoiding undici's stale keep-alive sockets after VPN path changes. - return await net.fetch(url, init) - } catch (error) { - span.setAttribute( - 'jira.transportErrorName', - error instanceof Error ? error.name : typeof error - ) - span.setAttribute( - 'jira.transportErrorMessage', - error instanceof Error ? error.message : String(error) - ) - const cause = describeErrorCause(error) - if (cause) { - span.setAttribute('jira.transportErrorCause', cause) - } - throw error - } - }, - { kind: 'client' } - ) -} - -async function requestWithCredentials( - siteUrl: string, - email: string, - apiToken: string, - path: string, - init?: RequestInit, - authType?: JiraAuthType -): Promise { - const headers = new Headers(init?.headers) - headers.set('Accept', 'application/json') - headers.set('Content-Type', 'application/json') - headers.set('User-Agent', JIRA_API_USER_AGENT) - headers.set('Authorization', authHeader(email, apiToken, authType)) - const response = await jiraFetch(`${siteUrl}${path}`, { - ...init, - headers - }) - if (!response.ok) { - throw new JiraApiError(await readJiraError(response), response.status) - } - if (response.status === 204) { - return null - } - return response.json() -} - -async function readJiraError(response: Response): Promise { - try { - const data = (await response.json()) as { - errorMessages?: string[] - errors?: Record - message?: string - } - const messages = [ - ...(Array.isArray(data.errorMessages) ? data.errorMessages : []), - ...Object.values(data.errors ?? {}), - ...(data.message ? [data.message] : []) - ].filter(Boolean) - if (messages.length > 0) { - return messages.join('; ') - } - } catch { - // Fall through to status text. - } - return response.statusText || `Jira request failed (${response.status})` -} - -export async function jiraRequest( - client: JiraClientForSite, - path: string, - init?: RequestInit -): Promise { - const headers = new Headers(init?.headers) - headers.set('Accept', 'application/json') - headers.set('Content-Type', 'application/json') - headers.set('User-Agent', JIRA_API_USER_AGENT) - headers.set('Authorization', client.authorization) - const response = await jiraFetch(`${client.site.siteUrl}${path}`, { - ...init, - headers - }) - if (!response.ok) { - throw new JiraApiError(await readJiraError(response), response.status) - } - if (response.status === 204) { - return null as T - } - return (await response.json()) as T -} - -export async function jiraRequestBinary( - client: JiraClientForSite, - pathOrUrl: string -): Promise<{ data: ArrayBuffer; contentType: string }> { - const siteUrl = new URL(client.site.siteUrl) - const requestUrl = /^https?:\/\//i.test(pathOrUrl) - ? new URL(pathOrUrl) - : new URL(`${client.site.siteUrl}${pathOrUrl}`) - if (requestUrl.origin !== siteUrl.origin) { - // Why: attachment metadata is provider-controlled; never forward Jira - // credentials if a malformed response points at another origin. - throw new JiraApiError('Jira attachment URL must use the configured site origin.', null) - } - const headers = new Headers() - // Why: attachment content is binary; forcing JSON Accept/Content-Type can - // break downloads and confuses some Atlassian edge responses. - headers.set('Accept', '*/*') - headers.set('User-Agent', JIRA_API_USER_AGENT) - headers.set('Authorization', client.authorization) - const response = await jiraFetch(requestUrl.toString(), { headers }) - if (!response.ok) { - throw new JiraApiError(await readJiraError(response), response.status) - } - const contentType = response.headers.get('content-type') || 'application/octet-stream' - return { - data: await response.arrayBuffer(), - contentType - } -} +import { acquire, release } from './request-queue' +import { + credentialErrors, + deleteToken, + getSiteFile, + hasStoredToken, + readToken, + saveToken, + writeSiteFile +} from './site-credential-store' +import { + apiBasePath, + authHeader, + JiraApiError, + jiraRequest, + requestWithCredentials, + type JiraClientForSite +} from './authenticated-request' +import { getSiteId, normalizeJiraSiteUrl, siteToViewer, toViewer } from './site-identity' export function getClients(selection?: JiraSiteSelection | null): JiraClientForSite[] { const file = getSiteFile() diff --git a/src/main/jira/issues.test.ts b/src/main/jira/issues.test.ts index e8080913d0b..2972fa6b531 100644 --- a/src/main/jira/issues.test.ts +++ b/src/main/jira/issues.test.ts @@ -1,5 +1,5 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -import type { JiraClientForSite } from './client' +import type { JiraClientForSite } from './authenticated-request' import { credentialDecryptionMessage } from '../../shared/integration-credential-errors' import { getJiraSummaryLookupErrorCode } from '../../shared/jira-summary-lookup' @@ -21,14 +21,11 @@ const { releaseMock: vi.fn() })) -vi.mock('./client', () => ({ - acquire: (...args: unknown[]) => acquireMock(...args), - release: (...args: unknown[]) => releaseMock(...args), +vi.mock('./request-queue', () => ({ acquire: acquireMock, release: releaseMock })) + +vi.mock('./authenticated-request', () => ({ apiBasePath: (site: { authType?: string }) => site.authType === 'server' ? '/rest/api/2' : '/rest/api/3', - clearToken: (...args: unknown[]) => clearTokenMock(...args), - getClients: (...args: unknown[]) => getClientsMock(...args), - isAuthError: (...args: unknown[]) => isAuthErrorMock(...args), jiraRequest: (...args: unknown[]) => jiraRequestMock(...args), jiraRequestBinary: (...args: unknown[]) => jiraRequestBinaryMock(...args), JiraApiError: class JiraApiError extends Error { @@ -40,6 +37,12 @@ vi.mock('./client', () => ({ } })) +vi.mock('./client', () => ({ + clearToken: (...args: unknown[]) => clearTokenMock(...args), + getClients: (...args: unknown[]) => getClientsMock(...args), + isAuthError: (...args: unknown[]) => isAuthErrorMock(...args) +})) + function makeEntry(id = 'site-1'): JiraClientForSite { return { site: { diff --git a/src/main/jira/issues.ts b/src/main/jira/issues.ts index 0629f6abfd3..976862c0e96 100644 --- a/src/main/jira/issues.ts +++ b/src/main/jira/issues.ts @@ -21,16 +21,9 @@ import type { JiraTransition, JiraUser } from '../../shared/jira-types' -import { - acquire, - apiBasePath, - clearToken, - getClients, - isAuthError, - jiraRequest, - release, - type JiraClientForSite -} from './client' +import { acquire, release } from './request-queue' +import { apiBasePath, jiraRequest, type JiraClientForSite } from './authenticated-request' +import { clearToken, getClients, isAuthError } from './client' import { adfToMarkdownText, collectAdfMediaAttrs, diff --git a/src/main/jira/jira-issue-summary-timeout.test.ts b/src/main/jira/jira-issue-summary-timeout.test.ts index 1ef80ed631a..ae8d07d60ed 100644 --- a/src/main/jira/jira-issue-summary-timeout.test.ts +++ b/src/main/jira/jira-issue-summary-timeout.test.ts @@ -1,5 +1,5 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -import type { JiraClientForSite } from './client' +import type { JiraClientForSite } from './authenticated-request' import { getJiraSummaryLookupErrorCode } from '../../shared/jira-summary-lookup' const { acquireMock, getClientsMock, jiraRequestMock, releaseMock } = vi.hoisted(() => ({ @@ -9,18 +9,24 @@ const { acquireMock, getClientsMock, jiraRequestMock, releaseMock } = vi.hoisted releaseMock: vi.fn() })) -vi.mock('./client', () => ({ +vi.mock('./request-queue', () => ({ acquire: (...args: unknown[]) => acquireMock(...args), - release: (...args: unknown[]) => releaseMock(...args), + release: (...args: unknown[]) => releaseMock(...args) +})) + +vi.mock('./authenticated-request', () => ({ apiBasePath: () => '/rest/api/3', - clearToken: vi.fn(), - getClients: (...args: unknown[]) => getClientsMock(...args), - isAuthError: vi.fn().mockReturnValue(false), jiraRequest: (...args: unknown[]) => jiraRequestMock(...args), jiraRequestBinary: vi.fn(), JiraApiError: class JiraApiError extends Error {} })) +vi.mock('./client', () => ({ + clearToken: vi.fn(), + getClients: (...args: unknown[]) => getClientsMock(...args), + isAuthError: vi.fn().mockReturnValue(false) +})) + function makeEntry(): JiraClientForSite { return { site: { diff --git a/src/main/jira/jira-search-abort.test.ts b/src/main/jira/jira-search-abort.test.ts index 2e1609f790f..09126ea1c41 100644 --- a/src/main/jira/jira-search-abort.test.ts +++ b/src/main/jira/jira-search-abort.test.ts @@ -1,5 +1,5 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -import type { JiraClientForSite } from './client' +import type { JiraClientForSite } from './authenticated-request' const { acquireMock, clearTokenMock, getClientsMock, isAuthErrorMock, jiraRequestMock } = vi.hoisted(() => ({ @@ -10,18 +10,24 @@ const { acquireMock, clearTokenMock, getClientsMock, isAuthErrorMock, jiraReques jiraRequestMock: vi.fn() })) -vi.mock('./client', () => ({ +vi.mock('./request-queue', () => ({ acquire: (...args: unknown[]) => acquireMock(...args), - release: vi.fn(), + release: vi.fn() +})) + +vi.mock('./authenticated-request', () => ({ apiBasePath: () => '/rest/api/3', - clearToken: (...args: unknown[]) => clearTokenMock(...args), - getClients: (...args: unknown[]) => getClientsMock(...args), - isAuthError: (...args: unknown[]) => isAuthErrorMock(...args), jiraRequest: (...args: unknown[]) => jiraRequestMock(...args), jiraRequestBinary: vi.fn(), JiraApiError: class JiraApiError extends Error {} })) +vi.mock('./client', () => ({ + clearToken: (...args: unknown[]) => clearTokenMock(...args), + getClients: (...args: unknown[]) => getClientsMock(...args), + isAuthError: (...args: unknown[]) => isAuthErrorMock(...args) +})) + function makeEntry(id: string): JiraClientForSite { return { site: { diff --git a/src/main/jira/request-queue.ts b/src/main/jira/request-queue.ts new file mode 100644 index 00000000000..990ccc9ef80 --- /dev/null +++ b/src/main/jira/request-queue.ts @@ -0,0 +1,57 @@ +const MAX_CONCURRENT = 4 +let running = 0 +type QueuedJiraRequest = { + resolve: () => void + reject: (error: Error) => void + signal?: AbortSignal + onAbort: () => void +} +const queue: QueuedJiraRequest[] = [] + +function createJiraRequestAbortError(): Error { + const error = new Error('Jira request aborted') + error.name = 'AbortError' + return error +} + +export function acquire(signal?: AbortSignal): Promise { + if (signal?.aborted) { + return Promise.reject(createJiraRequestAbortError()) + } + if (running < MAX_CONCURRENT) { + running += 1 + return Promise.resolve() + } + return new Promise((resolve, reject) => { + const entry: QueuedJiraRequest = { + resolve, + reject, + signal, + onAbort: () => { + const index = queue.indexOf(entry) + if (index === -1) { + return + } + queue.splice(index, 1) + reject(createJiraRequestAbortError()) + } + } + signal?.addEventListener('abort', entry.onAbort, { once: true }) + queue.push(entry) + }) +} + +export function release(): void { + running -= 1 + let next = queue.shift() + while (next) { + next.signal?.removeEventListener('abort', next.onAbort) + if (!next.signal?.aborted) { + running += 1 + next.resolve() + return + } + next.reject(createJiraRequestAbortError()) + next = queue.shift() + } +} diff --git a/src/main/jira/site-credential-store.ts b/src/main/jira/site-credential-store.ts new file mode 100644 index 00000000000..0f8933097d3 --- /dev/null +++ b/src/main/jira/site-credential-store.ts @@ -0,0 +1,210 @@ +import { existsSync, mkdirSync, readFileSync, unlinkSync, writeFileSync } from 'node:fs' +import { homedir } from 'node:os' +import { join } from 'node:path' +import { safeStorage } from 'electron' +import { + CredentialDecryptionError, + credentialFileHasContent, + readStoredCredentialToken +} from '../integration-credential-file' +import type { JiraSite, JiraSiteSelection } from '../../shared/jira-types' + +export type JiraSiteFile = { + version: 1 + activeSiteId: string | null + selectedSiteId: JiraSiteSelection | null + sites: JiraSite[] +} + +let cachedSiteFile: JiraSiteFile | null = null +let siteFileLoaded = false +const cachedTokens = new Map() +// Why: decrypt failures are recorded per site so getStatus can explain +// failing reads without re-touching the keychain on every status poll. +export const credentialErrors = new Map() + +function getOrcaDir(): string { + return join(homedir(), '.orca') +} + +function getSiteFilePath(): string { + return join(getOrcaDir(), 'jira-sites.json') +} + +function getTokenDir(): string { + return join(getOrcaDir(), 'jira-tokens') +} + +function getTokenPath(siteId: string): string { + return join(getTokenDir(), `${Buffer.from(siteId).toString('base64url')}.enc`) +} + +function ensureOrcaDir(): void { + const dir = getOrcaDir() + if (!existsSync(dir)) { + mkdirSync(dir, { recursive: true }) + } +} + +function ensureTokenDir(): void { + const dir = getTokenDir() + if (!existsSync(dir)) { + mkdirSync(dir, { recursive: true }) + } +} + +function emptySiteFile(): JiraSiteFile { + return { + version: 1, + activeSiteId: null, + selectedSiteId: null, + sites: [] + } +} + +export function hasStoredToken(siteId: string): boolean { + return cachedTokens.has(siteId) || credentialFileHasContent(getTokenPath(siteId)) +} + +function normalizeSite(input: unknown): JiraSite | null { + if (!input || typeof input !== 'object') { + return null + } + const record = input as Record + if ( + typeof record.id !== 'string' || + typeof record.siteUrl !== 'string' || + typeof record.email !== 'string' || + typeof record.displayName !== 'string' || + typeof record.accountId !== 'string' + ) { + return null + } + return { + id: record.id, + siteUrl: record.siteUrl, + email: record.email, + displayName: record.displayName, + accountId: record.accountId, + // Sites saved before self-hosted support have no authType; they are Cloud. + authType: record.authType === 'server' ? 'server' : 'cloud' + } +} + +function readSiteFileFromDisk(): JiraSiteFile { + const path = getSiteFilePath() + if (!existsSync(path)) { + return emptySiteFile() + } + try { + const parsed = JSON.parse(readFileSync(path, { encoding: 'utf-8' })) as Partial + const sites = Array.isArray(parsed.sites) + ? parsed.sites + .map((site) => normalizeSite(site)) + .filter((site): site is JiraSite => site !== null) + .filter((site) => hasStoredToken(site.id)) + : [] + const activeSiteId = + typeof parsed.activeSiteId === 'string' && + sites.some((site) => site.id === parsed.activeSiteId) + ? parsed.activeSiteId + : (sites[0]?.id ?? null) + const selectedSiteId = + parsed.selectedSiteId === 'all' || + (typeof parsed.selectedSiteId === 'string' && + sites.some((site) => site.id === parsed.selectedSiteId)) + ? parsed.selectedSiteId + : activeSiteId + return { version: 1, activeSiteId, selectedSiteId, sites } + } catch { + return emptySiteFile() + } +} + +export function getSiteFile(): JiraSiteFile { + if (!siteFileLoaded || !cachedSiteFile) { + cachedSiteFile = readSiteFileFromDisk() + siteFileLoaded = true + } + return cachedSiteFile +} + +export function writeSiteFile(file: JiraSiteFile): void { + ensureOrcaDir() + const sites = file.sites.filter((site) => hasStoredToken(site.id)) + const activeSiteId = + file.activeSiteId && sites.some((site) => site.id === file.activeSiteId) + ? file.activeSiteId + : (sites[0]?.id ?? null) + const selectedSiteId = + file.selectedSiteId === 'all' + ? 'all' + : file.selectedSiteId && sites.some((site) => site.id === file.selectedSiteId) + ? file.selectedSiteId + : activeSiteId + + cachedSiteFile = { + version: 1, + activeSiteId, + selectedSiteId, + sites + } + siteFileLoaded = true + writeFileSync(getSiteFilePath(), JSON.stringify(cachedSiteFile, null, 2), { + encoding: 'utf-8', + mode: 0o600 + }) +} + +function writeEncryptedToken(path: string, apiToken: string): void { + if (safeStorage.isEncryptionAvailable()) { + writeFileSync(path, safeStorage.encryptString(apiToken), { mode: 0o600 }) + return + } + console.warn('[jira] safeStorage encryption unavailable — storing token in plaintext') + writeFileSync(path, apiToken, { encoding: 'utf-8', mode: 0o600 }) +} + +export function readToken(siteId: string): string | null { + const cached = cachedTokens.get(siteId) + if (cached !== undefined) { + return cached + } + const path = getTokenPath(siteId) + if (!existsSync(path)) { + return null + } + try { + const raw = readFileSync(path) + const token = readStoredCredentialToken('Jira', raw) + if (token) { + cachedTokens.set(siteId, token) + } + credentialErrors.delete(siteId) + return token + } catch (error) { + if (error instanceof CredentialDecryptionError) { + credentialErrors.set(siteId, error.message) + throw error + } + return null + } +} + +export function saveToken(siteId: string, apiToken: string): void { + ensureOrcaDir() + ensureTokenDir() + writeEncryptedToken(getTokenPath(siteId), apiToken) + cachedTokens.set(siteId, apiToken) + credentialErrors.delete(siteId) +} + +export function deleteToken(siteId: string): void { + cachedTokens.delete(siteId) + credentialErrors.delete(siteId) + try { + unlinkSync(getTokenPath(siteId)) + } catch { + // Token may not exist — safe to ignore. + } +} diff --git a/src/main/jira/site-identity.ts b/src/main/jira/site-identity.ts new file mode 100644 index 00000000000..434e3039ac1 --- /dev/null +++ b/src/main/jira/site-identity.ts @@ -0,0 +1,54 @@ +import { createHash } from 'node:crypto' +import type { JiraSite, JiraViewer } from '../../shared/jira-types' + +export function normalizeJiraSiteUrl(siteUrl: string): string { + const trimmed = siteUrl.trim() + const withProtocol = /^[a-z][a-z0-9+.-]*:\/\//i.test(trimmed) ? trimmed : `https://${trimmed}` + const url = new URL(withProtocol) + url.pathname = url.pathname.replace(/\/+$/, '') + url.search = '' + url.hash = '' + return url.toString().replace(/\/$/, '') +} + +export function getSiteId(siteUrl: string, email: string): string { + return createHash('sha256') + .update(`${siteUrl}\n${email.toLowerCase()}`) + .digest('base64url') + .slice(0, 24) +} + +export function toViewer(data: Record, fallbackEmail: string): JiraViewer { + const avatarUrls = data.avatarUrls as Record | undefined + // Server/DC /myself has no accountId; its stable identifiers are name/key. + const accountId = + typeof data.accountId === 'string' + ? data.accountId + : typeof data.name === 'string' + ? data.name + : typeof data.key === 'string' + ? data.key + : '' + return { + accountId, + displayName: typeof data.displayName === 'string' ? data.displayName : fallbackEmail, + email: typeof data.emailAddress === 'string' ? data.emailAddress : fallbackEmail, + avatarUrl: + typeof avatarUrls?.['48x48'] === 'string' + ? avatarUrls['48x48'] + : typeof avatarUrls?.['32x32'] === 'string' + ? avatarUrls['32x32'] + : undefined + } +} + +export function siteToViewer(site: JiraSite | null): JiraViewer | null { + if (!site) { + return null + } + return { + accountId: site.accountId, + displayName: site.displayName, + email: site.email + } +} diff --git a/src/main/linear/client.ts b/src/main/linear/client.ts index 714fb6c165e..7e02938b54c 100644 --- a/src/main/linear/client.ts +++ b/src/main/linear/client.ts @@ -1,17 +1,29 @@ -/* eslint-disable max-lines -- Why: Linear credential storage and client - selection share one module so keychain-safe status reads and token mutation - stay in one consistency boundary. */ -import { safeStorage } from 'electron' import type { LinearClient } from '@linear/sdk' -import { existsSync, mkdirSync, readFileSync, unlinkSync, writeFileSync } from 'node:fs' -import { homedir } from 'node:os' -import { join } from 'node:path' import { loadLinearSdk } from './linear-sdk' +import { LEGACY_WORKSPACE_ID } from './linear-credential-paths' import { - CredentialDecryptionError, - credentialFileHasContent, - readStoredCredentialToken -} from '../integration-credential-file' + clearLegacyViewerOnDisk, + forgetLegacyViewer, + getLegacyViewer +} from './linear-legacy-viewer-store' +import { workspaceFromLinearData } from './linear-workspace-record' +import { + getCredentialError, + getLegacyWorkspace, + getWorkspaceFile, + getWorkspaceState, + resolveWorkspaceId, + upsertWorkspace, + writeWorkspaceFile +} from './linear-workspace-registry' +import { + clearToken, + clearTokenFile, + loadToken, + replaceLegacyWorkspace, + saveWorkspaceToken +} from './linear-token-store' +import { CredentialDecryptionError } from '../integration-credential-file' import type { LinearConnectionStatus, LinearViewer, @@ -19,45 +31,6 @@ import type { LinearWorkspaceSelection } from '../../shared/linear/workspace-types' -// ── Concurrency limiter — max 4 parallel Linear API calls ──────────── -const MAX_CONCURRENT = 4 -let running = 0 -const queue: (() => void)[] = [] - -export function acquire(): Promise { - if (running < MAX_CONCURRENT) { - running++ - return Promise.resolve() - } - return new Promise((resolve) => - queue.push(() => { - running++ - resolve() - }) - ) -} - -export function release(): void { - running-- - const next = queue.shift() - if (next) { - next() - } -} - -// ── Token + workspace storage ──────────────────────────────────────── -// Why: tokens remain encrypted via safeStorage, while workspace metadata stays -// plaintext so status checks can render connected accounts without decrypting -// and triggering OS keychain prompts after app updates. -const LEGACY_WORKSPACE_ID = 'legacy' - -type LinearWorkspaceFile = { - version: 1 - activeWorkspaceId: string | null - selectedWorkspaceId: LinearWorkspaceSelection | null - workspaces: LinearWorkspace[] -} - export type LinearClientForWorkspace = { workspace: LinearWorkspace client: LinearClient @@ -66,445 +39,6 @@ export type LinearClientForWorkspace = { export const LINEAR_PUBLIC_FILE_URL_EXPIRY_SECONDS = 60 * 60 -let cachedTokens = new Map() -// Why: decrypt failures are recorded per workspace so getStatus can explain -// failing reads without re-touching the keychain on every status poll. -const credentialErrors = new Map() -let cachedLegacyViewer: LinearViewer | null = null -let legacyViewerLoadedFromDisk = false -let cachedWorkspaceFile: LinearWorkspaceFile | null = null -let workspaceFileLoadedFromDisk = false - -function getOrcaDir(): string { - return join(homedir(), '.orca') -} - -function getLegacyTokenPath(): string { - return join(getOrcaDir(), 'linear-token.enc') -} - -function getLegacyViewerPath(): string { - return join(getOrcaDir(), 'linear-viewer.json') -} - -function getWorkspaceFilePath(): string { - return join(getOrcaDir(), 'linear-workspaces.json') -} - -function getWorkspaceTokenDir(): string { - return join(getOrcaDir(), 'linear-tokens') -} - -function getWorkspaceTokenPath(workspaceId: string): string { - if (workspaceId === LEGACY_WORKSPACE_ID) { - return getLegacyTokenPath() - } - return join(getWorkspaceTokenDir(), `${Buffer.from(workspaceId).toString('base64url')}.enc`) -} - -function ensureOrcaDir(): void { - const dir = getOrcaDir() - if (!existsSync(dir)) { - mkdirSync(dir, { recursive: true }) - } -} - -function ensureWorkspaceTokenDir(): void { - const dir = getWorkspaceTokenDir() - if (!existsSync(dir)) { - mkdirSync(dir, { recursive: true }) - } -} - -function readLegacyViewerFromDisk(): LinearViewer | null { - const path = getLegacyViewerPath() - if (!existsSync(path)) { - return null - } - try { - const raw = readFileSync(path, { encoding: 'utf-8' }) - const parsed = JSON.parse(raw) as Partial - if (typeof parsed?.displayName !== 'string' || typeof parsed?.organizationName !== 'string') { - return null - } - return { - displayName: parsed.displayName, - email: typeof parsed.email === 'string' ? parsed.email : null, - organizationId: typeof parsed.organizationId === 'string' ? parsed.organizationId : undefined, - organizationName: parsed.organizationName, - organizationUrlKey: - typeof parsed.organizationUrlKey === 'string' ? parsed.organizationUrlKey : undefined - } - } catch { - return null - } -} - -function getLegacyViewer(): LinearViewer | null { - if (!legacyViewerLoadedFromDisk) { - cachedLegacyViewer = readLegacyViewerFromDisk() - legacyViewerLoadedFromDisk = true - } - return cachedLegacyViewer -} - -function normalizeWorkspace(input: unknown): LinearWorkspace | null { - if (!input || typeof input !== 'object') { - return null - } - const record = input as Record - if (typeof record.id !== 'string' || typeof record.organizationName !== 'string') { - return null - } - if (typeof record.displayName !== 'string') { - return null - } - - const organizationId = - typeof record.organizationId === 'string' && record.organizationId - ? record.organizationId - : record.id - - return { - id: record.id, - organizationId, - organizationName: record.organizationName, - organizationUrlKey: - typeof record.organizationUrlKey === 'string' ? record.organizationUrlKey : undefined, - displayName: record.displayName, - email: typeof record.email === 'string' ? record.email : null, - credentialRevision: - typeof record.credentialRevision === 'number' && Number.isFinite(record.credentialRevision) - ? record.credentialRevision - : undefined - } -} - -function emptyWorkspaceFile(): LinearWorkspaceFile { - return { - version: 1, - activeWorkspaceId: null, - selectedWorkspaceId: null, - workspaces: [] - } -} - -function readWorkspaceFileFromDisk(): LinearWorkspaceFile { - const path = getWorkspaceFilePath() - if (!existsSync(path)) { - return emptyWorkspaceFile() - } - try { - const raw = readFileSync(path, { encoding: 'utf-8' }) - const parsed = JSON.parse(raw) as Partial - const workspaces = Array.isArray(parsed.workspaces) - ? parsed.workspaces - .map((workspace) => normalizeWorkspace(workspace)) - .filter((workspace): workspace is LinearWorkspace => workspace !== null) - .filter((workspace) => hasStoredToken(workspace.id)) - : [] - const activeWorkspaceId = - typeof parsed.activeWorkspaceId === 'string' && - workspaces.some((workspace) => workspace.id === parsed.activeWorkspaceId) - ? parsed.activeWorkspaceId - : (workspaces[0]?.id ?? null) - const selectedWorkspaceId = - parsed.selectedWorkspaceId === 'all' || - (typeof parsed.selectedWorkspaceId === 'string' && - workspaces.some((workspace) => workspace.id === parsed.selectedWorkspaceId)) - ? parsed.selectedWorkspaceId - : activeWorkspaceId - - return { - version: 1, - activeWorkspaceId, - selectedWorkspaceId, - workspaces - } - } catch { - return emptyWorkspaceFile() - } -} - -function getWorkspaceFile(): LinearWorkspaceFile { - if (!workspaceFileLoadedFromDisk || !cachedWorkspaceFile) { - cachedWorkspaceFile = readWorkspaceFileFromDisk() - workspaceFileLoadedFromDisk = true - } - return cachedWorkspaceFile -} - -function writeWorkspaceFile(file: LinearWorkspaceFile): void { - ensureOrcaDir() - const persistedWorkspaces = file.workspaces.filter( - (workspace) => workspace.id !== LEGACY_WORKSPACE_ID - ) - const selectableIds = new Set(persistedWorkspaces.map((workspace) => workspace.id)) - if (hasStoredToken(LEGACY_WORKSPACE_ID)) { - selectableIds.add(LEGACY_WORKSPACE_ID) - } - const activeWorkspaceId = - file.activeWorkspaceId && selectableIds.has(file.activeWorkspaceId) - ? file.activeWorkspaceId - : (persistedWorkspaces[0]?.id ?? - (selectableIds.has(LEGACY_WORKSPACE_ID) ? LEGACY_WORKSPACE_ID : null)) - const selectedWorkspaceId = - file.selectedWorkspaceId === 'all' - ? 'all' - : file.selectedWorkspaceId && selectableIds.has(file.selectedWorkspaceId) - ? file.selectedWorkspaceId - : activeWorkspaceId - - cachedWorkspaceFile = { - version: 1, - activeWorkspaceId, - selectedWorkspaceId, - workspaces: persistedWorkspaces - } - workspaceFileLoadedFromDisk = true - writeFileSync(getWorkspaceFilePath(), JSON.stringify(cachedWorkspaceFile, null, 2), { - encoding: 'utf-8', - mode: 0o600 - }) -} - -function getLegacyWorkspace(): LinearWorkspace | null { - if (!hasStoredToken(LEGACY_WORKSPACE_ID)) { - return null - } - const viewer = getLegacyViewer() - return { - id: LEGACY_WORKSPACE_ID, - organizationId: viewer?.organizationId ?? LEGACY_WORKSPACE_ID, - organizationName: viewer?.organizationName ?? 'Saved Linear workspace', - organizationUrlKey: viewer?.organizationUrlKey, - displayName: viewer?.displayName ?? 'Linear API key', - email: viewer?.email ?? null, - isLegacy: true - } -} - -function getWorkspaceState(): LinearWorkspaceFile { - const file = getWorkspaceFile() - const legacyWorkspace = getLegacyWorkspace() - const workspaces = [ - ...(legacyWorkspace ? [legacyWorkspace] : []), - ...file.workspaces.filter((workspace) => hasStoredToken(workspace.id)) - ] - const activeWorkspaceId = - file.activeWorkspaceId && - workspaces.some((workspace) => workspace.id === file.activeWorkspaceId) - ? file.activeWorkspaceId - : (workspaces[0]?.id ?? null) - const selectedWorkspaceId = - file.selectedWorkspaceId === 'all' - ? 'all' - : file.selectedWorkspaceId && - workspaces.some((workspace) => workspace.id === file.selectedWorkspaceId) - ? file.selectedWorkspaceId - : activeWorkspaceId - - return { - version: 1, - activeWorkspaceId, - selectedWorkspaceId, - workspaces - } -} - -function clearLegacyViewerOnDisk(): void { - try { - unlinkSync(getLegacyViewerPath()) - } catch { - // File may not exist — safe to ignore. - } -} - -function writeEncryptedToken(path: string, apiKey: string): void { - if (safeStorage.isEncryptionAvailable()) { - const encrypted = safeStorage.encryptString(apiKey) - writeFileSync(path, encrypted, { mode: 0o600 }) - return - } - - console.warn('[linear] safeStorage encryption unavailable — storing token in plaintext') - writeFileSync(path, apiKey, { encoding: 'utf-8', mode: 0o600 }) -} - -function saveWorkspaceToken(workspaceId: string, apiKey: string): void { - ensureOrcaDir() - if (workspaceId !== LEGACY_WORKSPACE_ID) { - ensureWorkspaceTokenDir() - } - const tokenPath = getWorkspaceTokenPath(workspaceId) - writeEncryptedToken(tokenPath, apiKey) - cachedTokens.set(workspaceId, apiKey) - credentialErrors.delete(workspaceId) -} - -// Backward-compatible export for the legacy single-workspace storage path. -export function saveToken(apiKey: string): void { - saveWorkspaceToken(LEGACY_WORKSPACE_ID, apiKey) -} - -export function loadToken(options: { force?: boolean; workspaceId?: string } = {}): string | null { - const workspaceId = options.workspaceId ?? resolveWorkspaceId() - if (!workspaceId) { - return null - } - const cached = cachedTokens.get(workspaceId) - if (cached !== undefined) { - return cached - } - if (!options.force) { - return null - } - const tokenPath = getWorkspaceTokenPath(workspaceId) - if (!existsSync(tokenPath)) { - return null - } - try { - const raw = readFileSync(tokenPath) - const token = readStoredCredentialToken('Linear', raw) - if (token) { - cachedTokens.set(workspaceId, token) - } - credentialErrors.delete(workspaceId) - return token - } catch (error) { - if (error instanceof CredentialDecryptionError) { - credentialErrors.set(workspaceId, error.message) - throw error - } - return null - } -} - -export function hasStoredToken(workspaceId?: string): boolean { - if (!workspaceId) { - return getWorkspaceState().workspaces.length > 0 - } - if (cachedTokens.has(workspaceId)) { - return true - } - return credentialFileHasContent(getWorkspaceTokenPath(workspaceId)) -} - -function clearTokenFile(workspaceId: string): void { - cachedTokens.delete(workspaceId) - credentialErrors.delete(workspaceId) - try { - unlinkSync(getWorkspaceTokenPath(workspaceId)) - } catch { - // File may not exist — safe to ignore. - } -} - -export function clearToken(workspaceId?: string): void { - if (!workspaceId) { - const state = getWorkspaceState() - for (const workspace of state.workspaces) { - clearTokenFile(workspace.id) - } - cachedTokens = new Map() - credentialErrors.clear() - cachedLegacyViewer = null - legacyViewerLoadedFromDisk = false - cachedWorkspaceFile = emptyWorkspaceFile() - workspaceFileLoadedFromDisk = true - clearLegacyViewerOnDisk() - writeWorkspaceFile(emptyWorkspaceFile()) - return - } - - clearTokenFile(workspaceId) - if (workspaceId === LEGACY_WORKSPACE_ID) { - cachedLegacyViewer = null - legacyViewerLoadedFromDisk = false - clearLegacyViewerOnDisk() - return - } - - const file = getWorkspaceFile() - const workspaces = file.workspaces.filter((workspace) => workspace.id !== workspaceId) - const activeWorkspaceId = - file.activeWorkspaceId === workspaceId ? (workspaces[0]?.id ?? null) : file.activeWorkspaceId - const selectedWorkspaceId = - file.selectedWorkspaceId === workspaceId ? activeWorkspaceId : file.selectedWorkspaceId - writeWorkspaceFile({ - version: 1, - activeWorkspaceId, - selectedWorkspaceId, - workspaces - }) -} - -function workspaceFromLinearData( - me: { displayName: string; email?: string | null }, - org: { id: string; name: string; urlKey?: string | null } -): LinearWorkspace { - return { - id: org.id, - organizationId: org.id, - organizationName: org.name, - organizationUrlKey: org.urlKey ?? undefined, - displayName: me.displayName, - email: me.email ?? null - } -} - -function upsertWorkspace(workspace: LinearWorkspace, options: { select?: boolean } = {}): void { - const file = getWorkspaceFile() - const current = file.workspaces.find((entry) => entry.id === workspace.id) - const credentialRevision = (current?.credentialRevision ?? 0) + 1 - const workspaceWithRevision = { ...workspace, credentialRevision } - const withoutCurrent = file.workspaces.filter((entry) => entry.id !== workspace.id) - const workspaces = [...withoutCurrent, workspaceWithRevision].sort((a, b) => - a.organizationName.localeCompare(b.organizationName) - ) - const selectedWorkspaceId = options.select - ? workspace.id - : file.selectedWorkspaceId && file.selectedWorkspaceId !== LEGACY_WORKSPACE_ID - ? file.selectedWorkspaceId - : workspace.id - writeWorkspaceFile({ - version: 1, - activeWorkspaceId: workspace.id, - selectedWorkspaceId, - workspaces - }) -} - -function replaceLegacyWorkspace(workspace: LinearWorkspace, token: string): void { - saveWorkspaceToken(workspace.id, token) - clearTokenFile(LEGACY_WORKSPACE_ID) - clearLegacyViewerOnDisk() - cachedLegacyViewer = null - legacyViewerLoadedFromDisk = true - upsertWorkspace(workspace, { select: true }) -} - -function resolveWorkspaceId(workspaceId?: string | null): string | null { - if (workspaceId && workspaceId !== 'all') { - return workspaceId - } - const state = getWorkspaceState() - if ( - state.selectedWorkspaceId && - state.selectedWorkspaceId !== 'all' && - state.workspaces.some((workspace) => workspace.id === state.selectedWorkspaceId) - ) { - return state.selectedWorkspaceId - } - if ( - state.activeWorkspaceId && - state.workspaces.some((workspace) => workspace.id === state.activeWorkspaceId) - ) { - return state.activeWorkspaceId - } - return state.workspaces[0]?.id ?? null -} - // ── Client factory ─────────────────────────────────────────────────── // Why: issues/teams modules call this for real Linear actions — at that point // decrypting the token and surfacing a keychain prompt is expected. @@ -594,8 +128,7 @@ export async function connect( ) { clearTokenFile(LEGACY_WORKSPACE_ID) clearLegacyViewerOnDisk() - cachedLegacyViewer = null - legacyViewerLoadedFromDisk = true + forgetLegacyViewer() } upsertWorkspace(workspace, { select: true }) return { ok: true, viewer: workspace, workspace } @@ -641,7 +174,7 @@ export function getStatus(): LinearConnectionStatus { null const credentialError = state.workspaces - .map((workspace) => credentialErrors.get(workspace.id)) + .map((workspace) => getCredentialError(workspace.id)) .find((message) => message !== undefined) return { diff --git a/src/main/linear/issue-context-client.test.ts b/src/main/linear/issue-context-client.test.ts index c284ae8a5d4..9dc9fc673dc 100644 --- a/src/main/linear/issue-context-client.test.ts +++ b/src/main/linear/issue-context-client.test.ts @@ -6,13 +6,19 @@ const getStatus = vi.fn() const isAuthError = vi.fn() const clearToken = vi.fn() -vi.mock('./client', () => ({ +vi.mock('./linear-request-concurrency', () => ({ acquire: vi.fn().mockResolvedValue(undefined), - release: vi.fn(), + release: vi.fn() +})) + +vi.mock('./linear-token-store', () => ({ + clearToken: (...args: unknown[]) => clearToken(...args) +})) + +vi.mock('./client', () => ({ getClients: (...args: unknown[]) => getClients(...args), getStatus: (...args: unknown[]) => getStatus(...args), isAuthError: (...args: unknown[]) => isAuthError(...args), - clearToken: (...args: unknown[]) => clearToken(...args), // The signed public-file-url client reuses the same underlying raw client, so // body reads route through the entry's rawRequest spy in these tests. getPublicFileUrlClient: (entry: LinearClientForWorkspace) => entry.client diff --git a/src/main/linear/issue-context-client.ts b/src/main/linear/issue-context-client.ts index 47c2bb219dc..edecce3d9bd 100644 --- a/src/main/linear/issue-context-client.ts +++ b/src/main/linear/issue-context-client.ts @@ -1,14 +1,13 @@ import type { LinearSearchIssueSummary, LinearSearchResult } from '../../shared/linear/agent-access' import { clampLinearSearchLimit } from '../../shared/linear/agent-access' import type { LinearWorkspace } from '../../shared/linear/workspace-types' +import { acquire, release } from './linear-request-concurrency' +import { clearToken } from './linear-token-store' import { - acquire, - clearToken, getClients, getPublicFileUrlClient, getStatus, isAuthError, - release, type LinearClientForWorkspace } from './client' import { diff --git a/src/main/linear/issue-context.test.ts b/src/main/linear/issue-context.test.ts index e94c490dbc1..56f8c6b57c5 100644 --- a/src/main/linear/issue-context.test.ts +++ b/src/main/linear/issue-context.test.ts @@ -7,13 +7,19 @@ const getStatus = vi.fn() const isAuthError = vi.fn() const clearToken = vi.fn() -vi.mock('./client', () => ({ +vi.mock('./linear-request-concurrency', () => ({ acquire: vi.fn().mockResolvedValue(undefined), - release: vi.fn(), + release: vi.fn() +})) + +vi.mock('./linear-token-store', () => ({ + clearToken: (...args: unknown[]) => clearToken(...args) +})) + +vi.mock('./client', () => ({ getClients: (...args: unknown[]) => getClients(...args), getStatus: (...args: unknown[]) => getStatus(...args), isAuthError: (...args: unknown[]) => isAuthError(...args), - clearToken: (...args: unknown[]) => clearToken(...args), // The signed public-file-url client reuses the same underlying raw client, so // body reads route through the entry's rawRequest spy in these tests. getPublicFileUrlClient: (entry: LinearClientForWorkspace) => entry.client diff --git a/src/main/linear/issue-relation-write.test.ts b/src/main/linear/issue-relation-write.test.ts index 6d5762a75fc..c1377abf8a3 100644 --- a/src/main/linear/issue-relation-write.test.ts +++ b/src/main/linear/issue-relation-write.test.ts @@ -4,14 +4,20 @@ import type { LinearClientForWorkspace } from './client' const rawRequest = vi.fn() const getClients = vi.fn() -vi.mock('./client', () => ({ +vi.mock('./linear-request-concurrency', () => ({ acquire: vi.fn().mockResolvedValue(undefined), - release: vi.fn(), - getClients: (...args: unknown[]) => getClients(...args), - isAuthError: vi.fn().mockReturnValue(false), + release: vi.fn() +})) + +vi.mock('./linear-token-store', () => ({ clearToken: vi.fn() })) +vi.mock('./client', () => ({ + getClients: (...args: unknown[]) => getClients(...args), + isAuthError: vi.fn().mockReturnValue(false) +})) + function entry(): LinearClientForWorkspace { return { workspace: { diff --git a/src/main/linear/issue-relation-write.ts b/src/main/linear/issue-relation-write.ts index 08f2a3bc91e..1f84d7f9d4b 100644 --- a/src/main/linear/issue-relation-write.ts +++ b/src/main/linear/issue-relation-write.ts @@ -5,7 +5,9 @@ import type { LinearIssueRelationWriteResult } from '../../shared/linear/issue-relation-write' import { LINEAR_ISSUE_API_PAGE_SIZE_MAX } from '../../shared/linear/issue-read-limits' -import { acquire, clearToken, getClients, isAuthError, release } from './client' +import { acquire, release } from './linear-request-concurrency' +import { clearToken } from './linear-token-store' +import { getClients, isAuthError } from './client' import { linearError } from './issue-context-errors' import { INVERSE_RELATIONS_QUERY, diff --git a/src/main/linear/issues.test.ts b/src/main/linear/issues.test.ts index 2b7469caf7b..a168b05471c 100644 --- a/src/main/linear/issues.test.ts +++ b/src/main/linear/issues.test.ts @@ -7,14 +7,20 @@ const getClients = vi.fn() const clearToken = vi.fn() const isAuthError = vi.fn() -vi.mock('./client', () => ({ +vi.mock('./linear-request-concurrency', () => ({ acquire: vi.fn().mockResolvedValue(undefined), - release: vi.fn(), - getClients: (...args: unknown[]) => getClients(...args), - isAuthError: (...args: unknown[]) => isAuthError(...args), + release: vi.fn() +})) + +vi.mock('./linear-token-store', () => ({ clearToken: (...args: unknown[]) => clearToken(...args) })) +vi.mock('./client', () => ({ + getClients: (...args: unknown[]) => getClients(...args), + isAuthError: (...args: unknown[]) => isAuthError(...args) +})) + function makeEntry(options?: { workspaceId?: string organizationName?: string diff --git a/src/main/linear/issues.ts b/src/main/linear/issues.ts index 43e9cf7b699..203cd4dfbb9 100644 --- a/src/main/linear/issues.ts +++ b/src/main/linear/issues.ts @@ -18,14 +18,9 @@ import { isEmptyLinearIssueAttributeFilter, type LinearIssueAttributeFilter } from '../../shared/linear/issue-attribute-filter' -import { - acquire, - release, - getClients, - isAuthError, - clearToken, - type LinearClientForWorkspace -} from './client' +import { acquire, release } from './linear-request-concurrency' +import { clearToken } from './linear-token-store' +import { getClients, isAuthError, type LinearClientForWorkspace } from './client' import { buildLinearListIssueFilter } from './issue-list-filter' import { mapLinearIssue } from './mappers' diff --git a/src/main/linear/linear-credential-paths.ts b/src/main/linear/linear-credential-paths.ts new file mode 100644 index 00000000000..9c842a4c78b --- /dev/null +++ b/src/main/linear/linear-credential-paths.ts @@ -0,0 +1,46 @@ +import { existsSync, mkdirSync } from 'node:fs' +import { homedir } from 'node:os' +import { join } from 'node:path' + +export const LEGACY_WORKSPACE_ID = 'legacy' + +function getOrcaDir(): string { + return join(homedir(), '.orca') +} + +function getLegacyTokenPath(): string { + return join(getOrcaDir(), 'linear-token.enc') +} + +export function getLegacyViewerPath(): string { + return join(getOrcaDir(), 'linear-viewer.json') +} + +export function getWorkspaceFilePath(): string { + return join(getOrcaDir(), 'linear-workspaces.json') +} + +function getWorkspaceTokenDir(): string { + return join(getOrcaDir(), 'linear-tokens') +} + +export function getWorkspaceTokenPath(workspaceId: string): string { + if (workspaceId === LEGACY_WORKSPACE_ID) { + return getLegacyTokenPath() + } + return join(getWorkspaceTokenDir(), `${Buffer.from(workspaceId).toString('base64url')}.enc`) +} + +export function ensureOrcaDir(): void { + const dir = getOrcaDir() + if (!existsSync(dir)) { + mkdirSync(dir, { recursive: true }) + } +} + +export function ensureWorkspaceTokenDir(): void { + const dir = getWorkspaceTokenDir() + if (!existsSync(dir)) { + mkdirSync(dir, { recursive: true }) + } +} diff --git a/src/main/linear/linear-legacy-viewer-store.ts b/src/main/linear/linear-legacy-viewer-store.ts new file mode 100644 index 00000000000..acb640cff20 --- /dev/null +++ b/src/main/linear/linear-legacy-viewer-store.ts @@ -0,0 +1,58 @@ +import { existsSync, readFileSync, unlinkSync } from 'node:fs' +import { getLegacyViewerPath } from './linear-credential-paths' +import type { LinearViewer } from '../../shared/linear/workspace-types' + +let cachedLegacyViewer: LinearViewer | null = null +let legacyViewerLoadedFromDisk = false + +function readLegacyViewerFromDisk(): LinearViewer | null { + const path = getLegacyViewerPath() + if (!existsSync(path)) { + return null + } + try { + const raw = readFileSync(path, { encoding: 'utf-8' }) + const parsed = JSON.parse(raw) as Partial + if (typeof parsed?.displayName !== 'string' || typeof parsed?.organizationName !== 'string') { + return null + } + return { + displayName: parsed.displayName, + email: typeof parsed.email === 'string' ? parsed.email : null, + organizationId: typeof parsed.organizationId === 'string' ? parsed.organizationId : undefined, + organizationName: parsed.organizationName, + organizationUrlKey: + typeof parsed.organizationUrlKey === 'string' ? parsed.organizationUrlKey : undefined + } + } catch { + return null + } +} + +export function getLegacyViewer(): LinearViewer | null { + if (!legacyViewerLoadedFromDisk) { + cachedLegacyViewer = readLegacyViewerFromDisk() + legacyViewerLoadedFromDisk = true + } + return cachedLegacyViewer +} + +export function clearLegacyViewerOnDisk(): void { + try { + unlinkSync(getLegacyViewerPath()) + } catch { + // File may not exist — safe to ignore. + } +} + +// Why: the viewer file is gone for good, so keep the cache "loaded" and empty +// rather than re-reading a file we just deleted. +export function forgetLegacyViewer(): void { + cachedLegacyViewer = null + legacyViewerLoadedFromDisk = true +} + +export function resetLegacyViewerCache(): void { + cachedLegacyViewer = null + legacyViewerLoadedFromDisk = false +} diff --git a/src/main/linear/linear-request-concurrency.ts b/src/main/linear/linear-request-concurrency.ts new file mode 100644 index 00000000000..b61d2c84aa7 --- /dev/null +++ b/src/main/linear/linear-request-concurrency.ts @@ -0,0 +1,25 @@ +// ── Concurrency limiter — max 4 parallel Linear API calls ──────────── +const MAX_CONCURRENT = 4 +let running = 0 +const queue: (() => void)[] = [] + +export function acquire(): Promise { + if (running < MAX_CONCURRENT) { + running++ + return Promise.resolve() + } + return new Promise((resolve) => + queue.push(() => { + running++ + resolve() + }) + ) +} + +export function release(): void { + running-- + const next = queue.shift() + if (next) { + next() + } +} diff --git a/src/main/linear/linear-token-store.ts b/src/main/linear/linear-token-store.ts new file mode 100644 index 00000000000..4d724c32a00 --- /dev/null +++ b/src/main/linear/linear-token-store.ts @@ -0,0 +1,145 @@ +import { safeStorage } from 'electron' +import { existsSync, readFileSync, unlinkSync, writeFileSync } from 'node:fs' +import { + LEGACY_WORKSPACE_ID, + ensureOrcaDir, + ensureWorkspaceTokenDir, + getWorkspaceTokenPath +} from './linear-credential-paths' +import { + clearLegacyViewerOnDisk, + forgetLegacyViewer, + resetLegacyViewerCache +} from './linear-legacy-viewer-store' +import { emptyWorkspaceFile } from './linear-workspace-record' +import { + cacheToken, + clearCredentialError, + forgetCachedToken, + getCachedToken, + getWorkspaceFile, + getWorkspaceState, + recordCredentialError, + resetCredentialCaches, + resetWorkspaceFileCacheToEmpty, + resolveWorkspaceId, + upsertWorkspace, + writeWorkspaceFile +} from './linear-workspace-registry' +import { + CredentialDecryptionError, + readStoredCredentialToken +} from '../integration-credential-file' +import type { LinearWorkspace } from '../../shared/linear/workspace-types' + +function writeEncryptedToken(path: string, apiKey: string): void { + if (safeStorage.isEncryptionAvailable()) { + const encrypted = safeStorage.encryptString(apiKey) + writeFileSync(path, encrypted, { mode: 0o600 }) + return + } + + console.warn('[linear] safeStorage encryption unavailable — storing token in plaintext') + writeFileSync(path, apiKey, { encoding: 'utf-8', mode: 0o600 }) +} + +export function saveWorkspaceToken(workspaceId: string, apiKey: string): void { + ensureOrcaDir() + if (workspaceId !== LEGACY_WORKSPACE_ID) { + ensureWorkspaceTokenDir() + } + const tokenPath = getWorkspaceTokenPath(workspaceId) + writeEncryptedToken(tokenPath, apiKey) + cacheToken(workspaceId, apiKey) + clearCredentialError(workspaceId) +} + +// Backward-compatible export for the legacy single-workspace storage path. +export function saveToken(apiKey: string): void { + saveWorkspaceToken(LEGACY_WORKSPACE_ID, apiKey) +} + +export function loadToken(options: { force?: boolean; workspaceId?: string } = {}): string | null { + const workspaceId = options.workspaceId ?? resolveWorkspaceId() + if (!workspaceId) { + return null + } + const cached = getCachedToken(workspaceId) + if (cached !== undefined) { + return cached + } + if (!options.force) { + return null + } + const tokenPath = getWorkspaceTokenPath(workspaceId) + if (!existsSync(tokenPath)) { + return null + } + try { + const raw = readFileSync(tokenPath) + const token = readStoredCredentialToken('Linear', raw) + if (token) { + cacheToken(workspaceId, token) + } + clearCredentialError(workspaceId) + return token + } catch (error) { + if (error instanceof CredentialDecryptionError) { + recordCredentialError(workspaceId, error.message) + throw error + } + return null + } +} + +export function clearTokenFile(workspaceId: string): void { + forgetCachedToken(workspaceId) + try { + unlinkSync(getWorkspaceTokenPath(workspaceId)) + } catch { + // File may not exist — safe to ignore. + } +} + +export function clearToken(workspaceId?: string): void { + if (!workspaceId) { + const state = getWorkspaceState() + for (const workspace of state.workspaces) { + clearTokenFile(workspace.id) + } + resetCredentialCaches() + resetLegacyViewerCache() + resetWorkspaceFileCacheToEmpty() + clearLegacyViewerOnDisk() + writeWorkspaceFile(emptyWorkspaceFile()) + return + } + + clearTokenFile(workspaceId) + if (workspaceId === LEGACY_WORKSPACE_ID) { + resetLegacyViewerCache() + clearLegacyViewerOnDisk() + return + } + + const file = getWorkspaceFile() + const workspaces = file.workspaces.filter((workspace) => workspace.id !== workspaceId) + const activeWorkspaceId = + file.activeWorkspaceId === workspaceId ? (workspaces[0]?.id ?? null) : file.activeWorkspaceId + const selectedWorkspaceId = + file.selectedWorkspaceId === workspaceId ? activeWorkspaceId : file.selectedWorkspaceId + writeWorkspaceFile({ + version: 1, + activeWorkspaceId, + selectedWorkspaceId, + workspaces + }) +} + +export function replaceLegacyWorkspace(workspace: LinearWorkspace, token: string): void { + saveWorkspaceToken(workspace.id, token) + clearTokenFile(LEGACY_WORKSPACE_ID) + clearLegacyViewerOnDisk() + forgetLegacyViewer() + upsertWorkspace(workspace, { select: true }) +} diff --git a/src/main/linear/linear-workspace-record.ts b/src/main/linear/linear-workspace-record.ts new file mode 100644 index 00000000000..ffa8625ca33 --- /dev/null +++ b/src/main/linear/linear-workspace-record.ts @@ -0,0 +1,63 @@ +import type { LinearWorkspace, LinearWorkspaceSelection } from '../../shared/linear/workspace-types' + +export type LinearWorkspaceFile = { + version: 1 + activeWorkspaceId: string | null + selectedWorkspaceId: LinearWorkspaceSelection | null + workspaces: LinearWorkspace[] +} + +export function normalizeWorkspace(input: unknown): LinearWorkspace | null { + if (!input || typeof input !== 'object') { + return null + } + const record = input as Record + if (typeof record.id !== 'string' || typeof record.organizationName !== 'string') { + return null + } + if (typeof record.displayName !== 'string') { + return null + } + + const organizationId = + typeof record.organizationId === 'string' && record.organizationId + ? record.organizationId + : record.id + + return { + id: record.id, + organizationId, + organizationName: record.organizationName, + organizationUrlKey: + typeof record.organizationUrlKey === 'string' ? record.organizationUrlKey : undefined, + displayName: record.displayName, + email: typeof record.email === 'string' ? record.email : null, + credentialRevision: + typeof record.credentialRevision === 'number' && Number.isFinite(record.credentialRevision) + ? record.credentialRevision + : undefined + } +} + +export function emptyWorkspaceFile(): LinearWorkspaceFile { + return { + version: 1, + activeWorkspaceId: null, + selectedWorkspaceId: null, + workspaces: [] + } +} + +export function workspaceFromLinearData( + me: { displayName: string; email?: string | null }, + org: { id: string; name: string; urlKey?: string | null } +): LinearWorkspace { + return { + id: org.id, + organizationId: org.id, + organizationName: org.name, + organizationUrlKey: org.urlKey ?? undefined, + displayName: me.displayName, + email: me.email ?? null + } +} diff --git a/src/main/linear/linear-workspace-registry.ts b/src/main/linear/linear-workspace-registry.ts new file mode 100644 index 00000000000..0a658a153ae --- /dev/null +++ b/src/main/linear/linear-workspace-registry.ts @@ -0,0 +1,240 @@ +// ── Token + workspace storage ──────────────────────────────────────── +// Why: tokens remain encrypted via safeStorage, while workspace metadata stays +// plaintext so status checks can render connected accounts without decrypting +// and triggering OS keychain prompts after app updates. +import { existsSync, readFileSync, writeFileSync } from 'node:fs' +import { + LEGACY_WORKSPACE_ID, + ensureOrcaDir, + getWorkspaceFilePath, + getWorkspaceTokenPath +} from './linear-credential-paths' +import { getLegacyViewer } from './linear-legacy-viewer-store' +import { + emptyWorkspaceFile, + normalizeWorkspace, + type LinearWorkspaceFile +} from './linear-workspace-record' +import { credentialFileHasContent } from '../integration-credential-file' +import type { LinearWorkspace } from '../../shared/linear/workspace-types' + +let cachedTokens = new Map() +// Why: decrypt failures are recorded per workspace so getStatus can explain +// failing reads without re-touching the keychain on every status poll. +const credentialErrors = new Map() +let cachedWorkspaceFile: LinearWorkspaceFile | null = null +let workspaceFileLoadedFromDisk = false + +export function getCachedToken(workspaceId: string): string | undefined { + return cachedTokens.get(workspaceId) +} + +export function cacheToken(workspaceId: string, token: string): void { + cachedTokens.set(workspaceId, token) +} + +export function forgetCachedToken(workspaceId: string): void { + cachedTokens.delete(workspaceId) + credentialErrors.delete(workspaceId) +} + +export function resetCredentialCaches(): void { + cachedTokens = new Map() + credentialErrors.clear() +} + +export function recordCredentialError(workspaceId: string, message: string): void { + credentialErrors.set(workspaceId, message) +} + +export function clearCredentialError(workspaceId: string): void { + credentialErrors.delete(workspaceId) +} + +export function getCredentialError(workspaceId: string): string | undefined { + return credentialErrors.get(workspaceId) +} + +export function resetWorkspaceFileCacheToEmpty(): void { + cachedWorkspaceFile = emptyWorkspaceFile() + workspaceFileLoadedFromDisk = true +} + +function readWorkspaceFileFromDisk(): LinearWorkspaceFile { + const path = getWorkspaceFilePath() + if (!existsSync(path)) { + return emptyWorkspaceFile() + } + try { + const raw = readFileSync(path, { encoding: 'utf-8' }) + const parsed = JSON.parse(raw) as Partial + const workspaces = Array.isArray(parsed.workspaces) + ? parsed.workspaces + .map((workspace) => normalizeWorkspace(workspace)) + .filter((workspace): workspace is LinearWorkspace => workspace !== null) + .filter((workspace) => hasStoredToken(workspace.id)) + : [] + const activeWorkspaceId = + typeof parsed.activeWorkspaceId === 'string' && + workspaces.some((workspace) => workspace.id === parsed.activeWorkspaceId) + ? parsed.activeWorkspaceId + : (workspaces[0]?.id ?? null) + const selectedWorkspaceId = + parsed.selectedWorkspaceId === 'all' || + (typeof parsed.selectedWorkspaceId === 'string' && + workspaces.some((workspace) => workspace.id === parsed.selectedWorkspaceId)) + ? parsed.selectedWorkspaceId + : activeWorkspaceId + + return { + version: 1, + activeWorkspaceId, + selectedWorkspaceId, + workspaces + } + } catch { + return emptyWorkspaceFile() + } +} + +export function getWorkspaceFile(): LinearWorkspaceFile { + if (!workspaceFileLoadedFromDisk || !cachedWorkspaceFile) { + cachedWorkspaceFile = readWorkspaceFileFromDisk() + workspaceFileLoadedFromDisk = true + } + return cachedWorkspaceFile +} + +export function writeWorkspaceFile(file: LinearWorkspaceFile): void { + ensureOrcaDir() + const persistedWorkspaces = file.workspaces.filter( + (workspace) => workspace.id !== LEGACY_WORKSPACE_ID + ) + const selectableIds = new Set(persistedWorkspaces.map((workspace) => workspace.id)) + if (hasStoredToken(LEGACY_WORKSPACE_ID)) { + selectableIds.add(LEGACY_WORKSPACE_ID) + } + const activeWorkspaceId = + file.activeWorkspaceId && selectableIds.has(file.activeWorkspaceId) + ? file.activeWorkspaceId + : (persistedWorkspaces[0]?.id ?? + (selectableIds.has(LEGACY_WORKSPACE_ID) ? LEGACY_WORKSPACE_ID : null)) + const selectedWorkspaceId = + file.selectedWorkspaceId === 'all' + ? 'all' + : file.selectedWorkspaceId && selectableIds.has(file.selectedWorkspaceId) + ? file.selectedWorkspaceId + : activeWorkspaceId + + cachedWorkspaceFile = { + version: 1, + activeWorkspaceId, + selectedWorkspaceId, + workspaces: persistedWorkspaces + } + workspaceFileLoadedFromDisk = true + writeFileSync(getWorkspaceFilePath(), JSON.stringify(cachedWorkspaceFile, null, 2), { + encoding: 'utf-8', + mode: 0o600 + }) +} + +export function getLegacyWorkspace(): LinearWorkspace | null { + if (!hasStoredToken(LEGACY_WORKSPACE_ID)) { + return null + } + const viewer = getLegacyViewer() + return { + id: LEGACY_WORKSPACE_ID, + organizationId: viewer?.organizationId ?? LEGACY_WORKSPACE_ID, + organizationName: viewer?.organizationName ?? 'Saved Linear workspace', + organizationUrlKey: viewer?.organizationUrlKey, + displayName: viewer?.displayName ?? 'Linear API key', + email: viewer?.email ?? null, + isLegacy: true + } +} + +export function getWorkspaceState(): LinearWorkspaceFile { + const file = getWorkspaceFile() + const legacyWorkspace = getLegacyWorkspace() + const workspaces = [ + ...(legacyWorkspace ? [legacyWorkspace] : []), + ...file.workspaces.filter((workspace) => hasStoredToken(workspace.id)) + ] + const activeWorkspaceId = + file.activeWorkspaceId && + workspaces.some((workspace) => workspace.id === file.activeWorkspaceId) + ? file.activeWorkspaceId + : (workspaces[0]?.id ?? null) + const selectedWorkspaceId = + file.selectedWorkspaceId === 'all' + ? 'all' + : file.selectedWorkspaceId && + workspaces.some((workspace) => workspace.id === file.selectedWorkspaceId) + ? file.selectedWorkspaceId + : activeWorkspaceId + + return { + version: 1, + activeWorkspaceId, + selectedWorkspaceId, + workspaces + } +} + +export function hasStoredToken(workspaceId?: string): boolean { + if (!workspaceId) { + return getWorkspaceState().workspaces.length > 0 + } + if (cachedTokens.has(workspaceId)) { + return true + } + return credentialFileHasContent(getWorkspaceTokenPath(workspaceId)) +} + +export function upsertWorkspace( + workspace: LinearWorkspace, + options: { select?: boolean } = {} +): void { + const file = getWorkspaceFile() + const current = file.workspaces.find((entry) => entry.id === workspace.id) + const credentialRevision = (current?.credentialRevision ?? 0) + 1 + const workspaceWithRevision = { ...workspace, credentialRevision } + const withoutCurrent = file.workspaces.filter((entry) => entry.id !== workspace.id) + const workspaces = [...withoutCurrent, workspaceWithRevision].sort((a, b) => + a.organizationName.localeCompare(b.organizationName) + ) + const selectedWorkspaceId = options.select + ? workspace.id + : file.selectedWorkspaceId && file.selectedWorkspaceId !== LEGACY_WORKSPACE_ID + ? file.selectedWorkspaceId + : workspace.id + writeWorkspaceFile({ + version: 1, + activeWorkspaceId: workspace.id, + selectedWorkspaceId, + workspaces + }) +} + +export function resolveWorkspaceId(workspaceId?: string | null): string | null { + if (workspaceId && workspaceId !== 'all') { + return workspaceId + } + const state = getWorkspaceState() + if ( + state.selectedWorkspaceId && + state.selectedWorkspaceId !== 'all' && + state.workspaces.some((workspace) => workspace.id === state.selectedWorkspaceId) + ) { + return state.selectedWorkspaceId + } + if ( + state.activeWorkspaceId && + state.workspaces.some((workspace) => workspace.id === state.activeWorkspaceId) + ) { + return state.activeWorkspaceId + } + return state.workspaces[0]?.id ?? null +} diff --git a/src/main/linear/mcp-issue-list.test.ts b/src/main/linear/mcp-issue-list.test.ts index 245d6c959f1..ffab8035b42 100644 --- a/src/main/linear/mcp-issue-list.test.ts +++ b/src/main/linear/mcp-issue-list.test.ts @@ -24,10 +24,16 @@ const clientEntry = ( client: { client: { rawRequest: request } } }) -vi.mock('./client', () => ({ +vi.mock('./linear-request-concurrency', () => ({ acquire, - release, - clearToken, + release +})) + +vi.mock('./linear-token-store', () => ({ + clearToken +})) + +vi.mock('./client', () => ({ getClients, getStatus, isAuthError: () => false diff --git a/src/main/linear/projects.test.ts b/src/main/linear/projects.test.ts index b472480788d..c8a5dd6ac09 100644 --- a/src/main/linear/projects.test.ts +++ b/src/main/linear/projects.test.ts @@ -7,14 +7,20 @@ const getClients = vi.fn() const clearToken = vi.fn() const isAuthError = vi.fn() -vi.mock('./client', () => ({ +vi.mock('./linear-request-concurrency', () => ({ acquire: vi.fn().mockResolvedValue(undefined), - release: vi.fn(), - getClients: (...args: unknown[]) => getClients(...args), - isAuthError: (...args: unknown[]) => isAuthError(...args), + release: vi.fn() +})) + +vi.mock('./linear-token-store', () => ({ clearToken: (...args: unknown[]) => clearToken(...args) })) +vi.mock('./client', () => ({ + getClients: (...args: unknown[]) => getClients(...args), + isAuthError: (...args: unknown[]) => isAuthError(...args) +})) + function makeEntry(): LinearClientForWorkspace { return { workspace: { diff --git a/src/main/linear/projects.ts b/src/main/linear/projects.ts index 5063a87b17c..483b1b032a6 100644 --- a/src/main/linear/projects.ts +++ b/src/main/linear/projects.ts @@ -18,14 +18,9 @@ import { LINEAR_ISSUE_API_PAGE_SIZE_MAX, clampLinearIssueListLimit } from '../../shared/linear/issue-read-limits' -import { - acquire, - clearToken, - getClients, - isAuthError, - release, - type LinearClientForWorkspace -} from './client' +import { acquire, release } from './linear-request-concurrency' +import { clearToken } from './linear-token-store' +import { getClients, isAuthError, type LinearClientForWorkspace } from './client' type LinearRawVariables = Record diff --git a/src/main/linear/teams.test.ts b/src/main/linear/teams.test.ts index 00d68988bab..d0d8986373d 100644 --- a/src/main/linear/teams.test.ts +++ b/src/main/linear/teams.test.ts @@ -8,14 +8,20 @@ const isAuthError = vi.fn() const acquire = vi.fn().mockResolvedValue(undefined) const release = vi.fn() -vi.mock('./client', () => ({ +vi.mock('./linear-request-concurrency', () => ({ acquire, - release, - getClients: (...args: unknown[]) => getClients(...args), - isAuthError: (...args: unknown[]) => isAuthError(...args), + release +})) + +vi.mock('./linear-token-store', () => ({ clearToken: (...args: unknown[]) => clearToken(...args) })) +vi.mock('./client', () => ({ + getClients: (...args: unknown[]) => getClients(...args), + isAuthError: (...args: unknown[]) => isAuthError(...args) +})) + type TeamNode = { id: string name: string diff --git a/src/main/linear/teams.ts b/src/main/linear/teams.ts index 8a103929b7e..d10974ccb3c 100644 --- a/src/main/linear/teams.ts +++ b/src/main/linear/teams.ts @@ -7,7 +7,9 @@ import type { LinearWorkspaceError, LinearWorkspaceSelection } from '../../shared/linear/workspace-types' -import { acquire, release, getClients, isAuthError, clearToken } from './client' +import { acquire, release } from './linear-request-concurrency' +import { clearToken } from './linear-token-store' +import { getClients, isAuthError } from './client' import { fetchAllTeamLabels, fetchAllTeamMembers, diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 5bb312cf565..b97db807ffe 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -889,7 +889,7 @@ import type { UpdateIssueTypeBySlugArgs, UpdateProjectItemFieldArgs, UpdatePullRequestBySlugArgs -} from '../../shared/github/project-types' +} from '../../shared/github/project-request-types' import { getBaseRefDefault, getDefaultRemote, diff --git a/src/preload/api/github-work-item-api.ts b/src/preload/api/github-work-item-api.ts index 064a8ed890b..ae95a908fb6 100644 --- a/src/preload/api/github-work-item-api.ts +++ b/src/preload/api/github-work-item-api.ts @@ -1,31 +1,33 @@ +import type { + GetProjectViewTableResult, + GitHubProjectCommentMutationResult, + GitHubProjectMutationResult, + ListAccessibleProjectsResult, + ListAssignableUsersBySlugResult, + ListIssueTypesBySlugResult, + ListLabelsBySlugResult, + ListProjectViewsResult, + ProjectWorkItemDetailsBySlugResult, + ResolveProjectRefResult +} from '../../shared/github/project-result-types' import type { AddIssueCommentBySlugArgs, ClearProjectItemFieldArgs, DeleteIssueCommentBySlugArgs, GetProjectViewTableArgs, - GetProjectViewTableResult, - GitHubProjectCommentMutationResult, - GitHubProjectMutationResult, ListAccessibleProjectsArgs, - ListAccessibleProjectsResult, ListAssignableUsersBySlugArgs, - ListAssignableUsersBySlugResult, ListIssueTypesBySlugArgs, - ListIssueTypesBySlugResult, ListLabelsBySlugArgs, - ListLabelsBySlugResult, ListProjectViewsArgs, - ListProjectViewsResult, ProjectWorkItemDetailsBySlugArgs, - ProjectWorkItemDetailsBySlugResult, ResolveProjectRefArgs, - ResolveProjectRefResult, UpdateIssueBySlugArgs, UpdateIssueCommentBySlugArgs, UpdateIssueTypeBySlugArgs, UpdateProjectItemFieldArgs, UpdatePullRequestBySlugArgs -} from '../../shared/github/project-types' +} from '../../shared/github/project-request-types' import type { TaskSourceContext } from '../../shared/task-source-context' import type { GitHubCommentResult } from '../../shared/github/comment-types' import type { diff --git a/src/preload/index.ts b/src/preload/index.ts index 2d8c288b397..67c2687fea8 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -164,34 +164,36 @@ import type { WorkspaceCleanupScanProgress } from '../shared/workspace-cleanup' import type { WorkspacePortAdvertisedUrlChangedEvent } from '../shared/workspace-ports' import type { GhAuthDiagnostic } from '../shared/github/auth-types' import type { TaskSourceContext } from '../shared/task-source-context' +import type { + GetProjectViewTableResult, + GitHubProjectCommentMutationResult, + GitHubProjectMutationResult, + ListAccessibleProjectsResult, + ListAssignableUsersBySlugResult, + ListIssueTypesBySlugResult, + ListLabelsBySlugResult, + ListProjectViewsResult, + ProjectWorkItemDetailsBySlugResult, + ResolveProjectRefResult +} from '../shared/github/project-result-types' import type { AddIssueCommentBySlugArgs, ClearProjectItemFieldArgs, DeleteIssueCommentBySlugArgs, GetProjectViewTableArgs, - GetProjectViewTableResult, - GitHubProjectCommentMutationResult, - GitHubProjectMutationResult, ListAccessibleProjectsArgs, - ListAccessibleProjectsResult, ListAssignableUsersBySlugArgs, - ListAssignableUsersBySlugResult, ListIssueTypesBySlugArgs, - ListIssueTypesBySlugResult, ListLabelsBySlugArgs, - ListLabelsBySlugResult, ListProjectViewsArgs, - ListProjectViewsResult, ProjectWorkItemDetailsBySlugArgs, - ProjectWorkItemDetailsBySlugResult, ResolveProjectRefArgs, - ResolveProjectRefResult, UpdateIssueBySlugArgs, UpdateIssueCommentBySlugArgs, UpdateIssueTypeBySlugArgs, UpdatePullRequestBySlugArgs, UpdateProjectItemFieldArgs -} from '../shared/github/project-types' +} from '../shared/github/project-request-types' import { richMarkdownContextMenuCommandChannel, richMarkdownContextMenuTargetChannel, diff --git a/src/renderer/src/components/github-project/GhAuthErrorHelp.tsx b/src/renderer/src/components/github-project/GhAuthErrorHelp.tsx index 6cec15c57b5..4791dc5ebca 100644 --- a/src/renderer/src/components/github-project/GhAuthErrorHelp.tsx +++ b/src/renderer/src/components/github-project/GhAuthErrorHelp.tsx @@ -12,7 +12,7 @@ import { useEffect, useState } from 'react' import { Copy, ExternalLink, RotateCw } from 'lucide-react' import { toast } from 'sonner' import { Button } from '@/components/ui/button' -import type { GitHubProjectViewError } from '../../../../shared/github/project-types' +import type { GitHubProjectViewError } from '../../../../shared/github/project-result-types' import type { GhAuthDiagnostic } from '../../../../shared/github/auth-types' import { translate } from '@/i18n/i18n' diff --git a/src/renderer/src/components/github-project/ProjectCell.tsx b/src/renderer/src/components/github-project/ProjectCell.tsx index ef22f2ffcaa..fd31df1679e 100644 --- a/src/renderer/src/components/github-project/ProjectCell.tsx +++ b/src/renderer/src/components/github-project/ProjectCell.tsx @@ -22,9 +22,9 @@ import type { GitHubProjectFieldMutationValue, GitHubProjectLabel, GitHubProjectRow, - GitHubProjectUser, - ListIssueTypesBySlugResult + GitHubProjectUser } from '../../../../shared/github/project-types' +import type { ListIssueTypesBySlugResult } from '../../../../shared/github/project-result-types' import type { GlobalSettings } from '../../../../shared/global-settings-types' import { translate } from '@/i18n/i18n' diff --git a/src/renderer/src/components/github-project/ProjectPicker.tsx b/src/renderer/src/components/github-project/ProjectPicker.tsx index cd142d5b04c..3c5d1ccaed7 100644 --- a/src/renderer/src/components/github-project/ProjectPicker.tsx +++ b/src/renderer/src/components/github-project/ProjectPicker.tsx @@ -18,12 +18,14 @@ import type { GitHubProjectOwnerType, GitHubProjectSettings, GitHubProjectSummary, + GitHubProjectViewSummary +} from '../../../../shared/github/project-types' +import type { GitHubProjectViewError, - GitHubProjectViewSummary, ListAccessibleProjectsResult, ListProjectViewsResult, ResolveProjectRefResult -} from '../../../../shared/github/project-types' +} from '../../../../shared/github/project-result-types' import { GITHUB_PROJECT_REF_INPUT_TOO_LARGE_ERROR, hasBoundedGitHubProjectRefInputText, diff --git a/src/renderer/src/components/github-project/ProjectViewWrapper.tsx b/src/renderer/src/components/github-project/ProjectViewWrapper.tsx index 3996a562715..211a73bd2f6 100644 --- a/src/renderer/src/components/github-project/ProjectViewWrapper.tsx +++ b/src/renderer/src/components/github-project/ProjectViewWrapper.tsx @@ -32,15 +32,17 @@ import { useAppStore } from '@/store' import { useMountedRef } from '@/hooks/useMountedRef' import { projectViewCacheKey } from '@/store/slices/github' import type { - GetProjectViewTableResult, GitHubIssueType, GitHubProjectFieldMutationValue, GitHubProjectRow, GitHubProjectTable, - GitHubProjectViewError, - GitHubProjectViewSummary, - ListProjectViewsResult + GitHubProjectViewSummary } from '../../../../shared/github/project-types' +import type { + GetProjectViewTableResult, + GitHubProjectViewError, + ListProjectViewsResult +} from '../../../../shared/github/project-result-types' import type { GitHubWorkItem } from '../../../../shared/github/work-item-types' import ProjectPicker, { type ResolvedProjectSelection } from './ProjectPicker' import ProjectViewList from './ProjectViewList' diff --git a/src/renderer/src/components/github-project/slug-dialog/Comments.tsx b/src/renderer/src/components/github-project/slug-dialog/Comments.tsx index d5ebf6f5740..998ea5dd444 100644 --- a/src/renderer/src/components/github-project/slug-dialog/Comments.tsx +++ b/src/renderer/src/components/github-project/slug-dialog/Comments.tsx @@ -17,7 +17,7 @@ import type { GlobalSettings } from '../../../../../shared/global-settings-types import type { GitHubProjectCommentMutationResult, GitHubProjectMutationResult -} from '../../../../../shared/github/project-types' +} from '../../../../../shared/github/project-result-types' import { translate } from '@/i18n/i18n' function getRuntimeTarget(settings: Parameters[0]) { diff --git a/src/renderer/src/hooks/useGitHubSlugMetadata.ts b/src/renderer/src/hooks/useGitHubSlugMetadata.ts index 8b7ab1b4c62..91cd10ce455 100644 --- a/src/renderer/src/hooks/useGitHubSlugMetadata.ts +++ b/src/renderer/src/hooks/useGitHubSlugMetadata.ts @@ -4,7 +4,7 @@ import type { GlobalSettings } from '../../../shared/global-settings-types' import type { ListAssignableUsersBySlugResult, ListLabelsBySlugResult -} from '../../../shared/github/project-types' +} from '../../../shared/github/project-result-types' import { clearMetadataRequestStore, createMetadataRequestStore } from './metadata-request-cache' import { githubRepoIdentityKey } from '../../../shared/github/repository-identity-key' import { githubProjectHost } from '../../../shared/github/project-identity' diff --git a/src/renderer/src/store/slices/github.ts b/src/renderer/src/store/slices/github.ts index e74e7a35586..541b88db919 100644 --- a/src/renderer/src/store/slices/github.ts +++ b/src/renderer/src/store/slices/github.ts @@ -30,14 +30,16 @@ import type { GlobalSettings } from '../../../../shared/global-settings-types' import type { IssueSourcePreference, Repo } from '../../../../shared/repo-types' import type { Worktree } from '../../../../shared/worktree/types' import type { - GetProjectViewTableArgs, - GetProjectViewTableResult, GitHubProjectFieldMutationValue, - GitHubProjectMutationResult, GitHubProjectRow, - GitHubProjectTable, - GitHubProjectViewError + GitHubProjectTable } from '../../../../shared/github/project-types' +import type { + GetProjectViewTableResult, + GitHubProjectMutationResult, + GitHubProjectViewError +} from '../../../../shared/github/project-result-types' +import type { GetProjectViewTableArgs } from '../../../../shared/github/project-request-types' import { isGitHubWorkItemsSshRemoteRequiredError, sortWorkItemsByNumber, diff --git a/src/shared/github/project-request-types.ts b/src/shared/github/project-request-types.ts new file mode 100644 index 00000000000..ea1f4ec07ba --- /dev/null +++ b/src/shared/github/project-request-types.ts @@ -0,0 +1,132 @@ +// Why: the request half of the Project IPC contract, consumed mainly by main +// and preload; kept apart from the ProjectV2 data model in `./project-types` +// so renderer components importing domain shapes don't pull in arg payloads. +import type { GitHubProjectFieldMutationValue, GitHubProjectOwnerType } from './project-types' +import type { GitHubIssueUpdate } from '../issue-mutation-types' + +// ─── IPC arg shapes (shared between main, preload, renderer) ────────── + +export type GetProjectViewTableArgs = { + owner: string + ownerType: GitHubProjectOwnerType + projectNumber: number + /** GitHub host (e.g. GHES); absent means github.com. */ + host?: string + /** View selection precedence: viewId > viewNumber > viewName > first + * TABLE_LAYOUT view. */ + viewId?: string + viewNumber?: number + viewName?: string + /** Ephemeral GitHub-search-syntax query that replaces the view's filter for + * this fetch only. The view's stored filter on GitHub is not modified. + * `undefined` uses the view's saved filter; `''` explicitly clears it for + * this fetch and gets a distinct renderer cache key. */ + queryOverride?: string +} + +export type ProjectWorkItemDetailsBySlugArgs = { + owner: string + repo: string + host?: string + number: number + type: 'issue' | 'pr' +} + +export type UpdateProjectItemFieldArgs = { + projectId: string + host?: string + itemId: string + fieldId: string + value: GitHubProjectFieldMutationValue +} + +export type ClearProjectItemFieldArgs = { + projectId: string + host?: string + itemId: string + fieldId: string +} + +export type UpdateIssueBySlugArgs = { + owner: string + repo: string + host?: string + number: number + updates: GitHubIssueUpdate & { body?: string } +} + +export type UpdatePullRequestBySlugArgs = { + owner: string + repo: string + host?: string + number: number + updates: { title?: string; body?: string; state?: 'open' | 'closed' } +} + +export type AddIssueCommentBySlugArgs = { + owner: string + repo: string + host?: string + number: number + body: string +} + +export type UpdateIssueCommentBySlugArgs = { + owner: string + repo: string + host?: string + commentId: number + body: string +} + +export type DeleteIssueCommentBySlugArgs = { + owner: string + repo: string + host?: string + commentId: number +} + +export type ListLabelsBySlugArgs = { + owner: string + repo: string + host?: string +} + +export type ListAssignableUsersBySlugArgs = { + owner: string + repo: string + host?: string + seedLogins?: string[] +} + +export type ListIssueTypesBySlugArgs = { + owner: string + repo: string + host?: string +} + +export type UpdateIssueTypeBySlugArgs = { + owner: string + repo: string + host?: string + number: number + /** null clears the issue type. */ + issueTypeId: string | null +} + +export type ResolveProjectRefArgs = { + input: string + host?: string +} + +export type ListProjectViewsArgs = { + owner: string + ownerType: GitHubProjectOwnerType + projectNumber: number + host?: string +} + +export type ListAccessibleProjectsArgs = { + /** GitHub host (e.g. GHES); absent means github.com. */ + host?: string +} diff --git a/src/shared/github/project-result-types.ts b/src/shared/github/project-result-types.ts new file mode 100644 index 00000000000..98412fbbb28 --- /dev/null +++ b/src/shared/github/project-result-types.ts @@ -0,0 +1,103 @@ +// Why: the ok/error envelopes returned by Project IPC calls all share one +// classified error, and they pull in cross-module work-item/comment/user types +// that the ProjectV2 data model in `./project-types` never needs. +import type { PRComment } from './comment-types' +import type { + GitHubIssueType, + GitHubProjectOwnerType, + GitHubProjectSummary, + GitHubProjectTable, + GitHubProjectViewSummary +} from './project-types' +import type { GitHubAssignableUser } from './pull-request-types' +import type { GitHubWorkItemDetails } from './work-item-types' + +// ─── Classified errors ───────────────────────────────────────────────── + +export type GitHubProjectViewErrorType = + | 'auth_required' + | 'scope_missing' + | 'not_found' + | 'unsupported_layout' + | 'too_large' + | 'schema_drift' + | 'validation_error' + | 'network_error' + | 'rate_limited' + | 'unknown' + +export type GitHubProjectViewError = { + type: GitHubProjectViewErrorType + message: string + /** Populated when the error is classifiable from a GraphQL response. Never + * includes tokens or full command stdout. */ + details?: { path?: (string | number)[]; code?: string } +} + +export type GetProjectViewTableResult = + | { ok: true; data: GitHubProjectTable } + | { + ok: false + error: GitHubProjectViewError + /** Populated for the `too_large` case and best-effort for + * `unsupported_layout` when a cheap count-only query succeeds. */ + totalCount?: number + } + +export type ListAccessibleProjectsResult = + | { + ok: true + projects: GitHubProjectSummary[] + /** Why: per-org discovery can partially fail (a single org 504s while + * the rest succeed). The picker renders a banner listing the affected + * org logins so the user knows their list is incomplete and can paste + * a URL to reach missing projects. Empty when discovery was clean. */ + partialFailures?: { owner: string; message: string }[] + } + | { ok: false; error: GitHubProjectViewError } + +export type ResolveProjectRefResult = + | { + ok: true + owner: string + ownerType: GitHubProjectOwnerType + number: number + title: string + host?: string + // Why: when the input is a /views/{n} URL, surface the parsed view + // number so the picker can skip the view-pick step and commit the + // selection directly. Absent for owner/number shorthand and project + // URLs without a /views/ segment. + viewNumber?: number + } + | { ok: false; error: GitHubProjectViewError } + +export type ListProjectViewsResult = + | { ok: true; views: GitHubProjectViewSummary[] } + | { ok: false; error: GitHubProjectViewError } + +export type ProjectWorkItemDetailsBySlugResult = + | { ok: true; details: GitHubWorkItemDetails } + | { ok: false; error: GitHubProjectViewError } + +// ─── Mutations ───────────────────────────────────────────────────────── + +export type GitHubProjectMutationResult = + | { ok: true } + | { ok: false; error: GitHubProjectViewError } + +export type GitHubProjectCommentMutationResult = + | { ok: true; comment: PRComment } + | { ok: false; error: GitHubProjectViewError } + +export type ListLabelsBySlugResult = + | { ok: true; labels: string[] } + | { ok: false; error: GitHubProjectViewError } + +export type ListAssignableUsersBySlugResult = + | { ok: true; users: GitHubAssignableUser[] } + | { ok: false; error: GitHubProjectViewError } + +export type ListIssueTypesBySlugResult = + | { ok: true; types: GitHubIssueType[] } + | { ok: false; error: GitHubProjectViewError } diff --git a/src/shared/github/project-types.ts b/src/shared/github/project-types.ts index dc1e20ea7ce..864bf90d18a 100644 --- a/src/shared/github/project-types.ts +++ b/src/shared/github/project-types.ts @@ -1,13 +1,8 @@ -/* eslint-disable max-lines -- Why: this module is the single source of truth for ProjectV2 shapes (settings, IPC payloads, view/field/value types) shared across main, preload, and renderer; splitting risks circular type imports. */ // Why: ProjectV2 shapes are distinct enough from the issue/PR work-item types // that we keep them in a dedicated module. Preload and main-process callers // import from here directly — do not re-export through `./types.ts` just to // match the existing import block; routing through the issue types module // would obscure ownership of the Project surface. -import type { PRComment } from './comment-types' -import type { GitHubAssignableUser } from './pull-request-types' -import type { GitHubWorkItemDetails } from './work-item-types' -import type { GitHubIssueUpdate } from '../issue-mutation-types' export type GitHubProjectViewLayout = 'TABLE_LAYOUT' | 'BOARD_LAYOUT' | 'ROADMAP_LAYOUT' export type GitHubProjectOwnerType = 'organization' | 'user' @@ -234,84 +229,6 @@ export type GitHubProjectSettings = { } | null } -// ─── Classified errors ───────────────────────────────────────────────── - -export type GitHubProjectViewErrorType = - | 'auth_required' - | 'scope_missing' - | 'not_found' - | 'unsupported_layout' - | 'too_large' - | 'schema_drift' - | 'validation_error' - | 'network_error' - | 'rate_limited' - | 'unknown' - -export type GitHubProjectViewError = { - type: GitHubProjectViewErrorType - message: string - /** Populated when the error is classifiable from a GraphQL response. Never - * includes tokens or full command stdout. */ - details?: { path?: (string | number)[]; code?: string } -} - -export type GetProjectViewTableResult = - | { ok: true; data: GitHubProjectTable } - | { - ok: false - error: GitHubProjectViewError - /** Populated for the `too_large` case and best-effort for - * `unsupported_layout` when a cheap count-only query succeeds. */ - totalCount?: number - } - -export type ListAccessibleProjectsResult = - | { - ok: true - projects: GitHubProjectSummary[] - /** Why: per-org discovery can partially fail (a single org 504s while - * the rest succeed). The picker renders a banner listing the affected - * org logins so the user knows their list is incomplete and can paste - * a URL to reach missing projects. Empty when discovery was clean. */ - partialFailures?: { owner: string; message: string }[] - } - | { ok: false; error: GitHubProjectViewError } - -export type ResolveProjectRefResult = - | { - ok: true - owner: string - ownerType: GitHubProjectOwnerType - number: number - title: string - host?: string - // Why: when the input is a /views/{n} URL, surface the parsed view - // number so the picker can skip the view-pick step and commit the - // selection directly. Absent for owner/number shorthand and project - // URLs without a /views/ segment. - viewNumber?: number - } - | { ok: false; error: GitHubProjectViewError } - -export type ListProjectViewsResult = - | { ok: true; views: GitHubProjectViewSummary[] } - | { ok: false; error: GitHubProjectViewError } - -export type ProjectWorkItemDetailsBySlugResult = - | { ok: true; details: GitHubWorkItemDetails } - | { ok: false; error: GitHubProjectViewError } - -// ─── Mutations ───────────────────────────────────────────────────────── - -export type GitHubProjectMutationResult = - | { ok: true } - | { ok: false; error: GitHubProjectViewError } - -export type GitHubProjectCommentMutationResult = - | { ok: true; comment: PRComment } - | { ok: false; error: GitHubProjectViewError } - export type GitHubProjectFieldMutationValue = | { kind: 'single-select'; optionId: string } | { kind: 'iteration'; iterationId: string } @@ -319,142 +236,3 @@ export type GitHubProjectFieldMutationValue = | { kind: 'number'; number: number } /** YYYY-MM-DD. */ | { kind: 'date'; date: string } - -export type ListLabelsBySlugResult = - | { ok: true; labels: string[] } - | { ok: false; error: GitHubProjectViewError } - -export type ListAssignableUsersBySlugResult = - | { ok: true; users: GitHubAssignableUser[] } - | { ok: false; error: GitHubProjectViewError } - -export type ListIssueTypesBySlugResult = - | { ok: true; types: GitHubIssueType[] } - | { ok: false; error: GitHubProjectViewError } - -// ─── IPC arg shapes (shared between main, preload, renderer) ────────── - -export type GetProjectViewTableArgs = { - owner: string - ownerType: GitHubProjectOwnerType - projectNumber: number - /** GitHub host (e.g. GHES); absent means github.com. */ - host?: string - /** View selection precedence: viewId > viewNumber > viewName > first - * TABLE_LAYOUT view. */ - viewId?: string - viewNumber?: number - viewName?: string - /** Ephemeral GitHub-search-syntax query that replaces the view's filter for - * this fetch only. The view's stored filter on GitHub is not modified. - * `undefined` uses the view's saved filter; `''` explicitly clears it for - * this fetch and gets a distinct renderer cache key. */ - queryOverride?: string -} - -export type ProjectWorkItemDetailsBySlugArgs = { - owner: string - repo: string - host?: string - number: number - type: 'issue' | 'pr' -} - -export type UpdateProjectItemFieldArgs = { - projectId: string - host?: string - itemId: string - fieldId: string - value: GitHubProjectFieldMutationValue -} - -export type ClearProjectItemFieldArgs = { - projectId: string - host?: string - itemId: string - fieldId: string -} - -export type UpdateIssueBySlugArgs = { - owner: string - repo: string - host?: string - number: number - updates: GitHubIssueUpdate & { body?: string } -} - -export type UpdatePullRequestBySlugArgs = { - owner: string - repo: string - host?: string - number: number - updates: { title?: string; body?: string; state?: 'open' | 'closed' } -} - -export type AddIssueCommentBySlugArgs = { - owner: string - repo: string - host?: string - number: number - body: string -} - -export type UpdateIssueCommentBySlugArgs = { - owner: string - repo: string - host?: string - commentId: number - body: string -} - -export type DeleteIssueCommentBySlugArgs = { - owner: string - repo: string - host?: string - commentId: number -} - -export type ListLabelsBySlugArgs = { - owner: string - repo: string - host?: string -} - -export type ListAssignableUsersBySlugArgs = { - owner: string - repo: string - host?: string - seedLogins?: string[] -} - -export type ListIssueTypesBySlugArgs = { - owner: string - repo: string - host?: string -} - -export type UpdateIssueTypeBySlugArgs = { - owner: string - repo: string - host?: string - number: number - /** null clears the issue type. */ - issueTypeId: string | null -} - -export type ResolveProjectRefArgs = { - input: string - host?: string -} - -export type ListProjectViewsArgs = { - owner: string - ownerType: GitHubProjectOwnerType - projectNumber: number - host?: string -} - -export type ListAccessibleProjectsArgs = { - /** GitHub host (e.g. GHES); absent means github.com. */ - host?: string -}