feat(infra): add Lambda-error alarm to GitHub webhook processor (#284)

cdk/src/constructs/github-screenshot-integration.ts

@@ -40,6 +40,15 @@ const PROCESSOR_DLQ_RETENTION_DAYS = 14;
 /** DLQ-depth alarm metric period (minutes). */
 const DLQ_ALARM_PERIOD_MINUTES = 5;
 
+/** Processor Lambda-Errors alarm metric period (minutes). */
+const ERROR_ALARM_PERIOD_MINUTES = 5;
+
+/** Processor Lambda-Errors alarm evaluation range (periods). Paired with
+ *  `datapointsToAlarm: 1` below, this is 1-of-2, NOT 2-of-2: any single
+ *  breaching period alarms. Do not remove `datapointsToAlarm` — CloudWatch
+ *  would default it back to this value and a one-off crash would never fire. */
+const ERROR_ALARM_EVALUATION_PERIODS = 2;
+
 /** Async screenshot-processor Lambda memory (MB). */
 const PROCESSOR_MEMORY_MB = 512;
 
@@ -128,6 +137,18 @@ export class GitHubScreenshotIntegration extends Construct {
    *  "for operator inspection" that no operator is ever told to make. */
   public readonly processorDlqDepthAlarm: cloudwatch.Alarm;
 
+  /** Fires when the processor Lambda's ``Errors`` metric breaches. The
+   *  handler swallows its per-step operational failures (log + return),
+   *  so this metric ticks only on faults that ESCAPE the handler —
+   *  init-time crashes (missing env at cold start, bundling defect),
+   *  unhandled throws in an unguarded path, or the 120s hard timeout.
+   *  For that population it fires one evaluation window sooner than the
+   *  DLQ-depth alarm (which waits for Lambda's async-retry ladder to land
+   *  a message). Follows the ``metricErrors`` + 2-eval-period shape of
+   *  ``task-orchestrator.ts`` OrchestratorErrorAlarm (with threshold 1,
+   *  since any escaped error here is significant). */
+  public readonly processorErrorAlarm: cloudwatch.Alarm;
+
   constructor(scope: Construct, id: string, props: GitHubScreenshotIntegrationProps) {
     super(scope, id);
 
@@ -219,6 +240,38 @@ export class GitHubScreenshotIntegration extends Construct {
       treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING,
     });
 
+    // Alarm on the processor's Errors metric (#284 AC). Complementary to
+    // the DLQ-depth alarm above, not redundant. The handler is best-effort
+    // by design — it catches its per-step operational failures (bad token,
+    // AgentCore/S3/comment-post errors) and returns success, so those
+    // surface as tagged log events, NOT as Lambda Errors. This alarm
+    // therefore fires on the faults that ESCAPE the handler: an init-time
+    // crash (missing env at cold start, bundling defect), an unhandled
+    // throw in an unguarded path, or the 120s hard timeout. That is the
+    // same failure class the DLQ eventually catches, but before Lambda's async-retry
+    // ladder has landed a message on the DLQ. The Errors metric is stamped
+    // at invocation time, so this alarm fires no later than — and, when the
+    // retry ladder straddles a period boundary, one window before — the
+    // DLQ-depth alarm. threshold 1 / 1-of-2 eval periods matches the AC;
+    // the metricErrors shape follows ``task-orchestrator.ts``
+    // OrchestratorErrorAlarm (which uses threshold 3 — we use 1 because any
+    // escaped error here is significant).
+    this.processorErrorAlarm = new cloudwatch.Alarm(this, 'WebhookProcessorErrorAlarm', {
+      metric: this.webhookProcessorFn.metricErrors({
+        period: Duration.minutes(ERROR_ALARM_PERIOD_MINUTES),
+      }),
+      threshold: 1,
+      evaluationPeriods: ERROR_ALARM_EVALUATION_PERIODS,
+      // Fire on the FIRST breaching datapoint. Unset, this defaults to
+      // evaluationPeriods (2 consecutive), which for a sparse invocation
+      // metric means a single-event crash never alarms: its retries land
+      // in one period and NOT_BREACHING fills the neighbour.
+      datapointsToAlarm: 1,
+      alarmDescription:
+        'Screenshot webhook processor Lambda errors breached threshold — deploy-preview screenshots are failing (check IAM, AgentCore Browser quota, and dependency health)',
+      treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING,
+    });
+
     this.screenshotBucket.bucket.grantPut(this.webhookProcessorFn);
     props.githubTokenSecret.grantRead(this.webhookProcessorFn);
 

cdk/test/constructs/github-screenshot-integration.test.ts

@@ -70,7 +70,12 @@ describe('GitHubScreenshotIntegration construct', () => {
     // crashes reach the queue — each one means the screenshot pipeline is
     // silently down. Without the alarm the queue is "for operator
     // inspection" that no operator is ever told to make.
-    template.resourceCountIs('AWS::CloudWatch::Alarm', 1);
+    //
+    // Two alarms total: this DLQ-depth alarm (a record LANDED) and the
+    // Lambda-Errors alarm below (invocations are FAILING, before Lambda's
+    // async retries have exhausted onto the DLQ). They are complementary,
+    // not redundant — see the next test.
+    template.resourceCountIs('AWS::CloudWatch::Alarm', 2);
     template.hasResourceProperties('AWS::CloudWatch::Alarm', {
       MetricName: 'ApproximateNumberOfMessagesVisible',
       Namespace: 'AWS/SQS',
@@ -90,6 +95,44 @@ describe('GitHubScreenshotIntegration construct', () => {
     });
   });
 
+  test('alarms on the processor Lambda Errors metric (>=1 in 5min, 2 eval periods)', () => {
+    // #284 AC: an alarm on the processor's Errors metric. This is DISTINCT
+    // from the DLQ-depth alarm above — the DLQ-depth alarm only fires once
+    // a failure has survived Lambda's async retry ladder and LANDED on the
+    // queue, whereas an Errors alarm fires as soon as invocations start
+    // failing, catching a systemic break (IAM regression, AgentCore quota
+    // exhaustion, dependency outage) at the first faulting invocation.
+    // Mirrors `task-orchestrator.ts` OrchestratorErrorAlarm's idiom.
+    template.hasResourceProperties('AWS::CloudWatch::Alarm', {
+      MetricName: 'Errors',
+      Namespace: 'AWS/Lambda',
+      // metricErrors() defaults to Sum — the correct statistic for an
+      // error COUNT (vs Average/Maximum). Pin it so a future edit that
+      // swaps the statistic can't silently change the alarm's semantics.
+      Statistic: 'Sum',
+      Period: 300,
+      Threshold: 1,
+      // >= 1, not > 1. Relies on a CDK default otherwise, and
+      // GreaterThanThreshold would silently require TWO errors to fire.
+      ComparisonOperator: 'GreaterThanOrEqualToThreshold',
+      EvaluationPeriods: 2,
+      // 1-of-2, not 2-of-2. Unset, DatapointsToAlarm defaults to
+      // EvaluationPeriods, and a single-event crash would never alarm
+      // (its Errors datapoints land in one period; NOT_BREACHING fills
+      // the neighbour). Pinned so that regression is caught here.
+      DatapointsToAlarm: 1,
+      TreatMissingData: 'notBreaching',
+      Dimensions: Match.arrayWith([
+        Match.objectLike({
+          Name: 'FunctionName',
+          Value: Match.objectLike({
+            Ref: Match.stringLikeRegexp('WebhookProcessorFn'),
+          }),
+        }),
+      ]),
+    });
+  });
+
   test('wires the DLQ as the processor Lambda async-invoke dead-letter target', () => {
     // The queue existing is not enough — it must be bound to the
     // processor function's DeadLetterConfig or failed async invokes

docs/guides/DEPLOY_PREVIEW_SCREENSHOTS_GUIDE.md

@@ -175,6 +175,35 @@ Then tail the function's CloudWatch log group. Common silent skips:
 - `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.
 - `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
+
+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.
+
+**Important:** the processor handler is best-effort by design — it catches its own per-step operational failures (bad token, AgentCore/S3/comment-post errors) and returns success, so those show up as tagged log events (see the section above), **not** as Lambda `Errors`. Both alarms below fire only on faults that *escape* the handler: an init-time crash (missing env at cold start, bundling defect), an unhandled throw in an unguarded path, or the 120s hard timeout. For a swallowed operational failure (e.g. a revoked S3/AgentCore permission), watch the processor **logs**, not these alarms.
+
+- **`WebhookProcessorErrorAlarm`** — fires on the processor's Lambda `Errors` metric: **any** 5-minute period with `>= 1` error alarms (evaluation range 2 periods, 1 datapoint to alarm). Early signal: an invocation is faulting *now*.
+- **`WebhookProcessorDlqDepthAlarm`** — fires when such a failed invocation has survived Lambda's built-in async retries and landed on the DLQ (construct id `WebhookProcessorDlq`; 14-day retention, SSL-enforced). Backstop: a payload is now parked undelivered. (The queue also gets SSE via SQS's service-side default; the construct doesn't set an explicit `encryption`.)
+
+The two catch the same failure class, but the `Errors` alarm fires no later than — and, when the retry ladder straddles a period boundary, one window before — the DLQ-depth alarm (the `Errors` metric is stamped at invocation time, before retries exhaust onto the DLQ). Find and inspect the DLQ — the construct sets no explicit queue name, so its physical name is CloudFormation-generated and *contains* the `WebhookProcessorDlq` construct id:
+
+```bash
+# Find the DLQ URL (physical name contains the construct id)
+aws sqs list-queues --region us-east-1 \
+  --query "QueueUrls[?contains(@, 'WebhookProcessorDlq')]" --output text
+
+# How many failed invocations are parked?
+aws sqs get-queue-attributes --region us-east-1 \
+  --queue-url <DLQ_URL> \
+  --attribute-names ApproximateNumberOfMessages
+
+# Peek at a parked event (the original async-invoke payload + Lambda error context)
+aws sqs receive-message --region us-east-1 \
+  --queue-url <DLQ_URL> --max-number-of-messages 1 \
+  --visibility-timeout 0
+```
+
+The message body is the original async-invoke event; the `RequestContext`/error attributes show why Lambda gave up. Fix the root cause (re-check the processor's IAM grants, AgentCore Browser quota, and the GitHub/Linear token secrets). The event source is GitHub, not the DLQ, so recovery is to **redeliver the webhook from GitHub's UI** (a Lambda async-invoke DLQ has no automatic re-invoke path — the parked messages are event copies for diagnosis). Purge the queue (`aws sqs purge-queue`) once the alarm has cleared and you no longer need the payloads.
+
 ### Screenshot lands on GitHub PR but not on Linear
 
 The GitHub-side post is the primary path; Linear is opt-in and best-effort. Skipping the Linear post is normal if you don't have Linear configured. If you do, look for the processor log line `Linear identifier did not resolve to an issue` — usually means:

docs/src/content/docs/using/Deploy-preview-screenshots-guide.md

@@ -179,6 +179,35 @@ Then tail the function's CloudWatch log group. Common silent skips:
 - `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.
 - `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
+
+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.
+
+**Important:** the processor handler is best-effort by design — it catches its own per-step operational failures (bad token, AgentCore/S3/comment-post errors) and returns success, so those show up as tagged log events (see the section above), **not** as Lambda `Errors`. Both alarms below fire only on faults that *escape* the handler: an init-time crash (missing env at cold start, bundling defect), an unhandled throw in an unguarded path, or the 120s hard timeout. For a swallowed operational failure (e.g. a revoked S3/AgentCore permission), watch the processor **logs**, not these alarms.
+
+- **`WebhookProcessorErrorAlarm`** — fires on the processor's Lambda `Errors` metric: **any** 5-minute period with `>= 1` error alarms (evaluation range 2 periods, 1 datapoint to alarm). Early signal: an invocation is faulting *now*.
+- **`WebhookProcessorDlqDepthAlarm`** — fires when such a failed invocation has survived Lambda's built-in async retries and landed on the DLQ (construct id `WebhookProcessorDlq`; 14-day retention, SSL-enforced). Backstop: a payload is now parked undelivered. (The queue also gets SSE via SQS's service-side default; the construct doesn't set an explicit `encryption`.)
+
+The two catch the same failure class, but the `Errors` alarm fires no later than — and, when the retry ladder straddles a period boundary, one window before — the DLQ-depth alarm (the `Errors` metric is stamped at invocation time, before retries exhaust onto the DLQ). Find and inspect the DLQ — the construct sets no explicit queue name, so its physical name is CloudFormation-generated and *contains* the `WebhookProcessorDlq` construct id:
+
+```bash
+# Find the DLQ URL (physical name contains the construct id)
+aws sqs list-queues --region us-east-1 \
+  --query "QueueUrls[?contains(@, 'WebhookProcessorDlq')]" --output text
+
+# How many failed invocations are parked?
+aws sqs get-queue-attributes --region us-east-1 \
+  --queue-url <DLQ_URL> \
+  --attribute-names ApproximateNumberOfMessages
+
+# Peek at a parked event (the original async-invoke payload + Lambda error context)
+aws sqs receive-message --region us-east-1 \
+  --queue-url <DLQ_URL> --max-number-of-messages 1 \
+  --visibility-timeout 0
+```
+
+The message body is the original async-invoke event; the `RequestContext`/error attributes show why Lambda gave up. Fix the root cause (re-check the processor's IAM grants, AgentCore Browser quota, and the GitHub/Linear token secrets). The event source is GitHub, not the DLQ, so recovery is to **redeliver the webhook from GitHub's UI** (a Lambda async-invoke DLQ has no automatic re-invoke path — the parked messages are event copies for diagnosis). Purge the queue (`aws sqs purge-queue`) once the alarm has cleared and you no longer need the payloads.
+
 ### Screenshot lands on GitHub PR but not on Linear
 
 The GitHub-side post is the primary path; Linear is opt-in and best-effort. Skipping the Linear post is normal if you don't have Linear configured. If you do, look for the processor log line `Linear identifier did not resolve to an issue` — usually means: