diff --git a/README.md b/README.md index e36e0a9..197ddb3 100644 --- a/README.md +++ b/README.md @@ -94,6 +94,8 @@ If a Paperclip issue was created locally or by an agent workflow before GitHub S Manual GitHub issue links are added to the same import registry and issue-link entity used by normal sync, so future syncs update the Paperclip issue from the GitHub issue. Manual pull request links are added to the PR-link entity used by the project Pull Requests page, so future syncs monitor PR status even when there is no closing GitHub issue. +When a Paperclip issue is linked to a GitHub issue and also has older direct pull request links, the GitHub issue remains the status source of truth. Direct pull request links only drive status for PR-only Paperclip issues, which prevents stale merged PR metadata from closing work while the GitHub issue and its current linked PR are still open. + Operators can unlink a linked Paperclip issue from the GitHub detail surface when they intentionally want GitHub Sync to stop updating it. Agent-facing tools and native agent API routes can create durable issue and pull request links, but they do not expose an unlink operation; internal sync repair may still tombstone a link when GitHub transfers an issue to an unmapped repository. ### Agent workflows built in diff --git a/SPEC.md b/SPEC.md index 43222e7..3eed670 100644 --- a/SPEC.md +++ b/SPEC.md @@ -105,6 +105,7 @@ The plugin MUST persist repository mappings, company-scoped advanced issue defau - Repeated sync runs MUST continue reconciling imported Paperclip issue statuses against the latest GitHub state. - Repeated sync runs MUST also monitor pull requests that were linked to Paperclip issues through the project Pull Requests page, the `create_pull_request` agent tool, the `link_github_item` agent tool, the agent-authenticated metric API route, the agent-authenticated `/issue-link` route, or the manual issue-page fallback when those pull requests do not close a synced GitHub issue, and MUST reconcile those Paperclip issue statuses from the pull request CI, merge, review decision, review-thread state, and terminal merge/closed state. - For Paperclip issues linked directly to pull requests, open linked PRs MUST continue to drive the active/review status from CI, mergeability, review decision, and review-thread state. When all directly linked PRs are terminal, any merged linked PR MUST map the Paperclip issue to `done`; only closed-unmerged linked PRs MUST map it to `cancelled`. +- Direct pull-request links MUST act as fallback status drivers only for Paperclip issues that are not also linked to a GitHub issue. When the same Paperclip issue is linked to a GitHub issue, the live GitHub issue snapshot and its closing pull-request references MUST own the status decision so stale direct pull-request links cannot terminalize still-open GitHub issue work. - Repeated sync runs MUST continue reconciling Paperclip issues linked directly to third-party GitHub issues from the live GitHub issue state, labels, description, linked pull requests, and trusted comment rules without requiring that GitHub repository to be mapped to a Paperclip project. - When the local Paperclip host API is available, sync-driven Paperclip status transitions SHOULD go through the same issue-update path Paperclip UI uses so timeline activity is recorded for agents and humans. - Repeated sync runs MUST continue reconciling imported Paperclip issue labels against the latest mapped GitHub labels, including removing labels that were removed on GitHub. diff --git a/src/worker.ts b/src/worker.ts index 15c6437..1274491 100644 --- a/src/worker.ts +++ b/src/worker.ts @@ -10681,6 +10681,35 @@ async function listGitHubPullRequestIssueLinksForMapping( return [...recordsByKey.values()]; } +async function listGitHubIssueLinkedPaperclipIssueIdsForMapping( + ctx: PluginSetupContext, + mapping: RepositoryMapping, + importedIssueRecords: ImportedIssueRecord[], + target?: ResolvedSyncTarget +): Promise> { + const issueIds = new Set( + importedIssueRecords.map((record) => record.paperclipIssueId).filter(Boolean) + ); + + if (target?.kind === 'issue' && target.issueId && target.githubIssueNumber) { + issueIds.add(target.issueId); + } + + const issueLinks = await listGitHubIssueLinkRecords(ctx, { + ...(target?.kind === 'issue' && target.issueId ? { paperclipIssueId: target.issueId } : {}) + }); + for (const record of issueLinks) { + if ( + doesGitHubIssueLinkRecordMatchMapping(record, mapping) + && doesGitHubIssueLinkRecordMatchTarget(record, target) + ) { + issueIds.add(record.paperclipIssueId); + } + } + + return issueIds; +} + function doesGitHubIssueLinkRecordMatchMapping( record: GitHubIssueLinkRecord, mapping: RepositoryMapping @@ -10744,6 +10773,7 @@ async function listExternalGitHubLinkSyncWork( const syncableMappings = getSyncableMappingsForTarget(mappings, target); const issueLinksByKey = new Map(); const pullRequestLinksByKey = new Map(); + const issueLinkedPaperclipIssueIds = new Set(); const [issueLinks, pullRequestLinks] = await Promise.all([ listGitHubIssueLinkRecords(ctx, { ...(target?.kind === 'issue' && target.issueId ? { paperclipIssueId: target.issueId } : {}) @@ -10754,10 +10784,12 @@ async function listExternalGitHubLinkSyncWork( ]); for (const record of issueLinks) { - if ( - !doesGitHubIssueLinkRecordMatchTarget(record, target) - || isGitHubIssueLinkCoveredByMappings(record, syncableMappings) - ) { + if (!doesGitHubIssueLinkRecordMatchTarget(record, target)) { + continue; + } + + issueLinkedPaperclipIssueIds.add(record.paperclipIssueId); + if (isGitHubIssueLinkCoveredByMappings(record, syncableMappings)) { continue; } @@ -10770,6 +10802,7 @@ async function listExternalGitHubLinkSyncWork( for (const record of pullRequestLinks) { if ( !doesGitHubPullRequestLinkRecordMatchTarget(record, target) + || issueLinkedPaperclipIssueIds.has(record.paperclipIssueId) || isGitHubPullRequestLinkCoveredByMappings(record, syncableMappings) ) { continue; @@ -19975,7 +20008,14 @@ async function performSync( const importRegistryByIssueId = new Map( importedIssueRecords.map((entry) => [entry.githubIssueId, entry]) ); - const pullRequestLinks = await listGitHubPullRequestIssueLinksForMapping(ctx, mapping, options.target); + const githubIssueLinkedPaperclipIssueIds = await listGitHubIssueLinkedPaperclipIssueIdsForMapping( + ctx, + mapping, + importedIssueRecords, + options.target + ); + const pullRequestLinks = (await listGitHubPullRequestIssueLinksForMapping(ctx, mapping, options.target)) + .filter((record) => !githubIssueLinkedPaperclipIssueIds.has(record.paperclipIssueId)); const ensuredPaperclipIssueIds = new Map(); const trackedIssueIds = new Set([ ...issues.map((issue) => issue.id), diff --git a/tests/plugin.spec.ts b/tests/plugin.spec.ts index d9ac674..879a325 100644 --- a/tests/plugin.spec.ts +++ b/tests/plugin.spec.ts @@ -7945,6 +7945,188 @@ test('sync.runNow lets open direct PRs drive status when another linked PR is al } }); +test('sync.runNow does not let stale direct PR links close open GitHub issue-linked work', async () => { + const harness = await createProjectPullRequestsHarness(); + const originalFetch = globalThis.fetch; + const originalCreateComment = harness.ctx.issues.createComment; + const statusTransitionComments: Array<{ issueId: string; body: string }> = []; + const issue = await harness.ctx.issues.create({ + companyId: 'company-1', + projectId: 'project-1', + title: 'Open issue with stale merged PR link', + description: 'The GitHub issue still has an open linked PR.' + }); + + harness.ctx.issues.createComment = async (issueId, body, companyId) => { + statusTransitionComments.push({ issueId, body }); + return originalCreateComment(issueId, body, companyId); + }; + + globalThis.fetch = async (input, init) => { + const requestUrl = getRequestUrl(input); + const requestPathname = getDecodedRequestPathname(input); + + if (requestPathname === '/repos/paperclipai/example-repo/issues/88') { + return jsonResponse({ + id: 8800, + number: 88, + title: 'Open GitHub issue with linked PR', + body: 'GitHub still owns the delivery state.', + html_url: 'https://github.com/paperclipai/example-repo/issues/88', + state: 'open', + state_reason: null, + comments: 0, + user: { + login: 'octocat', + html_url: 'https://github.com/octocat', + avatar_url: 'https://avatars.githubusercontent.com/u/583231?v=4' + }, + labels: [] + }); + } + + if (requestPathname === '/repos/third-party/external/pulls/89') { + return jsonResponse({ + number: 89, + title: 'Stale merged direct PR link', + body: 'This older PR was linked directly but is not the issue-closing PR.', + html_url: 'https://github.com/third-party/external/pull/89', + state: 'closed', + merged: true + }); + } + + if (requestPathname === '/graphql') { + const { query, variables } = getGraphqlRequest(init); + const pullRequestNumber = + typeof variables.pullRequestNumber === 'number' ? variables.pullRequestNumber : undefined; + + if (query.includes('query GitHubIssueStatusSnapshot') && variables.issueNumber === 88) { + return graphqlResponse({ + repository: { + issue: { + number: 88, + state: 'OPEN', + stateReason: null, + comments: { + totalCount: 0 + }, + closedByPullRequestsReferences: { + pageInfo: { + hasNextPage: false, + endCursor: null + }, + nodes: [ + { + number: 90, + state: 'OPEN', + repository: { + owner: { + login: 'paperclipai' + }, + name: 'example-repo' + } + } + ] + } + } + } + }); + } + + if (query.includes('query GitHubRepositoryOpenPullRequestStatuses')) { + return graphqlResponse({ + repository: { + pullRequests: { + pageInfo: { + hasNextPage: false, + endCursor: null + }, + nodes: [] + } + } + }); + } + + if (query.includes('query GitHubPullRequestReviewThreads') && pullRequestNumber === 90) { + return graphqlResponse({ + repository: { + pullRequest: { + reviewThreads: { + pageInfo: { + hasNextPage: false, + endCursor: null + }, + nodes: [] + } + } + } + }); + } + + if (query.includes('query GitHubPullRequestCiContexts') && pullRequestNumber === 90) { + return graphqlResponse({ + repository: { + pullRequest: { + mergeable: 'MERGEABLE', + mergeStateStatus: 'CLEAN', + reviewDecision: null, + statusCheckRollup: { + contexts: { + pageInfo: { + hasNextPage: false, + endCursor: null + }, + nodes: [ + { + __typename: 'StatusContext', + state: 'SUCCESS' + } + ] + } + } + } + } + }); + } + } + + throw new Error(`Unexpected fetch during stale direct PR edge-case sync test: ${requestUrl}`); + }; + + try { + await harness.performAction('issue.linkGitHubItem', { + companyId: 'company-1', + issueId: issue.id, + kind: 'issue', + reference: '88' + }); + await upsertDirectPullRequestLink(harness, issue.id, 89, { + repositoryUrl: 'https://github.com/third-party/external', + title: 'Stale merged direct PR link' + }); + await harness.ctx.issues.update(issue.id, { status: 'done' }, 'company-1'); + + const sync = await harness.performAction('sync.runNow', { + companyId: 'company-1', + issueId: issue.id, + waitForCompletion: true + }) as { + syncState: { status: string; syncedIssuesCount?: number }; + }; + + assert.equal(sync.syncState.status, 'success'); + const updatedIssue = await harness.ctx.issues.get(issue.id, 'company-1'); + assert.equal(updatedIssue?.status, 'in_review'); + assert.equal(statusTransitionComments.length, 1); + assert.match(statusTransitionComments[0]?.body ?? '', /from `done` to `in review`/); + assert.doesNotMatch(statusTransitionComments[0]?.body ?? '', /pull request was merged/); + } finally { + harness.ctx.issues.createComment = originalCreateComment; + globalThis.fetch = originalFetch; + } +}); + test('sync.runNow completes all-terminal direct PR groups when any linked PR merged', async () => { const harness = await createProjectPullRequestsHarness(); const originalFetch = globalThis.fetch;