fix(orchestration): idempotency-claim parent-epic comment handling — stop webhook-redelivery reply spam (aws-samples#247 UX.20)

CRITICAL live-caught: a no-match @bgagent comment on the parent epic
spammed 50+ duplicate disambiguation replies. Root cause: the UX.18
parent-comment handler posts its reply (and 👀) with NO idempotency
guard, and Linear REDELIVERS a comment webhook whenever the handler
exceeds its ~5s ack window (this path does several Linear API calls and
ran 7.7s). Each redelivery re-ran the whole handler → another reply. The
iteration path is deduped by ack_replied_at; the disambiguation path I
added in UX.18 had nothing. (Mitigated live by throttling the Linear
receiver+processor Lambdas to 0 concurrency; 49 spam comments deleted.)

Fix: new store op claimCommentAck — a conditional create-once write keyed
on (orchestration_id, 'ack#<commentId>') with a TTL, mirroring claimRollup.
handleParentEpicCommentTrigger claims BEFORE any side-effect; only the
first delivery proceeds (👀 + match/route/reply), redeliveries no-op. The
marker self-expires via the table's existing ttl attribute.
loadOrchestration now excludes any 'ack#…' (and any '<kind>#' marker) SK
from the children list so the dedup rows can't be mistaken for sub-issues
(real child SKs are UUIDs or '…__integration', never contain '#').

Tests: claimCommentAck (first-wins / redelivery-loses / error-propagates);
loadOrchestration excludes ack# rows; webhook 'redelivery posts EXACTLY
ONE reply' + 'matched iteration dedups to one task'. 58 green across
store+webhook suites.

Author: isadeks

cdk/src/handlers/linear-webhook-processor.ts

@@ -27,7 +27,7 @@ import { resolveLinearOauthToken } from './shared/linear-oauth-resolver';
 import { logger } from './shared/logger';
 import { discoverOrchestration } from './shared/orchestration-discovery';
 import { readConcurrencyBudget, releaseReadyChildren } from './shared/orchestration-release';
-import { deriveOrchestrationId, loadOrchestration, setStatusCommentId, type OrchestrationReleaseContext } from './shared/orchestration-store';
+import { claimCommentAck, deriveOrchestrationId, loadOrchestration, setStatusCommentId, type OrchestrationReleaseContext } from './shared/orchestration-store';
 import { buildIterationInstruction, parseCommentTrigger, type CommentTrigger } from './shared/orchestration-comment-trigger';
 import { parseParentNodeReference, renderParentDisambiguationReply, suggestClosestNode } from './shared/orchestration-parent-comment';
 import { fetchIssueParentId } from './shared/linear-subissue-fetch';
@@ -48,6 +48,12 @@ const DEFAULT_LABEL_FILTER = 'bgagent';
 // budget. Unset → release all roots (back-compat; admission still gates).
 const USER_CONCURRENCY_TABLE = process.env.USER_CONCURRENCY_TABLE_NAME;
 const MAX_CONCURRENT = Number(process.env.MAX_CONCURRENT_TASKS_PER_USER ?? '10');
+/**
+ * TTL (seconds) for the per-comment ack-claim marker (#247 UX.20). Only needs
+ * to outlive Linear's webhook redelivery window (minutes), but we keep a day of
+ * slack so a delayed redelivery still dedups; the row self-expires after.
+ */
+const ACK_CLAIM_TTL_SECONDS = 86_400;
 
 /**
  * Post a Linear comment + ❌ reaction without ever propagating an error.
@@ -720,6 +726,27 @@ async function handleParentEpicCommentTrigger(args: {
 }): Promise<void> {
   const { orchestrationId, snapshot, workspaceId, commentId, replyTargetId, trigger, resolved, registryTableName } = args;
   const feedbackCtx = { linearWorkspaceId: workspaceId, registryTableName };
+
+  // #247 UX.20: claim-once BEFORE any side-effect. Linear redelivers a comment
+  // webhook when the handler exceeds its ~5s ack window (this path does several
+  // Linear API calls and can run >5s), and EACH redelivery would otherwise
+  // re-react + re-post the disambiguation reply — live-caught spamming 50+
+  // duplicate replies. The conditional claim (keyed on this comment id) lets
+  // only the FIRST delivery proceed; redeliveries no-op here. The marker
+  // self-expires via the table TTL. (The iterate path also has its own
+  // createTaskCore idempotency key — this is the outer guard that also covers
+  // the 👀 + the ask-reply, which have no other dedup.)
+  const ttlEpochSeconds = Math.floor(Date.now() / 1000) + ACK_CLAIM_TTL_SECONDS;
+  const won = await claimCommentAck(
+    ddb, ORCHESTRATION_TABLE!, orchestrationId, commentId, new Date().toISOString(), ttlEpochSeconds,
+  );
+  if (!won) {
+    logger.info('A6 comment (parent epic): redelivery — already handled this comment, skipping', {
+      orchestration_id: orchestrationId, comment_id: commentId,
+    });
+    return;
+  }
+
   // ACK immediately — a parent comment is never silently dropped again.
   await reactToComment(feedbackCtx, commentId, EMOJI_STARTED);
 

cdk/src/handlers/shared/orchestration-store.ts

@@ -458,6 +458,45 @@ export async function clearRollupClaim(
   }));
 }
 
+/**
+ * Claim the one-time "I responded to this comment" marker so a webhook
+ * REDELIVERY doesn't re-post (#247 UX.20 — live-caught spam). Linear redelivers
+ * a comment webhook when the handler exceeds its ~5s ack window; without a
+ * claim, the parent-epic disambiguation reply re-posted on every redelivery
+ * (50+ duplicates). Keyed on the orchestration + the triggering comment id, so
+ * the FIRST delivery wins and every redelivery is a no-op. The marker carries a
+ * TTL (the table's ``ttl`` attribute) so these rows self-expire — they're only
+ * needed for the redelivery window. Returns true only for the first caller.
+ *
+ * @param ttlEpochSeconds absolute epoch-seconds expiry for the marker row.
+ */
+export async function claimCommentAck(
+  ddb: DynamoDBDocumentClient,
+  tableName: string,
+  orchestrationId: string,
+  commentId: string,
+  now: string,
+  ttlEpochSeconds: number,
+): Promise<boolean> {
+  try {
+    await ddb.send(new UpdateCommand({
+      TableName: tableName,
+      Key: { orchestration_id: orchestrationId, sub_issue_id: `ack#${commentId}` },
+      // attribute_not_exists on the PK is the standard "create-once" guard —
+      // a replay finds the row present and the condition fails. ``ttl`` is a
+      // DynamoDB reserved keyword → must be aliased via ExpressionAttributeNames.
+      UpdateExpression: 'SET acked_at = :now, #ttl = :ttl',
+      ConditionExpression: 'attribute_not_exists(orchestration_id)',
+      ExpressionAttributeNames: { '#ttl': 'ttl' },
+      ExpressionAttributeValues: { ':now': now, ':ttl': ttlEpochSeconds },
+    }));
+    return true;
+  } catch (err) {
+    if ((err as { name?: string })?.name === 'ConditionalCheckFailedException') return false;
+    throw err;
+  }
+}
+
 /** Sort-key of the parent-meta row. Exported so the reconciler can
  *  separate it from child rows after a Query. */
 export const ORCHESTRATION_META_SK = PARENT_META_SK;
@@ -532,7 +571,11 @@ export async function loadOrchestration(
   }
 
   const children = items
-    .filter((i) => i.sub_issue_id !== PARENT_META_SK)
+    // Exclude the meta row AND non-child marker rows (e.g. ``ack#<commentId>``
+    // dedup markers, #247 UX.20) — only real sub-issue rows are children.
+    // A real child SK is a Linear issue UUID or the ``…__integration`` synthetic
+    // id; markers use a ``<kind>#`` prefix that no real SK has.
+    .filter((i) => i.sub_issue_id !== PARENT_META_SK && !String(i.sub_issue_id).includes('#'))
     .map((i) => i as unknown as OrchestrationChildRow);
 
   const meta: OrchestrationMeta = {

cdk/test/handlers/linear-webhook-processor-orchestration.test.ts

@@ -504,7 +504,20 @@ describe('linear-webhook-processor — #247 A6 comment trigger', () => {
         repo: 'o/r', parent_linear_issue_id: parentIssueId, linear_workspace_id: 'WS',
         linear_identifier: 'ABCA-306', title: 'Add a newsletter signup section', child_task_id: 'task-news',
       };
+      // Stateful ack-claim (#247 UX.20): the conditional Update on ack#<comment>
+      // succeeds the FIRST time and ConditionalCheckFailed on every redelivery.
+      const claimedAcks = new Set<string>();
       ddbSend.mockImplementation(async (cmd: { _type: string; input: Record<string, unknown> }) => {
+        if (cmd._type === 'Update') {
+          const sk = (cmd.input.Key as { sub_issue_id?: string })?.sub_issue_id ?? '';
+          if (sk.startsWith('ack#')) {
+            if (claimedAcks.has(sk)) {
+              throw Object.assign(new Error('claim exists'), { name: 'ConditionalCheckFailedException' });
+            }
+            claimedAcks.add(sk);
+          }
+          return {};
+        }
         if (cmd._type === 'Query' && cmd.input.IndexName === 'LinearIssueIndex') return { Items: [] };
         if (cmd._type === 'Query') return { Items: [meta, footer, news] }; // loadOrchestration (parent's own)
         if (cmd._type === 'Get') {
@@ -575,5 +588,26 @@ describe('linear-webhook-processor — #247 A6 comment trigger', () => {
       expect(createTaskCoreMock).not.toHaveBeenCalled();
       expect(replyToCommentMock).toHaveBeenCalledTimes(1);
     });
+
+    test('#247 UX.20: webhook REDELIVERY of the same parent comment posts EXACTLY ONE reply (no spam)', async () => {
+      mockParentEpic('PARENT-EPIC');
+      const evt = eventWith(parentComment('@bgagent looks great, ship it', 'pc-dup'));
+      // Linear redelivers the same comment webhook 3× (handler exceeded its ack window).
+      await handler(evt);
+      await handler(evt);
+      await handler(evt);
+      // The conditional ack-claim lets only the FIRST delivery act: one 👀, one reply.
+      expect(replyToCommentMock).toHaveBeenCalledTimes(1);
+      expect(reactToCommentMock).toHaveBeenCalledTimes(1);
+    });
+
+    test('#247 UX.20: a matched-iteration parent comment also dedups under redelivery (one task, one ack)', async () => {
+      mockParentEpic('PARENT-EPIC');
+      const evt = eventWith(parentComment('@bgagent for the footer change the tagline', 'pc-iter'));
+      await handler(evt);
+      await handler(evt);
+      expect(createTaskCoreMock).toHaveBeenCalledTimes(1); // one iteration, not two
+      expect(reactToCommentMock).toHaveBeenCalledTimes(1);
+    });
   });
 });

cdk/test/handlers/shared/orchestration-store.test.ts

@@ -24,6 +24,8 @@ import {
   deriveOrchestrationId,
   claimRollup,
   clearRollupClaim,
+  claimCommentAck,
+  loadOrchestration,
   findOrchestrationChildByBranch,
 } from '../../../src/handlers/shared/orchestration-store';
 import type { SubIssueNode } from '../../../src/handlers/shared/linear-subissue-fetch';
@@ -290,6 +292,53 @@ describe('clearRollupClaim — release the claim so a re-completing epic re-sett
   });
 });
 
+describe('claimCommentAck — exactly-once per comment (#247 UX.20 redelivery dedup)', () => {
+  test('first delivery wins → true, conditional create-once on a per-comment SK + TTL', async () => {
+    const ddb = { send: jest.fn().mockResolvedValueOnce({}) };
+    const won = await claimCommentAck(ddb as never, TABLE, 'orch_1', 'cmt-9', NOW, 1781800000);
+    expect(won).toBe(true);
+    const cmd = ddb.send.mock.calls[0][0] as UpdateCommand;
+    expect(cmd).toBeInstanceOf(UpdateCommand);
+    expect(cmd.input.Key).toMatchObject({ orchestration_id: 'orch_1', sub_issue_id: 'ack#cmt-9' });
+    expect(cmd.input.ConditionExpression).toContain('attribute_not_exists(orchestration_id)');
+    expect(cmd.input.ExpressionAttributeValues).toMatchObject({ ':ttl': 1781800000 });
+    // ``ttl`` is a DynamoDB reserved keyword — must be aliased, else the write
+    // 400s with ValidationException (live-caught: the unaliased form errored
+    // out the whole handler, silently dropping the comment).
+    expect(cmd.input.ExpressionAttributeNames).toMatchObject({ '#ttl': 'ttl' });
+    expect(cmd.input.UpdateExpression).toContain('#ttl');
+  });
+
+  test('redelivery of the same comment loses (ConditionalCheckFailed) → false, no throw', async () => {
+    const ddb = { send: jest.fn().mockRejectedValueOnce(Object.assign(new Error('c'), { name: 'ConditionalCheckFailedException' })) };
+    expect(await claimCommentAck(ddb as never, TABLE, 'orch_1', 'cmt-9', NOW, 1781800000)).toBe(false);
+  });
+
+  test('non-conditional error propagates', async () => {
+    const ddb = { send: jest.fn().mockRejectedValueOnce(new Error('throttle')) };
+    await expect(claimCommentAck(ddb as never, TABLE, 'orch_1', 'cmt-9', NOW, 1781800000)).rejects.toThrow('throttle');
+  });
+});
+
+describe('loadOrchestration — marker rows are not children (#247 UX.20)', () => {
+  test('excludes ack#<commentId> marker rows from children (only real sub-issues count)', async () => {
+    const ddb = {
+      send: jest.fn().mockResolvedValueOnce({
+        Items: [
+          { orchestration_id: 'orch_1', sub_issue_id: '#meta', parent_linear_issue_id: 'P', linear_workspace_id: 'WS', repo: 'o/r', platform_user_id: 'u1', child_count: 2 },
+          { orchestration_id: 'orch_1', sub_issue_id: 'uuid-A', depends_on: [], child_status: 'succeeded' },
+          { orchestration_id: 'orch_1', sub_issue_id: 'orch_1__integration', depends_on: ['uuid-A'], child_status: 'succeeded' },
+          { orchestration_id: 'orch_1', sub_issue_id: 'ack#cmt-9', acked_at: NOW, ttl: 1781800000 }, // marker — must NOT be a child
+        ],
+      }),
+    };
+    const snap = await loadOrchestration(ddb as never, TABLE, 'orch_1');
+    expect(snap).not.toBeNull();
+    const ids = snap!.children.map((c) => c.sub_issue_id).sort();
+    expect(ids).toEqual(['orch_1__integration', 'uuid-A']); // ack# row excluded; integration kept
+  });
+});
+
 describe('findOrchestrationChildByBranch (#305 A6)', () => {
   test('queries the ChildBranchIndex GSI by branch and returns the child row', async () => {
     const ddb = makeDdb();