feat(app-blocks): W13 P3a PR-b — off-site approve/reject + assertListingAssetsComplete wiring (dark) (#2954)

* feat(app-blocks): W13 P3a — off-site approve/reject + assertListingAssetsComplete wiring (dark) PR-b of the App Store off-site (external-link) listing flow. Extends offsite-listing.service (PR-a, on main) with the moderator approve/reject state machine and ACTIVATES the dark P1 mandatory-asset gate at approve. Everything stays dark behind app-blocks-author / moderatorProcedure — no UI. - approveExternalRequest (moderatorProcedure): loads the request + its draft AppListing, asserts pending, then enforces two gates BEFORE any mutation: (1) assertListingAssetsComplete(listing) — THE P3 activation: approve FAILS BAD_REQUEST { missing } unless icon + cover + >=1 screenshot (a screenshot whose Image was deleted / imageId null does NOT count, mirroring getListingAssets); (2) re-validates the STORED externalUrl (defense-in-depth, a non-https stored value blocks approve). Then, in ONE transaction, flips the request pending->approved (status-guarded TOCTOU), flips the listing draft->approved (status-guarded), sets reviewedBy*/reviewedAt/approvalNotes, and supersedes any sibling pending request for the slug (parity w/ the on-site publish-request approve). - rejectExternalRequest (moderatorProcedure): requires rejectionReason >=10; flips the request pending->rejected + reviewedBy*, and DELETES the draft AppListing (status-guarded deleteMany({id, status:'draft'}) — releases the slug, can never remove an approved listing). - Router: wire approveExternalRequest / rejectExternalRequest on the appListings router as moderatorProcedure; new approveExternalRequestSchema / rejectExternalRequestSchema (mirror the on-site approve/reject shapes). Service failures map to BAD_REQUEST with the message (mirrors blocks.approve/reject). Locked decisions honored: - Reject deletes the draft (releases the slug); approve flips draft->approved so the approved-only read path surfaces it in the store. - v1 ALLOWS mod self-approve (reviewer == submitter) — trusted, enables single-mod dogfood + the approve e2e. A reviewer!=submitter restriction is DEFERRED to GA/P3b (noted in-code). Self-approve is NOT blocked. - No schema/DDL change, no migration; no ...input spread (update data built explicitly). Tests: offsite-listing.schema (+8 approve/reject shape), offsite-listing.service (+17 approve/reject: happy-path, gate-blocked per missing asset + all-present pass, non-pending, stored-URL re-validation, supersede, TOCTOU, mod self-approve allowed), router authz matrix (+7: both procs moderatorProcedure, non-mod FORBIDDEN, gate error -> BAD_REQUEST) — 92 in the three files, all green. New e2e preview-apps-external-approve.spec.ts (reject path + approve-gate path, mod, self-cleaning; approve-SUCCESS->store deferred to PR-c since there is no delete-approved path yet — noted in the spec header). Runs in Tekton pr-smoke-test, not locally. PR-c (UI: submit form + kind-aware review queue) and PR-d (#2821 retirement) follow as separate PRs off main. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(app-blocks): W13 P3a post-audit — primary-gate-in-tx, error mapping, atomic reject Post-audit hardening of the off-site listing approve/reject flow (PR #2954): 1. Gate on the PRIMARY inside the approve tx. assertListingAssetsComplete previously read iconId/coverId + the screenshot count via dbRead (replica), but the sibling asset mutators write to dbWrite — so under replica lag + a concurrent owner asset-edit the gate could pass on stale-complete state. The authoritative gate now re-reads iconId/coverId + the imageId-bearing screenshot count via the tx client (primary), row-consistent with the status flip; the cheap replica pre-tx check is retained as a fail-fast. 2. Proper error-code mapping in the router approve/reject catch. A new mapOffsiteError passes shaped TRPCErrors through, maps typed OffsiteRequestError codes (NOT_FOUND->NOT_FOUND, NOT_OWNED->FORBIDDEN, NOT_PENDING->BAD_REQUEST), and turns any unexpected infra/Prisma throw into INTERNAL_SERVER_ERROR with a generic message (raw error kept only on cause) — replacing the blanket BAD_REQUEST + raw-message that mis-coded typed failures and leaked infra messages to moderators. 3. Atomic reject. The request flip + draft delete are now wrapped in one $transaction (parity with approve) so a crash between them can't orphan a hidden draft listing squatting the slug. Tests: dbRead/dbWrite split into DISTINCT mocks so the gate's primary reads are asserted (incl. a replica-lag divergence test where the replica reads complete but the primary is incomplete -> approve BLOCKED); added typed->proper-code and untyped->INTERNAL_SERVER_ERROR (no message leak) router tests; added a reject-atomicity test. 147 related unit tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(app-blocks/offsite): TS2352 in mapOffsiteError — cast Error via {code?:unknown} Error -> {code:string} is an invalid direct cast (missing required prop); the line-124 guard already narrows via {code?:unknown} + typeof===string, so read it the same way. Would fail the Tekton typecheck otherwise. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-09-20 22:08:18 +08:00 · 2026-07-06 15:51:34 -05:00
parent 4cfeb05d9a
commit ab1c907f77
7 changed files with 1299 additions and 96 deletions
@@ -0,0 +1,214 @@
+import { expect, test } from '@playwright/test';
+import type { APIRequestContext } from '@playwright/test';
+import { storageStatePath } from './preview-fixtures';
+import { trpcMutation, trpcQuery } from './preview-trpc';
+
+/**
+ * Preview-e2e: App Blocks W13 P3a — OFF-SITE (external-link) APPROVE/REJECT leg
+ * (PR-b), run as `mod`. Exercises the two SAFE + SELF-CLEANING moderation paths:
+ *
+ *   (1) REJECT: submit (mod, per-preview slug) → `rejectExternalRequest(reason)`
+ *       → the request leaves the pending queue and its DRAFT listing is deleted
+ *       (reject is terminal + releases the slug → self-cleaning, no leftover row).
+ *
+ *   (2) APPROVE-GATE: submit (mod, NO assets — no icon/cover/screenshot) →
+ *       `approveExternalRequest` → asserts a BAD_REQUEST (the dark P1
+ *       `assertListingAssetsComplete` gate fires: "missing required assets"),
+ *       proving approve is BLOCKED without assets. Then `withdrawExternalRequest`
+ *       cleans the still-pending draft.
+ *
+ * WHY NOT an approve-SUCCESS → store-render spec here: a successful approve leaves
+ * an APPROVED listing with NO delete path in P3a (approve is one-way; there is no
+ * un-approve / delete-approved proc yet), which would POLLUTE the shared dev store
+ * across concurrent previews. That path (approve success + the store Visit-anchor
+ * invariant) is DEFERRED to PR-c (the UI + a listing-management surface). This spec
+ * deliberately only drives the two self-cleaning paths.
+ *
+ * ROLE — why `mod`: `submit/withdrawExternalRequest` are `appDeveloperProcedure`
+ * (`app-blocks-author`) and `approve/rejectExternalRequest`/`listPendingRequests`
+ * are `moderatorProcedure`. The `mod` fixture satisfies BOTH (mods author via the
+ * app-blocks-author floor), and v1 ALLOWS mod self-approve (reviewer==submitter),
+ * so the whole leg runs as a single mod — like every sibling apps smoke spec.
+ *
+ * GATES (Tekton `pr-smoke-test` is authoritative — do NOT run browser-mode locally
+ * on NixOS).
+ *
+ * SAFE + SELF-CLEANING (the dev DB is shared across concurrent previews):
+ *   - Each scenario uses its OWN per-preview slug so two previews never collide on
+ *     `AppListing.slug @unique`. A same-preview re-run pre-withdraws any leftover
+ *     pending row before submitting.
+ *   - No asset upload / no successful approve → NO Image rows, NO Tekton build, NO
+ *     CF DNS, NO approved store row.
+ *   - We withdraw in `finally` (deletes the draft + releases the slug) so a
+ *     mid-test failure leaves nothing behind.
+ */
+
+const ROLE = 'mod' as const;
+const PREVIEW_URL = process.env.PREVIEW_URL ?? '';
+
+/** Per-preview + per-scenario slug so concurrent previews never collide. */
+function previewSlug(suffix: string): string {
+  let label = 'local';
+  try {
+    label = new URL(PREVIEW_URL).hostname.split('.')[0] || 'local';
+  } catch {
+    /* fall through to default */
+  }
+  const sanitized = label.toLowerCase().replace(/[^a-z0-9-]/g, '-');
+  const slug = `ci-ext-${suffix}-${sanitized}`.slice(0, 40).replace(/-+$/, '');
+  return /[a-z0-9]$/.test(slug) ? slug : `${slug}0`;
+}
+
+const EXTERNAL_URL = 'https://example.com/ci-smoke-external-approve';
+
+type SubmitResult = { listingId: string; publishRequestId: string; slug: string };
+type PendingItem = { id: string; slug: string; appListingId: string | null };
+type PendingList = { items: PendingItem[]; nextCursor: string | null };
+
+function submitInput(slug: string) {
+  return {
+    slug,
+    name: 'CI Smoke — external approve/reject (P3a PR-b)',
+    externalUrl: EXTERNAL_URL,
+    tagline: 'a pure external-link app',
+    category: 'utility',
+    contentRating: 'g',
+    changelog: 'ci-smoke approve/reject',
+  };
+}
+
+/** Page the oldest-first pending queue to find our row by slug. */
+async function findPendingBySlug(
+  request: APIRequestContext,
+  slug: string
+): Promise<PendingItem | null> {
+  let cursor: string | null = null;
+  for (let page = 0; page < 25; page++) {
+    const input: { limit: number; cursor?: string } = { limit: 100 };
+    if (cursor) input.cursor = cursor;
+    const list = await trpcQuery<PendingList>(request, 'appListings.listPendingRequests', input);
+    const hit = list.items.find((i) => i.slug === slug);
+    if (hit) return hit;
+    if (!list.nextCursor) break;
+    cursor = list.nextCursor;
+  }
+  return null;
+}
+
+/** Best-effort: withdraw any leftover pending row for this slug (self-clean). */
+async function withdrawPendingForSlug(
+  request: APIRequestContext,
+  slug: string
+): Promise<void> {
+  const row = await findPendingBySlug(request, slug).catch(() => null);
+  if (row?.id) {
+    await trpcMutation(request, 'appListings.withdrawExternalRequest', {
+      publishRequestId: row.id,
+    }).catch(() => {});
+  }
+}
+
+test.describe('App Blocks P3a PR-b: off-site approve/reject (mod, self-cleaning)', () => {
+  test.use({ storageState: storageStatePath(ROLE) });
+
+  test('REJECT: submit → reject(reason) → request leaves the pending queue (draft deleted)', async ({
+    page,
+  }) => {
+    const SLUG = previewSlug('rej');
+    await page.goto('/', { waitUntil: 'domcontentloaded' });
+    const request = page.request;
+
+    let publishRequestId: string | null = null;
+    try {
+      await withdrawPendingForSlug(request, SLUG);
+
+      const result = await trpcMutation<SubmitResult>(
+        request,
+        'appListings.submitExternalListing',
+        submitInput(SLUG)
+      );
+      publishRequestId = result.publishRequestId;
+      expect(result.slug, 'slug echoes the submission').toBe(SLUG);
+
+      // It's in the pending queue before review.
+      const pending = await findPendingBySlug(request, SLUG);
+      expect(pending, 'the submitted request is pending before review').not.toBeNull();
+
+      // REJECT (reason ≥10) — terminal; deletes the draft listing.
+      await trpcMutation(request, 'appListings.rejectExternalRequest', {
+        publishRequestId,
+        rejectionReason: 'ci-smoke reject: not a real app, rejecting',
+      });
+      publishRequestId = null; // rejected + draft deleted — nothing to clean
+
+      // Gone from the pending queue.
+      const afterReject = await findPendingBySlug(request, SLUG);
+      expect(afterReject, 'the rejected request no longer appears in the pending queue').toBeNull();
+    } finally {
+      if (publishRequestId) {
+        await trpcMutation(request, 'appListings.withdrawExternalRequest', {
+          publishRequestId,
+        }).catch(() => {});
+      } else {
+        await withdrawPendingForSlug(request, SLUG);
+      }
+    }
+  });
+
+  test('APPROVE-GATE: submit with NO assets → approve is BLOCKED (missing assets) → withdraw cleans', async ({
+    page,
+  }) => {
+    const SLUG = previewSlug('gate');
+    await page.goto('/', { waitUntil: 'domcontentloaded' });
+    const request = page.request;
+
+    let publishRequestId: string | null = null;
+    try {
+      await withdrawPendingForSlug(request, SLUG);
+
+      const result = await trpcMutation<SubmitResult>(
+        request,
+        'appListings.submitExternalListing',
+        submitInput(SLUG)
+      );
+      publishRequestId = result.publishRequestId;
+
+      // APPROVE with NO icon/cover/screenshot attached → the assertListingAssetsComplete
+      // gate MUST fire (the router maps it to a BAD_REQUEST / HTTP 400 whose message
+      // names the missing assets). trpcMutation throws on a non-2xx response.
+      let approveError: Error | null = null;
+      try {
+        await trpcMutation(request, 'appListings.approveExternalRequest', {
+          publishRequestId,
+          approvalNotes: 'ci-smoke approve (expected to be gate-blocked)',
+        });
+      } catch (err) {
+        approveError = err as Error;
+      }
+      expect(approveError, 'approve without assets must be rejected by the gate').not.toBeNull();
+      expect(
+        approveError?.message ?? '',
+        'the gate error names the missing required assets'
+      ).toMatch(/missing required assets/i);
+
+      // The gate fired BEFORE any mutation → the request is still pending.
+      const stillPending = await findPendingBySlug(request, SLUG);
+      expect(stillPending, 'the gate-blocked request is still pending (no mutation)').not.toBeNull();
+
+      // WITHDRAW to clean (deletes the draft + releases the slug).
+      await trpcMutation(request, 'appListings.withdrawExternalRequest', { publishRequestId });
+      publishRequestId = null;
+
+      const afterWithdraw = await findPendingBySlug(request, SLUG);
+      expect(afterWithdraw, 'the withdrawn request no longer appears in the queue').toBeNull();
+    } finally {
+      if (publishRequestId) {
+        await trpcMutation(request, 'appListings.withdrawExternalRequest', {
+          publishRequestId,
+        }).catch(() => {});
+      } else {
+        await withdrawPendingForSlug(request, SLUG);
+      }
+    }
+  });
+});