fix(api): pentest retry follow-ups from production review

[tofikwest]

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.failed deliveries each spawn a retry child for the same parent (duplicate scans + nondeterministic active-attempt pick). Added a unique index on retry_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.

^{Written for commit d0b3d4e. Summary will update on new commits.}

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
comp-framework-editor	Ready	Preview, Comment	Jul 14, 2026 2:31am

2 Skipped Deployments

Project	Deployment	Actions	Updated (UTC)
app	Skipped		Jul 14, 2026 2:31am
portal	Skipped		Jul 14, 2026 2:31am

[cubic-dev-ai]

All reported issues were addressed across 8 files

Confidence score: 5/5

• Safe to merge after the addressed issues were fixed.

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[tofikwest]

Addressed in 97820f454. Two of these (1 & 2) were genuinely right and pushed me to a better fix:

• P1 (concurrent webhooks launch duplicate provider scans): Correct — the unique index only deduped DB rows after both requests had already called Maced, orphaning the loser's scan. Fixed properly with a Maced idempotency key (retry:<parentRunId>): concurrent duplicate pentest.failed webhooks now return the same provider run, so only one scan runs and the existing providerRunId upsert collapses the ownership rows.
• P1 (migration DELETE orphans runs): Eliminated — with the idempotency key, duplicates never form, so I dropped the unique index and its dedup migration entirely. No destructive row-delete.
• P2 (eligibility used shallow != null): Fixed — retry-eligibility (for status masking) now reuses fromScanParams validation, so malformed-but-non-null params reveal the failure immediately instead of masking it.
• P3 (oversized cap → "1e+24 minutes"): Fixed — the runtime-cap message now requires Number.isSafeInteger; absurd caps fall through to the generic message.

Net effect: idempotency is now enforced at the provider (where duplicate work actually happens), not just at DB insert time, and the risky migration is gone.

Tests: 154 green (isolation + serial), typecheck + prettier clean. Added coverage for the idempotency key (present on retry, absent on user creates), malformed-params reveal, and the oversized-cap message.

[cubic-dev-ai]

All reported issues were addressed across 6 files (changes from recent commits).

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

1 issue found across 2 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/api/src/security-penetration-tests/security-penetration-tests.service.ts">

<violation number="1" location="apps/api/src/security-penetration-tests/security-penetration-tests.service.ts:363">
P1: A transient or ambiguous Maced failure in one duplicate webhook can refund the shared `pending:retry:<parent>` reservation while the concurrent call succeeds, leaving the retry with no net allowance charge. Reservation/refund state needs to be tied to the winning create outcome, or a shared reservation must not be refunded until the provider result is known.</violation>
</file>

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

P1: A transient or ambiguous Maced failure in one duplicate webhook can refund the shared pending:retry:<parent> reservation while the concurrent call succeeds, leaving the retry with no net allowance charge. Reservation/refund state needs to be tied to the winning create outcome, or a shared reservation must not be refunded until the provider result is known.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/security-penetration-tests/security-penetration-tests.service.ts, line 363:

<comment>A transient or ambiguous Maced failure in one duplicate webhook can refund the shared `pending:retry:<parent>` reservation while the concurrent call succeeds, leaving the retry with no net allowance charge. Reservation/refund state needs to be tied to the winning create outcome, or a shared reservation must not be refunded until the provider result is known.</comment>

<file context>
@@ -354,7 +354,15 @@ export class SecurityPenetrationTestsService {
+    // so duplicates debit exactly once — otherwise two random ids would debit
+    // twice and only one would land on the shared ownership row, orphaning the
+    // other. User-initiated creates keep a unique random id.
+    const billingUsageSourceId = lineage.retryOfProviderRunId
+      ? `pending:retry:${lineage.retryOfProviderRunId}`
+      : `pending:${randomUUID()}`;
</file context>

[claudfuen]

🎉 This PR is included in version 3.101.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀