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
- New autoPostAcknowledgment(campaignId, actor) mirrors the VERIFIED
remediateApprovedActions shape: campaign row lock, idempotency check
against an existing acknowledge_user remediation_actions row, insert +
audit inside one transaction, note post after commit (non-fatal on
failure)
- Closes CR-01/WR-01: gives the auto_report webhook path a persisted
record so a repeat ticket-create webhook joining an already-acked
campaign does not re-insert/re-audit/re-post
- Adds Test A/B/C in remediation-service.test.ts covering first-pass
insert, idempotent skip, and non-fatal note-post failure
- remediateApprovedActions now captures the transaction's RemediateResult,
then post-commit checks whether an approved acknowledge_user row was
transitioned this pass (alreadyCompleted === false) and, if so, calls
generateAndPostAcknowledgment(campaignId) exactly once
- Call happens outside the DB transaction (network I/O hazard) and is
wrapped in its own try/catch that logs and swallows failures -- the DB
transition has already committed
- Every other action type (block_sender, purge_message, warn_user,
reset_password, isolate_endpoint, disable_forwarding_rule, quarantine)
remains a simulated status-only transition, unchanged
- Updated top-of-file D-01 doc comment to record the narrow D-04 carve-out
- Tests: acknowledge_user IS posted once when remediated, NOT called for
block_sender/warn_user-only remediation, NOT called on idempotent re-run
of an already-completed acknowledge_user row, and a post rejection does
not propagate out of remediateApprovedActions
- Guards against marking false positive when any approved/completed
remediation exists for the campaign (RemediationConflictError, T-20-04)
- Sets campaigns.status='false_positive' and writes one atomic audit row
recording previousStatus + reason (REMED-05, REMED-06)
- Fixes test mock SQL substring match for the D-04 guard query
- Reworded a header comment to avoid a literal "not_implemented" string
that tripped the D-01 grep acceptance check
