From c641e554867e70f5e4828832473d4018940764cb Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sat, 4 Jul 2026 15:16:50 -0700 Subject: [PATCH] Normalize GitHub work item type from URL to prevent issue/PR mismatch (#7371) - Resolve true work item identity (issue vs. PR) using the URL path to override stale or incorrect cached payload types. - Prevent invalid PR start point resolution when launching an issue misclassified as a PR. - Validate and reject mismatched URL types in the worktree metadata dialog fields to avoid incorrect associations. --- src/main/github/pr-start-point.test.ts | 19 +- .../components/sidebar/WorktreeMetaDialog.tsx | 13 +- .../sidebar/worktree-meta-updates.test.ts | 66 +++++++ .../sidebar/worktree-meta-updates.ts | 18 +- ...poserState-host-context-boundaries.test.ts | 16 +- src/renderer/src/hooks/useComposerState.ts | 162 +++++++++++++----- src/renderer/src/lib/github-links.test.ts | 31 +++- src/renderer/src/lib/github-links.ts | 19 +- ...github-work-item-background-create.test.ts | 63 ++++++- .../lib/github-work-item-background-create.ts | 6 +- .../github-work-item-background-request.ts | 15 +- .../src/lib/github-work-item-identity.ts | 20 +++ .../src/lib/launch-work-item-direct.test.ts | 47 ++++- .../src/lib/launch-work-item-direct.ts | 31 ++-- .../src/lib/smart-github-submit.test.ts | 22 +++ src/renderer/src/lib/smart-github-submit.ts | 15 +- 16 files changed, 464 insertions(+), 99 deletions(-) create mode 100644 src/renderer/src/components/sidebar/worktree-meta-updates.test.ts create mode 100644 src/renderer/src/lib/github-work-item-identity.ts diff --git a/src/main/github/pr-start-point.test.ts b/src/main/github/pr-start-point.test.ts index 91d8e5ad07e..8783b2a4af4 100644 --- a/src/main/github/pr-start-point.test.ts +++ b/src/main/github/pr-start-point.test.ts @@ -22,12 +22,12 @@ describe('resolveGitHubPrStartPoint', () => { getPullRequestPushTargetMock.mockResolvedValue({ pushTarget: { remoteName: 'pr-contributor-orca', - branchName: 'feat/onboarding-model-choice-782', + branchName: 'fix-issue-6933', remoteUrl: 'git@github.com:contributor/orca.git' } }) const fetchRemoteTrackingRef = vi.fn(async (_remote: string, branch: string) => { - if (branch === 'feat/onboarding-model-choice-782') { + if (branch === 'fix-issue-6933') { throw new Error('fatal: could not find remote ref') } }) @@ -40,28 +40,25 @@ describe('resolveGitHubPrStartPoint', () => { const result = await resolveGitHubPrStartPoint({ repoPath: '/repo-root', - prNumber: 1849, - headRefName: 'feat/onboarding-model-choice-782', + prNumber: 6934, + headRefName: 'fix-issue-6933', baseRefName: 'main', gitExec, fetchRemoteTrackingRef, resolveRemote: async () => 'origin' }) - expect(fetchRemoteTrackingRef).toHaveBeenCalledWith( - 'origin', - 'feat/onboarding-model-choice-782' - ) + expect(fetchRemoteTrackingRef).toHaveBeenCalledWith('origin', 'fix-issue-6933') expect(fetchRemoteTrackingRef).toHaveBeenCalledWith('origin', 'main') - expect(gitExec).toHaveBeenCalledWith(['fetch', 'origin', 'refs/pull/1849/head']) + expect(gitExec).toHaveBeenCalledWith(['fetch', 'origin', 'refs/pull/6934/head']) expect(result).toEqual({ baseBranch: 'def456', compareBaseRef: 'refs/remotes/origin/main', headSha: 'def456', - branchNameOverride: 'feat/onboarding-model-choice-782', + branchNameOverride: 'fix-issue-6933', pushTarget: { remoteName: 'pr-contributor-orca', - branchName: 'feat/onboarding-model-choice-782', + branchName: 'fix-issue-6933', remoteUrl: 'git@github.com:contributor/orca.git' } }) diff --git a/src/renderer/src/components/sidebar/WorktreeMetaDialog.tsx b/src/renderer/src/components/sidebar/WorktreeMetaDialog.tsx index 217d2fd6735..5163a839ebf 100644 --- a/src/renderer/src/components/sidebar/WorktreeMetaDialog.tsx +++ b/src/renderer/src/components/sidebar/WorktreeMetaDialog.tsx @@ -11,8 +11,11 @@ import { import { Button } from '@/components/ui/button' import { Input } from '@/components/ui/input' import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' -import { parseGitHubIssueOrPRNumber } from '@/lib/github-links' -import { buildWorktreeMetaUpdates, type WorktreeMetaSavedPayload } from './worktree-meta-updates' +import { + buildWorktreeMetaUpdates, + parseGitHubWorkItemNumberForMetaField, + type WorktreeMetaSavedPayload +} from './worktree-meta-updates' import { useWorktreeIssueLink } from './use-worktree-issue-link' import { getScreenSubmitShortcutLabel, isScreenSubmitShortcut } from '@/lib/screen-submit-shortcut' import { ExternalLink, LoaderCircle } from 'lucide-react' @@ -95,8 +98,10 @@ const WorktreeMetaDialog = React.memo(function WorktreeMetaDialog() { } const trimmedIssue = issueInput.trim() const trimmedPR = prInput.trim() - const issueValid = trimmedIssue === '' || parseGitHubIssueOrPRNumber(trimmedIssue) !== null - const prValid = trimmedPR === '' || parseGitHubIssueOrPRNumber(trimmedPR) !== null + const issueValid = + trimmedIssue === '' || parseGitHubWorkItemNumberForMetaField(trimmedIssue, 'issue') !== null + const prValid = + trimmedPR === '' || parseGitHubWorkItemNumberForMetaField(trimmedPR, 'pr') !== null return issueValid && prValid }, [worktreeId, issueInput, prInput]) diff --git a/src/renderer/src/components/sidebar/worktree-meta-updates.test.ts b/src/renderer/src/components/sidebar/worktree-meta-updates.test.ts new file mode 100644 index 00000000000..a53eeae4163 --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-meta-updates.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, it } from 'vitest' +import { buildWorktreeMetaUpdates } from './worktree-meta-updates' + +describe('buildWorktreeMetaUpdates', () => { + it('rejects issue URLs in the PR input', () => { + expect( + buildWorktreeMetaUpdates({ + displayNameInput: 'Workspace', + currentDisplayName: 'Workspace', + issueInput: '', + prInput: 'https://github.com/stablyai/orca/issues/6933', + commentInput: '' + }) + ).toEqual({ + comment: '', + linkedIssue: null + }) + }) + + it('accepts PR URLs in the PR input', () => { + expect( + buildWorktreeMetaUpdates({ + displayNameInput: 'Workspace', + currentDisplayName: 'Workspace', + issueInput: '', + prInput: 'https://github.com/stablyai/orca/pull/6934', + commentInput: '' + }) + ).toEqual({ + comment: '', + linkedIssue: null, + linkedPR: 6934 + }) + }) + + it('accepts issue URLs in the issue input', () => { + expect( + buildWorktreeMetaUpdates({ + displayNameInput: 'Workspace', + currentDisplayName: 'Workspace', + issueInput: 'https://github.com/stablyai/orca/issues/6933', + prInput: '', + commentInput: '' + }) + ).toEqual({ + comment: '', + linkedIssue: 6933, + linkedPR: null + }) + }) + + it('rejects PR URLs in the issue input', () => { + expect( + buildWorktreeMetaUpdates({ + displayNameInput: 'Workspace', + currentDisplayName: 'Workspace', + issueInput: 'https://github.com/stablyai/orca/pull/6934', + prInput: '', + commentInput: '' + }) + ).toEqual({ + comment: '', + linkedPR: null + }) + }) +}) diff --git a/src/renderer/src/components/sidebar/worktree-meta-updates.ts b/src/renderer/src/components/sidebar/worktree-meta-updates.ts index 7f6f5d70935..26769c65061 100644 --- a/src/renderer/src/components/sidebar/worktree-meta-updates.ts +++ b/src/renderer/src/components/sidebar/worktree-meta-updates.ts @@ -16,6 +16,20 @@ export function parseExplicitGitHubIssueUrl(input: string): string | null { return trimmed } +export function parseGitHubWorkItemNumberForMetaField( + input: string, + expectedType: 'issue' | 'pr' +): number | null { + const link = parseGitHubIssueOrPRLink(input) + if (link) { + // Why: issue and PR numbers live in separate GitHub namespaces for refs; + // a URL path mismatch must not silently link the other field. + return link.type === expectedType ? link.number : null + } + + return parseGitHubIssueOrPRNumber(input) +} + /** Pure save-payload builder for the worktree meta dialog: empty inputs clear * the link (null), unparseable inputs leave it untouched (omitted). */ export function buildWorktreeMetaUpdates(args: { @@ -26,11 +40,11 @@ export function buildWorktreeMetaUpdates(args: { commentInput: string }): Partial { const trimmedIssue = args.issueInput.trim() - const linkedIssueNumber = parseGitHubIssueOrPRNumber(trimmedIssue) + const linkedIssueNumber = parseGitHubWorkItemNumberForMetaField(trimmedIssue, 'issue') const finalLinkedIssue = trimmedIssue === '' ? null : linkedIssueNumber !== null ? linkedIssueNumber : undefined const trimmedPR = args.prInput.trim() - const linkedPRNumber = parseGitHubIssueOrPRNumber(trimmedPR) + const linkedPRNumber = parseGitHubWorkItemNumberForMetaField(trimmedPR, 'pr') const finalLinkedPR = trimmedPR === '' ? null : linkedPRNumber !== null ? linkedPRNumber : undefined diff --git a/src/renderer/src/hooks/useComposerState-host-context-boundaries.test.ts b/src/renderer/src/hooks/useComposerState-host-context-boundaries.test.ts index f45f7ee3c17..c43a051c8ab 100644 --- a/src/renderer/src/hooks/useComposerState-host-context-boundaries.test.ts +++ b/src/renderer/src/hooks/useComposerState-host-context-boundaries.test.ts @@ -209,6 +209,8 @@ describe('useComposerState host-context boundaries', () => { 'const applyLinkedWorkItem = useCallback' ) expect(directLookup).toContain('sourceContext: selectedRepoGitHubSourceContext') + expect(directLookup).toContain('lookupGitHubWorkItemByOwnerRepoForSource') + expect(directLookup).toContain('type: normalizedLinkQuery.directLink.type') const submitLookup = sourceBetween( HOOK_SOURCE, @@ -236,7 +238,11 @@ describe('useComposerState host-context boundaries', () => { 'const intent = getSmartGitHubSubmitIntent(name)' ) expect(selectedPrSubmitLookup).toContain('smartGitHubPrStartPointSelectionRef.current') - expect(selectedPrSubmitLookup).toContain("linkedWorkItem.type === 'pr'") + expect(selectedPrSubmitLookup).toContain("linkedWorkItemIdentity?.type === 'pr'") + expect(selectedPrSubmitLookup).toContain("startPointIdentity?.type === 'pr'") + expect(selectedPrSubmitLookup).toContain( + 'startPointIdentity.number === linkedWorkItemIdentity.number' + ) expect(selectedPrSubmitLookup).toContain('resolveGitHubPrStartPointForRepo') expect(selectedPrSubmitLookup.indexOf('resolveGitHubPrStartPointForRepo')).toBeLessThan( selectedPrSubmitLookup.indexOf("return { kind: 'none' }") @@ -421,8 +427,12 @@ describe('useComposerState host-context boundaries', () => { ) expect(projectGroupSmartHandlers).toContain('setLinkedGitLabIssue(null)') expect(projectGroupSmartHandlers).toContain('setLinkedGitLabMR(null)') - expect(projectGroupSmartHandlers).toContain("setLinkedIssue('')") - expect(projectGroupSmartHandlers).toContain('setLinkedPR(null)') + expect(projectGroupSmartHandlers).toContain( + "setLinkedIssue(identity.type === 'issue' ? String(identity.number) : '')" + ) + expect(projectGroupSmartHandlers).toContain( + "setLinkedPR(identity.type === 'pr' ? identity.number : null)" + ) }) it('disables repo-backed folder smart lookup when a folder target has no source repos', () => { diff --git a/src/renderer/src/hooks/useComposerState.ts b/src/renderer/src/hooks/useComposerState.ts index 202563742eb..dc6de85382b 100644 --- a/src/renderer/src/hooks/useComposerState.ts +++ b/src/renderer/src/hooks/useComposerState.ts @@ -95,6 +95,10 @@ import { lookupGitHubWorkItemByOwnerRepoForSource, lookupGitHubWorkItemForSource } from '@/lib/github-work-item-source-lookup' +import { + resolveGitHubWorkItemIdentity, + type GitHubWorkItemIdentity +} from '@/lib/github-work-item-identity' import { resolveGitHubPrStartPointForRepo } from '@/lib/github-pr-start-point' import { isWorkItemLookupText } from '@/lib/work-item-lookup-text' import { @@ -475,6 +479,38 @@ function getLinkedWorkItemSeedName(item: LinkedWorkItemSummary | null | undefine return getLinkedWorkItemWorkspaceName(item)?.seedName ?? getLinkedWorkItemSuggestedName(item) } +function getGitHubLinkedWorkItemIdentity( + item: LinkedWorkItemSummary | null | undefined +): GitHubWorkItemIdentity | null { + if ( + !item || + getLinkedWorkItemProvider(item) !== 'github' || + (item.type !== 'issue' && item.type !== 'pr') + ) { + return null + } + + return resolveGitHubWorkItemIdentity({ + type: item.type, + number: item.number, + url: item.url + }) +} + +function normalizeGitHubLinkedWorkItem( + item: LinkedWorkItemSummary | null | undefined +): LinkedWorkItemSummary | null { + if (!item) { + return null + } + const identity = getGitHubLinkedWorkItemIdentity(item) + if (!identity || (identity.type === item.type && identity.number === item.number)) { + return item + } + + return { ...item, type: identity.type, number: identity.number } +} + export function getInitialAutoManagedWorkspaceName({ draftName, draftLinkedWorkItem, @@ -905,10 +941,16 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS const [attachmentPaths, setAttachmentPaths] = useState( persistDraft ? (newWorkspaceDraft?.attachments ?? []) : [] ) + const initialLinkedWorkItemSeed = normalizeGitHubLinkedWorkItem(initialLinkedWorkItem) + const draftLinkedWorkItemSeed = persistDraft + ? normalizeGitHubLinkedWorkItem(newWorkspaceDraft?.linkedWorkItem) + : null + const linkedWorkItemSeed = persistDraft + ? (draftLinkedWorkItemSeed ?? initialLinkedWorkItemSeed) + : initialLinkedWorkItemSeed + const linkedWorkItemSeedIdentity = getGitHubLinkedWorkItemIdentity(linkedWorkItemSeed) const [linkedWorkItem, setLinkedWorkItem] = useState( - persistDraft - ? (newWorkspaceDraft?.linkedWorkItem ?? initialLinkedWorkItem) - : initialLinkedWorkItem + () => linkedWorkItemSeed ) const taskSourceContext = useMemo(() => { if ( @@ -982,6 +1024,9 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS }) }, [projects, selectedRepo, selectedRepoIsGit, selectedWorkspaceTarget, taskSourceContext]) const [linkedIssue, setLinkedIssue] = useState(() => { + if (linkedWorkItemSeedIdentity?.type === 'issue') { + return String(linkedWorkItemSeedIdentity.number) + } if (persistDraft && newWorkspaceDraft?.linkedIssue) { return newWorkspaceDraft.linkedIssue } @@ -994,6 +1039,12 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS return '' }) const [linkedPR, setLinkedPR] = useState(() => { + if (linkedWorkItemSeedIdentity?.type === 'pr') { + return linkedWorkItemSeedIdentity.number + } + if (linkedWorkItemSeedIdentity?.type === 'issue') { + return null + } if (persistDraft && newWorkspaceDraft?.linkedPR !== undefined) { return newWorkspaceDraft.linkedPR } @@ -1134,9 +1185,9 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS const lastAutoNameRef = useRef( getInitialAutoManagedWorkspaceName({ draftName: persistDraft ? newWorkspaceDraft?.name : null, - draftLinkedWorkItem: persistDraft ? newWorkspaceDraft?.linkedWorkItem : null, + draftLinkedWorkItem: persistDraft ? draftLinkedWorkItemSeed : null, initialName, - initialLinkedWorkItem + initialLinkedWorkItem: initialLinkedWorkItemSeed }) ) const nameRef = useRef(name) @@ -1922,16 +1973,28 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS let cancelled = false setLinkDirectLoading(true) // Why: Superset lets users paste a full GitHub URL or type a raw issue/PR - // number and still get a concrete selectable result. Orca mirrors that by - // resolving direct lookups against the selected repo instead of requiring a - // text match in the recent-items list. + // number and still get a concrete selectable result. Full URLs carry + // issue-vs-PR intent, so preserve the URL route instead of probing by + // number only. const lookupRepoId = selectedRepo.id - void lookupGitHubWorkItemForSource({ - repoPath: selectedRepo.path, - repoId: selectedRepo.id, - sourceContext: selectedRepoGitHubSourceContext, - number: normalizedLinkQuery.directNumber - }) + const lookup = + normalizedLinkQuery.directLink !== undefined + ? lookupGitHubWorkItemByOwnerRepoForSource({ + repoPath: selectedRepo.path, + repoId: selectedRepo.id, + sourceContext: selectedRepoGitHubSourceContext, + owner: normalizedLinkQuery.directLink.slug.owner, + repo: normalizedLinkQuery.directLink.slug.repo, + number: normalizedLinkQuery.directLink.number, + type: normalizedLinkQuery.directLink.type + }) + : lookupGitHubWorkItemForSource({ + repoPath: selectedRepo.path, + repoId: selectedRepo.id, + sourceContext: selectedRepoGitHubSourceContext, + number: normalizedLinkQuery.directNumber + }) + void lookup .then((item) => { if (!cancelled) { setLinkDirectItem( @@ -1954,6 +2017,7 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS cancelled = true } }, [ + normalizedLinkQuery.directLink, linkPopoverOpen, normalizedLinkQuery.directNumber, selectedRepo, @@ -1963,24 +2027,31 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS const applyLinkedWorkItem = useCallback( (item: GitHubWorkItem, options: { preserveBranchNameOverride?: boolean } = {}): void => { - if (item.type === 'issue') { - setLinkedIssue(String(item.number)) + const identity = resolveGitHubWorkItemIdentity(item) + const normalizedItem: GitHubWorkItem = { + ...item, + type: identity.type, + number: identity.number + } + if (identity.type === 'issue') { + setLinkedIssue(String(identity.number)) setLinkedPR(null) } else { setLinkedIssue('') - setLinkedPR(item.number) + setLinkedPR(identity.number) } setLinkedGitLabIssue(null) setLinkedGitLabMR(null) setLinkedWorkItem({ - type: item.type, + type: identity.type, provider: 'github', - number: item.number, + number: identity.number, title: item.title, url: item.url }) const suggestedName = - getLinkedWorkItemWorkspaceName(item)?.seedName ?? getLinkedWorkItemSuggestedName(item) + getLinkedWorkItemWorkspaceName(normalizedItem)?.seedName ?? + getLinkedWorkItemSuggestedName(normalizedItem) // Why: a pasted URL/#123 in the field is the lookup query that found // this item, not a deliberate name — replace it with the title-derived // name or it silently becomes a slugified-URL workspace name. @@ -2002,20 +2073,25 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS useCallback(async (): Promise => { if (linkedWorkItem) { const startPointSelection = smartGitHubPrStartPointSelectionRef.current + const linkedWorkItemIdentity = getGitHubLinkedWorkItemIdentity(linkedWorkItem) + const startPointIdentity = startPointSelection + ? resolveGitHubWorkItemIdentity(startPointSelection.item) + : null if ( !isProjectGroupTarget && - linkedWorkItem.type === 'pr' && + linkedWorkItemIdentity?.type === 'pr' && + startPointIdentity?.type === 'pr' && getLinkedWorkItemProvider(linkedWorkItem) === 'github' && selectedRepo && selectedRepoIsGit && startPointSelection?.repoId === selectedRepo.id && - startPointSelection.item.number === linkedWorkItem.number + startPointIdentity.number === linkedWorkItemIdentity.number ) { const selectedPrStartPoint = startPointSelection.resolved ?? (await resolveGitHubPrStartPointForRepo({ repoId: selectedRepo.id, - prNumber: startPointSelection.item.number, + prNumber: startPointIdentity.number, settings: getSettingsForRepoRuntimeOwner( { repos: [selectedRepo], settings }, selectedRepo.id @@ -2102,11 +2178,12 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS throw new Error('Could not resolve the GitHub item before creating the workspace.') } + const itemIdentity = resolveGitHubWorkItemIdentity(item) const prStartPoint = - !isProjectGroupTarget && item.type === 'pr' && selectedRepo && selectedRepoIsGit + !isProjectGroupTarget && itemIdentity.type === 'pr' && selectedRepo && selectedRepoIsGit ? await resolveGitHubPrStartPointForRepo({ repoId: selectedRepo.id, - prNumber: item.number, + prNumber: itemIdentity.number, settings: getSettingsForRepoRuntimeOwner( { repos: [selectedRepo], settings }, selectedRepo.id @@ -2757,8 +2834,9 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS // worktree's comment should surface (`orca worktree current`, sidebar). // Prefill the note if it's empty or still equal to a prior auto-fill, so // we don't overwrite anything the user has typed. - if (item.type === 'pr') { - const suggestedNote = `PR #${item.number} — ${item.title}` + const identity = resolveGitHubWorkItemIdentity(item) + if (identity.type === 'pr') { + const suggestedNote = `PR #${identity.number} — ${item.title}` const currentNote = noteRef.current if (!currentNote.trim() || currentNote === lastAutoNoteRef.current) { setNote(suggestedNote) @@ -2800,10 +2878,16 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS const handleSmartGitHubItemSelect = useCallback( (item: GitHubWorkItem): void => { + const identity = resolveGitHubWorkItemIdentity(item) + const normalizedItem: GitHubWorkItem = { + ...item, + type: identity.type, + number: identity.number + } if (isProjectGroupTarget) { - const linkedItem = toGitHubLinkedWorkItem(item) - setLinkedIssue(String(item.number)) - setLinkedPR(item.type === 'pr' ? item.number : null) + const linkedItem = toGitHubLinkedWorkItem(normalizedItem) + setLinkedIssue(identity.type === 'issue' ? String(identity.number) : '') + setLinkedPR(identity.type === 'pr' ? identity.number : null) setLinkedGitLabIssue(null) setLinkedGitLabMR(null) setLinkedWorkItem(linkedItem) @@ -2826,8 +2910,8 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS // selected run host. Resolve git refs against the run repo; keep item // metadata/source context separate for provider identity. const runRepo = selectedRepo ?? eligibleRepos.find((repo) => repo.id === item.repoId) - applyLinkedWorkItem(item) - if (item.type !== 'pr' || !runRepo) { + applyLinkedWorkItem(normalizedItem) + if (identity.type !== 'pr' || !runRepo) { setBaseBranch(undefined) setCompareBaseRef(undefined) setPushTarget(undefined) @@ -2838,7 +2922,7 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS setPushTarget(undefined) const startPointSelection: SmartGitHubPrStartPointSelection = { repoId: runRepo.id, - item + item: normalizedItem } smartGitHubPrStartPointSelectionRef.current = startPointSelection const itemRepoSettings = getSettingsForRepoRuntimeOwner( @@ -2847,12 +2931,12 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS ) const resolvePrBase = resolveGitHubPrStartPointForRepo({ repoId: runRepo.id, - prNumber: item.number, + prNumber: identity.number, settings: itemRepoSettings, - ...(item.branchName ? { headRefName: item.branchName } : {}), - ...(item.baseRefName ? { baseRefName: item.baseRefName } : {}), - ...(item.isCrossRepository !== undefined - ? { isCrossRepository: item.isCrossRepository } + ...(normalizedItem.branchName ? { headRefName: normalizedItem.branchName } : {}), + ...(normalizedItem.baseRefName ? { baseRefName: normalizedItem.baseRefName } : {}), + ...(normalizedItem.isCrossRepository !== undefined + ? { isCrossRepository: normalizedItem.isCrossRepository } : {}) }) void resolvePrBase @@ -2863,7 +2947,7 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS startPointSelection.resolved = result handleBaseBranchPrSelect( result.baseBranch, - item, + normalizedItem, result.pushTarget, result.branchNameOverride, result.compareBaseRef diff --git a/src/renderer/src/lib/github-links.test.ts b/src/renderer/src/lib/github-links.test.ts index 31c7f72aadd..64a9bd59df3 100644 --- a/src/renderer/src/lib/github-links.test.ts +++ b/src/renderer/src/lib/github-links.test.ts @@ -133,7 +133,36 @@ describe('normalizeGitHubLinkQuery', () => { it('accepts full GitHub URLs whose slug differs from the selected repo slug', () => { expect(normalizeGitHubLinkQuery('https://github.com/stablyai/orca/issues/923')).toEqual({ query: 'https://github.com/stablyai/orca/issues/923', - directNumber: 923 + directNumber: 923, + directLink: { + slug: { owner: 'stablyai', repo: 'orca' }, + number: 923, + type: 'issue' + } + }) + }) + + it('preserves PR route intent for full GitHub URLs', () => { + expect(normalizeGitHubLinkQuery('https://github.com/stablyai/orca/pull/6934')).toEqual({ + query: 'https://github.com/stablyai/orca/pull/6934', + directNumber: 6934, + directLink: { + slug: { owner: 'stablyai', repo: 'orca' }, + number: 6934, + type: 'pr' + } + }) + }) + + it('preserves route intent for URLs with uppercase schemes', () => { + expect(normalizeGitHubLinkQuery('HTTPS://github.com/stablyai/orca/pull/6934')).toEqual({ + query: 'HTTPS://github.com/stablyai/orca/pull/6934', + directNumber: 6934, + directLink: { + slug: { owner: 'stablyai', repo: 'orca' }, + number: 6934, + type: 'pr' + } }) }) diff --git a/src/renderer/src/lib/github-links.ts b/src/renderer/src/lib/github-links.ts index 0f1fc4f25a6..05ea8c812f5 100644 --- a/src/renderer/src/lib/github-links.ts +++ b/src/renderer/src/lib/github-links.ts @@ -1,15 +1,23 @@ import { isWorkItemLinkQueryTooLarge } from './work-item-link-query-bounds' const GH_ITEM_PATH_RE = /^\/([^/]+)\/([^/]+)\/(issues|pull)\/(\d+)(?:\/.*)?$/i +const HTTP_URL_PREFIX_RE = /^https?:\/\//i export type RepoSlug = { owner: string repo: string } +export type GitHubIssueOrPRLink = { + slug: RepoSlug + number: number + type: 'issue' | 'pr' +} + export type GitHubLinkQuery = { query: string directNumber: number | null + directLink?: GitHubIssueOrPRLink tooLarge?: boolean } @@ -67,11 +75,7 @@ export function parseGitHubIssueOrPRNumber(input: string): number | null { * Parses an owner/repo slug plus issue/PR number from a GitHub URL. Returns * null for anything that isn't a recognizable GitHub-shaped issue or pull URL. */ -export function parseGitHubIssueOrPRLink(input: string): { - slug: RepoSlug - number: number - type: 'issue' | 'pr' -} | null { +export function parseGitHubIssueOrPRLink(input: string): GitHubIssueOrPRLink | null { const trimmed = input.trim() if (!trimmed) { return null @@ -118,7 +122,7 @@ export function normalizeGitHubLinkQuery(raw: string): GitHubLinkQuery { } const direct = parseGitHubIssueOrPRNumber(trimmed) - if (direct !== null && !trimmed.startsWith('http')) { + if (direct !== null && !HTTP_URL_PREFIX_RE.test(trimmed)) { return { query: trimmed, directNumber: direct } } @@ -132,6 +136,7 @@ export function normalizeGitHubLinkQuery(raw: string): GitHubLinkQuery { // slug differs from the origin remote. return { query: trimmed, - directNumber: link.number + directNumber: link.number, + directLink: link } } diff --git a/src/renderer/src/lib/github-work-item-background-create.test.ts b/src/renderer/src/lib/github-work-item-background-create.test.ts index a4e6d8f0adf..ef7adfdb2b9 100644 --- a/src/renderer/src/lib/github-work-item-background-create.test.ts +++ b/src/renderer/src/lib/github-work-item-background-create.test.ts @@ -163,6 +163,46 @@ describe('createGitHubWorkItemWorkspaceInBackground', () => { ) }) + it('does not resolve a PR start point for a stale PR-typed issue URL', async () => { + const deps = makeDeps() + + await createGitHubWorkItemWorkspaceInBackground( + { + item: makeIssue({ + type: 'pr', + number: 6933, + title: 'The board columns are displayed backwards', + url: 'https://github.com/stablyai/orca/issues/6933', + branchName: 'fix-issue-6933', + baseRefName: 'main', + isCrossRepository: true + }), + repoId: 'repo-1', + telemetrySource: 'sidebar', + openModalFallback: vi.fn() + }, + deps + ) + + expect(deps.resolvePrStartPoint).not.toHaveBeenCalled() + const beginCalls = deps.beginBackgroundCreate.mock.calls as unknown as [ + WorktreeCreationRequest + ][] + expect(beginCalls[0]?.[0].linkedIssue).toBe(6933) + expect(beginCalls[0]?.[0].linkedPR).toBeUndefined() + const continueCalls = deps.continueBackgroundCreate.mock.calls as unknown as [ + string, + WorktreeCreationRequest + ][] + const request = continueCalls[0]?.[1] + expect(request?.linkedIssue).toBe(6933) + expect(request?.linkedPR).toBeUndefined() + expect(request?.baseBranch).toBeUndefined() + expect(request?.pushTarget).toBeUndefined() + expect(request?.branchNameOverride).toBeUndefined() + expect(request?.compareBaseRef).toBeUndefined() + }) + it('shows the pending workspace before async preflight resolves', async () => { let resolveSetupDecision: ((value: { kind: 'decided'; decision: 'inherit' }) => void) | null = null @@ -566,17 +606,36 @@ describe('createGitHubWorkItemWorkspaceInBackground', () => { await createGitHubWorkItemWorkspaceInBackground( { - item: makeIssue({ type: 'pr', number: 7, url: 'https://github.com/stablyai/orca/pull/7' }), + item: makeIssue({ + type: 'pr', + number: 6934, + url: 'https://github.com/stablyai/orca/pull/6934', + branchName: 'fix-issue-6933', + baseRefName: 'main', + isCrossRepository: true + }), repoId: 'repo-1', openModalFallback: vi.fn() }, deps ) + expect(deps.resolvePrStartPoint).toHaveBeenCalledWith( + 'repo-1', + 6934, + expect.anything(), + expect.objectContaining({ + type: 'pr', + number: 6934, + branchName: 'fix-issue-6933', + baseRefName: 'main', + isCrossRepository: true + }) + ) expect(deps.continueBackgroundCreate).toHaveBeenCalledWith( 'creation-1', expect.objectContaining({ - linkedPR: 7, + linkedPR: 6934, baseBranch: 'feature/from-pr', pushTarget: { remote: 'origin', branch: 'feature/from-pr' }, branchNameOverride: 'feature/from-pr', diff --git a/src/renderer/src/lib/github-work-item-background-create.ts b/src/renderer/src/lib/github-work-item-background-create.ts index 7fa94fd9d2b..260cd2ad657 100644 --- a/src/renderer/src/lib/github-work-item-background-create.ts +++ b/src/renderer/src/lib/github-work-item-background-create.ts @@ -32,6 +32,7 @@ import { import type { GitHubWorkItem, SetupDecision } from '../../../shared/types' import type { Repo } from '../../../shared/types' import type { TaskSourceContext, WorkspaceRunContext } from '../../../shared/task-source-context' +import { resolveGitHubWorkItemIdentity } from '@/lib/github-work-item-identity' export type BackgroundGitHubWorkItemCreateResult = | { kind: 'background-started' } @@ -182,6 +183,7 @@ export async function createGitHubWorkItemWorkspaceInBackground( const restoreView = deps.getActiveView() const creationId = deps.beginBackgroundCreate(initialRequest) + const itemIdentity = resolveGitHubWorkItemIdentity(args.item) try { const repoOwnerSettings = getSettingsForRepoRuntimeOwner(store, args.repoId) @@ -201,11 +203,11 @@ export async function createGitHubWorkItemWorkspaceInBackground( let pushTarget: WorktreeCreationRequest['pushTarget'] let branchNameOverride: string | undefined let compareBaseRef: string | undefined - if (args.item.type === 'pr' && args.item.number) { + if (itemIdentity.type === 'pr' && itemIdentity.number) { try { const result = await deps.resolvePrStartPoint( args.repoId, - args.item.number, + itemIdentity.number, repoOwnerSettings, args.item ) diff --git a/src/renderer/src/lib/github-work-item-background-request.ts b/src/renderer/src/lib/github-work-item-background-request.ts index f37c71ded0a..5b157525053 100644 --- a/src/renderer/src/lib/github-work-item-background-request.ts +++ b/src/renderer/src/lib/github-work-item-background-request.ts @@ -14,6 +14,7 @@ import { CLIENT_PLATFORM, getWorkspaceIntentName, getWorkspaceSeedName } from '@ import { getLocalRepoProjectExecutionRuntimeContext } from '@/lib/local-preflight-context' import { resolveSourceControlLaunchPlatform } from '@/lib/source-control-launch-platform' import { repoIsRemote } from '../../../shared/agent-launch-remote' +import { resolveGitHubWorkItemIdentity } from '@/lib/github-work-item-identity' import { resolveTuiAgentLaunchArgs, resolveTuiAgentLaunchEnv @@ -232,19 +233,20 @@ export function buildGitHubWorkItemStartupPlan(args: { } function getGitHubWorkItemName(item: GitHubWorkItem): { seedName: string; displayName?: string } { + const identity = resolveGitHubWorkItemIdentity(item) const intent = - item.number !== null + identity.number !== null ? getWorkspaceIntentName({ sourceText: item.title, - workItem: { type: item.type, number: item.number, title: item.title } + workItem: { type: identity.type, number: identity.number, title: item.title } }) : null return { seedName: getWorkspaceSeedName({ explicitName: intent?.seedName ?? '', prompt: '', - linkedIssueNumber: item.type === 'issue' ? item.number : null, - linkedPR: item.type === 'pr' ? item.number : null + linkedIssueNumber: identity.type === 'issue' ? identity.number : null, + linkedPR: identity.type === 'pr' ? identity.number : null }), ...(intent?.displayName ? { displayName: intent.displayName } : {}) } @@ -257,6 +259,7 @@ export function buildInitialGitHubWorkItemRequest( const { seedName, displayName } = getGitHubWorkItemName(args.item) const workspaceRunContext = getWorkspaceRunContextForRepo(repo, args.workspaceRunContext) const ownerHost = parseExecutionHostId(getRepoExecutionHostId(repo)) + const identity = resolveGitHubWorkItemIdentity(args.item) return { repoId: args.repoId, worktreeCreateProgressMode: ownerHost?.kind === 'local' ? 'stepped' : 'indeterminate', @@ -264,8 +267,8 @@ export function buildInitialGitHubWorkItemRequest( ...(workspaceRunContext ? { workspaceRunContext } : {}), name: seedName, ...(displayName ? { displayName } : {}), - ...(args.item.type === 'issue' && args.item.number ? { linkedIssue: args.item.number } : {}), - ...(args.item.type === 'pr' && args.item.number ? { linkedPR: args.item.number } : {}), + ...(identity.type === 'issue' && identity.number ? { linkedIssue: identity.number } : {}), + ...(identity.type === 'pr' && identity.number ? { linkedPR: identity.number } : {}), ...(args.telemetrySource ? { telemetrySource: args.telemetrySource } : {}), setupDecision: 'inherit', agent: null, diff --git a/src/renderer/src/lib/github-work-item-identity.ts b/src/renderer/src/lib/github-work-item-identity.ts new file mode 100644 index 00000000000..c9b24e316ff --- /dev/null +++ b/src/renderer/src/lib/github-work-item-identity.ts @@ -0,0 +1,20 @@ +import { parseGitHubIssueOrPRLink } from '@/lib/github-links' + +export type GitHubWorkItemIdentity = { + type: 'issue' | 'pr' + number: number +} + +export function resolveGitHubWorkItemIdentity(item: { + type: 'issue' | 'pr' + number: number + url?: string | null +}): GitHubWorkItemIdentity { + const link = item.url ? parseGitHubIssueOrPRLink(item.url) : null + if (link) { + // Why: stale cached work-item payloads can disagree with a pasted URL. The + // URL path is the user-visible intent, so it decides issue-vs-PR launches. + return { type: link.type, number: link.number } + } + return { type: item.type, number: item.number } +} diff --git a/src/renderer/src/lib/launch-work-item-direct.test.ts b/src/renderer/src/lib/launch-work-item-direct.test.ts index 98913111af8..33ae88fde57 100644 --- a/src/renderer/src/lib/launch-work-item-direct.test.ts +++ b/src/renderer/src/lib/launch-work-item-direct.test.ts @@ -229,9 +229,9 @@ describe('launchWorkItemDirect', () => { openModalFallback: vi.fn(), item: { type: 'pr', - number: 42, + number: 6934, title: 'Fix the bug', - url: 'https://github.com/acme/repo/pull/42', + url: 'https://github.com/stablyai/orca/pull/6934', branchName: 'feature/fix', baseRefName: 'main', isCrossRepository: true @@ -240,21 +240,21 @@ describe('launchWorkItemDirect', () => { expect(mocks.resolvePrBase).toHaveBeenCalledWith({ repoId: 'repo-1', - prNumber: 42, + prNumber: 6934, headRefName: 'feature/fix', baseRefName: 'main', isCrossRepository: true }) expect(mocks.createWorktree).toHaveBeenCalledWith( 'repo-1', - 'review-pr-42', + 'review-pr-6934', 'abc123', 'inherit', undefined, 'sidebar', - 'Review PR 42', + 'Review PR 6934', undefined, - 42, + 6934, { remoteName: 'origin', branchName: 'feature/fix' }, undefined, undefined, @@ -274,6 +274,41 @@ describe('launchWorkItemDirect', () => { ) }) + it('treats a PR-typed GitHub issue URL as an issue without resolving a PR head', async () => { + const { launchWorkItemDirect } = await import('./launch-work-item-direct') + const openModalFallback = vi.fn() + + await expect( + launchWorkItemDirect({ + repoId: 'repo-1', + launchSource: 'task_page', + telemetrySource: 'sidebar', + openModalFallback, + item: { + type: 'pr', + number: 6933, + title: 'The board columns are displayed backwards', + url: 'https://github.com/stablyai/orca/issues/6933', + branchName: 'fix-issue-6933', + baseRefName: 'main', + isCrossRepository: true + } + }) + ).resolves.toBe(true) + + expect(mocks.resolvePrBase).not.toHaveBeenCalled() + expect(openModalFallback).not.toHaveBeenCalled() + const createArgs = mocks.createWorktree.mock.calls[0] + expect(createArgs?.[1]).toBe('issue-6933') + expect(createArgs?.[2]).toBeUndefined() + expect(createArgs?.[6]).toBe('Issue 6933') + expect(createArgs?.[7]).toBe(6933) + expect(createArgs?.[8]).toBeUndefined() + expect(createArgs?.[9]).toBeUndefined() + expect(createArgs?.[12]).toBeUndefined() + expect(createArgs?.[24]).toBeUndefined() + }) + it('uses the Linear identifier in direct-launch workspace names', async () => { const { launchWorkItemDirect } = await import('./launch-work-item-direct') diff --git a/src/renderer/src/lib/launch-work-item-direct.ts b/src/renderer/src/lib/launch-work-item-direct.ts index def1c5c6c8a..61f49274edb 100644 --- a/src/renderer/src/lib/launch-work-item-direct.ts +++ b/src/renderer/src/lib/launch-work-item-direct.ts @@ -16,6 +16,7 @@ import { ensureHooksConfirmed } from '@/lib/ensure-hooks-confirmed' import { getConnectionId } from '@/lib/connection-context' import type { GitPushTarget, SetupDecision, TuiAgent } from '../../../shared/types' import { getLinearIssueWorkspaceName } from '../../../shared/workspace-name' +import { resolveGitHubWorkItemIdentity } from '@/lib/github-work-item-identity' import { buildDirectWorkItemAgentStartupPlan, buildDirectWorkItemStartupOpts, @@ -70,6 +71,16 @@ export async function launchWorkItemDirect(args: LaunchWorkItemDirectArgs): Prom const repoOwnerSettings = getSettingsForRepoRuntimeOwner(store, repoId) const promptDelivery = args.promptDelivery ?? 'draft' const repoConnectionId = repo.connectionId?.trim() || null + const githubIdentity = + item.number !== null && (item.type === 'issue' || item.type === 'pr') + ? resolveGitHubWorkItemIdentity({ + type: item.type, + number: item.number, + url: item.url + }) + : null + const itemType = githubIdentity?.type ?? item.type + const itemNumber = githubIdentity?.number ?? item.number const repoProjectRuntime = repoConnectionId ? undefined : getLocalRepoProjectExecutionRuntimeContext(store, repoId, CLIENT_PLATFORM) @@ -107,10 +118,10 @@ export async function launchWorkItemDirect(args: LaunchWorkItemDirectArgs): Prom trustDecision === 'skip' ? 'skip' : setupResolution.decision const workspaceIntentName = - item.number !== null + itemNumber !== null ? getWorkspaceIntentName({ sourceText: item.pasteContent, - workItem: { ...item, number: item.number } + workItem: { ...item, type: itemType, number: itemNumber } }) : null const workspaceName = getWorkspaceSeedName({ @@ -118,18 +129,18 @@ export async function launchWorkItemDirect(args: LaunchWorkItemDirectArgs): Prom ? getLinearIssueWorkspaceName({ identifier: item.linearIdentifier, title: item.title }) : (workspaceIntentName?.seedName ?? ''), prompt: '', - linkedIssueNumber: item.type === 'issue' ? (item.number ?? null) : null, - linkedPR: item.type === 'pr' ? (item.number ?? null) : null + linkedIssueNumber: itemType === 'issue' ? (itemNumber ?? null) : null, + linkedPR: itemType === 'pr' ? (itemNumber ?? null) : null }) let resolvedBaseBranch = baseBranch let resolvedPushTarget: GitPushTarget | undefined let resolvedBranchNameOverride: string | undefined let resolvedCompareBaseRef: string | undefined - if (!resolvedBaseBranch && item.type === 'pr' && item.number) { + if (!resolvedBaseBranch && itemType === 'pr' && itemNumber) { try { // Why: direct "Use PR" launches bypass the Start-from picker, so they // must still resolve the PR head before `git worktree add`. - const result = await resolveDirectPrStartPoint(repoId, item.number, repoOwnerSettings, item) + const result = await resolveDirectPrStartPoint(repoId, itemNumber, repoOwnerSettings, item) resolvedBaseBranch = result.baseBranch resolvedPushTarget = result.pushTarget resolvedBranchNameOverride = result.branchNameOverride @@ -157,15 +168,15 @@ export async function launchWorkItemDirect(args: LaunchWorkItemDirectArgs): Prom undefined, telemetrySource, workspaceIntentName?.displayName ?? item.title, - item.type === 'issue' && item.number ? item.number : undefined, - item.type === 'pr' && item.number ? item.number : undefined, + itemType === 'issue' && itemNumber ? itemNumber : undefined, + itemType === 'pr' && itemNumber ? itemNumber : undefined, resolvedPushTarget, undefined, item.linearIdentifier, resolvedBranchNameOverride, undefined, - item.type === 'mr' && item.number ? item.number : undefined, - gitLabIssueNumber(item), + itemType === 'mr' && itemNumber ? itemNumber : undefined, + gitLabIssueNumber({ ...item, type: itemType, number: itemNumber }), undefined, undefined, undefined, diff --git a/src/renderer/src/lib/smart-github-submit.test.ts b/src/renderer/src/lib/smart-github-submit.test.ts index 43ec3c36325..64e58824e0c 100644 --- a/src/renderer/src/lib/smart-github-submit.test.ts +++ b/src/renderer/src/lib/smart-github-submit.test.ts @@ -299,4 +299,26 @@ describe('getSmartGitHubSubmitResolution', () => { expect(resolution.linkedIssueNumber).toBe(2050) expect(resolution.linkedPR).toBeNull() }) + + it('uses the URL path to normalize stale PR-typed issue results', () => { + expect( + getSmartGitHubSubmitResolution({ + type: 'pr', + number: 6933, + title: 'The board columns are displayed backwards', + url: 'https://github.com/stablyai/orca/issues/6933' + }) + ).toEqual({ + workspaceName: 'the-board-columns-are-displayed-backwards', + displayName: 'The board columns are displayed backwards', + linkedWorkItem: { + type: 'issue', + number: 6933, + title: 'The board columns are displayed backwards', + url: 'https://github.com/stablyai/orca/issues/6933' + }, + linkedIssueNumber: 6933, + linkedPR: null + }) + }) }) diff --git a/src/renderer/src/lib/smart-github-submit.ts b/src/renderer/src/lib/smart-github-submit.ts index c6ee5abff47..1b4e818f26a 100644 --- a/src/renderer/src/lib/smart-github-submit.ts +++ b/src/renderer/src/lib/smart-github-submit.ts @@ -4,6 +4,7 @@ import { getTaskSourceCacheScope } from '../../../shared/task-source-context' import { getLinkedWorkItemWorkspaceName } from '../../../shared/workspace-name' import type { LinkedWorkItemSummary } from './new-workspace' import { parseGitHubIssueOrPRLink } from './github-links' +import { resolveGitHubWorkItemIdentity } from '@/lib/github-work-item-identity' export type SmartGitHubSubmitIntent = | { @@ -196,12 +197,14 @@ export function getSmartGitHubSubmitLookupCacheSizeForTests(): number { export function getSmartGitHubSubmitResolution( item: Pick ): SmartGitHubSubmitResolution { - const fallbackName = `${item.type}-${item.number}` - const titleName = getLinkedWorkItemWorkspaceName(item) + const identity = resolveGitHubWorkItemIdentity(item) + const normalizedItem = { ...item, type: identity.type, number: identity.number } + const fallbackName = `${identity.type}-${identity.number}` + const titleName = getLinkedWorkItemWorkspaceName(normalizedItem) const workspaceName = titleName?.seedName || fallbackName const linkedWorkItem: LinkedWorkItemSummary = { - type: item.type, - number: item.number, + type: identity.type, + number: identity.number, title: item.title, url: item.url } @@ -210,7 +213,7 @@ export function getSmartGitHubSubmitResolution( workspaceName, displayName: titleName?.displayName ?? fallbackName, linkedWorkItem, - linkedIssueNumber: item.type === 'issue' ? item.number : null, - linkedPR: item.type === 'pr' ? item.number : null + linkedIssueNumber: identity.type === 'issue' ? identity.number : null, + linkedPR: identity.type === 'pr' ? identity.number : null } }