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

apps/api/src/security-penetration-tests/pentest-lineage.util.spec.ts

@@ -109,5 +109,17 @@ describe('collapsedStatus', () => {
         }),
       ).toBe('failed');
     });
+
+    it('reveals failed immediately when not retry-eligible (e.g. no scan params)', () => {
+      expect(
+        collapsedStatus({
+          activeStatus: 'failed',
+          attemptNumber: 1,
+          retryEligible: false,
+          failedAtMs: now - 5_000, // well within grace, but can't retry
+          nowMs: now,
+        }),
+      ).toBe('failed');
+    });
   });
 });

apps/api/src/security-penetration-tests/pentest-lineage.util.ts

@@ -34,7 +34,9 @@ export const RETRY_GRACE_MS = 10 * 60 * 1000;
  * - `completed` / in-progress statuses pass through unchanged.
  * - `cancelled` is terminal and never retried → always revealed.
  * - `failed`:
- *     - lineage exhausted (`attemptNumber >= maxAttempts`) → revealed.
+ *     - not retry-eligible (lineage exhausted, or the run can't actually be
+ *       re-run — e.g. a pre-feature row with no stored scan params) → revealed
+ *       immediately, so it's never masked as in-progress pointlessly.
  *     - otherwise masked as `provisioning` (a retry is expected) while inside
  *       the grace window; revealed once the window elapses without a retry
  *       appearing, or when the failure time is unknown (can't bound the mask).
@@ -44,19 +46,24 @@ export function collapsedStatus(params: {
   attemptNumber: number;
   failedAtMs: number | null;
   nowMs: number;
+  /** Whether this run can actually be auto-retried (has re-runnable params). */
+  retryEligible?: boolean;
   maxAttempts?: number;
   graceMs?: number;
 }): PentestRunStatus {
   const { activeStatus, attemptNumber, failedAtMs, nowMs } = params;
   const maxAttempts = params.maxAttempts ?? MAX_ATTEMPTS;
   const graceMs = params.graceMs ?? RETRY_GRACE_MS;
+  const retryEligible = params.retryEligible ?? true;
 
   // Only `failed` triggers an auto-retry, so only `failed` is ever masked.
   if (activeStatus !== 'failed') {
     return activeStatus;
   }
 
-  if (attemptNumber >= maxAttempts) {
+  // Reveal immediately if no retry can happen — lineage exhausted or the run
+  // isn't re-runnable — so a failure that will never retry isn't masked.
+  if (attemptNumber >= maxAttempts || !retryEligible) {
     return 'failed';
   }
   if (failedAtMs === null || nowMs - failedAtMs >= graceMs) {

apps/api/src/security-penetration-tests/pentest-run-error.util.spec.ts

@@ -68,6 +68,22 @@ describe('toCustomerFacingError', () => {
       expect(message).not.toContain('1 minutes');
     });
 
+    it('falls back to the generic message for a malformed zero cap', () => {
+      const message = toCustomerFacingError(
+        'Hard timeout: workflow ran 0m (cap 0m)',
+      );
+      expect(message).not.toContain('maximum runtime');
+      expect(message).toContain("The scan couldn't be completed");
+    });
+
+    it('falls back to the generic message for an absurd oversized cap', () => {
+      const message = toCustomerFacingError(
+        'Hard timeout: workflow ran 1m (cap 999999999999999999999999m)',
+      );
+      expect(message).not.toContain('maximum runtime');
+      expect(message).toContain("The scan couldn't be completed");
+    });
+
     it('classifies the "; Daytona unreachable" variant as infra, not runtime cap', () => {
       const message = toCustomerFacingError(
         'Hard timeout: 4m (cap 120m); Daytona unreachable',

apps/api/src/security-penetration-tests/pentest-run-error.util.ts

@@ -76,8 +76,13 @@ export function toCustomerFacingError(
   // has already claimed the "; Daytona unreachable" variant.
   const capMatch =
     /hard timeout:\s*workflow ran\s+\d+m\s*\(cap\s+(\d+)m\)/i.exec(raw);
-  if (capMatch) {
-    const cap = humanizeMinutes(Number(capMatch[1]));
+  const capMinutes = capMatch ? Number(capMatch[1]) : NaN;
+  // Only use the runtime-cap message for a sane, positive whole-minute cap — a
+  // malformed "cap 0m" would render "maximum runtime of its time limit", and an
+  // absurd oversized cap would render e.g. "1e+24 minutes". Anything else falls
+  // through to the generic message.
+  if (Number.isSafeInteger(capMinutes) && capMinutes > 0) {
+    const cap = humanizeMinutes(capMinutes);
     return (
       `Your scan reached its maximum runtime of ${cap} before completing. ` +
       "You won't be charged for this scan. Large targets may need a lighter " +

apps/api/src/security-penetration-tests/security-penetration-tests.service.spec.ts

@@ -164,6 +164,8 @@ describe('SecurityPenetrationTestsService', () => {
           return Promise.resolve({
             providerRunId: where.rootRunId,
             attemptNumber: 1,
+            // Retry-eligible by default (has stored scan params).
+            scanParams: { targetUrl: 'https://app.example.com' },
           });
         }
         return Promise.resolve(null);
@@ -215,6 +217,13 @@ describe('SecurityPenetrationTestsService', () => {
     return input instanceof Request ? input.url : String(input);
   }
 
+  function getRequestHeader(name: string, callIndex = 0): string | null {
+    const [input, init] = fetchMock.mock.calls[callIndex];
+    const headers =
+      input instanceof Request ? input.headers : new Headers(init?.headers);
+    return headers.get(name);
+  }
+
   it('lists reports with organization context', async () => {
     const expectedPayload = [
       {
@@ -1157,6 +1166,20 @@ describe('SecurityPenetrationTestsService', () => {
           pipelineTesting: true,
         }),
       );
+      // Deterministic idempotency key tied to the parent → Maced dedupes
+      // concurrent duplicate failure webhooks into one provider scan.
+      expect(getRequestHeader('Idempotency-Key')).toBe('retry:run_orig');
+      // Deterministic billing reservation id (idempotent on sourceResourceId)
+      // so concurrent duplicates debit the allowance exactly once.
+      expect(
+        mockBillingEntitlementsService.tryConsumeIncludedUsageForProduct,
+      ).toHaveBeenCalledWith(
+        expect.objectContaining({
+          organizationId: 'org_123',
+          productKey: 'pentest',
+          sourceResourceId: 'pending:retry:run_orig',
+        }),
+      );
       // The user's briefing survives the round-trip (re-persisted for any
       // further retry).
       expect(mockedDb.securityPenetrationTestRun.upsert).toHaveBeenCalledWith(
@@ -1175,6 +1198,43 @@ describe('SecurityPenetrationTestsService', () => {
       );
     });
 
+    it('preserves an explicit empty check selection on retry', async () => {
+      mockedDb.securityPenetrationTestRun.findUnique.mockResolvedValueOnce({
+        organizationId: 'org_123',
+        attemptNumber: 1,
+        rootRunId: 'run_orig',
+        scanParams: { targetUrl: 'https://app.example.com', checks: [] },
+      });
+      fetchMock.mockResolvedValueOnce(
+        new Response(
+          JSON.stringify({ id: 'run_retry', status: 'provisioning' }),
+          { status: 200 },
+        ),
+      );
+
+      await service['maybeAutoRetry']('run_orig');
+
+      // Empty selection is sent through, not dropped (which would let the
+      // provider fall back to its default check set).
+      const createBody = await getRequestBody();
+      expect(createBody.checks).toEqual([]);
+    });
+
+    it('does not send an idempotency key for user-initiated creates', async () => {
+      fetchMock.mockResolvedValueOnce(
+        new Response(
+          JSON.stringify({ id: 'run_new', status: 'provisioning' }),
+          { status: 200 },
+        ),
+      );
+
+      await service.createReport('org_123', {
+        targetUrl: 'https://app.example.com',
+      });
+
+      expect(getRequestHeader('Idempotency-Key')).toBeNull();
+    });
+
     it('blocks auto-retry across the whole lineage when a run is cancelled', async () => {
       mockedDb.securityPenetrationTestRun.findUnique.mockResolvedValueOnce({
         rootRunId: 'run_orig',
@@ -1329,6 +1389,7 @@ describe('SecurityPenetrationTestsService', () => {
       mockedDb.securityPenetrationTestRun.findFirst.mockResolvedValueOnce({
         providerRunId: 'run_orig',
         attemptNumber: 1,
+        scanParams: { targetUrl: 'https://a.com' }, // retry-eligible
       });
       const recent = new Date().toISOString();
       fetchMock.mockResolvedValueOnce(
@@ -1352,6 +1413,69 @@ describe('SecurityPenetrationTestsService', () => {
       expect(report.failedReason).toBeNull();
     });
 
+    it('reveals a legacy failed run (no scan params) immediately instead of masking', async () => {
+      mockedDb.securityPenetrationTestRun.findUnique.mockResolvedValue({
+        organizationId: 'org_123',
+        rootRunId: 'run_legacy',
+      });
+      // Pre-feature row: attempt 1 but no scanParams → not retry-eligible.
+      mockedDb.securityPenetrationTestRun.findFirst.mockResolvedValueOnce({
+        providerRunId: 'run_legacy',
+        attemptNumber: 1,
+        scanParams: null,
+      });
+      const recent = new Date().toISOString();
+      fetchMock.mockResolvedValueOnce(
+        new Response(
+          JSON.stringify({
+            id: 'run_legacy',
+            status: 'failed',
+            targetUrl: 'https://a.com',
+            createdAt: recent,
+            updatedAt: recent,
+            error: 'some failure',
+          }),
+          { status: 200 },
+        ),
+      );
+
+      const report = await service.getReport('org_123', 'run_legacy');
+
+      expect(report.status).toBe('failed');
+    });
+
+    it('reveals a failed run with malformed (non-null) scan params instead of masking', async () => {
+      mockedDb.securityPenetrationTestRun.findUnique.mockResolvedValue({
+        organizationId: 'org_123',
+        rootRunId: 'run_bad',
+      });
+      // Non-null but invalid params (no targetUrl) → fromScanParams rejects →
+      // not retry-eligible.
+      mockedDb.securityPenetrationTestRun.findFirst.mockResolvedValueOnce({
+        providerRunId: 'run_bad',
+        attemptNumber: 1,
+        scanParams: { nonsense: true },
+      });
+      const recent = new Date().toISOString();
+      fetchMock.mockResolvedValueOnce(
+        new Response(
+          JSON.stringify({
+            id: 'run_bad',
+            status: 'failed',
+            targetUrl: 'https://a.com',
+            createdAt: recent,
+            updatedAt: recent,
+            error: 'some failure',
+          }),
+          { status: 200 },
+        ),
+      );
+
+      const report = await service.getReport('org_123', 'run_bad');
+
+      expect(report.status).toBe('failed');
+    });
+
     it('reveals a clean, white-labeled failure once the lineage is exhausted', async () => {
       mockedDb.securityPenetrationTestRun.findUnique.mockResolvedValue({
         organizationId: 'org_123',