diff --git a/packages/sdk/src/server/lib/pull-requests/__tests__/source-control-pull-requests.test.ts b/packages/sdk/src/server/lib/pull-requests/__tests__/source-control-pull-requests.test.ts index a2ae1ea3c..27c0079d9 100644 --- a/packages/sdk/src/server/lib/pull-requests/__tests__/source-control-pull-requests.test.ts +++ b/packages/sdk/src/server/lib/pull-requests/__tests__/source-control-pull-requests.test.ts @@ -1578,6 +1578,75 @@ describe('optional targetBranch', () => { ); }); + it('does not duplicate entity-prefixed harness attribution when the canonical line uses a zero-width space', async () => { + const octokit = makeOctokit({ + list: [], + created: { + number: 13, + node_id: 'node-13', + html_url: 'https://github.com/acme/web/pull/13', + title: '[Feature] X', + draft: true, + base: { ref: 'develop' }, + }, + }); + const harnessAttribution = + '> ​Created by Roomote. [View the task](https://example.com/task/1) or mention @roomote-roomote for follow-up asks.'; + const canonicalAttribution = + '> \u200BCreated by Roomote. Follow up by mentioning @roomote-roomote or in [the web UI](https://example.com/task/1).'; + mockGetPrBodyAttributionLine.mockReturnValueOnce(canonicalAttribution); + + await createOrUpdateSourceControlPullRequestForTaskRun({ + taskRun: makeTaskRun({ repo: 'acme/web' }), + input: { + ...baseInput, + targetBranch: 'develop', + body: `${harnessAttribution}\n\n## What changed\n\nDone.`, + }, + }); + + expect(octokit.rest.pulls.create).toHaveBeenCalledWith( + expect.objectContaining({ + body: `${canonicalAttribution}\n\n## What changed\n\nDone.`, + }), + ); + }); + + it('collapses the exact duplicated PR attribution shape to one canonical line', async () => { + const harnessAttribution = + '> ​Created by Roomote. [View the task](https://example.com/task/1) or mention @roomote-roomote for follow-up asks.'; + const staleCanonicalAttribution = + '> \u200BCreated by Roomote. Follow up by mentioning @roomote-roomote or in [the web UI](https://example.com/task/1).'; + const duplicatedBody = `${harnessAttribution}\n\n${staleCanonicalAttribution}\n\n## What changed\n\nDone.`; + const existing = { + number: 13, + node_id: 'node-13', + html_url: 'https://github.com/acme/web/pull/13', + title: '[Feature] X', + draft: true, + base: { ref: 'develop' }, + body: duplicatedBody, + }; + const octokit = makeOctokit({ list: [existing], updated: existing }); + const canonicalAttribution = attributionBody('Created by Roomote.'); + mockGetPrBodyAttributionLine.mockReturnValueOnce(canonicalAttribution); + + await createOrUpdateSourceControlPullRequestForTaskRun({ + taskRun: makeTaskRun({ repo: 'acme/web' }), + input: { + ...baseInput, + targetBranch: 'develop', + body: duplicatedBody, + }, + }); + + expect(octokit.rest.pulls.update).toHaveBeenCalledWith( + expect.objectContaining({ + body: `${canonicalAttribution}\n\n## What changed\n\nDone.`, + }), + ); + }); + it('preserves the opener attribution when updating a private pull request', async () => { const existing = { number: 11, diff --git a/packages/sdk/src/server/lib/pull-requests/source-control-pull-requests.ts b/packages/sdk/src/server/lib/pull-requests/source-control-pull-requests.ts index 325f8aedb..b98d5baad 100644 --- a/packages/sdk/src/server/lib/pull-requests/source-control-pull-requests.ts +++ b/packages/sdk/src/server/lib/pull-requests/source-control-pull-requests.ts @@ -25,7 +25,7 @@ import { import { buildPullRequestUrl, getSourceControlProviderLabel, - findPrBodyAttributionLine, + findPrBodyAttributionMarkers, preservePrBodyAttribution, getCommunicationProviderFromTaskPayload, getCommunicationGuildIdFromTaskPayload, @@ -666,22 +666,31 @@ function buildPrAttributionTaskUrl(taskRun: TaskRun): string { } function prependCanonicalPrAttribution(body: string, line: string): string { - const firstLineEnd = body.indexOf('\n'); - const firstLine = body.slice( + let remainingBody = body.trimStart(); + + while (remainingBody) { + const markers = findPrBodyAttributionMarkers(remainingBody); + if (!markers || remainingBody.slice(0, markers.lineStart).trim()) { + break; + } + + remainingBody = remainingBody.slice(markers.lineEnd).trimStart(); + } + + const firstLineEnd = remainingBody.indexOf('\n'); + const firstLine = remainingBody.slice( 0, - firstLineEnd === -1 ? body.length : firstLineEnd, + firstLineEnd === -1 ? remainingBody.length : firstLineEnd, ); - const normalizedFirstLine = firstLine.trimStart(); - const hasLeadingAttribution = - findPrBodyAttributionLine(firstLine) !== null || + if ( /^> (?:Opened on behalf of .+\.|Created by Roomote\.) (?:Follow up by mentioning @|\[View the task\]\().+$/u.test( - normalizedFirstLine, - ); - const remainingBody = hasLeadingAttribution - ? body - .slice(firstLineEnd === -1 ? body.length : firstLineEnd + 1) - .trimStart() - : body.trimStart(); + firstLine.trimStart(), + ) + ) { + remainingBody = remainingBody + .slice(firstLineEnd === -1 ? remainingBody.length : firstLineEnd + 1) + .trimStart(); + } return remainingBody ? `${line}\n\n${remainingBody}` : line; } diff --git a/packages/types/src/__tests__/github-bot-identity.test.ts b/packages/types/src/__tests__/github-bot-identity.test.ts index 0475d8c26..ba8b6ae7a 100644 --- a/packages/types/src/__tests__/github-bot-identity.test.ts +++ b/packages/types/src/__tests__/github-bot-identity.test.ts @@ -1,4 +1,5 @@ import { + findPrBodyAttributionLine, formatPrBodyAttribution, matchesRoomoteGitHubLogin, normalizePrBodyAttributionAppMention, @@ -17,6 +18,15 @@ describe('Roomote GitHub bot identity helpers', () => { '> ​Opened on behalf of @octocat. Follow up in [the web UI](https://example.com/task/1).', ); }); + + it.each(['​', '​', '\u200B'])( + 'finds marker-wrapped attribution with the %s prefix', + (prefix) => { + const body = `\n\n> ${prefix}Created by Roomote. Follow up by mentioning @roomote.`; + + expect(findPrBodyAttributionLine(body)).toBe('> Created by Roomote.'); + }, + ); }); describe('matchesRoomoteGitHubLogin', () => { diff --git a/packages/types/src/constants.ts b/packages/types/src/constants.ts index 6848e6232..53cadafb7 100644 --- a/packages/types/src/constants.ts +++ b/packages/types/src/constants.ts @@ -91,14 +91,14 @@ export const PR_BODY_ATTRIBUTION_END_MARKER = ''; const PR_BODY_ATTRIBUTION_INLINE_PREFIX = '​'; -type PrBodyAttributionMarkerMatch = { +export type PrBodyAttributionMarkerMatch = { start: number; end: number; lineStart: number; lineEnd: number; }; -function findPrBodyAttributionMarkers( +export function findPrBodyAttributionMarkers( body: string, ): PrBodyAttributionMarkerMatch | null { const startMarker = body.indexOf(PR_BODY_ATTRIBUTION_START_MARKER); @@ -114,7 +114,9 @@ function findPrBodyAttributionMarkers( const lineStart = body.lastIndexOf('\n', startMarker - 1) + 1; if ( - !/^[ \t]*>[ \t]*(?:​)?$/u.test(body.slice(lineStart, startMarker)) + !/^[ \t]*>[ \t]*(?:(?:&(?:amp;)?#8203;)|\u200B)?$/u.test( + body.slice(lineStart, startMarker), + ) ) { return null; }