From e5e3d2b8343b64645b8bd133d5219912e0ef32f3 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Mon, 4 May 2026 22:51:56 -0700 Subject: [PATCH] refactor(github-project): split project-view and slug-dialog into modules Co-authored-by: Orca --- 00-review-context.md | 64 +- docs/design/github-project-view-tasks.md | 961 ------------ docs/design/mockups/github-project-view.html | 611 -------- src/main/git/runner.ts | 59 +- src/main/github/project-view.ts | 1282 ++--------------- src/main/github/project-view/internals.ts | 355 +++++ src/main/github/project-view/mutations.ts | 738 ++++++++++ .../src/components/GitHubItemDialog.tsx | 58 +- .../components/github-project/ProjectCell.tsx | 76 +- .../github-project/ProjectGroupHeader.tsx | 2 +- .../github-project/ProjectItemSlugDialog.tsx | 603 +------- .../github-project/ProjectPicker.tsx | 66 +- .../github-project/ProjectViewList.tsx | 16 +- .../github-project/ProjectViewWrapper.tsx | 197 +-- .../src/components/github-project/columns.ts | 4 +- .../components/github-project/group-sort.ts | 30 +- .../slug-dialog/AssigneesEditor.tsx | 95 ++ .../github-project/slug-dialog/Comments.tsx | 165 +++ .../slug-dialog/LabelsEditor.tsx | 78 + .../slug-dialog/SlugDialogBody.tsx | 282 ++++ .../src/hooks/useGitHubSlugMetadata.ts | 19 +- src/renderer/src/lib/repo-slug-index.ts | 26 +- src/renderer/src/store/slices/github.ts | 90 +- src/shared/github-project-types.ts | 13 +- 24 files changed, 2355 insertions(+), 3535 deletions(-) delete mode 100644 docs/design/github-project-view-tasks.md delete mode 100644 docs/design/mockups/github-project-view.html create mode 100644 src/main/github/project-view/internals.ts create mode 100644 src/main/github/project-view/mutations.ts create mode 100644 src/renderer/src/components/github-project/slug-dialog/AssigneesEditor.tsx create mode 100644 src/renderer/src/components/github-project/slug-dialog/Comments.tsx create mode 100644 src/renderer/src/components/github-project/slug-dialog/LabelsEditor.tsx create mode 100644 src/renderer/src/components/github-project/slug-dialog/SlugDialogBody.tsx diff --git a/00-review-context.md b/00-review-context.md index ba5d2d59546..e24188eb491 100644 --- a/00-review-context.md +++ b/00-review-context.md @@ -1,88 +1,74 @@ # Review Context ## Branch Info -- Base: origin/main (merge-base: 201685b2) +- Base: origin/main (merge-base 201685b2) - Current: github-ui ## Changed Files Summary -Modified: - M src/main/git/runner.ts +- A src/main/git/runner.test.ts +- A src/main/github/project-view.ts (1515 lines NEW) +- A src/main/github/project-view.test.ts +- A src/main/github/rate-limit.ts - M src/main/ipc/github.ts - M src/preload/api-types.ts - M src/preload/index.ts - M src/renderer/src/components/GitHubItemDialog.tsx - M src/renderer/src/components/TaskPage.tsx -- M src/renderer/src/store/slices/github.ts -- M src/shared/constants.ts -- M src/shared/types.ts - -Added: -- A src/main/git/runner.test.ts -- A src/main/github/project-view.ts -- A src/main/github/project-view.test.ts -- A src/main/github/rate-limit.ts -- A src/renderer/src/components/github-project/group-sort.ts -- A src/renderer/src/components/github-project/group-sort.test.ts -- A src/renderer/src/components/github-project/ProjectCell.tsx +- A src/renderer/src/components/github-project/ProjectCell.tsx (984 lines) - A src/renderer/src/components/github-project/ProjectGroupHeader.tsx - A src/renderer/src/components/github-project/ProjectItemSlugDialog.tsx -- A src/renderer/src/components/github-project/ProjectPicker.tsx +- A src/renderer/src/components/github-project/ProjectPicker.tsx (702 lines) - A src/renderer/src/components/github-project/ProjectRow.tsx - A src/renderer/src/components/github-project/ProjectViewList.tsx -- A src/renderer/src/components/github-project/ProjectViewWrapper.tsx +- A src/renderer/src/components/github-project/ProjectViewWrapper.tsx (844 lines) +- A src/renderer/src/components/github-project/columns.ts +- A src/renderer/src/components/github-project/group-sort.ts +- A src/renderer/src/components/github-project/group-sort.test.ts - A src/renderer/src/components/github/GitHubRateLimitPill.tsx - A src/renderer/src/hooks/useGitHubSlugMetadata.ts - A src/renderer/src/lib/repo-slug-index.ts -- A src/shared/github-project-types.ts - -## Changed Line Ranges (PR Scope) -For added files, all lines are in scope. For modified files, see git diff hunks. Authoritative source: `git diff $(git merge-base origin/main HEAD) `. +- M src/renderer/src/store/slices/github.ts (~609 added) +- M src/shared/constants.ts +- A src/shared/github-project-types.ts (432 lines) +- M src/shared/types.ts ## Review Standards Reference - Follow /review-code standards - Focus on: correctness, security, performance, maintainability -- Priority levels: Critical > High > Medium > Low +- Priority: Critical > High > Medium > Low ## File Categories -### Backend/API (priority 4) +### Backend/API (main process): - src/main/git/runner.ts - src/main/git/runner.test.ts -- src/main/ipc/github.ts - src/main/github/project-view.ts - src/main/github/project-view.test.ts - src/main/github/rate-limit.ts +- src/main/ipc/github.ts - src/preload/api-types.ts - src/preload/index.ts -### Frontend/UI (priority 5) +### Frontend/UI: - src/renderer/src/components/GitHubItemDialog.tsx - src/renderer/src/components/TaskPage.tsx -- src/renderer/src/components/github/GitHubRateLimitPill.tsx -- src/renderer/src/components/github-project/ProjectCell.tsx -- src/renderer/src/components/github-project/ProjectGroupHeader.tsx -- src/renderer/src/components/github-project/ProjectItemSlugDialog.tsx -- src/renderer/src/components/github-project/ProjectPicker.tsx -- src/renderer/src/components/github-project/ProjectRow.tsx -- src/renderer/src/components/github-project/ProjectViewList.tsx -- src/renderer/src/components/github-project/ProjectViewWrapper.tsx +- src/renderer/src/components/github-project/*.tsx +- src/renderer/src/components/github-project/columns.ts - src/renderer/src/components/github-project/group-sort.ts - src/renderer/src/components/github-project/group-sort.test.ts -- src/renderer/src/store/slices/github.ts +- src/renderer/src/components/github/GitHubRateLimitPill.tsx - src/renderer/src/hooks/useGitHubSlugMetadata.ts - src/renderer/src/lib/repo-slug-index.ts +- src/renderer/src/store/slices/github.ts -### Utility/Common (priority 7) +### Utility/Common: - src/shared/constants.ts -- src/shared/types.ts - src/shared/github-project-types.ts +- src/shared/types.ts ## Skipped Issues (Do Not Re-validate) - - -(none yet) ## Iteration State Current iteration: 1 Last completed phase: Setup -Files fixed this iteration: [] diff --git a/docs/design/github-project-view-tasks.md b/docs/design/github-project-view-tasks.md deleted file mode 100644 index 8ca238003ef..00000000000 --- a/docs/design/github-project-view-tasks.md +++ /dev/null @@ -1,961 +0,0 @@ -# Design: GitHub Project View Tasks - -## Problem - -Orca's current GitHub Tasks view lists issues/PRs per repo. Users who plan work in a GitHub ProjectV2 cannot see that plan here — their filter, grouping, and sort order live on github.com and are lost the moment they switch to Orca to start a task. - -The workflow we care about is: *"open the Project view I already use, find the row I want, update its fields, and start work on it."* The Project's *configuration* (views, filter, groupBy, sortBy, field schema) is still edited on GitHub — Orca does not recreate the Projects admin UI. But a user who plans in a Project should be able to change a row's **field values** (Status, Priority, Size, Sprint, Triage Group, custom single-selects / iterations / text / number / date) without context-switching to the browser. Concretely, on `https://github.com/orgs/stablyai/projects/3` with the `Me` view selected, a user should see the same filtered row set, grouped by `Sprint`, sorted by `Priority` then `Size` using the comparators documented below, be able to flip a row from `Todo → In progress` or bump `Priority` from `P1 → P0`, and launch a worktree from a row. - -The built-in `gh project` commands don't expose enough view configuration to reproduce this. `gh api graphql` does, using the user's existing `gh` token. - -## Goal - -Add a **Project mode** inside Tasks that renders **one** selected ProjectV2 view as a grouped row list whose **row data is editable in place**. The list uses the view's filter, first group field, and sort fields as configured on GitHub; exactness is enforced for the filtered item set and for the supported comparator semantics below, with unknown fields falling back to GitHub rank order instead of inventing a misleading sort. Existing Orca task actions (open details, open in browser, start work) continue to work from a row, and common row-level edits — project field values, the issue/PR title and body, labels/assignees, and comments — write back to GitHub through the same `gh` token. - -In scope: - -- One opinionated grouped-row rendering (not a full Projects table clone). -- Use the selected view's `filter`, `groupByFields[0]`, and `sortByFields` as-is. -- Project picker (pinned / recent / paste-to-add) so users can switch projects. -- Block render until the view is fully paginated; hard cap at 500 items. -- **Row-level editing** of: - - **Status** and other ProjectV2 single-select fields (Priority, Size, Triage Group, …) — inline click-to-edit on the list cell and in the dialog — via `updateProjectV2ItemFieldValue`. Status is called out because it is the most frequently edited field and the one users expect to flip directly from the list (`Todo → In progress`) without opening a dialog. - - Other ProjectV2 field values — iteration, text, number, date — via `updateProjectV2ItemFieldValue` / `clearProjectV2ItemFieldValue`. - - **Assignees** on the underlying issue/PR — inline on the list (click the assignees cell to open the same user-picker popover the existing dialog uses) and in the dialog — through the existing `GitHubItemDialog` UI, backed by slug-addressed mutation helpers for Project-origin rows. Called out because "assign this row to me" is the second most common ownership action after Status. - - **Labels** on the underlying issue/PR — in the dialog — through the existing `GitHubItemDialog` UI, backed by slug-addressed mutation helpers for Project-origin rows. - - Issue/PR **title** and **body** via new issue/PR mutation helpers that target the resolved repo slug from the Project row (dialog only, not inline). - - **Comments**: create, edit, delete via issue-comment REST endpoints (dialog only). Reuses the same comment-thread surface as Issues/PRs mode — no new comment UI. - All writes use classified-error envelopes with optimistic patches and rollback, mirroring the existing labels/assignees flow. - -## Non-Goals - -All rejected during Phase A in favor of shipping a single focused workflow (alternative **A1: Workflow-first single view**): - -- **No view tabs / multi-view switcher in v1.** Users select one view via the picker and work with it. A later pass can add view switching. -- **No View menu (Fields / Group by / Sort by / Field sum / Slice by / Show hierarchy / Show agent sessions).** The view's GitHub-owned configuration is authoritative; Orca does not let users edit it in v1. -- **No editable filter bar.** Filter is rendered as a read-only label sourced from the view. -- **No column visibility toggling.** Rendered columns are derived from the view's `fields`. -- **No board or roadmap rendering.** Non-table layouts surface an unsupported state. -- **No view-configuration editing** — no drag-to-rank, project-item creation, view creation, filter editing, field/schema editing, group/sort changes, or column visibility changes. Row field-value edits are in scope; view structure remains GitHub-owned. -- **No separate PAT.** All access is through the user's `gh` auth. -- **No "Show agent sessions" in v1.** Deferred; see Open Questions. -- **No provisional sort while loading.** Rows are not rendered until pagination completes. -- **No background auto-pagination.** Views over the cap surface an explicit oversize state. - -## Prototype Result - -Prototype ran against `gh 2.88.1` with `project` scope on the active token. The queries below are frozen into the design — they validated the GraphQL shape. - -View configuration: - -```sh -gh api graphql -F org=stablyai -F num=3 -f query=' -query($org:String!, $num:Int!) { - organization(login:$org) { - projectV2(number:$num) { - id - title - url - views(first:20) { - nodes { - id - number - name - layout - filter - # Prototype used small page sizes for inspection. Implementation - # paginates view fields/views; see "GraphQL Pagination Requirements". - fields(first:20) { nodes { ...FieldConfig } } - groupByFields(first:10) { nodes { ...FieldConfig } } - sortByFields(first:10) { - nodes { direction field { ...FieldConfig } } - } - } - } - } - } -} -fragment FieldConfig on ProjectV2FieldConfiguration { - __typename - ... on ProjectV2Field { id name dataType } - ... on ProjectV2SingleSelectField { - id - name - dataType - options { id name color } - } - ... on ProjectV2IterationField { - id - name - dataType - configuration { - iterations { id title startDate duration } - completedIterations { id title startDate duration } - } - } -} -' -``` - -The `Me` view returned `filter: "assignee:@me is:open no:triage-group"`, `groupByFields: Sprint`, `sortByFields: Priority ASC, Size ASC`. - -Item query: - -```sh -gh api graphql \ - -F org=stablyai \ - -F num=3 \ - -F q='assignee:@me is:open no:triage-group' \ - -f query=' -query($org:String!, $num:Int!, $q:String!) { - organization(login:$org) { - projectV2(number:$num) { - # Prototype query only fetched one page. Implementation uses first:100 - # plus an after cursor; see "Pagination and Size Cap". - items(first:20, query:$q) { - totalCount - pageInfo { hasNextPage endCursor } - nodes { - id - type - updatedAt - content { - __typename - ... on Issue { - id - number - title - url - state - stateReason - repository { nameWithOwner } - assignees(first:5) { nodes { login name avatarUrl } } - labels(first:10) { nodes { name color } } - parent { number title url } - } - ... on PullRequest { - id - number - title - url - state - isDraft - repository { nameWithOwner } - assignees(first:5) { nodes { login name avatarUrl } } - labels(first:10) { nodes { name color } } - } - ... on DraftIssue { id title body } - } - fieldValues(first:30) { - nodes { - __typename - ... on ProjectV2ItemFieldSingleSelectValue { field { ...FieldConfig } name color optionId } - ... on ProjectV2ItemFieldIterationValue { field { ...FieldConfig } title startDate duration iterationId } - ... on ProjectV2ItemFieldTextValue { field { ...FieldConfig } text } - ... on ProjectV2ItemFieldNumberValue { field { ...FieldConfig } number } - ... on ProjectV2ItemFieldDateValue { field { ...FieldConfig } date } - ... on ProjectV2ItemFieldLabelValue { field { ...FieldConfig } labels(first:10) { nodes { name color } } } - ... on ProjectV2ItemFieldUserValue { field { ...FieldConfig } users(first:5) { nodes { login name avatarUrl } } } - } - } - } - } - } - } -} -fragment FieldConfig on ProjectV2FieldConfiguration { - __typename - ... on ProjectV2Field { id name dataType } - ... on ProjectV2SingleSelectField { id name dataType } - ... on ProjectV2IterationField { id name dataType } -} -' -``` - -`totalCount` already reflects the filtered count — no extra unfiltered count call is needed. - -### GraphQL Pagination Requirements - -The prototype queries above validate the schema shape only. The production implementation must not freeze those small connection sizes into the client: - -- `project.views` must paginate until the requested view is found, or until all views have been inspected. If no requested view is supplied, paginate until the first `TABLE_LAYOUT` view is found; continue only as far as needed. -- `view.fields` must paginate all visible fields for the selected view. Rendering "the view's fields" from `first:20` would silently drop columns on larger Project views. -- `view.groupByFields` and `view.sortByFields` can stay capped at `10` because v1 consumes all returned sort fields but only the first group field; if GitHub ever returns `pageInfo.hasNextPage` for either, log a dev warning and proceed with the first page. -- `item.fieldValues` must request at least `first:100` and include `pageInfo.hasNextPage`. If any item reports additional field-value pages, classify the table as `schema_drift` rather than rendering incomplete columns. Nested per-item pagination would multiply subprocess/runtime cost and is out of v1 scope. -- Multi-value label/user field values are capped at `10` labels and `5` users per cell in v1. Overflow renders a `+N` indicator only if GitHub exposes `totalCount` in the nested connection; otherwise it truncates without claiming completeness. - -The production item query uses variables `q`, `first`, and `after`, with `items(first: $first, after: $after, query: $q, orderBy: { field: POSITION, direction: ASC })`. The normalizer stamps `row.position` from the zero-based index in this fully paginated `POSITION ASC` stream, so client-side sort tie-breaks preserve GitHub's ranked row order. - -`ProjectV2View.filter` is nullable in GitHub's schema. Normalize null/undefined to `''` in `selectedView.filter`. For the item query, pass the normalized string as `$q`; an empty string means "no filter" and keeps the query shape stable across filtered and unfiltered views. - -**`Issue.parent` is gated.** The sub-issues relationship backing `parent` is opt-in and some tokens return `FIELD_NOT_FOUND` / `UNDEFINED_FIELD` / a `preview` header error. The module must: - -- Issue the item query with `parent` included first. -- On a GraphQL error whose path ends in `parent` or whose type is `FIELD_NOT_FOUND` / `FIELD_ERRORS` / `UNDEFINED_FIELD`, retry the whole table fetch once without the `parent` selection and mark `content.parentIssue = null` on every row. Why the whole table, not just the failed page: otherwise early pages could carry parent data while later pages do not, and the UI would silently mix capability states. -- Log one warning per process lifetime so regressions are observable without spamming logs. - -Minimum supported `gh` version: `2.40.0` (first stable GraphQL variable handling for ProjectV2). Required scopes for v1: `repo`, `read:org`, and `project`. `read:project` is enough to render the table but not enough to edit row fields, so the feature treats a read-only token as `scope_missing` once the user attempts a write and the recovery copy asks for `project`. - -## Current Code Map - -| Concern | Current location | -|---|---| -| Tasks page shell, source toggle, GitHub table | `src/renderer/src/components/TaskPage.tsx` | -| GitHub work item renderer state (cache envelope, inflight dedup, concurrency gate) | `src/renderer/src/store/slices/github.ts` | -| GitHub IPC handlers | `src/main/ipc/github.ts` | -| GitHub API client logic | `src/main/github/client.ts` | -| Shared GitHub item types (`ClassifiedError`, `ListWorkItemsResult`) | `src/shared/types.ts` | -| Preload API types | `src/preload/api-types.ts`, `src/preload/index.ts` | - -`TaskPage.tsx` is already large. ProjectV2 rendering ships as its own component tree, not another inline table inside TaskPage. - -## Proposed Architecture - -### New main-process module: `src/main/github/project-view.ts` - -Why a new module instead of piling onto `client.ts`: ProjectV2 is a distinct GraphQL shape with its own normalization layer and retry policy (the `parent`-field dance), and we want those concerns isolated from the issues/PRs client. - -Responsibilities: - -- Resolve an organization or user project by owner + number. -- Fetch the selected view's config and all items (paginated internally). -- Normalize GitHub GraphQL unions into a renderer-friendly shape. -- Fetch slug-addressed issue/PR conversation details for Project-origin rows that do not have a registered local repo. -- Preserve raw field ids so inline field mutations address Project fields accurately. -- Mutate ProjectV2 item field values using `updateProjectV2ItemFieldValue` and `clearProjectV2ItemFieldValue`. -- Execute `gh api graphql` through the same `execFile` helpers as the rest of the GitHub integration — no direct network calls with a separate token. - -### Mutation helpers - -Project rows are not guaranteed to belong to the currently selected local repo. Any issue/PR mutation launched from Project mode must target the row's `content.repository` slug (`owner/repo`) instead of deriving owner/repo from an arbitrary `repoPath` preference. The existing `updateIssue` and `addIssueComment` helpers in `src/main/github/issues.ts` intentionally resolve owner/repo from a local repo path to avoid silent source switching in Issues/PRs mode; Project mode needs slug-addressed variants so a row from `org/other-repo` cannot accidentally write to the active workspace repo. - -Add slug-addressed main-process helpers for: - -- `updateProjectItemFieldValue(projectId, itemId, fieldId, value)` and `clearProjectItemFieldValue(projectId, itemId, fieldId)` in `project-view.ts`. -- `getWorkItemDetailsBySlug(owner, repo, number, type)` returning the conversation payload needed by `GitHubItemDialog`: item summary, body, issue comments, labels, assignees, and participants. For PR rows, PR files/checks/review-thread data remain available only when a registered repo path is present; the slug-backed dialog hides those tabs instead of failing the conversation view. -- `updateIssueBySlug(owner, repo, number, updates)` supporting `title`, `body`, labels, and assignees. Extend `GitHubIssueUpdate` with optional `body?: string`; body writes use `gh api -X PATCH repos/{owner}/{repo}/issues/{number}` because `gh issue edit` does not consistently cover every body-edit case the dialog needs. -- `updatePullRequestBySlug(owner, repo, number, updates)` supporting `title` and `body` through `gh api -X PATCH repos/{owner}/{repo}/pulls/{number}`. Keep the existing `updatePRTitle(repoPath, prNumber, title)` for current PR flows; Project mode should not route through it. -- `addIssueCommentBySlug`, `updateIssueCommentBySlug`, and `deleteIssueCommentBySlug` using `/repos/{owner}/{repo}/issues/{issue_number}/comments` and `/repos/{owner}/{repo}/issues/comments/{comment_id}`. GitHub issue comments back both issues and PR conversation comments, so the same helpers cover Project rows whose content is an issue or PR. Review-thread comments remain out of scope for Project row editing. -- `listLabelsBySlug(owner, repo)` and `listAssignableUsersBySlug(owner, repo, seedLogins?)` for Project-origin dialog and inline assignee controls. Existing label/user lookup helpers are repoPath-addressed and honor issue-source preferences; Project rows must populate their pickers from the row's repository slug or they can show options from the wrong repo. `seedLogins` lets the dialog include currently-visible assignees even when the repo participant search is sparse. - -All Project-mode slug IPC helpers return classified envelopes; none should throw expected auth, scope, validation, not-found, or rate-limit failures across IPC. The renderer applies optimistic patches only after it can identify the affected table cache entry and rolls back from the saved previous row state if a mutation envelope is `{ ok: false }`. - -All slug-addressed helpers validate `owner`, `repo`, numbers, ids, and body/title lengths in the IPC handler before invoking `gh`. They must reject path separators, whitespace, and empty slugs so untrusted Project row data cannot become an arbitrary REST path. This is validation, not access control — GitHub still enforces permissions through the token. - -### Owner resolution cache - -v1 ships with the picker, so `ownerType` is supplied by the caller in most flows. For paste-to-add inputs where the type is ambiguous, the module tries `organization(login:)` and falls back to `user(login:)` on `NOT_FOUND`. - -Resolved `(owner → ownerType)` mappings live in a **process-lifetime in-memory map inside `project-view.ts`**. It is reset on module reload (i.e., HMR in dev rebuilds the map from zero). The per-process `parent`-field retry flag lives in the same module — both are plain `let` / `Map` module locals so HMR's "reload the module" behavior naturally re-probes both. This is intentional: we want HMR to re-test capability, not carry a stale "unsupported" flag into a fresh code path. - -### New shared types - -Prefer a `.ts` file (`src/shared/github-project-types.ts`) over `.d.ts` so the compiler checks references. - -```ts -export type GitHubProjectViewLayout = 'TABLE_LAYOUT' | 'BOARD_LAYOUT' | 'ROADMAP_LAYOUT' -export type GitHubProjectOwnerType = 'organization' | 'user' - -// Anything outside this union renders as an empty cell — never throw. -export type GitHubProjectFieldDataType = - | 'TITLE' | 'ASSIGNEES' | 'LABELS' | 'LINKED_PULL_REQUESTS' | 'REVIEWERS' - | 'REPOSITORY' | 'MILESTONE' | 'PARENT_ISSUE' | 'SUB_ISSUES_PROGRESS' - | 'TRACKS' | 'TRACKED_BY' | 'ISSUE_TYPE' | 'TEXT' | 'NUMBER' | 'DATE' - | 'SINGLE_SELECT' | 'ITERATION' - -export type GitHubProjectSingleSelectOption = { id: string; name: string; color: string } - -export type GitHubProjectIteration = { - id: string - title: string - startDate: string // YYYY-MM-DD - duration: number // days - completed: boolean -} - -export type GitHubProjectField = - | { kind: 'field'; id: string; name: string; dataType: Exclude | (string & {}) } - | { kind: 'single-select'; id: string; name: string; dataType: 'SINGLE_SELECT'; options: GitHubProjectSingleSelectOption[] } - | { kind: 'iteration'; id: string; name: string; dataType: 'ITERATION'; iterations: GitHubProjectIteration[] } - -// Unknown `dataType` values are preserved verbatim for debuggability; the -// column renderer falls through to a value-kind-based generic cell per the -// column-dispatch rule. - -export type GitHubProjectSortDirection = 'ASC' | 'DESC' -export type GitHubProjectSort = { direction: GitHubProjectSortDirection; field: GitHubProjectField } - -export type GitHubProjectView = { - id: string - number: number - name: string - layout: GitHubProjectViewLayout - filter: string // normalized to '' when GitHub returns null - fields: GitHubProjectField[] - groupByFields: GitHubProjectField[] - sortByFields: GitHubProjectSort[] -} - -export type GitHubProjectUser = { login: string; name: string | null; avatarUrl: string | null } -export type GitHubProjectLabel = { name: string; color: string } -export type GitHubProjectParentIssue = { number: number; title: string; url: string } - -export type GitHubProjectFieldValue = - | { kind: 'single-select'; fieldId: string; optionId: string; name: string; color: string } - | { kind: 'iteration'; fieldId: string; iterationId: string; title: string; startDate: string; duration: number } - | { kind: 'text'; fieldId: string; text: string } - | { kind: 'number'; fieldId: string; number: number } - | { kind: 'date'; fieldId: string; date: string } - | { kind: 'labels'; fieldId: string; labels: GitHubProjectLabel[] } - | { kind: 'users'; fieldId: string; users: GitHubProjectUser[] } - -export type GitHubProjectRow = { - id: string - itemType: 'ISSUE' | 'PULL_REQUEST' | 'DRAFT_ISSUE' | 'REDACTED' - content: { - number: number | null - title: string - body: string | null // DraftIssue body and optional detail-cache patch target; list rows do not render issue/PR body. - url: string | null - state: string | null - stateReason: string | null // Issue stateReason; null for PR/draft. Why: closed-as-not-planned needs a different glyph. - isDraft: boolean | null // PullRequest.isDraft; null otherwise. - repository: string | null // nameWithOwner, e.g. 'stablyai/orca' - assignees: GitHubProjectUser[] - labels: GitHubProjectLabel[] - parentIssue: GitHubProjectParentIssue | null - } - fieldValuesByFieldId: Record - updatedAt: string - position: number // original fetched order; final tie-break so equal sort values keep GitHub rank order -} - -export type GitHubProjectTable = { - project: { - id: string - owner: string - ownerType: GitHubProjectOwnerType - number: number - title: string - url: string - } - selectedView: GitHubProjectView - rows: GitHubProjectRow[] - totalCount: number // echoes ProjectV2.items.totalCount for the view filter - parentFieldDropped: boolean // true when the `parent` retry fallback fired; UI can hint "sub-issues unavailable" -} - -export type GitHubProjectSummary = { - id: string - owner: string - ownerType: GitHubProjectOwnerType - number: number - title: string - url: string - source: 'viewer' | `org:${string}` -} - -export type GitHubProjectViewSummary = { - id: string - number: number - name: string - layout: GitHubProjectViewLayout -} - -export type GitHubProjectSettings = { - pinned: Array<{ owner: string; ownerType: GitHubProjectOwnerType; number: number }> - recent: Array<{ owner: string; ownerType: GitHubProjectOwnerType; number: number; lastOpenedAt: string }> - lastViewByProject: Record - activeProject: { owner: string; ownerType: GitHubProjectOwnerType; number: number } | null -} -``` - -Note: `views: GitHubProjectView[]` is intentionally **not** part of the table envelope. v1 does not render a view switcher, so bundling every view's full config into every table fetch would be dead payload. - -The new project types live in `src/shared/github-project-types.ts`. `src/preload/api-types.ts` must import them from that file directly; do not route them through `src/shared/types.ts` just to match the existing import block. - -### IPC surface - -Follow the same classified-error envelope as `ListWorkItemsResult` in `src/shared/types.ts` — **do not throw across IPC for expected failures**. Handlers resolve with a discriminated union so the renderer can render a typed error banner. - -```ts -// Classified failure modes the UI must distinguish. -export type GitHubProjectViewErrorType = - | 'auth_required' // gh not logged in - | 'scope_missing' // token lacks project, read:org, or repo access - | 'not_found' // project or view doesn't exist / user has no access - | 'unsupported_layout' // view is BOARD_LAYOUT or ROADMAP_LAYOUT - | 'too_large' // totalCount > 500; no rows returned - | 'schema_drift' // GraphQL shape we don't understand - | 'validation_error' // mutation input rejected by GitHub - | '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?: Array; code?: string } -} - -export type GetProjectViewTableResult = - | { ok: true; data: GitHubProjectTable } - | { ok: false; error: GitHubProjectViewError; totalCount?: number } - // totalCount is populated for the 'too_large' case and best-effort for - // unsupported_layout when a cheap count-only query succeeds. -``` - -IPC handlers (added to `src/main/ipc/github.ts`) are registered unconditionally, like the existing `gh:*` handlers. The renderer gates entry points; the main process should not conditionally register IPC from a mutable feature flag because enabling the flag at runtime would otherwise require an app restart. - -```ts -gh:listAccessibleProjects(): Promise< - | { ok: true; projects: GitHubProjectSummary[] } - | { ok: false; error: GitHubProjectViewError } -> - -gh:resolveProjectRef(args: { input: string }): Promise< - | { ok: true; owner: string; ownerType: GitHubProjectOwnerType; number: number; title: string } - | { ok: false; error: GitHubProjectViewError } -> - -gh:listProjectViews(args: { - owner: string - ownerType: GitHubProjectOwnerType - projectNumber: number -}): Promise< - | { ok: true; views: GitHubProjectViewSummary[] } - | { ok: false; error: GitHubProjectViewError } -> - -// View selection precedence: viewId > viewNumber > viewName > first TABLE_LAYOUT view. -gh:getProjectViewTable(args: { - owner: string - ownerType: GitHubProjectOwnerType - projectNumber: number - viewId?: string - viewNumber?: number - viewName?: string -}): Promise - -gh:projectWorkItemDetailsBySlug(args: { - owner: string - repo: string - number: number - type: 'issue' | 'pr' -}): Promise< - | { ok: true; details: GitHubWorkItemDetails } - | { ok: false; error: GitHubProjectViewError } -> -``` - -Mutation IPC uses the same envelope style: - -```ts -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 } - | { kind: 'text'; text: string } - | { kind: 'number'; number: number } - | { kind: 'date'; date: string } // YYYY-MM-DD - -gh:updateProjectItemField(args: { - projectId: string - itemId: string - fieldId: string - value: GitHubProjectFieldMutationValue -}): Promise - -gh:clearProjectItemField(args: { - projectId: string - itemId: string - fieldId: string -}): Promise - -gh:updateIssueBySlug(args: { - owner: string - repo: string - number: number - updates: GitHubIssueUpdate -}): Promise - -gh:updatePullRequestBySlug(args: { - owner: string - repo: string - number: number - updates: { title?: string; body?: string } -}): Promise - -gh:addIssueCommentBySlug(args: { - owner: string - repo: string - number: number - body: string -}): Promise - -gh:updateIssueCommentBySlug(args: { - owner: string - repo: string - commentId: number - body: string -}): Promise - -gh:deleteIssueCommentBySlug(args: { - owner: string - repo: string - commentId: number -}): Promise - -gh:listLabelsBySlug(args: { - owner: string - repo: string -}): Promise< - | { ok: true; labels: string[] } - | { ok: false; error: GitHubProjectViewError } -> - -gh:listAssignableUsersBySlug(args: { - owner: string - repo: string - seedLogins?: string[] -}): Promise< - | { ok: true; users: GitHubAssignableUser[] } - | { ok: false; error: GitHubProjectViewError } -> -``` - -Exposed through `src/preload/index.ts` and `src/preload/api-types.ts`. - -`gh:getProjectViewTable` fetches view config **and** all item pages in one call (see "Pagination and Size Cap"). It validates unsupported layouts before row pagination, then performs a cheap count-only item query only when needed to populate `totalCount` for `unsupported_layout`; that count is best-effort because unsupported views should still render a useful recovery state if the count query fails. There is no split config/rows IPC — the cap + block-until-done policy makes splitting unnecessary and the single envelope keeps the renderer state trivial. - -### Renderer state - -Extend the GitHub store slice (`src/renderer/src/store/slices/github.ts`) with a **parallel, separately-declared** `ProjectViewCacheEntry` that mirrors the work-items `CacheEntry` pattern but not its type. Identical field names — `data`, `fetchedAt`, `error?: GitHubProjectViewError` — and nothing else: `sources` and `issueSourceFellBack` don't apply (project-view has a single GraphQL source, no issue/PR-source fallback). Why not generify `CacheEntry`: the ceremony pays off only if both slices share error-handling code, and they don't (different error union, different classifier, different UI banners). Keep the shapes structurally similar, not identically typed. - -```ts -type ProjectRowContentUpdate = { - title?: string - body?: string - addLabels?: string[] - removeLabels?: string[] - addAssignees?: string[] - removeAssignees?: string[] -} - -projectViewCache: Record> -fetchProjectViewTable: (args: FetchProjectViewTableArgs, options?: FetchOptions) => Promise -updateProjectFieldValue: (cacheKey: string, rowId: string, fieldId: string, value: GitHubProjectFieldMutationValue) => Promise -clearProjectFieldValue: (cacheKey: string, rowId: string, fieldId: string) => Promise -patchProjectIssueOrPr: (cacheKey: string, rowId: string, updates: ProjectRowContentUpdate) => Promise -``` - -Cache key — resolved view id only, no filter hash (v1 does not allow filter overrides): - -```ts -`github-project:${ownerType}:${owner}:${projectNumber}:${resolvedViewId}` -``` - -The renderer cannot know `resolvedViewId` until the first successful IPC response when callers pass `viewNumber`, `viewName`, or no view selector. For those calls, use a request key based on the input args for in-flight dedup, then write the returned table under the resolved-view cache key. Subsequent refreshes should use `selectedView.id`. - -**Inflight dedup + concurrency gate must mirror the work-items slice.** Add at module scope: - -```ts -const inflightProjectViewRequests = new Map; force: boolean }>() -// Reuse acquireWorkItemSlot / releaseWorkItemSlot / WORK_ITEM_FETCH_CONCURRENCY -// from the same module — the gate is about gh-subprocess pressure, and a -// ProjectV2 fetch is just as fan-outable as a work-item list. -``` - -Why reuse the gate instead of adding a parallel one: the concurrency limit exists to bound `gh` subprocess pressure at the renderer boundary. Two independent gates would let concurrent project-view fetches blow past the intended cap when a user also hover-prefetches work items on the same page. - -Dedup semantics must match the work-items slice exactly: non-forcing callers collapse onto any in-flight request; a user-initiated `force: true` refresh waits for an in-flight non-forcing request to settle, then issues a new forced fetch. See `fetchWorkItems` for the reference implementation. - -Mutation actions patch `projectViewCache[cacheKey]` optimistically and never render directly from the mutation return value. They must capture the previous row before patching, call the relevant IPC helper, and restore that row if the mutation fails. If the cache entry has moved or no longer contains the row by the time the response returns, skip rollback rather than resurrecting stale data into a newly selected project. - -**Concurrency tuning (post-launch).** `WORK_ITEM_FETCH_CONCURRENCY` is now shared across work-items and project-view traffic. A project-view fetch is a multi-page fan-out of `gh` subprocesses, and under the same user-visible gate it can starve an opportunistic work-items prefetch (or vice versa). This is acceptable for v1 — correctness of the subprocess cap outweighs throughput — but revisit the split if users report Project mode stalling Issues/PRs refresh. - -**Argument types.** `FetchProjectViewTableArgs` matches the `gh:getProjectViewTable` IPC args type exactly: `{ owner; ownerType; projectNumber; viewId?; viewNumber?; viewName? }`. `FetchOptions` is the existing type declared at module scope in `src/renderer/src/store/slices/github.ts` (used by `fetchWorkItems`); the project-view action reuses it verbatim — do not fork. - -## UI Design - -### Placement - -Inside the existing GitHub source, add a mode toggle: `Issues/PRs` (default) · `Project`. - -**Gate the toggle's visibility.** Do not render the `Project` toggle at all unless: - -- `settings.githubProjects.activeProject` is non-null (user has opened a project at least once), **or** -- `settings.experimentalGithubProjectView` is on. - -Why: without a picked project, the Project tab is a dead-end empty state. Hiding the toggle until the user has committed to a project keeps the Issues/PRs flow uncluttered for users who never touch Projects. - -### Header - -Project mode header, from left to right: - -1. **Project picker button** — `stablyai / Stably Product Board ▾`. Clicking opens the picker popover (see Project Selection). -2. **Selected view label** — a plain read-only chip: `View: Me`. Not a tab strip, not a dropdown. v1 does not switch views; to change view the user re-picks from the picker. -3. **Filter label** — read-only display of the view's `filter` string, e.g. `assignee:@me is:open no:triage-group`. Small muted text, truncated with tooltip on overflow. No input, no editing, no token chips. -4. **Count pill** — `totalCount` from the fully paginated result. -5. **Open in GitHub** — external-link icon button to the selected view URL. - -GitHub's `ProjectV2View` schema does not expose a view URL directly. Build it as `${project.url}/views/${selectedView.number}` everywhere the UI needs to open or recover to the selected view. Use `project.url` only as a fallback if `selectedView.number` is missing due to schema drift handling. Why: opening the project default view would break the core "open the Project view I already use" workflow whenever the selected view is not the default. - -No View menu. No column visibility control. No `Fields` / `Group by` / `Sort by` / `Field sum` / `Slice by` / `Show hierarchy` / `Show agent sessions` controls. Those were considered in Phase A and cut as part of direction **A1** — the view's GitHub-owned configuration is authoritative. - -### Grouped row list - -Components live under `src/renderer/src/components/github-project/`: - -```text -ProjectViewList.tsx // grouped row container, header, empty/error states -ProjectGroupHeader.tsx // expand/collapse, label, count, iteration date range -ProjectRow.tsx // single item row, primary/secondary actions -ProjectCell.tsx // one cell per visible column, dataType-driven -``` - -Only `TABLE_LAYOUT` views render as rows. `BOARD_LAYOUT` / `ROADMAP_LAYOUT` surface `unsupported_layout` (see Interaction States). - -Visible columns are derived from `selectedView.fields`: - -| Data type | Cell source | -|---|---| -| `TITLE` | `row.content.title` + number + state glyph (issue: `state` + `stateReason` for closed-as-not-planned; PR: `state` + `isDraft`) | -| `ASSIGNEES` (built-in) | `row.content.assignees` — editable users pill list for issue/PR rows | -| `LABELS` (built-in) | `row.content.labels` — labels pill list; edited in the dialog | -| Custom user field (value arrives as `ProjectV2ItemFieldUserValue`) | `fieldValuesByFieldId[field.id]` as `kind: 'users'` — render as users pill list. Why: the ProjectV2 GraphQL `dataType` enum does not expose a distinct custom-user-field member, so the column renderer dispatches on the *value* `kind`, not `field.dataType`. | -| Custom label field (value arrives as `ProjectV2ItemFieldLabelValue`) | `fieldValuesByFieldId[field.id]` as `kind: 'labels'` — render as labels pill list. Same reason as above. | -| `REPOSITORY` | `row.content.repository` | -| `PARENT_ISSUE` | `row.content.parentIssue`; empty when `parentFieldDropped` | -| `SINGLE_SELECT` | `fieldValuesByFieldId[field.id]` | -| `ITERATION` | `fieldValuesByFieldId[field.id]` | -| `TEXT` / `NUMBER` / `DATE` | corresponding field value | -| `LINKED_PULL_REQUESTS`, `REVIEWERS`, `MILESTONE`, `ISSUE_TYPE`, `SUB_ISSUES_PROGRESS`, `TRACKS`, `TRACKED_BY` | Empty cell in v1 | -| Unknown dataType | If `fieldValuesByFieldId[field.id]` is present, fall through to a kind-based generic renderer (labels / users / text / number / date / single-select / iteration) so a fetched value is never silently dropped. Otherwise empty cell — the normalizer **must never throw** on unknown `__typename`. | - -`ASSIGNEES` and `LABELS` are the **only** built-in multi-value dataTypes v1 recognizes. Custom multi-value fields render strictly through the value-kind fallback above — the column renderer dispatches on `field.dataType` first, then on `fieldValuesByFieldId[field.id].kind` as a safety net so the renderer never silently drops a fetched value. - -`REDACTED` Project items render as disabled rows when GitHub returns `type: REDACTED` or `content: null` because the token cannot see the underlying issue/PR. Use title copy `Restricted item`, leave repo/number/url/actions empty, keep field values if GitHub returned any, and include the row in group counts. Why: dropping redacted rows would make Orca's rendered count disagree with GitHub's view and make the grouped plan look incomplete without explanation. - -#### Inline field editing - -Only editable ProjectV2 field-value columns enter edit mode inline: `SINGLE_SELECT`, `ITERATION`, `TEXT`, `NUMBER`, and `DATE`. Built-in `ASSIGNEES` is also editable inline for issue/PR rows because ownership changes are a core planning action, but it must route through issue/PR assignee APIs, not `updateProjectV2ItemFieldValue`. Built-in title, repository, labels, parent issue, linked PRs, reviewers, milestone, issue type, sub-issues progress, tracks, and tracked-by cells are read-only in the list. Title/body/labels/assignees/comment editing also happens in `GitHubItemDialog`, where the existing full-detail context already exists. - -Inline editors: - -- Single-select: popover list in field option order, plus `Clear`. -- Iteration: popover grouped as completed/current/future in field configuration order, plus `Clear`. -- Text: compact input committed on Enter/blur and canceled on Escape. -- Number: numeric input; invalid or empty input shows local validation. Empty maps to `clearProjectV2ItemFieldValue`, not `number: 0`. -- Date: native date input where available; empty maps to clear. - -Optimistic updates must preserve the current group/sort semantics. After a successful or optimistic field change, the row is re-grouped and re-sorted from the cached table data, so changing `Sprint`, `Priority`, or `Size` immediately moves the row to the same location GitHub would show after refresh. If the write fails, rollback restores the previous row and the previous sorted/grouped position. - -#### Grouping - -Use `selectedView.groupByFields[0]` (first entry only; GitHub UI treats subsequent groupByFields as tie-break, which isn't needed for rendering). The `Me` view groups by `Sprint`. - -Group header order: - -1. Iteration field: use `configuration` order — completed oldest-to-newest, then current, then future iterations (matches GitHub's natural ordering). -2. Single-select field: use option order as declared on the field. -3. Otherwise: alphabetical by label. -4. Empty groups always last (`No Sprint`, `No Priority`, etc.). - -Header shows: expand/collapse chevron · label · count pill. For iterations, also: date range and a `Current` pill when today falls inside `[startDate, startDate + duration)`. - -#### Sorting - -Apply `selectedView.sortByFields` client-side. GitHub's `ProjectV2.items(orderBy:)` only supports `POSITION`, so field sorting has to happen in the renderer. Because v1 blocks rendering until all pages are loaded (see Pagination), sort results are globally correct — no "provisional sort while loading" UX is needed. - -Semantics: - -- Single-select: option order. -- Iteration: iteration order. -- Number / text / date: numeric / string / chronological comparison. -- `ASSIGNEES` / custom user field: sort by the first user's `login` alphabetically. Empty → sort last. Why: GitHub's own Project view effectively uses "first user" ordering; matching it avoids surprising reshuffles. -- `LABELS` / custom label field: sort by the first label's `name` alphabetically. Empty → sort last. Same rationale. -- Missing values sort last. -- Stable final tie-break by `row.position`, the original order returned by GitHub before client-side sort. Why: GitHub uses project rank/position to order rows with equal sort-field values; falling back to issue number/title would reshuffle ties and break exact view mirroring. Only use item number then title if a test fixture or malformed row lacks `position`. -- Any sort-field `kind` not in this table → ignore that sort field, continue to the next configured sort field, and emit a one-time `console.warn` gated on `process.env.NODE_ENV !== 'production'`. If every configured sort field is unknown, preserve `row.position`. Never surface to the user. Why: unknown kinds mean a renderer/comparator drift, not a user-visible defect — we want a dev-time signal without reshuffling away from GitHub rank order. - -The comparator is a contract and must be covered by fixtures captured from `stablyai/3` before the feature flag flips on by default. If GitHub's UI orders a supported data type differently from the rules above, update the comparator and the documented rule together; do not special-case the `Me` view by field name. - -#### Row actions - -- **Primary (click row title)**: open Orca's existing GitHub work-item details dialog for issue/PR rows. If `row.content.repository` matches a registered repo slug, the dialog may fetch details through the matched `repo.path` so existing PR files/checks surfaces keep working. If the repo is not registered, the dialog uses `gh:projectWorkItemDetailsBySlug` and renders the conversation/details surface only; Files/checks/review-thread tabs are hidden because they still depend on local repo context in v1. In both paths, the dialog receives row-origin metadata so writes use slug-addressed mutations. For `DRAFT_ISSUE` rows (no repo, no number) the title is non-interactive and shows `row.content.body` in a hover card. -- **Secondary — open in browser**: icon action on hover; opens `row.content.url`. -- **Secondary — start work**: icon action on hover; creates a worktree linked to this issue/PR (same flow as the existing `Use` action in Issues/PRs mode). -- **Fallback — repo not added to Orca**: if `row.content.repository` is not in `state.repos`, the dialog's Start work CTA and the hover Start work action open a lightweight prompt: *"`owner/repo` isn't added to Orca. Add it to start work, or open in GitHub."* with `Add repo` and `Open in GitHub` buttons. Why not auto-add: adding a repo is a clone + worktree decision the user owns; silently cloning from a row click is surprising. - -`state.repos` does not persist `owner/repo`, so Project rows need a small renderer-side repo slug index before repo-context actions can work. Build it from existing registered repos using `window.api.gh.repoSlug({ repoPath })`, cache it by `repo.id`, and match `row.content.repository` against `${owner}/${repo}`. If slug resolution fails for a repo, exclude that repo from the index and keep the unknown-repo fallback. When a match exists, the resolved `repo.id` is stamped onto a `GitHubWorkItem`-compatible object before opening `GitHubItemDialog` or calling `launchWorkItemDirect`. When no match exists, the dialog still opens from slug-backed details but Start work stays blocked behind the add-repo prompt. Do **not** fabricate a `repoId` for unknown-repo rows; extend `GitHubItemDialog` to accept a Project-origin item shape where `repoId`/`repoPath` are optional and `owner`/`repo` are required. Project field edits and inline built-in assignee edits do **not** require the repo to be registered because they address GitHub by `projectId`/`itemId` or row slug. - -Draft issues and redacted items never start work directly in v1 because they have no repo/number. Draft titles are non-interactive and show the draft body in a hover card; redacted rows show no local action. - -#### Dialog editing from Project rows - -Project mode opens the existing `GitHubItemDialog`, but it supplies a row-origin context: `owner`, `repo`, `number`, `itemType`, `projectId`, `projectItemId`, the active table `cacheKey`, and the optional matched `repoPath`/`repoId`. That context lets the dialog choose slug-addressed read/write IPC when it has no local repo path, and slug-addressed mutation IPC even when a repo path exists. Without it, a Project row from a repo that is not the active local workspace could edit the wrong issue number. - -Dialog-only edits: - -- Issue title/body: `gh:updateIssueBySlug`. -- PR title/body: `gh:updatePullRequestBySlug`. -- Labels/assignees: slug-addressed issue mutation for issues; PR label/assignee edits also use issue endpoints because GitHub PRs are issues for labels/assignees. -- Label and assignee pickers: `gh:listLabelsBySlug` / `gh:listAssignableUsersBySlug`, not the repoPath-based picker loaders. Reads may use local repo context for PR files/checks when a match exists, but picker option sources must stay slug-addressed for consistency with the write target. -- Comments: create/edit/delete via issue-comment endpoints. Existing PR review-thread reply controls remain available only on PR details loaded through the current PR comment surface; editing/deleting review comments is out of Project mode scope. - -The dialog patches both its local detail state and the Project table cache when a row-origin context is present. A successful title edit updates `row.content.title`; successful labels/assignees update both `row.content.*` and matching built-in field cells if those columns are visible. Issue/PR body and comment edits do not change list rows unless a future row column depends on them; draft issue bodies live in `row.content.body` only for the draft hover card and are read-only in v1. - -## Project Selection - -The picker is **in v1**, not a follow-up. It is the only way to switch projects, so it has to land alongside Project mode. - -### Picker UX - -Clicking the project picker button opens a popover: - -- Search input (filters by title, owner, or number substring). -- **Pinned projects** — user-favorited, local, persisted. -- **Recent projects** — last 10 opened, MRU order. -- **Browse all** — lazily loads projects owned by the viewer and projects owned by their orgs (paginated, cached 5 minutes). -- **Add by URL or `owner/number`** — text input at the bottom. - -Selecting a project row does not immediately lock the view unless Orca already has `lastViewByProject[key].viewId`. If no last view exists, the picker opens a second lightweight step listing that project's `TABLE_LAYOUT` views by name, plus disabled rows for unsupported Board/Roadmap views. Choosing a view closes the picker and fetches that table. This is still not a header view switcher: the choice lives in the picker because v1 needs exactly one selected view, but the user must be able to choose `Me` without depending on GitHub's first-view order. - -Dedup rules across sections (a project must appear in at most one list): - -- Pinned renders first, as-is. -- Recent filters out anything already in Pinned. -- Browse-all filters out anything in Pinned or Recent. - -Why: a project showing up twice in the picker is confusing and makes pin/unpin state ambiguous. - -**Pin-before-open.** Pins are only creatable from a project that has been successfully opened at least once (i.e., has an entry in `lastViewByProject`); paste-to-add attempts that fail classification or unused browse rows cannot create a pin. - -**Zombie pins.** A pinned project key may fail to appear in `listAccessibleProjects` (lost access, project archived/deleted, transient auth) or resolve `not_found` when clicked. Pins are **not auto-removed** — silently dropping a pin loses user intent. Instead, render the pin row with a warning icon and an inline `Remove pin` affordance; clicking it is the only path that mutates `settings.githubProjects.pinned`. - -**Auth/scope errors in the picker.** When `gh:listAccessibleProjects` resolves `{ ok: false, error: { type: 'auth_required' | 'scope_missing' | ... } }`, the picker renders an inline banner with the same copy and `Copy command` affordance as the table-level `error-auth` / `error-scope` states. Pinned projects still render (they come from settings, not the network) so the user can retry a known-good pin once auth is fixed. - -Selecting a project: - -1. Resolves owner + ownerType + number (via `gh:resolveProjectRef` for paste inputs, direct for list clicks). -2. Resolves the selected view (URL `viewNumber`, existing `lastViewByProject[key].viewId`, or explicit choice from the picker view step). -3. Renders the table (or an Interaction State — see next section). -4. Adds the project to MRU, updates `activeProject`, and writes `lastViewByProject[key].viewId`. - -### Discovery query - -The query shape is: - -```graphql -query { - viewer { - login - projectsV2(first: 50) { nodes { id number title url owner { ... on Organization { login } ... on User { login } } } } - organizations(first: 30) { - nodes { - login - projectsV2(first: 50) { nodes { id number title url } } - } - } - } -} -``` - -Production discovery paginates `viewer.projectsV2`, `viewer.organizations`, and each organization's `projectsV2`. Cap at 50 orgs and 100 projects per owner for v1; if the org cap is hit, render a small "Some organizations are not shown" footer in Browse all rather than claiming exhaustive results. - -### Paste-to-add parser - -Accepts: - -- `https://github.com/orgs/{owner}/projects/{n}` -- `https://github.com/users/{owner}/projects/{n}` -- `https://github.com/orgs/{owner}/projects/{n}/views/{viewNumber}` -- `https://github.com/users/{owner}/projects/{n}/views/{viewNumber}` -- `owner/number` shorthand (e.g. `stablyai/3`) - -Parsed locally, then confirmed with a project fetch. URLs with `/views/{viewNumber}` seed `viewNumber` directly; bare project URLs and `owner/number` use `lastViewByProject[key].viewId` if present, otherwise open the view-pick step. Failure surfaces the standard `not_found` Interaction State. - -### Persistence - -Stored through the existing settings IPC: - -```ts -settings.githubProjects = { - pinned: Array<{ owner: string; ownerType: GitHubProjectOwnerType; number: number }> // cap 20 - recent: Array<{ owner: string; ownerType: GitHubProjectOwnerType; number: number; lastOpenedAt: string }> // cap 10, MRU - lastViewByProject: Record // keyed by `${ownerType}:${owner}:${number}` - activeProject: { owner: string; ownerType: GitHubProjectOwnerType; number: number } | null -} -``` - -Dedup key: `${ownerType}:${owner}:${number}`. - -`settings.githubProjects` must be written as a complete object unless this feature also adds an explicit deep-merge path for that field in both `Store.updateSettings()` and the renderer settings slice. The existing settings update path deep-merges only `notifications` and `telemetry`; partial writes such as `{ githubProjects: { pinned } }` would otherwise clobber `recent`, `lastViewByProject`, and `activeProject`. Picker code should therefore read the current `settings.githubProjects`, apply the local mutation, enforce caps/dedup, and call `updateSettings({ githubProjects: nextGithubProjects })`. - -### Settings defaults and migration - -Defaults for upgraded installs (the key is absent on any profile created before this feature): - -- `settings.githubProjects` defaults to `{ pinned: [], recent: [], lastViewByProject: {}, activeProject: null }`. -- `settings.experimentalGithubProjectView` defaults to `false`. - -Defaults are applied by the existing `{ ...defaults.settings, ...parsed.settings }` merge in `src/main/persistence.ts`. Add the `githubProjects` default to `getDefaultSettings()` so upgraded installs hydrate it automatically. No one-shot `_githubProjectsInitialized` flag is needed unless a later migration mutates non-empty user data; this rollout only supplies a missing default. - -Toggle-gating in the renderer reads: - -```ts -settings?.githubProjects?.activeProject != null || - settings?.experimentalGithubProjectView === true -``` - -Use the repo's existing flat experimental settings pattern. Add `experimentalGithubProjectView: boolean` to `GlobalSettings` and `getDefaultSettings()`, not a nested `settings.experimental` object. - -Optional-chaining is required on both `settings` and `githubProjects`: renderer state initializes `settings` as `null`, and tests/upgraded sessions can briefly observe missing nested settings before hydration. Reading `.activeProject` off `undefined` would throw and blank the Tasks page. - -### First-run - -When `activeProject` is null, Project mode renders a "Choose a project" CTA that opens the picker — no auto-selection. Fresh installs should not surface unrelated project content. (And per the toggle-gating rule above, the user won't even see the Project mode toggle until they've picked at least once or flipped the experimental flag.) - -## Interaction States - -One consolidated table. All states render inside the Project mode body; placement and recovery below. - -| State | Trigger | Copy | Placement | Recovery | -|---|---|---|---|---| -| `loading` | Fetch in flight | Skeleton rows + "Loading project view…" | Replace list | — | -| `empty` | Fetch OK, 0 rows | "No items match this view's filter." | Replace list | `Open in GitHub` link | -| `error-auth` (`auth_required`) | `gh` not logged in | "Sign in to GitHub to load project tasks. Run `gh auth login`." | Banner above list | `Copy command` button | -| `error-scope` (`scope_missing`) | Token lacks `project`, `read:org`, or `repo` for private repo-backed items | "GitHub project access needs additional scopes. Run `gh auth refresh -s project -s read:org -s repo`." | Banner above list | `Copy command` button | -| `error-notfound` (`not_found`) | Project or view missing | "Could not find `{owner}/{number}`." or "Could not find the selected view." | Replace list | `Choose another project` (opens picker) | -| `error-drift` (`schema_drift`) | Required envelope field missing/null on success (see Error Handling), **including** null/undefined `items.totalCount` | "Could not read this project view." | Replace list | `Open in GitHub`, `Copy error details` | -| `too-large-for-v1` (`too_large`) | `totalCount > 500` | "This view has {N} items — too large to render in Orca. Narrow the view's filter on GitHub." | Replace list | `Open in GitHub` (links to selected view URL) | -| `unsupported-layout` | `view.layout !== 'TABLE_LAYOUT'` | "Orca only renders table views yet. This is a {Board/Roadmap} view." | Replace list | `Open in GitHub` | -| `repo-not-in-orca` | User tries to start work from a row whose repo isn't registered | "`{repo}` isn't added to Orca. Add it to start work, or open in GitHub." | Modal prompt | `Add repo`, `Open in GitHub`, `Cancel` | -| `parent-dropped` | `parentFieldDropped === true` after retry fallback | "Sub-issue data is unavailable for your token." | Dismissable toast, once per session | — | -| `mutation-failed` | Field, title/body, labels/assignees, or comment write returns `{ ok: false }` | Error-specific message from the classified envelope | Toast near the edited surface | Roll back optimistic patch; keep editor open when possible | - -**"Banner above list" vs "Replace list"**: auth/scope errors keep any previously cached rows visible so a transient auth hiccup doesn't blank the screen. Structural errors (not-found, drift, oversize, unsupported-layout) replace the list — rendering stale rows under a new picked project would lie about what the user is looking at. - -## Pagination and Size Cap - -**v1 blocks render until the view is fully paginated.** No progressive/provisional row display. - -Why: GitHub's `ProjectV2.items(orderBy:)` only supports `POSITION`, so client-side sort and grouping are only globally correct once every page is loaded. Phase A considered the "provisional sort while loading" alternative and rejected it — group headers jumping and rows reshuffling mid-scroll is worse than a loading skeleton. - -Policy: - -1. First page request returns `totalCount`. -2. **If `totalCount > 500`**: abort pagination, do not return rows, resolve IPC as `{ ok: false, error: { type: 'too_large' }, totalCount }`. Renderer shows the `too-large-for-v1` state. Copy tells the user to narrow the view's filter on GitHub. Why 500: well above typical personal views (the `Me` prototype view has 300 items) and comfortably under a threshold where a single synchronous render would stall the UI thread. -3. **If `totalCount <= 500`**: paginate sequentially (`first: 100`, follow `pageInfo.endCursor`) until `hasNextPage === false`, then resolve with the full table. -4. **No background auto-pagination** after an initial partial render — the initial render doesn't happen until pagination is complete. - -Page size is a main-process implementation detail (`100`), not exposed across IPC. Cache entries hold the complete row set. Grouping and sorting are deterministic renderer/shared logic derived from `selectedView`; they should not depend on fetch completion order. - -## Error Handling - -Every failure classifies into `GitHubProjectViewErrorType` and resolves through `GetProjectViewTableResult`. The main process **does not throw across IPC** for expected failures — matches the `ListWorkItemsResult` convention so renderer error handling stays uniform. - -**`schema_drift` trigger rule.** `schema_drift` fires **only** when a successful GraphQL response is missing/null on a required envelope field. The explicit list: `items.totalCount`, `items.pageInfo.hasNextPage`, `items.nodes`, `view.id`, `view.layout`, and `items.pageInfo.endCursor` only when `hasNextPage === true`. `endCursor` may legitimately be null on empty or final pages, so treating it as always-required would turn valid empty views into drift errors. Unknown `__typename` on a `fieldValues` node, unknown `dataType` on a field, and redacted Project items are **not** drift — those render through the fallback/disabled-row rules above. Why: we reserve `schema_drift` for cases where the response is structurally unusable; gracefully ignoring unknown union members is a forward-compat feature, not an error. - -Special case: if `items.totalCount` is null/undefined on an otherwise-successful response, classify as `schema_drift` — not `too_large`, not `empty`. Why: `too_large` needs a real count to show ("N items — too large"), and `empty` would lie about what happened. - -On GraphQL drift, the main process logs `path`, `type`, and `extensions.code` to the main log. It **never** logs the token, and never logs raw command stdout that might embed one. The `error-drift` state surfaces a `Copy error details` affordance populated from the structured `details` field so users can paste into a bug report. - -## SSH and Cross-Platform Notes - -- Continue invoking `gh` from the main process with a repo-aware `cwd` when a repo context exists. Org-level ProjectV2 calls don't need a local repo path, but subprocess invocation must still work when the active workspace is SSH-backed. -- Build `gh` argv as arrays; pass the GraphQL query as an argument. Match existing `execFile` patterns — no shell-specific quoting. -- Use `path` utilities for any local files introduced for query fixtures or tests. -- Platform shortcut labels follow existing rules if shortcuts are added later. - -## Testing Plan - -### Unit tests — `src/main/github/project-view.test.ts` - -- View config normalization for `ProjectV2Field`, `ProjectV2SingleSelectField`, `ProjectV2IterationField`. -- Item normalization for issues, PRs, draft issues, and redacted/null-content items. -- Field values keyed by `field.id`. -- `parent`-field retry — three explicit cases, each must trigger the retry-without-`parent` path and set `parentFieldDropped: true` on the result: - (a) GraphQL error `type: FIELD_NOT_FOUND` on path ending in `parent`. - (b) GraphQL error `type: UNDEFINED_FIELD` on path ending in `parent`. - (c) `preview` header error from the `gh` subprocess stderr (Issue.parent behind a preview gate). -- `parent` fallback retries the whole table, not only the failing page, so no result mixes parent data with dropped-parent rows. -- Oversize guard: mocked `totalCount: 501` returns `too_large` without fetching pages. -- Error classification: auth, scope, not-found, drift, rate-limited. -- Null `view.filter` normalizes to `''` and still fetches unfiltered items. -- Null `items.pageInfo.endCursor` is accepted when `hasNextPage === false` and classified as `schema_drift` only when `hasNextPage === true`. -- View/field pagination does not silently truncate `views` or `fields`. -- `item.fieldValues.pageInfo.hasNextPage === true` returns `schema_drift`. -- `updateProjectItemFieldValue` sends the correct GraphQL value shape for single-select, iteration, text, number, and date, and `clearProjectItemFieldValue` clears the same field ids. -- Slug-addressed issue/PR mutations use the row's `owner/repo` and never call `getIssueOwnerRepo(repoPath)`. -- Slug-addressed details fetch returns body/comments/labels/assignees/participants for issue and PR rows without requiring a registered local repo, and omits PR files/checks/review-thread data in that mode. -- Issue/PR body edits use REST PATCH endpoints and classify validation/scope/not-found failures. -- Issue comment create/edit/delete helpers work for both issue rows and PR rows because both use GitHub issue comments. -- Slug-addressed label and assignable-user lookups use `repos/{owner}/{repo}` APIs and never call repoPath-based issue-source resolution. -- Draft issues normalize `body` into `row.content.body`; issue/PR rows set it to `null` unless populated by a later detail patch. - -### Renderer/shared tests — `src/renderer/src/components/github-project/*.test.tsx` or a focused shared helper test - -- Group key generation for iteration, single-select, assignees, text, empty. -- Sort comparator using single-select option order and iteration order. -- Equal sort-field values preserve `row.position`; malformed rows without `position` fall back to item number then title. -- Repo slug index maps `row.content.repository` to the correct registered `repo.id`; unresolved repos take the `repo-not-in-orca` path only when Start work is invoked. -- Unknown-repo title clicks still open `GitHubItemDialog` with slug-backed conversation details; only Start work is blocked by `repo-not-in-orca`. -- Unknown-repo dialog context does not fabricate `GitHubWorkItem.repoId`; components branch on Project-origin context for slug-backed reads and disabled Start work. -- Inline field editors call update vs clear correctly and re-run grouping/sorting after optimistic patches. -- Inline assignee edits call slug-addressed issue mutation APIs and patch `row.content.assignees`. -- Dialog edits from Project rows call slug-addressed mutation APIs and patch the Project table cache when title, labels, or assignees change. -- Dialog label/assignee pickers call slug-addressed lookup APIs when row-origin context is present. - -### Store tests — `src/renderer/src/store/slices/github.test.ts` - -- `projectViewCache` writes use `ProjectViewCacheEntry` (same field names as `CacheEntry`, distinct type, `error` typed as `GitHubProjectViewError`). -- Inflight dedup via `inflightProjectViewRequests`: concurrent non-forcing calls share a promise; a forcing call waits for an in-flight non-forcing call to settle, then issues a new fetch. -- Concurrency gate: a project-view fetch and a work-items fetch both consume slots from the shared gate. -- A failed fetch stores a typed `error` without breaking existing work-item cache entries. -- A failed mutation rolls back only the affected cached row and does not resurrect stale rows after a rapid project switch. -- `githubProjects` settings updates write the full nested object, or the settings layer deep-merges it, so updating pins cannot clobber recent/activeProject/lastViewByProject. -- **Render-from-cache invariant**: renderer components must read rows reactively from `projectViewCache[key]`, never from the awaited return value of `fetchProjectViewTable`. Add a test: a rapid project switch (A → B) while the A fetch is still in flight must not render A's rows after B's cache entry is populated. Why: awaiting the returned promise and rendering its payload directly would race — the in-flight A resolves after B and paints stale rows. - -### E2E — `tests/e2e/tasks-page.spec.ts` (or a focused `project-view.spec.ts`) - -- Open Tasks → GitHub source → Project mode gated off for fresh install. -- Enable experimental flag → Project mode visible → picker opens → paste `stablyai/3` → choose `Me` in the picker view step → view renders with `Sprint` groups. -- Oversize state renders with `Open in GitHub` when `totalCount > 500`. -- Click row title with known repo → work-item dialog opens with repo-backed details. Click row title with unknown repo → work-item dialog opens with slug-backed conversation details; clicking Start work shows the `repo-not-in-orca` prompt. -- Edit a single-select field inline → row optimistically moves according to the configured group/sort, mocked mutation succeeds, and cache keeps the new value. -- Edit assignees inline → mocked slug-addressed issue mutation succeeds and the row's assignee cell updates. -- Edit a field with a mocked mutation failure → toast renders and the row returns to its original value and position. -- From a Project row dialog, edit title/body and add/edit/delete an issue comment through slug-addressed mocks. - -Mock `gh api graphql` in E2E. The live `stablyai/3` project is a manual smoke, not a CI dependency. - -## Rollout Plan - -Fewer steps than the original plan — dropping view tabs, filter editing, column management, and the View menu collapses most of the former middle phases. - -1. Land `project-view.ts` with normalization, view/field pagination, `parent`-field retry, oversize guard, ProjectV2 field mutation helpers, and unit tests. Add slug-addressed issue/PR/comment mutation helpers plus label/assignable-user lookups. Wire IPC + preload unconditionally; gate only the renderer entry point behind `settings.experimentalGithubProjectView` (default `false`) or an existing `activeProject`. -2. Add renderer slice (`projectViewCache`, `fetchProjectViewTable`, mutation actions, inflight dedup) with store tests. Add the Project mode toggle, gated on the flag or `activeProject`. -3. Build the picker (pinned / recent / browse / paste-to-add) + persistence. -4. Build `ProjectViewList` / `ProjectGroupHeader` / `ProjectRow` / `ProjectCell` + interaction-state rendering, including inline editors for editable Project fields. -5. Wire row actions into existing work-item dialog and worktree-start flow. Add row-origin context to the dialog so unknown-repo rows can read conversation details by slug, and title/body, labels/assignees, and comments use slug-addressed mutations and patch the Project table cache. -6. Manual QA against `stablyai/3` `Me` view. Flip the experimental flag on by default. - -## Open Questions - -1. **Should header-level view switching land in v2?** v1 can choose a view from the picker, but there is no persistent tab strip or header dropdown for rapid view changes. Deferred because always-visible switching doubles the state space (active project × active view) and complicates "block until fully paginated" when users rapidly switch. Revisit after usage data shows how often users want to swap views without swapping projects. -2. **Should filter editing land in v2?** Currently the view's GitHub filter is authoritative. Adding a local filter override re-introduces the "viewFilter / draftFilter / appliedFilter" tri-state the original design considered and the stale-filter surprise ("Orca looks like Me but is actually using my old local query") that motivated dropping it. If we do add it, the local override must be ephemeral (not persisted) and visibly distinct from the view's filter. -3. **How does "Show agent sessions" interact with project rows when implemented?** Leaning toward: a per-row indicator sourced from Orca's local session store, keyed on repo + issue number, rendered as a leading badge on the title cell. Does not touch the GraphQL query. Needs its own design pass. -4. **Board-layout vertical grouping.** If/when a Board-layout renderer is added, restore `verticalGroupByFields` to the query and type. -5. **Should v2 support Project item creation and rank/position editing?** v1 edits existing row values but does not create Project items, reorder rows, or change view configuration. Those require different UX and mutation semantics from field-value edits because they can change the underlying plan shape for everyone using the GitHub Project. -6. **Does 500 hold up?** The `Me` view returns 300. If multi-repo engineering views regularly exceed 500, we need either a higher cap with a deferred-sort path or a follow-up "first N then paginate on demand" mode. Instrument the `too_large` state frequency before tuning. diff --git a/docs/design/mockups/github-project-view.html b/docs/design/mockups/github-project-view.html deleted file mode 100644 index 80dd14642b3..00000000000 --- a/docs/design/mockups/github-project-view.html +++ /dev/null @@ -1,611 +0,0 @@ - - - - -Mockup — GitHub Project view (Tasks) - - - -
- - -
-
- - -
-
- -
- - -
-
-
- - -
-
-
- - -
- - View:Me - assignee:@me is:open no:triage-group - 127 items -
- -
- - -
- -
-
Title
-
Status
-
Priority
-
Size
-
Sprint
-
Repository
-
Assignees
-
- - -
-
- ▾ - Sprint 42 - Apr 28 – May 12 - Current - 4 -
- -
-
- - - Ship grouped Project view rendering - stablyai/orca#1402 - -
-
In progress
-
P0
-
S
-
Sprint 42 Apr 28 – May 12
-
stablyai/orca
-
JL
-
- - -
-
- -
-
- - - feat(project-view): normalize GraphQL unions - stablyai/orca#1411 - -
-
In review
-
P0
-
M
-
Sprint 42 Apr 28 – May 12
-
stablyai/orca
-
JLKT
-
- - -
-
- -
-
- - - Picker: pinned / recent / browse + paste-to-add - stablyai/orca#1415 - -
-
Todo
-
P1
-
M
-
Sprint 42 Apr 28 – May 12
-
stablyai/orca
-
—
-
- - -
-
- -
-
- - - Block render until full pagination; 500-item cap - stablyai/orca#1418 - -
-
Todo
-
P2
-
XS
-
Sprint 42 Apr 28 – May 12
-
stablyai/orca
-
JL
-
- - -
-
-
- - -
-
- ▾ - Sprint 43 - May 13 – May 26 - 2 -
- -
-
- - - Design: view switching (Phase B) - stablyai/orca#1421 - -
-
Todo
-
P1
-
L
-
Sprint 43 May 13 – May 26
-
stablyai/orca
-
JL
-
- - -
-
- -
-
- - - Spike: board-layout vertical grouping feasibility - stablyai/orca#1425 - -
-
Todo
-
P2
-
S
-
Sprint 43 May 13 – May 26
-
stablyai/orca
-
MF
-
- - -
-
-
- - -
- -
- -
- Read-only. Rows, columns, grouping, and sort all come from GitHub. To - change the view, edit it on GitHub or pick a different project. - Primary click opens Orca's work-item dialog; hover actions open in GitHub or start a worktree. -
-
- -
-
- - - -
Too-large state (totalCount > 500)
-
-
This view has 873 items — too large to render in Orca.
-
Narrow the view's filter on GitHub, then retry.
-
- - -
-
- -
Unsupported layout (Board / Roadmap)
-
-
Orca only renders table views yet.
-
This is a Board view. Switch to a table view or open in GitHub.
-
- -
-
- -
Auth / scope error (banner above list, cached rows remain)
- - -
- - - - - - - diff --git a/src/main/git/runner.ts b/src/main/git/runner.ts index 9bd10e3dac5..1be5acc9415 100644 --- a/src/main/git/runner.ts +++ b/src/main/git/runner.ts @@ -70,13 +70,26 @@ function translateArgsForWsl(args: string[]): string[] { function resolveCommand( command: string, args: string[], - cwd: string | undefined + cwd: string | undefined, + wslDistroOverride?: string ): ResolvedCommand { - if (!cwd || process.platform !== 'win32') { + if (process.platform !== 'win32') { return { binary: command, args, cwd, wsl: null } } - const wsl = parseWslPath(cwd) + // Why: global gh callers (rate_limit, listAccessibleProjects) have no + // meaningful cwd to derive a WSL distro from. On WSL-only Windows setups, + // gh.exe isn't on the host PATH and the spawn fails with ENOENT. Allow + // callers to pass a distro hint so we can route through wsl.exe regardless. + // TODO(wsl-default-distro): the codebase currently has no persistent + // "default WSL distro" setting — distros are derived from individual repo + // paths. Until such a setting exists, global gh callers without an explicit + // override silently fall back to host gh.exe, which on WSL-only Windows + // installs will ENOENT. The wslDistroOverride parameter is the hook for + // wiring a future setting in without re-plumbing the runner. + const cwdWsl = cwd ? parseWslPath(cwd) : null + const wsl: WslPathInfo | null = + cwdWsl ?? (wslDistroOverride ? { distro: wslDistroOverride, linuxPath: '' } : null) if (!wsl) { return { binary: command, args, cwd, wsl: null } } @@ -89,8 +102,13 @@ function resolveCommand( const escapedArgs = translatedArgs.map( (a) => `'${a.replace(/'/g, "'\\''")}'` ) - const escapedCwd = wsl.linuxPath.replace(/'/g, "'\\''") - const shellCmd = `cd '${escapedCwd}' && ${command} ${escapedArgs.join(' ')}` + // Why: when cwd is supplied as a WSL UNC path, prepend `cd &&` + // so the command runs in the expected directory. When the caller only + // supplied a distro override (no cwd), skip the cd entirely — the gh CLI + // doesn't need a particular cwd for global calls like `api rate_limit`. + const shellCmd = cwdWsl + ? `cd '${cwdWsl.linuxPath.replace(/'/g, "'\\''")}' && ${command} ${escapedArgs.join(' ')}` + : `${command} ${escapedArgs.join(' ')}` return { binary: 'wsl.exe', @@ -193,7 +211,12 @@ export function gitSpawn( // serves both repo-scoped and global callers and we stop having two spawn // sites (the other one — a plain execFileAsync in project-view.ts — bypasses // retry/backoff and any future quota tracker). -type GhExecOptions = Omit & { cwd?: string } +// Why: `wslDistro` is an explicit hint for global (cwd-less) gh callers on +// WSL-only Windows installs where gh.exe isn't on the host PATH. When set, +// resolveCommand routes the spawn through `wsl.exe -d -- gh ...` +// even without a UNC cwd to parse a distro from. Repo-scoped callers should +// keep using cwd — the distro derives from the path automatically there. +type GhExecOptions = Omit & { cwd?: string; wslDistro?: string } /** * Extract stderr from an execFile rejection. @@ -293,6 +316,13 @@ export function isTransientGhError(stderr: string): boolean { // total attempts = length + 1. const GH_RETRY_DELAYS_MS = [250, 1000] as const +// Why: the upstream Retry-After header is server-suggested but unbounded — +// GitHub has been observed to send tens-of-seconds values on rare incidents, +// and a malicious or misconfigured proxy could send anything. Cap the wait +// at 30s so a single transient gh call can never block the IPC main thread +// for longer than the user's patience budget for an interactive action. +const GH_RETRY_AFTER_MAX_MS = 30_000 + async function sleep(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)) } @@ -309,7 +339,7 @@ export async function ghExecFileAsync( args: string[], options: GhExecOptions = {} ): Promise<{ stdout: string; stderr: string }> { - const resolved = resolveCommand('gh', args, options.cwd) + const resolved = resolveCommand('gh', args, options.cwd, options.wslDistro) let lastError: unknown for (let attempt = 0; attempt <= GH_RETRY_DELAYS_MS.length; attempt++) { try { @@ -326,7 +356,20 @@ export async function ghExecFileAsync( const { stderr } = extractExecError(err) const isLastAttempt = attempt >= GH_RETRY_DELAYS_MS.length if (!isLastAttempt && isTransientGhError(stderr)) { - await sleep(GH_RETRY_DELAYS_MS[attempt]) + // Why: when the upstream surfaced a Retry-After (e.g. on a transient + // 5xx that GitHub explicitly recommends backing off for), honor it + // instead of using our default backoff — sleeping less than the + // server suggests just earns another failure and burns our retry + // budget. Cap at GH_RETRY_AFTER_MAX_MS so a pathologically large + // hint can't block IPC for minutes; if the real wait is longer, the + // attempt will fail again and the error will propagate to the UI + // where the user can see it. + const retryAfterMs = parseRetryAfterMs(stderr) + const delayMs = + retryAfterMs !== null + ? Math.min(retryAfterMs, GH_RETRY_AFTER_MAX_MS) + : GH_RETRY_DELAYS_MS[attempt] + await sleep(delayMs) continue } throw err diff --git a/src/main/github/project-view.ts b/src/main/github/project-view.ts index 021a8505158..305f5152f01 100644 --- a/src/main/github/project-view.ts +++ b/src/main/github/project-view.ts @@ -1,29 +1,34 @@ /* eslint-disable max-lines -- Why: ProjectV2 GraphQL has its own normalization -layer, retry policy (parent-field dance), paste-to-add parser, discovery -pagination, and slug-addressed mutation helpers. Co-locating them keeps the -retry/classify/normalize contract reviewable as one surface. Splitting would -risk drifting classification semantics between read and write paths. */ -// Why: `ghExecFileAsync` (WSL-aware, retry-enabled) is the single spawn site -// for gh calls. The legacy plain `execFileAsync` is NOT used here — routing -// every gh call through the runner gives us transient-5xx retry, WSL path -// translation, and a single hook point for future quota tracking. -import { acquire, release } from './gh-utils' -import { extractExecError, ghExecFileAsync } from '../git/runner' -import { rateLimitGuard, noteRateLimitSpend, type RateLimitBucketKind } from './rate-limit' -import type { GitHubAssignableUser, GitHubWorkItemDetails, PRComment } from '../../shared/types' +layer, retry policy (parent-field dance), paste-to-add parser, and discovery +pagination. Co-locating the read path keeps the retry/classify/normalize +contract reviewable as one surface. Lower-level plumbing (slug validation, +error classifier, runGraphql/runRest) lives in ./project-view/internals; the +slug-addressed write path lives in ./project-view/mutations. */ +import { + acquire, + release, + extractExecError, + ghExecFileAsync, + rateLimitGuard, + noteRateLimitSpend, + classifyProjectError, + driftError, + errorsIndicateParentField, + rateLimitedError, + runGraphql, + isValidOwnerSlug, + assertSlug, + assertPositiveInt, + type GhGraphqlErrorShape, + type GraphqlVars +} from './project-view/internals' import type { - AddIssueCommentBySlugArgs, - ClearProjectItemFieldArgs, - DeleteIssueCommentBySlugArgs, GetProjectViewTableArgs, GetProjectViewTableResult, - GitHubProjectCommentMutationResult, GitHubProjectField, - GitHubProjectFieldMutationValue, GitHubProjectFieldValue, GitHubProjectIteration, GitHubProjectLabel, - GitHubProjectMutationResult, GitHubProjectOwnerType, GitHubProjectRow, GitHubProjectRowItemType, @@ -37,25 +42,35 @@ import type { GitHubProjectViewLayout, GitHubProjectViewSummary, ListAccessibleProjectsResult, - ListAssignableUsersBySlugArgs, - ListAssignableUsersBySlugResult, - ListIssueTypesBySlugArgs, - ListIssueTypesBySlugResult, - ListLabelsBySlugArgs, - ListLabelsBySlugResult, ListProjectViewsArgs, ListProjectViewsResult, - ProjectWorkItemDetailsBySlugArgs, - ProjectWorkItemDetailsBySlugResult, ResolveProjectRefArgs, - ResolveProjectRefResult, - UpdateIssueBySlugArgs, - UpdateIssueCommentBySlugArgs, - UpdateIssueTypeBySlugArgs, - UpdatePullRequestBySlugArgs, - UpdateProjectItemFieldArgs + ResolveProjectRefResult } from '../../shared/github-project-types' +// Re-export the public API so existing call sites (`./project-view`) keep +// working unchanged. The split is internal-only. +export { + isValidOwnerSlug, + isValidRepoSlug, + isValidSlug, + classifyProjectError +} from './project-view/internals' +export { + updateProjectItemFieldValue, + clearProjectItemFieldValue, + updateIssueBySlug, + updatePullRequestBySlug, + addIssueCommentBySlug, + updateIssueCommentBySlug, + deleteIssueCommentBySlug, + listLabelsBySlug, + listAssignableUsersBySlug, + listIssueTypesBySlug, + updateIssueTypeBySlug, + getWorkItemDetailsBySlug +} from './project-view/mutations' + // ─── Constants ───────────────────────────────────────────────────────── // Why: these defaults were deliberately shrunk from 50/50/100 to cut quota @@ -98,324 +113,6 @@ export function _resetProjectViewModuleState(): void { parentFieldProbeInFlight = null } -// ─── Slug validation ────────────────────────────────────────────────── - -// Why: GitHub usernames/org logins disallow `_`, `.`, leading `-`. Repo names -// are looser — they allow leading `_`, `.`, `-` (`.` and `..` reserved). We -// validate each separately so untrusted Project row data (`nameWithOwner`) -// can't become an arbitrary REST path while still accepting realistic repo -// names like `_internal` or `.github`. -const OWNER_SLUG_RE = /^[A-Za-z0-9][A-Za-z0-9-]*$/ -const REPO_SLUG_RE = /^[A-Za-z0-9._-]+$/ -const REPO_SLUG_RESERVED = new Set(['.', '..']) - -export function isValidOwnerSlug(value: unknown): value is string { - return typeof value === 'string' && value.length > 0 && OWNER_SLUG_RE.test(value) -} - -export function isValidRepoSlug(value: unknown): value is string { - return ( - typeof value === 'string' && - value.length > 0 && - REPO_SLUG_RE.test(value) && - !REPO_SLUG_RESERVED.has(value) - ) -} - -// Backwards-compatible alias for callers that don't distinguish owner vs repo. -// Prefer `isValidOwnerSlug` / `isValidRepoSlug` at new call sites. -export function isValidSlug(value: unknown): value is string { - return isValidOwnerSlug(value) || isValidRepoSlug(value) -} - -function assertSlug( - value: unknown, - field: 'owner' | 'repo' -): { ok: true; slug: string } | { ok: false; error: GitHubProjectViewError } { - const valid = field === 'owner' ? isValidOwnerSlug(value) : isValidRepoSlug(value) - if (!valid) { - return { - ok: false, - error: { - type: 'validation_error', - message: `Invalid ${field}: "${String(value)}" is not a valid GitHub slug.` - } - } - } - return { ok: true, slug: value as string } -} - -function assertPositiveInt( - value: unknown, - field: string -): { ok: true; n: number } | { ok: false; error: GitHubProjectViewError } { - if (typeof value !== 'number' || !Number.isInteger(value) || value < 1) { - return { - ok: false, - error: { - type: 'validation_error', - message: `Invalid ${field}: must be a positive integer.` - } - } - } - return { ok: true, n: value } -} - -// ─── Error classification ────────────────────────────────────────────── - -type GhGraphqlErrorShape = { - type?: string - message?: string - path?: Array - extensions?: { code?: string } -} - -function extractGraphqlErrors(stderr: string, stdout: string): GhGraphqlErrorShape[] { - // `gh api graphql` prints the response JSON to stdout even on GraphQL - // errors, and the stderr carries a summary. Try stdout first; if parsing - // fails, fall back to stderr. - const sources = [stdout, stderr] - for (const src of sources) { - if (!src) continue - try { - const parsed = JSON.parse(src) as { errors?: GhGraphqlErrorShape[] } - if (parsed.errors && parsed.errors.length > 0) { - return parsed.errors - } - } catch { - // not JSON — continue - } - } - return [] -} - -function errorsIndicateParentField(errors: GhGraphqlErrorShape[], stderr: string): boolean { - const lower = stderr.toLowerCase() - // Preview-header shape: gh returns a 4xx with "preview" in the message. - if (lower.includes('preview') && lower.includes('parent')) return true - return errors.some((e) => { - const type = (e.type ?? '').toUpperCase() - if (type === 'FIELD_NOT_FOUND' || type === 'UNDEFINED_FIELD' || type === 'FIELD_ERRORS') { - const tail = e.path?.[e.path.length - 1] - if (tail === 'parent') return true - // FIELD_ERRORS often omits `path`; match on message for the parent field. - if ((e.message ?? '').toLowerCase().includes('parent')) return true - } - return false - }) -} - -export function classifyProjectError(stderr: string, stdout: string): GitHubProjectViewError { - const errors = extractGraphqlErrors(stderr, stdout) - const s = stderr.toLowerCase() - - // Auth - if (s.includes('authentication required') || s.includes('not logged in') || s.includes('gh auth login')) { - return { - type: 'auth_required', - message: 'Sign in to GitHub to load project tasks. Run `gh auth login`.' - } - } - // Scope - if ( - s.includes('missing required scope') || - s.includes("your token has not been granted") || - (s.includes('resource not accessible') && (s.includes('project') || s.includes('scope'))) - ) { - return { - type: 'scope_missing', - message: - 'GitHub project access needs additional scopes. Run `gh auth refresh -s project -s read:org -s repo`.' - } - } - // Rate limit - if (s.includes('rate limit') || s.includes('api rate limit exceeded')) { - return { type: 'rate_limited', message: 'GitHub rate limit hit. Try again in a few minutes.' } - } - // Network — checked BEFORE not_found because DNS failures surface as - // "could not resolve host", which would otherwise be partially matched by - // the not_found branch's "could not resolve" check. Substring matching here - // is a one-way trapdoor: a real GraphQL "Could not resolve to a User…" - // error always contains "to a", so we tighten the not_found check below to - // require that token. - if ( - s.includes('timeout') || - s.includes('no such host') || - s.includes('network') || - s.includes('could not resolve host') || - s.includes('dial tcp') - ) { - return { type: 'network_error', message: 'Network error — check your connection.' } - } - // Not found - if ( - s.includes('http 404') || - errors.some((e) => (e.type ?? '').toUpperCase() === 'NOT_FOUND') || - s.includes('could not resolve to a ') - ) { - const firstNotFound = errors.find((e) => (e.type ?? '').toUpperCase() === 'NOT_FOUND') - return { - type: 'not_found', - message: 'Project or view not found.', - details: firstNotFound - ? { path: firstNotFound.path, code: firstNotFound.extensions?.code } - : undefined - } - } - // Validation - if (s.includes('http 422') || s.includes('validation failed')) { - return { type: 'validation_error', message: `Invalid request — ${stderr.trim()}` } - } - // GraphQL error with structured info - if (errors.length > 0) { - const first = errors[0] - return { - type: 'unknown', - message: first.message ?? 'Unknown GraphQL error.', - details: { path: first.path, code: first.extensions?.code } - } - } - return { type: 'unknown', message: `GitHub request failed: ${stderr.trim()}` } -} - -function driftError( - reason: string, - details?: { path?: Array; code?: string } -): GitHubProjectViewError { - return { type: 'schema_drift', message: `Could not read this project view: ${reason}.`, details } -} - -// Why: the rate-limit circuit breaker short-circuits before we spawn `gh` -// when the cached snapshot says we're below the safety floor. Synthesize the -// same `rate_limited` error shape as the post-hoc classifier so the UI path -// is unchanged. We DO NOT fail open here when there's no cached snapshot — -// rateLimitGuard already handles that case (returns `blocked:false`). -function rateLimitedError( - blocked: { remaining: number; limit: number; resetAt: number } -): GitHubProjectViewError { - const resetIn = Math.max(0, blocked.resetAt - Math.floor(Date.now() / 1000)) - const mins = Math.ceil(resetIn / 60) - return { - type: 'rate_limited', - message: `GitHub rate limit nearly exhausted (${blocked.remaining}/${blocked.limit} left). Resets in ~${mins}m.` - } -} - -// ─── Low-level gh api graphql invocation ─────────────────────────────── - -type GraphqlVars = Record - -async function runGraphql( - query: string, - vars: GraphqlVars, - cwd?: string -): Promise< - | { ok: true; data: T } - | { ok: false; error: GitHubProjectViewError; raw: { stderr: string; stdout: string } } -> { - const guard = rateLimitGuard('graphql') - if (guard.blocked) { - return { ok: false, error: rateLimitedError(guard), raw: { stderr: '', stdout: '' } } - } - // Why: build argv as an array. `-f` for strings (including numbers passed - // as strings), `-F` coerces to typed. We use `-f` uniformly and coerce in - // the query via Int! casts, because `gh` can confuse empty strings. - const args: string[] = ['api', 'graphql', '-f', `query=${query}`] - for (const [k, v] of Object.entries(vars)) { - if (typeof v === 'number' || typeof v === 'boolean') { - args.push('-F', `${k}=${String(v)}`) - } else { - args.push('-f', `${k}=${v}`) - } - } - await acquire() - noteRateLimitSpend('graphql') - try { - const { stdout, stderr } = await ghExecFileAsync(args, { - encoding: 'utf-8', - ...(cwd ? { cwd } : {}) - }) - try { - const parsed = JSON.parse(stdout) as { data?: T; errors?: GhGraphqlErrorShape[] } - if (parsed.errors && parsed.errors.length > 0) { - return { - ok: false, - error: classifyProjectError(stderr, stdout), - raw: { stderr, stdout } - } - } - if (parsed.data === undefined) { - return { - ok: false, - error: driftError('response missing data'), - raw: { stderr, stdout } - } - } - return { ok: true, data: parsed.data } - } catch (parseErr) { - return { - ok: false, - error: driftError( - `failed to parse response (${parseErr instanceof Error ? parseErr.message : String(parseErr)})` - ), - raw: { stderr, stdout } - } - } - } catch (err) { - // gh executable failures (non-zero exit). Read stderr/stdout from the - // exec rejection's explicit fields — `err.message` may truncate stderr. - const { stderr, stdout: maybeStdout } = extractExecError(err) - return { - ok: false, - error: classifyProjectError(stderr, maybeStdout), - raw: { stderr, stdout: maybeStdout } - } - } finally { - release() - } -} - -async function runRest( - args: string[], - cwd?: string, - bucket: RateLimitBucketKind = 'core', - options?: { expectEmpty?: boolean } -): Promise<{ ok: true; data: T } | { ok: false; error: GitHubProjectViewError }> { - const guard = rateLimitGuard(bucket) - if (guard.blocked) { - return { ok: false, error: rateLimitedError(guard) } - } - await acquire() - noteRateLimitSpend(bucket) - try { - const { stdout, stderr } = await ghExecFileAsync(['api', ...args], { - encoding: 'utf-8', - ...(cwd ? { cwd } : {}) - }) - // Why: 204/empty-body endpoints (DELETE label, DELETE comment) return no - // body. Treat empty stdout as success rather than misclassifying the - // unparseable response as 'unknown' — which the caller would otherwise - // need to special-case and risks masking real failures whose stderr the - // classifier also tags as 'unknown'. - if (options?.expectEmpty && stdout.trim() === '') { - return { ok: true, data: undefined as T } - } - try { - return { ok: true, data: JSON.parse(stdout) as T } - } catch { - return { - ok: false, - error: { type: 'unknown', message: `Unexpected REST response: ${stderr.trim()}` } - } - } - } catch (err) { - const { stderr, stdout: maybeStdout } = extractExecError(err) - return { ok: false, error: classifyProjectError(stderr, maybeStdout) } - } finally { - release() - } -} - // ─── Normalizers ─────────────────────────────────────────────────────── type RawProjectV2Field = { @@ -423,15 +120,15 @@ type RawProjectV2Field = { id?: string name?: string dataType?: string - options?: Array<{ id?: string; name?: string; color?: string }> + options?: { id?: string; name?: string; color?: string }[] configuration?: { - iterations?: Array<{ id?: string; title?: string; startDate?: string; duration?: number }> - completedIterations?: Array<{ + iterations?: { id?: string; title?: string; startDate?: string; duration?: number }[] + completedIterations?: { id?: string title?: string startDate?: string duration?: number - }> + }[] } } @@ -487,7 +184,7 @@ type RawUser = { } function normalizeUser(raw: RawUser | null | undefined): GitHubProjectUser | null { - if (!raw || typeof raw.login !== 'string') return null + if (!raw || typeof raw.login !== 'string') {return null} return { login: raw.login, name: raw.name ?? null, @@ -498,7 +195,7 @@ function normalizeUser(raw: RawUser | null | undefined): GitHubProjectUser | nul type RawLabel = { name?: string; color?: string } function normalizeLabel(raw: RawLabel | null | undefined): GitHubProjectLabel | null { - if (!raw || typeof raw.name !== 'string') return null + if (!raw || typeof raw.name !== 'string') {return null} return { name: raw.name, color: raw.color ?? '' } } @@ -520,11 +217,11 @@ type RawFieldValue = { } export function normalizeFieldValue(raw: RawFieldValue | null | undefined): GitHubProjectFieldValue | null { - if (!raw || !raw.field || typeof raw.field.id !== 'string') return null + if (!raw || !raw.field || typeof raw.field.id !== 'string') {return null} const fieldId = raw.field.id switch (raw.__typename) { case 'ProjectV2ItemFieldSingleSelectValue': - if (typeof raw.optionId !== 'string') return null + if (typeof raw.optionId !== 'string') {return null} return { kind: 'single-select', fieldId, @@ -533,7 +230,7 @@ export function normalizeFieldValue(raw: RawFieldValue | null | undefined): GitH color: raw.color ?? '' } case 'ProjectV2ItemFieldIterationValue': - if (typeof raw.iterationId !== 'string') return null + if (typeof raw.iterationId !== 'string') {return null} return { kind: 'iteration', fieldId, @@ -545,7 +242,7 @@ export function normalizeFieldValue(raw: RawFieldValue | null | undefined): GitH case 'ProjectV2ItemFieldTextValue': return { kind: 'text', fieldId, text: raw.text ?? '' } case 'ProjectV2ItemFieldNumberValue': - if (typeof raw.number !== 'number') return null + if (typeof raw.number !== 'number') {return null} return { kind: 'number', fieldId, number: raw.number } case 'ProjectV2ItemFieldDateValue': return { kind: 'date', fieldId, date: raw.date ?? '' } @@ -601,10 +298,10 @@ type NormalizedItemOutcome = | { ok: false; drift: GitHubProjectViewError } function mapItemType(raw: string | undefined, hasContent: boolean): GitHubProjectRowItemType { - if (raw === 'ISSUE') return 'ISSUE' - if (raw === 'PULL_REQUEST') return 'PULL_REQUEST' - if (raw === 'DRAFT_ISSUE') return 'DRAFT_ISSUE' - if (raw === 'REDACTED' || !hasContent) return 'REDACTED' + if (raw === 'ISSUE') {return 'ISSUE'} + if (raw === 'PULL_REQUEST') {return 'PULL_REQUEST'} + if (raw === 'DRAFT_ISSUE') {return 'DRAFT_ISSUE'} + if (raw === 'REDACTED' || !hasContent) {return 'REDACTED'} // Unknown item type with content — treat as redacted rather than dropping. return 'REDACTED' } @@ -771,7 +468,7 @@ type RawProjectConfig = { url?: string views?: { pageInfo?: { hasNextPage?: boolean; endCursor?: string | null } - nodes?: Array + nodes?: (RawProjectView | null)[] } } @@ -783,11 +480,11 @@ type RawProjectView = { filter?: string | null fields?: { pageInfo?: { hasNextPage?: boolean; endCursor?: string | null } - nodes?: Array + nodes?: (RawProjectV2Field | null)[] } - groupByFields?: { nodes?: Array } + groupByFields?: { nodes?: (RawProjectV2Field | null)[] } sortByFields?: { - nodes?: Array<{ direction?: string; field?: RawProjectV2Field | null } | null> + nodes?: ({ direction?: string; field?: RawProjectV2Field | null } | null)[] } } @@ -838,12 +535,12 @@ async function fetchProjectViewsPage(args: { ${FIELD_CONFIG_FRAGMENT} ` const vars: GraphqlVars = { owner: args.owner, num: args.projectNumber } - if (args.after) vars.after = args.after + if (args.after) {vars.after = args.after} const res = await runGraphql>( query, vars ) - if (!res.ok) return res + if (!res.ok) {return res} const top = res.data[root] const project = top?.projectV2 ?? null if (!project || typeof project.id !== 'string') { @@ -893,11 +590,11 @@ async function fetchViewFieldsContinuation( id?: string fields?: { pageInfo?: { hasNextPage?: boolean; endCursor?: string | null } - nodes?: Array + nodes?: (RawProjectV2Field | null)[] } } | null }>(query, { viewId, after: cursor }) - if (!res.ok) return res + if (!res.ok) {return res} const view = res.data.node ?? null if (!view) { return { ok: false, error: driftError('view disappeared during field pagination') } @@ -924,18 +621,18 @@ function finalizeView( const all = [...(raw.fields?.nodes ?? []), ...extraFields.map((f) => f as RawProjectV2Field)] for (const f of all) { const n = normalizeField(f) - if (n) fields.push(n) + if (n) {fields.push(n)} } const groupByFields: GitHubProjectField[] = [] for (const f of raw.groupByFields?.nodes ?? []) { const n = normalizeField(f) - if (n) groupByFields.push(n) + if (n) {groupByFields.push(n)} } const sortByFields: GitHubProjectSort[] = [] for (const s of raw.sortByFields?.nodes ?? []) { - if (!s || (s.direction !== 'ASC' && s.direction !== 'DESC')) continue + if (!s || (s.direction !== 'ASC' && s.direction !== 'DESC')) {continue} const n = normalizeField(s.field) - if (n) sortByFields.push({ direction: s.direction, field: n }) + if (n) {sortByFields.push({ direction: s.direction, field: n })} } return { ok: true, @@ -959,9 +656,9 @@ function matchesSelector( raw: RawProjectView, sel: { viewId?: string; viewNumber?: number; viewName?: string } ): 'none' | 'id' | 'number' | 'name' | 'default' { - if (sel.viewId && raw.id === sel.viewId) return 'id' - if (sel.viewNumber !== undefined && raw.number === sel.viewNumber) return 'number' - if (sel.viewName && raw.name === sel.viewName) return 'name' + if (sel.viewId && raw.id === sel.viewId) {return 'id'} + if (sel.viewNumber !== undefined && raw.number === sel.viewNumber) {return 'number'} + if (sel.viewName && raw.name === sel.viewName) {return 'name'} if ( sel.viewId === undefined && sel.viewNumber === undefined && @@ -978,7 +675,7 @@ function matchesSelector( type RawItemsPage = { totalCount?: number pageInfo?: { hasNextPage?: boolean; endCursor?: string | null } - nodes?: Array + nodes?: (RawItem | null)[] } // Why: runGraphql returns the classified error but not the raw GraphQL @@ -1029,7 +726,7 @@ async function fetchItemsPageWithRaw(args: { argsArr.push('-F', `num=${args.projectNumber}`) argsArr.push('-f', `q=${args.query}`) argsArr.push('-F', `first=${args.first}`) - if (args.after) argsArr.push('-f', `after=${args.after}`) + if (args.after) {argsArr.push('-f', `after=${args.after}`)} const guard = rateLimitGuard('graphql') if (guard.blocked) { @@ -1124,34 +821,42 @@ async function fetchAllItems(args: { > { // Why: if another caller is currently probing whether Issue.parent is // supported, await its decision so we don't fire a duplicate with-parent - // probe and don't capture a stale includeParent. + // probe and don't capture a stale includeParent. We must re-read + // parentFieldRetried AFTER awaiting because the probe may have flipped it. if (parentFieldProbeInFlight) { await parentFieldProbeInFlight.catch(() => {}) } let includeParent = !parentFieldRetried let parentFieldDropped = parentFieldRetried // First page — single-flight the with-parent attempt so concurrent callers - // observe one probe result instead of each issuing their own. + // observe one probe result instead of each issuing their own. We must + // assign parentFieldProbeInFlight synchronously (no await between the + // null check and the assignment) so concurrent callers race on the same + // promise rather than each creating a duplicate probe. let first: Awaited> + let probePromise: Promise>> | null = null if (includeParent && !parentFieldProbeInFlight) { let resolveProbe: () => void = () => {} parentFieldProbeInFlight = new Promise((resolve) => { resolveProbe = resolve }) - try { - first = await fetchItemsPageWithRaw({ - owner: args.owner, - ownerType: args.ownerType, - projectNumber: args.projectNumber, - query: args.query, - first: ITEM_PAGE_SIZE, - after: null, - includeParent: true - }) - } finally { - resolveProbe() - parentFieldProbeInFlight = null - } + probePromise = (async () => { + try { + return await fetchItemsPageWithRaw({ + owner: args.owner, + ownerType: args.ownerType, + projectNumber: args.projectNumber, + query: args.query, + first: ITEM_PAGE_SIZE, + after: null, + includeParent: true + }) + } finally { + resolveProbe() + parentFieldProbeInFlight = null + } + })() + first = await probePromise } else { first = await fetchItemsPageWithRaw({ owner: args.owner, @@ -1185,7 +890,7 @@ async function fetchAllItems(args: { includeParent: false }) } - if (!first.ok) return { ok: false, error: first.error } + if (!first.ok) {return { ok: false, error: first.error }} // Drift guards if (first.page.totalCount === undefined || first.page.totalCount === null) { @@ -1206,18 +911,18 @@ async function fetchAllItems(args: { const rows: GitHubProjectRow[] = [] let position = 0 - const appendNodes = (nodes: Array): GitHubProjectViewError | null => { + const appendNodes = (nodes: (RawItem | null)[]): GitHubProjectViewError | null => { for (const n of nodes) { - if (!n) continue + if (!n) {continue} const norm = normalizeItem(n, position) - if (!norm.ok) return norm.drift + if (!norm.ok) {return norm.drift} rows.push(norm.row) position++ } return null } const e1 = appendNodes(first.page.nodes) - if (e1) return { ok: false, error: e1, totalCount } + if (e1) {return { ok: false, error: e1, totalCount }} // Paginate let hasNext = first.page.pageInfo.hasNextPage === true @@ -1239,7 +944,7 @@ async function fetchAllItems(args: { after: cursor as string, includeParent }) - if (!next.ok) return { ok: false, error: next.error, totalCount } + if (!next.ok) {return { ok: false, error: next.error, totalCount }} if (!Array.isArray(next.page.nodes)) { return { ok: false, error: driftError('items.nodes missing on follow page'), totalCount } } @@ -1251,7 +956,7 @@ async function fetchAllItems(args: { } } const e2 = appendNodes(next.page.nodes) - if (e2) return { ok: false, error: e2, totalCount } + if (e2) {return { ok: false, error: e2, totalCount }} hasNext = next.page.pageInfo.hasNextPage === true cursor = next.page.pageInfo.endCursor if (hasNext && typeof cursor !== 'string') { @@ -1286,7 +991,7 @@ async function fetchItemsCountOnly(args: { const res = await runGraphql< Record >(query, { owner: args.owner, num: args.projectNumber, q: args.query }) - if (!res.ok) return null + if (!res.ok) {return null} const count = res.data[root]?.projectV2?.items?.totalCount return typeof count === 'number' ? count : null } @@ -1297,9 +1002,9 @@ export async function getProjectViewTable( args: GetProjectViewTableArgs ): Promise { const ownerCheck = assertSlug(args.owner, 'owner') - if (!ownerCheck.ok) return { ok: false, error: ownerCheck.error } + if (!ownerCheck.ok) {return { ok: false, error: ownerCheck.error }} const numCheck = assertPositiveInt(args.projectNumber, 'projectNumber') - if (!numCheck.ok) return { ok: false, error: numCheck.error } + if (!numCheck.ok) {return { ok: false, error: numCheck.error }} if (args.ownerType !== 'organization' && args.ownerType !== 'user') { return { ok: false, @@ -1320,7 +1025,7 @@ export async function getProjectViewTable( projectNumber: args.projectNumber, after: cursor }) - if (!page.ok) return { ok: false, error: page.error } + if (!page.ok) {return { ok: false, error: page.error }} project = page.project for (const v of page.views) { viewsSeen.push(v) @@ -1329,7 +1034,7 @@ export async function getProjectViewTable( viewNumber: args.viewNumber, viewName: args.viewName }) - if (m === 'none') continue + if (m === 'none') {continue} // Precedence: id > number > name > default. const rank: Record = { id: 4, number: 3, name: 2, default: 1 } const currentRank = matchStrength ? rank[matchStrength] : 0 @@ -1348,10 +1053,10 @@ export async function getProjectViewTable( // selector promotes a 'default' to a stronger match within the same // selector input — those ranks only matter when the caller supplied // a selector. Bail early on any non-null selectedRaw. - if (selectedRaw) break - if (!page.hasNextPage) break + if (selectedRaw) {break} + if (!page.hasNextPage) {break} cursor = page.endCursor - if (typeof cursor !== 'string') break + if (typeof cursor !== 'string') {break} } if (!project) { return { ok: false, error: { type: 'not_found', message: 'Project not found.' } } @@ -1365,12 +1070,12 @@ export async function getProjectViewTable( const fieldsPi = selectedRaw.fields?.pageInfo if (fieldsPi?.hasNextPage === true && typeof fieldsPi.endCursor === 'string' && selectedRaw.id) { const cont = await fetchViewFieldsContinuation(selectedRaw.id, fieldsPi.endCursor) - if (!cont.ok) return { ok: false, error: cont.error } + if (!cont.ok) {return { ok: false, error: cont.error }} extraFields = cont.fields } const finalized = finalizeView(selectedRaw, extraFields) - if (!finalized.ok) return { ok: false, error: finalized.drift } + if (!finalized.ok) {return { ok: false, error: finalized.drift }} const selectedView = finalized.view // Why: an explicit empty-string override means "no filter"; treat undefined @@ -1438,23 +1143,23 @@ type RawViewerDiscovery = { login?: string projectsV2?: { pageInfo?: { hasNextPage?: boolean; endCursor?: string | null } - nodes?: Array<{ + nodes?: ({ id?: string number?: number title?: string url?: string owner?: { __typename?: string; login?: string } - } | null> + } | null)[] } organizations?: { pageInfo?: { hasNextPage?: boolean; endCursor?: string | null } - nodes?: Array<{ + nodes?: ({ login?: string projectsV2?: { pageInfo?: { hasNextPage?: boolean; endCursor?: string | null } nodes?: Array<{ id?: string; number?: number; title?: string; url?: string } | null> } - } | null> + } | null)[] } } } @@ -1491,7 +1196,7 @@ export async function listAccessibleProjects(): Promise(query, vars) if (!res.ok) { // Why: a viewer-level failure is structural — if we can't list the @@ -1503,10 +1208,10 @@ export async function listAccessibleProjects(): Promise= DISCOVERY_PROJECTS_PER_OWNER) break + if (viewerFetched >= DISCOVERY_PROJECTS_PER_OWNER) {break} } const pi = res.data.viewer.projectsV2?.pageInfo viewerMore = pi?.hasNextPage === true && typeof pi.endCursor === 'string' @@ -1558,7 +1263,7 @@ export async function listAccessibleProjects(): Promise(query, vars) if (!res.ok) { // Why: the org-listing query itself failed (not a nested projectsV2). @@ -1571,8 +1276,8 @@ export async function listAccessibleProjects(): Promise= DISCOVERY_MAX_ORGS) break + if (!org || typeof org.login !== 'string') {continue} + if (orgsSeen >= DISCOVERY_MAX_ORGS) {break} orgsSeen++ const login = org.login // Cache owner → ownerType for downstream paste/resolve even when the @@ -1582,8 +1287,8 @@ export async function listAccessibleProjects(): Promise= DISCOVERY_PROJECTS_PER_OWNER) break + if (!n || typeof n.id !== 'string' || typeof n.number !== 'number') {continue} + if (ownerCount >= DISCOVERY_PROJECTS_PER_OWNER) {break} orgProjects.push({ id: n.id, owner: login, @@ -1601,7 +1306,7 @@ export async function listAccessibleProjects(): Promise >(query, vars) - if (!res.ok) return { ok: false, error: res.error } + if (!res.ok) {return { ok: false, error: res.error }} const top = res.data[root] - if (!top) return { ok: false, error: { type: 'not_found', message: 'Owner not found.' } } + if (!top) {return { ok: false, error: { type: 'not_found', message: 'Owner not found.' } }} if (num) { const p = top.projectV2 if (!p || typeof p.id !== 'string') { @@ -1748,7 +1453,7 @@ export async function resolveProjectRef( parsed.kind === 'org' ? 'organization' : parsed.kind === 'user' ? 'user' : null // Verify by fetching project title. const ownerRes = await resolveOwnerType(parsed.owner, preferred) - if (!ownerRes.ok) return { ok: false, error: ownerRes.error } + if (!ownerRes.ok) {return { ok: false, error: ownerRes.error }} const ownerType = ownerRes.ownerType const root = ownerQueryRoot(ownerType) const query = ` @@ -1759,7 +1464,7 @@ export async function resolveProjectRef( const res = await runGraphql< Record >(query, { owner: parsed.owner, num: parsed.number }) - if (!res.ok) return { ok: false, error: res.error } + if (!res.ok) {return { ok: false, error: res.error }} const p = res.data[root]?.projectV2 if (!p || typeof p.id !== 'string') { return { ok: false, error: { type: 'not_found', message: 'Project not found.' } } @@ -1769,7 +1474,13 @@ export async function resolveProjectRef( owner: parsed.owner, ownerType, number: parsed.number, - title: p.title ?? '' + title: p.title ?? '', + // Why: forward the parsed view number from /views/{n} URLs so the + // renderer can skip the view-pick step. parsed.kind === 'bare' has no + // viewNumber (owner/number shorthand carries no view). + ...(parsed.kind !== 'bare' && parsed.viewNumber !== undefined + ? { viewNumber: parsed.viewNumber } + : {}) } } @@ -1779,9 +1490,9 @@ export async function listProjectViews( args: ListProjectViewsArgs ): Promise { const ownerCheck = assertSlug(args.owner, 'owner') - if (!ownerCheck.ok) return { ok: false, error: ownerCheck.error } + if (!ownerCheck.ok) {return { ok: false, error: ownerCheck.error }} const numCheck = assertPositiveInt(args.projectNumber, 'projectNumber') - if (!numCheck.ok) return { ok: false, error: numCheck.error } + if (!numCheck.ok) {return { ok: false, error: numCheck.error }} if (args.ownerType !== 'organization' && args.ownerType !== 'user') { return { ok: false, error: { type: 'validation_error', message: 'Invalid ownerType.' } } } @@ -1794,9 +1505,9 @@ export async function listProjectViews( projectNumber: args.projectNumber, after: cursor }) - if (!page.ok) return { ok: false, error: page.error } + if (!page.ok) {return { ok: false, error: page.error }} for (const v of page.views) { - if (typeof v.id !== 'string' || typeof v.layout !== 'string') continue + if (typeof v.id !== 'string' || typeof v.layout !== 'string') {continue} summaries.push({ id: v.id, number: typeof v.number === 'number' ? v.number : 0, @@ -1804,692 +1515,9 @@ export async function listProjectViews( layout: v.layout as GitHubProjectViewLayout }) } - if (!page.hasNextPage) break + if (!page.hasNextPage) {break} cursor = page.endCursor - if (typeof cursor !== 'string') break + if (typeof cursor !== 'string') {break} } return { ok: true, views: summaries } } - -// ─── Project field mutations ────────────────────────────────────────── - -class UnknownFieldMutationKindError extends Error { - constructor(kind: string) { - super(`Unknown project field mutation kind: ${kind}`) - } -} - -function graphqlValueForFieldMutation(value: GitHubProjectFieldMutationValue): string { - // Serialize the value fragment for the GraphQL mutation. We use GraphQL - // variables for every dynamic piece, so here we only pick the variable name - // to reference per value kind. - switch (value.kind) { - case 'single-select': - return 'singleSelectOptionId: $value' - case 'iteration': - return 'iterationId: $value' - case 'text': - return 'text: $value' - case 'number': - return 'number: $value' - case 'date': - return 'date: $value' - default: - // Why: defensive default. If a new mutation kind is added to the type - // but not handled here, returning undefined would silently produce a - // broken GraphQL query. Throw so updateProjectItemFieldValue can map it - // to a validation_error rather than dispatching a malformed mutation. - throw new UnknownFieldMutationKindError((value as { kind: string }).kind) - } -} - -function mutationValueVar(value: GitHubProjectFieldMutationValue): { - type: string - val: string | number -} { - switch (value.kind) { - case 'single-select': - return { type: 'String!', val: value.optionId } - case 'iteration': - return { type: 'String!', val: value.iterationId } - case 'text': - return { type: 'String!', val: value.text } - case 'number': - return { type: 'Float!', val: value.number } - case 'date': - return { type: 'Date!', val: value.date } - default: - // Why: see graphqlValueForFieldMutation — surface unknown kinds loudly - // instead of returning undefined and dispatching an invalid mutation. - throw new UnknownFieldMutationKindError((value as { kind: string }).kind) - } -} - -export async function updateProjectItemFieldValue( - args: UpdateProjectItemFieldArgs -): Promise { - if (!args.projectId || !args.itemId || !args.fieldId) { - return { ok: false, error: { type: 'validation_error', message: 'Missing ids.' } } - } - let valFrag: string - let valVar: { type: string; val: string | number } - try { - valFrag = graphqlValueForFieldMutation(args.value) - valVar = mutationValueVar(args.value) - } catch (err) { - if (err instanceof UnknownFieldMutationKindError) { - return { ok: false, error: { type: 'validation_error', message: err.message } } - } - throw err - } - const query = ` - mutation($projectId:ID!, $itemId:ID!, $fieldId:ID!, $value:${valVar.type}) { - updateProjectV2ItemFieldValue(input: { - projectId: $projectId - itemId: $itemId - fieldId: $fieldId - value: { ${valFrag} } - }) { projectV2Item { id } } - } - ` - const vars: GraphqlVars = { - projectId: args.projectId, - itemId: args.itemId, - fieldId: args.fieldId, - value: valVar.val - } - const res = await runGraphql(query, vars) - if (!res.ok) return { ok: false, error: res.error } - return { ok: true } -} - -export async function clearProjectItemFieldValue( - args: ClearProjectItemFieldArgs -): Promise { - if (!args.projectId || !args.itemId || !args.fieldId) { - return { ok: false, error: { type: 'validation_error', message: 'Missing ids.' } } - } - const query = ` - mutation($projectId:ID!, $itemId:ID!, $fieldId:ID!) { - clearProjectV2ItemFieldValue(input: { - projectId: $projectId - itemId: $itemId - fieldId: $fieldId - }) { projectV2Item { id } } - } - ` - const res = await runGraphql(query, { - projectId: args.projectId, - itemId: args.itemId, - fieldId: args.fieldId - }) - if (!res.ok) return { ok: false, error: res.error } - return { ok: true } -} - -// ─── Slug-addressed issue/PR mutations ──────────────────────────────── - -function validateSlugArgs( - owner: unknown, - repo: unknown -): { ok: true } | { ok: false; error: GitHubProjectViewError } { - const o = assertSlug(owner, 'owner') - if (!o.ok) return { ok: false, error: o.error } - const r = assertSlug(repo, 'repo') - if (!r.ok) return { ok: false, error: r.error } - return { ok: true } -} - -export async function updateIssueBySlug( - args: UpdateIssueBySlugArgs -): Promise { - const v = validateSlugArgs(args.owner, args.repo) - if (!v.ok) return v - const n = assertPositiveInt(args.number, 'number') - if (!n.ok) return { ok: false, error: n.error } - if (!args.updates || typeof args.updates !== 'object') { - return { ok: false, error: { type: 'validation_error', message: 'Updates required.' } } - } - const { title, body, state, addLabels, removeLabels, addAssignees, removeAssignees } = args.updates - - // Title / body / state go through PATCH /repos/{owner}/{repo}/issues/{n}. - // Labels/assignees go through their dedicated endpoints. - const base = `repos/${args.owner}/${args.repo}/issues/${args.number}` - - // 1) PATCH body - if (title !== undefined || body !== undefined || state !== undefined) { - const patchArgs: string[] = ['-X', 'PATCH', base] - if (title !== undefined) patchArgs.push('--raw-field', `title=${title}`) - if (body !== undefined) patchArgs.push('--raw-field', `body=${body}`) - if (state !== undefined) patchArgs.push('--raw-field', `state=${state}`) - const r = await runRest(patchArgs) - if (!r.ok) return { ok: false, error: r.error } - } - - // 2) Labels — collapse multi-delete fan-out into a single PUT when removing - // >1 label. PUT /labels replaces the entire label set, so we fetch the - // current labels first and compute the resulting set client-side. This - // turns an N-delete + 1-add (=N+1 calls) into 1-fetch + 1-PUT (=2 calls) - // once removeLabels has more than one entry, capping the cost at 2 even - // for a "remove all 20 labels" mutation. - const removeCount = removeLabels?.length ?? 0 - const addCount = addLabels?.length ?? 0 - if (removeCount > 1) { - type RawLabelResp = { name?: string }[] - const fetched = await runRest(['-X', 'GET', `${base}/labels`]) - if (!fetched.ok) return { ok: false, error: fetched.error } - const currentNames = new Set( - fetched.data.map((l) => l.name).filter((n): n is string => typeof n === 'string') - ) - for (const l of removeLabels ?? []) currentNames.delete(l) - for (const l of addLabels ?? []) currentNames.add(l) - const putArgs = ['-X', 'PUT', `${base}/labels`] - for (const name of currentNames) putArgs.push('--raw-field', `labels[]=${name}`) - // Why: PUT with empty `labels[]` clears all labels — that's the desired - // outcome when the user removed every label. gh requires `--raw-field` to - // construct the request body; an empty form is a valid clear-all. - const r = await runRest(putArgs) - if (!r.ok) return { ok: false, error: r.error } - } else { - if (addCount > 0) { - const restArgs = ['-X', 'POST', `${base}/labels`] - for (const l of addLabels ?? []) restArgs.push('--raw-field', `labels[]=${l}`) - const r = await runRest(restArgs) - if (!r.ok) return { ok: false, error: r.error } - } - if (removeCount === 1) { - const r = await runRest( - ['-X', 'DELETE', `${base}/labels/${encodeURIComponent(removeLabels![0])}`], - undefined, - 'core', - { expectEmpty: true } - ) - if (!r.ok && r.error.type !== 'not_found') return { ok: false, error: r.error } - } - } - - // 3) Assignees — POST and DELETE both accept arrays in a single call, so - // add/remove are at most 2 calls regardless of array size. - if (addAssignees && addAssignees.length > 0) { - const restArgs = ['-X', 'POST', `${base}/assignees`] - for (const u of addAssignees) restArgs.push('--raw-field', `assignees[]=${u}`) - const r = await runRest(restArgs) - if (!r.ok) return { ok: false, error: r.error } - } - if (removeAssignees && removeAssignees.length > 0) { - const restArgs = ['-X', 'DELETE', `${base}/assignees`] - for (const u of removeAssignees) restArgs.push('--raw-field', `assignees[]=${u}`) - const r = await runRest(restArgs) - if (!r.ok) return { ok: false, error: r.error } - } - return { ok: true } -} - -export async function updatePullRequestBySlug( - args: UpdatePullRequestBySlugArgs -): Promise { - const v = validateSlugArgs(args.owner, args.repo) - if (!v.ok) return v - const n = assertPositiveInt(args.number, 'number') - if (!n.ok) return { ok: false, error: n.error } - if (!args.updates || typeof args.updates !== 'object') { - return { ok: false, error: { type: 'validation_error', message: 'Updates required.' } } - } - const patchArgs: string[] = ['-X', 'PATCH', `repos/${args.owner}/${args.repo}/pulls/${args.number}`] - // Why: count fields explicitly rather than inferring from patchArgs.length — - // adding a future header/flag arg silently breaks an array-length check. - let fieldCount = 0 - if (args.updates.title !== undefined) { - patchArgs.push('--raw-field', `title=${args.updates.title}`) - fieldCount++ - } - if (args.updates.body !== undefined) { - patchArgs.push('--raw-field', `body=${args.updates.body}`) - fieldCount++ - } - if (fieldCount === 0) { - // No fields to update — nothing to do. - return { ok: true } - } - const r = await runRest(patchArgs) - if (!r.ok) return { ok: false, error: r.error } - return { ok: true } -} - -type RawIssueCommentResponse = { - id?: number - user?: { login?: string; avatar_url?: string; type?: string } | null - body?: string - created_at?: string - html_url?: string -} - -function mapIssueComment(data: RawIssueCommentResponse, fallbackBody: string): PRComment { - return { - id: data.id ?? Date.now(), - author: data.user?.login ?? 'You', - authorAvatarUrl: data.user?.avatar_url ?? '', - body: data.body ?? fallbackBody, - createdAt: data.created_at ?? new Date().toISOString(), - url: data.html_url ?? '', - isBot: data.user?.type === 'Bot' - } -} - -export async function addIssueCommentBySlug( - args: AddIssueCommentBySlugArgs -): Promise { - const v = validateSlugArgs(args.owner, args.repo) - if (!v.ok) return v - const n = assertPositiveInt(args.number, 'number') - if (!n.ok) return { ok: false, error: n.error } - if (typeof args.body !== 'string' || !args.body.trim()) { - return { ok: false, error: { type: 'validation_error', message: 'Comment body required.' } } - } - const r = await runRest([ - '-X', - 'POST', - `repos/${args.owner}/${args.repo}/issues/${args.number}/comments`, - '--raw-field', - `body=${args.body}` - ]) - if (!r.ok) return { ok: false, error: r.error } - return { ok: true, comment: mapIssueComment(r.data, args.body) } -} - -export async function updateIssueCommentBySlug( - args: UpdateIssueCommentBySlugArgs -): Promise { - const v = validateSlugArgs(args.owner, args.repo) - if (!v.ok) return v - const n = assertPositiveInt(args.commentId, 'commentId') - if (!n.ok) return { ok: false, error: n.error } - if (typeof args.body !== 'string' || !args.body.trim()) { - return { ok: false, error: { type: 'validation_error', message: 'Comment body required.' } } - } - const r = await runRest([ - '-X', - 'PATCH', - `repos/${args.owner}/${args.repo}/issues/comments/${args.commentId}`, - '--raw-field', - `body=${args.body}` - ]) - if (!r.ok) return { ok: false, error: r.error } - return { ok: true } -} - -export async function deleteIssueCommentBySlug( - args: DeleteIssueCommentBySlugArgs -): Promise { - const v = validateSlugArgs(args.owner, args.repo) - if (!v.ok) return v - const n = assertPositiveInt(args.commentId, 'commentId') - if (!n.ok) return { ok: false, error: n.error } - const r = await runRest( - ['-X', 'DELETE', `repos/${args.owner}/${args.repo}/issues/comments/${args.commentId}`], - undefined, - 'core', - { expectEmpty: true } - ) - if (!r.ok) return { ok: false, error: r.error } - return { ok: true } -} - -// ─── Slug-addressed picker sources ──────────────────────────────────── - -export async function listLabelsBySlug( - args: ListLabelsBySlugArgs -): Promise { - const v = validateSlugArgs(args.owner, args.repo) - if (!v.ok) return v - const guard = rateLimitGuard('core') - if (guard.blocked) return { ok: false, error: rateLimitedError(guard) } - await acquire() - // Why: `--paginate` may fan out to multiple pages; we can only reasonably - // estimate a 1-call spend up front. The next probe will reconcile. - noteRateLimitSpend('core') - try { - const { stdout } = await ghExecFileAsync( - ['api', '--paginate', `repos/${args.owner}/${args.repo}/labels`, '--jq', '.[].name'], - { encoding: 'utf-8' } - ) - return { - ok: true, - labels: stdout - .trim() - .split('\n') - .filter((l) => l.length > 0) - } - } catch (err) { - const { stderr, stdout: maybeStdout } = extractExecError(err) - return { ok: false, error: classifyProjectError(stderr, maybeStdout) } - } finally { - release() - } -} - -export async function listAssignableUsersBySlug( - args: ListAssignableUsersBySlugArgs -): Promise { - const v = validateSlugArgs(args.owner, args.repo) - if (!v.ok) return v - // Seed logins merge after the fetch so callers can include currently-visible - // assignees even if the repo participant search is sparse. - const result: GitHubAssignableUser[] = [] - const guard = rateLimitGuard('core') - if (guard.blocked) return { ok: false, error: rateLimitedError(guard) } - await acquire() - noteRateLimitSpend('core') - try { - const { stdout } = await ghExecFileAsync( - [ - 'api', - '--paginate', - `repos/${args.owner}/${args.repo}/assignees`, - '--jq', - '.[] | {login: .login, name: null, avatarUrl: .avatar_url}' - ], - { encoding: 'utf-8' } - ) - for (const line of stdout.trim().split('\n').filter((l) => l.length > 0)) { - try { - const u = JSON.parse(line) as { login?: string; avatarUrl?: string; name?: string | null } - if (typeof u.login === 'string') { - result.push({ login: u.login, name: u.name ?? null, avatarUrl: u.avatarUrl ?? '' }) - } - } catch { - // skip malformed jq line - } - } - } catch (err) { - const { stderr } = extractExecError(err) - return { ok: false, error: classifyProjectError(stderr, '') } - } finally { - release() - } - if (args.seedLogins) { - const seen = new Set(result.map((u) => u.login)) - for (const login of args.seedLogins) { - if (typeof login === 'string' && !seen.has(login)) { - result.push({ login, name: null, avatarUrl: '' }) - seen.add(login) - } - } - } - return { ok: true, users: result } -} - -// Why: Issue Types are a repo-level taxonomy (Bug/Feature/Task/etc) only -// available on repos opted into typed-issues. Empty list (or schema_drift on -// older GitHub deployments) is the legitimate "this repo doesn't use issue -// types" signal — callers should treat it as "no editor". -export async function listIssueTypesBySlug( - args: ListIssueTypesBySlugArgs -): Promise { - const v = validateSlugArgs(args.owner, args.repo) - if (!v.ok) return v - const query = ` - query($owner:String!, $repo:String!) { - repository(owner:$owner, name:$repo) { - issueTypes(first:50) { - nodes { id name color description } - } - } - } - ` - const res = await runGraphql<{ - repository?: { - issueTypes?: { - nodes?: Array<{ - id?: string - name?: string - color?: string | null - description?: string | null - } | null> - } | null - } | null - }>(query, { owner: args.owner, repo: args.repo }) - if (!res.ok) { - // Why: repos without issue types respond with a GraphQL error claiming the - // `issueTypes` field is unknown. Map that to an empty list so the UI shows - // "no editor" instead of an angry banner. - if (res.error.type === 'schema_drift' || res.error.type === 'validation_error') { - return { ok: true, types: [] } - } - return { ok: false, error: res.error } - } - const nodes = res.data.repository?.issueTypes?.nodes ?? [] - const types = nodes - .filter((n): n is NonNullable => n !== null && typeof n.id === 'string' && typeof n.name === 'string') - .map((n) => ({ - id: n.id as string, - name: n.name as string, - color: typeof n.color === 'string' ? n.color : null, - description: typeof n.description === 'string' ? n.description : null - })) - return { ok: true, types } -} - -export async function updateIssueTypeBySlug( - args: UpdateIssueTypeBySlugArgs -): Promise { - const v = validateSlugArgs(args.owner, args.repo) - if (!v.ok) return v - const n = assertPositiveInt(args.number, 'number') - if (!n.ok) return { ok: false, error: n.error } - // Why: `updateIssueIssueType` is the dedicated mutation; passing null for - // `issueTypeId` clears the type. We resolve the issue id via a lightweight - // GraphQL lookup because the REST endpoint doesn't accept issue types. - const lookup = await runGraphql<{ - repository?: { issue?: { id?: string } | null } | null - }>( - `query($owner:String!, $repo:String!, $num:Int!) { - repository(owner:$owner, name:$repo) { issue(number:$num) { id } } - }`, - { owner: args.owner, repo: args.repo, num: args.number } - ) - if (!lookup.ok) return { ok: false, error: lookup.error } - const issueId = lookup.data.repository?.issue?.id - if (!issueId) { - return { ok: false, error: { type: 'not_found', message: 'Issue not found.' } } - } - // Why: build the mutation conditionally so a null clear doesn't have to - // smuggle a null GraphQL variable through `gh api graphql -f`. The - // mutation accepts a literal `null` in the input object directly. - const query = args.issueTypeId - ? ` - mutation($issueId:ID!, $issueTypeId:ID!) { - updateIssueIssueType(input: { issueId: $issueId, issueTypeId: $issueTypeId }) { - issue { id } - } - } - ` - : ` - mutation($issueId:ID!) { - updateIssueIssueType(input: { issueId: $issueId, issueTypeId: null }) { - issue { id } - } - } - ` - const vars: GraphqlVars = args.issueTypeId - ? { issueId, issueTypeId: args.issueTypeId } - : { issueId } - const res = await runGraphql(query, vars) - if (!res.ok) return { ok: false, error: res.error } - return { ok: true } -} - -// ─── Slug-addressed work-item details ───────────────────────────────── - -export async function getWorkItemDetailsBySlug( - args: ProjectWorkItemDetailsBySlugArgs -): Promise { - const v = validateSlugArgs(args.owner, args.repo) - if (!v.ok) return v - const n = assertPositiveInt(args.number, 'number') - if (!n.ok) return { ok: false, error: n.error } - if (args.type !== 'issue' && args.type !== 'pr') { - return { ok: false, error: { type: 'validation_error', message: 'Invalid type.' } } - } - - // Single GraphQL round-trip to fetch the issue/PR summary + comments + labels + assignees. - const contentFrag = - args.type === 'issue' - ? ` - issue(number:$num) { - id number title url state stateReason updatedAt - body - author { login } - labels(first:50) { nodes { name } } - assignees(first:50) { nodes { login } } - participants(first:50) { nodes { login name avatarUrl } } - comments(first:100) { - nodes { - databaseId - author { login avatarUrl __typename } - body createdAt url - } - } - } - ` - : ` - pullRequest(number:$num) { - id number title url state isDraft updatedAt headRefName baseRefName - body - author { login } - labels(first:50) { nodes { name } } - assignees(first:50) { nodes { login } } - participants(first:50) { nodes { login name avatarUrl } } - comments(first:100) { - nodes { - databaseId - author { login avatarUrl __typename } - body createdAt url - } - } - } - ` - const query = ` - query($owner:String!, $repo:String!, $num:Int!) { - repository(owner:$owner, name:$repo) { - ${contentFrag} - } - } - ` - const res = await runGraphql<{ - repository?: { - issue?: RawContent & { - updatedAt?: string - body?: string - author?: { login?: string } | null - participants?: { nodes?: RawUser[] } - comments?: { - nodes?: Array<{ - databaseId?: number - author?: { login?: string; avatarUrl?: string; __typename?: string } | null - body?: string - createdAt?: string - url?: string - } | null> - } - } | null - pullRequest?: RawContent & { - updatedAt?: string - body?: string - headRefName?: string - baseRefName?: string - author?: { login?: string } | null - participants?: { nodes?: RawUser[] } - comments?: { - nodes?: Array<{ - databaseId?: number - author?: { login?: string; avatarUrl?: string; __typename?: string } | null - body?: string - createdAt?: string - url?: string - } | null> - } - } | null - } | null - }>(query, { owner: args.owner, repo: args.repo, num: args.number }) - if (!res.ok) return { ok: false, error: res.error } - const raw = args.type === 'issue' ? res.data.repository?.issue : res.data.repository?.pullRequest - if (!raw) { - return { ok: false, error: { type: 'not_found', message: 'Item not found.' } } - } - - const labels = (raw.labels?.nodes ?? []) - .map((l) => l?.name) - .filter((n): n is string => typeof n === 'string') - const assignees = (raw.assignees?.nodes ?? []) - .map((a) => a?.login) - .filter((l): l is string => typeof l === 'string') - const comments: PRComment[] = [] - for (const c of raw.comments?.nodes ?? []) { - if (!c || typeof c.body !== 'string') continue - comments.push({ - id: typeof c.databaseId === 'number' ? c.databaseId : Date.now(), - author: c.author?.login ?? '', - authorAvatarUrl: c.author?.avatarUrl ?? '', - body: c.body, - createdAt: typeof c.createdAt === 'string' ? c.createdAt : '', - url: typeof c.url === 'string' ? c.url : '', - isBot: c.author?.__typename === 'Bot' - }) - } - const participants: GitHubAssignableUser[] = [] - for (const p of raw.participants?.nodes ?? []) { - if (p && typeof p.login === 'string') { - participants.push({ login: p.login, name: p.name ?? null, avatarUrl: p.avatarUrl ?? '' }) - } - } - - const state: 'open' | 'closed' | 'merged' | 'draft' = - args.type === 'pr' - ? raw.isDraft - ? 'draft' - : raw.state === 'MERGED' - ? 'merged' - : raw.state === 'CLOSED' - ? 'closed' - : 'open' - : raw.state === 'CLOSED' - ? 'closed' - : 'open' - - const details: GitHubWorkItemDetails = { - item: { - id: typeof raw.id === 'string' ? raw.id : '', - type: args.type, - number: typeof raw.number === 'number' ? raw.number : args.number, - title: typeof raw.title === 'string' ? raw.title : '', - state, - url: typeof raw.url === 'string' ? raw.url : '', - labels, - updatedAt: - typeof (raw as { updatedAt?: string }).updatedAt === 'string' - ? (raw as { updatedAt: string }).updatedAt - : '', - author: - typeof (raw as { author?: { login?: string } | null }).author?.login === 'string' - ? ((raw as { author: { login: string } }).author.login as string) - : null, - branchName: - args.type === 'pr' && typeof (raw as { headRefName?: string }).headRefName === 'string' - ? ((raw as { headRefName: string }).headRefName as string) - : undefined, - baseRefName: - args.type === 'pr' && typeof (raw as { baseRefName?: string }).baseRefName === 'string' - ? ((raw as { baseRefName: string }).baseRefName as string) - : undefined - }, - body: typeof raw.body === 'string' ? raw.body : '', - comments, - participants, - // Why: PR files/checks/review-thread tabs depend on a local repo path and - // are out of Project-mode slug scope for v1. Omit them here; the dialog - // branches on their absence and hides those tabs. - ...(args.type === 'issue' ? { assignees } : {}) - } - return { ok: true, details } -} diff --git a/src/main/github/project-view/internals.ts b/src/main/github/project-view/internals.ts new file mode 100644 index 00000000000..8cd24ccb499 --- /dev/null +++ b/src/main/github/project-view/internals.ts @@ -0,0 +1,355 @@ +/* eslint-disable max-lines -- Why: shared infrastructure for project-view — +slug validation, error classification, runGraphql/runRest, and rate-limit +synthesis. Co-located so the read and write paths observe identical +classification semantics. */ +// Why: `ghExecFileAsync` (WSL-aware, retry-enabled) is the single spawn site +// for gh calls. The legacy plain `execFileAsync` is NOT used here — routing +// every gh call through the runner gives us transient-5xx retry, WSL path +// translation, and a single hook point for future quota tracking. +import { acquire, release } from '../gh-utils' +import { extractExecError, ghExecFileAsync } from '../../git/runner' +import { rateLimitGuard, noteRateLimitSpend, type RateLimitBucketKind } from '../rate-limit' +import type { GitHubProjectViewError } from '../../../shared/github-project-types' + +export { acquire, release, extractExecError, ghExecFileAsync, rateLimitGuard, noteRateLimitSpend } +export type { RateLimitBucketKind } + +// ─── Slug validation ────────────────────────────────────────────────── + +// Why: GitHub usernames/org logins disallow `_`, `.`, leading `-`. Repo names +// are looser — they allow leading `_`, `.`, `-` (`.` and `..` reserved). We +// validate each separately so untrusted Project row data (`nameWithOwner`) +// can't become an arbitrary REST path while still accepting realistic repo +// names like `_internal` or `.github`. +const OWNER_SLUG_RE = /^[A-Za-z0-9][A-Za-z0-9-]*$/ +const REPO_SLUG_RE = /^[A-Za-z0-9._-]+$/ +const REPO_SLUG_RESERVED = new Set(['.', '..']) + +export function isValidOwnerSlug(value: unknown): value is string { + return typeof value === 'string' && value.length > 0 && OWNER_SLUG_RE.test(value) +} + +export function isValidRepoSlug(value: unknown): value is string { + return ( + typeof value === 'string' && + value.length > 0 && + REPO_SLUG_RE.test(value) && + !REPO_SLUG_RESERVED.has(value) + ) +} + +// Backwards-compatible alias for callers that don't distinguish owner vs repo. +// Prefer `isValidOwnerSlug` / `isValidRepoSlug` at new call sites. +export function isValidSlug(value: unknown): value is string { + return isValidOwnerSlug(value) || isValidRepoSlug(value) +} + +export function assertSlug( + value: unknown, + field: 'owner' | 'repo' +): { ok: true; slug: string } | { ok: false; error: GitHubProjectViewError } { + const valid = field === 'owner' ? isValidOwnerSlug(value) : isValidRepoSlug(value) + if (!valid) { + return { + ok: false, + error: { + type: 'validation_error', + message: `Invalid ${field}: "${String(value)}" is not a valid GitHub slug.` + } + } + } + return { ok: true, slug: value as string } +} + +export function assertPositiveInt( + value: unknown, + field: string +): { ok: true; n: number } | { ok: false; error: GitHubProjectViewError } { + if (typeof value !== 'number' || !Number.isInteger(value) || value < 1) { + return { + ok: false, + error: { + type: 'validation_error', + message: `Invalid ${field}: must be a positive integer.` + } + } + } + return { ok: true, n: value } +} + +export function validateSlugArgs( + owner: unknown, + repo: unknown +): { ok: true } | { ok: false; error: GitHubProjectViewError } { + const o = assertSlug(owner, 'owner') + if (!o.ok) {return { ok: false, error: o.error }} + const r = assertSlug(repo, 'repo') + if (!r.ok) {return { ok: false, error: r.error }} + return { ok: true } +} + +// ─── Error classification ────────────────────────────────────────────── + +export type GhGraphqlErrorShape = { + type?: string + message?: string + path?: (string | number)[] + extensions?: { code?: string } +} + +export function extractGraphqlErrors(stderr: string, stdout: string): GhGraphqlErrorShape[] { + // `gh api graphql` prints the response JSON to stdout even on GraphQL + // errors, and the stderr carries a summary. Try stdout first; if parsing + // fails, fall back to stderr. + const sources = [stdout, stderr] + for (const src of sources) { + if (!src) {continue} + try { + const parsed = JSON.parse(src) as { errors?: GhGraphqlErrorShape[] } + if (parsed.errors && parsed.errors.length > 0) { + return parsed.errors + } + } catch { + // not JSON — continue + } + } + return [] +} + +export function errorsIndicateParentField( + errors: GhGraphqlErrorShape[], + stderr: string +): boolean { + const lower = stderr.toLowerCase() + // Preview-header shape: gh returns a 4xx with "preview" in the message. + if (lower.includes('preview') && lower.includes('parent')) {return true} + return errors.some((e) => { + const type = (e.type ?? '').toUpperCase() + if (type === 'FIELD_NOT_FOUND' || type === 'UNDEFINED_FIELD' || type === 'FIELD_ERRORS') { + const tail = e.path.at(-1) + if (tail === 'parent') {return true} + // FIELD_ERRORS often omits `path`; match on message for the parent field. + if ((e.message ?? '').toLowerCase().includes('parent')) {return true} + } + return false + }) +} + +export function classifyProjectError(stderr: string, stdout: string): GitHubProjectViewError { + const errors = extractGraphqlErrors(stderr, stdout) + const s = stderr.toLowerCase() + + // Auth + if (s.includes('authentication required') || s.includes('not logged in') || s.includes('gh auth login')) { + return { + type: 'auth_required', + message: 'Sign in to GitHub to load project tasks. Run `gh auth login`.' + } + } + // Scope + if ( + s.includes('missing required scope') || + s.includes("your token has not been granted") || + (s.includes('resource not accessible') && (s.includes('project') || s.includes('scope'))) + ) { + return { + type: 'scope_missing', + message: + 'GitHub project access needs additional scopes. Run `gh auth refresh -s project -s read:org -s repo`.' + } + } + // Rate limit + if (s.includes('rate limit') || s.includes('api rate limit exceeded')) { + return { type: 'rate_limited', message: 'GitHub rate limit hit. Try again in a few minutes.' } + } + // Network — checked BEFORE not_found because DNS failures surface as + // "could not resolve host", which would otherwise be partially matched by + // the not_found branch's "could not resolve" check. Substring matching here + // is a one-way trapdoor: a real GraphQL "Could not resolve to a User…" + // error always contains "to a", so we tighten the not_found check below to + // require that token. + if ( + s.includes('timeout') || + s.includes('no such host') || + s.includes('network') || + s.includes('could not resolve host') || + s.includes('dial tcp') + ) { + return { type: 'network_error', message: 'Network error — check your connection.' } + } + // Not found + if ( + s.includes('http 404') || + errors.some((e) => (e.type ?? '').toUpperCase() === 'NOT_FOUND') || + s.includes('could not resolve to a ') + ) { + const firstNotFound = errors.find((e) => (e.type ?? '').toUpperCase() === 'NOT_FOUND') + return { + type: 'not_found', + message: 'Project or view not found.', + details: firstNotFound + ? { path: firstNotFound.path, code: firstNotFound.extensions?.code } + : undefined + } + } + // Validation + if (s.includes('http 422') || s.includes('validation failed')) { + return { type: 'validation_error', message: `Invalid request — ${stderr.trim()}` } + } + // GraphQL error with structured info + if (errors.length > 0) { + const first = errors[0] + return { + type: 'unknown', + message: first.message ?? 'Unknown GraphQL error.', + details: { path: first.path, code: first.extensions?.code } + } + } + // Why: don't leak full stderr to the UI — it can include verbose request + // dumps with header diagnostics. Truncate to the first non-empty line and + // cap length so unexpected diagnostics stay readable but bounded. + const firstLine = stderr + .split('\n') + .map((l) => l.trim()) + .find((l) => l.length > 0) ?? '' + const safe = firstLine.length > 200 ? `${firstLine.slice(0, 200)}…` : firstLine + return { type: 'unknown', message: safe ? `GitHub request failed: ${safe}` : 'GitHub request failed.' } +} + +export function driftError( + reason: string, + details?: { path?: (string | number)[]; code?: string } +): GitHubProjectViewError { + return { type: 'schema_drift', message: `Could not read this project view: ${reason}.`, details } +} + +// Why: the rate-limit circuit breaker short-circuits before we spawn `gh` +// when the cached snapshot says we're below the safety floor. Synthesize the +// same `rate_limited` error shape as the post-hoc classifier so the UI path +// is unchanged. We DO NOT fail open here when there's no cached snapshot — +// rateLimitGuard already handles that case (returns `blocked:false`). +export function rateLimitedError( + blocked: { remaining: number; limit: number; resetAt: number } +): GitHubProjectViewError { + const resetIn = Math.max(0, blocked.resetAt - Math.floor(Date.now() / 1000)) + const mins = Math.ceil(resetIn / 60) + return { + type: 'rate_limited', + message: `GitHub rate limit nearly exhausted (${blocked.remaining}/${blocked.limit} left). Resets in ~${mins}m.` + } +} + +// ─── Low-level gh api graphql invocation ─────────────────────────────── + +export type GraphqlVars = Record + +export async function runGraphql( + query: string, + vars: GraphqlVars, + cwd?: string +): Promise< + | { ok: true; data: T } + | { ok: false; error: GitHubProjectViewError; raw: { stderr: string; stdout: string } } +> { + const guard = rateLimitGuard('graphql') + if (guard.blocked) { + return { ok: false, error: rateLimitedError(guard), raw: { stderr: '', stdout: '' } } + } + // Why: build argv as an array. `-f` for strings (including numbers passed + // as strings), `-F` coerces to typed. We use `-f` uniformly and coerce in + // the query via Int! casts, because `gh` can confuse empty strings. + const args: string[] = ['api', 'graphql', '-f', `query=${query}`] + for (const [k, v] of Object.entries(vars)) { + if (typeof v === 'number' || typeof v === 'boolean') { + args.push('-F', `${k}=${String(v)}`) + } else { + args.push('-f', `${k}=${v}`) + } + } + await acquire() + noteRateLimitSpend('graphql') + try { + const { stdout, stderr } = await ghExecFileAsync(args, { + encoding: 'utf-8', + ...(cwd ? { cwd } : {}) + }) + try { + const parsed = JSON.parse(stdout) as { data?: T; errors?: GhGraphqlErrorShape[] } + if (parsed.errors && parsed.errors.length > 0) { + return { + ok: false, + error: classifyProjectError(stderr, stdout), + raw: { stderr, stdout } + } + } + if (parsed.data === undefined) { + return { + ok: false, + error: driftError('response missing data'), + raw: { stderr, stdout } + } + } + return { ok: true, data: parsed.data } + } catch (parseErr) { + return { + ok: false, + error: driftError( + `failed to parse response (${parseErr instanceof Error ? parseErr.message : String(parseErr)})` + ), + raw: { stderr, stdout } + } + } + } catch (err) { + // gh executable failures (non-zero exit). Read stderr/stdout from the + // exec rejection's explicit fields — `err.message` may truncate stderr. + const { stderr, stdout: maybeStdout } = extractExecError(err) + return { + ok: false, + error: classifyProjectError(stderr, maybeStdout), + raw: { stderr, stdout: maybeStdout } + } + } finally { + release() + } +} + +export async function runRest( + args: string[], + cwd?: string, + bucket: RateLimitBucketKind = 'core', + options?: { expectEmpty?: boolean } +): Promise<{ ok: true; data: T } | { ok: false; error: GitHubProjectViewError }> { + const guard = rateLimitGuard(bucket) + if (guard.blocked) { + return { ok: false, error: rateLimitedError(guard) } + } + await acquire() + noteRateLimitSpend(bucket) + try { + const { stdout, stderr } = await ghExecFileAsync(['api', ...args], { + encoding: 'utf-8', + ...(cwd ? { cwd } : {}) + }) + // Why: 204/empty-body endpoints (DELETE label, DELETE comment) return no + // body. Treat empty stdout as success rather than misclassifying the + // unparseable response as 'unknown' — which the caller would otherwise + // need to special-case and risks masking real failures whose stderr the + // classifier also tags as 'unknown'. + if (options?.expectEmpty && stdout.trim() === '') { + return { ok: true, data: undefined as T } + } + try { + return { ok: true, data: JSON.parse(stdout) as T } + } catch { + return { + ok: false, + error: { type: 'unknown', message: `Unexpected REST response: ${stderr.trim()}` } + } + } + } catch (err) { + const { stderr, stdout: maybeStdout } = extractExecError(err) + return { ok: false, error: classifyProjectError(stderr, maybeStdout) } + } finally { + release() + } +} diff --git a/src/main/github/project-view/mutations.ts b/src/main/github/project-view/mutations.ts new file mode 100644 index 00000000000..97cdfeb2ad8 --- /dev/null +++ b/src/main/github/project-view/mutations.ts @@ -0,0 +1,738 @@ +/* eslint-disable max-lines -- Why: slug-addressed mutations + work-item +details share the validate/runRest/runGraphql plumbing with the read path. +Keeping them together preserves a single review surface for the write side. */ +import { + acquire, + release, + extractExecError, + ghExecFileAsync, + rateLimitGuard, + noteRateLimitSpend, + classifyProjectError, + rateLimitedError, + runGraphql, + runRest, + validateSlugArgs, + assertPositiveInt, + type GraphqlVars +} from './internals' +import type { GitHubAssignableUser, GitHubWorkItemDetails, PRComment } from '../../../shared/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' + +// ─── Project field mutations ────────────────────────────────────────── + +class UnknownFieldMutationKindError extends Error { + constructor(kind: string) { + super(`Unknown project field mutation kind: ${kind}`) + } +} + +function graphqlValueForFieldMutation(value: GitHubProjectFieldMutationValue): string { + // Serialize the value fragment for the GraphQL mutation. We use GraphQL + // variables for every dynamic piece, so here we only pick the variable name + // to reference per value kind. + switch (value.kind) { + case 'single-select': + return 'singleSelectOptionId: $value' + case 'iteration': + return 'iterationId: $value' + case 'text': + return 'text: $value' + case 'number': + return 'number: $value' + case 'date': + return 'date: $value' + default: + // Why: defensive default. If a new mutation kind is added to the type + // but not handled here, returning undefined would silently produce a + // broken GraphQL query. Throw so updateProjectItemFieldValue can map it + // to a validation_error rather than dispatching a malformed mutation. + throw new UnknownFieldMutationKindError((value as { kind: string }).kind) + } +} + +function mutationValueVar(value: GitHubProjectFieldMutationValue): { + type: string + val: string | number +} { + switch (value.kind) { + case 'single-select': + return { type: 'String!', val: value.optionId } + case 'iteration': + return { type: 'String!', val: value.iterationId } + case 'text': + return { type: 'String!', val: value.text } + case 'number': + return { type: 'Float!', val: value.number } + case 'date': + return { type: 'Date!', val: value.date } + default: + // Why: see graphqlValueForFieldMutation — surface unknown kinds loudly + // instead of returning undefined and dispatching an invalid mutation. + throw new UnknownFieldMutationKindError((value as { kind: string }).kind) + } +} + +export async function updateProjectItemFieldValue( + args: UpdateProjectItemFieldArgs +): Promise { + if (!args.projectId || !args.itemId || !args.fieldId) { + return { ok: false, error: { type: 'validation_error', message: 'Missing ids.' } } + } + let valFrag: string + let valVar: { type: string; val: string | number } + try { + valFrag = graphqlValueForFieldMutation(args.value) + valVar = mutationValueVar(args.value) + } catch (err) { + if (err instanceof UnknownFieldMutationKindError) { + return { ok: false, error: { type: 'validation_error', message: err.message } } + } + throw err + } + const query = ` + mutation($projectId:ID!, $itemId:ID!, $fieldId:ID!, $value:${valVar.type}) { + updateProjectV2ItemFieldValue(input: { + projectId: $projectId + itemId: $itemId + fieldId: $fieldId + value: { ${valFrag} } + }) { projectV2Item { id } } + } + ` + const vars: GraphqlVars = { + projectId: args.projectId, + itemId: args.itemId, + fieldId: args.fieldId, + value: valVar.val + } + const res = await runGraphql(query, vars) + if (!res.ok) {return { ok: false, error: res.error }} + return { ok: true } +} + +export async function clearProjectItemFieldValue( + args: ClearProjectItemFieldArgs +): Promise { + if (!args.projectId || !args.itemId || !args.fieldId) { + return { ok: false, error: { type: 'validation_error', message: 'Missing ids.' } } + } + const query = ` + mutation($projectId:ID!, $itemId:ID!, $fieldId:ID!) { + clearProjectV2ItemFieldValue(input: { + projectId: $projectId + itemId: $itemId + fieldId: $fieldId + }) { projectV2Item { id } } + } + ` + const res = await runGraphql(query, { + projectId: args.projectId, + itemId: args.itemId, + fieldId: args.fieldId + }) + if (!res.ok) {return { ok: false, error: res.error }} + return { ok: true } +} + +// ─── Slug-addressed issue/PR mutations ──────────────────────────────── + +export async function updateIssueBySlug( + args: UpdateIssueBySlugArgs +): Promise { + const v = validateSlugArgs(args.owner, args.repo) + if (!v.ok) {return v} + const n = assertPositiveInt(args.number, 'number') + if (!n.ok) {return { ok: false, error: n.error }} + if (!args.updates || typeof args.updates !== 'object') { + return { ok: false, error: { type: 'validation_error', message: 'Updates required.' } } + } + const { title, body, state, addLabels, removeLabels, addAssignees, removeAssignees } = args.updates + + // Title / body / state go through PATCH /repos/{owner}/{repo}/issues/{n}. + // Labels/assignees go through their dedicated endpoints. + const base = `repos/${args.owner}/${args.repo}/issues/${args.number}` + + // 1) PATCH body + if (title !== undefined || body !== undefined || state !== undefined) { + const patchArgs: string[] = ['-X', 'PATCH', base] + if (title !== undefined) {patchArgs.push('--raw-field', `title=${title}`)} + if (body !== undefined) {patchArgs.push('--raw-field', `body=${body}`)} + if (state !== undefined) {patchArgs.push('--raw-field', `state=${state}`)} + const r = await runRest(patchArgs) + if (!r.ok) {return { ok: false, error: r.error }} + } + + // 2) Labels — collapse multi-delete fan-out into a single PUT when removing + // >1 label. PUT /labels replaces the entire label set, so we fetch the + // current labels first and compute the resulting set client-side. This + // turns an N-delete + 1-add (=N+1 calls) into 1-fetch + 1-PUT (=2 calls) + // once removeLabels has more than one entry, capping the cost at 2 even + // for a "remove all 20 labels" mutation. + const removeCount = removeLabels?.length ?? 0 + const addCount = addLabels?.length ?? 0 + if (removeCount > 1) { + type RawLabelResp = { name?: string }[] + const fetched = await runRest(['-X', 'GET', `${base}/labels`]) + if (!fetched.ok) {return { ok: false, error: fetched.error }} + const currentNames = new Set( + fetched.data.map((l) => l.name).filter((n): n is string => typeof n === 'string') + ) + for (const l of removeLabels ?? []) {currentNames.delete(l)} + for (const l of addLabels ?? []) {currentNames.add(l)} + if (currentNames.size === 0) { + // Why: `gh api -X PUT` with no `--raw-field` arguments sends an empty + // body — GitHub does NOT interpret that as "clear labels". The + // dedicated DELETE endpoint is the documented way to remove all + // labels in a single call. + const r = await runRest( + ['-X', 'DELETE', `${base}/labels`], + undefined, + 'core', + { expectEmpty: true } + ) + if (!r.ok && r.error.type !== 'not_found') {return { ok: false, error: r.error }} + } else { + const putArgs = ['-X', 'PUT', `${base}/labels`] + for (const name of currentNames) {putArgs.push('--raw-field', `labels[]=${name}`)} + const r = await runRest(putArgs) + if (!r.ok) {return { ok: false, error: r.error }} + } + } else { + if (addCount > 0) { + const restArgs = ['-X', 'POST', `${base}/labels`] + for (const l of addLabels ?? []) {restArgs.push('--raw-field', `labels[]=${l}`)} + const r = await runRest(restArgs) + if (!r.ok) {return { ok: false, error: r.error }} + } + if (removeCount === 1) { + const r = await runRest( + ['-X', 'DELETE', `${base}/labels/${encodeURIComponent(removeLabels![0])}`], + undefined, + 'core', + { expectEmpty: true } + ) + if (!r.ok && r.error.type !== 'not_found') {return { ok: false, error: r.error }} + } + } + + // 3) Assignees — POST and DELETE both accept arrays in a single call, so + // add/remove are at most 2 calls regardless of array size. + if (addAssignees && addAssignees.length > 0) { + const restArgs = ['-X', 'POST', `${base}/assignees`] + for (const u of addAssignees) {restArgs.push('--raw-field', `assignees[]=${u}`)} + const r = await runRest(restArgs) + if (!r.ok) {return { ok: false, error: r.error }} + } + if (removeAssignees && removeAssignees.length > 0) { + const restArgs = ['-X', 'DELETE', `${base}/assignees`] + for (const u of removeAssignees) {restArgs.push('--raw-field', `assignees[]=${u}`)} + const r = await runRest(restArgs) + if (!r.ok) {return { ok: false, error: r.error }} + } + return { ok: true } +} + +export async function updatePullRequestBySlug( + args: UpdatePullRequestBySlugArgs +): Promise { + const v = validateSlugArgs(args.owner, args.repo) + if (!v.ok) {return v} + const n = assertPositiveInt(args.number, 'number') + if (!n.ok) {return { ok: false, error: n.error }} + if (!args.updates || typeof args.updates !== 'object') { + return { ok: false, error: { type: 'validation_error', message: 'Updates required.' } } + } + const patchArgs: string[] = ['-X', 'PATCH', `repos/${args.owner}/${args.repo}/pulls/${args.number}`] + // Why: count fields explicitly rather than inferring from patchArgs.length — + // adding a future header/flag arg silently breaks an array-length check. + let fieldCount = 0 + if (args.updates.title !== undefined) { + patchArgs.push('--raw-field', `title=${args.updates.title}`) + fieldCount++ + } + if (args.updates.body !== undefined) { + patchArgs.push('--raw-field', `body=${args.updates.body}`) + fieldCount++ + } + if (fieldCount === 0) { + // No fields to update — nothing to do. + return { ok: true } + } + const r = await runRest(patchArgs) + if (!r.ok) {return { ok: false, error: r.error }} + return { ok: true } +} + +type RawIssueCommentResponse = { + id?: number + user?: { login?: string; avatar_url?: string; type?: string } | null + body?: string + created_at?: string + html_url?: string +} + +function mapIssueComment(data: RawIssueCommentResponse, fallbackBody: string): PRComment { + return { + id: data.id ?? Date.now(), + author: data.user?.login ?? 'You', + authorAvatarUrl: data.user?.avatar_url ?? '', + body: data.body ?? fallbackBody, + createdAt: data.created_at ?? new Date().toISOString(), + url: data.html_url ?? '', + isBot: data.user?.type === 'Bot' + } +} + +export async function addIssueCommentBySlug( + args: AddIssueCommentBySlugArgs +): Promise { + const v = validateSlugArgs(args.owner, args.repo) + if (!v.ok) {return v} + const n = assertPositiveInt(args.number, 'number') + if (!n.ok) {return { ok: false, error: n.error }} + if (typeof args.body !== 'string' || !args.body.trim()) { + return { ok: false, error: { type: 'validation_error', message: 'Comment body required.' } } + } + const r = await runRest([ + '-X', + 'POST', + `repos/${args.owner}/${args.repo}/issues/${args.number}/comments`, + '--raw-field', + `body=${args.body}` + ]) + if (!r.ok) {return { ok: false, error: r.error }} + return { ok: true, comment: mapIssueComment(r.data, args.body) } +} + +export async function updateIssueCommentBySlug( + args: UpdateIssueCommentBySlugArgs +): Promise { + const v = validateSlugArgs(args.owner, args.repo) + if (!v.ok) {return v} + const n = assertPositiveInt(args.commentId, 'commentId') + if (!n.ok) {return { ok: false, error: n.error }} + if (typeof args.body !== 'string' || !args.body.trim()) { + return { ok: false, error: { type: 'validation_error', message: 'Comment body required.' } } + } + const r = await runRest([ + '-X', + 'PATCH', + `repos/${args.owner}/${args.repo}/issues/comments/${args.commentId}`, + '--raw-field', + `body=${args.body}` + ]) + if (!r.ok) {return { ok: false, error: r.error }} + return { ok: true } +} + +export async function deleteIssueCommentBySlug( + args: DeleteIssueCommentBySlugArgs +): Promise { + const v = validateSlugArgs(args.owner, args.repo) + if (!v.ok) {return v} + const n = assertPositiveInt(args.commentId, 'commentId') + if (!n.ok) {return { ok: false, error: n.error }} + const r = await runRest( + ['-X', 'DELETE', `repos/${args.owner}/${args.repo}/issues/comments/${args.commentId}`], + undefined, + 'core', + { expectEmpty: true } + ) + if (!r.ok) {return { ok: false, error: r.error }} + return { ok: true } +} + +// ─── Slug-addressed picker sources ──────────────────────────────────── + +export async function listLabelsBySlug( + args: ListLabelsBySlugArgs +): Promise { + const v = validateSlugArgs(args.owner, args.repo) + if (!v.ok) {return v} + const guard = rateLimitGuard('core') + if (guard.blocked) {return { ok: false, error: rateLimitedError(guard) }} + await acquire() + // Why: `--paginate` may fan out to multiple pages; we can only reasonably + // estimate a 1-call spend up front. The next probe will reconcile. + noteRateLimitSpend('core') + try { + const { stdout } = await ghExecFileAsync( + ['api', '--paginate', `repos/${args.owner}/${args.repo}/labels`, '--jq', '.[].name'], + { encoding: 'utf-8' } + ) + return { + ok: true, + labels: stdout + .trim() + .split('\n') + .filter((l) => l.length > 0) + } + } catch (err) { + const { stderr, stdout: maybeStdout } = extractExecError(err) + return { ok: false, error: classifyProjectError(stderr, maybeStdout) } + } finally { + release() + } +} + +export async function listAssignableUsersBySlug( + args: ListAssignableUsersBySlugArgs +): Promise { + const v = validateSlugArgs(args.owner, args.repo) + if (!v.ok) {return v} + // Seed logins merge after the fetch so callers can include currently-visible + // assignees even if the repo participant search is sparse. + const result: GitHubAssignableUser[] = [] + const guard = rateLimitGuard('core') + if (guard.blocked) {return { ok: false, error: rateLimitedError(guard) }} + await acquire() + noteRateLimitSpend('core') + try { + const { stdout } = await ghExecFileAsync( + [ + 'api', + '--paginate', + `repos/${args.owner}/${args.repo}/assignees`, + '--jq', + '.[] | {login: .login, name: null, avatarUrl: .avatar_url}' + ], + { encoding: 'utf-8' } + ) + for (const line of stdout.trim().split('\n').filter((l) => l.length > 0)) { + try { + const u = JSON.parse(line) as { login?: string; avatarUrl?: string; name?: string | null } + if (typeof u.login === 'string') { + result.push({ login: u.login, name: u.name ?? null, avatarUrl: u.avatarUrl ?? '' }) + } + } catch { + // skip malformed jq line + } + } + } catch (err) { + const { stderr } = extractExecError(err) + return { ok: false, error: classifyProjectError(stderr, '') } + } finally { + release() + } + if (args.seedLogins) { + const seen = new Set(result.map((u) => u.login)) + for (const login of args.seedLogins) { + if (typeof login === 'string' && !seen.has(login)) { + result.push({ login, name: null, avatarUrl: '' }) + seen.add(login) + } + } + } + return { ok: true, users: result } +} + +// Why: Issue Types are a repo-level taxonomy (Bug/Feature/Task/etc) only +// available on repos opted into typed-issues. Empty list (or schema_drift on +// older GitHub deployments) is the legitimate "this repo doesn't use issue +// types" signal — callers should treat it as "no editor". +export async function listIssueTypesBySlug( + args: ListIssueTypesBySlugArgs +): Promise { + const v = validateSlugArgs(args.owner, args.repo) + if (!v.ok) {return v} + const query = ` + query($owner:String!, $repo:String!) { + repository(owner:$owner, name:$repo) { + issueTypes(first:50) { + nodes { id name color description } + } + } + } + ` + const res = await runGraphql<{ + repository?: { + issueTypes?: { + nodes?: ({ + id?: string + name?: string + color?: string | null + description?: string | null + } | null)[] + } | null + } | null + }>(query, { owner: args.owner, repo: args.repo }) + if (!res.ok) { + // Why: repos without issue types respond with a GraphQL error claiming the + // `issueTypes` field is unknown. Map that to an empty list so the UI shows + // "no editor" instead of an angry banner. + if (res.error.type === 'schema_drift' || res.error.type === 'validation_error') { + return { ok: true, types: [] } + } + return { ok: false, error: res.error } + } + const nodes = res.data.repository?.issueTypes?.nodes ?? [] + const types = nodes + .filter((n): n is NonNullable => n !== null && typeof n.id === 'string' && typeof n.name === 'string') + .map((n) => ({ + id: n.id as string, + name: n.name as string, + color: typeof n.color === 'string' ? n.color : null, + description: typeof n.description === 'string' ? n.description : null + })) + return { ok: true, types } +} + +export async function updateIssueTypeBySlug( + args: UpdateIssueTypeBySlugArgs +): Promise { + const v = validateSlugArgs(args.owner, args.repo) + if (!v.ok) {return v} + const n = assertPositiveInt(args.number, 'number') + if (!n.ok) {return { ok: false, error: n.error }} + // Why: `updateIssueIssueType` is the dedicated mutation; passing null for + // `issueTypeId` clears the type. We resolve the issue id via a lightweight + // GraphQL lookup because the REST endpoint doesn't accept issue types. + const lookup = await runGraphql<{ + repository?: { issue?: { id?: string } | null } | null + }>( + `query($owner:String!, $repo:String!, $num:Int!) { + repository(owner:$owner, name:$repo) { issue(number:$num) { id } } + }`, + { owner: args.owner, repo: args.repo, num: args.number } + ) + if (!lookup.ok) {return { ok: false, error: lookup.error }} + const issueId = lookup.data.repository?.issue?.id + if (!issueId) { + return { ok: false, error: { type: 'not_found', message: 'Issue not found.' } } + } + // Why: build the mutation conditionally so a null clear doesn't have to + // smuggle a null GraphQL variable through `gh api graphql -f`. The + // mutation accepts a literal `null` in the input object directly. + const query = args.issueTypeId + ? ` + mutation($issueId:ID!, $issueTypeId:ID!) { + updateIssueIssueType(input: { issueId: $issueId, issueTypeId: $issueTypeId }) { + issue { id } + } + } + ` + : ` + mutation($issueId:ID!) { + updateIssueIssueType(input: { issueId: $issueId, issueTypeId: null }) { + issue { id } + } + } + ` + const vars: GraphqlVars = args.issueTypeId + ? { issueId, issueTypeId: args.issueTypeId } + : { issueId } + const res = await runGraphql(query, vars) + if (!res.ok) {return { ok: false, error: res.error }} + return { ok: true } +} + +// ─── Slug-addressed work-item details ───────────────────────────────── + +type RawUser = { login?: string; name?: string | null; avatarUrl?: string | null } +type RawLabel = { name?: string; color?: string } +type RawWorkItemContent = { + id?: string + number?: number + title?: string + url?: string + state?: string + stateReason?: string | null + isDraft?: boolean + labels?: { nodes?: RawLabel[] } + assignees?: { nodes?: RawUser[] } +} + +export async function getWorkItemDetailsBySlug( + args: ProjectWorkItemDetailsBySlugArgs +): Promise { + const v = validateSlugArgs(args.owner, args.repo) + if (!v.ok) {return v} + const n = assertPositiveInt(args.number, 'number') + if (!n.ok) {return { ok: false, error: n.error }} + if (args.type !== 'issue' && args.type !== 'pr') { + return { ok: false, error: { type: 'validation_error', message: 'Invalid type.' } } + } + + // Single GraphQL round-trip to fetch the issue/PR summary + comments + labels + assignees. + const contentFrag = + args.type === 'issue' + ? ` + issue(number:$num) { + id number title url state stateReason updatedAt + body + author { login } + labels(first:50) { nodes { name } } + assignees(first:50) { nodes { login } } + participants(first:50) { nodes { login name avatarUrl } } + comments(first:100) { + nodes { + databaseId + author { login avatarUrl __typename } + body createdAt url + } + } + } + ` + : ` + pullRequest(number:$num) { + id number title url state isDraft updatedAt headRefName baseRefName + body + author { login } + labels(first:50) { nodes { name } } + assignees(first:50) { nodes { login } } + participants(first:50) { nodes { login name avatarUrl } } + comments(first:100) { + nodes { + databaseId + author { login avatarUrl __typename } + body createdAt url + } + } + } + ` + const query = ` + query($owner:String!, $repo:String!, $num:Int!) { + repository(owner:$owner, name:$repo) { + ${contentFrag} + } + } + ` + const res = await runGraphql<{ + repository?: { + issue?: RawWorkItemContent & { + updatedAt?: string + body?: string + author?: { login?: string } | null + participants?: { nodes?: RawUser[] } + comments?: { + nodes?: ({ + databaseId?: number + author?: { login?: string; avatarUrl?: string; __typename?: string } | null + body?: string + createdAt?: string + url?: string + } | null)[] + } + } | null + pullRequest?: RawWorkItemContent & { + updatedAt?: string + body?: string + headRefName?: string + baseRefName?: string + author?: { login?: string } | null + participants?: { nodes?: RawUser[] } + comments?: { + nodes?: ({ + databaseId?: number + author?: { login?: string; avatarUrl?: string; __typename?: string } | null + body?: string + createdAt?: string + url?: string + } | null)[] + } + } | null + } | null + }>(query, { owner: args.owner, repo: args.repo, num: args.number }) + if (!res.ok) {return { ok: false, error: res.error }} + const raw = args.type === 'issue' ? res.data.repository?.issue : res.data.repository?.pullRequest + if (!raw) { + return { ok: false, error: { type: 'not_found', message: 'Item not found.' } } + } + + const labels = (raw.labels?.nodes ?? []) + .map((l) => l?.name) + .filter((n): n is string => typeof n === 'string') + const assignees = (raw.assignees?.nodes ?? []) + .map((a) => a?.login) + .filter((l): l is string => typeof l === 'string') + const comments: PRComment[] = [] + for (const c of raw.comments?.nodes ?? []) { + if (!c || typeof c.body !== 'string') {continue} + comments.push({ + id: typeof c.databaseId === 'number' ? c.databaseId : Date.now(), + author: c.author?.login ?? '', + authorAvatarUrl: c.author?.avatarUrl ?? '', + body: c.body, + createdAt: typeof c.createdAt === 'string' ? c.createdAt : '', + url: typeof c.url === 'string' ? c.url : '', + isBot: c.author?.__typename === 'Bot' + }) + } + const participants: GitHubAssignableUser[] = [] + for (const p of raw.participants?.nodes ?? []) { + if (p && typeof p.login === 'string') { + participants.push({ login: p.login, name: p.name ?? null, avatarUrl: p.avatarUrl ?? '' }) + } + } + + const state: 'open' | 'closed' | 'merged' | 'draft' = + args.type === 'pr' + ? raw.isDraft + ? 'draft' + : raw.state === 'MERGED' + ? 'merged' + : raw.state === 'CLOSED' + ? 'closed' + : 'open' + : raw.state === 'CLOSED' + ? 'closed' + : 'open' + + const details: GitHubWorkItemDetails = { + item: { + id: typeof raw.id === 'string' ? raw.id : '', + type: args.type, + number: typeof raw.number === 'number' ? raw.number : args.number, + title: typeof raw.title === 'string' ? raw.title : '', + state, + url: typeof raw.url === 'string' ? raw.url : '', + labels, + updatedAt: + typeof (raw as { updatedAt?: string }).updatedAt === 'string' + ? (raw as { updatedAt: string }).updatedAt + : '', + author: + typeof (raw as { author?: { login?: string } | null }).author?.login === 'string' + ? ((raw as { author: { login: string } }).author.login as string) + : null, + branchName: + args.type === 'pr' && typeof (raw as { headRefName?: string }).headRefName === 'string' + ? ((raw as { headRefName: string }).headRefName as string) + : undefined, + baseRefName: + args.type === 'pr' && typeof (raw as { baseRefName?: string }).baseRefName === 'string' + ? ((raw as { baseRefName: string }).baseRefName as string) + : undefined + }, + body: typeof raw.body === 'string' ? raw.body : '', + comments, + participants, + // Why: PR files/checks/review-thread tabs depend on a local repo path and + // are out of Project-mode slug scope for v1. Omit them here; the dialog + // branches on their absence and hides those tabs. + ...(args.type === 'issue' ? { assignees } : {}) + } + return { ok: true, details } +} + diff --git a/src/renderer/src/components/GitHubItemDialog.tsx b/src/renderer/src/components/GitHubItemDialog.tsx index c7a37c3b441..2b8a86acc04 100644 --- a/src/renderer/src/components/GitHubItemDialog.tsx +++ b/src/renderer/src/components/GitHubItemDialog.tsx @@ -1736,8 +1736,22 @@ function GHEditSection({ const [localAssignees, setLocalAssignees] = useState(assignees) const hasEditedAssigneesRef = useRef(false) const patchWorkItem = useAppStore((s) => s.patchWorkItem) + const patchProjectRowContent = useAppStore((s) => s.patchProjectRowContent) const { isPending, run } = useImmediateMutation() + // Why: when the dialog opens from a Project view, mutations route through + // *BySlug IPCs and we must keep `projectViewCache` in sync alongside + // `workItemsCache` — `patchWorkItem` only walks the latter, so without this + // helper the Project table would render stale data until manual refresh. + // See docs/design/github-project-view-tasks.md §Dialog editing from Project rows. + const patchProjectRowIfNeeded = useCallback( + (patch: Parameters[2]) => { + if (!projectOrigin) {return} + patchProjectRowContent(projectOrigin.cacheKey, projectOrigin.projectItemId, patch) + }, + [projectOrigin, patchProjectRowContent] + ) + // Why: when projectOrigin is set we MUST read labels/assignees from the // row's repo, not from the workspace path — otherwise the popovers list // values from a different repo than the writes target. @@ -1782,18 +1796,31 @@ function GHEditSection({ onOptimistic: () => { onStateChange(newState) patchWorkItem(item.id, { state: newState }) + patchProjectRowIfNeeded({ state: newState }) }, onRevert: () => { onStateChange(prevState) patchWorkItem(item.id, { state: prevState }) + patchProjectRowIfNeeded({ state: prevState }) }, onSuccess: () => { patchWorkItem(item.id, { state: newState }) + patchProjectRowIfNeeded({ state: newState }) }, onError: (err) => toast.error(err) }) }, - [item.id, item.number, localState, repoPath, projectOrigin, patchWorkItem, run, onStateChange] + [ + item.id, + item.number, + localState, + repoPath, + projectOrigin, + patchWorkItem, + patchProjectRowIfNeeded, + run, + onStateChange + ] ) const handleLabelToggle = useCallback( @@ -1814,11 +1841,13 @@ function GHEditSection({ onOptimistic: () => { onLabelsChange(newLabels) patchWorkItem(item.id, { labels: newLabels }) + patchProjectRowIfNeeded({ labels: newLabels }) }, onSuccess: () => {}, onRevert: () => { onLabelsChange(prevLabels) patchWorkItem(item.id, { labels: prevLabels }) + patchProjectRowIfNeeded({ labels: prevLabels }) }, onError: (err) => toast.error(err) }) @@ -1834,17 +1863,29 @@ function GHEditSection({ onOptimistic: () => { onLabelsChange(newLabels) patchWorkItem(item.id, { labels: newLabels }) + patchProjectRowIfNeeded({ labels: newLabels }) }, onRevert: () => { onLabelsChange(prevLabels) patchWorkItem(item.id, { labels: prevLabels }) + patchProjectRowIfNeeded({ labels: prevLabels }) }, onSuccess: () => {}, onError: (err) => toast.error(err) }) } }, - [item.id, item.number, localLabels, repoPath, projectOrigin, patchWorkItem, run, onLabelsChange] + [ + item.id, + item.number, + localLabels, + repoPath, + projectOrigin, + patchWorkItem, + patchProjectRowIfNeeded, + run, + onLabelsChange + ] ) const handleAssigneeToggle = useCallback( @@ -1867,9 +1908,11 @@ function GHEditSection({ }), onOptimistic: () => { setLocalAssignees(newAssignees) + patchProjectRowIfNeeded({ assignees: newAssignees }) }, onRevert: () => { setLocalAssignees(prevAssignees) + patchProjectRowIfNeeded({ assignees: prevAssignees }) }, onSuccess: () => {}, onError: (err) => toast.error(err) @@ -1885,16 +1928,18 @@ function GHEditSection({ }), onOptimistic: () => { setLocalAssignees(newAssignees) + patchProjectRowIfNeeded({ assignees: newAssignees }) }, onSuccess: () => {}, onRevert: () => { setLocalAssignees(prevAssignees) + patchProjectRowIfNeeded({ assignees: prevAssignees }) }, onError: (err) => toast.error(err) }) } }, - [item.number, repoPath, projectOrigin, localAssignees, run] + [item.number, repoPath, projectOrigin, localAssignees, patchProjectRowIfNeeded, run] ) if (item.type === 'pr') { @@ -2142,12 +2187,7 @@ function GHCommentComposer({ ) return ( -
+
{ - if (!open || !owner || !repo) return + if (!open || !owner || !repo) {return} let cancelled = false setLoading(true) window.api.gh .listIssueTypesBySlug({ owner, repo }) .then((res) => { - if (cancelled) return - if (res.ok) setOptions(res.types) + if (cancelled) {return} + if (res.ok) {setOptions(res.types)} }) .finally(() => { - if (!cancelled) setLoading(false) + if (!cancelled) {setLoading(false)} }) return () => { cancelled = true @@ -595,13 +595,13 @@ function TextCell({ onChange={(e) => setDraft(e.target.value)} onBlur={() => { setEditing(false) - if (draft !== value) onCommit(draft) + if (draft !== value) {onCommit(draft)} }} onKeyDown={(e) => { if (e.key === 'Enter') { e.preventDefault() setEditing(false) - if (draft !== value) onCommit(draft) + if (draft !== value) {onCommit(draft)} } else if (e.key === 'Escape') { e.preventDefault() setEditing(false) @@ -622,14 +622,34 @@ function DateCell({ editable: boolean onCommit: (next: string) => void }): React.JSX.Element { + // Why: a date fires onChange on every digit/spinner adjustment. + // Committing on each fires a GraphQL mutation per keystroke. Buffer the + // edit locally and commit on blur or Enter — same UX as TextCell. + const [draft, setDraft] = React.useState(value ?? '') + React.useEffect(() => { + setDraft(value ?? '') + }, [value]) if (!editable) { return {value || '—'} } return ( onCommit(e.target.value)} + value={draft} + onChange={(e) => setDraft(e.target.value)} + onBlur={() => { + if (draft !== (value ?? '')) {onCommit(draft)} + }} + onKeyDown={(e) => { + if (e.key === 'Enter') { + e.preventDefault() + ;(e.target as HTMLInputElement).blur() + } else if (e.key === 'Escape') { + e.preventDefault() + setDraft(value ?? '') + ;(e.target as HTMLInputElement).blur() + } + }} className="h-6 cursor-pointer rounded border border-border/50 bg-background px-1 text-xs" /> ) @@ -700,7 +720,7 @@ function AssigneesCell({ // Why: only hit the slug-addressed user list when the popover actually // opens — the assignable-users query can be expensive for large repos. React.useEffect(() => { - if (!open || !owner || !repo) return + if (!open || !owner || !repo) {return} let cancelled = false setLoading(true) window.api.gh @@ -710,11 +730,11 @@ function AssigneesCell({ seedLogins: seedKey ? seedKey.split(',') : [] }) .then((res) => { - if (cancelled) return - if (res.ok) setOptions(res.users) + if (cancelled) {return} + if (res.ok) {setOptions(res.users)} }) .finally(() => { - if (!cancelled) setLoading(false) + if (!cancelled) {setLoading(false)} }) return () => { cancelled = true @@ -764,8 +784,8 @@ function AssigneesCell({ type="button" className="flex w-full items-center gap-2 rounded px-2 py-1 text-xs hover:bg-muted/50" onClick={() => { - if (isOn) onEditAssignees?.([], [u.login]) - else onEditAssignees?.([u.login], []) + if (isOn) {onEditAssignees?.([], [u.login])} + else {onEditAssignees?.([u.login], [])} }} > { - if (!open || !owner || !repo) return + if (!open || !owner || !repo) {return} let cancelled = false setLoading(true) window.api.gh .listLabelsBySlug({ owner, repo }) .then((res) => { - if (cancelled) return - if (res.ok) setOptions(res.labels) + if (cancelled) {return} + if (res.ok) {setOptions(res.labels)} }) .finally(() => { - if (!cancelled) setLoading(false) + if (!cancelled) {setLoading(false)} }) return () => { cancelled = true @@ -868,8 +888,8 @@ function LabelsCell({ type="button" className="flex w-full cursor-pointer items-center gap-2 rounded px-2 py-1 text-xs hover:bg-muted/50" onClick={() => { - if (isOn) onEditLabels?.([], [name]) - else onEditLabels?.([name], []) + if (isOn) {onEditLabels?.([], [name])} + else {onEditLabels?.([name], [])} }} > = { } function singleSelectChipColors(color: string): { bg: string; fg: string; border: string } { - if (!color) return labelChipColors('') + if (!color) {return labelChipColors('')} const upper = color.toUpperCase() const hex = SINGLE_SELECT_HEX[upper] - if (hex) return labelChipColors(hex) + if (hex) {return labelChipColors(hex)} return labelChipColors(color) } @@ -940,9 +960,9 @@ function singleSelectChipColors(color: string): { bg: string; fg: string; border // approximate Primer's algorithm so our chips match the GitHub UI. function labelChipColors(color: string): { bg: string; fg: string; border: string } { const fallback = { bg: 'rgba(125,125,125,0.2)', fg: '#e6edf3', border: 'rgba(125,125,125,0.4)' } - if (!color) return fallback + if (!color) {return fallback} const hex = color.startsWith('#') ? color.slice(1) : color - if (!/^[0-9a-fA-F]{6}$/.test(hex)) return fallback + if (!/^[0-9a-fA-F]{6}$/.test(hex)) {return fallback} const r = parseInt(hex.slice(0, 2), 16) const g = parseInt(hex.slice(2, 4), 16) const b = parseInt(hex.slice(4, 6), 16) @@ -963,7 +983,7 @@ function rgbToHsl(r: number, g: number, b: number): [number, number, number] { const min = Math.min(rn, gn, bn) const l = (max + min) / 2 const d = max - min - if (d === 0) return [0, 0, l] + if (d === 0) {return [0, 0, l]} const s = l > 0.5 ? d / (2 - max - min) : d / (max + min) let h = 0 switch (max) { diff --git a/src/renderer/src/components/github-project/ProjectGroupHeader.tsx b/src/renderer/src/components/github-project/ProjectGroupHeader.tsx index 0155f0988d6..00367b72a22 100644 --- a/src/renderer/src/components/github-project/ProjectGroupHeader.tsx +++ b/src/renderer/src/components/github-project/ProjectGroupHeader.tsx @@ -46,7 +46,7 @@ export default function ProjectGroupHeader({ function formatDateRange(startDate: string, duration: number): string { const start = new Date(`${startDate}T00:00:00Z`) - if (Number.isNaN(start.getTime())) return '' + if (Number.isNaN(start.getTime())) {return ''} const end = new Date(start.getTime() + (duration - 1) * 86_400_000) const fmt = (d: Date): string => `${d.getUTCMonth() + 1}/${d.getUTCDate()}` diff --git a/src/renderer/src/components/github-project/ProjectItemSlugDialog.tsx b/src/renderer/src/components/github-project/ProjectItemSlugDialog.tsx index 2c9eaf796d9..4b3af2f38e2 100644 --- a/src/renderer/src/components/github-project/ProjectItemSlugDialog.tsx +++ b/src/renderer/src/components/github-project/ProjectItemSlugDialog.tsx @@ -7,45 +7,19 @@ // Files, Checks, and review-thread tabs hidden. This component is that // simplified surface; it also routes every write through slug-addressed // mutation helpers and patches the Project table cache on success. -import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react' -import { - CircleDot, - ExternalLink, - GitPullRequest, - LoaderCircle, - Send, - X -} from 'lucide-react' -import { toast } from 'sonner' +import React from 'react' import { VisuallyHidden } from 'radix-ui' import { Sheet, SheetContent, SheetDescription, SheetTitle } from '@/components/ui/sheet' -import { Button } from '@/components/ui/button' -import { Input } from '@/components/ui/input' -import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover' -import CommentMarkdown from '@/components/sidebar/CommentMarkdown' -import { useAppStore } from '@/store' -import { cn } from '@/lib/utils' -import type { - GitHubAssignableUser, - GitHubWorkItemDetails, - PRComment -} from '../../../../shared/types' import type { GitHubItemDialogProjectOrigin } from '@/components/GitHubItemDialog' +import { SlugDialogBody } from './slug-dialog/SlugDialogBody' type Props = { projectOrigin: GitHubItemDialogProjectOrigin | null - /** Present if the repo is registered; when set, Start work is enabled - * and calls this callback with the matched work item + repo id. Unknown - * repos instead raise the repo-not-in-orca prompt from the parent. */ - onStartWork?: () => void - startWorkEnabled?: boolean onClose: () => void } export default function ProjectItemSlugDialog({ projectOrigin, - onStartWork, - startWorkEnabled, onClose }: Props): React.JSX.Element { const open = projectOrigin !== null @@ -65,580 +39,9 @@ export default function ProjectItemSlugDialog({ Project row preview. {projectOrigin ? ( - + ) : null} ) } - -function SlugDialogBody({ - projectOrigin, - onStartWork, - startWorkEnabled, - onClose -}: { - projectOrigin: GitHubItemDialogProjectOrigin - onStartWork?: () => void - startWorkEnabled: boolean - onClose: () => void -}): React.JSX.Element { - const { owner, repo, number, type, cacheKey } = projectOrigin - const patchProjectIssueOrPr = useAppStore((s) => s.patchProjectIssueOrPr) - const projectViewCache = useAppStore((s) => s.projectViewCache) - - // Why: the Project row is the source of truth for the list-side columns; - // reading it reactively here keeps the dialog in sync with optimistic - // patches applied by the table (e.g. inline assignee edits). - const row = useMemo(() => { - const table = projectViewCache[cacheKey]?.data - if (!table) return null - return table.rows.find( - (r) => r.content.number === number && r.content.repository?.toLowerCase() === `${owner}/${repo}`.toLowerCase() - ) ?? null - }, [projectViewCache, cacheKey, owner, repo, number]) - - const [details, setDetails] = useState(null) - const [loading, setLoading] = useState(false) - const [error, setError] = useState(null) - const requestIdRef = useRef(0) - - useEffect(() => { - requestIdRef.current += 1 - const rid = requestIdRef.current - setLoading(true) - setError(null) - setDetails(null) - window.api.gh - .projectWorkItemDetailsBySlug({ owner, repo, number, type }) - .then((res) => { - if (rid !== requestIdRef.current) return - if (res.ok) { - setDetails(res.details) - } else { - setError(res.error.message) - } - }) - .catch((err) => { - if (rid !== requestIdRef.current) return - setError(err instanceof Error ? err.message : 'Failed to load details') - }) - .finally(() => { - if (rid !== requestIdRef.current) return - setLoading(false) - }) - }, [owner, repo, number, type]) - - const title = row?.content.title ?? details?.item.title ?? '' - const url = row?.content.url ?? details?.item.url ?? null - const Icon = type === 'pr' ? GitPullRequest : CircleDot - - const [editingTitle, setEditingTitle] = useState(false) - const [titleDraft, setTitleDraft] = useState('') - - const commitTitle = useCallback(async () => { - const next = titleDraft.trim() - setEditingTitle(false) - if (!next || next === title) return - const res = await patchProjectIssueOrPr(cacheKey, row?.id ?? '', { title: next }) - if (!res.ok) toast.error(res.error.message) - }, [titleDraft, title, patchProjectIssueOrPr, cacheKey, row?.id]) - - const [editingBody, setEditingBody] = useState(false) - const [bodyDraft, setBodyDraft] = useState('') - const body = details?.body ?? '' - const commitBody = useCallback(async () => { - setEditingBody(false) - if (bodyDraft === body) return - const res = await patchProjectIssueOrPr(cacheKey, row?.id ?? '', { body: bodyDraft }) - if (!res.ok) { - toast.error(res.error.message) - return - } - setDetails((prev) => (prev ? { ...prev, body: bodyDraft } : prev)) - }, [bodyDraft, body, patchProjectIssueOrPr, cacheKey, row?.id]) - - const labels = row?.content.labels.map((l) => l.name) ?? [] - const assignees = row?.content.assignees.map((u) => u.login) ?? [] - - return ( -
-
-
- -
-
- - {owner}/{repo}#{number} - -
- {editingTitle ? ( - setTitleDraft(e.target.value)} - onBlur={() => void commitTitle()} - onKeyDown={(e) => { - if (e.key === 'Enter') { - e.preventDefault() - void commitTitle() - } else if (e.key === 'Escape') { - e.preventDefault() - setEditingTitle(false) - } - }} - className="mt-1 h-8" - /> - ) : ( - - )} -
-
- {url ? ( - - ) : null} - -
-
-
- { - const res = await patchProjectIssueOrPr(cacheKey, row?.id ?? '', { - ...(add.length ? { addLabels: add } : {}), - ...(remove.length ? { removeLabels: remove } : {}) - }) - if (!res.ok) toast.error(res.error.message) - }} - /> - { - const res = await patchProjectIssueOrPr(cacheKey, row?.id ?? '', { - ...(add.length ? { addAssignees: add } : {}), - ...(remove.length ? { removeAssignees: remove } : {}) - }) - if (!res.ok) toast.error(res.error.message) - }} - /> - {onStartWork ? ( - - ) : null} -
-
- -
- {loading && !details ? ( -
- Loading… -
- ) : error ? ( -
{error}
- ) : details ? ( -
-
- {editingBody ? ( -
-