From dcb225bd1e55319aa8b8def77351b2d26d2685b8 Mon Sep 17 00:00:00 2001 From: ayushtr-aws Date: Thu, 17 Sep 2026 12:41:20 -0400 Subject: [PATCH 1/3] fix(screenshots): preserve Amplify preview PR routing (#900) Keep branch environment filters for deployment statuses, log Amplify rejection reasons, and carry validated PR identity through screenshot and Jira/Linear delivery. Fixes #900 Co-Authored-By: Codex --- .../github-screenshot-integration.ts | 1 + cdk/src/handlers/github-webhook-processor.ts | 60 ++++++++-- cdk/src/handlers/github-webhook.ts | 29 +++-- .../shared/github-deployment-status.ts | 81 +++++++++---- .../handlers/github-webhook-processor.test.ts | 106 ++++++++++++++++++ cdk/test/handlers/github-webhook.test.ts | 97 +++++++++++----- .../DEPLOY_PREVIEW_SCREENSHOTS_GUIDE.md | 38 +++++-- docs/guides/JIRA_SETUP_GUIDE.md | 2 +- .../using/Deploy-preview-screenshots-guide.md | 38 +++++-- .../content/docs/using/Jira-setup-guide.md | 2 +- 10 files changed, 358 insertions(+), 96 deletions(-) diff --git a/cdk/src/constructs/github-screenshot-integration.ts b/cdk/src/constructs/github-screenshot-integration.ts index a64cbb412..b86cf14c5 100644 --- a/cdk/src/constructs/github-screenshot-integration.ts +++ b/cdk/src/constructs/github-screenshot-integration.ts @@ -102,6 +102,7 @@ export interface GitHubScreenshotIntegrationProps { * (Amplify Hosting), `Deploy Preview ` (Netlify), or whatever * your GitHub Actions workflow passes. Set this when your provider * uses a different name and you want per-PR-only screenshots. + * Validated Amplify PR preview check runs bypass this filter. * @default 'Preview' */ readonly screenshotTargetEnvironment?: string; diff --git a/cdk/src/handlers/github-webhook-processor.ts b/cdk/src/handlers/github-webhook-processor.ts index 5ae7f61bf..14ed2af86 100644 --- a/cdk/src/handlers/github-webhook-processor.ts +++ b/cdk/src/handlers/github-webhook-processor.ts @@ -101,15 +101,17 @@ const PR_LOOKUP_RETRY_DELAYS_MS = [ interface ProcessorEvent { readonly raw_body: string; + /** Set only by the receiver after Amplify check URL/PR/SHA validation. */ + readonly validated_pr_number?: number; } /** - * Async processor for verified GitHub `deployment_status` webhooks. + * Async processor for verified deployment statuses and normalized Amplify checks. * * Flow: * 1. Parse the payload (already validated as deployment_status by the * receiver, but we re-extract the fields we need). - * 2. Find the open PR for the deploy SHA via the GitHub Commits API. + * 2. Fetch the validated Amplify PR, or find the deployment SHA's open PR. * 3. Capture a screenshot of `deployment.environment_url` via * AgentCore Browser. * 4. PUT the PNG to the screenshot bucket. @@ -143,6 +145,15 @@ export async function handler(event: ProcessorEvent): Promise { return; } + const validatedPrNumber = event.validated_pr_number; + if (validatedPrNumber !== undefined + && (!Number.isSafeInteger(validatedPrNumber) || validatedPrNumber <= 0)) { + logger.warn('Processor received invalid validated PR number', { + event: 'screenshot.amplify_pr_rejected', reason: 'invalid_pr_number', + }); + return; + } + const payload = validateDeploymentStatusPayload(raw); if (!payload) { // The receiver runs the same validation, so this branch should be @@ -192,7 +203,7 @@ export async function handler(event: ProcessorEvent): Promise { // Retry the PR lookup, but cap by remaining budget so the screenshot // half always gets at least MIN_CAPTURE_BUDGET_MS. const prLookupBudget = Math.max(0, remaining() - POST_CAPTURE_RESERVE_MS - MIN_CAPTURE_BUDGET_MS); - const pr = await findPullRequestForShaWithRetry(repo, sha, token, prLookupBudget); + const pr = await findPullRequestForShaWithRetry(repo, sha, token, prLookupBudget, validatedPrNumber); if (!pr) { // Promote to error: "no PR after the retry budget" is the shape of // a systematic break (deploy-without-PR, token regression, GitHub @@ -541,6 +552,7 @@ async function findPullRequestForShaWithRetry( sha: string, token: string, budgetMs: number, + validatedPrNumber?: number, ): Promise { const deadline = Date.now() + budgetMs; for (let i = 0; i < PR_LOOKUP_RETRY_DELAYS_MS.length; i++) { @@ -552,7 +564,7 @@ async function findPullRequestForShaWithRetry( await new Promise((r) => setTimeout(r, Math.min(delay, remaining))); } if (Date.now() >= deadline) return null; - const pr = await findPullRequestForSha(repo, sha, token); + const pr = await findPullRequestForSha(repo, sha, token, validatedPrNumber); if (pr) return pr; const next = PR_LOOKUP_RETRY_DELAYS_MS[i + 1]; if (next !== undefined) { @@ -574,14 +586,20 @@ async function findPullRequestForShaWithRetry( * * Returns the OPEN PR that the deploy is *for* (head SHA == `sha`), or * the first open PR as a fallback, or null if none. Closed/merged PRs - * are filtered out — v1 only screenshots active reviews. + * are filtered out. For Amplify, only the validated PR with a matching head + * is accepted; there is no fallback to another PR. */ async function findPullRequestForSha( repo: string, sha: string, token: string, + validatedPrNumber?: number, ): Promise { - const url = `https://api.github.com/repos/${repo}/commits/${sha}/pulls`; + // A commit can head multiple PRs. Amplify's validated PR is authoritative; + // fetch it directly so pagination or commit-pulls ordering cannot redirect it. + const url = validatedPrNumber === undefined + ? `https://api.github.com/repos/${repo}/commits/${sha}/pulls` + : `https://api.github.com/repos/${repo}/pulls/${validatedPrNumber}`; let res: Response; // 5s per-request timeout via AbortController. Mirrors the Linear // path, where unbounded fetches were previously blamed for budget @@ -601,7 +619,7 @@ async function findPullRequestForSha( signal: ac.signal, }); } catch (err) { - logger.warn('GitHub commit-pulls fetch failed', { + logger.warn('GitHub PR lookup fetch failed', { repo, sha, timed_out: ac.signal.aborted, @@ -613,7 +631,7 @@ async function findPullRequestForSha( } if (!res.ok) { - logger.warn('GitHub commit-pulls returned non-2xx', { + logger.warn('GitHub PR lookup returned non-2xx', { repo, sha, status: res.status, @@ -621,20 +639,38 @@ async function findPullRequestForSha( return null; } - // GitHub's contract is a JSON array, but a transient 2xx HTML body or - // a malformed payload would crash an unguarded `.find` and throw out - // of the (un-DLQ'd) processor. Treat anything non-array as no-PR. + // Parse defensively: both endpoints can return an unexpected response body. + // A validated Amplify PR must still be open and head this exact commit. let parsed: unknown; try { parsed = await res.json(); } catch (err) { - logger.warn('GitHub commit-pulls returned non-JSON body', { + logger.warn('GitHub PR lookup returned non-JSON body', { repo, sha, error: err instanceof Error ? err.message : String(err), }); return null; // nosemgrep: ts-silent-success-masking -- malformed GitHub body treated as no-PR; prevents processor crash on transient HTML/502 } + if (validatedPrNumber !== undefined) { + const pr = parsed as { number?: unknown; state?: unknown; title?: unknown; body?: unknown; head?: { sha?: unknown; ref?: unknown } } | null; + const reject = (reason: string): null => { + logger.warn('Validated Amplify PR no longer matches preview', { + event: 'screenshot.amplify_pr_rejected', reason, repo, pr_number: validatedPrNumber, + }); + return null; + }; + if (!pr || Array.isArray(pr) || pr.number !== validatedPrNumber) return reject('pr_number_mismatch'); + if (pr.state !== 'open') return reject('pr_not_open'); + if (pr.head?.sha !== sha) return reject('head_sha_mismatch'); + if (typeof pr.head.ref !== 'string' || !pr.head.ref) return reject('missing_head_ref'); + return { + number: validatedPrNumber, + title: typeof pr.title === 'string' ? pr.title : '', + body: typeof pr.body === 'string' ? pr.body : '', + headRefName: pr.head.ref, + }; + } if (!Array.isArray(parsed)) { logger.warn('GitHub commit-pulls did not return an array', { repo, sha }); return null; diff --git a/cdk/src/handlers/github-webhook.ts b/cdk/src/handlers/github-webhook.ts index fdadd47e4..01ac93f57 100644 --- a/cdk/src/handlers/github-webhook.ts +++ b/cdk/src/handlers/github-webhook.ts @@ -50,8 +50,9 @@ const DEDUP_TTL_SECONDS = 60 * 60; * Verifies `X-Hub-Signature-256` (per * https://docs.github.com/en/webhooks/using-webhooks/validating-webhook-deliveries), * filters to successful `deployment_status` events and Amplify PR preview - * `check_run` completions whose normalized environment - * matches `SCREENSHOT_TARGET_ENVIRONMENT` (default `Preview`), dedups + * `check_run` completions. Deployment statuses must match + * `SCREENSHOT_TARGET_ENVIRONMENT` (default `Preview`); validated Amplify + * PR previews bypass that environment filter. Dedups * on `(repo, deployment_id, status_id)`, and async-invokes the * processor Lambda so we can ack within GitHub's 10s timeout. Other * event types (push, pull_request, ping, …) get an immediate 200 so @@ -95,20 +96,23 @@ export async function handler(event: APIGatewayProxyEvent): Promise` // Operators on non-Vercel backends override via // `SCREENSHOT_TARGET_ENVIRONMENT` (Lambda env var, redeploy required). const targetEnv = process.env.SCREENSHOT_TARGET_ENVIRONMENT ?? 'Preview'; - if (raw.deployment?.environment !== targetEnv) { + if (eventType === 'deployment_status' && raw.deployment?.environment !== targetEnv) { return jsonResponse(200, { ok: true, skipped_environment: raw.deployment?.environment, @@ -190,7 +196,8 @@ export async function handler(event: APIGatewayProxyEvent): Promise => input !== null && typeof input === 'object' && !Array.isArray(input) ? input as Record : {}; + const reject = (reason: AmplifyPreviewRejectionReason): AmplifyPreviewCheckResult => ({ ok: false, reason }); const raw = record(value); const check = record(raw.check_run); const app = record(check.app); - if (raw.action !== 'completed' || check.status !== 'completed' || check.conclusion !== 'success' - || check.name !== 'AWS Amplify Console Web Preview' - || record(app.owner).login !== 'aws-amplify-console' - || typeof app.slug !== 'string' || !/^aws-amplify-[a-z0-9-]+$/.test(app.slug) - || typeof check.id !== 'number' || !Number.isSafeInteger(check.id) || check.id <= 0 - || typeof check.head_sha !== 'string' || !/^[0-9a-f]{40}$/i.test(check.head_sha) - || typeof check.details_url !== 'string') { - return null; - } + if (Object.keys(check).length === 0) return reject('invalid_payload'); + if (raw.action !== 'completed') return reject('action_not_completed'); + if (check.status !== 'completed') return reject('check_not_completed'); + if (check.conclusion !== 'success') return reject('check_not_successful'); + if (check.name !== 'AWS Amplify Console Web Preview') return reject('unexpected_check_name'); + if (record(app.owner).login !== 'aws-amplify-console') return reject('unexpected_app_owner'); + if (typeof app.slug !== 'string' || !/^aws-amplify-[a-z0-9-]+$/.test(app.slug)) return reject('unexpected_app_slug'); + if (typeof check.id !== 'number' || !Number.isSafeInteger(check.id) || check.id <= 0) return reject('invalid_check_id'); + if (typeof check.head_sha !== 'string' || !/^[0-9a-f]{40}$/i.test(check.head_sha)) return reject('invalid_head_sha'); + if (typeof check.details_url !== 'string') return reject('invalid_details_url'); let url: URL; try { url = new URL(check.details_url); } catch { - return null; // nosemgrep: ts-silent-success-masking -- Invalid URL means an ineligible check; the receiver returns skipped_check without starting capture. + return reject('invalid_details_url'); } - const preview = /^pr-(\d+)\.[a-z0-9]+\.amplifyapp\.com$/.exec(url.hostname); - if (url.protocol !== 'https:' || url.username || url.password || url.port || !preview - || !Array.isArray(check.pull_requests) - || !check.pull_requests.some((pr: unknown) => record(pr).number === Number(preview[1]) - && record(record(pr).head).sha === check.head_sha)) { - return null; + const preview = /^pr-([1-9]\d*)\.[a-z0-9]+\.amplifyapp\.com$/.exec(url.hostname); + if (url.protocol !== 'https:' || url.username || url.password || url.port || !preview) { + return reject('untrusted_preview_url'); + } + const prNumber = Number(preview[1]); + if (!Number.isSafeInteger(prNumber)) return reject('invalid_preview_pr_number'); + if (!Array.isArray(check.pull_requests)) return reject('invalid_pull_requests'); + const previewPrs = check.pull_requests.filter((pr: unknown) => record(pr).number === prNumber); + if (previewPrs.length === 0) return reject('preview_pr_not_found'); + if (!previewPrs.some((pr: unknown) => record(record(pr).head).sha === check.head_sha)) { + return reject('head_sha_mismatch'); } const repository = record(raw.repository); - if (typeof repository.full_name !== 'string' || !isValidRepo(repository.full_name)) return null; + if (typeof repository.full_name !== 'string' || !isValidRepo(repository.full_name)) return reject('invalid_repository'); return { - repository: { full_name: repository.full_name }, - deployment: { id: check.id, sha: check.head_sha, environment: 'Preview' }, - deployment_status: { id: check.id, state: 'success', environment_url: check.details_url }, + ok: true, + prNumber, + payload: { + repository: { full_name: repository.full_name }, + deployment: { id: check.id, sha: check.head_sha, environment: 'Preview' }, + deployment_status: { id: check.id, state: 'success', environment_url: check.details_url }, + }, }; } diff --git a/cdk/test/handlers/github-webhook-processor.test.ts b/cdk/test/handlers/github-webhook-processor.test.ts index c9de4ff46..14a789042 100644 --- a/cdk/test/handlers/github-webhook-processor.test.ts +++ b/cdk/test/handlers/github-webhook-processor.test.ts @@ -75,6 +75,7 @@ process.env.LINEAR_WORKSPACE_REGISTRY_TABLE_NAME = 'LinearWorkspaceRegistry'; process.env.TASK_TABLE_NAME = 'TaskTable'; import { handler } from '../../src/handlers/github-webhook-processor'; +import { normalizeAmplifyPreviewCheck } from '../../src/handlers/shared/github-deployment-status'; import { logger } from '../../src/handlers/shared/logger'; function payload(overrides: Record = {}): { raw_body: string } { @@ -488,3 +489,108 @@ describe('authoritative Jira deployment routing', () => { expect(findLinearIssueMock).not.toHaveBeenCalled(); }); }); + +describe('validated Amplify PR routing', () => { + const sha = 'a'.repeat(40); + const taskId = '01JXABCDEF1234567890ABCDEF'; + const branch = `bgagent/${taskId}/eng-42`; + const pr41 = { number: 41, state: 'open', title: 'ENG-41', head: { ref: 'bgagent/wrong-task/eng-41', sha } }; + const pr42 = { number: 42, state: 'open', title: 'ENG-42', head: { ref: branch, sha } }; + + function amplifyEvent() { + const result = normalizeAmplifyPreviewCheck({ + action: 'completed', + repository: { full_name: 'owner/repo' }, + check_run: { + id: 123, + name: 'AWS Amplify Console Web Preview', + status: 'completed', + conclusion: 'success', + head_sha: sha, + details_url: 'https://pr-42.app123.amplifyapp.com', + app: { slug: 'aws-amplify-us-east-1', owner: { login: 'aws-amplify-console' } }, + pull_requests: [pr41, pr42], + }, + }); + if (!result.ok) throw new Error(result.reason); + return { raw_body: JSON.stringify(result.payload), validated_pr_number: result.prNumber }; + } + + beforeEach(() => { + jest.restoreAllMocks(); + process.env.JIRA_WORKSPACE_REGISTRY_TABLE_NAME = 'JiraRegistry'; + deliverJiraMock.mockReset().mockResolvedValue(undefined); + resolveGitHubTokenMock.mockReset().mockResolvedValue('token'); + captureScreenshotMock.mockReset().mockResolvedValue(Buffer.from('png')); + s3Send.mockReset().mockResolvedValue({}); + ddbSend.mockReset(); + upsertTaskCommentMock.mockReset().mockResolvedValue({ commentId: 12 }); + postIssueCommentMock.mockReset().mockResolvedValue(true); + findLinearIssueMock.mockReset().mockResolvedValue({ issueId: 'issue-42', linearWorkspaceId: 'ws' }); + extractFromBranchMock.mockReset().mockImplementation((ref) => ref === branch ? 'ENG-42' : 'ENG-41'); + }); + + test.each(['jira', 'linear'])('two PRs with one SHA route GitHub and %s feedback to the validated PR', async (source) => { + const fetchMock = jest.spyOn(global, 'fetch').mockImplementation(async (url) => ({ + ok: true, + status: 200, + // The commit-pulls endpoint would pick PR 41. Only a lookup by number + // preserves the PR encoded in the preview URL through task persistence. + json: async () => String(url).endsWith('/pulls/42') ? pr42 : [pr41, pr42], + } as Response)); + const task = { task_id: taskId, repo: 'owner/repo', head_sha: sha, channel_source: source, channel_metadata: { jira_cloud_id: 'cloud', jira_issue_key: 'TG-42' } }; + ddbSend.mockResolvedValue({ Attributes: task }); + + await handler(amplifyEvent()); + + expect(fetchMock).toHaveBeenCalledWith('https://api.github.com/repos/owner/repo/pulls/42', expect.anything()); + expect(captureScreenshotMock).toHaveBeenCalledWith('https://pr-42.app123.amplifyapp.com', expect.anything()); + expect(upsertTaskCommentMock).toHaveBeenCalledWith(expect.objectContaining({ repo: 'owner/repo', issueOrPrNumber: 42 })); + expect(ddbSend.mock.calls[0][0].input.Key).toEqual({ task_id: taskId }); + if (source === 'jira') { + expect(deliverJiraMock).toHaveBeenCalledWith(expect.anything(), 'TaskTable', 'JiraRegistry', task, + 'owner/repo', sha, expect.stringContaining('https://d1.cloudfront.net/'), + 'https://pr-42.app123.amplifyapp.com', expect.any(Function)); + expect(findLinearIssueMock).not.toHaveBeenCalled(); + } else { + expect(findLinearIssueMock).toHaveBeenCalledWith('ENG-42', 'LinearWorkspaceRegistry'); + expect(postIssueCommentMock).toHaveBeenCalledWith(expect.anything(), 'issue-42', expect.stringContaining('https://pr-42.app123.amplifyapp.com')); + expect(deliverJiraMock).not.toHaveBeenCalled(); + } + }); + + test.each([ + [pr41, 'pr_number_mismatch'], + [{ ...pr42, state: 'closed' }, 'pr_not_open'], + [{ ...pr42, head: { ref: branch, sha: 'b'.repeat(40) } }, 'head_sha_mismatch'], + [{ ...pr42, head: { sha } }, 'missing_head_ref'], + [null, 'pr_number_mismatch'], + [[pr41, pr42], 'pr_number_mismatch'], + ])('rejects a changed or malformed PR without capturing or falling back: %j', async (pr, reason) => { + jest.useFakeTimers(); + try { + const log = jest.spyOn(logger, 'warn'); + const fetchMock = jest.spyOn(global, 'fetch').mockResolvedValue({ ok: true, status: 200, json: async () => pr } as Response); + const pending = handler(amplifyEvent()); + await jest.runAllTimersAsync(); + await pending; + expect(log).toHaveBeenCalledWith('Validated Amplify PR no longer matches preview', { + event: 'screenshot.amplify_pr_rejected', reason, repo: 'owner/repo', pr_number: 42, + }); + expect(fetchMock.mock.calls.every(([url]) => String(url).endsWith('/pulls/42'))).toBe(true); + expect(captureScreenshotMock).not.toHaveBeenCalled(); + expect(ddbSend).not.toHaveBeenCalled(); + expect(upsertTaskCommentMock).not.toHaveBeenCalled(); + expect(deliverJiraMock).not.toHaveBeenCalled(); + expect(postIssueCommentMock).not.toHaveBeenCalled(); + } finally { + jest.useRealTimers(); + } + }); + + test.each([0, -1, 1.5, Number.MAX_SAFE_INTEGER + 1])('rejects invalid forwarded PR number %s', async (number) => { + await handler({ ...amplifyEvent(), validated_pr_number: number }); + expect(resolveGitHubTokenMock).not.toHaveBeenCalled(); + expect(captureScreenshotMock).not.toHaveBeenCalled(); + }); +}); diff --git a/cdk/test/handlers/github-webhook.test.ts b/cdk/test/handlers/github-webhook.test.ts index fbc8da9ef..62ce9621e 100644 --- a/cdk/test/handlers/github-webhook.test.ts +++ b/cdk/test/handlers/github-webhook.test.ts @@ -52,6 +52,7 @@ process.env.GITHUB_WEBHOOK_DEDUP_TABLE_NAME = 'GhWebhookDedup'; process.env.GITHUB_WEBHOOK_PROCESSOR_FUNCTION_NAME = 'gh-webhook-processor'; import { handler } from '../../src/handlers/github-webhook'; +import { logger } from '../../src/handlers/shared/logger'; function event(body: string | null, headers: Record = {}): APIGatewayProxyEvent { return { @@ -111,6 +112,7 @@ function amplifyBody(overrides: Record = {}): string { describe('github-webhook receiver', () => { beforeEach(() => { + jest.restoreAllMocks(); ddbSend.mockReset(); lambdaSend.mockReset(); verifyMock.mockReset(); @@ -247,6 +249,7 @@ describe('github-webhook receiver', () => { ); const invoke = lambdaSend.mock.calls[0][0].input; const forwarded = JSON.parse(new TextDecoder().decode(invoke.Payload)); + expect(forwarded.validated_pr_number).toBe(41); expect(JSON.parse(forwarded.raw_body)).toEqual({ repository: { full_name: 'owner/repo' }, deployment: { id: 104550173395, sha: amplifySha, environment: 'Preview' }, @@ -261,28 +264,38 @@ describe('github-webhook receiver', () => { }); test.each([ - { status: 'in_progress' }, - { conclusion: 'failure' }, - { name: 'unrelated CI check' }, - { app: { slug: 'aws-amplify-us-east-1', owner: { login: 'another-owner' } } }, - { app: { slug: 'another-app', owner: { login: 'aws-amplify-console' } } }, - { details_url: 'https://console.aws.amazon.com/amplify/home' }, - { details_url: 'https://pr-41.example.com' }, - { details_url: 'https://pr-41.d1prbufb0nhsx2.amplifyapp.com.evil.example' }, - { details_url: 'http://pr-41.d1prbufb0nhsx2.amplifyapp.com' }, - { details_url: 'https://user:password@pr-41.d1prbufb0nhsx2.amplifyapp.com' }, - { details_url: 'https://pr-41.d1prbufb0nhsx2.amplifyapp.com:8080' }, - { details_url: 'not-a-url' }, - { details_url: null }, - { pull_requests: [] }, - { pull_requests: [{ number: 42, head: { sha: amplifySha } }] }, - { pull_requests: [{ number: 41, head: { sha: 'b'.repeat(40) } }] }, - { head_sha: '../invalid' }, - { id: -1 }, - { id: '104550173395' }, - ])('ignores an ineligible Amplify check: %j', async (overrides) => { + [{ status: 'in_progress' }, 'check_not_completed'], + [{ conclusion: 'failure' }, 'check_not_successful'], + [{ name: 'unrelated CI check' }, 'unexpected_check_name'], + [{ app: { slug: 'aws-amplify-us-east-1', owner: { login: 'another-owner' } } }, 'unexpected_app_owner'], + [{ app: { slug: 'another-app', owner: { login: 'aws-amplify-console' } } }, 'unexpected_app_slug'], + [{ details_url: 'https://console.aws.amazon.com/amplify/home' }, 'untrusted_preview_url'], + [{ details_url: 'https://pr-41.example.com' }, 'untrusted_preview_url'], + [{ details_url: 'https://pr-41.d1prbufb0nhsx2.amplifyapp.com.evil.example' }, 'untrusted_preview_url'], + [{ details_url: 'http://pr-41.d1prbufb0nhsx2.amplifyapp.com' }, 'untrusted_preview_url'], + [{ details_url: 'https://user:password@pr-41.d1prbufb0nhsx2.amplifyapp.com' }, 'untrusted_preview_url'], + [{ details_url: 'https://pr-41.d1prbufb0nhsx2.amplifyapp.com:8080' }, 'untrusted_preview_url'], + [{ details_url: 'not-a-url' }, 'invalid_details_url'], + [{ details_url: null }, 'invalid_details_url'], + [{ details_url: 'https://pr-0.app.amplifyapp.com' }, 'untrusted_preview_url'], + [{ details_url: 'https://pr-9007199254740992.app.amplifyapp.com' }, 'invalid_preview_pr_number'], + [{ pull_requests: null }, 'invalid_pull_requests'], + [{ pull_requests: [] }, 'preview_pr_not_found'], + [{ pull_requests: [null, {}, { number: 42, head: { sha: amplifySha } }] }, 'preview_pr_not_found'], + [{ pull_requests: [{ number: 41, head: { sha: 'b'.repeat(40) } }] }, 'head_sha_mismatch'], + [{ head_sha: '../invalid' }, 'invalid_head_sha'], + [{ id: -1 }, 'invalid_check_id'], + [{ id: 1.5 }, 'invalid_check_id'], + [{ id: Number.MAX_SAFE_INTEGER + 1 }, 'invalid_check_id'], + [{ id: '104550173395' }, 'invalid_check_id'], + ] as const)('rejects an ineligible Amplify check: %j (%s)', async (overrides, reason) => { + const log = jest.spyOn(logger, 'info'); const res = await handler(event(amplifyBody(overrides), { 'X-GitHub-Event': 'check_run' })); - expect(JSON.parse(res.body)).toEqual({ ok: true, skipped_check: true }); + expect(JSON.parse(res.body)).toEqual({ ok: true, skipped_check: true, reason }); + // Only a static reason is logged, never the URL, credentials, or raw payload. + expect(log).toHaveBeenCalledWith('Amplify preview check rejected', { + event: 'screenshot.amplify_check_rejected', reason, + }); expect(ddbSend).not.toHaveBeenCalled(); expect(lambdaSend).not.toHaveBeenCalled(); }); @@ -291,6 +304,7 @@ describe('github-webhook receiver', () => { 'ignores malformed check envelopes: %s', async (body) => { const res = await handler(event(body, { 'X-GitHub-Event': 'check_run' })); expect(res.statusCode).toBe(200); + expect(JSON.parse(res.body).reason).toBe('invalid_payload'); expect(lambdaSend).not.toHaveBeenCalled(); }, ); @@ -320,14 +334,47 @@ describe('github-webhook receiver', () => { expect(lambdaSend).not.toHaveBeenCalled(); }); - test('Amplify preview checks respect the configured environment filter', async () => { - process.env.SCREENSHOT_TARGET_ENVIRONMENT = 'Production'; + test('Amplify previews bypass a branch filter while deployment statuses still require it', async () => { + process.env.SCREENSHOT_TARGET_ENVIRONMENT = 'main'; try { const res = await handler(event(amplifyBody(), { 'X-GitHub-Event': 'check_run' })); - expect(JSON.parse(res.body).skipped_environment).toBe('Preview'); - expect(lambdaSend).not.toHaveBeenCalled(); + expect(JSON.parse(res.body)).toEqual({ ok: true }); + expect(lambdaSend).toHaveBeenCalledTimes(1); + await handler(event(deploymentStatusBody({ environment: 'main' }))); + expect(lambdaSend).toHaveBeenCalledTimes(2); + const skipped = await handler(event(deploymentStatusBody())); + expect(JSON.parse(skipped.body).skipped_environment).toBe('Preview'); + expect(lambdaSend).toHaveBeenCalledTimes(2); } finally { delete process.env.SCREENSHOT_TARGET_ENVIRONMENT; } }); + test('deployment statuses and Amplify checks with identical IDs deduplicate independently', async () => { + const keys = new Set(); + ddbSend.mockImplementation(async ({ input }) => { + expect(input.ConditionExpression).toBe('attribute_not_exists(dedup_key)'); + const key = input.Item.dedup_key; + if (keys.has(key)) throw new FakeConditionalCheckFailedException(); + keys.add(key); + return {}; + }); + const deployment = event(deploymentStatusBody({ deploymentId: 104550173395, statusId: 104550173395 })); + const check = event(amplifyBody(), { 'X-GitHub-Event': 'check_run' }); + await handler(deployment); + await handler(check); + expect(lambdaSend).toHaveBeenCalledTimes(2); + expect(JSON.parse((await handler(check)).body).deduped).toBe(true); + expect(JSON.parse((await handler(deployment)).body).deduped).toBe(true); + expect(lambdaSend).toHaveBeenCalledTimes(2); + }); + + test('invalid check JSON logs a static reason without including body fragments', async () => { + const log = jest.spyOn(logger, 'warn'); + const res = await handler(event('secret-token-not-json', { 'X-GitHub-Event': 'check_run' })); + expect(res.statusCode).toBe(400); + expect(log).toHaveBeenCalledWith('GitHub webhook body is not valid JSON', { + event: 'screenshot.webhook_rejected', reason: 'invalid_json', + }); + expect(lambdaSend).not.toHaveBeenCalled(); + }); }); diff --git a/docs/guides/DEPLOY_PREVIEW_SCREENSHOTS_GUIDE.md b/docs/guides/DEPLOY_PREVIEW_SCREENSHOTS_GUIDE.md index 3ca8cd906..958f1faa4 100644 --- a/docs/guides/DEPLOY_PREVIEW_SCREENSHOTS_GUIDE.md +++ b/docs/guides/DEPLOY_PREVIEW_SCREENSHOTS_GUIDE.md @@ -11,7 +11,7 @@ The pipeline accepts GitHub `deployment_status` events and successful AWS Amplif | Provider | Out of the box? | Notes | |---|---|---| | **Vercel** (managed hosting + GitHub app) | ✅ | The worked example below uses this. Default `environment` is `Preview`. | -| **AWS Amplify Hosting** (Connected to GitHub) | ✅ | Enable PR previews and subscribe the ABCA webhook to **Check runs**. Successful `AWS Amplify Console Web Preview` checks are normalized to environment `Preview`. | +| **AWS Amplify Hosting** (Connected to GitHub) | ✅ | Enable PR previews and subscribe the ABCA webhook to **Check runs**. Successful `AWS Amplify Console Web Preview` checks bypass the deployment environment filter. | | **Netlify** (managed hosting + GitHub app) | ⚠ | `environment` is `Deploy Preview `, which the current single-string `SCREENSHOT_TARGET_ENVIRONMENT` filter doesn't match across all PRs. Workable today only by picking one specific PR's environment string; broader pattern matching isn't shipped. | | **GitHub Actions** that calls `POST /repos/.../deployments` (typical for ECS/Fargate, Cloud Run, Fly.io, Railway, Cloudflare Pages, etc.) | ✅ | Your workflow controls the `environment` field; pass whatever you want and set `SCREENSHOT_TARGET_ENVIRONMENT` to match. | | **External CI** (CircleCI, GitLab, ArgoCD) that doesn't touch GitHub Deployments | ❌ | Add a final job that calls the GitHub Deployments API after the deploy succeeds — see [GitHub's example](https://docs.github.com/en/rest/deployments/deployments#create-a-deployment). | @@ -23,11 +23,13 @@ For a deployment-status event, ABCA needs: If your provider gives you that, you're done. The example below is Vercel because that's what we smoke-tested on; the pipeline doesn't otherwise prefer one provider over another. -For Amplify, enable **Hosting → Previews** on the PR's target branch and add **Check runs** to the repository's ABCA webhook events. Amplify publishes the URL in the completed check's `details_url`; a green GitHub check alone will not trigger capture if the webhook only subscribes to Deployment statuses. ABCA accepts successful preview checks from the `aws-amplify-console` app owner with a matching PR number, head SHA, and HTTPS `pr-..amplifyapp.com` URL. No manual deployment event or extra GitHub Actions workflow is needed. +For Amplify, enable **Hosting → Previews** on the PR's target branch and add **Check runs** to the repository's ABCA webhook events. Amplify publishes the URL in the completed check's `details_url`; a green GitHub check alone will not trigger capture if the webhook only subscribes to Deployment statuses. ABCA accepts successful preview checks from the `aws-amplify-console` app owner with a matching PR number, head SHA, and HTTPS `pr-..amplifyapp.com` URL. No manual deployment event or extra GitHub Actions workflow is needed. The processor fetches that exact PR and confirms it is still open with the same head SHA before capture, so two PRs sharing a commit cannot redirect the screenshot or Jira/Linear feedback. + +**Existing Amplify operators:** redeploy ABCA to pick up this receiver and processor, then add **Check runs** to your existing webhook while keeping **Deployment statuses** selected. Keep any branch-name `SCREENSHOT_TARGET_ENVIRONMENT` value (for example, `main`): deployment statuses still use it, while validated Amplify PR checks bypass it. You do not need to change it to `Preview`. Subscriptions affect future events only; rebuild an existing PR preview to verify the change. If an earlier receiver already accepted and deduplicated a completion, replaying that same check within the one-hour dedup window will not capture again. ## What you get -When you (or the agent) push to a branch that triggers a preview deploy, your provider deploys the preview, posts a `deployment_status` event back to GitHub, and ABCA's webhook receiver: +When you (or the agent) push to a branch that triggers a preview deploy, your provider deploys the preview, posts a deployment status or Amplify preview check back to GitHub, and ABCA's webhook receiver: 1. Captures a full-page screenshot of the preview URL via AgentCore Browser 2. Uploads the PNG to a private S3 bucket served via CloudFront @@ -39,13 +41,13 @@ End-to-end latency: typically 10–15 seconds after your provider reports the de ## How it works ``` -agent push → provider preview build → deployment_status webhook +agent push → provider preview build → deployment_status / Amplify check_run ↓ POST /v1/github/webhook ↓ receiver Lambda (HMAC verify, dedup, - state=success + - environment filter) + successful deploy/check + + provider validation) ↓ processor Lambda ↓ @@ -55,7 +57,7 @@ agent push → provider preview build → deployment_status webhook ↓ CloudFront-served public URL ↓ - GitHub PR comment (+ Linear issue comment if linked) + GitHub PR comment (+ Jira/Linear feedback if linked) ``` Architecture notes: @@ -143,12 +145,13 @@ Open any PR on the configured repo (push a commit, open a PR however you normall ## Configuring for non-Vercel providers -The pipeline filters incoming webhooks against `SCREENSHOT_TARGET_ENVIRONMENT` (default `Preview`, matches Vercel's per-PR environment label). To use a different value, pass `screenshotTargetEnvironment` to the `GitHubScreenshotIntegration` construct in your CDK app and redeploy. +The pipeline filters `deployment_status` webhooks against `SCREENSHOT_TARGET_ENVIRONMENT` (default `Preview`, matches Vercel's per-PR environment label). To use a different value, pass `screenshotTargetEnvironment` to the `GitHubScreenshotIntegration` construct in your CDK app and redeploy. | Provider | Typical `environment` value | What to set | |---|---|---| | Vercel | `Preview` | leave default | -| Amplify Hosting PR check | normalized to `Preview` | leave default; subscribe to Check runs | +| Amplify Hosting PR check | not used for filtering | keep existing value; subscribe to Check runs | +| Amplify branch deployment status | branch name | match the branch name exactly | | Netlify | `Deploy Preview ` | currently not directly matchable across all PRs (single fixed-string filter only) | | GitHub Actions custom | whatever your workflow passes | match it exactly | @@ -162,7 +165,19 @@ The pipeline filters incoming webhooks against `SCREENSHOT_TARGET_ENVIRONMENT` ( ### Webhook delivers 200 but no screenshot lands -For Amplify, confirm **Check runs** is selected on the ABCA webhook, the `AWS Amplify Console Web Preview` check completed successfully, and its details link opens the PR preview. `skipped_check` means the event was not an eligible successful Amplify PR preview. Adding the subscription only affects future events; rebuild an existing preview to exercise the automatic path. +For Amplify, confirm **Check runs** is selected on the ABCA webhook, the `AWS Amplify Console Web Preview` check completed successfully, and its details link opens the PR preview. `skipped_check` means the event was not an eligible successful Amplify PR preview. Its `reason` is also logged by the receiver as `screenshot.amplify_check_rejected`, without the raw payload or URL. Adding the subscription only affects future events; rebuild an existing preview to exercise the automatic path. + +Inspect the receiver logs for rejected checks: + +| Reason | What to check | +|---|---| +| `action_not_completed`, `check_not_completed`, `check_not_successful` | Wait for a successful completed check. | +| `unexpected_check_name`, `unexpected_app_owner`, `unexpected_app_slug` | The check must be `AWS Amplify Console Web Preview`, owned by `aws-amplify-console`, with an `aws-amplify-*` app slug. Other CI checks are ignored. | +| `invalid_details_url`, `untrusted_preview_url`, `invalid_preview_pr_number` | The details link must be a trusted HTTPS `pr-N..amplifyapp.com` preview URL with a positive PR number, no credentials, and no non-default port. | +| `invalid_pull_requests`, `preview_pr_not_found`, `head_sha_mismatch` | The check must list the preview PR with the same head SHA as the check. | +| `invalid_payload`, `invalid_check_id`, `invalid_head_sha`, `invalid_repository` | The webhook payload is malformed; inspect the delivery in GitHub. | + +Malformed JSON returns 400 and logs `screenshot.webhook_rejected` with `reason: invalid_json`. Invalid signatures return 401 before normalization. Valid checks use a separate `amplify#` dedup namespace, so a deployment-status event with identical IDs does not suppress the check. Duplicate checks return `deduped` without dispatching another capture. Check the screenshot processor logs: @@ -175,8 +190,9 @@ aws lambda list-functions --region us-east-1 \ Then tail the function's CloudWatch log group. Common silent skips: - `skipped_state` — the delivery was for a non-`success` status (e.g. `pending`, `in_progress`); ignore. -- `skipped_environment` — the deploy's `environment` field doesn't match `SCREENSHOT_TARGET_ENVIRONMENT`. Common cause for non-Vercel providers; see "Configuring for non-Vercel providers" above. +- `skipped_environment` (deployment statuses only) — the deploy's `environment` field doesn't match `SCREENSHOT_TARGET_ENVIRONMENT`. Common cause for non-Vercel providers; see "Configuring for non-Vercel providers" above. - `skipped_no_url` — the `success` status didn't include `environment_url`. Some providers post URL-less success events; the next push usually carries the URL. +- `screenshot.amplify_pr_rejected` — the validated PR is closed, its head SHA changed, or GitHub returned mismatched/malformed PR data. Capture stops without falling back to another PR. Rebuild the current PR preview after a new push. - `No open PR found for SHA after retries` — the deploy provider built and reported faster than the agent could `gh pr create` (race window > 35s). Rare; redeliver the webhook from GitHub's UI to retry. ### No screenshots at all: check the processor alarms and DLQ diff --git a/docs/guides/JIRA_SETUP_GUIDE.md b/docs/guides/JIRA_SETUP_GUIDE.md index 1fa6c0560..a1445696d 100644 --- a/docs/guides/JIRA_SETUP_GUIDE.md +++ b/docs/guides/JIRA_SETUP_GUIDE.md @@ -313,7 +313,7 @@ After the PR exists, add a Jira comment such as `@bgagent update the README too` When a successful GitHub preview deployment is captured, ABCA also adds **Open screenshot** and **Open live preview** links to the originating Jira issue. Configure the GitHub deployment-status webhook described in the [deploy-preview screenshots guide](DEPLOY_PREVIEW_SCREENSHOTS_GUIDE.md). Jira uses explicit ADF links because these externally hosted screenshots do not have Atlassian media IDs. -For AWS Amplify Hosting, enable PR previews and select **Check runs** on that GitHub webhook. Amplify's successful preview check triggers capture and Jira delivery automatically; subscribing only to Deployment statuses does not capture Amplify previews. +For AWS Amplify Hosting, enable PR previews and select **Check runs** on that GitHub webhook. Amplify's successful preview check triggers capture and Jira delivery automatically; subscribing only to Deployment statuses does not capture Amplify previews. Existing operators can keep their branch-name `SCREENSHOT_TARGET_ENVIRONMENT` setting: validated PR checks bypass it. After redeploying ABCA and updating the subscription, rebuild a PR preview. The screenshot and Jira task lookup follow the PR validated against the preview URL and head SHA, including when multiple PRs share a commit. Rejected checks log `screenshot.amplify_check_rejected` with a reason in the receiver logs; see the screenshots guide for diagnostics. For an iteration, the links appear in its existing status comment and survive later heartbeat and terminal edits. A fan-out orchestration's combined preview appears in the parent rollup. Delivery targets the task encoded in the deploy branch and then routes through that task's stored Jira tenant/issue metadata. The Jira issue is never parsed from branch or PR text. Cross-repository delivery is rejected, and an iteration must match the deployment SHA. Duplicate deployment events update the existing preview comment or block. diff --git a/docs/src/content/docs/using/Deploy-preview-screenshots-guide.md b/docs/src/content/docs/using/Deploy-preview-screenshots-guide.md index 2a6e96eb7..37316e946 100644 --- a/docs/src/content/docs/using/Deploy-preview-screenshots-guide.md +++ b/docs/src/content/docs/using/Deploy-preview-screenshots-guide.md @@ -15,7 +15,7 @@ The pipeline accepts GitHub `deployment_status` events and successful AWS Amplif | Provider | Out of the box? | Notes | |---|---|---| | **Vercel** (managed hosting + GitHub app) | ✅ | The worked example below uses this. Default `environment` is `Preview`. | -| **AWS Amplify Hosting** (Connected to GitHub) | ✅ | Enable PR previews and subscribe the ABCA webhook to **Check runs**. Successful `AWS Amplify Console Web Preview` checks are normalized to environment `Preview`. | +| **AWS Amplify Hosting** (Connected to GitHub) | ✅ | Enable PR previews and subscribe the ABCA webhook to **Check runs**. Successful `AWS Amplify Console Web Preview` checks bypass the deployment environment filter. | | **Netlify** (managed hosting + GitHub app) | ⚠ | `environment` is `Deploy Preview `, which the current single-string `SCREENSHOT_TARGET_ENVIRONMENT` filter doesn't match across all PRs. Workable today only by picking one specific PR's environment string; broader pattern matching isn't shipped. | | **GitHub Actions** that calls `POST /repos/.../deployments` (typical for ECS/Fargate, Cloud Run, Fly.io, Railway, Cloudflare Pages, etc.) | ✅ | Your workflow controls the `environment` field; pass whatever you want and set `SCREENSHOT_TARGET_ENVIRONMENT` to match. | | **External CI** (CircleCI, GitLab, ArgoCD) that doesn't touch GitHub Deployments | ❌ | Add a final job that calls the GitHub Deployments API after the deploy succeeds — see [GitHub's example](https://docs.github.com/en/rest/deployments/deployments#create-a-deployment). | @@ -27,11 +27,13 @@ For a deployment-status event, ABCA needs: If your provider gives you that, you're done. The example below is Vercel because that's what we smoke-tested on; the pipeline doesn't otherwise prefer one provider over another. -For Amplify, enable **Hosting → Previews** on the PR's target branch and add **Check runs** to the repository's ABCA webhook events. Amplify publishes the URL in the completed check's `details_url`; a green GitHub check alone will not trigger capture if the webhook only subscribes to Deployment statuses. ABCA accepts successful preview checks from the `aws-amplify-console` app owner with a matching PR number, head SHA, and HTTPS `pr-..amplifyapp.com` URL. No manual deployment event or extra GitHub Actions workflow is needed. +For Amplify, enable **Hosting → Previews** on the PR's target branch and add **Check runs** to the repository's ABCA webhook events. Amplify publishes the URL in the completed check's `details_url`; a green GitHub check alone will not trigger capture if the webhook only subscribes to Deployment statuses. ABCA accepts successful preview checks from the `aws-amplify-console` app owner with a matching PR number, head SHA, and HTTPS `pr-..amplifyapp.com` URL. No manual deployment event or extra GitHub Actions workflow is needed. The processor fetches that exact PR and confirms it is still open with the same head SHA before capture, so two PRs sharing a commit cannot redirect the screenshot or Jira/Linear feedback. + +**Existing Amplify operators:** redeploy ABCA to pick up this receiver and processor, then add **Check runs** to your existing webhook while keeping **Deployment statuses** selected. Keep any branch-name `SCREENSHOT_TARGET_ENVIRONMENT` value (for example, `main`): deployment statuses still use it, while validated Amplify PR checks bypass it. You do not need to change it to `Preview`. Subscriptions affect future events only; rebuild an existing PR preview to verify the change. If an earlier receiver already accepted and deduplicated a completion, replaying that same check within the one-hour dedup window will not capture again. ## What you get -When you (or the agent) push to a branch that triggers a preview deploy, your provider deploys the preview, posts a `deployment_status` event back to GitHub, and ABCA's webhook receiver: +When you (or the agent) push to a branch that triggers a preview deploy, your provider deploys the preview, posts a deployment status or Amplify preview check back to GitHub, and ABCA's webhook receiver: 1. Captures a full-page screenshot of the preview URL via AgentCore Browser 2. Uploads the PNG to a private S3 bucket served via CloudFront @@ -43,13 +45,13 @@ End-to-end latency: typically 10–15 seconds after your provider reports the de ## How it works ``` -agent push → provider preview build → deployment_status webhook +agent push → provider preview build → deployment_status / Amplify check_run ↓ POST /v1/github/webhook ↓ receiver Lambda (HMAC verify, dedup, - state=success + - environment filter) + successful deploy/check + + provider validation) ↓ processor Lambda ↓ @@ -59,7 +61,7 @@ agent push → provider preview build → deployment_status webhook ↓ CloudFront-served public URL ↓ - GitHub PR comment (+ Linear issue comment if linked) + GitHub PR comment (+ Jira/Linear feedback if linked) ``` Architecture notes: @@ -147,12 +149,13 @@ Open any PR on the configured repo (push a commit, open a PR however you normall ## Configuring for non-Vercel providers -The pipeline filters incoming webhooks against `SCREENSHOT_TARGET_ENVIRONMENT` (default `Preview`, matches Vercel's per-PR environment label). To use a different value, pass `screenshotTargetEnvironment` to the `GitHubScreenshotIntegration` construct in your CDK app and redeploy. +The pipeline filters `deployment_status` webhooks against `SCREENSHOT_TARGET_ENVIRONMENT` (default `Preview`, matches Vercel's per-PR environment label). To use a different value, pass `screenshotTargetEnvironment` to the `GitHubScreenshotIntegration` construct in your CDK app and redeploy. | Provider | Typical `environment` value | What to set | |---|---|---| | Vercel | `Preview` | leave default | -| Amplify Hosting PR check | normalized to `Preview` | leave default; subscribe to Check runs | +| Amplify Hosting PR check | not used for filtering | keep existing value; subscribe to Check runs | +| Amplify branch deployment status | branch name | match the branch name exactly | | Netlify | `Deploy Preview ` | currently not directly matchable across all PRs (single fixed-string filter only) | | GitHub Actions custom | whatever your workflow passes | match it exactly | @@ -166,7 +169,19 @@ The pipeline filters incoming webhooks against `SCREENSHOT_TARGET_ENVIRONMENT` ( ### Webhook delivers 200 but no screenshot lands -For Amplify, confirm **Check runs** is selected on the ABCA webhook, the `AWS Amplify Console Web Preview` check completed successfully, and its details link opens the PR preview. `skipped_check` means the event was not an eligible successful Amplify PR preview. Adding the subscription only affects future events; rebuild an existing preview to exercise the automatic path. +For Amplify, confirm **Check runs** is selected on the ABCA webhook, the `AWS Amplify Console Web Preview` check completed successfully, and its details link opens the PR preview. `skipped_check` means the event was not an eligible successful Amplify PR preview. Its `reason` is also logged by the receiver as `screenshot.amplify_check_rejected`, without the raw payload or URL. Adding the subscription only affects future events; rebuild an existing preview to exercise the automatic path. + +Inspect the receiver logs for rejected checks: + +| Reason | What to check | +|---|---| +| `action_not_completed`, `check_not_completed`, `check_not_successful` | Wait for a successful completed check. | +| `unexpected_check_name`, `unexpected_app_owner`, `unexpected_app_slug` | The check must be `AWS Amplify Console Web Preview`, owned by `aws-amplify-console`, with an `aws-amplify-*` app slug. Other CI checks are ignored. | +| `invalid_details_url`, `untrusted_preview_url`, `invalid_preview_pr_number` | The details link must be a trusted HTTPS `pr-N..amplifyapp.com` preview URL with a positive PR number, no credentials, and no non-default port. | +| `invalid_pull_requests`, `preview_pr_not_found`, `head_sha_mismatch` | The check must list the preview PR with the same head SHA as the check. | +| `invalid_payload`, `invalid_check_id`, `invalid_head_sha`, `invalid_repository` | The webhook payload is malformed; inspect the delivery in GitHub. | + +Malformed JSON returns 400 and logs `screenshot.webhook_rejected` with `reason: invalid_json`. Invalid signatures return 401 before normalization. Valid checks use a separate `amplify#` dedup namespace, so a deployment-status event with identical IDs does not suppress the check. Duplicate checks return `deduped` without dispatching another capture. Check the screenshot processor logs: @@ -179,8 +194,9 @@ aws lambda list-functions --region us-east-1 \ Then tail the function's CloudWatch log group. Common silent skips: - `skipped_state` — the delivery was for a non-`success` status (e.g. `pending`, `in_progress`); ignore. -- `skipped_environment` — the deploy's `environment` field doesn't match `SCREENSHOT_TARGET_ENVIRONMENT`. Common cause for non-Vercel providers; see "Configuring for non-Vercel providers" above. +- `skipped_environment` (deployment statuses only) — the deploy's `environment` field doesn't match `SCREENSHOT_TARGET_ENVIRONMENT`. Common cause for non-Vercel providers; see "Configuring for non-Vercel providers" above. - `skipped_no_url` — the `success` status didn't include `environment_url`. Some providers post URL-less success events; the next push usually carries the URL. +- `screenshot.amplify_pr_rejected` — the validated PR is closed, its head SHA changed, or GitHub returned mismatched/malformed PR data. Capture stops without falling back to another PR. Rebuild the current PR preview after a new push. - `No open PR found for SHA after retries` — the deploy provider built and reported faster than the agent could `gh pr create` (race window > 35s). Rare; redeliver the webhook from GitHub's UI to retry. ### No screenshots at all: check the processor alarms and DLQ diff --git a/docs/src/content/docs/using/Jira-setup-guide.md b/docs/src/content/docs/using/Jira-setup-guide.md index 0d890cfc1..ddc12a4ba 100644 --- a/docs/src/content/docs/using/Jira-setup-guide.md +++ b/docs/src/content/docs/using/Jira-setup-guide.md @@ -317,7 +317,7 @@ After the PR exists, add a Jira comment such as `@bgagent update the README too` When a successful GitHub preview deployment is captured, ABCA also adds **Open screenshot** and **Open live preview** links to the originating Jira issue. Configure the GitHub deployment-status webhook described in the [deploy-preview screenshots guide](/sample-autonomous-cloud-coding-agents/using/deploy-preview-screenshots-guide). Jira uses explicit ADF links because these externally hosted screenshots do not have Atlassian media IDs. -For AWS Amplify Hosting, enable PR previews and select **Check runs** on that GitHub webhook. Amplify's successful preview check triggers capture and Jira delivery automatically; subscribing only to Deployment statuses does not capture Amplify previews. +For AWS Amplify Hosting, enable PR previews and select **Check runs** on that GitHub webhook. Amplify's successful preview check triggers capture and Jira delivery automatically; subscribing only to Deployment statuses does not capture Amplify previews. Existing operators can keep their branch-name `SCREENSHOT_TARGET_ENVIRONMENT` setting: validated PR checks bypass it. After redeploying ABCA and updating the subscription, rebuild a PR preview. The screenshot and Jira task lookup follow the PR validated against the preview URL and head SHA, including when multiple PRs share a commit. Rejected checks log `screenshot.amplify_check_rejected` with a reason in the receiver logs; see the screenshots guide for diagnostics. For an iteration, the links appear in its existing status comment and survive later heartbeat and terminal edits. A fan-out orchestration's combined preview appears in the parent rollup. Delivery targets the task encoded in the deploy branch and then routes through that task's stored Jira tenant/issue metadata. The Jira issue is never parsed from branch or PR text. Cross-repository delivery is rejected, and an iteration must match the deployment SHA. Duplicate deployment events update the existing preview comment or block. From ce13512918ebf408aeaa8679996a80924c3bcf96 Mon Sep 17 00:00:00 2001 From: ayushtr-aws Date: Thu, 17 Sep 2026 14:49:33 -0400 Subject: [PATCH 2/3] fix(screenshots): address Amplify preview review feedback (#900) --- cdk/src/handlers/github-webhook-processor.ts | 138 ++++++++++------ cdk/src/handlers/github-webhook.ts | 38 ++--- .../shared/github-deployment-status.ts | 40 ++++- .../handlers/github-webhook-contract.test.ts | 154 ++++++++++++++++++ .../handlers/github-webhook-processor.test.ts | 139 +++++++++++++++- cdk/test/handlers/github-webhook.test.ts | 51 ++++-- .../DEPLOY_PREVIEW_SCREENSHOTS_GUIDE.md | 23 ++- .../using/Deploy-preview-screenshots-guide.md | 23 ++- 8 files changed, 503 insertions(+), 103 deletions(-) create mode 100644 cdk/test/handlers/github-webhook-contract.test.ts diff --git a/cdk/src/handlers/github-webhook-processor.ts b/cdk/src/handlers/github-webhook-processor.ts index 14ed2af86..562b862a2 100644 --- a/cdk/src/handlers/github-webhook-processor.ts +++ b/cdk/src/handlers/github-webhook-processor.ts @@ -24,6 +24,9 @@ import { resolveGitHubToken } from './shared/context-hydration'; import { upsertTaskComment } from './shared/github-comment'; import { type GitHubDeploymentStatusPayload, + type ProcessorEvent, + type AmplifyPreviewRejectionReason, + AMPLIFY_PREVIEW_HOST, validateDeploymentStatusPayload, } from './shared/github-deployment-status'; import { renderPreviewBlock } from './shared/iteration-reply'; @@ -86,6 +89,7 @@ const POST_CAPTURE_RESERVE_MS = 30_000; * session that's already doomed. */ const MIN_CAPTURE_BUDGET_MS = 15_000; +const HTTP_REQUEST_TIMEOUT = 408; /** Backoff schedule (ms) while waiting for GitHub to link a PR to a deploy SHA. */ const PR_LOOKUP_RETRY_DELAY_0_MS = 0; @@ -99,12 +103,6 @@ const PR_LOOKUP_RETRY_DELAYS_MS = [ PR_LOOKUP_RETRY_DELAY_3_MS, ] as const; -interface ProcessorEvent { - readonly raw_body: string; - /** Set only by the receiver after Amplify check URL/PR/SHA validation. */ - readonly validated_pr_number?: number; -} - /** * Async processor for verified deployment statuses and normalized Amplify checks. * @@ -138,10 +136,8 @@ export async function handler(event: ProcessorEvent): Promise { let raw: GitHubDeploymentStatusPayload; try { raw = JSON.parse(event.raw_body) as GitHubDeploymentStatusPayload; - } catch (err) { - logger.error('GitHub webhook processor could not parse raw_body', { - error: err instanceof Error ? err.message : String(err), - }); + } catch { + logger.error('GitHub webhook processor could not parse raw_body', { reason: 'invalid_json' }); return; } @@ -171,10 +167,18 @@ export async function handler(event: ProcessorEvent): Promise { // sits outside the customer VPC, but whatever renders ends up on a // public CloudFront URL. Reject obviously-wrong shapes (non-https, // literal-IP, link-local, loopback) at the boundary. - if (!isAllowedScreenshotUrl(previewUrl)) { + let preview: URL | undefined; + try { + preview = new URL(previewUrl); + } catch { /* Rejected below without logging untrusted URL content. */ } + const amplifyHost = preview && AMPLIFY_PREVIEW_HOST.exec(preview.hostname); + if (!isAllowedScreenshotUrl(previewUrl) + || (validatedPrNumber !== undefined && (!preview || preview.username || preview.password || preview.port + || !amplifyHost || Number(amplifyHost[1]) !== validatedPrNumber))) { logger.warn('Rejected deployment_status preview URL on allowlist', { repo, - preview_url: previewUrl, + preview_host: preview?.hostname, + reason: 'untrusted_preview_url', }); return; } @@ -182,7 +186,7 @@ export async function handler(event: ProcessorEvent): Promise { logger.info('Screenshot pipeline starting', { repo, sha, - preview_url: previewUrl, + preview_host: preview?.hostname, deployment_id: deploymentId, budget_ms: TOTAL_BUDGET_MS, }); @@ -203,8 +207,18 @@ export async function handler(event: ProcessorEvent): Promise { // Retry the PR lookup, but cap by remaining budget so the screenshot // half always gets at least MIN_CAPTURE_BUDGET_MS. const prLookupBudget = Math.max(0, remaining() - POST_CAPTURE_RESERVE_MS - MIN_CAPTURE_BUDGET_MS); - const pr = await findPullRequestForShaWithRetry(repo, sha, token, prLookupBudget, validatedPrNumber); - if (!pr) { + const lookup = await findPullRequestForShaWithRetry(repo, sha, token, prLookupBudget, validatedPrNumber); + if (!lookup.ok && lookup.terminal) { + logger.warn('Validated Amplify PR no longer matches preview', { + event: 'screenshot.amplify_pr_rejected', + reason: lookup.reason, + repo, + pr_number: validatedPrNumber, + ...(lookup.status !== undefined && { status: lookup.status }), + }); + return; + } + if (!lookup.ok) { // Promote to error: "no PR after the retry budget" is the shape of // a systematic break (deploy-without-PR, token regression, GitHub // outage). At warn level it went unnoticed. Add a @@ -215,10 +229,13 @@ export async function handler(event: ProcessorEvent): Promise { repo, sha, budget_ms: prLookupBudget, + reason: lookup.reason, }); return; } + const pr = lookup.pr; + // Confirm we have enough wall-clock left to even try a capture; if // PR lookup ate the budget on a slow GitHub day, fail fast rather // than start an AgentCore session that's already doomed. @@ -242,7 +259,7 @@ export async function handler(event: ProcessorEvent): Promise { logger.error('Screenshot capture failed', { event: 'screenshot.capture_failed', error_id: 'SCREENSHOT_CAPTURE_FAILED', - preview_url: previewUrl, + preview_host: preview?.hostname, error: err instanceof Error ? err.message : String(err), }); return; @@ -535,6 +552,11 @@ interface OpenPr { readonly headRefName: string; } +type PrLookup = + | { readonly ok: true; readonly pr: OpenPr } + | { readonly ok: false; readonly terminal: true; readonly reason: AmplifyPreviewRejectionReason; readonly status?: number } + | { readonly ok: false; readonly terminal: false; readonly reason: 'fetch_failed' | 'http_error' | 'non_json_response' | 'malformed_pr_response' | 'pr_not_linked' | 'budget_exhausted' }; + /** * Wait for an open PR to exist for the given SHA, retrying with a * small backoff. Managed providers commonly post `deployment_status` @@ -545,7 +567,8 @@ interface OpenPr { * Schedule: 0s, 5s, 10s, 20s — covers the observed gap with one * generous bonus retry. Capped by `budgetMs` so the caller can hand * over only what it can afford to spend off the shared deadline. Returns - * null on exhaustion (no PR yet) or budget timeout. + * a typed failure on exhaustion or timeout. Terminal Amplify rejections stop + * immediately; only transient lookup failures enter the backoff. */ async function findPullRequestForShaWithRetry( repo: string, @@ -553,19 +576,20 @@ async function findPullRequestForShaWithRetry( token: string, budgetMs: number, validatedPrNumber?: number, -): Promise { +): Promise { const deadline = Date.now() + budgetMs; + let result: PrLookup = { ok: false, terminal: false, reason: 'budget_exhausted' }; for (let i = 0; i < PR_LOOKUP_RETRY_DELAYS_MS.length; i++) { const delay = PR_LOOKUP_RETRY_DELAYS_MS[i]; if (delay > 0) { // Skip the wait if the deadline would land mid-sleep. const remaining = deadline - Date.now(); - if (remaining <= 0) return null; + if (remaining <= 0) return result; await new Promise((r) => setTimeout(r, Math.min(delay, remaining))); } - if (Date.now() >= deadline) return null; - const pr = await findPullRequestForSha(repo, sha, token, validatedPrNumber); - if (pr) return pr; + if (Date.now() >= deadline) return result; + result = await findPullRequestForSha(repo, sha, token, validatedPrNumber); + if (result.ok || result.terminal) return result; const next = PR_LOOKUP_RETRY_DELAYS_MS[i + 1]; if (next !== undefined) { logger.info('Open PR not found yet for SHA — will retry', { @@ -576,7 +600,7 @@ async function findPullRequestForShaWithRetry( }); } } - return null; + return result; } /** @@ -585,7 +609,7 @@ async function findPullRequestForShaWithRetry( * (https://docs.github.com/rest/commits/commits#list-pull-requests-associated-with-a-commit). * * Returns the OPEN PR that the deploy is *for* (head SHA == `sha`), or - * the first open PR as a fallback, or null if none. Closed/merged PRs + * the first open PR as a fallback, or a retryable failure if none. Closed/merged PRs * are filtered out. For Amplify, only the validated PR with a matching head * is accepted; there is no fallback to another PR. */ @@ -594,7 +618,7 @@ async function findPullRequestForSha( sha: string, token: string, validatedPrNumber?: number, -): Promise { +): Promise { // A commit can head multiple PRs. Amplify's validated PR is authoritative; // fetch it directly so pagination or commit-pulls ordering cannot redirect it. const url = validatedPrNumber === undefined @@ -625,18 +649,29 @@ async function findPullRequestForSha( timed_out: ac.signal.aborted, error: err instanceof Error ? err.message : String(err), }); - return null; // nosemgrep: ts-silent-success-masking -- GitHub commit-pulls lookup is best-effort; null means "no PR for this SHA" + return { ok: false, terminal: false, reason: 'fetch_failed' }; } finally { clearTimeout(timer); } if (!res.ok) { + if (validatedPrNumber !== undefined && res.status === 404) { + return { ok: false, terminal: true, reason: 'pr_not_found' }; + } + // GitHub can report rate limiting as 403. Keep those and request timeouts + // retryable; authentication and invalid-request failures need operator action. + const rateLimited = res.status === 429 || (res.status === 403 + && (res.headers?.get('x-ratelimit-remaining') === '0' || res.headers?.has('retry-after'))); + if (validatedPrNumber !== undefined && res.status >= 400 && res.status < 500 + && res.status !== HTTP_REQUEST_TIMEOUT && !rateLimited) { + return { ok: false, terminal: true, reason: 'pr_request_rejected', status: res.status }; + } logger.warn('GitHub PR lookup returned non-2xx', { repo, sha, status: res.status, }); - return null; + return { ok: false, terminal: false, reason: 'http_error' }; } // Parse defensively: both endpoints can return an unexpected response body. @@ -644,36 +679,32 @@ async function findPullRequestForSha( let parsed: unknown; try { parsed = await res.json(); - } catch (err) { - logger.warn('GitHub PR lookup returned non-JSON body', { - repo, - sha, - error: err instanceof Error ? err.message : String(err), - }); - return null; // nosemgrep: ts-silent-success-masking -- malformed GitHub body treated as no-PR; prevents processor crash on transient HTML/502 + } catch { + logger.warn('GitHub PR lookup returned non-JSON body', { repo, sha }); + return { ok: false, terminal: false, reason: 'non_json_response' }; } if (validatedPrNumber !== undefined) { const pr = parsed as { number?: unknown; state?: unknown; title?: unknown; body?: unknown; head?: { sha?: unknown; ref?: unknown } } | null; - const reject = (reason: string): null => { - logger.warn('Validated Amplify PR no longer matches preview', { - event: 'screenshot.amplify_pr_rejected', reason, repo, pr_number: validatedPrNumber, - }); - return null; - }; - if (!pr || Array.isArray(pr) || pr.number !== validatedPrNumber) return reject('pr_number_mismatch'); + const reject = (reason: AmplifyPreviewRejectionReason): PrLookup => ({ ok: false, terminal: true, reason }); + if (!pr || typeof pr !== 'object' || Array.isArray(pr)) return reject('malformed_pr_response'); + if (pr.number !== validatedPrNumber) return reject('pr_number_mismatch'); if (pr.state !== 'open') return reject('pr_not_open'); - if (pr.head?.sha !== sha) return reject('head_sha_mismatch'); + if (pr.head?.sha !== sha) return reject('live_pr_head_sha_mismatch'); if (typeof pr.head.ref !== 'string' || !pr.head.ref) return reject('missing_head_ref'); return { - number: validatedPrNumber, - title: typeof pr.title === 'string' ? pr.title : '', - body: typeof pr.body === 'string' ? pr.body : '', - headRefName: pr.head.ref, + ok: true, + pr: { + number: validatedPrNumber, + title: typeof pr.title === 'string' ? pr.title : '', + body: typeof pr.body === 'string' ? pr.body : '', + headRefName: pr.head.ref, + }, }; } + // A non-array body would crash array operations and fault the async processor. if (!Array.isArray(parsed)) { logger.warn('GitHub commit-pulls did not return an array', { repo, sha }); - return null; + return { ok: false, terminal: false, reason: 'malformed_pr_response' }; } const pulls = parsed as Array<{ number?: number; @@ -683,7 +714,7 @@ async function findPullRequestForSha( head?: { ref?: string; sha?: string } | null; }>; const openPulls = pulls.filter((p) => p.state === 'open' && typeof p.number === 'number'); - if (openPulls.length === 0) return null; + if (openPulls.length === 0) return { ok: false, terminal: false, reason: 'pr_not_linked' }; // Prefer the PR whose own head is this SHA — the PR that introduced the // commit. For a stacked PR chain the commit-pulls API also lists every // PR stacked on top (their history contains the commit); routing reads @@ -691,10 +722,13 @@ async function findPullRequestForSha( // the first open PR for non-head SHAs (e.g. a merge/base commit). const owner = openPulls.find((p) => p.head?.sha === sha) ?? openPulls[0]; return { - number: owner.number!, - title: owner.title ?? '', - body: owner.body ?? '', - headRefName: owner.head?.ref ?? '', + ok: true, + pr: { + number: owner.number!, + title: owner.title ?? '', + body: owner.body ?? '', + headRefName: owner.head?.ref ?? '', + }, }; } diff --git a/cdk/src/handlers/github-webhook.ts b/cdk/src/handlers/github-webhook.ts index 01ac93f57..f71656161 100644 --- a/cdk/src/handlers/github-webhook.ts +++ b/cdk/src/handlers/github-webhook.ts @@ -23,6 +23,7 @@ import { DeleteCommand, PutCommand } from '@aws-sdk/lib-dynamodb'; import type { APIGatewayProxyEvent, APIGatewayProxyResult } from 'aws-lambda'; import { type GitHubDeploymentStatusPayload, + type ProcessorEvent, normalizeAmplifyPreviewCheck, validateDeploymentStatusPayload, } from './shared/github-deployment-status'; @@ -93,19 +94,23 @@ export async function handler(event: APIGatewayProxyEvent): Promise` - // Operators on non-Vercel backends override via - // `SCREENSHOT_TARGET_ENVIRONMENT` (Lambda env var, redeploy required). + // Filter deployment statuses to SCREENSHOT_TARGET_ENVIRONMENT (default + // `Preview`, matching Vercel). Amplify deployment statuses use branch names; + // GitHub Actions uses the workflow's environment; Netlify uses `Deploy Preview + // `. Operators can override the Lambda variable and redeploy. + // Validated Amplify checks already identify a PR preview and bypass this filter, + // including branch-name values that previously excluded those previews. const targetEnv = process.env.SCREENSHOT_TARGET_ENVIRONMENT ?? 'Preview'; - if (eventType === 'deployment_status' && raw.deployment?.environment !== targetEnv) { + if (!normalized && raw.deployment?.environment !== targetEnv) { return jsonResponse(200, { ok: true, skipped_environment: raw.deployment?.environment, @@ -191,14 +190,15 @@ export async function handler(event: APIGatewayProxyEvent): Promise record(pr).number === prNumber); if (previewPrs.length === 0) return reject('preview_pr_not_found'); - if (!previewPrs.some((pr: unknown) => record(record(pr).head).sha === check.head_sha)) { + const sha = check.head_sha.toLowerCase(); + if (!previewPrs.some((pr: unknown) => { + const prSha = record(record(pr).head).sha; + return typeof prSha === 'string' && prSha.toLowerCase() === sha; + })) { return reject('head_sha_mismatch'); } const repository = record(raw.repository); @@ -130,7 +154,7 @@ export function normalizeAmplifyPreviewCheck(value: unknown): AmplifyPreviewChec prNumber, payload: { repository: { full_name: repository.full_name }, - deployment: { id: check.id, sha: check.head_sha, environment: 'Preview' }, + deployment: { id: check.id, sha, environment: 'Preview' }, deployment_status: { id: check.id, state: 'success', environment_url: check.details_url }, }, }; diff --git a/cdk/test/handlers/github-webhook-contract.test.ts b/cdk/test/handlers/github-webhook-contract.test.ts new file mode 100644 index 000000000..b6e2dab68 --- /dev/null +++ b/cdk/test/handlers/github-webhook-contract.test.ts @@ -0,0 +1,154 @@ +/** + * MIT No Attribution + * + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +const s3Send = jest.fn(); +jest.mock('@aws-sdk/client-s3', () => ({ + S3Client: jest.fn(() => ({ send: s3Send })), + PutObjectCommand: jest.fn((input: unknown) => ({ _type: 'Put', input })), +})); + +// DynamoDB doc client — drives persistScreenshotUrl. +const ddbSend = jest.fn(); +jest.mock('@aws-sdk/client-dynamodb', () => ({ + DynamoDBClient: jest.fn(() => ({})), + ConditionalCheckFailedException: class extends Error {}, +})); +jest.mock('@aws-sdk/lib-dynamodb', () => ({ + DynamoDBDocumentClient: { from: jest.fn(() => ({ send: ddbSend })) }, + PutCommand: jest.fn((input: unknown) => ({ _type: 'Put', input })), + DeleteCommand: jest.fn((input: unknown) => ({ _type: 'Delete', input })), + UpdateCommand: jest.fn((input: unknown) => ({ _type: 'Update', input })), + QueryCommand: jest.fn((input: unknown) => ({ _type: 'Query', input })), + GetCommand: jest.fn((input: unknown) => ({ _type: 'Get', input })), +})); + +const captureScreenshotMock = jest.fn(); +jest.mock('../../src/handlers/shared/agentcore-browser', () => ({ + captureScreenshot: (...args: unknown[]) => captureScreenshotMock(...args), +})); + +const resolveGitHubTokenMock = jest.fn(); +jest.mock('../../src/handlers/shared/context-hydration', () => ({ + resolveGitHubToken: (...args: unknown[]) => resolveGitHubTokenMock(...args), +})); + +const upsertTaskCommentMock = jest.fn(); +jest.mock('../../src/handlers/shared/github-comment', () => ({ + upsertTaskComment: (...args: unknown[]) => upsertTaskCommentMock(...args), +})); + +const postIssueCommentMock = jest.fn(); +jest.mock('../../src/handlers/shared/linear-feedback', () => ({ + postIssueComment: (...args: unknown[]) => postIssueCommentMock(...args), +})); + +const findLinearIssueMock = jest.fn(); +const extractLinearIdentifierMock = jest.fn(); +const extractFromBranchMock = jest.fn(); +jest.mock('../../src/handlers/shared/linear-issue-lookup', () => ({ + findLinearIssueByIdentifier: (...args: unknown[]) => findLinearIssueMock(...args), + extractLinearIdentifier: (...args: unknown[]) => extractLinearIdentifierMock(...args), + extractLinearIdentifierFromBranch: (...args: unknown[]) => extractFromBranchMock(...args), +})); + +const deliverJiraMock = jest.fn(); +jest.mock('../../src/handlers/shared/jira-deployment-preview', () => ({ + deliverJiraDeploymentPreview: (...args: unknown[]) => deliverJiraMock(...args), +})); + +const lambdaSend = jest.fn(); +jest.mock('@aws-sdk/client-lambda', () => ({ + LambdaClient: jest.fn(() => ({ send: lambdaSend })), + InvokeCommand: jest.fn((input: unknown) => ({ input })), +})); +jest.mock('../../src/handlers/shared/github-webhook-verify', () => ({ + verifyGitHubRequest: jest.fn().mockResolvedValue(true), +})); + +process.env.SCREENSHOT_BUCKET_NAME = 'screenshots'; +process.env.SCREENSHOT_PUBLIC_HOST = 'd1.cloudfront.net'; +process.env.GITHUB_TOKEN_SECRET_ARN = 'gh-token'; +process.env.GITHUB_WEBHOOK_SECRET_ARN = 'webhook-secret'; +process.env.GITHUB_WEBHOOK_DEDUP_TABLE_NAME = 'dedup'; +process.env.GITHUB_WEBHOOK_PROCESSOR_FUNCTION_NAME = 'processor'; + +import type { APIGatewayProxyEvent } from 'aws-lambda'; +import { handler as receiverHandler } from '../../src/handlers/github-webhook'; +import { handler as processorHandler } from '../../src/handlers/github-webhook-processor'; + +const sha = 'a'.repeat(40); +const pr41 = { number: 41, state: 'open', title: 'other PR', head: { sha, ref: 'other' } }; +const pr42 = { number: 42, state: 'open', title: 'preview PR', head: { sha, ref: 'preview' } }; + +beforeEach(() => { + jest.restoreAllMocks(); + jest.clearAllMocks(); + ddbSend.mockResolvedValue({}); + lambdaSend.mockResolvedValue({}); + s3Send.mockResolvedValue({}); + resolveGitHubTokenMock.mockResolvedValue('token'); + captureScreenshotMock.mockResolvedValue(Buffer.from('png')); + upsertTaskCommentMock.mockResolvedValue({ commentId: 1 }); +}); + +test.each(['check_run', 'deployment_status'])('receiver %s payload drives the processor to the correct PR', async (eventType) => { + const body = eventType === 'check_run' ? { + action: 'completed', + repository: { full_name: 'owner/repo' }, + check_run: { + id: 123, + name: 'AWS Amplify Console Web Preview', + status: 'completed', + conclusion: 'success', + head_sha: sha.toUpperCase(), + details_url: 'https://pr-42.app123.amplifyapp.com', + app: { slug: 'aws-amplify-us-east-1', owner: { login: 'aws-amplify-console' } }, + pull_requests: [pr41, pr42], + }, + } : { + repository: { full_name: 'owner/repo' }, + deployment: { id: 123, sha, environment: 'Preview' }, + deployment_status: { id: 456, state: 'success', environment_url: 'https://preview.vercel.app' }, + }; + const fetchMock = jest.spyOn(global, 'fetch').mockImplementation(async (url) => ({ + ok: true, + status: 200, + json: async () => String(url).endsWith('/pulls/42') ? pr42 : [pr41, pr42], + } as Response)); + + const response = await receiverHandler({ + body: JSON.stringify(body), + headers: { 'X-GitHub-Event': eventType, 'X-Hub-Signature-256': 'sha256=verified-by-mock' }, + } as unknown as APIGatewayProxyEvent); + expect(response.statusCode).toBe(200); + expect(lambdaSend).toHaveBeenCalledTimes(1); + // Pass the actual bytes emitted by the receiver without rebuilding any fields. + const forwarded = JSON.parse(new TextDecoder().decode(lambdaSend.mock.calls[0][0].input.Payload)); + if (eventType === 'deployment_status') expect(forwarded).toEqual({ raw_body: JSON.stringify(body) }); + await processorHandler(forwarded); + + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(fetchMock).toHaveBeenCalledWith(eventType === 'check_run' + ? 'https://api.github.com/repos/owner/repo/pulls/42' + : `https://api.github.com/repos/owner/repo/commits/${sha}/pulls`, expect.anything()); + expect(captureScreenshotMock).toHaveBeenCalledTimes(1); + expect(upsertTaskCommentMock).toHaveBeenCalledWith(expect.objectContaining({ + repo: 'owner/repo', issueOrPrNumber: eventType === 'check_run' ? 42 : 41, + })); +}); diff --git a/cdk/test/handlers/github-webhook-processor.test.ts b/cdk/test/handlers/github-webhook-processor.test.ts index 14a789042..47dc334bc 100644 --- a/cdk/test/handlers/github-webhook-processor.test.ts +++ b/cdk/test/handlers/github-webhook-processor.test.ts @@ -67,7 +67,12 @@ jest.mock('../../src/handlers/shared/jira-deployment-preview', () => ({ deliverJiraDeploymentPreview: (...args: unknown[]) => deliverJiraMock(...args), })); -process.env.JIRA_WORKSPACE_REGISTRY_TABLE_NAME = 'JiraRegistry'; +const originalJiraRegistry = process.env.JIRA_WORKSPACE_REGISTRY_TABLE_NAME; +beforeEach(() => { process.env.JIRA_WORKSPACE_REGISTRY_TABLE_NAME = 'JiraRegistry'; }); +afterEach(() => { + if (originalJiraRegistry === undefined) delete process.env.JIRA_WORKSPACE_REGISTRY_TABLE_NAME; + else process.env.JIRA_WORKSPACE_REGISTRY_TABLE_NAME = originalJiraRegistry; +}); process.env.SCREENSHOT_BUCKET_NAME = 'screenshot-bucket'; process.env.SCREENSHOT_PUBLIC_HOST = 'd1.cloudfront.net'; process.env.GITHUB_TOKEN_SECRET_ARN = 'arn:aws:secretsmanager:us-east-1:123:secret:gh-token'; @@ -562,14 +567,17 @@ describe('validated Amplify PR routing', () => { test.each([ [pr41, 'pr_number_mismatch'], [{ ...pr42, state: 'closed' }, 'pr_not_open'], - [{ ...pr42, head: { ref: branch, sha: 'b'.repeat(40) } }, 'head_sha_mismatch'], + [{ ...pr42, head: { ref: branch, sha: 'b'.repeat(40) } }, 'live_pr_head_sha_mismatch'], [{ ...pr42, head: { sha } }, 'missing_head_ref'], - [null, 'pr_number_mismatch'], - [[pr41, pr42], 'pr_number_mismatch'], + [null, 'malformed_pr_response'], + [[pr41, pr42], 'malformed_pr_response'], + ['invalid', 'malformed_pr_response'], ])('rejects a changed or malformed PR without capturing or falling back: %j', async (pr, reason) => { jest.useFakeTimers(); try { const log = jest.spyOn(logger, 'warn'); + const error = jest.spyOn(logger, 'error'); + const start = Date.now(); const fetchMock = jest.spyOn(global, 'fetch').mockResolvedValue({ ok: true, status: 200, json: async () => pr } as Response); const pending = handler(amplifyEvent()); await jest.runAllTimersAsync(); @@ -577,7 +585,11 @@ describe('validated Amplify PR routing', () => { expect(log).toHaveBeenCalledWith('Validated Amplify PR no longer matches preview', { event: 'screenshot.amplify_pr_rejected', reason, repo: 'owner/repo', pr_number: 42, }); - expect(fetchMock.mock.calls.every(([url]) => String(url).endsWith('/pulls/42'))).toBe(true); + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(fetchMock).toHaveBeenCalledWith('https://api.github.com/repos/owner/repo/pulls/42', expect.anything()); + expect(log).toHaveBeenCalledTimes(1); + expect(error).not.toHaveBeenCalled(); + expect(Date.now()).toBe(start); expect(captureScreenshotMock).not.toHaveBeenCalled(); expect(ddbSend).not.toHaveBeenCalled(); expect(upsertTaskCommentMock).not.toHaveBeenCalled(); @@ -588,6 +600,123 @@ describe('validated Amplify PR routing', () => { } }); + test.each([404, 400, 401, 403, 422])('HTTP %s stops after one request with one reason and no exhausted error', async (status) => { + jest.useFakeTimers(); + try { + const warn = jest.spyOn(logger, 'warn'); + const error = jest.spyOn(logger, 'error'); + const fetchMock = fetchOk({}, status); + const start = Date.now(); + const pending = handler(amplifyEvent()); + await jest.runAllTimersAsync(); + await pending; + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(warn).toHaveBeenCalledTimes(1); + expect(warn).toHaveBeenCalledWith(expect.any(String), { + event: 'screenshot.amplify_pr_rejected', + reason: status === 404 ? 'pr_not_found' : 'pr_request_rejected', + repo: 'owner/repo', + pr_number: 42, + ...(status !== 404 && { status }), + }); + expect(error).not.toHaveBeenCalled(); + expect(Date.now()).toBe(start); + expect(captureScreenshotMock).not.toHaveBeenCalled(); + expect(upsertTaskCommentMock).not.toHaveBeenCalled(); + } finally { + jest.useRealTimers(); + } + }); + + test.each(['fetch error', 'timeout', '5xx', 'non-JSON', '403 quota', '403 retry-after', '408', '429'])('retries transient %s and captures the same PR after recovery', async (failure) => { + jest.useFakeTimers(); + try { + const fetchMock = jest.spyOn(global, 'fetch'); + if (failure === 'fetch error') { + fetchMock.mockRejectedValueOnce(new Error('network unavailable')); + } else if (failure === 'timeout') { + fetchMock.mockImplementationOnce((_url, options) => new Promise((_resolve, reject) => { + options?.signal?.addEventListener('abort', () => reject(new Error('aborted'))); + })); + } else if (failure === '5xx') { + fetchMock.mockResolvedValueOnce({ ok: false, status: 503 } as Response); + } else if (failure === '403 quota' || failure === '403 retry-after') { + fetchMock.mockResolvedValueOnce({ + ok: false, + status: 403, + headers: new Headers(failure === '403 quota' ? { 'x-ratelimit-remaining': '0' } : { 'retry-after': '5' }), + } as Response); + } else if (failure === '408' || failure === '429') { + fetchMock.mockResolvedValueOnce({ ok: false, status: Number(failure) } as Response); + } else { + fetchMock.mockResolvedValueOnce({ ok: true, json: async () => { throw new SyntaxError('secret-response-fragment'); } } as unknown as Response); + } + fetchMock.mockResolvedValueOnce({ ok: true, json: async () => pr42 } as Response); + const error = jest.spyOn(logger, 'error'); + const warn = jest.spyOn(logger, 'warn'); + const pending = handler(amplifyEvent()); + await jest.runAllTimersAsync(); + await pending; + expect(fetchMock).toHaveBeenCalledTimes(2); + for (const [url] of fetchMock.mock.calls) expect(url).toBe('https://api.github.com/repos/owner/repo/pulls/42'); + expect(captureScreenshotMock).toHaveBeenCalledTimes(1); + expect(upsertTaskCommentMock).toHaveBeenCalledWith(expect.objectContaining({ issueOrPrNumber: 42 })); + expect(error).not.toHaveBeenCalled(); + expect(JSON.stringify(warn.mock.calls)).not.toContain('secret-response-fragment'); + } finally { + jest.useRealTimers(); + } + }); + + test('exhausted transient Amplify failures retain the operational error', async () => { + jest.useFakeTimers(); + try { + const fetchMock = jest.spyOn(global, 'fetch').mockResolvedValue({ ok: false, status: 503 } as Response); + const error = jest.spyOn(logger, 'error'); + const pending = handler(amplifyEvent()); + await jest.runAllTimersAsync(); + await pending; + expect(fetchMock).toHaveBeenCalledTimes(4); + expect(error).toHaveBeenCalledWith(expect.any(String), expect.objectContaining({ + error_id: 'SCREENSHOT_PR_LOOKUP_EXHAUSTED', reason: 'http_error', + })); + expect(captureScreenshotMock).not.toHaveBeenCalled(); + } finally { + jest.useRealTimers(); + } + }); + + test('coerces non-string PR title/body before Linear identifier lookup', async () => { + fetchOk({ ...pr42, title: {}, body: 42 }); + ddbSend.mockResolvedValue({ Attributes: {} }); + extractLinearIdentifierMock.mockReset().mockReturnValue(null); + extractFromBranchMock.mockReturnValue(null); + await handler(amplifyEvent()); + expect(extractLinearIdentifierMock).toHaveBeenCalledTimes(2); + expect(extractLinearIdentifierMock).toHaveBeenNthCalledWith(1, ''); + expect(extractLinearIdentifierMock).toHaveBeenNthCalledWith(2, ''); + expect(upsertTaskCommentMock).toHaveBeenCalledTimes(1); + }); + + test.each([ + 'https://preview.example.com/path?token=secret', + 'https://pr-41.app123.amplifyapp.com/path?token=secret', + 'https://user:secret@pr-42.app123.amplifyapp.com', + 'https://pr-42.app123.amplifyapp.com:8080/path?token=secret', + 'http://pr-42.app123.amplifyapp.com/path?token=secret', + ])('revalidates the forwarded Amplify URL without logging secrets: %s', async (url) => { + const warn = jest.spyOn(logger, 'warn'); + const forwarded = amplifyEvent(); + const raw = JSON.parse(forwarded.raw_body); + raw.deployment_status.environment_url = url; + await handler({ ...forwarded, raw_body: JSON.stringify(raw) }); + expect(resolveGitHubTokenMock).not.toHaveBeenCalled(); + expect(captureScreenshotMock).not.toHaveBeenCalled(); + expect(warn).toHaveBeenCalledWith(expect.any(String), expect.objectContaining({ reason: 'untrusted_preview_url' })); + expect(JSON.stringify(warn.mock.calls)).not.toContain('secret'); + expect(JSON.stringify(warn.mock.calls)).not.toContain(url); + }); + test.each([0, -1, 1.5, Number.MAX_SAFE_INTEGER + 1])('rejects invalid forwarded PR number %s', async (number) => { await handler({ ...amplifyEvent(), validated_pr_number: number }); expect(resolveGitHubTokenMock).not.toHaveBeenCalled(); diff --git a/cdk/test/handlers/github-webhook.test.ts b/cdk/test/handlers/github-webhook.test.ts index 62ce9621e..e5a06bed9 100644 --- a/cdk/test/handlers/github-webhook.test.ts +++ b/cdk/test/handlers/github-webhook.test.ts @@ -92,7 +92,7 @@ function deploymentStatusBody(overrides: { } const amplifySha = '6a19dae554d1f33615a468a95c0035e05c41de3e'; -function amplifyBody(overrides: Record = {}): string { +function amplifyBody(overrides: Record = {}, envelope: Record = {}): string { return JSON.stringify({ action: 'completed', repository: { full_name: 'owner/repo' }, @@ -107,6 +107,7 @@ function amplifyBody(overrides: Record = {}): string { pull_requests: [{ number: 41, head: { sha: amplifySha } }], ...overrides, }, + ...envelope, }); } @@ -218,7 +219,7 @@ describe('github-webhook receiver', () => { // Forwarded payload preserves the raw body verbatim. const invokeArg = (lambdaSend.mock.calls[0][0] as { input: { Payload: Uint8Array } }).input; const decoded = JSON.parse(new TextDecoder().decode(invokeArg.Payload)); - expect(decoded.raw_body).toBeDefined(); + expect(decoded).toEqual({ raw_body: deploymentStatusBody() }); }); test('rolls back the dedup row when processor invoke fails', async () => { @@ -278,7 +279,7 @@ describe('github-webhook receiver', () => { [{ details_url: 'not-a-url' }, 'invalid_details_url'], [{ details_url: null }, 'invalid_details_url'], [{ details_url: 'https://pr-0.app.amplifyapp.com' }, 'untrusted_preview_url'], - [{ details_url: 'https://pr-9007199254740992.app.amplifyapp.com' }, 'invalid_preview_pr_number'], + [{ details_url: 'https://pr-9007199254740992.app.amplifyapp.com' }, 'invalid_pr_number'], [{ pull_requests: null }, 'invalid_pull_requests'], [{ pull_requests: [] }, 'preview_pr_not_found'], [{ pull_requests: [null, {}, { number: 42, head: { sha: amplifySha } }] }, 'preview_pr_not_found'], @@ -309,14 +310,39 @@ describe('github-webhook receiver', () => { }, ); - test('ignores a non-completed check action and an invalid repository', async () => { - const body = JSON.parse(amplifyBody()); - body.action = 'rerequested'; - await handler(event(JSON.stringify(body), { 'X-GitHub-Event': 'check_run' })); - body.action = 'completed'; - body.repository.full_name = '../invalid/repo'; - await handler(event(JSON.stringify(body), { 'X-GitHub-Event': 'check_run' })); + test.each([ + [{ action: 'rerequested' }, 'action_not_completed'], + [{ repository: { full_name: '../invalid/repo' } }, 'invalid_repository'], + [{ repository: null }, 'invalid_repository'], + ])('returns a specific reason for rejected envelopes: %j', async (envelope, reason) => { + const response = await handler(event(amplifyBody({}, envelope), { 'X-GitHub-Event': 'check_run' })); + expect(JSON.parse(response.body)).toEqual({ ok: true, skipped_check: true, reason }); expect(lambdaSend).not.toHaveBeenCalled(); + expect(ddbSend).not.toHaveBeenCalled(); + }); + + test.each([amplifySha, amplifySha.toUpperCase()])('normalizes mixed-case check and PR SHAs: %s', async (prSha) => { + await handler(event(amplifyBody({ head_sha: amplifySha.toUpperCase(), pull_requests: [{ number: 41, head: { sha: prSha } }] }), { 'X-GitHub-Event': 'check_run' })); + expect(lambdaSend).toHaveBeenCalledTimes(1); + const forwarded = JSON.parse(new TextDecoder().decode(lambdaSend.mock.calls[0][0].input.Payload)); + expect(JSON.parse(forwarded.raw_body).deployment.sha).toBe(amplifySha); + }); + + test.each(['X-GitHub-Delivery', 'x-github-delivery'])('correlates rejected checks with a validated %s', async (header) => { + const log = jest.spyOn(logger, 'info'); + const delivery = '12345678-abcd-1234-abcd-123456789abc'; + await handler(event(amplifyBody({ app: {} }), { 'X-GitHub-Event': 'check_run', [header]: delivery })); + expect(log).toHaveBeenCalledWith('Amplify preview check rejected', { + event: 'screenshot.amplify_check_rejected', reason: 'unexpected_app_owner', delivery_id: delivery, + }); + }); + + test('does not log untrusted delivery header content', async () => { + const log = jest.spyOn(logger, 'info'); + await handler(event(amplifyBody({ app: {} }), { 'X-GitHub-Event': 'check_run', 'X-GitHub-Delivery': 'secret-token' })); + expect(log).toHaveBeenCalledWith('Amplify preview check rejected', { + event: 'screenshot.amplify_check_rejected', reason: 'unexpected_app_owner', + }); }); test('deduplicates redelivered Amplify completions', async () => { @@ -352,7 +378,6 @@ describe('github-webhook receiver', () => { test('deployment statuses and Amplify checks with identical IDs deduplicate independently', async () => { const keys = new Set(); ddbSend.mockImplementation(async ({ input }) => { - expect(input.ConditionExpression).toBe('attribute_not_exists(dedup_key)'); const key = input.Item.dedup_key; if (keys.has(key)) throw new FakeConditionalCheckFailedException(); keys.add(key); @@ -366,6 +391,10 @@ describe('github-webhook receiver', () => { expect(JSON.parse((await handler(check)).body).deduped).toBe(true); expect(JSON.parse((await handler(deployment)).body).deduped).toBe(true); expect(lambdaSend).toHaveBeenCalledTimes(2); + expect(ddbSend).toHaveBeenCalledTimes(4); + for (const [command] of ddbSend.mock.calls) { + expect(command.input.ConditionExpression).toBe('attribute_not_exists(dedup_key)'); + } }); test('invalid check JSON logs a static reason without including body fragments', async () => { diff --git a/docs/guides/DEPLOY_PREVIEW_SCREENSHOTS_GUIDE.md b/docs/guides/DEPLOY_PREVIEW_SCREENSHOTS_GUIDE.md index 958f1faa4..99885b57d 100644 --- a/docs/guides/DEPLOY_PREVIEW_SCREENSHOTS_GUIDE.md +++ b/docs/guides/DEPLOY_PREVIEW_SCREENSHOTS_GUIDE.md @@ -25,7 +25,7 @@ If your provider gives you that, you're done. The example below is Vercel becaus For Amplify, enable **Hosting → Previews** on the PR's target branch and add **Check runs** to the repository's ABCA webhook events. Amplify publishes the URL in the completed check's `details_url`; a green GitHub check alone will not trigger capture if the webhook only subscribes to Deployment statuses. ABCA accepts successful preview checks from the `aws-amplify-console` app owner with a matching PR number, head SHA, and HTTPS `pr-..amplifyapp.com` URL. No manual deployment event or extra GitHub Actions workflow is needed. The processor fetches that exact PR and confirms it is still open with the same head SHA before capture, so two PRs sharing a commit cannot redirect the screenshot or Jira/Linear feedback. -**Existing Amplify operators:** redeploy ABCA to pick up this receiver and processor, then add **Check runs** to your existing webhook while keeping **Deployment statuses** selected. Keep any branch-name `SCREENSHOT_TARGET_ENVIRONMENT` value (for example, `main`): deployment statuses still use it, while validated Amplify PR checks bypass it. You do not need to change it to `Preview`. Subscriptions affect future events only; rebuild an existing PR preview to verify the change. If an earlier receiver already accepted and deduplicated a completion, replaying that same check within the one-hour dedup window will not capture again. +**Existing Amplify operators:** redeploy ABCA to pick up this receiver and processor, then add **Check runs** to your existing webhook while keeping **Deployment statuses** selected. Keep any branch-name `SCREENSHOT_TARGET_ENVIRONMENT` value (for example, `main`): deployment statuses still use it, while validated Amplify PR checks bypass it. After redeploy, a webhook already subscribed to **Check runs** will start capturing and publishing Amplify PR previews that a branch-name filter previously excluded, with no further configuration change; screenshots are publicly readable through CloudFront and may contain customer data rendered by the preview. To keep Amplify check-triggered capture disabled, deselect **Check runs** on the ABCA webhook. You do not need to change the environment filter to `Preview`. Subscriptions affect future events only; rebuild an existing PR preview to verify the change. If an earlier receiver already accepted and deduplicated a completion, replaying that same check within the one-hour dedup window will not capture again. ## What you get @@ -165,7 +165,7 @@ The pipeline filters `deployment_status` webhooks against `SCREENSHOT_TARGET_ENV ### Webhook delivers 200 but no screenshot lands -For Amplify, confirm **Check runs** is selected on the ABCA webhook, the `AWS Amplify Console Web Preview` check completed successfully, and its details link opens the PR preview. `skipped_check` means the event was not an eligible successful Amplify PR preview. Its `reason` is also logged by the receiver as `screenshot.amplify_check_rejected`, without the raw payload or URL. Adding the subscription only affects future events; rebuild an existing preview to exercise the automatic path. +For Amplify, confirm **Check runs** is selected on the ABCA webhook, the `AWS Amplify Console Web Preview` check completed successfully, and its details link opens the PR preview. `skipped_check` means the event was not an eligible successful Amplify PR preview. Its `reason` is also logged by the receiver as `screenshot.amplify_check_rejected`, without the raw payload or URL. A valid `X-GitHub-Delivery` UUID is logged as `delivery_id` so you can locate the event in GitHub. Expected incomplete or unrelated checks retain info-level reason logs because every rejected check must be diagnosable. Adding the subscription only affects future events; rebuild an existing preview to exercise the automatic path. Inspect the receiver logs for rejected checks: @@ -173,7 +173,7 @@ Inspect the receiver logs for rejected checks: |---|---| | `action_not_completed`, `check_not_completed`, `check_not_successful` | Wait for a successful completed check. | | `unexpected_check_name`, `unexpected_app_owner`, `unexpected_app_slug` | The check must be `AWS Amplify Console Web Preview`, owned by `aws-amplify-console`, with an `aws-amplify-*` app slug. Other CI checks are ignored. | -| `invalid_details_url`, `untrusted_preview_url`, `invalid_preview_pr_number` | The details link must be a trusted HTTPS `pr-N..amplifyapp.com` preview URL with a positive PR number, no credentials, and no non-default port. | +| `invalid_details_url`, `untrusted_preview_url`, `invalid_pr_number` | The details link must be a trusted HTTPS `pr-N..amplifyapp.com` preview URL with a positive PR number, no credentials, and no non-default port. | | `invalid_pull_requests`, `preview_pr_not_found`, `head_sha_mismatch` | The check must list the preview PR with the same head SHA as the check. | | `invalid_payload`, `invalid_check_id`, `invalid_head_sha`, `invalid_repository` | The webhook payload is malformed; inspect the delivery in GitHub. | @@ -192,9 +192,24 @@ Then tail the function's CloudWatch log group. Common silent skips: - `skipped_state` — the delivery was for a non-`success` status (e.g. `pending`, `in_progress`); ignore. - `skipped_environment` (deployment statuses only) — the deploy's `environment` field doesn't match `SCREENSHOT_TARGET_ENVIRONMENT`. Common cause for non-Vercel providers; see "Configuring for non-Vercel providers" above. - `skipped_no_url` — the `success` status didn't include `environment_url`. Some providers post URL-less success events; the next push usually carries the URL. -- `screenshot.amplify_pr_rejected` — the validated PR is closed, its head SHA changed, or GitHub returned mismatched/malformed PR data. Capture stops without falling back to another PR. Rebuild the current PR preview after a new push. +- `screenshot.amplify_pr_rejected` — a terminal rejection of the validated PR, logged once at warn level. Capture stops immediately without retrying, falling back to another PR, or emitting `SCREENSHOT_PR_LOOKUP_EXHAUSTED`. See the reasons below. - `No open PR found for SHA after retries` — the deploy provider built and reported faster than the agent could `gh pr create` (race window > 35s). Rare; redeliver the webhook from GitHub's UI to retry. +Processor-side Amplify reasons: + +| Reason | What to check | +|---|---| +| `invalid_pr_number` | The forwarded PR number must be a positive safe integer. Deploy the receiver and processor together. | +| `pr_not_found` | GitHub returned 404 for the validated PR. Confirm the PR exists and the GitHub token can access it; GitHub also uses 404 to conceal inaccessible resources. | +| `pr_request_rejected` | GitHub rejected the PR lookup with a non-retryable 4xx response. Inspect the logged `status`, token permissions, and request. | +| `pr_number_mismatch` | GitHub returned a different PR number than the validated preview. Inspect the PR lookup response. | +| `pr_not_open` | The PR closed or merged while Amplify built the preview. No capture is needed. | +| `live_pr_head_sha_mismatch` | The live PR head changed after the check started. Rebuild the current preview. Receiver code `head_sha_mismatch` instead compares the check's embedded PR head with its own SHA. | +| `missing_head_ref` | GitHub returned the expected PR and SHA without a usable branch name. Inspect the PR response. | +| `malformed_pr_response` | GitHub returned a null, array, or non-object PR body. Inspect the API response. | + +The processor also rechecks the Amplify URL's HTTPS origin, credentials, port, and PR number before requesting a token or capturing; rejection logs use `untrusted_preview_url` and only the hostname. Fetch failures, timeouts, non-JSON bodies, 5xx responses, HTTP 408/429, and 403 responses carrying rate-limit headers retain bounded retries. Exhaustion logs `SCREENSHOT_PR_LOOKUP_EXHAUSTED` with the final failure reason. After resolving a transient failure, rebuild the preview or wait for the one-hour dedup window before replaying the same check. + ### No screenshots at all: check the processor alarms and DLQ The receiver Lambda async-invokes the processor (`InvocationType: Event`) and returns `200` to GitHub as soon as that invoke is accepted, so a *processor*-side fault never propagates back — GitHub sees success and never redelivers. (Only a failure to even enqueue the invoke returns `500`.) Two operator-visible signals catch a hard processor fault that would otherwise stop screenshots silently. diff --git a/docs/src/content/docs/using/Deploy-preview-screenshots-guide.md b/docs/src/content/docs/using/Deploy-preview-screenshots-guide.md index 37316e946..265d45ff5 100644 --- a/docs/src/content/docs/using/Deploy-preview-screenshots-guide.md +++ b/docs/src/content/docs/using/Deploy-preview-screenshots-guide.md @@ -29,7 +29,7 @@ If your provider gives you that, you're done. The example below is Vercel becaus For Amplify, enable **Hosting → Previews** on the PR's target branch and add **Check runs** to the repository's ABCA webhook events. Amplify publishes the URL in the completed check's `details_url`; a green GitHub check alone will not trigger capture if the webhook only subscribes to Deployment statuses. ABCA accepts successful preview checks from the `aws-amplify-console` app owner with a matching PR number, head SHA, and HTTPS `pr-..amplifyapp.com` URL. No manual deployment event or extra GitHub Actions workflow is needed. The processor fetches that exact PR and confirms it is still open with the same head SHA before capture, so two PRs sharing a commit cannot redirect the screenshot or Jira/Linear feedback. -**Existing Amplify operators:** redeploy ABCA to pick up this receiver and processor, then add **Check runs** to your existing webhook while keeping **Deployment statuses** selected. Keep any branch-name `SCREENSHOT_TARGET_ENVIRONMENT` value (for example, `main`): deployment statuses still use it, while validated Amplify PR checks bypass it. You do not need to change it to `Preview`. Subscriptions affect future events only; rebuild an existing PR preview to verify the change. If an earlier receiver already accepted and deduplicated a completion, replaying that same check within the one-hour dedup window will not capture again. +**Existing Amplify operators:** redeploy ABCA to pick up this receiver and processor, then add **Check runs** to your existing webhook while keeping **Deployment statuses** selected. Keep any branch-name `SCREENSHOT_TARGET_ENVIRONMENT` value (for example, `main`): deployment statuses still use it, while validated Amplify PR checks bypass it. After redeploy, a webhook already subscribed to **Check runs** will start capturing and publishing Amplify PR previews that a branch-name filter previously excluded, with no further configuration change; screenshots are publicly readable through CloudFront and may contain customer data rendered by the preview. To keep Amplify check-triggered capture disabled, deselect **Check runs** on the ABCA webhook. You do not need to change the environment filter to `Preview`. Subscriptions affect future events only; rebuild an existing PR preview to verify the change. If an earlier receiver already accepted and deduplicated a completion, replaying that same check within the one-hour dedup window will not capture again. ## What you get @@ -169,7 +169,7 @@ The pipeline filters `deployment_status` webhooks against `SCREENSHOT_TARGET_ENV ### Webhook delivers 200 but no screenshot lands -For Amplify, confirm **Check runs** is selected on the ABCA webhook, the `AWS Amplify Console Web Preview` check completed successfully, and its details link opens the PR preview. `skipped_check` means the event was not an eligible successful Amplify PR preview. Its `reason` is also logged by the receiver as `screenshot.amplify_check_rejected`, without the raw payload or URL. Adding the subscription only affects future events; rebuild an existing preview to exercise the automatic path. +For Amplify, confirm **Check runs** is selected on the ABCA webhook, the `AWS Amplify Console Web Preview` check completed successfully, and its details link opens the PR preview. `skipped_check` means the event was not an eligible successful Amplify PR preview. Its `reason` is also logged by the receiver as `screenshot.amplify_check_rejected`, without the raw payload or URL. A valid `X-GitHub-Delivery` UUID is logged as `delivery_id` so you can locate the event in GitHub. Expected incomplete or unrelated checks retain info-level reason logs because every rejected check must be diagnosable. Adding the subscription only affects future events; rebuild an existing preview to exercise the automatic path. Inspect the receiver logs for rejected checks: @@ -177,7 +177,7 @@ Inspect the receiver logs for rejected checks: |---|---| | `action_not_completed`, `check_not_completed`, `check_not_successful` | Wait for a successful completed check. | | `unexpected_check_name`, `unexpected_app_owner`, `unexpected_app_slug` | The check must be `AWS Amplify Console Web Preview`, owned by `aws-amplify-console`, with an `aws-amplify-*` app slug. Other CI checks are ignored. | -| `invalid_details_url`, `untrusted_preview_url`, `invalid_preview_pr_number` | The details link must be a trusted HTTPS `pr-N..amplifyapp.com` preview URL with a positive PR number, no credentials, and no non-default port. | +| `invalid_details_url`, `untrusted_preview_url`, `invalid_pr_number` | The details link must be a trusted HTTPS `pr-N..amplifyapp.com` preview URL with a positive PR number, no credentials, and no non-default port. | | `invalid_pull_requests`, `preview_pr_not_found`, `head_sha_mismatch` | The check must list the preview PR with the same head SHA as the check. | | `invalid_payload`, `invalid_check_id`, `invalid_head_sha`, `invalid_repository` | The webhook payload is malformed; inspect the delivery in GitHub. | @@ -196,9 +196,24 @@ Then tail the function's CloudWatch log group. Common silent skips: - `skipped_state` — the delivery was for a non-`success` status (e.g. `pending`, `in_progress`); ignore. - `skipped_environment` (deployment statuses only) — the deploy's `environment` field doesn't match `SCREENSHOT_TARGET_ENVIRONMENT`. Common cause for non-Vercel providers; see "Configuring for non-Vercel providers" above. - `skipped_no_url` — the `success` status didn't include `environment_url`. Some providers post URL-less success events; the next push usually carries the URL. -- `screenshot.amplify_pr_rejected` — the validated PR is closed, its head SHA changed, or GitHub returned mismatched/malformed PR data. Capture stops without falling back to another PR. Rebuild the current PR preview after a new push. +- `screenshot.amplify_pr_rejected` — a terminal rejection of the validated PR, logged once at warn level. Capture stops immediately without retrying, falling back to another PR, or emitting `SCREENSHOT_PR_LOOKUP_EXHAUSTED`. See the reasons below. - `No open PR found for SHA after retries` — the deploy provider built and reported faster than the agent could `gh pr create` (race window > 35s). Rare; redeliver the webhook from GitHub's UI to retry. +Processor-side Amplify reasons: + +| Reason | What to check | +|---|---| +| `invalid_pr_number` | The forwarded PR number must be a positive safe integer. Deploy the receiver and processor together. | +| `pr_not_found` | GitHub returned 404 for the validated PR. Confirm the PR exists and the GitHub token can access it; GitHub also uses 404 to conceal inaccessible resources. | +| `pr_request_rejected` | GitHub rejected the PR lookup with a non-retryable 4xx response. Inspect the logged `status`, token permissions, and request. | +| `pr_number_mismatch` | GitHub returned a different PR number than the validated preview. Inspect the PR lookup response. | +| `pr_not_open` | The PR closed or merged while Amplify built the preview. No capture is needed. | +| `live_pr_head_sha_mismatch` | The live PR head changed after the check started. Rebuild the current preview. Receiver code `head_sha_mismatch` instead compares the check's embedded PR head with its own SHA. | +| `missing_head_ref` | GitHub returned the expected PR and SHA without a usable branch name. Inspect the PR response. | +| `malformed_pr_response` | GitHub returned a null, array, or non-object PR body. Inspect the API response. | + +The processor also rechecks the Amplify URL's HTTPS origin, credentials, port, and PR number before requesting a token or capturing; rejection logs use `untrusted_preview_url` and only the hostname. Fetch failures, timeouts, non-JSON bodies, 5xx responses, HTTP 408/429, and 403 responses carrying rate-limit headers retain bounded retries. Exhaustion logs `SCREENSHOT_PR_LOOKUP_EXHAUSTED` with the final failure reason. After resolving a transient failure, rebuild the preview or wait for the one-hour dedup window before replaying the same check. + ### No screenshots at all: check the processor alarms and DLQ The receiver Lambda async-invokes the processor (`InvocationType: Event`) and returns `200` to GitHub as soon as that invoke is accepted, so a *processor*-side fault never propagates back — GitHub sees success and never redelivers. (Only a failure to even enqueue the invoke returns `500`.) Two operator-visible signals catch a hard processor fault that would otherwise stop screenshots silently. From 0840ce7dccaccee58e79a7930be70973d6d6cb2f Mon Sep 17 00:00:00 2001 From: ayushtr-aws Date: Thu, 17 Sep 2026 16:14:05 -0400 Subject: [PATCH 3/3] fix(screenshots): classify PR lookup failures and complete review coverage Refs #900 Co-Authored-By: OpenAI Codex --- cdk/src/handlers/github-webhook-processor.ts | 87 ++++++++----- cdk/src/handlers/github-webhook.ts | 9 +- .../shared/github-deployment-status.ts | 26 +++- .../handlers/github-webhook-contract.test.ts | 22 +++- .../handlers/github-webhook-processor.test.ts | 118 +++++++++++++++--- cdk/test/handlers/github-webhook.test.ts | 7 ++ .../DEPLOY_PREVIEW_SCREENSHOTS_GUIDE.md | 23 +++- .../using/Deploy-preview-screenshots-guide.md | 23 +++- 8 files changed, 247 insertions(+), 68 deletions(-) diff --git a/cdk/src/handlers/github-webhook-processor.ts b/cdk/src/handlers/github-webhook-processor.ts index 562b862a2..8cd5a7649 100644 --- a/cdk/src/handlers/github-webhook-processor.ts +++ b/cdk/src/handlers/github-webhook-processor.ts @@ -25,7 +25,9 @@ import { upsertTaskComment } from './shared/github-comment'; import { type GitHubDeploymentStatusPayload, type ProcessorEvent, - type AmplifyPreviewRejectionReason, + type PrLookupRejectionReason, + type PrLookupRequestRejectionReason, + type PrLookupRetryableReason, AMPLIFY_PREVIEW_HOST, validateDeploymentStatusPayload, } from './shared/github-deployment-status'; @@ -89,7 +91,6 @@ const POST_CAPTURE_RESERVE_MS = 30_000; * session that's already doomed. */ const MIN_CAPTURE_BUDGET_MS = 15_000; -const HTTP_REQUEST_TIMEOUT = 408; /** Backoff schedule (ms) while waiting for GitHub to link a PR to a deploy SHA. */ const PR_LOOKUP_RETRY_DELAY_0_MS = 0; @@ -145,7 +146,7 @@ export async function handler(event: ProcessorEvent): Promise { if (validatedPrNumber !== undefined && (!Number.isSafeInteger(validatedPrNumber) || validatedPrNumber <= 0)) { logger.warn('Processor received invalid validated PR number', { - event: 'screenshot.amplify_pr_rejected', reason: 'invalid_pr_number', + event: 'screenshot.amplify_pr_rejected', reason: 'invalid_forwarded_pr_number', }); return; } @@ -173,16 +174,30 @@ export async function handler(event: ProcessorEvent): Promise { } catch { /* Rejected below without logging untrusted URL content. */ } const amplifyHost = preview && AMPLIFY_PREVIEW_HOST.exec(preview.hostname); if (!isAllowedScreenshotUrl(previewUrl) - || (validatedPrNumber !== undefined && (!preview || preview.username || preview.password || preview.port - || !amplifyHost || Number(amplifyHost[1]) !== validatedPrNumber))) { + || (validatedPrNumber !== undefined && (preview?.username || preview?.password || preview?.port + || !amplifyHost))) { logger.warn('Rejected deployment_status preview URL on allowlist', { repo, + event: 'screenshot.preview_url_rejected', preview_host: preview?.hostname, + url_parsed: Boolean(preview), reason: 'untrusted_preview_url', }); return; } + if (validatedPrNumber !== undefined && amplifyHost && Number(amplifyHost[1]) !== validatedPrNumber) { + logger.error('Amplify preview URL does not match forwarded PR number', { + event: 'screenshot.preview_url_rejected', + error_id: 'SCREENSHOT_PREVIEW_PR_MISMATCH', + reason: 'preview_pr_number_mismatch', + repo, + preview_host: preview?.hostname, + pr_number: validatedPrNumber, + }); + return; + } + logger.info('Screenshot pipeline starting', { repo, sha, @@ -208,13 +223,23 @@ export async function handler(event: ProcessorEvent): Promise { // half always gets at least MIN_CAPTURE_BUDGET_MS. const prLookupBudget = Math.max(0, remaining() - POST_CAPTURE_RESERVE_MS - MIN_CAPTURE_BUDGET_MS); const lookup = await findPullRequestForShaWithRetry(repo, sha, token, prLookupBudget, validatedPrNumber); - if (!lookup.ok && lookup.terminal) { + if (!lookup.ok && lookup.kind === 'request_rejected') { + logger.error('GitHub rejected the validated Amplify PR lookup', { + event: 'screenshot.pr_lookup_rejected', + error_id: 'SCREENSHOT_PR_LOOKUP_REJECTED', + reason: lookup.reason, + repo, + pr_number: validatedPrNumber, + status: lookup.status, + }); + return; + } + if (!lookup.ok && lookup.kind === 'pr_rejected') { logger.warn('Validated Amplify PR no longer matches preview', { event: 'screenshot.amplify_pr_rejected', reason: lookup.reason, repo, pr_number: validatedPrNumber, - ...(lookup.status !== undefined && { status: lookup.status }), }); return; } @@ -554,8 +579,9 @@ interface OpenPr { type PrLookup = | { readonly ok: true; readonly pr: OpenPr } - | { readonly ok: false; readonly terminal: true; readonly reason: AmplifyPreviewRejectionReason; readonly status?: number } - | { readonly ok: false; readonly terminal: false; readonly reason: 'fetch_failed' | 'http_error' | 'non_json_response' | 'malformed_pr_response' | 'pr_not_linked' | 'budget_exhausted' }; + | { readonly ok: false; readonly kind: 'pr_rejected'; readonly reason: PrLookupRejectionReason } + | { readonly ok: false; readonly kind: 'request_rejected'; readonly reason: PrLookupRequestRejectionReason; readonly status: number } + | { readonly ok: false; readonly kind: 'retryable'; readonly reason: PrLookupRetryableReason }; /** * Wait for an open PR to exist for the given SHA, retrying with a @@ -578,7 +604,7 @@ async function findPullRequestForShaWithRetry( validatedPrNumber?: number, ): Promise { const deadline = Date.now() + budgetMs; - let result: PrLookup = { ok: false, terminal: false, reason: 'budget_exhausted' }; + let result: PrLookup = { ok: false, kind: 'retryable', reason: 'budget_exhausted' }; for (let i = 0; i < PR_LOOKUP_RETRY_DELAYS_MS.length; i++) { const delay = PR_LOOKUP_RETRY_DELAYS_MS[i]; if (delay > 0) { @@ -589,7 +615,7 @@ async function findPullRequestForShaWithRetry( } if (Date.now() >= deadline) return result; result = await findPullRequestForSha(repo, sha, token, validatedPrNumber); - if (result.ok || result.terminal) return result; + if (result.ok || result.kind !== 'retryable') return result; const next = PR_LOOKUP_RETRY_DELAYS_MS[i + 1]; if (next !== undefined) { logger.info('Open PR not found yet for SHA — will retry', { @@ -604,14 +630,13 @@ async function findPullRequestForShaWithRetry( } /** - * Look up an open PR associated with `sha`. Uses the - * "List pull requests associated with a commit" GitHub API - * (https://docs.github.com/rest/commits/commits#list-pull-requests-associated-with-a-commit). + * With a validated Amplify PR number, fetch GET /repos/{repo}/pulls/{number}. + * Accept only that open PR with a matching head; body validation and permanent + * HTTP failures are terminal. Transient request failures remain retryable. * - * Returns the OPEN PR that the deploy is *for* (head SHA == `sha`), or - * the first open PR as a fallback, or a retryable failure if none. Closed/merged PRs - * are filtered out. For Amplify, only the validated PR with a matching head - * is accepted; there is no fallback to another PR. + * For deployment statuses, use GET /repos/{repo}/commits/{sha}/pulls. Prefer + * the open PR whose head matches the SHA, falling back to the first open PR. + * Missing PRs and request failures are retryable to cover the PR-creation race. */ async function findPullRequestForSha( repo: string, @@ -649,29 +674,29 @@ async function findPullRequestForSha( timed_out: ac.signal.aborted, error: err instanceof Error ? err.message : String(err), }); - return { ok: false, terminal: false, reason: 'fetch_failed' }; + return { ok: false, kind: 'retryable', reason: 'fetch_failed' }; } finally { clearTimeout(timer); } if (!res.ok) { if (validatedPrNumber !== undefined && res.status === 404) { - return { ok: false, terminal: true, reason: 'pr_not_found' }; + return { ok: false, kind: 'request_rejected', reason: 'pr_not_found', status: res.status }; } - // GitHub can report rate limiting as 403. Keep those and request timeouts - // retryable; authentication and invalid-request failures need operator action. - const rateLimited = res.status === 429 || (res.status === 403 - && (res.headers?.get('x-ratelimit-remaining') === '0' || res.headers?.has('retry-after'))); + // Secondary rate limits can return 403 without rate-limit headers. Retry + // all 403s conservatively; persistent permission failures exhaust into ERROR. + const HTTP_STATUS_REQUEST_TIMEOUT = 408; + const retryableStatus = [403, HTTP_STATUS_REQUEST_TIMEOUT, 429].includes(res.status); if (validatedPrNumber !== undefined && res.status >= 400 && res.status < 500 - && res.status !== HTTP_REQUEST_TIMEOUT && !rateLimited) { - return { ok: false, terminal: true, reason: 'pr_request_rejected', status: res.status }; + && !retryableStatus) { + return { ok: false, kind: 'request_rejected', reason: 'pr_request_rejected', status: res.status }; } logger.warn('GitHub PR lookup returned non-2xx', { repo, sha, status: res.status, }); - return { ok: false, terminal: false, reason: 'http_error' }; + return { ok: false, kind: 'retryable', reason: 'http_error' }; } // Parse defensively: both endpoints can return an unexpected response body. @@ -681,11 +706,11 @@ async function findPullRequestForSha( parsed = await res.json(); } catch { logger.warn('GitHub PR lookup returned non-JSON body', { repo, sha }); - return { ok: false, terminal: false, reason: 'non_json_response' }; + return { ok: false, kind: 'retryable', reason: 'non_json_response' }; } if (validatedPrNumber !== undefined) { const pr = parsed as { number?: unknown; state?: unknown; title?: unknown; body?: unknown; head?: { sha?: unknown; ref?: unknown } } | null; - const reject = (reason: AmplifyPreviewRejectionReason): PrLookup => ({ ok: false, terminal: true, reason }); + const reject = (reason: PrLookupRejectionReason): PrLookup => ({ ok: false, kind: 'pr_rejected', reason }); if (!pr || typeof pr !== 'object' || Array.isArray(pr)) return reject('malformed_pr_response'); if (pr.number !== validatedPrNumber) return reject('pr_number_mismatch'); if (pr.state !== 'open') return reject('pr_not_open'); @@ -704,7 +729,7 @@ async function findPullRequestForSha( // A non-array body would crash array operations and fault the async processor. if (!Array.isArray(parsed)) { logger.warn('GitHub commit-pulls did not return an array', { repo, sha }); - return { ok: false, terminal: false, reason: 'malformed_pr_response' }; + return { ok: false, kind: 'retryable', reason: 'malformed_pr_response' }; } const pulls = parsed as Array<{ number?: number; @@ -714,7 +739,7 @@ async function findPullRequestForSha( head?: { ref?: string; sha?: string } | null; }>; const openPulls = pulls.filter((p) => p.state === 'open' && typeof p.number === 'number'); - if (openPulls.length === 0) return { ok: false, terminal: false, reason: 'pr_not_linked' }; + if (openPulls.length === 0) return { ok: false, kind: 'retryable', reason: 'pr_not_linked' }; // Prefer the PR whose own head is this SHA — the PR that introduced the // commit. For a stacked PR chain the commit-pulls API also lists every // PR stacked on top (their history contains the commit); routing reads diff --git a/cdk/src/handlers/github-webhook.ts b/cdk/src/handlers/github-webhook.ts index f71656161..e1d9d2b97 100644 --- a/cdk/src/handlers/github-webhook.ts +++ b/cdk/src/handlers/github-webhook.ts @@ -54,7 +54,8 @@ const DEDUP_TTL_SECONDS = 60 * 60; * `check_run` completions. Deployment statuses must match * `SCREENSHOT_TARGET_ENVIRONMENT` (default `Preview`); validated Amplify * PR previews bypass that environment filter. Dedups - * on `(repo, deployment_id, status_id)`, and async-invokes the + * on `(repo, deployment_id, status_id)` with a separate `amplify#` namespace + * for check runs, and async-invokes the * processor Lambda so we can ack within GitHub's 10s timeout. Other * event types (push, pull_request, ping, …) get an immediate 200 so * GitHub doesn't retry them. @@ -129,8 +130,7 @@ export async function handler(event: APIGatewayProxyEvent): Promise`. Operators can override the Lambda variable and redeploy. - // Validated Amplify checks already identify a PR preview and bypass this filter, - // including branch-name values that previously excluded those previews. + // Validated Amplify checks already identify a PR preview and bypass this filter. const targetEnv = process.env.SCREENSHOT_TARGET_ENVIRONMENT ?? 'Preview'; if (!normalized && raw.deployment?.environment !== targetEnv) { return jsonResponse(200, { @@ -167,7 +167,8 @@ export async function handler(event: APIGatewayProxyEvent): Promise : {}; - const reject = (reason: AmplifyPreviewRejectionReason): AmplifyPreviewCheckResult => ({ ok: false, reason }); + const reject = (reason: AmplifyCheckRejectionReason): AmplifyPreviewCheckResult => ({ ok: false, reason }); const raw = record(value); const check = record(raw.check_run); const app = record(check.app); diff --git a/cdk/test/handlers/github-webhook-contract.test.ts b/cdk/test/handlers/github-webhook-contract.test.ts index b6e2dab68..1825fd3ab 100644 --- a/cdk/test/handlers/github-webhook-contract.test.ts +++ b/cdk/test/handlers/github-webhook-contract.test.ts @@ -96,7 +96,16 @@ const sha = 'a'.repeat(40); const pr41 = { number: 41, state: 'open', title: 'other PR', head: { sha, ref: 'other' } }; const pr42 = { number: 42, state: 'open', title: 'preview PR', head: { sha, ref: 'preview' } }; +const originalTargetEnvironment = process.env.SCREENSHOT_TARGET_ENVIRONMENT; +afterEach(() => { + jest.useRealTimers(); + jest.restoreAllMocks(); + if (originalTargetEnvironment === undefined) delete process.env.SCREENSHOT_TARGET_ENVIRONMENT; + else process.env.SCREENSHOT_TARGET_ENVIRONMENT = originalTargetEnvironment; +}); + beforeEach(() => { + delete process.env.SCREENSHOT_TARGET_ENVIRONMENT; jest.restoreAllMocks(); jest.clearAllMocks(); ddbSend.mockResolvedValue({}); @@ -140,7 +149,18 @@ test.each(['check_run', 'deployment_status'])('receiver %s payload drives the pr expect(lambdaSend).toHaveBeenCalledTimes(1); // Pass the actual bytes emitted by the receiver without rebuilding any fields. const forwarded = JSON.parse(new TextDecoder().decode(lambdaSend.mock.calls[0][0].input.Payload)); - if (eventType === 'deployment_status') expect(forwarded).toEqual({ raw_body: JSON.stringify(body) }); + if (eventType === 'deployment_status') { + expect(forwarded).toEqual({ raw_body: JSON.stringify(body) }); + } else { + expect(forwarded).toEqual({ + raw_body: JSON.stringify({ + repository: { full_name: 'owner/repo' }, + deployment: { id: 123, sha, environment: 'Preview' }, + deployment_status: { id: 123, state: 'success', environment_url: 'https://pr-42.app123.amplifyapp.com' }, + }), + validated_pr_number: 42, + }); + } await processorHandler(forwarded); expect(fetchMock).toHaveBeenCalledTimes(1); diff --git a/cdk/test/handlers/github-webhook-processor.test.ts b/cdk/test/handlers/github-webhook-processor.test.ts index 47dc334bc..aca20bea2 100644 --- a/cdk/test/handlers/github-webhook-processor.test.ts +++ b/cdk/test/handlers/github-webhook-processor.test.ts @@ -70,6 +70,8 @@ jest.mock('../../src/handlers/shared/jira-deployment-preview', () => ({ const originalJiraRegistry = process.env.JIRA_WORKSPACE_REGISTRY_TABLE_NAME; beforeEach(() => { process.env.JIRA_WORKSPACE_REGISTRY_TABLE_NAME = 'JiraRegistry'; }); afterEach(() => { + jest.useRealTimers(); + jest.restoreAllMocks(); if (originalJiraRegistry === undefined) delete process.env.JIRA_WORKSPACE_REGISTRY_TABLE_NAME; else process.env.JIRA_WORKSPACE_REGISTRY_TABLE_NAME = originalJiraRegistry; }); @@ -101,12 +103,14 @@ function fetchOk(jsonValue: unknown, status = 200): jest.SpyInstance { return jest.spyOn(global, 'fetch').mockResolvedValueOnce({ ok: status >= 200 && status < 300, status, + headers: new Headers(), json: async () => jsonValue, } as unknown as Response); } describe('github-webhook-processor handler', () => { beforeEach(() => { + jest.restoreAllMocks(); process.env.JIRA_WORKSPACE_REGISTRY_TABLE_NAME = 'JiraRegistry'; deliverJiraMock.mockReset(); s3Send.mockReset(); @@ -121,7 +125,6 @@ describe('github-webhook-processor handler', () => { // task record (no orchestration_sub_issue_id) → standalone Linear comment // still posts, as the pre-existing tests expect. ddbSend.mockReset().mockResolvedValue({ Attributes: { channel_metadata: {} } }); - jest.restoreAllMocks(); }); test('returns silently when raw_body is empty', async () => { @@ -600,7 +603,7 @@ describe('validated Amplify PR routing', () => { } }); - test.each([404, 400, 401, 403, 422])('HTTP %s stops after one request with one reason and no exhausted error', async (status) => { + test.each([404, 400, 401, 422])('HTTP %s stops immediately with an actionable request error', async (status) => { jest.useFakeTimers(); try { const warn = jest.spyOn(logger, 'warn'); @@ -611,15 +614,16 @@ describe('validated Amplify PR routing', () => { await jest.runAllTimersAsync(); await pending; expect(fetchMock).toHaveBeenCalledTimes(1); - expect(warn).toHaveBeenCalledTimes(1); - expect(warn).toHaveBeenCalledWith(expect.any(String), { - event: 'screenshot.amplify_pr_rejected', + expect(warn).not.toHaveBeenCalled(); + expect(error).toHaveBeenCalledTimes(1); + expect(error).toHaveBeenCalledWith('GitHub rejected the validated Amplify PR lookup', { + event: 'screenshot.pr_lookup_rejected', + error_id: 'SCREENSHOT_PR_LOOKUP_REJECTED', reason: status === 404 ? 'pr_not_found' : 'pr_request_rejected', repo: 'owner/repo', pr_number: 42, - ...(status !== 404 && { status }), + status, }); - expect(error).not.toHaveBeenCalled(); expect(Date.now()).toBe(start); expect(captureScreenshotMock).not.toHaveBeenCalled(); expect(upsertTaskCommentMock).not.toHaveBeenCalled(); @@ -628,7 +632,7 @@ describe('validated Amplify PR routing', () => { } }); - test.each(['fetch error', 'timeout', '5xx', 'non-JSON', '403 quota', '403 retry-after', '408', '429'])('retries transient %s and captures the same PR after recovery', async (failure) => { + test.each(['fetch error', 'timeout', '500', '502', '503', 'non-JSON', '403 quota', '403 retry-after', '403 no headers', '403 remaining quota', '408', '429'])('retries transient %s and captures the same PR after recovery', async (failure) => { jest.useFakeTimers(); try { const fetchMock = jest.spyOn(global, 'fetch'); @@ -638,16 +642,16 @@ describe('validated Amplify PR routing', () => { fetchMock.mockImplementationOnce((_url, options) => new Promise((_resolve, reject) => { options?.signal?.addEventListener('abort', () => reject(new Error('aborted'))); })); - } else if (failure === '5xx') { - fetchMock.mockResolvedValueOnce({ ok: false, status: 503 } as Response); - } else if (failure === '403 quota' || failure === '403 retry-after') { + } else if (failure.startsWith('403')) { fetchMock.mockResolvedValueOnce({ ok: false, status: 403, - headers: new Headers(failure === '403 quota' ? { 'x-ratelimit-remaining': '0' } : { 'retry-after': '5' }), + headers: new Headers(failure === '403 quota' ? { 'x-ratelimit-remaining': '0' } + : failure === '403 retry-after' ? { 'retry-after': '5' } + : failure === '403 remaining quota' ? { 'x-ratelimit-remaining': '4999' } : {}), } as Response); - } else if (failure === '408' || failure === '429') { - fetchMock.mockResolvedValueOnce({ ok: false, status: Number(failure) } as Response); + } else if (['408', '429', '500', '502', '503'].includes(failure)) { + fetchMock.mockResolvedValueOnce({ ok: false, status: Number(failure), headers: new Headers() } as Response); } else { fetchMock.mockResolvedValueOnce({ ok: true, json: async () => { throw new SyntaxError('secret-response-fragment'); } } as unknown as Response); } @@ -668,10 +672,10 @@ describe('validated Amplify PR routing', () => { } }); - test('exhausted transient Amplify failures retain the operational error', async () => { + test.each([503, 403])('persistent HTTP %s failures retain the operational error', async (status) => { jest.useFakeTimers(); try { - const fetchMock = jest.spyOn(global, 'fetch').mockResolvedValue({ ok: false, status: 503 } as Response); + const fetchMock = jest.spyOn(global, 'fetch').mockResolvedValue({ ok: false, status, headers: new Headers({ 'x-ratelimit-remaining': '4999' }) } as Response); const error = jest.spyOn(logger, 'error'); const pending = handler(amplifyEvent()); await jest.runAllTimersAsync(); @@ -700,7 +704,7 @@ describe('validated Amplify PR routing', () => { test.each([ 'https://preview.example.com/path?token=secret', - 'https://pr-41.app123.amplifyapp.com/path?token=secret', + 'not-a-url?token=secret', 'https://user:secret@pr-42.app123.amplifyapp.com', 'https://pr-42.app123.amplifyapp.com:8080/path?token=secret', 'http://pr-42.app123.amplifyapp.com/path?token=secret', @@ -712,13 +716,91 @@ describe('validated Amplify PR routing', () => { await handler({ ...forwarded, raw_body: JSON.stringify(raw) }); expect(resolveGitHubTokenMock).not.toHaveBeenCalled(); expect(captureScreenshotMock).not.toHaveBeenCalled(); - expect(warn).toHaveBeenCalledWith(expect.any(String), expect.objectContaining({ reason: 'untrusted_preview_url' })); + expect(warn).toHaveBeenCalledWith(expect.any(String), expect.objectContaining({ event: 'screenshot.preview_url_rejected', reason: 'untrusted_preview_url', url_parsed: url.startsWith('http') })); expect(JSON.stringify(warn.mock.calls)).not.toContain('secret'); expect(JSON.stringify(warn.mock.calls)).not.toContain(url); }); + test.each([41, 99])('a preview for PR %s cannot override forwarded PR 42', async (previewPr) => { + const warn = jest.spyOn(logger, 'warn'); + const error = jest.spyOn(logger, 'error'); + const forwarded = amplifyEvent(); + const raw = JSON.parse(forwarded.raw_body); + raw.deployment_status.environment_url = `https://pr-${previewPr}.app123.amplifyapp.com/path?token=secret`; + await handler({ ...forwarded, raw_body: JSON.stringify(raw) }); + expect(resolveGitHubTokenMock).not.toHaveBeenCalled(); + expect(captureScreenshotMock).not.toHaveBeenCalled(); + expect(warn).not.toHaveBeenCalled(); + expect(error).toHaveBeenCalledTimes(1); + expect(error).toHaveBeenCalledWith(expect.any(String), expect.objectContaining({ + event: 'screenshot.preview_url_rejected', + error_id: 'SCREENSHOT_PREVIEW_PR_MISMATCH', + reason: 'preview_pr_number_mismatch', + pr_number: 42, + })); + expect(JSON.stringify(error.mock.calls)).not.toContain('secret'); + }); + + test.each([ + ['deployment', 'http_error', 404], + ['deployment', 'malformed_pr_response', 200], + ['deployment', 'pr_not_linked', 200], + ['amplify', 'fetch_failed', 200], + ['amplify', 'non_json_response', 200], + ] as const)('%s lookup exhausts %s without capture', async (provider, reason, status) => { + jest.useFakeTimers(); + const fetchMock = jest.spyOn(global, 'fetch').mockImplementation(async () => { + if (reason === 'fetch_failed') throw new Error('network unavailable'); + return { + ok: status === 200, + status, + headers: new Headers(), + json: async () => { + if (reason === 'non_json_response') throw new SyntaxError('secret-response-fragment'); + return reason === 'pr_not_linked' ? [] : {}; + }, + } as Response; + }); + const error = jest.spyOn(logger, 'error'); + const warn = jest.spyOn(logger, 'warn'); + const pending = handler(provider === 'amplify' ? amplifyEvent() : payload()); + await jest.runAllTimersAsync(); + await pending; + expect(fetchMock).toHaveBeenCalledTimes(4); + if (provider === 'deployment') { + expect(fetchMock).toHaveBeenCalledWith('https://api.github.com/repos/owner/repo/commits/abc1234/pulls', expect.anything()); + } + expect(error).toHaveBeenCalledTimes(1); + expect(error).toHaveBeenCalledWith(expect.any(String), expect.objectContaining({ + event: 'screenshot.pr_lookup_exhausted', error_id: 'SCREENSHOT_PR_LOOKUP_EXHAUSTED', reason, + })); + expect(JSON.stringify(warn.mock.calls)).not.toContain('secret-response-fragment'); + expect(captureScreenshotMock).not.toHaveBeenCalled(); + expect(upsertTaskCommentMock).not.toHaveBeenCalled(); + }); + + test('token resolution consuming the lookup budget skips GitHub and capture', async () => { + jest.useFakeTimers(); + resolveGitHubTokenMock.mockImplementationOnce(async () => { + jest.setSystemTime(Date.now() + 65_000); + return 'token'; + }); + const fetchMock = jest.spyOn(global, 'fetch'); + const error = jest.spyOn(logger, 'error'); + await handler(amplifyEvent()); + expect(fetchMock).not.toHaveBeenCalled(); + expect(captureScreenshotMock).not.toHaveBeenCalled(); + expect(error).toHaveBeenCalledWith(expect.any(String), expect.objectContaining({ + error_id: 'SCREENSHOT_PR_LOOKUP_EXHAUSTED', reason: 'budget_exhausted', budget_ms: 0, + })); + }); + test.each([0, -1, 1.5, Number.MAX_SAFE_INTEGER + 1])('rejects invalid forwarded PR number %s', async (number) => { + const warn = jest.spyOn(logger, 'warn'); await handler({ ...amplifyEvent(), validated_pr_number: number }); + expect(warn).toHaveBeenCalledWith(expect.any(String), { + event: 'screenshot.amplify_pr_rejected', reason: 'invalid_forwarded_pr_number', + }); expect(resolveGitHubTokenMock).not.toHaveBeenCalled(); expect(captureScreenshotMock).not.toHaveBeenCalled(); }); diff --git a/cdk/test/handlers/github-webhook.test.ts b/cdk/test/handlers/github-webhook.test.ts index e5a06bed9..329d277e1 100644 --- a/cdk/test/handlers/github-webhook.test.ts +++ b/cdk/test/handlers/github-webhook.test.ts @@ -112,7 +112,14 @@ function amplifyBody(overrides: Record = {}, envelope: Record { + const originalTargetEnvironment = process.env.SCREENSHOT_TARGET_ENVIRONMENT; + afterEach(() => { + jest.restoreAllMocks(); + if (originalTargetEnvironment === undefined) delete process.env.SCREENSHOT_TARGET_ENVIRONMENT; + else process.env.SCREENSHOT_TARGET_ENVIRONMENT = originalTargetEnvironment; + }); beforeEach(() => { + delete process.env.SCREENSHOT_TARGET_ENVIRONMENT; jest.restoreAllMocks(); ddbSend.mockReset(); lambdaSend.mockReset(); diff --git a/docs/guides/DEPLOY_PREVIEW_SCREENSHOTS_GUIDE.md b/docs/guides/DEPLOY_PREVIEW_SCREENSHOTS_GUIDE.md index 99885b57d..a1c2e4e29 100644 --- a/docs/guides/DEPLOY_PREVIEW_SCREENSHOTS_GUIDE.md +++ b/docs/guides/DEPLOY_PREVIEW_SCREENSHOTS_GUIDE.md @@ -192,14 +192,16 @@ Then tail the function's CloudWatch log group. Common silent skips: - `skipped_state` — the delivery was for a non-`success` status (e.g. `pending`, `in_progress`); ignore. - `skipped_environment` (deployment statuses only) — the deploy's `environment` field doesn't match `SCREENSHOT_TARGET_ENVIRONMENT`. Common cause for non-Vercel providers; see "Configuring for non-Vercel providers" above. - `skipped_no_url` — the `success` status didn't include `environment_url`. Some providers post URL-less success events; the next push usually carries the URL. -- `screenshot.amplify_pr_rejected` — a terminal rejection of the validated PR, logged once at warn level. Capture stops immediately without retrying, falling back to another PR, or emitting `SCREENSHOT_PR_LOOKUP_EXHAUSTED`. See the reasons below. -- `No open PR found for SHA after retries` — the deploy provider built and reported faster than the agent could `gh pr create` (race window > 35s). Rare; redeliver the webhook from GitHub's UI to retry. +- `screenshot.amplify_pr_rejected` — invalid forwarded PR metadata or a terminal rejection of the live PR response, logged once at warn level. Capture stops immediately without retrying, falling back to another PR, or emitting `SCREENSHOT_PR_LOOKUP_EXHAUSTED`. See the reasons below. +- `screenshot.pr_lookup_rejected` / `SCREENSHOT_PR_LOOKUP_REJECTED` — GitHub rejected the validated PR lookup with a permanent HTTP failure, logged once at ERROR with `status`. Check token access and permissions, including for 404 responses that can conceal inaccessible repositories. Capture stops without retrying. +- `screenshot.pr_lookup_exhausted` / `SCREENSHOT_PR_LOOKUP_EXHAUSTED` — retries or the lookup budget ran out. Check GitHub availability, token permissions, rate limits, or whether a deployment completed before the PR was created. +- `screenshot.preview_url_rejected` — URL validation stopped capture. `untrusted_preview_url` logs at WARN with `preview_host` and `url_parsed`; `preview_pr_number_mismatch` logs at ERROR with `SCREENSHOT_PREVIEW_PR_MISMATCH` when the preview hostname disagrees with the forwarded PR number. Processor-side Amplify reasons: | Reason | What to check | |---|---| -| `invalid_pr_number` | The forwarded PR number must be a positive safe integer. Deploy the receiver and processor together. | +| `invalid_forwarded_pr_number` | The forwarded PR number must be a positive safe integer. Deploy the receiver and processor together. | | `pr_not_found` | GitHub returned 404 for the validated PR. Confirm the PR exists and the GitHub token can access it; GitHub also uses 404 to conceal inaccessible resources. | | `pr_request_rejected` | GitHub rejected the PR lookup with a non-retryable 4xx response. Inspect the logged `status`, token permissions, and request. | | `pr_number_mismatch` | GitHub returned a different PR number than the validated preview. Inspect the PR lookup response. | @@ -208,7 +210,20 @@ Processor-side Amplify reasons: | `missing_head_ref` | GitHub returned the expected PR and SHA without a usable branch name. Inspect the PR response. | | `malformed_pr_response` | GitHub returned a null, array, or non-object PR body. Inspect the API response. | -The processor also rechecks the Amplify URL's HTTPS origin, credentials, port, and PR number before requesting a token or capturing; rejection logs use `untrusted_preview_url` and only the hostname. Fetch failures, timeouts, non-JSON bodies, 5xx responses, HTTP 408/429, and 403 responses carrying rate-limit headers retain bounded retries. Exhaustion logs `SCREENSHOT_PR_LOOKUP_EXHAUSTED` with the final failure reason. After resolving a transient failure, rebuild the preview or wait for the one-hour dedup window before replaying the same check. +The processor rechecks the Amplify URL's HTTPS origin, credentials, port, and PR number before requesting a token or capturing. URL rejection logs include only the hostname, never credentials, paths, or queries. A hostname/forwarded-number mismatch indicates an inconsistent invocation; inspect the receiver and processor deployment versions. + +Fetch failures, timeouts, non-JSON bodies, 5xx responses, HTTP 408/429, and all 403 responses retain bounded retries. Secondary rate limits can omit rate-limit headers, so even an ambiguous 403 retries; persistent permission failures then emit `SCREENSHOT_PR_LOOKUP_EXHAUSTED`. Deployment-status commit-pulls HTTP failures, including 404, also remain retryable. Exhaustion records the final reason: + +| Reason | What to check | +|---|---| +| `fetch_failed` | GitHub network reachability or per-request timeout. | +| `http_error` | GitHub returned non-2xx responses; inspect the preceding logged HTTP statuses, token permissions, and rate limits. | +| `non_json_response` | GitHub's response could not be parsed as JSON. | +| `malformed_pr_response` | The deployment-status commit-pulls endpoint returned a non-array body. | +| `pr_not_linked` | No open PR is associated with the deployment SHA yet. | +| `budget_exhausted` | Earlier processing consumed the PR lookup budget before the first request. | + +After resolving a failure, rebuild the preview or wait for the one-hour dedup window before replaying the same check. ### No screenshots at all: check the processor alarms and DLQ diff --git a/docs/src/content/docs/using/Deploy-preview-screenshots-guide.md b/docs/src/content/docs/using/Deploy-preview-screenshots-guide.md index 265d45ff5..4369eb08d 100644 --- a/docs/src/content/docs/using/Deploy-preview-screenshots-guide.md +++ b/docs/src/content/docs/using/Deploy-preview-screenshots-guide.md @@ -196,14 +196,16 @@ Then tail the function's CloudWatch log group. Common silent skips: - `skipped_state` — the delivery was for a non-`success` status (e.g. `pending`, `in_progress`); ignore. - `skipped_environment` (deployment statuses only) — the deploy's `environment` field doesn't match `SCREENSHOT_TARGET_ENVIRONMENT`. Common cause for non-Vercel providers; see "Configuring for non-Vercel providers" above. - `skipped_no_url` — the `success` status didn't include `environment_url`. Some providers post URL-less success events; the next push usually carries the URL. -- `screenshot.amplify_pr_rejected` — a terminal rejection of the validated PR, logged once at warn level. Capture stops immediately without retrying, falling back to another PR, or emitting `SCREENSHOT_PR_LOOKUP_EXHAUSTED`. See the reasons below. -- `No open PR found for SHA after retries` — the deploy provider built and reported faster than the agent could `gh pr create` (race window > 35s). Rare; redeliver the webhook from GitHub's UI to retry. +- `screenshot.amplify_pr_rejected` — invalid forwarded PR metadata or a terminal rejection of the live PR response, logged once at warn level. Capture stops immediately without retrying, falling back to another PR, or emitting `SCREENSHOT_PR_LOOKUP_EXHAUSTED`. See the reasons below. +- `screenshot.pr_lookup_rejected` / `SCREENSHOT_PR_LOOKUP_REJECTED` — GitHub rejected the validated PR lookup with a permanent HTTP failure, logged once at ERROR with `status`. Check token access and permissions, including for 404 responses that can conceal inaccessible repositories. Capture stops without retrying. +- `screenshot.pr_lookup_exhausted` / `SCREENSHOT_PR_LOOKUP_EXHAUSTED` — retries or the lookup budget ran out. Check GitHub availability, token permissions, rate limits, or whether a deployment completed before the PR was created. +- `screenshot.preview_url_rejected` — URL validation stopped capture. `untrusted_preview_url` logs at WARN with `preview_host` and `url_parsed`; `preview_pr_number_mismatch` logs at ERROR with `SCREENSHOT_PREVIEW_PR_MISMATCH` when the preview hostname disagrees with the forwarded PR number. Processor-side Amplify reasons: | Reason | What to check | |---|---| -| `invalid_pr_number` | The forwarded PR number must be a positive safe integer. Deploy the receiver and processor together. | +| `invalid_forwarded_pr_number` | The forwarded PR number must be a positive safe integer. Deploy the receiver and processor together. | | `pr_not_found` | GitHub returned 404 for the validated PR. Confirm the PR exists and the GitHub token can access it; GitHub also uses 404 to conceal inaccessible resources. | | `pr_request_rejected` | GitHub rejected the PR lookup with a non-retryable 4xx response. Inspect the logged `status`, token permissions, and request. | | `pr_number_mismatch` | GitHub returned a different PR number than the validated preview. Inspect the PR lookup response. | @@ -212,7 +214,20 @@ Processor-side Amplify reasons: | `missing_head_ref` | GitHub returned the expected PR and SHA without a usable branch name. Inspect the PR response. | | `malformed_pr_response` | GitHub returned a null, array, or non-object PR body. Inspect the API response. | -The processor also rechecks the Amplify URL's HTTPS origin, credentials, port, and PR number before requesting a token or capturing; rejection logs use `untrusted_preview_url` and only the hostname. Fetch failures, timeouts, non-JSON bodies, 5xx responses, HTTP 408/429, and 403 responses carrying rate-limit headers retain bounded retries. Exhaustion logs `SCREENSHOT_PR_LOOKUP_EXHAUSTED` with the final failure reason. After resolving a transient failure, rebuild the preview or wait for the one-hour dedup window before replaying the same check. +The processor rechecks the Amplify URL's HTTPS origin, credentials, port, and PR number before requesting a token or capturing. URL rejection logs include only the hostname, never credentials, paths, or queries. A hostname/forwarded-number mismatch indicates an inconsistent invocation; inspect the receiver and processor deployment versions. + +Fetch failures, timeouts, non-JSON bodies, 5xx responses, HTTP 408/429, and all 403 responses retain bounded retries. Secondary rate limits can omit rate-limit headers, so even an ambiguous 403 retries; persistent permission failures then emit `SCREENSHOT_PR_LOOKUP_EXHAUSTED`. Deployment-status commit-pulls HTTP failures, including 404, also remain retryable. Exhaustion records the final reason: + +| Reason | What to check | +|---|---| +| `fetch_failed` | GitHub network reachability or per-request timeout. | +| `http_error` | GitHub returned non-2xx responses; inspect the preceding logged HTTP statuses, token permissions, and rate limits. | +| `non_json_response` | GitHub's response could not be parsed as JSON. | +| `malformed_pr_response` | The deployment-status commit-pulls endpoint returned a non-array body. | +| `pr_not_linked` | No open PR is associated with the deployment SHA yet. | +| `budget_exhausted` | Earlier processing consumed the PR lookup budget before the first request. | + +After resolving a failure, rebuild the preview or wait for the one-hour dedup window before replaying the same check. ### No screenshots at all: check the processor alarms and DLQ