diff --git a/docs/superpowers/plans/2026-08-03-buyer-tasks-mail-failed-recovery.md b/docs/superpowers/plans/2026-08-03-buyer-tasks-mail-failed-recovery.md new file mode 100644 index 0000000..f0b2f1e --- /dev/null +++ b/docs/superpowers/plans/2026-08-03-buyer-tasks-mail-failed-recovery.md @@ -0,0 +1,539 @@ +# Buyer Tasks: Mail-Failed PO Recovery Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Keep `mail_failed` purchase orders visible in the buyer-tasks **PO Draft** tab with a clear failure reason and a one-click **Resend** action that re-opens the shared `SendPoModal`. + +**Architecture:** Single-line service filter change + one new field on the buyer-task group + a conditional red banner / button-label change on the card. The existing send-email pipeline already supports the `mail_failed` status (it's in `SENDABLE_STATUSES`), so no server-side send logic changes. One wiring fix: the buyer-tasks page's `SendPoModal` stub must also pass `status` and `email_error` so the modal renders the "Resend" label and the inline failure banner. + +**Tech Stack:** Next.js (App Router), TypeScript, Vitest, React Testing Library, Tailwind, PocketBase (admin SDK), n8n (send pipeline, unchanged). + +--- + +## File Structure + +**Modify:** +- `src/lib/server/buyer-tasks-service.ts` — add `emailError` to `BuyerTaskGroup`, include `mail_failed` in `po_draft` filter, fetch `email_error` from the expanded PO. +- `src/components/buyer-tasks/buyer-task-group-card.tsx` — render mail-failed banner + Resend button + destructive class when `poStatus === "mail_failed"`. +- `src/components/buyer-tasks/buyer-tasks-page.tsx` — pass `status` and `email_error` into the `SendPoModal` stub. +- `src/lib/server/buyer-tasks-service.test.ts` — new tests for `mail_failed` in `po_draft` and `emailError` field. +- `src/components/buyer-tasks/buyer-task-group-card.test.tsx` — new tests for the mail-failed visual + button label. +- `src/app/api/v1/buyer-tasks/route.test.ts` — add a `mail_failed` smoke test for the route. + +**No new files, no new collections, no schema changes.** + +--- + +## Task 1: Service — add `emailError` field to the group + +**Files:** +- Modify: `src/lib/server/buyer-tasks-service.ts:9-31` +- Test: `src/lib/server/buyer-tasks-service.test.ts` + +- [ ] **Step 1: Write the failing test** + +Add to `src/lib/server/buyer-tasks-service.test.ts` inside `describe("listBuyerTasks", ...)`: + +```ts +it("populates emailError from the related PO when the PO has an email_error", async () => { + const { admin } = makeAdmin([ + baseItem({ + purchase_order: "po-1", + item_status: "po_generated", + expand: { + order: baseOrder, + supplier: baseSupplier, + purchase_order: { + id: "po-1", + status: "draft", + po_number: "PO-26080301", + email_error: "[webhook] Webhook returned HTTP 502", + expand: { + supplier: { id: "sup-1", email: "orders@steelco.example" }, + }, + }, + }, + }), + ]); + vi.mocked(getAdminPocketBase).mockResolvedValue(admin as never); + + const groups = await listBuyerTasks("po_draft"); + + expect(groups).toHaveLength(1); + expect(groups[0].emailError).toBe("[webhook] Webhook returned HTTP 502"); +}); +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `npx vitest run src/lib/server/buyer-tasks-service.test.ts -t "populates emailError"` +Expected: FAIL with `TypeError: Cannot read properties of undefined (reading 'toBe')` (or similar — `groups[0].emailError` is `undefined` and the test will fail on the toBe assertion). + +- [ ] **Step 3: Add the field to the type** + +In `src/lib/server/buyer-tasks-service.ts`, add to `BuyerTaskGroup` (after `supplierEmail: string | null;` on line 29): + +```ts + /** Last send-pipeline error. Null on a clean draft or after a successful send. + * Pulled from `purchase_orders.email_error` and truncated to 200 chars at + * the service boundary so a runaway SMTP error doesn't blow up the card + * layout. The full reason remains visible in the order page's send-modal + * banner. */ + emailError: string | null; +``` + +- [ ] **Step 4: Populate the field in the group builder** + +In `src/lib/server/buyer-tasks-service.ts`, inside the `group = { ... }` block (after `supplierEmail:` on line 105), add: + +```ts + // Truncate the SMTP error to 200 chars at the service boundary so + // a runaway error string can't blow up the card layout. The full + // reason is still visible in the order page's send-modal banner. + emailError: + (po as { email_error?: string } | undefined)?.email_error + ? String((po as { email_error?: string }).email_error).slice(0, 200) + : null, +``` + +- [ ] **Step 5: Run the test to verify it passes** + +Run: `npx vitest run src/lib/server/buyer-tasks-service.test.ts -t "populates emailError"` +Expected: PASS + +- [ ] **Step 6: Commit** + +```bash +git add src/lib/server/buyer-tasks-service.ts src/lib/server/buyer-tasks-service.test.ts +git commit -m "feat(buyer-tasks): surface email_error on buyer task groups + +Adds an emailError field on BuyerTaskGroup so the UI can render +the send-pipeline failure reason. The value is truncated to 200 +chars at the service boundary to keep the card layout bounded. +" +``` + +--- + +## Task 2: Service — include `mail_failed` in the `po_draft` filter + +**Files:** +- Modify: `src/lib/server/buyer-tasks-service.ts:78` +- Test: `src/lib/server/buyer-tasks-service.test.ts` + +- [ ] **Step 1: Write the failing tests** + +Add to `src/lib/server/buyer-tasks-service.test.ts` inside `describe("listBuyerTasks", ...)`: + +```ts +it("includes mail_failed POs in the po_draft tab so the operator can retry", async () => { + const { admin } = makeAdmin([ + baseItem({ + purchase_order: "po-1", + item_status: "po_generated", + expand: { + order: baseOrder, + supplier: baseSupplier, + purchase_order: { + id: "po-1", + status: "mail_failed", + email_error: "[webhook] Webhook returned HTTP 502", + }, + }, + }), + ]); + vi.mocked(getAdminPocketBase).mockResolvedValue(admin as never); + + const groups = await listBuyerTasks("po_draft"); + + expect(groups).toHaveLength(1); + expect(groups[0].purchaseOrderId).toBe("po-1"); + expect(groups[0].poStatus).toBe("mail_failed"); + expect(groups[0].emailError).toBe("[webhook] Webhook returned HTTP 502"); +}); + +it("excludes mail_failed POs from the po_sent tab", async () => { + const { admin } = makeAdmin([ + baseItem({ + purchase_order: "po-1", + item_status: "po_generated", + expand: { + order: baseOrder, + supplier: baseSupplier, + purchase_order: { + id: "po-1", + status: "mail_failed", + email_error: "[webhook] Webhook returned HTTP 502", + }, + }, + }), + ]); + vi.mocked(getAdminPocketBase).mockResolvedValue(admin as never); + + const groups = await listBuyerTasks("po_sent"); + + expect(groups).toHaveLength(0); +}); +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `npx vitest run src/lib/server/buyer-tasks-service.test.ts -t "mail_failed"` +Expected: Both FAIL (the first because the filter excludes `mail_failed`; the second because the filter *includes* `mail_failed` in the `po_sent` item-status broad set on line 56 — wait, no: `po_sent` filter checks `po?.status` is in `["sent", "sending"]`, so `mail_failed` is currently excluded and the second test will already pass; the first is the one that needs the code change). + +Adjust the first test run to verify only the new failure: `npx vitest run src/lib/server/buyer-tasks-service.test.ts -t "includes mail_failed POs in the po_draft tab"`. Expect FAIL. + +- [ ] **Step 3: Loosen the `po_draft` filter** + +In `src/lib/server/buyer-tasks-service.ts`, change line 78 from: + +```ts + if (tab === "po_draft" && po?.status !== "draft") continue; +``` + +to: + +```ts + if (tab === "po_draft" && po?.status !== "draft" && po?.status !== "mail_failed") continue; +``` + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `npx vitest run src/lib/server/buyer-tasks-service.test.ts -t "mail_failed"` +Expected: Both PASS + +- [ ] **Step 5: Run the full service test file to confirm no regressions** + +Run: `npx vitest run src/lib/server/buyer-tasks-service.test.ts` +Expected: All tests PASS (the existing "excludes sent/cancelled POs from po_draft tab" test should still pass because it uses `status: "sent"`, which is not in the new allowed set). + +- [ ] **Step 6: Commit** + +```bash +git add src/lib/server/buyer-tasks-service.ts src/lib/server/buyer-tasks-service.test.ts +git commit -m "fix(buyer-tasks): keep mail_failed POs visible in the PO Draft tab + +The po_draft filter previously required po.status === 'draft', so +a failed send (status: 'mail_failed') caused the group to vanish +from buyer-tasks entirely. The only recovery path was the per-PO +Resend button on the order detail page, which is friction. Now +mail_failed groups stay in place so the operator can re-open the +SendPoModal and retry from the same list. po_sent is unchanged +— mail_failed must not appear there. +" +``` + +--- + +## Task 3: Card — render mail-failed banner and Resend button + +**Files:** +- Modify: `src/components/buyer-tasks/buyer-task-group-card.tsx` +- Test: `src/components/buyer-tasks/buyer-task-group-card.test.tsx` + +- [ ] **Step 1: Write the failing tests** + +Add to `src/components/buyer-tasks/buyer-task-group-card.test.tsx`: + +```tsx +it("renders the email_error banner and 'Resend' button when poStatus is mail_failed", () => { + render( + , + ); + expect(screen.getByText(/mail failed/i)).toBeInTheDocument(); + expect(screen.getByText(/webhook returned http 502/i)).toBeInTheDocument(); + // Button label changes to "Resend" when the underlying PO is mail_failed. + const action = screen.getByTestId(`buyer-task-action-${baseGroup.groupId}-po_draft`); + expect(action).toHaveTextContent(/resend/i); +}); + +it("keeps the original 'Send PO' label and amber class when poStatus is draft", () => { + render( + , + ); + const action = screen.getByTestId(`buyer-task-action-${baseGroup.groupId}-po_draft`); + expect(action).toHaveTextContent(/send po/i); + // Amber class is the pre-existing po_draft class. + expect(action.className).toMatch(/amber/i); + // No failure banner on a clean draft. + expect(screen.queryByText(/mail failed/i)).toBeNull(); +}); +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `npx vitest run src/components/buyer-tasks/buyer-task-group-card.test.tsx -t "mail_failed"` +Expected: Both FAIL (the first because the banner and label are not yet wired; the second because the test will pass for the Send label — actually the second will already pass, since the current card always renders "Send PO" for `po_draft`. The first is the failure-driving test.) + +Adjust the first test run to verify only the new failure: `npx vitest run src/components/buyer-tasks/buyer-task-group-card.test.tsx -t "renders the email_error banner"`. Expect FAIL. + +- [ ] **Step 3: Add the conditional banner and button label to the card** + +In `src/components/buyer-tasks/buyer-task-group-card.tsx`: + +(a) At the top of the file, add an import for the warning icon. Replace the `FileText` import on line 3: + +```tsx +import { AlertTriangle, FileText } from "lucide-react"; +``` + +(b) Add a `data-po-status` attribute on the action button so the test can target it without depending on text. In the existing `Button` element (lines 97-105), add `data-po-status={group.poStatus ?? "none"}`: + +```tsx + +``` + +(c) Add the `cn` import alongside the existing imports. Update the top import block (line 1-2 area) to include `cn` if it isn't already imported: + +```tsx +import { cn } from "@/lib/utils"; +``` + +(d) Render the failure banner above the items list, only when `poStatus === "mail_failed"`. Inside the `` after the existing `
` block (after line 106, before `` on line 107), add: + +```tsx + {group.poStatus === "mail_failed" && group.emailError && ( +
+
+ )} +``` + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `npx vitest run src/components/buyer-tasks/buyer-task-group-card.test.tsx -t "mail_failed"` +Expected: Both PASS + +- [ ] **Step 5: Run the full card test file to confirm no regressions** + +Run: `npx vitest run src/components/buyer-tasks/buyer-task-group-card.test.tsx` +Expected: All tests PASS (the existing "renders the right action button per step" test should still pass — it only checks the label, not the new `data-po-status` attribute). + +- [ ] **Step 6: Commit** + +```bash +git add src/components/buyer-tasks/buyer-task-group-card.tsx src/components/buyer-tasks/buyer-task-group-card.test.tsx +git commit -m "feat(buyer-tasks): render mail-failed banner + Resend button + +When a card's poStatus is 'mail_failed', the card now renders a +red warning row with the email_error reason and switches the +action button label to 'Resend' with the destructive colour. A +new data-po-status attribute on the button lets tests target the +state without depending on the human-readable label. +" +``` + +--- + +## Task 4: Page — pass `status` and `email_error` into the `SendPoModal` stub + +**Files:** +- Modify: `src/components/buyer-tasks/buyer-tasks-page.tsx:215-232` + +- [ ] **Step 1: Inspect the current stub and confirm the change** + +Open `src/components/buyer-tasks/buyer-tasks-page.tsx` lines 215-232. The current `purchaseOrder` stub passes only `id`, `po_number`, and `expand.supplier.email`. The `SendPoModal` reads `purchaseOrder.status` (line 426 — for the "Resend" vs "Send" label) and `purchaseOrder.email_error` (line 326 — for the failure banner). Without these, the modal currently shows "Send" instead of "Resend" and the inline banner is empty when opened from buyer-tasks. + +- [ ] **Step 2: Update the stub** + +In `src/components/buyer-tasks/buyer-tasks-page.tsx`, replace the existing `purchaseOrder={{...}}` prop on lines 218-226 with: + +```tsx + purchaseOrder={{ + id: sendPoFor.purchaseOrderId ?? "", + po_number: sendPoFor.poNumber ?? "", + status: sendPoFor.poStatus ?? undefined, + email_error: sendPoFor.emailError ?? undefined, + expand: { + supplier: { + email: sendPoFor.supplierEmail ?? "", + }, + }, + } as never} +``` + +`as never` is already there to silence the `PurchaseOrder`-shaped type check, since the modal lazy-fetches the full PO itself. The new fields ride along so the modal's banner + Resend label render correctly even before the lazy fetch completes. + +- [ ] **Step 3: Run the full buyer-tasks page test file to confirm no regressions** + +Run: `npx vitest run src/components/buyer-tasks/buyer-tasks-page.test.tsx` +Expected: All tests PASS. If the test file mocks `SendPoModal` and asserts on the stub shape, update the mock to include the two new fields so the assertion still matches. + +- [ ] **Step 4: Commit** + +```bash +git add src/components/buyer-tasks/buyer-tasks-page.tsx +git commit -m "fix(buyer-tasks): pass status + email_error into SendPoModal stub + +The SendPoModal renders its 'Resend' label and inline failure +banner from purchaseOrder.status / email_error, but the buyer-tasks +page stub didn't include either field. As a result, the modal +showed 'Send' and an empty banner even when the underlying PO was +mail_failed. Now the stub includes both so the operator sees the +correct affordance. +" +``` + +--- + +## Task 5: Route — smoke test for `mail_failed` group in the `po_draft` response + +**Files:** +- Modify: `src/app/api/v1/buyer-tasks/route.test.ts` + +- [ ] **Step 1: Add the smoke test** + +Add to `src/app/api/v1/buyer-tasks/route.test.ts` inside the existing `describe("GET /api/v1/buyer-tasks", ...)` block: + +```ts +it("returns mail_failed groups when tab=po_draft (operator can retry from buyer-tasks)", async () => { + vi.mocked(listBuyerTasks).mockResolvedValue([ + { + groupId: "order-1::sup-1", + orderId: "order-1", + orderNumber: "ORD-001", + supplierId: "sup-1", + supplierName: "Steel Co", + customerName: "J. Smith", + companyName: "Acme", + items: [], + itemCount: 1, + totalCost: 50, + purchaseOrderId: "po-1", + poStatus: "mail_failed", + poNumber: "PO-26080301", + supplierEmail: "orders@steelco.example", + emailError: "[webhook] Webhook returned HTTP 502", + step: "po_draft", + }, + ]); + + const res = await GET(makeRequest("po_draft") as never); + expect(res.status).toBe(200); + const body = await res.json(); + expect(body.groups).toHaveLength(1); + expect(body.groups[0].poStatus).toBe("mail_failed"); + expect(body.groups[0].emailError).toMatch(/webhook/i); +}); +``` + +- [ ] **Step 2: Run the test to verify it passes** + +Run: `npx vitest run src/app/api/v1/buyer-tasks/route.test.ts -t "mail_failed"` +Expected: PASS (the route is a thin pass-through; this test exists to lock the wire shape so a future refactor can't silently drop the new `emailError` field). + +- [ ] **Step 3: Run the full route test file to confirm no regressions** + +Run: `npx vitest run src/app/api/v1/buyer-tasks/route.test.ts` +Expected: All tests PASS + +- [ ] **Step 4: Commit** + +```bash +git add src/app/api/v1/buyer-tasks/route.test.ts +git commit -m "test(buyer-tasks): lock wire shape of mail_failed group in po_draft + +The route is a pass-through, but the smoke test guards against a +future refactor silently dropping the new emailError field — the +operator's retry affordance depends on it. +" +``` + +--- + +## Task 6: Full verification + +- [ ] **Step 1: Run the full test suite** + +Run: `npm test` +Expected: All tests PASS. Watch for any snapshot or component test that depends on the `BuyerTaskGroup` shape — those need the new `emailError` field on their fixtures (the route test in Task 5 already includes it; check for others). + +- [ ] **Step 2: Type-check the project** + +Run: `npx tsc --noEmit` +Expected: No errors. The `BuyerTaskGroup` change is in the canonical place, and the new card test + route test use the new field on their fixtures, so the compiler should be happy. + +- [ ] **Step 3: Lint** + +Run: `npm run lint` (or whatever the project's lint script is — check `package.json` if unsure) +Expected: No errors. The card's new `data-po-status` attribute and the new import (`AlertTriangle`, `cn`) are the only surface changes outside the service. + +- [ ] **Step 4: Manual smoke test in the browser** + +1. Start the app: `npm run dev` (or whatever the project's dev script is). +2. Navigate to `/orders/tasks`. +3. Trigger a `mail_failed` state for a PO: easiest is to point `PO_SEND_WORKFLOW_WEBHOOK_URL` at an unreachable URL (or temporarily unset it) and click **Send PO** on a draft. The PO flips to `mail_failed` and the group should re-appear in **PO Draft** with: + - Red banner: `⚠ Mail failed — [webhook] PO_SEND_WORKFLOW_WEBHOOK_URL is not configured. The cron pass will retry.` + - Action button label **Resend** in destructive colour. +4. Click **Resend**. The `SendPoModal` opens with: + - The supplier email pre-filled. + - "Resend PO" CTA. + - The failure banner still visible inside the modal (so the operator can confirm the last error before retrying). +5. Restore `PO_SEND_WORKFLOW_WEBHOOK_URL` and click the modal's **Resend PO**. The group should move to **PO Sent**. +6. Take a screenshot for the spec, if useful. + +- [ ] **Step 5: Commit the verification notes (no code changes)** + +If any manual findings prompt a small follow-up (e.g. an off-by-one in the truncation, a contrast issue with the destructive colour on a dark background), file a follow-up commit. Otherwise, no commit needed for the verification step itself. + +--- + +## Self-Review + +**1. Spec coverage:** +- "Filter change to include `mail_failed` in `po_draft`" → Task 2 ✓ +- "Add `emailError` field on the group" → Task 1 ✓ +- "Card visual: red banner + Resend button + destructive class when `poStatus === 'mail_failed'`" → Task 3 ✓ +- "Page wiring: pass `status` and `email_error` into `SendPoModal` stub" → Task 4 ✓ +- "Service test: `mail_failed` group in `po_draft` + `emailError` field + `po_sent` excludes `mail_failed`" → Tasks 1, 2 ✓ +- "Card test: render banner + Resend label + still renders Send PO when draft" → Task 3 ✓ +- "Route smoke test: `po_draft` response includes the new field" → Task 5 ✓ +- "Out of scope items (Failed tab, auto-retry, schema changes) not implemented" → confirmed not in plan ✓ + +**2. Placeholder scan:** No TBDs, no "fill in later", no "similar to Task N". Every code block shows the actual code. + +**3. Type consistency:** `emailError` is added in Task 1 and used in Tasks 2-5 with the same name and shape. `poStatus` is read in Task 3 against the type union already defined in `buyer-tasks-service.ts:27` (which already includes `"mail_failed"`). `data-po-status` is only used in Task 3 and not referenced elsewhere. No drift. + +**4. Risky bits called out:** +- Task 2 step 2: the second test (`po_sent` excludes `mail_failed`) actually passes against the current code, since the existing `po_sent` filter on line 79 already excludes `mail_failed`. The plan acknowledges this and asks the implementer to run the failing test in isolation first. +- Task 3 step 5: the existing "renders the right action button per step" test asserts the label, which is the only thing that could regress if the label-gate change is too broad. The plan includes a full-file re-run to catch this. +- Task 4 step 3: if the page test file mocks `SendPoModal` and asserts on the stub shape, the new fields are required on the mock fixture. The plan flags this. +- Task 6 step 4: the manual smoke test is the only way to confirm the modal renders correctly with the new stub fields — automated tests don't cover the modal's banner-rendering path here.