fix(23-06): capture actionId in auto-post audit payload, guard manual re-approval of acknowledge_user

23-06-REVIEW.md found two real defects in the just-merged idempotency fix:
- CR-01: autoPostAcknowledgment's audit payload omitted actionId, which the
  campaign-detail API requires to derive completedAt — every auto-posted
  acknowledge_user row rendered a null completion date in the Action Area UI.
- CR-02: the manual approve/remediate path had no server-side guard against
  re-approving acknowledge_user for a campaign that already got auto-posted —
  only a client-side UI check prevented the exact duplicate-note bug 23-06
  was chartered to close, reachable via a direct API call.

Fixes both: capture RETURNING id from the insert and include it in the audit
payload; add an existence check in approveRemediationActions that rejects
acknowledge_user when already posted for the campaign.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W6RuWdiUiXrPK6FLBHjtpY

lib/services/remediation-service.test.ts

@@ -161,6 +161,35 @@ describe('approveRemediationActions', () => {
     expect(result).toHaveLength(2);
     expect(result[0]).toMatchObject({ id: 'action-1', actionType: 'block_sender', status: 'approved' });
   });
+
+  it('rejects re-approving acknowledge_user when it has already been posted for the campaign (CR-02 fix)', async () => {
+    stage({
+      classification: [{ recommended_actions: ['acknowledge_user'] }],
+      existingAckRows: [{ id: 'auto-posted-action' }],
+    });
+
+    await expect(
+      approveRemediationActions('campaign-1', [{ actionType: 'acknowledge_user' }], 'operator@example.com')
+    ).rejects.toThrow(RemediationValidationError);
+    expect(callsContaining('INSERT INTO remediation_actions')).toHaveLength(0);
+  });
+
+  it('still allows approving acknowledge_user when no prior post exists for the campaign', async () => {
+    stage({
+      classification: [{ recommended_actions: ['acknowledge_user'] }],
+      existingAckRows: [],
+      insertRemediation: [{ id: 'action-1' }],
+    });
+
+    const result = await approveRemediationActions(
+      'campaign-1',
+      [{ actionType: 'acknowledge_user' }],
+      'operator@example.com'
+    );
+
+    expect(result).toHaveLength(1);
+    expect(result[0]).toMatchObject({ id: 'action-1', actionType: 'acknowledge_user', status: 'approved' });
+  });
 });
 
 // =============================================================================
@@ -322,8 +351,8 @@ describe('markCampaignFalsePositive', () => {
 // =============================================================================
 
 describe('autoPostAcknowledgment', () => {
-  it('first pass: inserts one acknowledge_user row, writes one audit row, posts the note once, returns posted:true', async () => {
-    stage({ existingAckRows: [] });
+  it('first pass: inserts one acknowledge_user row, writes one audit row (with actionId), posts the note once, returns posted:true', async () => {
+    stage({ existingAckRows: [], insertRemediation: [{ id: 'auto-action-1' }] });
 
     const result = await autoPostAcknowledgment('campaign-1', 'system:auto_report');
 
@@ -331,6 +360,7 @@ describe('autoPostAcknowledgment', () => {
     expect(insertCalls).toHaveLength(1);
     expect(insertCalls[0].sql).toContain("'acknowledge_user'");
     expect(insertCalls[0].sql).toContain("'completed'");
+    expect(insertCalls[0].sql).toContain('RETURNING id');
     expect(insertCalls[0].params).toEqual(['campaign-1', 'system:auto_report']);
 
     expect(writeAuditEventMock).toHaveBeenCalledTimes(1);
@@ -338,6 +368,7 @@ describe('autoPostAcknowledgment', () => {
       campaignId: 'campaign-1',
       actor: 'system:auto_report',
       eventType: 'remediation_completed',
+      payload: { actionId: 'auto-action-1', actionType: 'acknowledge_user', auto: true },
     });
 
     expect(generateAndPostAcknowledgmentMock).toHaveBeenCalledTimes(1);

lib/services/remediation-service.ts

@@ -109,12 +109,27 @@ export async function approveRemediationActions(
     }
     const recommendedActions = new Set(parseRecommendedActions(classification.recommended_actions));
 
+    const requestsAcknowledgeUser = actions.some((action) => action.actionType === 'acknowledge_user');
+    let acknowledgeUserAlreadyPosted = false;
+    if (requestsAcknowledgeUser) {
+      const existingAckRes = await client.query<{ id: string }>(
+        `SELECT id FROM remediation_actions WHERE campaign_id = $1 AND action_type = 'acknowledge_user' LIMIT 1`,
+        [campaignId]
+      );
+      acknowledgeUserAlreadyPosted = existingAckRes.rows.length > 0;
+    }
+
     for (const action of actions) {
       if (!recommendedActions.has(action.actionType)) {
         throw new RemediationValidationError(
           `Action ${action.actionType} is not a recommended action for this campaign`
         );
       }
+      if (action.actionType === 'acknowledge_user' && acknowledgeUserAlreadyPosted) {
+        throw new RemediationValidationError(
+          'acknowledge_user has already been posted for this campaign'
+        );
+      }
     }
 
     const approved: ApprovedRemediationAction[] = [];
@@ -353,9 +368,10 @@ export async function autoPostAcknowledgment(
       return { inserted: false };
     }
 
-    await client.query(
+    const insertRes = await client.query<{ id: string }>(
       `INSERT INTO remediation_actions (campaign_id, action_type, status, approved_by, approved_at)
-       VALUES ($1, 'acknowledge_user', 'completed', $2, NOW())`,
+       VALUES ($1, 'acknowledge_user', 'completed', $2, NOW())
+       RETURNING id::text AS id`,
       [campaignId, actor]
     );
     await writeAuditEvent(
@@ -363,7 +379,7 @@ export async function autoPostAcknowledgment(
         campaignId,
         actor,
         eventType: 'remediation_completed',
-        payload: { actionType: 'acknowledge_user', auto: true },
+        payload: { actionId: insertRes.rows[0].id, actionType: 'acknowledge_user', auto: true },
       },
       client
     );