fix(api): pentest retry follow-ups from production review
Merged
TO
3mo ago+227+227+012345678901234567890123456789-19-19-01234567890123456789
Follow-ups from the review on the production deploy PR (#3399) for the pentest auto-retry feature. Investigated all 5 findings; fixed the 4 real ones, left 1 that's a deliberate product decision.
Fixed
- Concurrent duplicate failure webhooks → duplicate retries (P1). At-least-once webhook delivery could let two
pentest.faileddeliveries each spawn a retry child for the same parent (duplicate scans + nondeterministic active-attempt pick). Added a unique index onretry_of_provider_run_id(Postgres treats NULLs as distinct, so originals are unaffected) — at most one retry child per parent. The migration dedups first so it's deploy-safe. The losing concurrent spawn's ownership insert fails and is refunded by the existing error path. - Explicit empty check selection dropped on retry (P2).
checks: []was omitted on retry, letting the provider fall back to its default check set. Now the stored selection is preserved faithfully (including empty); stale entries are filtered out without dropping the whole list. - Legacy runs masked pointlessly (P2). A failure is now masked as in-progress only when the run is actually retry-eligible (under the attempt cap and has stored scan params). Pre-feature rows (no scan params) reveal their failure immediately instead of showing "provisioning" for the grace window.
- Malformed zero runtime cap (P3).
"cap 0m"produced "maximum runtime of its time limit"; a non-positive/non-finite cap now falls through to the generic message.
Not changed (deliberate)
- "Retries all failures, including runtime-cap/config." This is the intended behavior — we chose to retry uniformly with the same parameters rather than classify transient vs permanent (agent runtime varies run-to-run, and it's a temporary posture while we're on the current sandbox provider). Not a bug.
Testing
npx jest src/security-penetration-tests → 151 tests green (isolation + serial), typecheck + prettier clean. Added coverage for empty-check preservation, retry-eligibility masking, and the zero-cap message.
Summary by cubic
Fixes pentest auto-retries to dedupe duplicates at the provider, preserve check selection, only mask failures when a retry can happen, and make billing reservations idempotent to avoid double charges. Tightens the runtime-cap message.
- Bug Fixes
- Dedupe concurrent failure webhooks at the provider (
Idempotency-Key: retry:<parentRunId>) and make retry billing reservation idempotent (pending:retry:<parentRunId>), so duplicates create one scan and debit once; user-initiated creates send no key and keep a random reservation id. - Preserve an explicit empty check selection (
checks: []) on retry. - Mask failures as “provisioning” only when retry-eligible: under the attempt cap and when stored params validate via
fromScanParams; legacy or malformed params reveal “failed” immediately. - Show the generic error for non‑positive or absurd runtime caps; only positive, safe‑integer caps use the “maximum runtime” message.
- Dedupe concurrent failure webhooks at the provider (