From 70befde7744d7b8ee5d56cabd5bec7c51435e27f Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 1 May 2026 21:48:51 -0700 Subject: [PATCH] feat(github): per-repo issue-source selector (#1317) Co-authored-by: Orca --- src/main/github/client-issue-source.test.ts | 193 ++++++++++++- src/main/github/client-work-items.test.ts | 25 +- src/main/github/client.ts | 39 ++- src/main/github/gh-utils.test.ts | 115 +++++++- src/main/github/gh-utils.ts | 54 +++- src/main/github/issues.test.ts | 18 +- src/main/github/issues.ts | 82 +++++- src/main/ipc/github.test.ts | 104 +++++-- src/main/ipc/github.ts | 29 +- src/main/ipc/repos.ts | 28 +- src/main/persistence.test.ts | 28 ++ src/main/persistence.ts | 23 +- src/preload/api-types.ts | 10 +- src/renderer/src/components/TaskPage.tsx | 257 ++++++++++++++---- .../components/github/IssueSourceSelector.tsx | 178 ++++++++++++ .../new-workspace/CreateFromTab.tsx | 87 +++++- src/renderer/src/store/slices/github.ts | 123 ++++++++- src/renderer/src/store/slices/repos.ts | 10 +- src/shared/types.ts | 36 +++ 19 files changed, 1316 insertions(+), 123 deletions(-) create mode 100644 src/renderer/src/components/github/IssueSourceSelector.tsx diff --git a/src/main/github/client-issue-source.test.ts b/src/main/github/client-issue-source.test.ts index d1e3f9618..051f9ffeb 100644 --- a/src/main/github/client-issue-source.test.ts +++ b/src/main/github/client-issue-source.test.ts @@ -1,3 +1,7 @@ +/* eslint-disable max-lines -- Why: the issue-source test suite covers the +heuristic split (#1076), the partial-failure envelope (feature 1), and the +three-state preference matrix (feature 2) as one surface so a regression in +any of them blocks the same merge gate. */ import { beforeEach, describe, expect, it, vi } from 'vitest' import type * as GhUtils from './gh-utils' @@ -6,6 +10,8 @@ const { ghExecFileAsyncMock, getOwnerRepoMock, getIssueOwnerRepoMock, + getOwnerRepoForRemoteMock, + resolveIssueSourceMock, acquireMock, releaseMock } = vi.hoisted(() => ({ @@ -13,6 +19,8 @@ const { ghExecFileAsyncMock: vi.fn(), getOwnerRepoMock: vi.fn(), getIssueOwnerRepoMock: vi.fn(), + getOwnerRepoForRemoteMock: vi.fn(), + resolveIssueSourceMock: vi.fn(), acquireMock: vi.fn(), releaseMock: vi.fn() })) @@ -25,6 +33,8 @@ vi.mock('./gh-utils', async () => { ghExecFileAsync: ghExecFileAsyncMock, getOwnerRepo: getOwnerRepoMock, getIssueOwnerRepo: getIssueOwnerRepoMock, + getOwnerRepoForRemote: getOwnerRepoForRemoteMock, + resolveIssueSource: resolveIssueSourceMock, acquire: acquireMock, release: releaseMock, _resetOwnerRepoCache: vi.fn() @@ -39,9 +49,24 @@ describe('GitHub issue source split', () => { ghExecFileAsyncMock.mockReset() getOwnerRepoMock.mockReset() getIssueOwnerRepoMock.mockReset() + getOwnerRepoForRemoteMock.mockReset() + resolveIssueSourceMock.mockReset() acquireMock.mockReset() releaseMock.mockReset() acquireMock.mockResolvedValue(undefined) + // Why: default the preference-aware resolver to 'auto' semantics so the + // pre-existing test cases (which don't think about preference at all) + // still pass. `listWorkItems` now calls `resolveIssueSource` instead of + // `getIssueOwnerRepo` directly — we delegate back to the single-call + // mock to preserve the one-fetch-per-test invariant each test sets up. + resolveIssueSourceMock.mockImplementation(async () => ({ + source: await getIssueOwnerRepoMock(), + fellBack: false + })) + // Default the upstream-candidate lookup to null so existing tests that + // only mock `getIssueOwnerRepo` + `getOwnerRepo` don't need to think + // about it. Tests that care set it explicitly. + getOwnerRepoForRemoteMock.mockResolvedValue(null) _resetOwnerRepoCache() }) @@ -237,7 +262,7 @@ describe('GitHub issue source split', () => { const result = await listWorkItems('/repo-root', 10) expect(result.items).toEqual([]) - expect(result.sources).toEqual({ + expect(result.sources).toMatchObject({ issues: { owner: 'stablyai', repo: 'orca' }, prs: { owner: 'fork', repo: 'orca' } }) @@ -288,4 +313,170 @@ describe('GitHub issue source split', () => { expect(getOwnerRepoMock).not.toHaveBeenCalled() expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(1) }) + + describe('per-repo issue-source preference', () => { + // Why: 3 preference states × 2 remote-topology states = 6 cases per the + // design doc §9. These tests isolate `listWorkItems` against a mocked + // `resolveIssueSource` to verify the preference is threaded all the way + // to the gh call and that `fellBack` propagates into the envelope. + + it("preference='auto' + upstream exists → queries upstream", async () => { + resolveIssueSourceMock.mockResolvedValueOnce({ + source: { owner: 'stablyai', repo: 'orca' }, + fellBack: false + }) + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'fork', repo: 'orca' }) + ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: '[]' }).mockResolvedValueOnce({ + stdout: '[]' + }) + + const result = await listWorkItems('/repo-root', 10, undefined, undefined, 'auto') + + expect(resolveIssueSourceMock).toHaveBeenCalledWith('/repo-root', 'auto') + expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( + 1, + [ + 'api', + '--cache', + '120s', + 'repos/stablyai/orca/issues?per_page=10&state=open&sort=updated&direction=desc' + ], + { cwd: '/repo-root' } + ) + expect(result.issueSourceFellBack).toBeUndefined() + }) + + it("preference='auto' + no upstream → queries origin", async () => { + resolveIssueSourceMock.mockResolvedValueOnce({ + source: { owner: 'solo', repo: 'orca' }, + fellBack: false + }) + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'solo', repo: 'orca' }) + ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: '[]' }).mockResolvedValueOnce({ + stdout: '[]' + }) + + await listWorkItems('/repo-root', 10, undefined, undefined, 'auto') + + expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( + 1, + [ + 'api', + '--cache', + '120s', + 'repos/solo/orca/issues?per_page=10&state=open&sort=updated&direction=desc' + ], + { cwd: '/repo-root' } + ) + }) + + it("preference='upstream' + upstream exists → queries upstream", async () => { + resolveIssueSourceMock.mockResolvedValueOnce({ + source: { owner: 'stablyai', repo: 'orca' }, + fellBack: false + }) + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'fork', repo: 'orca' }) + ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: '[]' }).mockResolvedValueOnce({ + stdout: '[]' + }) + + const result = await listWorkItems('/repo-root', 10, undefined, undefined, 'upstream') + + expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( + 1, + expect.arrayContaining([ + 'repos/stablyai/orca/issues?per_page=10&state=open&sort=updated&direction=desc' + ]), + { cwd: '/repo-root' } + ) + expect(result.issueSourceFellBack).toBeUndefined() + }) + + it("preference='upstream' + no upstream → falls back to origin with fellBack=true", async () => { + resolveIssueSourceMock.mockResolvedValueOnce({ + source: { owner: 'solo', repo: 'orca' }, + fellBack: true + }) + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'solo', repo: 'orca' }) + ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: '[]' }).mockResolvedValueOnce({ + stdout: '[]' + }) + + const result = await listWorkItems('/repo-root', 10, undefined, undefined, 'upstream') + + expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( + 1, + expect.arrayContaining([ + 'repos/solo/orca/issues?per_page=10&state=open&sort=updated&direction=desc' + ]), + { cwd: '/repo-root' } + ) + expect(result.issueSourceFellBack).toBe(true) + }) + + it("preference='origin' + upstream exists → queries origin (not upstream)", async () => { + resolveIssueSourceMock.mockResolvedValueOnce({ + source: { owner: 'fork', repo: 'orca' }, + fellBack: false + }) + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'fork', repo: 'orca' }) + ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: '[]' }).mockResolvedValueOnce({ + stdout: '[]' + }) + + await listWorkItems('/repo-root', 10, undefined, undefined, 'origin') + + expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( + 1, + expect.arrayContaining([ + 'repos/fork/orca/issues?per_page=10&state=open&sort=updated&direction=desc' + ]), + { cwd: '/repo-root' } + ) + }) + + it("preference='origin' + no upstream → queries origin", async () => { + resolveIssueSourceMock.mockResolvedValueOnce({ + source: { owner: 'solo', repo: 'orca' }, + fellBack: false + }) + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'solo', repo: 'orca' }) + ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: '[]' }).mockResolvedValueOnce({ + stdout: '[]' + }) + + await listWorkItems('/repo-root', 10, undefined, undefined, 'origin') + + expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( + 1, + expect.arrayContaining([ + 'repos/solo/orca/issues?per_page=10&state=open&sort=updated&direction=desc' + ]), + { cwd: '/repo-root' } + ) + }) + + it('surfaces upstreamCandidate in sources regardless of effective preference', async () => { + // Why: the renderer selector needs to keep rendering after the user picks + // 'origin'. That requires the envelope to carry the raw upstream even + // when `sources.issues` has collapsed onto origin. + resolveIssueSourceMock.mockResolvedValueOnce({ + source: { owner: 'fork', repo: 'orca' }, + fellBack: false + }) + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'fork', repo: 'orca' }) + getOwnerRepoForRemoteMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' }) + ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: '[]' }).mockResolvedValueOnce({ + stdout: '[]' + }) + + const result = await listWorkItems('/repo-root', 10, undefined, undefined, 'origin') + + expect(result.sources).toEqual({ + issues: { owner: 'fork', repo: 'orca' }, + prs: { owner: 'fork', repo: 'orca' }, + upstreamCandidate: { owner: 'stablyai', repo: 'orca' } + }) + }) + }) }) diff --git a/src/main/github/client-work-items.test.ts b/src/main/github/client-work-items.test.ts index b098ceade..3dadca518 100644 --- a/src/main/github/client-work-items.test.ts +++ b/src/main/github/client-work-items.test.ts @@ -1,3 +1,6 @@ +/* eslint-disable max-lines -- Why: work-items coverage stays in one file so +the fan-out mock plumbing (issue + PR gh calls, allSettled handling) does +not drift across split files. */ import { beforeEach, describe, expect, it, vi } from 'vitest' const { @@ -5,6 +8,8 @@ const { ghExecFileAsyncMock, getOwnerRepoMock, getIssueOwnerRepoMock, + getOwnerRepoForRemoteMock, + resolveIssueSourceMock, gitExecFileAsyncMock, acquireMock, releaseMock @@ -13,6 +18,8 @@ const { ghExecFileAsyncMock: vi.fn(), getOwnerRepoMock: vi.fn(), getIssueOwnerRepoMock: vi.fn(), + getOwnerRepoForRemoteMock: vi.fn(), + resolveIssueSourceMock: vi.fn(), gitExecFileAsyncMock: vi.fn(), acquireMock: vi.fn(), releaseMock: vi.fn() @@ -23,9 +30,13 @@ vi.mock('./gh-utils', () => ({ ghExecFileAsync: ghExecFileAsyncMock, getOwnerRepo: getOwnerRepoMock, getIssueOwnerRepo: getIssueOwnerRepoMock, + getOwnerRepoForRemote: getOwnerRepoForRemoteMock, + resolveIssueSource: resolveIssueSourceMock, acquire: acquireMock, release: releaseMock, - _resetOwnerRepoCache: vi.fn() + _resetOwnerRepoCache: vi.fn(), + classifyGhError: (stderr: string) => ({ type: 'unknown', message: stderr }), + classifyListIssuesError: (stderr: string) => ({ type: 'unknown', message: stderr }) })) vi.mock('../git/runner', () => ({ @@ -40,10 +51,20 @@ describe('listWorkItems', () => { ghExecFileAsyncMock.mockReset() getOwnerRepoMock.mockReset() getIssueOwnerRepoMock.mockReset() + getOwnerRepoForRemoteMock.mockReset() + resolveIssueSourceMock.mockReset() gitExecFileAsyncMock.mockReset() acquireMock.mockReset() releaseMock.mockReset() acquireMock.mockResolvedValue(undefined) + // Why: preference-aware `listWorkItems` calls `resolveIssueSource`. + // Route through the same `getIssueOwnerRepoMock` so existing tests that + // only set up `getIssueOwnerRepoMock` continue to work. + resolveIssueSourceMock.mockImplementation(async () => ({ + source: await getIssueOwnerRepoMock(), + fellBack: false + })) + getOwnerRepoForRemoteMock.mockResolvedValue(null) _resetOwnerRepoCache() }) @@ -81,7 +102,7 @@ describe('listWorkItems', () => { ]) }) const { items, sources } = await listWorkItems('/repo-root', 10, 'assignee:@me') - expect(sources).toEqual({ + expect(sources).toMatchObject({ issues: { owner: 'acme', repo: 'widgets' }, prs: { owner: 'acme', repo: 'widgets' } }) diff --git a/src/main/github/client.ts b/src/main/github/client.ts index 794c83f0e..ebf866f32 100644 --- a/src/main/github/client.ts +++ b/src/main/github/client.ts @@ -2,6 +2,7 @@ concurrency acquire/release pattern and error handling consistent across operations. */ import type { ClassifiedError, + IssueSourcePreference, ListWorkItemsResult, PRInfo, PRMergeableState, @@ -22,6 +23,8 @@ import { release, getOwnerRepo, getIssueOwnerRepo, + getOwnerRepoForRemote, + resolveIssueSource, classifyGhError, classifyListIssuesError, type OwnerRepo @@ -578,12 +581,20 @@ export async function listWorkItems( repoPath: string, limit = 24, query?: string, - before?: string + before?: string, + preference?: IssueSourcePreference ): Promise> { - const [issueOwnerRepo, prOwnerRepo] = await Promise.all([ - getIssueOwnerRepo(repoPath), - getOwnerRepo(repoPath) + // Why: resolve the raw upstream candidate alongside the preference-aware + // issue source. The selector needs to know whether an upstream remote + // *exists* to decide whether to render — independent of whether the user + // has picked 'origin' (which would otherwise make `sources.issues` equal + // origin and hide the selector permanently). + const [issueResolved, prOwnerRepo, upstreamCandidate] = await Promise.all([ + resolveIssueSource(repoPath, preference), + getOwnerRepo(repoPath), + getOwnerRepoForRemote(repoPath, 'upstream') ]) + const issueOwnerRepo = issueResolved.source const trimmedQuery = query?.trim() ?? '' await acquire() try { @@ -605,8 +616,13 @@ export async function listWorkItems( const errors = partial.issuesError ? { issues: partial.issuesError } : undefined return { items: partial.items, - sources: { issues: issueOwnerRepo, prs: prOwnerRepo }, - ...(errors ? { errors } : {}) + sources: { + issues: issueOwnerRepo, + prs: prOwnerRepo, + upstreamCandidate: upstreamCandidate ?? null + }, + ...(errors ? { errors } : {}), + ...(issueResolved.fellBack ? { issueSourceFellBack: true } : {}) } } finally { release() @@ -701,11 +717,16 @@ function defaultOpenWorkItemQuery(): ParsedTaskQuery { // Why: uses GitHub's search API to get total_count without fetching items. // This powers the pagination bar so the user sees total pages upfront. // Cached for 120s to avoid burning the search rate limit (30 req/min). -export async function countWorkItems(repoPath: string, query?: string): Promise { - const [issueOwnerRepo, prOwnerRepo] = await Promise.all([ - getIssueOwnerRepo(repoPath), +export async function countWorkItems( + repoPath: string, + query?: string, + preference?: IssueSourcePreference +): Promise { + const [issueResolved, prOwnerRepo] = await Promise.all([ + resolveIssueSource(repoPath, preference), getOwnerRepo(repoPath) ]) + const issueOwnerRepo = issueResolved.source const ownerRepo = prOwnerRepo ?? issueOwnerRepo if (!ownerRepo) { return 0 diff --git a/src/main/github/gh-utils.test.ts b/src/main/github/gh-utils.test.ts index afb73e833..8e1b8d839 100644 --- a/src/main/github/gh-utils.test.ts +++ b/src/main/github/gh-utils.test.ts @@ -11,9 +11,12 @@ vi.mock('../git/runner', () => ({ import { _resetOwnerRepoCache, + classifyGhError, + classifyListIssuesError, getIssueOwnerRepo, getOwnerRepo, - parseGitHubOwnerRepo + parseGitHubOwnerRepo, + resolveIssueSource } from './gh-utils' describe('github owner/repo resolution', () => { @@ -83,3 +86,113 @@ describe('github owner/repo resolution', () => { await expect(getIssueOwnerRepo('/repo')).resolves.toEqual({ owner: 'stablyai', repo: 'orca' }) }) }) + +describe('resolveIssueSource', () => { + beforeEach(() => { + gitExecFileAsyncMock.mockReset() + _resetOwnerRepoCache() + }) + + it("'auto' + upstream exists → upstream, fellBack=false", async () => { + gitExecFileAsyncMock.mockResolvedValueOnce({ + stdout: 'git@github.com:stablyai/orca.git\n' + }) + + await expect(resolveIssueSource('/repo', 'auto')).resolves.toEqual({ + source: { owner: 'stablyai', repo: 'orca' }, + fellBack: false + }) + }) + + it("'auto' + no upstream → origin, fellBack=false", async () => { + gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'git@example.com:stablyai/orca.git\n' }) + .mockResolvedValueOnce({ stdout: 'git@github.com:solo/orca.git\n' }) + + await expect(resolveIssueSource('/repo', 'auto')).resolves.toEqual({ + source: { owner: 'solo', repo: 'orca' }, + fellBack: false + }) + }) + + it("'upstream' + upstream exists → upstream, fellBack=false", async () => { + gitExecFileAsyncMock.mockResolvedValueOnce({ + stdout: 'git@github.com:stablyai/orca.git\n' + }) + + await expect(resolveIssueSource('/repo', 'upstream')).resolves.toEqual({ + source: { owner: 'stablyai', repo: 'orca' }, + fellBack: false + }) + }) + + it("'upstream' + no upstream remote → origin, fellBack=true", async () => { + // No upstream remote configured — the first call fails. + gitExecFileAsyncMock + .mockRejectedValueOnce(new Error('fatal: No such remote')) + .mockResolvedValueOnce({ stdout: 'git@github.com:solo/orca.git\n' }) + + await expect(resolveIssueSource('/repo', 'upstream')).resolves.toEqual({ + source: { owner: 'solo', repo: 'orca' }, + fellBack: true + }) + }) + + it("'origin' + upstream exists → origin (ignores upstream), fellBack=false", async () => { + // Only one gh call should happen — origin. Upstream is never consulted. + gitExecFileAsyncMock.mockResolvedValueOnce({ + stdout: 'git@github.com:fork/orca.git\n' + }) + + await expect(resolveIssueSource('/repo', 'origin')).resolves.toEqual({ + source: { owner: 'fork', repo: 'orca' }, + fellBack: false + }) + expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(1) + expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['remote', 'get-url', 'origin'], { + cwd: '/repo' + }) + }) + + it("'origin' + no upstream → origin, fellBack=false", async () => { + gitExecFileAsyncMock.mockResolvedValueOnce({ + stdout: 'git@github.com:solo/orca.git\n' + }) + + await expect(resolveIssueSource('/repo', 'origin')).resolves.toEqual({ + source: { owner: 'solo', repo: 'orca' }, + fellBack: false + }) + }) + + it('undefined preference is treated identically to auto', async () => { + gitExecFileAsyncMock.mockResolvedValueOnce({ + stdout: 'git@github.com:stablyai/orca.git\n' + }) + + await expect(resolveIssueSource('/repo', undefined)).resolves.toEqual({ + source: { owner: 'stablyai', repo: 'orca' }, + fellBack: false + }) + }) +}) + +describe('gh error classification', () => { + // Why: a fork with Issues turned off triggers `gh issue list` stderr + // "the '' repository has disabled issues". Without a dedicated branch + // the raw "Command failed: gh issue list …" line leaks into the Tasks banner + // via the `unknown` fallback — which is what users see when they flip the + // per-repo selector to an origin fork that has issues disabled. + it('classifies "has disabled issues" stderr as issues_disabled', () => { + const stderr = + "Command failed: gh issue list --limit 36 --json number,title,state --repo brennanb2025/orca --state open\nthe 'brennanb2025/orca' repository has disabled issues" + expect(classifyGhError(stderr)).toEqual({ + type: 'issues_disabled', + message: 'Issues are disabled on this repository.' + }) + expect(classifyListIssuesError(stderr)).toEqual({ + type: 'issues_disabled', + message: 'Issues are disabled on this repository.' + }) + }) +}) diff --git a/src/main/github/gh-utils.ts b/src/main/github/gh-utils.ts index e60cd026d..48f7afedb 100644 --- a/src/main/github/gh-utils.ts +++ b/src/main/github/gh-utils.ts @@ -1,7 +1,7 @@ import { execFile } from 'child_process' import { promisify } from 'util' import { gitExecFileAsync, ghExecFileAsync } from '../git/runner' -import type { ClassifiedError, GitHubOwnerRepo } from '../../shared/types' +import type { ClassifiedError, GitHubOwnerRepo, IssueSourcePreference } from '../../shared/types' // Why: legacy generic execFile wrapper — only used by callers that don't need // WSL-aware routing (e.g. non-repo-scoped gh commands). Repo-scoped callers @@ -54,6 +54,14 @@ export function classifyGhError(stderr: string): ClassifiedError { if (s.includes('http 404') || s.includes('could not resolve to a repository')) { return { type: 'not_found', message: 'Issue not found — it may have been deleted.' } } + // Why: `gh issue list` prints "the '/' repository has disabled + // issues" when Issues are turned off in repo settings (common on forks). This + // hits during feature-2 when a user flips the selector to an origin fork — + // without a dedicated branch the raw "Command failed: gh issue list …" line + // leaks verbatim into the banner via the `unknown` fallback. + if (s.includes('has disabled issues')) { + return { type: 'issues_disabled', message: 'Issues are disabled on this repository.' } + } if (s.includes('http 422') || s.includes('validation failed')) { return { type: 'validation_error', message: `Invalid update — ${stderr.trim()}` } } @@ -89,6 +97,7 @@ export function classifyListIssuesError(stderr: string): ClassifiedError { permission_denied: "You don't have permission to read issues for this repository. Check your GitHub token scopes.", not_found: 'Repository not found.', + issues_disabled: 'Issues are disabled on this repository.', validation_error: `Invalid request — ${trimmed}`, rate_limited: 'GitHub rate limit hit. Try again in a few minutes.', network_error: 'Network error — check your connection.', @@ -118,7 +127,7 @@ export function parseGitHubOwnerRepo(remoteUrl: string): OwnerRepo | null { return { owner: match[1], repo: match[2] } } -async function getOwnerRepoForRemote( +export async function getOwnerRepoForRemote( repoPath: string, remoteName: string ): Promise { @@ -153,3 +162,44 @@ export async function getIssueOwnerRepo(repoPath: string): Promise { + if (preference === 'upstream') { + const upstream = await getOwnerRepoForRemote(repoPath, 'upstream') + if (upstream) { + return { source: upstream, fellBack: false } + } + // Why: explicit upstream is gone — fall back to origin but only flag the + // fallback when it actually produced an origin source. If origin is also + // missing (or non-GitHub), there's nothing to "fall back to" and the + // UI toast "using origin" would be misleading. Do NOT auto-reset the + // preference: the user may be mid-way through a workflow and expect + // their choice to re-engage if `upstream` is re-added. + const origin = await getOwnerRepoForRemote(repoPath, 'origin') + return { source: origin, fellBack: origin !== null } + } + if (preference === 'origin') { + return { source: await getOwnerRepoForRemote(repoPath, 'origin'), fellBack: false } + } + // 'auto' or undefined + return { source: await getIssueOwnerRepo(repoPath), fellBack: false } +} diff --git a/src/main/github/issues.test.ts b/src/main/github/issues.test.ts index d82e7933b..bc5a8e0a4 100644 --- a/src/main/github/issues.test.ts +++ b/src/main/github/issues.test.ts @@ -1,9 +1,16 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import type * as GhUtils from './gh-utils' -const { ghExecFileAsyncMock, getIssueOwnerRepoMock, acquireMock, releaseMock } = vi.hoisted(() => ({ +const { + ghExecFileAsyncMock, + getIssueOwnerRepoMock, + resolveIssueSourceMock, + acquireMock, + releaseMock +} = vi.hoisted(() => ({ ghExecFileAsyncMock: vi.fn(), getIssueOwnerRepoMock: vi.fn(), + resolveIssueSourceMock: vi.fn(), acquireMock: vi.fn(), releaseMock: vi.fn() })) @@ -14,6 +21,7 @@ vi.mock('./gh-utils', async () => { ...actual, ghExecFileAsync: ghExecFileAsyncMock, getIssueOwnerRepo: getIssueOwnerRepoMock, + resolveIssueSource: resolveIssueSourceMock, acquire: acquireMock, release: releaseMock } @@ -25,9 +33,17 @@ describe('issue source operations', () => { beforeEach(() => { ghExecFileAsyncMock.mockReset() getIssueOwnerRepoMock.mockReset() + resolveIssueSourceMock.mockReset() acquireMock.mockReset() releaseMock.mockReset() acquireMock.mockResolvedValue(undefined) + // Why: preference-aware paths call resolveIssueSource instead of + // getIssueOwnerRepo. Route through the same mock so existing tests that + // set up getIssueOwnerRepoMock continue to work. + resolveIssueSourceMock.mockImplementation(async () => ({ + source: await getIssueOwnerRepoMock(), + fellBack: false + })) }) it('gets a single issue from the issue owner/repo', async () => { diff --git a/src/main/github/issues.ts b/src/main/github/issues.ts index 46e79296d..d9e2b7f5e 100644 --- a/src/main/github/issues.ts +++ b/src/main/github/issues.ts @@ -1,23 +1,44 @@ +/* eslint-disable max-lines -- Why: co-locating issue list/create/update/ +comment operations keeps the shared acquire/release + error-classification +pattern obvious. Each function is short; the file is long because the +surface is broad. */ import type { ClassifiedError, GitHubAssignableUser, GitHubCommentResult, GitHubIssueUpdate, IssueInfo, + IssueSourcePreference, PRComment } from '../../shared/types' import { mapIssueInfo } from './mappers' // prettier-ignore -import { ghExecFileAsync, acquire, release, getIssueOwnerRepo, classifyGhError, classifyListIssuesError } from './gh-utils' +import { ghExecFileAsync, acquire, release, getIssueOwnerRepo, resolveIssueSource, classifyGhError, classifyListIssuesError } from './gh-utils' // Why: distinguishes a successful-empty listing from a failed fetch. The // previous `catch { return [] }` conflated a 403 on a private upstream with an // empty backlog. Callers decide how to surface `error`. -export type IssueListResult = { items: IssueInfo[]; error?: ClassifiedError } +// +// Why no `fellBack` here: the fell-back signal for the renderer toast rides on +// `ListWorkItemsResult.issueSourceFellBack` (the Tasks list's envelope). The +// only consumer of `listIssues` — the `gh:listIssues` IPC handler — unwraps +// to `.items` and has no UI hook to surface a fallback toast. Adding a dead +// `fellBack` field here invited drift between the JSDoc promise and reality. +export type IssueListResult = { + items: IssueInfo[] + error?: ClassifiedError +} /** * Get a single issue by number. * Uses gh api --cache so 304 Not Modified responses don't count against the rate limit. + * + * Why this path doesn't take a preference: linked-issue lookups persist a + * number to a worktree at creation time. Routing detail lookups through the + * live per-repo preference would silently flip an existing link to a + * different repo after the user toggled the selector — the opposite of what + * #1186 / the parent design doc guard against. List and create paths honor + * preference; number-resolution stays on the heuristic. */ export async function getIssue(repoPath: string, issueNumber: number): Promise { const ownerRepo = await getIssueOwnerRepo(repoPath) @@ -61,8 +82,12 @@ export async function getIssue(repoPath: string, issueNumber: number): Promise { - const ownerRepo = await getIssueOwnerRepo(repoPath) +export async function listIssues( + repoPath: string, + limit = 20, + preference?: IssueSourcePreference +): Promise { + const { source: ownerRepo } = await resolveIssueSource(repoPath, preference) await acquire() try { if (ownerRepo) { @@ -92,10 +117,15 @@ export async function listIssues(repoPath: string, limit = 20): Promise mapIssueInfo(d as Parameters[0])) } + return { + items: data.map((d) => mapIssueInfo(d as Parameters[0])) + } } catch (err) { const stderr = err instanceof Error ? err.message : String(err) - return { items: [], error: classifyListIssuesError(stderr) } + return { + items: [], + error: classifyListIssuesError(stderr) + } } finally { release() } @@ -109,13 +139,14 @@ export async function listIssues(repoPath: string, limit = 20): Promise { const trimmedTitle = title.trim() if (!trimmedTitle) { return { ok: false, error: 'Title is required' } } - const ownerRepo = await getIssueOwnerRepo(repoPath) + const { source: ownerRepo } = await resolveIssueSource(repoPath, preference) if (!ownerRepo) { return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' } } @@ -154,6 +185,15 @@ export async function createIssue( /** * Update an existing GitHub issue. Fans out to separate gh commands for * state changes vs field edits since `gh issue edit` does not support state. + * + * Why this path doesn't take a preference (mirrors `getIssue`): mutations + * target an issue number already bound to a worktree / linked elsewhere in + * the UI. Routing an update through the live per-repo preference would let + * a user open upstream#N, toggle the selector to origin, save, and silently + * write to origin#N — a different issue (or 404). That is the exact + * silent-source-switch class of wrongness #1186 / the parent design doc + * guard against. List and create paths honor preference; mutations stay on + * the heuristic `getIssueOwnerRepo`. */ export async function updateIssue( repoPath: string, @@ -230,6 +270,18 @@ export async function updateIssue( return { ok: true } } +/** + * Add a comment to an existing GitHub issue. + * + * Why this path doesn't take a preference (mirrors `getIssue` / `updateIssue`): + * a comment is posted against an issue number already bound to a worktree or + * surfaced from a prior read. Routing through the live per-repo preference + * would let a user read upstream#N, toggle the selector to origin, and have + * their reply silently post on origin#N — a different issue entirely. That + * is the same silent-source-switch class of wrongness #1186 / the parent + * design doc guard against. List and create paths honor preference; + * mutations stay on the heuristic `getIssueOwnerRepo`. + */ export async function addIssueComment( repoPath: string, issueNumber: number, @@ -277,8 +329,11 @@ export async function addIssueComment( } } -export async function listLabels(repoPath: string): Promise { - const ownerRepo = await getIssueOwnerRepo(repoPath) +export async function listLabels( + repoPath: string, + preference?: IssueSourcePreference +): Promise { + const { source: ownerRepo } = await resolveIssueSource(repoPath, preference) if (!ownerRepo) { return [] } @@ -305,8 +360,11 @@ export async function listLabels(repoPath: string): Promise { } } -export async function listAssignableUsers(repoPath: string): Promise { - const ownerRepo = await getIssueOwnerRepo(repoPath) +export async function listAssignableUsers( + repoPath: string, + preference?: IssueSourcePreference +): Promise { + const { source: ownerRepo } = await resolveIssueSource(repoPath, preference) if (!ownerRepo) { return [] } diff --git a/src/main/ipc/github.test.ts b/src/main/ipc/github.test.ts index 5b762d46f..8e0957530 100644 --- a/src/main/ipc/github.test.ts +++ b/src/main/ipc/github.test.ts @@ -1,13 +1,20 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -const { handleMock, getPRForBranchMock, getIssueMock, listIssuesMock, getAuthenticatedViewerMock } = - vi.hoisted(() => ({ - handleMock: vi.fn(), - getPRForBranchMock: vi.fn(), - getIssueMock: vi.fn(), - listIssuesMock: vi.fn(), - getAuthenticatedViewerMock: vi.fn() - })) +const { + handleMock, + getPRForBranchMock, + getIssueMock, + listIssuesMock, + listWorkItemsMock, + getAuthenticatedViewerMock +} = vi.hoisted(() => ({ + handleMock: vi.fn(), + getPRForBranchMock: vi.fn(), + getIssueMock: vi.fn(), + listIssuesMock: vi.fn(), + listWorkItemsMock: vi.fn(), + getAuthenticatedViewerMock: vi.fn() +})) vi.mock('electron', () => ({ ipcMain: { @@ -19,6 +26,7 @@ vi.mock('../github/client', () => ({ getPRForBranch: getPRForBranchMock, getIssue: getIssueMock, listIssues: listIssuesMock, + listWorkItems: listWorkItemsMock, getAuthenticatedViewer: getAuthenticatedViewerMock })) @@ -28,16 +36,17 @@ type HandlerMap = Record unknown> describe('registerGitHubHandlers', () => { const handlers: HandlerMap = {} + type FixtureRepo = { + id: string + path: string + displayName: string + badgeColor: string + addedAt: number + issueSourcePreference?: 'origin' | 'upstream' + } + let repos: FixtureRepo[] = [] const store = { - getRepos: () => [ - { - id: 'repo-1', - path: '/workspace/repo', - displayName: 'repo', - badgeColor: '#000', - addedAt: 0 - } - ] + getRepos: () => repos } const stats = { hasCountedPR: () => false, @@ -49,11 +58,26 @@ describe('registerGitHubHandlers', () => { getPRForBranchMock.mockReset() getIssueMock.mockReset() listIssuesMock.mockReset() + listWorkItemsMock.mockReset() getAuthenticatedViewerMock.mockReset() for (const key of Object.keys(handlers)) { delete handlers[key] } + // Reset fixture repos to the default single-repo fixture each test, so + // individual tests can mutate the list without leaking preferences across + // tests (e.g. a preference-threading test could otherwise shadow the + // default-undefined assertions in sibling tests). + repos = [ + { + id: 'repo-1', + path: '/workspace/repo', + displayName: 'repo', + badgeColor: '#000', + addedAt: 0 + } + ] + handleMock.mockImplementation((channel, handler) => { handlers[channel] = handler }) @@ -95,7 +119,7 @@ describe('registerGitHubHandlers', () => { limit: 5 }) - expect(listIssuesMock).toHaveBeenCalledWith('/workspace/repo', 5) + expect(listIssuesMock).toHaveBeenCalledWith('/workspace/repo', 5, undefined) expect(result).toEqual([]) }) @@ -123,10 +147,52 @@ describe('registerGitHubHandlers', () => { limit: 5 }) - expect(listIssuesMock).toHaveBeenCalledWith('/workspace/repo', 5) + expect(listIssuesMock).toHaveBeenCalledWith('/workspace/repo', 5, undefined) expect(result).toEqual([]) }) + it('threads issueSourcePreference through gh:listIssues', async () => { + // Why: repo.issueSourcePreference must reach listIssues so the upstream + // repo is queried when configured. A regression that drops the arg would + // pass the default-fixture tests (which assert `undefined`) silently, so + // this test pins the non-undefined preference-threading contract. + repos[0].issueSourcePreference = 'upstream' + listIssuesMock.mockResolvedValue({ items: [] }) + + registerGitHubHandlers(store as never, stats as never) + + await handlers['gh:listIssues'](null, { + repoPath: '/workspace/repo', + limit: 5 + }) + + expect(listIssuesMock).toHaveBeenCalledWith('/workspace/repo', 5, 'upstream') + }) + + it('threads issueSourcePreference through gh:listWorkItems', async () => { + // Why: gh:listWorkItems must also forward repo.issueSourcePreference + // (5th arg) so the work-items view honors the per-repo source selector. + repos[0].issueSourcePreference = 'origin' + listWorkItemsMock.mockResolvedValue({ items: [] }) + + registerGitHubHandlers(store as never, stats as never) + + await handlers['gh:listWorkItems'](null, { + repoPath: '/workspace/repo', + limit: 10, + query: 'is:open', + before: 'cursor-1' + }) + + expect(listWorkItemsMock).toHaveBeenCalledWith( + '/workspace/repo', + 10, + 'is:open', + 'cursor-1', + 'origin' + ) + }) + it('forwards the authenticated viewer lookup', async () => { getAuthenticatedViewerMock.mockResolvedValue({ login: 'octocat', email: 'octocat@example.com' }) diff --git a/src/main/ipc/github.ts b/src/main/ipc/github.ts index a75e1b66e..371bf6cd2 100644 --- a/src/main/ipc/github.ts +++ b/src/main/ipc/github.ts @@ -1,3 +1,7 @@ +/* eslint-disable max-lines -- Why: all GitHub IPC handlers stay co-located so +the repo-path validation, preference-threading, and stats wiring patterns are +reviewable as one surface. Splitting by feature area would risk drifting +validation/gate conventions across handler files. */ import { ipcMain } from 'electron' import { resolve } from 'path' import type { Repo, GitHubIssueUpdate } from '../../shared/types' @@ -70,14 +74,14 @@ export function registerGitHubHandlers(store: Store, stats: StatsCollector): voi // Why: listIssues now returns { items, error? }. The IPC handler unwraps to // the items array for the existing contract; feature 1's UI consumes the // richer envelope through `gh:listWorkItems` instead. - return listIssues(repo.path, args.limit).then((r) => r.items) + return listIssues(repo.path, args.limit, repo.issueSourcePreference).then((r) => r.items) }) ipcMain.handle( 'gh:createIssue', (_event, args: { repoPath: string; title: string; body: string }) => { const repo = assertRegisteredRepo(args.repoPath, store) - return createIssue(repo.path, args.title, args.body) + return createIssue(repo.path, args.title, args.body, repo.issueSourcePreference) } ) @@ -85,13 +89,19 @@ export function registerGitHubHandlers(store: Store, stats: StatsCollector): voi 'gh:listWorkItems', (_event, args: { repoPath: string; limit?: number; query?: string; before?: string }) => { const repo = assertRegisteredRepo(args.repoPath, store) - return listWorkItems(repo.path, args.limit, args.query, args.before) + return listWorkItems( + repo.path, + args.limit, + args.query, + args.before, + repo.issueSourcePreference + ) } ) ipcMain.handle('gh:countWorkItems', (_event, args: { repoPath: string; query?: string }) => { const repo = assertRegisteredRepo(args.repoPath, store) - return countWorkItems(repo.path, args.query) + return countWorkItems(repo.path, args.query, repo.issueSourcePreference) }) ipcMain.handle('gh:workItem', (_event, args: WorkItemArgs) => @@ -315,16 +325,23 @@ export function registerGitHubHandlers(store: Store, stats: StatsCollector): voi ipcMain.handle('gh:listLabels', (_event, args: { repoPath: string }) => { const repo = assertRegisteredRepo(args.repoPath, store) - return listLabels(repo.path) + return listLabels(repo.path, repo.issueSourcePreference) }) ipcMain.handle('gh:listAssignableUsers', (_event, args: { repoPath: string }) => { const repo = assertRegisteredRepo(args.repoPath, store) - return listAssignableUsers(repo.path) + return listAssignableUsers(repo.path, repo.issueSourcePreference) }) // Star operations target the Orca repo itself — no repoPath validation needed ipcMain.handle('gh:viewer', () => getAuthenticatedViewer()) ipcMain.handle('gh:checkOrcaStarred', () => checkOrcaStarred()) ipcMain.handle('gh:starOrca', () => starOrca()) + + // Why: issue-source preference writes go through the generic `repos:update` + // IPC (extended in this PR to accept `issueSourcePreference`). Routing + // through the same channel keeps a single write path, guarantees the + // `repos:changed` broadcast is emitted, and avoids two channels racing to + // persist the same field with different validation and eviction semantics. + // Reads piggyback on the `Repo` record already delivered by `repos:list`. } diff --git a/src/main/ipc/repos.ts b/src/main/ipc/repos.ts index 3f1b13df5..d35624fac 100644 --- a/src/main/ipc/repos.ts +++ b/src/main/ipc/repos.ts @@ -211,11 +211,35 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v args: { repoId: string updates: Partial< - Pick + Pick< + Repo, + | 'displayName' + | 'badgeColor' + | 'hookSettings' + | 'worktreeBaseRef' + | 'kind' + | 'issueSourcePreference' + > > } ) => { - const updated = store.updateRepo(args.repoId, args.updates) + // Why: validate the persisted preference string at the IPC boundary + // — the TypeScript signature is erased at runtime, and a preload + // version skew or renderer bug could otherwise persist a garbage + // string that silently collapses to 'auto' in `resolveIssueSource` + // (see gh-utils.ts#resolveIssueSource). Strip rather than throw so + // other valid fields in the same call still persist. + const updates = { ...args.updates } + if ( + 'issueSourcePreference' in updates && + updates.issueSourcePreference !== undefined && + updates.issueSourcePreference !== 'upstream' && + updates.issueSourcePreference !== 'origin' && + updates.issueSourcePreference !== 'auto' + ) { + delete updates.issueSourcePreference + } + const updated = store.updateRepo(args.repoId, updates) if (updated) { notifyReposChanged(mainWindow) } diff --git a/src/main/persistence.test.ts b/src/main/persistence.test.ts index 3dab3a20b..53560349c 100644 --- a/src/main/persistence.test.ts +++ b/src/main/persistence.test.ts @@ -249,6 +249,34 @@ describe('Store', () => { expect(store.updateRepo('nope', { displayName: 'x' })).toBeNull() }) + it('updateRepo persists issueSourcePreference across reloads', async () => { + const store = await createStore() + store.addRepo(makeRepo()) + + const updated = store.updateRepo('r1', { issueSourcePreference: 'upstream' }) + expect(updated!.issueSourcePreference).toBe('upstream') + + store.flush() + const reloaded = await createStore() + expect(reloaded.getRepo('r1')!.issueSourcePreference).toBe('upstream') + }) + + it('updateRepo with issueSourcePreference=undefined clears the preference', async () => { + const store = await createStore() + store.addRepo(makeRepo({ issueSourcePreference: 'origin' })) + expect(store.getRepo('r1')!.issueSourcePreference).toBe('origin') + + // Why: passing the key with value `undefined` must clear the preference. + // Plain `Object.assign` skips undefined values, so without the explicit + // delete branch in updateRepo, the persisted record would keep 'origin'. + store.updateRepo('r1', { issueSourcePreference: undefined }) + expect(store.getRepo('r1')!.issueSourcePreference).toBeUndefined() + + store.flush() + const reloaded = await createStore() + expect(reloaded.getRepo('r1')!.issueSourcePreference).toBeUndefined() + }) + // ── 8. setWorktreeMeta and getWorktreeMeta ───────────────────────── it('setWorktreeMeta creates meta with defaults for missing fields', async () => { diff --git a/src/main/persistence.ts b/src/main/persistence.ts index 578c926c2..2815254ec 100644 --- a/src/main/persistence.ts +++ b/src/main/persistence.ts @@ -365,14 +365,33 @@ export class Store { updateRepo( id: string, updates: Partial< - Pick + Pick< + Repo, + | 'displayName' + | 'badgeColor' + | 'hookSettings' + | 'worktreeBaseRef' + | 'kind' + | 'issueSourcePreference' + > > ): Repo | null { const repo = this.state.repos.find((r) => r.id === id) if (!repo) { return null } - Object.assign(repo, updates) + // Why: `issueSourcePreference === undefined` in the patch means "reset to + // auto" (and the persisted record should drop the key, not preserve a + // stale explicit value via Object.assign's skip-on-undefined behavior). + // Without this delete branch, toggling explicit → auto would silently + // leave the old preference in place on disk. + if ('issueSourcePreference' in updates && updates.issueSourcePreference === undefined) { + delete repo.issueSourcePreference + const { issueSourcePreference: _drop, ...rest } = updates + Object.assign(repo, rest) + } else { + Object.assign(repo, updates) + } this.scheduleSave() return this.hydrateRepo(repo) } diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index f722057a2..e0a0e8ccf 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -312,7 +312,15 @@ export type PreloadApi = { update: (args: { repoId: string updates: Partial< - Pick + Pick< + Repo, + | 'displayName' + | 'badgeColor' + | 'hookSettings' + | 'worktreeBaseRef' + | 'kind' + | 'issueSourcePreference' + > > }) => Promise pickFolder: () => Promise diff --git a/src/renderer/src/components/TaskPage.tsx b/src/renderer/src/components/TaskPage.tsx index a2555244c..4319dd131 100644 --- a/src/renderer/src/components/TaskPage.tsx +++ b/src/renderer/src/components/TaskPage.tsx @@ -58,6 +58,7 @@ import RepoMultiCombobox from '@/components/ui/repo-multi-combobox' import TeamMultiCombobox from '@/components/ui/team-multi-combobox' import RepoDotLabel from '@/components/repo/RepoDotLabel' import IssueSourceIndicator, { sameGitHubOwnerRepo } from '@/components/github/IssueSourceIndicator' +import IssueSourceSelector, { issueSourceChipClass } from '@/components/github/IssueSourceSelector' import { stripRepoQualifiers } from '../../../shared/task-query' import GitHubItemDialog from '@/components/GitHubItemDialog' import LinearItemDrawer from '@/components/LinearItemDrawer' @@ -635,6 +636,20 @@ const hasDivergentSources = ( sources: { issues: GitHubOwnerRepo; prs: GitHubOwnerRepo } } => !!s.sources?.issues && !!s.sources.prs && !sameGitHubOwnerRepo(s.sources.issues, s.sources.prs) +// Why: the selector keeps rendering even after the user picks 'origin' (which +// collapses `sources.issues` onto origin). Upstream-candidate divergence is +// the right render gate — a repo that has an `upstream` remote pointing +// somewhere different from origin is always a candidate for the toggle, +// regardless of the current effective preference. +const hasUpstreamCandidateDivergence = ( + s: RepoSourceState +): s is RepoSourceState & { + sources: { prs: GitHubOwnerRepo; upstreamCandidate: GitHubOwnerRepo } +} => + !!s.sources?.prs && + !!s.sources.upstreamCandidate && + !sameGitHubOwnerRepo(s.sources.prs, s.sources.upstreamCandidate) + export default function TaskPage(): React.JSX.Element { const settings = useAppStore((s) => s.settings) const pageData = useAppStore((s) => s.taskPageData) @@ -646,6 +661,12 @@ export default function TaskPage(): React.JSX.Element { const updateSettings = useAppStore((s) => s.updateSettings) const fetchWorkItemsAcrossRepos = useAppStore((s) => s.fetchWorkItemsAcrossRepos) const getCachedWorkItems = useAppStore((s) => s.getCachedWorkItems) + const setIssueSourcePreference = useAppStore((s) => s.setIssueSourcePreference) + // Why: bumped by `setIssueSourcePreference` after cache eviction so the + // fetch effect below re-runs and repopulates work-items against the new + // source. Eviction alone isn't enough because the effect's deps don't + // include `workItemsCache`. + const workItemsInvalidationNonce = useAppStore((s) => s.workItemsInvalidationNonce) const linearStatus = useAppStore((s) => s.linearStatus) const linearStatusChecked = useAppStore((s) => s.linearStatusChecked) const connectLinear = useAppStore((s) => s.connectLinear) @@ -781,6 +802,12 @@ export default function TaskPage(): React.JSX.Element { // user clicking the refresh button (force=true) vs. re-running for any // other reason — e.g. a repo change while the nonce happens to be > 0. const lastFetchedNonceRef = useRef(-1) + // Why: analogous to `lastFetchedNonceRef` for the invalidation nonce. A + // preference flip should force the dispatch past fetch-dedupe (same repos + + // same query, cache just evicted — without `force: true` the fan-out could + // collapse onto a stale in-flight request that resolved against the + // pre-flip source). + const lastFetchedInvalidationNonceRef = useRef(0) // Why: pages holds all fetched pages of work items. Page 0 is seeded from // cache for instant first paint; subsequent pages are loaded via date cursors. const [pages, setPages] = useState(() => { @@ -879,6 +906,36 @@ export default function TaskPage(): React.JSX.Element { }) }, [selectedRepos, appliedTaskSearch, workItemsCache]) + // Why: surface a one-time toast per session per repo when the user's + // preferred `'upstream'` is no longer configured and we fell back to + // origin. Gated on a ref-backed set so repeated list refreshes don't + // re-toast. We deliberately do NOT auto-reset the preference — the user + // may re-add `upstream` later and expect it to pick up again. + const fellBackToastedRef = useRef>(new Set()) + useEffect(() => { + if (taskSource !== 'github') { + return + } + const appliedQ = stripRepoQualifiers(appliedTaskSearch.trim()) + for (const r of selectedRepos) { + const key = workItemsCacheKey(r.path, PER_REPO_FETCH_LIMIT, appliedQ) + const entry = workItemsCache[key] + if (!entry?.issueSourceFellBack) { + continue + } + if (fellBackToastedRef.current.has(r.id)) { + continue + } + const prSlug = entry.sources?.prs + ? `${entry.sources.prs.owner}/${entry.sources.prs.repo}` + : r.displayName + toast.message( + `Your preferred issue source (upstream) is no longer configured for ${prSlug}. Using origin.` + ) + fellBackToastedRef.current.add(r.id) + } + }, [selectedRepos, appliedTaskSearch, workItemsCache, taskSource]) + // Why: on a partial-failure retry the cache still holds successful-side // data, so `tasksLoading` (which is gated on `anyUncached`) never flips // true and the Retry button would otherwise give no feedback. Track @@ -1186,6 +1243,13 @@ export default function TaskPage(): React.JSX.Element { // Preserve the existing nonce-gated force behavior. const forceRefresh = taskRefreshNonce !== lastFetchedNonceRef.current lastFetchedNonceRef.current = taskRefreshNonce + // Why: a preference flip bumps `workItemsInvalidationNonce`. Treat that + // bump as a forced refresh so the fan-out bypasses the in-flight dedupe + // map — otherwise an overlapping request started before the flip could + // resolve the new fetch and repopulate the cache with pre-flip data. + const preferenceInvalidated = + workItemsInvalidationNonce !== lastFetchedInvalidationNonceRef.current + lastFetchedInvalidationNonceRef.current = workItemsInvalidationNonce const repoArgs = selectedRepos.map((r) => ({ repoId: r.id, path: r.path })) // Why: snapshot the retrying paths at effect-dispatch so overlapping @@ -1195,7 +1259,7 @@ export default function TaskPage(): React.JSX.Element { // when this effect dispatched preserves later additions. const dispatchedRetryPaths = retryingRepoPaths void fetchWorkItemsAcrossRepos(repoArgs, PER_REPO_FETCH_LIMIT, CROSS_REPO_DISPLAY_LIMIT, q, { - force: forceRefresh && taskRefreshNonce > 0 + force: (forceRefresh && taskRefreshNonce > 0) || preferenceInvalidated }) .then(({ items, failedCount: failed }) => { // Why: clear only the repos this effect was responsible for @@ -1266,9 +1330,10 @@ export default function TaskPage(): React.JSX.Element { } // Why: getCachedWorkItems and fetchWorkItemsAcrossRepos are stable zustand // selectors; depending on them would re-run the effect on unrelated store - // updates. + // updates. `workItemsInvalidationNonce` is explicitly included so a + // preference flip (which only evicts cache) re-dispatches this effect. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [selectedRepos, appliedTaskSearch, taskRefreshNonce, taskSource]) + }, [selectedRepos, appliedTaskSearch, taskRefreshNonce, taskSource, workItemsInvalidationNonce]) const handleApplyTaskSearch = useCallback((): void => { const trimmed = taskSearchInput.trim() @@ -1927,43 +1992,71 @@ export default function TaskPage(): React.JSX.Element { {(() => { - // Why: compute the visible list once so the visibility - // gate and the map don't re-run the same predicate. - // `hasDivergentSources` lives at module scope so the - // predicate isn't re-allocated on every render; the - // narrowed return type means `sources.issues` and - // `sources.prs` are known non-null inside the map. - const visibleRepoSources = perRepoSourceState.filter(hasDivergentSources) - if (visibleRepoSources.length === 0) { + // Why: unify feature 1 (indicator) and feature 2 (selector) + // into a single chip per repo. Rendering both separately + // produced visually redundant output — two local-repo + // dot-labels, duplicate slugs. The selector's active pill + // + tooltip already announce the source, so the "Issues + // from {slug}" chip is only shown when the selector does + // not render (no upstream remote — nothing to toggle). + const rows = perRepoSourceState.filter( + (s) => hasUpstreamCandidateDivergence(s) || hasDivergentSources(s) + ) + if (rows.length === 0) { return null } - // Why: per parent design doc §2, the indicator lives near - // the filter row. It's the self-announcing surface that - // makes the convention-based routing in #1076 legitimate - // — without it, a fork contributor filing a personal TODO - // against `upstream` has no way to tell before submit. return (
- {visibleRepoSources.map((s) => { - // Why: when multiple repos are selected, attach the - // local dot-label so readers can tell which chip - // describes which repo. Single-repo views omit it — - // the repo is already unambiguous from context. - const repo = - selectedRepos.length > 1 - ? selectedRepos.find((r) => r.id === s.repoId) - : undefined + {rows.map((s) => { + const repo = selectedRepos.find((r) => r.id === s.repoId) + const showDotLabel = selectedRepos.length > 1 && repo + const selectorRenderable = hasUpstreamCandidateDivergence(s) + // Why: the static indicator has its own wrapping + // chip styles, so we render it standalone and don't + // nest it inside our own chip — nesting would + // double-border it. + if (!selectorRenderable && hasDivergentSources(s)) { + return ( + + ) + } + if (!selectorRenderable || !repo) { + return null + } + // Why: must be a
(not ) because the child + // renders a
, and + // a block-level
nested inside an inline is + // invalid HTML — React emits a hydration warning and + // browsers may auto-close the span. `issueSourceChipClass` + // uses `inline-flex`, so the visual rendering is identical. return ( - +
+ {showDotLabel ? ( + + ) : null} + { + void setIssueSourcePreference(repo.id, repo.path, next) + }} + /> +
) })}
@@ -2593,30 +2686,76 @@ export default function TaskPage(): React.JSX.Element { > New GitHub issue - - {(() => { - // Why: parent design doc §1 surface 2 — the composer is the - // non-negotiable surface because User D's regression (filing a - // personal TODO against upstream/fork after #1076 changed - // routing) is specifically about this dialog. The description - // line doubles as the source indicator: inlining the resolved - // `{owner}/{repo}` slug (e.g. "stablyai/orca") means the - // destination is impossible to miss before the user submits, - // without needing a secondary chip that duplicates the info. - // Falls back to the local displayName when the slug isn't - // resolved yet (pre-IPC cache hit, or non-GitHub remote). The - // multi-repo case uses the same computation — the Select below - // drives `newIssueTargetRepo`, so the active target is known. - const entry = newIssueTargetRepo - ? perRepoSourceState.find((s) => s.repoId === newIssueTargetRepo.id) - : undefined - const issuesSlug = entry?.sources?.issues - ? `${entry.sources.issues.owner}/${entry.sources.issues.repo}` - : null - const fallback = newIssueTargetRepo?.displayName ?? 'this repository' - return `Filing in ${issuesSlug ?? fallback}` - })()} - + {(() => { + // Why: parent design doc §1 surface 2 — the composer is the + // non-negotiable surface because User D's regression (filing a + // personal TODO against upstream/fork after #1076 changed + // routing) is specifically about this dialog. The description + // line doubles as the source indicator: inlining the resolved + // `{owner}/{repo}` slug (e.g. "stablyai/orca") means the + // destination is impossible to miss before the user submits, + // without needing a secondary chip that duplicates the info. + // Falls back to the local displayName when the slug isn't + // resolved yet (pre-IPC cache hit, or non-GitHub remote). The + // multi-repo case uses the same computation — the Select below + // drives `newIssueTargetRepo`, so the active target is known. + const entry = newIssueTargetRepo + ? perRepoSourceState.find((s) => s.repoId === newIssueTargetRepo.id) + : undefined + const issuesSlug = entry?.sources?.issues + ? `${entry.sources.issues.owner}/${entry.sources.issues.repo}` + : null + const fallback = newIssueTargetRepo?.displayName ?? 'this repository' + return Filing in {issuesSlug ?? fallback} + })()} + {(() => { + // Why: mirror the Tasks-view selector in the composer so User D + // (fork contributor filing a personal TODO against their own + // fork) can flip the target *at the moment of filing* — the + // only moment that matters for this regression. Reuses the + // same cache entry the description line reads so no extra + // IPC round-trip is needed. + // + // Why sibling of DialogDescription (not nested inside it): + // DialogDescription renders a

, and `IssueSourceSelector` + // renders a

with + +
+ ) + + // Why: on surfaces where only issues are ever relevant (Create Issue + // composer) the "Issues from " hover text is redundant and can even + // mislead by implying a PR/issue split the user isn't thinking about. Let + // the caller opt out via `suppressTooltip` rather than branching on page + // identity inside this component. + if (suppressTooltip) { + return group + } + + return ( + + {group} + + Showing issues from{' '} + {effective === 'upstream' ? upstreamSlug : originSlug} + + + ) +} diff --git a/src/renderer/src/components/new-workspace/CreateFromTab.tsx b/src/renderer/src/components/new-workspace/CreateFromTab.tsx index b472e1b71..a0f8d07fc 100644 --- a/src/renderer/src/components/new-workspace/CreateFromTab.tsx +++ b/src/renderer/src/components/new-workspace/CreateFromTab.tsx @@ -17,6 +17,8 @@ import { Input } from '@/components/ui/input' import { Popover, PopoverAnchor, PopoverContent } from '@/components/ui/popover' import RepoCombobox from '@/components/repo/RepoCombobox' import { Tabs, TabsList, TabsTrigger } from '@/components/ui/tabs' +import IssueSourceSelector from '@/components/github/IssueSourceSelector' +import { sameGitHubOwnerRepo } from '@/components/github/IssueSourceIndicator' import { useAppStore } from '@/store' import { cn } from '@/lib/utils' import { normalizeGitHubLinkQuery } from '@/lib/github-links' @@ -100,7 +102,9 @@ export default function CreateFromTab({ setRememberedSubTab, fetchWorkItems, getCachedWorkItems, - getWorkItemsSourcesAndError + getWorkItemsSourcesAndError, + setIssueSourcePreference, + workItemsInvalidationNonce } = useAppStore( useShallow((s) => ({ activeRepoId: s.activeRepoId, @@ -112,7 +116,14 @@ export default function CreateFromTab({ setRememberedSubTab: s.setCreateFromSubTab, fetchWorkItems: s.fetchWorkItems, getCachedWorkItems: s.getCachedWorkItems, - getWorkItemsSourcesAndError: s.getWorkItemsSourcesAndError + getWorkItemsSourcesAndError: s.getWorkItemsSourcesAndError, + setIssueSourcePreference: s.setIssueSourcePreference, + // Why: re-run the issues fetch effect after `setIssueSourcePreference` + // evicts this repo's cache entries — otherwise flipping the selector + // clears the cache but the effect's deps wouldn't force a refetch and + // the Issues sub-tab would briefly show stale (then empty-once-cached- + // expires) data until the user typed or switched tabs. + workItemsInvalidationNonce: s.workItemsInvalidationNonce })) ) @@ -367,7 +378,11 @@ export default function CreateFromTab({ normalizedGhQuery.directNumber, fetchWorkItems, getCachedWorkItems, - getWorkItemsSourcesAndError + getWorkItemsSourcesAndError, + // Why: flipping the issue-source selector evicts this repo's cache and + // bumps the nonce so this effect re-runs against the new preference. + // Matches the identical pattern in TaskPage's Tasks-list fetch effect. + workItemsInvalidationNonce ]) // --------------------------------------------------------------------- @@ -752,6 +767,29 @@ export default function CreateFromTab({ const showGhRepoPicker = subTab !== 'linear' + // Why: render the issue-source selector inline with the Issues sub-tab's + // search input so a user who lands on an upstream/origin mismatch can + // correct it in place instead of closing the modal, flipping from the + // Tasks page, and re-opening. Sources come from whatever `workItemsCache` + // entry exists for this repo (empty-query or a past search) — the per-repo + // `listWorkItems` envelope always stamps both `prs` and `upstreamCandidate` + // so we can re-use the same derived shape the Tasks header consumes. + // Subscribing through `getWorkItemsAnySourcesForRepo` (a selector that + // returns a stable reference for unchanged entries) keeps this cheap — + // unrelated cache writes don't re-render the tab. + const selectorSources = useAppStore((s) => + selectedRepo?.path && !isRemoteRepo + ? s.getWorkItemsAnySourcesForRepo(selectedRepo.path, COMBINED_WORK_ITEM_LIMIT) + : null + ) + const selectorOrigin = selectorSources?.prs ?? null + const selectorUpstream = selectorSources?.upstreamCandidate ?? null + const selectorRenderable = + subTab === 'issues' && + !!selectorOrigin && + !!selectorUpstream && + !sameGitHubOwnerRepo(selectorOrigin, selectorUpstream) + // Why: each row gets a stable, unique `value` so cmdk's keyboard // navigation can track selection across sub-tab changes. Values also // feed the controlled `commandValue` state below. @@ -1058,8 +1096,49 @@ export default function CreateFromTab({ } }} placeholder={placeholderBySubTab[subTab]} - className="h-9 pl-8 text-sm" + className={cn( + 'h-9 pl-8 text-sm', + // Why: reserve trailing space for the absolute-positioned + // IssueSourceSelector so typed text never slides under + // the pills. Only widen the padding when the selector + // is actually rendered. + selectorRenderable && !launching ? 'pr-[140px]' : '' + )} /> + {selectorRenderable && selectedRepo && !launching ? ( + // Why: right-aligned inside the input so the selector + // stays visually attached to the surface whose contents + // it controls. `pointer-events-auto` is needed because + // the sibling Search/Loader icons set + // `pointer-events-none` on the absolute-positioned layer + // — we override that for the clickable pills. + // `suppressTooltip`: the selector is only visible on the + // Issues sub-tab, so "Showing issues from X" restates + // what's already implied (same reasoning as the Create + // Issue composer mirror in TaskPage). +
{ + // Why: clicking a pill would otherwise blur the input + // and close the results popover before the click + // lands on the button. Blocking the default focus + // shift keeps the popover open across a flip so the + // user sees the list repopulate against the new + // source without re-clicking the input. + e.preventDefault() + }} + > + { + void setIssueSourcePreference(selectedRepo.id, selectedRepo.path, next) + }} + suppressTooltip + /> +
+ ) : null}
= { * render together. */ error?: WorkItemsCacheError + /** + * True when the resolver fell back to origin because the user's preferred + * `'upstream'` remote is no longer configured for this repo. Consumers + * surface a one-time toast per session/repo; TaskPage tracks the + * already-toasted set so repeated refreshes don't re-toast. + * Typed as `?: true` (not `?: boolean`) to encode the invariant "present + * iff fell-back" — an explicit `false` write would be a bug. + */ + issueSourceFellBack?: true } type FetchOptions = { @@ -54,6 +70,10 @@ const CHECKS_CACHE_TTL = 60_000 // 1 minute — checks change more frequently // source of truth, so 60s staleness is fine — stale data renders instantly // while a background refresh keeps it current. const WORK_ITEMS_CACHE_TTL = 60_000 +// Why: match repos.ts so error toasts surfaced from this slice share the same +// long-lived duration — the user needs time to read + act on persist failures +// rather than having the toast vanish behind default short-lived timings. +const ERROR_TOAST_DURATION = 60_000 const inflightPRRequests = new Map< string, @@ -267,6 +287,30 @@ export type GitHubSlice = { */ prefetchWorkItems: (repoId: string, repoPath: string, limit?: number, query?: string) => void patchWorkItem: (itemId: string, patch: Partial) => void + /** + * Monotonic counter bumped whenever a repo's issue-source preference is + * flipped. Subscribers (TaskPage's fetch effect) include this in their + * dependency array to force a re-fetch after preference changes — the + * work-items cache eviction alone isn't enough because the effect keys on + * `selectedRepos`/`appliedTaskSearch`/`taskRefreshNonce` and wouldn't + * otherwise notice the cache went empty. + */ + workItemsInvalidationNonce: number + /** + * Persist a per-repo issue-source preference, update the local Repo record + * for reactive UI, and invalidate all cached work-items entries that key + * off this repo's path so the Tasks list re-fetches against the new source. + * + * Why invalidate all `${repoPath}::*` keys and not only the primary entry: + * preferences flip the issue source for every list query (query-less + + * user-entered queries alike). Surgical eviction of the primary key alone + * would leave stale results in alternate-query cache lines. + */ + setIssueSourcePreference: ( + repoId: string, + repoPath: string, + preference: IssueSourcePreference + ) => Promise } export const createGitHubSlice: StateCreator = (set, get) => ({ @@ -275,6 +319,7 @@ export const createGitHubSlice: StateCreator = (s checksCache: {}, commentsCache: {}, workItemsCache: {}, + workItemsInvalidationNonce: 0, getCachedWorkItems: (repoPath, limit, query) => { const key = workItemsCacheKey(repoPath, limit, query) @@ -366,7 +411,8 @@ export const createGitHubSlice: StateCreator = (s data: items, fetchedAt: Date.now(), sources: envelope.sources, - ...(errorForCache ? { error: errorForCache } : {}) + ...(errorForCache ? { error: errorForCache } : {}), + ...(envelope.issueSourceFellBack ? { issueSourceFellBack: true } : {}) } } })) @@ -824,6 +870,81 @@ export const createGitHubSlice: StateCreator = (s }) }, + setIssueSourcePreference: async (repoId, repoPath, preference) => { + // Why: optimistically patch the local Repo first so the segmented control + // reflects the new selection on the same frame. On IPC failure we resync + // from disk via `fetchRepos()` below so the UI doesn't lie about what's + // persisted. + set((s) => ({ + repos: s.repos.map((r) => + r.id === repoId + ? { + ...r, + issueSourcePreference: preference === 'auto' ? undefined : preference + } + : r + ) + })) + try { + // Why: persist via the generic `repos:update` channel rather than a + // dedicated gh-namespaced handler. Single write path → single + // `repos:changed` broadcast → other windows re-fetch. The store layer + // normalizes `'auto'` to `undefined` so the persisted record drops + // the key entirely (see main/persistence.ts#updateRepo). + await window.api.repos.update({ + repoId, + updates: { issueSourcePreference: preference === 'auto' ? undefined : preference } + }) + } catch (err) { + console.error('Failed to persist issue-source preference:', err) + // Why: surface the persist failure so the user understands why the + // pill visually reverts (optimistic patch above → resync via + // fetchRepos below). Without this toast, the UI silently snaps back + // and the user has no clue the write failed. + toast.error('Failed to save issue-source preference', { + duration: ERROR_TOAST_DURATION + }) + // Why: the optimistic patch above may now disagree with disk. Resync + // rather than leave a lie on screen. We only refetch repos — the cache + // eviction below is still safe to run; worst case we trigger a + // harmless re-fetch of work items against the pre-flip preference. + void get().fetchRepos() + } + // Why: wipe in-flight dedupe entries for this repo BEFORE bumping the + // invalidation nonce. The bump triggers a re-run of TaskPage's fetch + // effect; if the inflight map still held a pre-flip entry, the new + // dispatch could collapse onto it and skip the source swap. Clearing + // first makes the "new fetch gets a fresh request" invariant impossible + // to trip on later refactors that change zustand or React flush timing. + for (const key of Array.from(inflightWorkItemsRequests.keys())) { + if (key.startsWith(`${repoPath}::`)) { + inflightWorkItemsRequests.delete(key) + } + } + // Why: evict every cache entry keyed on this repo's path AFTER the IPC + // resolves. If we evicted before awaiting, an overlapping fetch triggered + // by a different subscriber would hit main with the pre-flip persisted + // preference and repopulate the cache with stale-source data. Work-items + // cache keys are `${repoPath}::${limit}::${query}` so we can't selectively + // invalidate by query — the preference change affects all queries against + // this repo. + set((s) => { + const prefix = `${repoPath}::` + const next: Record> = {} + for (const [key, entry] of Object.entries(s.workItemsCache)) { + if (!key.startsWith(prefix)) { + next[key] = entry + } + } + // Why: bump the invalidation nonce so the Tasks list's fetch effect + // — which keys on `[selectedRepos, appliedTaskSearch, taskRefreshNonce, + // taskSource, workItemsInvalidationNonce]` — re-runs and re-populates + // the just-evicted entries. Evicting alone wouldn't trigger the effect + // because it doesn't depend on the cache. + return { workItemsCache: next, workItemsInvalidationNonce: s.workItemsInvalidationNonce + 1 } + }) + }, + // Why: worktree switches previously force-refreshed GitHub data on every // click, bypassing the 5-min TTL. This variant only fetches when stale, // avoiding unnecessary API calls and latency during rapid switching. diff --git a/src/renderer/src/store/slices/repos.ts b/src/renderer/src/store/slices/repos.ts index 8ab45b17b..f203a36b6 100644 --- a/src/renderer/src/store/slices/repos.ts +++ b/src/renderer/src/store/slices/repos.ts @@ -16,7 +16,15 @@ export type RepoSlice = { updateRepo: ( repoId: string, updates: Partial< - Pick + Pick< + Repo, + | 'displayName' + | 'badgeColor' + | 'hookSettings' + | 'worktreeBaseRef' + | 'kind' + | 'issueSourcePreference' + > > ) => Promise setActiveRepo: (repoId: string | null) => void diff --git a/src/shared/types.ts b/src/shared/types.ts index b96d33000..8b81b9898 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -4,6 +4,23 @@ import type { SshTarget } from './ssh-types' // ─── Repo ──────────────────────────────────────────────────────────── export type RepoKind = 'git' | 'folder' +/** + * Per-repo user choice for where issues are fetched and filed. + * + * Why three states, not two: storage must distinguish "user explicitly chose + * upstream" from "heuristic happens to resolve to upstream right now." Collapsing + * the two would let a remote-topology change (someone removes `upstream`, or + * adds one later) silently move the effective source — the exact silent-source- + * switch class the upstream-issue-source design rejects. + * + * - `'auto'` (or undefined): honor the heuristic in `getIssueOwnerRepo` + * (upstream-if-exists, else origin). Initial state for every repo. + * - `'upstream'`: explicit upstream. Wins over heuristic and future topology + * changes. Falls back to origin if `upstream` remote vanishes, with a toast. + * - `'origin'`: explicit origin. Same precedence. + */ +export type IssueSourcePreference = 'upstream' | 'origin' | 'auto' + export type Repo = { id: string path: string @@ -16,6 +33,10 @@ export type Repo = { hookSettings?: RepoHookSettings /** SSH target ID for remote repos. null/undefined = local. */ connectionId?: string | null + /** Per-repo override for issue-source resolution. `undefined` is treated + * identically to `'auto'`; writers leave it undefined on creation so + * existing persisted records stay forward-compatible. */ + issueSourcePreference?: IssueSourcePreference } export type SetupRunPolicy = 'ask' | 'run-by-default' | 'skip-by-default' @@ -609,6 +630,7 @@ export type ClassifiedError = { type: | 'permission_denied' | 'not_found' + | 'issues_disabled' | 'validation_error' | 'rate_limited' | 'network_error' @@ -641,10 +663,24 @@ export type ListWorkItemsResult = { sources: { issues: GitHubOwnerRepo | null prs: GitHubOwnerRepo | null + /** Raw `upstream` remote resolved for this repo, independent of the + * user's preference. Present so the renderer's issue-source selector + * can always decide whether to render (upstream exists & differs from + * origin) and show both slugs in its tooltips, even when the user has + * picked 'origin' and `sources.issues` has collapsed onto origin. */ + upstreamCandidate: GitHubOwnerRepo | null } errors?: { issues?: ClassifiedError } + /** True when the user's per-repo preference was `'upstream'` but no upstream + * remote is configured, so the resolver fell back to origin. Renderer uses + * this to surface a one-time-per-session toast. Omitted when absent so + * existing consumers and test fixtures don't care about it. + * Typed as `?: true` (not `?: boolean`) to encode the invariant "present + * iff fell-back" — an explicit `false` write would be a bug, so make it a + * compile error. */ + issueSourceFellBack?: true } export type LinearWorkflowState = {