fix(pentest): second cubic review pass — refund retry, ux + correctness

API:
- refundOnTerminalFailure no longer swallows errors. Failures now
  propagate through handleWebhook → 5xx response → Maced redelivery →
  rolled-back creditRefundedAt allows retry. Previously a transient
  refund-tx blip would silently drop the customer's refund forever.
- Removed dead grantInitialTrial method. Trial credits are granted
  via Prisma nested-create on Organization in the org-create server
  actions, atomic with org insert. The unused service method just
  invited future divergence; existing orgs were backfilled by the
  20260427000000_pentest_credits migration. Added a comment in
  pentest-credits.service explaining where the trial actually fires.
- Service spec updated (-1 dead test for the deleted method, total 19).

Frontend:
- AgentActivityLog: track open state in React so user collapse isn't
  fought by the next parent re-render. Was rendering as
  `<details open={defaultOpen}>` which forced the panel re-open every
  SWR poll.
- OverviewPane: coverage and stale-target stats now derive from
  `completed` runs only, not the full runs list. A target with only
  failed scans is no longer counted as "covered" or "fresh."
- AgentProgressGrid: clamp `done` to grid size so an out-of-sync
  Maced progress payload (`done > total`) doesn't render extra cells
  past the configured columns.
- usePenetrationTestEvents: added the same final-revalidation pattern
  as usePenetrationTestIssues so the last events written during the
  completion phase aren't missed when polling stops.
- StatusPill: unknown status values now render as "Unknown" instead
  of falling back to "Provisioning" (which would mislead users into
  thinking the run was still starting up).
- RunningDetail useNewFindingHighlights: removed the timeout-cleanup
  that was canceling pending highlight removals when issues changed
  in <2s (during a 3s SWR poll, very common). Each batch now
  schedules its own fire-and-forget removal so they can't step on
  each other.
- RunList: removed unused `overviewActive` prop (declared but never
  consumed) and stopped passing it from SplitView.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: tofikwest

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

@@ -78,23 +78,6 @@ describe('PentestCreditsService', () => {
     });
   });
 
-  describe('grantInitialTrial', () => {
-    it('upserts with create-side defaults and update-side no-op', async () => {
-      mockedDb.pentestCredits.upsert.mockResolvedValue({});
-      await service.grantInitialTrial('org_1');
-      expect(mockedDb.pentestCredits.upsert).toHaveBeenCalledWith({
-        where: { organizationId: 'org_1' },
-        create: {
-          organizationId: 'org_1',
-          balance: 1,
-          totalGranted: 1,
-          lastGrantSource: 'trial',
-        },
-        update: {},
-      });
-    });
-  });
-
   describe('grant', () => {
     it('rejects non-positive amounts', async () => {
       await expect(service.grant('org_1', 0, 'topup')).rejects.toThrow();

apps/api/src/security-penetration-tests/pentest-credits.service.ts

@@ -75,28 +75,15 @@ export class PentestCreditsService {
     };
   }
 
-  /**
-   * Grant the initial trial credit when an org is created. Idempotent —
-   * safe to call repeatedly; subsequent calls no-op so we never accidentally
-   * top a user up just by re-running the org-create hook.
-   */
-  async grantInitialTrial(organizationId: string): Promise<void> {
-    const amount = this.initialTrialAmount;
-    await db.pentestCredits.upsert({
-      where: { organizationId },
-      create: {
-        organizationId,
-        balance: amount,
-        totalGranted: amount,
-        lastGrantSource: 'trial',
-      },
-      // Already exists — leave it alone. Idempotent by design.
-      update: {},
-    });
-    this.logger.log(
-      `[Credits] grantInitialTrial org=${organizationId} amount=${amount}`,
-    );
-  }
+  // Initial trial credits are granted via Prisma's nested-create on
+  // `Organization` in the org-create server actions
+  // (apps/app/src/app/(app)/setup/actions/create-organization*.ts) so
+  // the credit row is inserted in the same transaction as the
+  // organization itself — no chance of an org existing without its
+  // trial credit. Existing orgs were backfilled by migration
+  // `20260427000000_pentest_credits`. There is intentionally no
+  // service-side `grantInitialTrial` method; if a manual re-grant is
+  // ever needed, it goes through the platform-admin grant flow.
 
   /**
    * Add credits from any source. Used for v2 Stripe webhooks (subscription

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

@@ -494,7 +494,10 @@ export class SecurityPenetrationTestsService {
     // Refund the credit on terminal failure events. The user paid for a
     // run that didn't deliver value, so they shouldn't lose the credit.
     // Idempotent via the run row's `creditRefundedAt` column — webhook
-    // redelivery cannot double-credit.
+    // redelivery cannot double-credit. If the refund transaction fails
+    // (e.g. transient DB blip), the error propagates so this handler
+    // returns 5xx and Maced redelivers the webhook — without that, the
+    // customer would silently lose their credit.
     if (event.type === 'pentest.failed' || event.type === 'pentest.cancelled') {
       await this.refundOnTerminalFailure(event.data.pentestId, event.type);
     }
@@ -591,56 +594,50 @@ export class SecurityPenetrationTestsService {
     // wallet write would leave `creditRefundedAt` set with no actual
     // refund, and webhook redelivery would short-circuit forever — the
     // customer never gets their credit back.
-    try {
-      await db.$transaction(async (tx) => {
-        const claimed = await tx.securityPenetrationTestRun.updateMany({
-          where: { providerRunId, creditRefundedAt: null },
-          data: { creditRefundedAt: new Date() },
-        });
+    //
+    // Errors are NOT swallowed here — they propagate to handleWebhook
+    // → Maced sees 5xx → redelivers the webhook. On the redelivery
+    // the rolled-back `creditRefundedAt` is null again, so the claim
+    // re-fires and the refund is retried.
+    await db.$transaction(async (tx) => {
+      const claimed = await tx.securityPenetrationTestRun.updateMany({
+        where: { providerRunId, creditRefundedAt: null },
+        data: { creditRefundedAt: new Date() },
+      });
 
-        if (claimed.count === 0) {
-          // Either we don't own this run (orphan from a fast-click race
-          // — ownership row never persisted) OR the credit has already
-          // been refunded. Either way: do nothing further.
-          this.logger.log(
-            `[Webhook] ${eventType} refund skipped run=${providerRunId} (no ownership row or already refunded)`,
-          );
-          return;
-        }
-
-        const run = await tx.securityPenetrationTestRun.findUnique({
-          where: { providerRunId },
-          select: { organizationId: true },
-        });
-        if (!run) {
-          // Vanishingly rare race; abort the transaction so the claim
-          // rolls back. Webhook redelivery will retry.
-          throw new Error(
-            `Run row vanished after claim for ${providerRunId}`,
-          );
-        }
-
-        // Pass the tx client through so the wallet write happens in
-        // the same transaction as the claim. If this throws (or the
-        // caller's outer try/catch surfaces an error), the claim is
-        // undone.
-        await this.credits.refund(
-          run.organizationId,
-          providerRunId,
-          eventType,
-          tx,
+      if (claimed.count === 0) {
+        // Either we don't own this run (orphan from a fast-click race
+        // — ownership row never persisted) OR the credit has already
+        // been refunded. Either way: do nothing further.
+        this.logger.log(
+          `[Webhook] ${eventType} refund skipped run=${providerRunId} (no ownership row or already refunded)`,
         );
+        return;
+      }
+
+      const run = await tx.securityPenetrationTestRun.findUnique({
+        where: { providerRunId },
+        select: { organizationId: true },
       });
-    } catch (error) {
-      // Log and let the webhook retry. The transaction rolled back, so
-      // `creditRefundedAt` is null again — a redelivered webhook will
-      // re-attempt the refund.
-      this.logger.error(
-        `[Webhook] ${eventType} refund transaction failed run=${providerRunId}: ${
-          error instanceof Error ? error.message : String(error)
-        }`,
+      if (!run) {
+        // Vanishingly rare race; abort the transaction so the claim
+        // rolls back. Webhook redelivery will retry.
+        throw new Error(
+          `Run row vanished after claim for ${providerRunId}`,
+        );
+      }
+
+      // Pass the tx client through so the wallet write happens in
+      // the same transaction as the claim. If this throws, the claim
+      // is undone and the error propagates up to handleWebhook → Maced
+      // sees 5xx and redelivers, allowing retry.
+      await this.credits.refund(
+        run.organizationId,
+        providerRunId,
+        eventType,
+        tx,
       );
-    }
+    });
   }
 
   private formatDurationMs(ms: number): string {

apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/AgentActivityLog.tsx

@@ -1,5 +1,6 @@
 'use client';
 
+import { useState } from 'react';
 import { cn } from '@trycompai/design-system/cn';
 import type { PentestAgentEvent } from '@/lib/security/penetration-tests-client';
 
@@ -46,14 +47,20 @@ export function AgentActivityLog({
   events,
   defaultOpen = true,
 }: AgentActivityLogProps) {
+  // Track open state ourselves so the user can collapse the details
+  // panel without React re-forcing it open on the next render. With
+  // `open={defaultOpen}` as a controlled prop the user's collapse
+  // would be undone on every parent re-render (e.g. SWR poll).
+  const [open, setOpen] = useState(defaultOpen);
   const recent = [...events]
     .filter(isCustomerVisible)
     .sort((a, b) => b.timestamp - a.timestamp)
     .slice(0, 200);
 
   return (
     <details
-      open={defaultOpen}
+      open={open}
+      onToggle={(e) => setOpen((e.currentTarget as HTMLDetailsElement).open)}
       className="overflow-hidden rounded-[var(--radius)] border border-border"
     >
       <summary className="flex cursor-pointer items-center justify-between gap-3 px-4 py-3 text-sm hover:bg-muted">

apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/AgentProgressGrid.tsx

@@ -22,8 +22,13 @@ export function AgentProgressGrid({
   className,
 }: AgentProgressGridProps) {
   const count = Math.max(total, 1);
-  const running = done < count ? 1 : 0;
-  const pending = Math.max(count - done - running, 0);
+  // Clamp `done` to the grid size — if Maced ever reports done > total
+  // (rare, but possible when their progress payload is briefly out of
+  // sync), we'd otherwise render extra cells past the configured columns
+  // and break the visual width.
+  const doneClamped = Math.min(Math.max(done, 0), count);
+  const running = doneClamped < count ? 1 : 0;
+  const pending = Math.max(count - doneClamped - running, 0);
 
   return (
     <div
@@ -32,9 +37,9 @@ export function AgentProgressGrid({
         className,
       )}
       style={{ gridTemplateColumns: `repeat(${count}, minmax(0, 1fr))` }}
-      aria-label={`Agents: ${done} of ${count} complete`}
+      aria-label={`Agents: ${doneClamped} of ${count} complete`}
     >
-      {Array.from({ length: done }).map((_, i) => (
+      {Array.from({ length: doneClamped }).map((_, i) => (
         <span key={`done-${i}`} className="bg-primary" />
       ))}
       {running > 0 ? (

apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/OverviewPane.tsx

@@ -128,12 +128,17 @@ function PostureOverview({
   onDownloadMarkdown,
   onDownloadPdf,
 }: PostureOverviewProps) {
-  const targets = uniqueTargets(runs);
+  // Coverage and stale-target stats use ONLY completed runs — a target
+  // that's only ever had failed/cancelled scans isn't truly "covered,"
+  // and a target whose latest scan failed shouldn't reset the staleness
+  // clock. The full `runs` list is only used for the recent activity
+  // sidebar elsewhere.
+  const targets = uniqueTargets(completed);
   const lastScan = mostRecent(completed);
   const avgDuration = avgDurationMs(completed);
   const scansLast30d = countWithin(completed, 30 * 24 * 60 * 60 * 1000);
   const recentScans = sortByUpdatedDesc(completed).slice(0, 6);
-  const staleTargets = computeStaleTargets(runs, targets);
+  const staleTargets = computeStaleTargets(completed, targets);
 
   return (
     <div className="min-h-0 overflow-y-auto">

apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/RunList.tsx

@@ -14,8 +14,6 @@ interface RunListProps {
   runs: PentestRun[];
   selectedRunId: string | null;
   onCreateClick: () => void;
-  /** When true, highlights an implicit "Overview" state (no selection). */
-  overviewActive?: boolean;
   /** Spendable credit balance — drives the "X runs left" badge. */
   balance?: number;
   /** True when the user has used their initial trial (balance 0, totalGranted > 0). */

apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/RunningDetail.tsx

@@ -157,15 +157,20 @@ function useNewFindingHighlights(
       return next;
     });
 
-    const timer = window.setTimeout(() => {
+    // Schedule per-batch removal independently — fire and forget.
+    // Don't `clearTimeout` on cleanup: if `issues` changes in <2s
+    // (very common during a live scan polling at 3s), the cleanup
+    // would cancel the pending removal and the highlight class would
+    // stick to those rows forever. Each scheduled removal targets only
+    // the IDs from its batch, so multiple in-flight timers can't
+    // step on each other.
+    window.setTimeout(() => {
       setHighlighted((prev) => {
         const next = new Set(prev);
         for (const id of newlyLanded) next.delete(id);
         return next;
       });
     }, 2000);
-
-    return () => window.clearTimeout(timer);
   }, [issues]);
 
   return highlighted;

apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/SplitView.tsx

@@ -159,7 +159,6 @@ export function SplitView({
           runs={reports as PentestRun[]}
           selectedRunId={selectedRunId}
           onCreateClick={goToCreate}
-          overviewActive={selectedRunId === null && !isCreateMode}
           balance={balance}
           trialUsed={trialUsed}
         />

apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/StatusPill.tsx

@@ -59,10 +59,18 @@ const STATUS_CONFIG: Record<
   },
 };
 
-const CONFIG_DEFAULT = STATUS_CONFIG.provisioning;
+// Fallback for status values we don't know how to render. Better to
+// show "Unknown" than to silently render an unrelated status (e.g.
+// previously this defaulted to "Provisioning", which would mislead
+// users into thinking the scan was still starting up).
+const CONFIG_UNKNOWN: { label: string; dotClass: string; textClass: string } = {
+  label: 'Unknown',
+  dotClass: 'bg-muted-foreground',
+  textClass: 'text-muted-foreground',
+};
 
 export function StatusPill({ status, className }: StatusPillProps) {
-  const config = STATUS_CONFIG[status as StatusKind] ?? CONFIG_DEFAULT;
+  const config = STATUS_CONFIG[status as StatusKind] ?? CONFIG_UNKNOWN;
 
   return (
     <span