From 9e1e6507868f7971006440c4e92447d912b63f30 Mon Sep 17 00:00:00 2001
From: Thom Lamb <thomlamb@gmail.com>
Date: Thu, 3 Sep 2026 17:28:45 -0500
Subject: [PATCH 01/13] docs(intake): design emailing an upload link to its
 recipient (#260)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The issue asked for the security question to be settled before any code, and it is: the mail carries the working link.

The issue framed that as a loosening comparable to a password reset, and on inspection that framing overstates it. A reset token takes over an account. An upload token grants exactly one capability — submit photos into a queue where a person must approve them before anything reaches the storefront. It reads nothing, it is revocable, max_submissions caps it, and #227 caps the whole intake surface regardless of any single link. The worst outcome of a leaked upload link is junk in the review queue, which is bounded and reversible. That is a reasonable thing to put in an inbox, and this project already makes the much larger bet with reset links.

The address is required for new links while the column stays nullable, which is not a contradiction: links already exist in QA and a migration cannot invent addresses for them, so they are grandfathered, and the requirement belongs in the route where new links are actually made. It lives on the link rather than on a contributor entity, because a link already carries a label naming who it is for and nothing yet suggests the same people submit repeatedly.

A failed send does not roll the link back. The token is shown exactly once, so a rollback would leave the admin retrying and holding a different link, discarding work that succeeded. The link is created, the send is attempted, and the response says which happened — which matters concretely because QA's MAIL_ALLOWLIST silently skips any address outside it and returns as though it sent. Without an explicit outcome, testing this in QA against a contributor's real address looks exactly like success, which is the afternoon the issue warned would otherwise be wasted.

Writing it turned up one thing the design had assumed and the code does not support. sendMail returns Promise<void> and returns early both when SMTP is unconfigured and when the recipient is not allowlisted, so a caller cannot tell either from success. It gains a MailOutcome return value instead. No existing caller changes — there are seven and every one ignores the result — and the alternative would have duplicated isAllowedRecipient and the SMTP check at a second site, which is the drift the guard-in-one-place comment in mailer.ts exists to prevent.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 .../2026-09-03-upload-link-email-design.md    | 91 +++++++++++++++++++
 1 file changed, 91 insertions(+)
 create mode 100644 docs/superpowers/specs/2026-09-03-upload-link-email-design.md

diff --git a/docs/superpowers/specs/2026-09-03-upload-link-email-design.md b/docs/superpowers/specs/2026-09-03-upload-link-email-design.md
new file mode 100644
index 0000000..51e9253
--- /dev/null
+++ b/docs/superpowers/specs/2026-09-03-upload-link-email-design.md
@@ -0,0 +1,91 @@
+# Emailing an upload link to the person who will use it
+
+**Issue:** #260. Builds on #222 (upload links) and #224 (the intake notification, which is the closest existing mail).
+
+Today an admin creates an upload link, the token is shown once in the console, and getting it to a contributor is a copy-and-paste into whatever the admin happens to use. This makes the address part of creating the link, and sends the link to it.
+
+## Decisions, and what each one rests on
+
+**The mail carries the working link.** The issue framed this as a loosening comparable to a password reset, and that framing overstates it. A reset token takes over an account; an upload token grants exactly one capability — submit photos into a queue where a person must approve them before anything is published. It reads nothing, it is revocable, `max_submissions` caps it, and #227 caps the whole intake surface regardless. The worst outcome of a leaked upload link is junk in the review queue, which is bounded and reversible. That is a reasonable thing to put in an inbox; a reset link is a much larger bet and this project already makes that one.
+
+**The address is required for new links, and the column is nullable.** Those are not in tension: links already exist in QA and a migration cannot invent addresses for them, so they are grandfathered at null, and the requirement lives in the route where new links are made. Making the column `NOT NULL` would mean either inventing data or a backfill that lies.
+
+**The address lives on the link, not on a contributor.** A link already carries a label naming who it is for. A contributor entity would be the better model only if the same people submit repeatedly through different links, which nothing yet suggests, and it is materially more work.
+
+**A failed send does not lose the link.** The token is displayed exactly once, so rolling the creation back on a send failure would leave the admin retrying and getting a different link — harmless but confusing, and it throws away work that succeeded. Instead the link is created, the send is attempted, and the response says which happened.
+
+That last point is not defensive programming for its own sake. **QA sets `MAIL_ALLOWLIST` and silently skips any address outside it**, logging `[mail-blocked]` and returning as though it sent. Without an explicit outcome in the response, testing this in QA against a contributor's real address looks exactly like success. The issue itself warns this would otherwise waste an afternoon.
+
+## Architecture
+
+```
+Admin | Upload Links
+  label + email + cap
+        │
+        ▼
+POST /api/admin/upload-links
+  ├─ isValidEmail(email)            400 naming the field
+  ├─ INSERT upload_links            contact_email alongside label and cap
+  ├─ renderTemplate('uploadLink')   submitUrl required, as verification requires verifyUrl
+  └─ sendMail(...) → outcome        'sent' | 'skipped-unconfigured' | 'skipped-blocked'
+        │
+        ▼
+201 { ...link, token, url, mail: { sent: boolean, outcome: MailOutcome } }
+```
+
+### `sendMail` gains a return value
+
+`sendMail` currently returns `Promise<void>` and returns early in two cases — SMTP unconfigured, and the recipient not on `MAIL_ALLOWLIST` — both indistinguishable from success at the call site. It will return a small result instead:
+
+```ts
+export type MailOutcome = 'sent' | 'skipped-unconfigured' | 'skipped-blocked';
+```
+
+**No existing caller changes.** There are seven, and every one ignores the return value; ignoring a returned value is legal, so this is additive rather than a breaking change. The alternative — re-deriving "would this address be blocked?" in the route — would duplicate `isAllowedRecipient` and the SMTP check at a second site, which is exactly the drift the guard-in-one-place comment in `mailer.ts` exists to prevent.
+
+### Data
+
+`upload_links.contact_email TEXT` — nullable, no default. Null means a link made before this existed, or by a caller that predates the requirement. Nothing reads it except the admin list and the mail at creation.
+
+### The template
+
+A new `uploadLink` key in `TEMPLATES`, editable in the admin like the others.
+
+- `required: ['submitUrl']` — the same guard that stops a verification mail shipping without its link. A link email with no link is the one failure worth making impossible.
+- `available: ['submitUrl', 'label', 'submissionsAllowed']`
+- `submissionsAllowed` renders as a number for a capped link and as words for an uncapped one, so an unlimited link reads as a sentence rather than as a missing value. Pinned exactly, so it is not a coin-flip at implementation time: a capped link renders `25 items`, or `1 item` at a cap of one; an uncapped link renders `as many items as you like`. The default body uses it as "You can send {{submissionsAllowed}}."
+
+### The admin screen
+
+Email becomes a required field in the create form, beside the label. The links table gains the address, so it is visible who a link was sent to. When the response reports the mail was not sent, the admin is told plainly and the link is still shown to copy.
+
+## Failure handling
+
+| What happens | Result |
+|---|---|
+| Address missing or malformed | 400 naming the field. No link created. |
+| SMTP not configured (local development) | Link created. `mail.sent` false, `mail.outcome` `'skipped-unconfigured'`. |
+| Address not on `MAIL_ALLOWLIST` (QA) | Link created. `mail.sent` false, `mail.outcome` `'skipped-blocked'`. |
+| SMTP rejects the message | Link created. `mail.sent` false. The admin copies the link by hand. |
+| Everything works | Link created, `mail.sent` true, `mail.outcome` `'sent'`. |
+
+`mail.outcome` is always present and always one of the three values, including on success — a field that appears only on failure is one every consumer has to remember to check for.
+
+A send failure never rolls back the link, and never turns into a 500. The admin always ends up holding a usable link and an honest statement of whether it was delivered.
+
+## Testing
+
+- **Integration:** the address is required; a malformed one is refused naming the field; a valid one is stored; the mail is attempted with the rendered template; a send failure still returns 201 with a usable token and `mail.sent` false. Existing links with a null address still list and still work.
+- **Unit:** the `uploadLink` template refuses a body with no `submitUrl`, through the existing `missingPlaceholders` guard.
+- **E2E:** the create form refuses to submit without an address, and a created link shows its address in the table.
+- **Existing callers:** the e2e fixtures and `admin-upload-links.spec.ts` create links with only a label and must be updated. That is the cost of the requirement, and the compiler and the suite find every one.
+
+## Out of scope
+
+**Re-sending a lost link.** Only the digest is stored, so the original token cannot be recovered. "Re-send" would mean minting a new link and revoking the old one — a different feature with its own decisions about what the recipient is told.
+
+**Notifying the address on revoke.** Not asked for, and it is a separate judgement about whether a contributor should learn their link was withdrawn.
+
+**A contributor entity.** See the decisions above.
+
+**Any change to the seven existing `sendMail` call sites.** They keep ignoring the outcome. Making each of them report delivery is a larger piece of work with no demand behind it.

From a9e865b4bc3b287c60e77abda72aeef39d85ab86 Mon Sep 17 00:00:00 2001
From: Thom Lamb <thomlamb@gmail.com>
Date: Thu, 3 Sep 2026 17:32:09 -0500
Subject: [PATCH 02/13] docs(intake): plan emailing the upload link (#260)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Four tasks over the approved design: sendMail gains an outcome, the column and template land together, the route requires an address and reports what the send did, and the admin screen asks for it.

sendMail goes first deliberately. Everything else depends on being able to tell a skipped send from a real one, and it is the only change touching a file seven other things already use — so if it is going to break anything, it should break before three tasks are stacked on top of it.

The plan is explicit that no existing caller changes. Ignoring a returned value is legal, which is what makes widening the return type additive rather than breaking, and re-deriving "would this address be blocked?" in the route would have duplicated isAllowedRecipient and the SMTP check at a second site.

Two specs in unrelated features create links with only a label, and the route will refuse that. They are fixed in the same task as the form rather than left for the suite to find, because the alternative is two unrelated features going red on somebody else's branch. That cost is named in the plan rather than discovered.

One inaccuracy is left in deliberately and said out loud: an SMTP rejection is reported as skipped-unconfigured rather than a fourth outcome. The distinction is real, nothing consumes it, and the admin's next action is identical either way — copy the link and send it by hand.

Self-review caught the failure mode from #281, where tasks referred to helpers that did not exist. Task 4 originally said the created-link state "may not be called created". Reading the component showed it is `issued` and holds a bare URL string with nowhere to put a delivery outcome, so the plan now adds a separate `mailed` state beside it rather than widening the one-time token display. Every name in that task is now one that exists.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 .../plans/2026-09-03-upload-link-email.md     | 809 ++++++++++++++++++
 1 file changed, 809 insertions(+)
 create mode 100644 docs/superpowers/plans/2026-09-03-upload-link-email.md

diff --git a/docs/superpowers/plans/2026-09-03-upload-link-email.md b/docs/superpowers/plans/2026-09-03-upload-link-email.md
new file mode 100644
index 0000000..266e832
--- /dev/null
+++ b/docs/superpowers/plans/2026-09-03-upload-link-email.md
@@ -0,0 +1,809 @@
+# Upload Link Email 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:** Make an email address part of creating an upload link, and send the link to it.
+
+**Architecture:** One new nullable column, one new editable mail template, and a return value on `sendMail` so the route can tell the admin whether the mail actually went. The link is created whether or not the send succeeds, and the response says which.
+
+**Tech Stack:** Express 4 + TypeScript, `pg`, `node-pg-migrate`, `nodemailer`, Jest + supertest, React + antd (`antd/es/...` deep imports), Playwright.
+
+**Spec:** `docs/superpowers/specs/2026-09-03-upload-link-email-design.md`
+**Issue:** #260
+
+## Global Constraints
+
+- **A failed send never loses the link and never becomes a 500.** The link is created, the send attempted, and the outcome reported. The token is shown exactly once, so rolling back on a send failure would discard work that succeeded.
+- **`mail.outcome` is always present**, on success as well as failure, and is always one of `'sent' | 'skipped-unconfigured' | 'skipped-blocked'`. A field that appears only on failure is one every consumer must remember to check for.
+- **No existing `sendMail` caller changes.** There are seven and every one ignores the return value.
+- **`upload_links.contact_email` is nullable.** Links already exist in QA; the requirement lives in the route, not the column.
+- **antd imports are deep and from `es`**: `import Input from 'antd/es/input';`. Never `import { Input } from 'antd'`.
+- **Verify the frontend with `npm run build`, never a bare `npx tsc --noEmit`** — the app tsconfig excludes `tests/`, and a green bare `tsc` once broke a deploy here.
+- **Branch:** `feature/260-email-the-upload-link`, already created off `main`, already carrying the spec commit. Commit there. Subjects end `(#260)`. Commit bodies are **not hard-wrapped** — one long line per paragraph, blank lines between. Write each message to a temporary file and use `git commit -F <file>`, then delete it. Do not push; the user pushes.
+- End every commit message with:
+  `Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>`
+- Integration tests need the test database: `cd backend && npm run db:test:up`. Port 55432 is Hyper-V-reserved on this machine; if it will not bind, set `TEST_PGPORT` rather than editing the compose file.
+- **Do NOT run `scripts/start-local.ps1`, `scripts/run-tests.ps1`, or any nvm script.** They prompt for UAC elevation and have previously stripped Node from this machine. Playwright needs the stack running, which only the user can start.
+
+---
+
+### Task 1: `sendMail` reports what it did
+
+**Files:**
+- Modify: `backend/src/mailer.ts:88-113`
+- Test: `backend/tests/unit/mailOutcome.test.ts` (create)
+
+**Interfaces:**
+- Consumes: nothing.
+- Produces:
+  - `export type MailOutcome = 'sent' | 'skipped-unconfigured' | 'skipped-blocked';`
+  - `sendMail(to: string, subject: string, html: string): Promise<MailOutcome>`
+
+**Why first.** Every later task depends on being able to tell a skipped send from a real one, and this is the only change that touches a file seven other things already use.
+
+- [ ] **Step 1: Write the failing test**
+
+Create `backend/tests/unit/mailOutcome.test.ts`:
+
+```typescript
+import { sendMail } from '../../src/mailer';
+
+/**
+ * What sendMail says it did.
+ *
+ * It returns early in two cases that are indistinguishable from success at the
+ * call site — SMTP unconfigured, and the recipient not on MAIL_ALLOWLIST — and
+ * #260 needs to tell them apart so the admin is not told a link was emailed
+ * when it was not.
+ *
+ * Only the two skip paths are covered. A real send needs an SMTP server, which
+ * a unit test has no business starting; the integration test in Task 4 covers
+ * the route's behaviour instead.
+ */
+describe('what sendMail reports', () => {
+  const original = { ...process.env };
+
+  afterEach(() => {
+    process.env = { ...original };
+  });
+
+  it('says so when SMTP is not configured', async () => {
+    delete process.env.SMTP_USER;
+    delete process.env.SMTP_PASSWORD;
+
+    await expect(sendMail('someone@example.com', 'subject', '<p>body</p>')).resolves.toBe(
+      'skipped-unconfigured'
+    );
+  });
+
+  // The case that matters most: QA restricts delivery, and a blocked address
+  // previously returned exactly as though it had sent.
+  it('says so when the recipient is not on the allowlist', async () => {
+    process.env.SMTP_USER = 'user';
+    process.env.SMTP_PASSWORD = 'password';
+    process.env.MAIL_ALLOWLIST = 'allowed@example.com';
+
+    await expect(sendMail('someone-else@example.com', 'subject', '<p>body</p>')).resolves.toBe(
+      'skipped-blocked'
+    );
+  });
+
+  it('does not report blocked for an address that is on the allowlist', async () => {
+    process.env.SMTP_USER = 'user';
+    process.env.SMTP_PASSWORD = 'password';
+    process.env.MAIL_ALLOWLIST = 'allowed@example.com';
+
+    // Not asserting 'sent': that would need a live SMTP server. Asserting only
+    // that the allowlist did not refuse it, which is this test's subject.
+    await expect(
+      sendMail('allowed@example.com', 'subject', '<p>body</p>').catch(() => 'threw')
+    ).resolves.not.toBe('skipped-blocked');
+  });
+});
+```
+
+- [ ] **Step 2: Run it to verify it fails**
+
+```bash
+cd backend && npx jest -c jest.unit.config.js tests/unit/mailOutcome.test.ts
+```
+
+Expected: FAIL — `sendMail` resolves to `undefined`, not `'skipped-unconfigured'`.
+
+- [ ] **Step 3: Write the implementation**
+
+In `backend/src/mailer.ts`, add the type above `sendMail`:
+
+```typescript
+/**
+ * What a send attempt actually did.
+ *
+ * `sendMail` returns early in two cases that used to be indistinguishable from
+ * success — no SMTP credentials, and a recipient outside MAIL_ALLOWLIST — which
+ * meant a caller could report "emailed" for a message nobody would ever
+ * receive. QA restricts delivery by design, so that was not a hypothetical: it
+ * is the normal case there. See #260.
+ */
+export type MailOutcome = 'sent' | 'skipped-unconfigured' | 'skipped-blocked';
+```
+
+Then change the signature and the three exits, leaving every existing comment in place:
+
+```typescript
+export async function sendMail(to: string, subject: string, html: string): Promise<MailOutcome> {
+  if (!process.env.SMTP_USER || !process.env.SMTP_PASSWORD) {
+    console.warn(`SMTP not configured — skipping email to ${to}: "${subject}"`);
+    return 'skipped-unconfigured';
+  }
+```
+
+```typescript
+  if (!isAllowedRecipient(to, process.env.MAIL_ALLOWLIST)) {
+    console.warn(`[mail-blocked] ${to} is not on MAIL_ALLOWLIST — skipping "${subject}"`);
+    return 'skipped-blocked';
+  }
+
+  await transporter.sendMail({
+    from: process.env.SMTP_FROM || process.env.SMTP_USER,
+    to,
+    subject,
+    html
+  });
+  return 'sent';
+}
+```
+
+**Change nothing else in this file, and no call site.** All seven callers ignore the return value, which is legal — that is what makes this additive rather than breaking.
+
+- [ ] **Step 4: Run the test and the whole unit suite**
+
+```bash
+cd backend && npx jest -c jest.unit.config.js tests/unit/mailOutcome.test.ts && npm run test:unit && npm run build && npm run lint
+```
+
+Expected: PASS, 3 tests; the full unit suite still green; clean build and lint. The build is what proves no caller broke.
+
+- [ ] **Step 5: Commit**
+
+```bash
+git add backend/src/mailer.ts backend/tests/unit/mailOutcome.test.ts
+git commit -F- <<'EOF'
+feat(mail): have sendMail say what it actually did (#260)
+
+It returned Promise<void> and returned early in two cases that were indistinguishable from success at the call site: no SMTP credentials, and a recipient outside MAIL_ALLOWLIST. A caller could therefore report that it had emailed somebody a message nobody would ever receive, and in QA — which restricts delivery deliberately, as its entire safety property — that is the normal case rather than an edge one.
+
+It now returns a MailOutcome saying which of the three happened. No existing caller changes: there are seven and every one ignores the result, so this is additive. Re-deriving the answer at a second site would have duplicated isAllowedRecipient and the SMTP check, which is exactly the drift the guard-in-one-place comment above them exists to prevent.
+
+Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
+EOF
+```
+
+---
+
+### Task 2: The column and the template
+
+**Files:**
+- Create: `backend/migrations/1787700000000_add-upload-link-contact-email.js`
+- Modify: `backend/src/db-drizzle/schema.ts` (the `uploadLinks` block)
+- Modify: `backend/src/emailTemplates.ts` (the `TemplateKey` union and `TEMPLATES`)
+- Test: `backend/tests/unit/uploadLinkTemplate.test.ts` (create)
+
+**Interfaces:**
+- Consumes: nothing from Task 1.
+- Produces: `upload_links.contact_email TEXT` (nullable); the `'uploadLink'` `TemplateKey` with `required: ['submitUrl']` and `available: ['submitUrl', 'label', 'submissionsAllowed']`.
+
+- [ ] **Step 1: Write the failing test**
+
+Create `backend/tests/unit/uploadLinkTemplate.test.ts`:
+
+```typescript
+import { TEMPLATES, missingPlaceholders, renderTemplate } from '../../src/emailTemplates';
+
+/**
+ * The template that carries an upload link (#260).
+ *
+ * The one failure worth making impossible is a link email with no link in it:
+ * it sends, it looks fine in the log, and it is useless to the person who gets
+ * it. That is the same guard `verification` has on `verifyUrl`.
+ */
+describe('the upload link template', () => {
+  it('is offered in the admin like the others', () => {
+    expect(TEMPLATES.uploadLink.label).toBeTruthy();
+  });
+
+  it('requires the submit link', () => {
+    expect(TEMPLATES.uploadLink.required).toContain('submitUrl');
+  });
+
+  it('refuses a body that has no link in it', () => {
+    expect(missingPlaceholders('uploadLink', 'Hello, a link is on its way.')).toEqual(['submitUrl']);
+  });
+
+  it('accepts a body that has one', () => {
+    expect(missingPlaceholders('uploadLink', 'Send your items: {{submitUrl}}')).toEqual([]);
+  });
+
+  it('offers the label and the allowance as placeholders', () => {
+    expect(TEMPLATES.uploadLink.available).toEqual(
+      expect.arrayContaining(['submitUrl', 'label', 'submissionsAllowed'])
+    );
+  });
+
+  // The default body has to satisfy the guard it declares, or the feature ships
+  // unable to send its own default.
+  it('has a default body that satisfies its own requirement', () => {
+    expect(missingPlaceholders('uploadLink', TEMPLATES.uploadLink.defaultBody)).toEqual([]);
+  });
+
+  it('renders the link into the body', () => {
+    const rendered = renderTemplate(
+      'uploadLink',
+      { subject: 'Send us your items', body: 'Here: {{submitUrl}}' },
+      { submitUrl: 'https://example.com/submit/abc', label: 'Sarah', submissionsAllowed: '25 items' }
+    );
+
+    expect(rendered.html).toContain('https://example.com/submit/abc');
+  });
+});
+```
+
+Read `renderTemplate`'s real signature in `backend/src/emailTemplates.ts` before writing this — the third argument is the placeholder values, and the second is the stored subject/body. Adjust the call above to match it exactly rather than guessing.
+
+- [ ] **Step 2: Run it to verify it fails**
+
+```bash
+cd backend && npx jest -c jest.unit.config.js tests/unit/uploadLinkTemplate.test.ts
+```
+
+Expected: FAIL — `TEMPLATES.uploadLink` is undefined.
+
+- [ ] **Step 3: Add the template**
+
+In `backend/src/emailTemplates.ts`, add to the `TemplateKey` union:
+
+```typescript
+  | 'intakeDraft'
+  | 'uploadLink';
+```
+
+And add to `TEMPLATES`, following the shape of `intakeDraft`:
+
+```typescript
+  uploadLink: {
+    label: 'Upload link for a contributor',
+    // The link itself, for the same reason verification requires verifyUrl: an
+    // email inviting somebody to send in photos, with no way to do it, sends
+    // perfectly happily and wastes everyone's time.
+    required: ['submitUrl'],
+    available: ['submitUrl', 'label', 'submissionsAllowed'],
+    defaultSubject: 'Send us your items',
+    defaultBody:
+      'You can send us photos of items you would like us to sell.\n\n' +
+      '[Send in an item]({{submitUrl}})\n\n' +
+      'You can send {{submissionsAllowed}}. Photograph one item at a time, and tell us anything you know about it — where it came from, what it is made of, any damage. A photo cannot show any of that.\n\n' +
+      'Keep this link to yourself: anyone who has it can send us items in your name.'
+  }
+```
+
+- [ ] **Step 4: Write the migration**
+
+Create `backend/migrations/1787700000000_add-upload-link-contact-email.js`:
+
+```javascript
+exports.up = (pgm) => {
+  pgm.sql(`
+    -- Where the link was sent (#260). Nullable, and deliberately so: links
+    -- already exist in QA and a migration cannot invent an address for them, so
+    -- they are grandfathered rather than backfilled with something untrue.
+    --
+    -- The requirement lives in the create route instead, which is where new
+    -- links are actually made. A NOT NULL column would have forced a choice
+    -- between inventing data and refusing to migrate.
+    ALTER TABLE upload_links
+      ADD COLUMN IF NOT EXISTS contact_email TEXT;
+  `);
+};
+
+exports.down = (pgm) => {
+  pgm.sql(`ALTER TABLE upload_links DROP COLUMN IF EXISTS contact_email;`);
+};
+```
+
+- [ ] **Step 5: Update the Drizzle mirror**
+
+`backend/src/db-drizzle/schema.ts` is generated by `drizzle-kit pull`, and `drizzleSchema.integration.test.ts` asserts it declares every column of every table. The local dev database is probably not running, so add the line by hand in the `uploadLinks` block, matching the file's tab indentation and generated style:
+
+```typescript
+	contactEmail: text("contact_email"),
+```
+
+`text` is already imported — do not add an import.
+
+- [ ] **Step 6: Run the tests**
+
+```bash
+cd backend && npm run db:test:up && npx jest -c jest.unit.config.js tests/unit/uploadLinkTemplate.test.ts && npm run test:integration -- drizzleSchema && npm run build && npm run lint
+```
+
+Expected: PASS. The Drizzle guard is what proves the mirror is not stale.
+
+- [ ] **Step 7: Commit**
+
+```bash
+git add backend/migrations/1787700000000_add-upload-link-contact-email.js backend/src/db-drizzle/schema.ts backend/src/emailTemplates.ts backend/tests/unit/uploadLinkTemplate.test.ts
+git commit -F- <<'EOF'
+feat(intake): record where an upload link was sent, and how to say it (#260)
+
+Adds upload_links.contact_email and the uploadLink mail template.
+
+The column is nullable on purpose. Links already exist in QA and a migration cannot invent addresses for them, so they are grandfathered rather than backfilled with something untrue; the requirement belongs in the create route, which is where new links are actually made.
+
+The template requires submitUrl, the same guard verification has on verifyUrl. An email inviting somebody to send in photos, with no way for them to do it, sends perfectly happily and looks fine in the log — it is the one failure here worth making impossible, and a test asserts the default body satisfies the guard it declares.
+
+Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
+EOF
+```
+
+---
+
+### Task 3: The route requires an address and sends the mail
+
+**Files:**
+- Modify: `backend/src/routes/adminUploadLinks.ts` (`LINK_SELECT` at :29, the create route at :59)
+- Test: `backend/tests/integration/uploadLinks.integration.test.ts`
+
+**Interfaces:**
+- Consumes: `MailOutcome` and `sendMail` from Task 1; the column and template from Task 2; `isValidEmail` from `../utils`; `renderTemplate` from `../emailTemplates`; `loadStoredTemplate` from `./adminEmailTemplates`.
+- Produces: `POST /api/admin/upload-links` requires `email`; responds `201 { ...link, token, url, mail: { sent: boolean, outcome: MailOutcome } }`. `GET` returns `contact_email` on every row.
+
+- [ ] **Step 1: Write the failing test**
+
+Append to `backend/tests/integration/uploadLinks.integration.test.ts`, following the arrangement its existing cases use. Read them first — they use `request(app)` directly.
+
+```typescript
+describe('emailing the link to its recipient', () => {
+  const original = { ...process.env };
+
+  afterEach(() => {
+    process.env = { ...original };
+  });
+
+  it('refuses to create a link with no address', async () => {
+    const res = await request(app)
+      .post('/api/admin/upload-links')
+      .send({ label: 'Sarah' });
+
+    expect(res.status).toBe(400);
+    expect(res.body.error).toMatch(/email/i);
+  });
+
+  it('refuses an address that is not one', async () => {
+    const res = await request(app)
+      .post('/api/admin/upload-links')
+      .send({ label: 'Sarah', email: 'not-an-address' });
+
+    expect(res.status).toBe(400);
+    expect(res.body.error).toMatch(/email/i);
+  });
+
+  it('stores the address on the link', async () => {
+    const res = await request(app)
+      .post('/api/admin/upload-links')
+      .send({ label: 'Sarah', email: 'sarah@example.com' });
+
+    expect(res.status).toBe(201);
+
+    const { rows } = await pool.query<{ contact_email: string }>(
+      `SELECT contact_email FROM upload_links WHERE id = $1`,
+      [res.body.id]
+    );
+    expect(rows[0]?.contact_email).toBe('sarah@example.com');
+  });
+
+  // The point of the whole change: QA blocks delivery by design, so a link that
+  // was not actually emailed must not be reported as though it was.
+  it('says the mail was not sent when SMTP is not configured', async () => {
+    delete process.env.SMTP_USER;
+    delete process.env.SMTP_PASSWORD;
+
+    const res = await request(app)
+      .post('/api/admin/upload-links')
+      .send({ label: 'Sarah', email: 'sarah@example.com' });
+
+    expect(res.status).toBe(201);
+    expect(res.body.mail).toEqual({ sent: false, outcome: 'skipped-unconfigured' });
+  });
+
+  it('says the mail was not sent when the address is not allowlisted', async () => {
+    process.env.SMTP_USER = 'user';
+    process.env.SMTP_PASSWORD = 'password';
+    process.env.MAIL_ALLOWLIST = 'someone@example.com';
+
+    const res = await request(app)
+      .post('/api/admin/upload-links')
+      .send({ label: 'Sarah', email: 'sarah@example.com' });
+
+    expect(res.status).toBe(201);
+    expect(res.body.mail).toEqual({ sent: false, outcome: 'skipped-blocked' });
+  });
+
+  // A send that could not happen must never cost the admin the link, because
+  // the token is shown exactly once and a rollback would hand them a different
+  // one on the retry.
+  it('still returns a usable link when the mail did not go', async () => {
+    delete process.env.SMTP_USER;
+
+    const res = await request(app)
+      .post('/api/admin/upload-links')
+      .send({ label: 'Sarah', email: 'sarah@example.com' });
+
+    expect(res.body.token).toBeTruthy();
+    expect(res.body.url).toContain(res.body.token);
+    expect(res.body.mail.sent).toBe(false);
+  });
+
+  it('lists the address, and tolerates a link that has none', async () => {
+    await pool.query(`INSERT INTO upload_links (label, token_hash) VALUES ('Older link', 'digest')`);
+    await request(app)
+      .post('/api/admin/upload-links')
+      .send({ label: 'Newer link', email: 'sarah@example.com' });
+
+    const res = await request(app).get('/api/admin/upload-links');
+
+    const older = res.body.find((row: { label: string }) => row.label === 'Older link');
+    const newer = res.body.find((row: { label: string }) => row.label === 'Newer link');
+    expect(older.contact_email).toBeNull();
+    expect(newer.contact_email).toBe('sarah@example.com');
+  });
+});
+```
+
+- [ ] **Step 2: Run it to verify it fails**
+
+```bash
+cd backend && npm run test:integration -- uploadLinks
+```
+
+Expected: FAIL — links are created without an address and there is no `mail` in the response.
+
+- [ ] **Step 3: Write the implementation**
+
+In `backend/src/routes/adminUploadLinks.ts`, add the imports:
+
+```typescript
+import { sendMail } from '../mailer';
+import { renderTemplate } from '../emailTemplates';
+import { loadStoredTemplate } from './adminEmailTemplates';
+import { isValidEmail } from '../utils';
+```
+
+Add `contact_email` to `LINK_SELECT` and to the `UploadLinkRow` interface:
+
+```typescript
+const LINK_SELECT = `
+  SELECT id, label, contact_email, revoked_at, submission_count, max_submissions, last_used_at, created_at
+  FROM upload_links
+`;
+```
+
+```typescript
+interface UploadLinkRow {
+  id: number;
+  label: string;
+  contact_email: string | null;
+  revoked_at: string | null;
+  submission_count: number;
+  max_submissions: number | null;
+  last_used_at: string | null;
+  created_at: string;
+}
+```
+
+Add a helper above the create route:
+
+```typescript
+/**
+ * What the mail tells the recipient about how much they may send.
+ *
+ * Words rather than a bare number for an uncapped link, so the sentence reads
+ * as a sentence instead of showing an empty space where a figure should be.
+ * An uncapped link is a deliberate choice the admin already had to make, so it
+ * is emailable like any other.
+ */
+function submissionsAllowed(maxSubmissions: number | null): string {
+  if (maxSubmissions === null) return 'as many items as you like';
+  return maxSubmissions === 1 ? '1 item' : `${maxSubmissions} items`;
+}
+```
+
+In the create route, validate the address immediately after the label check:
+
+```typescript
+  const email = typeof req.body?.email === 'string' ? req.body.email.trim() : '';
+  if (email === '' || !isValidEmail(email)) {
+    return res.status(400).json({ error: 'a valid email address is required' });
+  }
+```
+
+Store it in the INSERT:
+
+```typescript
+  const { rows } = await pool.query<UploadLinkRow>(
+    `INSERT INTO upload_links (label, token_hash, max_submissions, contact_email)
+     VALUES ($1, $2, $3, $4)
+     RETURNING id, label, contact_email, revoked_at, submission_count, max_submissions, last_used_at, created_at`,
+    [label, hashToken(token), maxSubmissions, email]
+  );
+```
+
+And replace the final response with the send and the report:
+
+```typescript
+  const url = `${base}/submit/${token}`;
+
+  // Awaited, and its outcome reported rather than swallowed. Every other sender
+  // in this codebase fires and forgets because nobody is waiting on the answer;
+  // here somebody is — the admin is looking at the screen, and whether they
+  // now have to send the link by hand is the thing they need to know.
+  //
+  // A failure does not roll the link back. The token is shown exactly once, so
+  // a rollback would leave the admin retrying and holding a different link,
+  // discarding work that succeeded for the sake of tidiness.
+  let outcome: MailOutcome = 'skipped-unconfigured';
+  try {
+    const template = renderTemplate('uploadLink', await loadStoredTemplate('uploadLink'), {
+      submitUrl: url,
+      label: link.label,
+      submissionsAllowed: submissionsAllowed(link.max_submissions)
+    });
+    outcome = await sendMail(email, template.subject, template.html);
+  } catch (err) {
+    // Reported, not thrown. The link exists and is usable; the admin needs to
+    // be told the mail did not go, not handed a 500 for a link that was made.
+    console.error(`[upload-links] could not email ${email}:`, err);
+    outcome = 'skipped-unconfigured';
+  }
+
+  res.status(201).json({
+    ...link,
+    token,
+    url,
+    mail: { sent: outcome === 'sent', outcome }
+  });
+```
+
+Import `MailOutcome` as a type alongside `sendMail`. Check `renderTemplate`'s real signature and adjust the call to match it exactly.
+
+**A note on the catch:** an SMTP rejection lands here and is reported as `skipped-unconfigured`, which is not strictly accurate. Leave it. Adding a fourth outcome for "the server refused it" would be a real distinction, but nothing consumes it and the admin's action is identical either way — copy the link and send it by hand. Say so in the commit rather than inventing the case.
+
+- [ ] **Step 4: Run the tests**
+
+```bash
+cd backend && npm run test:integration -- uploadLinks && npx jest -c jest.unit.config.js tests/unit/routesAreWrapped.test.ts && npm run build && npm run lint
+```
+
+Expected: PASS. The wrapper guard confirms no handler was added unwrapped.
+
+- [ ] **Step 5: Run the whole backend suite**
+
+```bash
+cd backend && npm run test:unit && npm run test:integration
+```
+
+Expected: everything green. This is what catches any other test that created a link with only a label.
+
+- [ ] **Step 6: Commit**
+
+```bash
+git add backend/src/routes/adminUploadLinks.ts backend/tests/integration/uploadLinks.integration.test.ts
+git commit -F- <<'EOF'
+feat(intake): require an address for an upload link and send the link to it (#260)
+
+Creating a link now requires a valid email address and mails the link to it, which is the whole point: getting a link to a contributor was previously a copy-and-paste into whatever the admin happened to use.
+
+The send is awaited and its outcome reported, unlike every other sender in this codebase, which fires and forgets because nobody is waiting on the answer. Here somebody is. The admin is looking at the screen, and whether they now have to send the link by hand is exactly the thing they need to know — and QA blocks delivery to any address outside MAIL_ALLOWLIST by design, so a link that was never emailed would otherwise look precisely like one that was.
+
+A send that could not happen does not roll the link back. The token is displayed exactly once, so a rollback would leave the admin retrying and holding a different link, discarding work that had succeeded. They end up with a usable link and an honest statement about delivery instead.
+
+One inaccuracy left deliberately: an SMTP rejection is reported as skipped-unconfigured rather than a fourth outcome of its own. The distinction is real but nothing consumes it, and the admin's next action is identical either way.
+
+Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
+EOF
+```
+
+---
+
+### Task 4: The admin screen, and the callers this breaks
+
+**Files:**
+- Modify: `frontend/src/admin/UploadLinks.tsx`
+- Modify: `frontend/tests/e2e/admin-upload-links.spec.ts`
+- Modify: `frontend/tests/e2e/intake-submit.spec.ts:18`, `frontend/tests/e2e/admin-draft-queue.spec.ts:16`
+
+**Interfaces:**
+- Consumes: the route from Task 3.
+- Produces: nothing later tasks depend on.
+
+**Why the last three files are here.** Both specs create a link with `api.post('/api/admin/upload-links', { data: { label } })`, which the route now refuses. They are not incidental damage — they are the cost of the requirement, and leaving them for the suite to find later would mean two unrelated features going red.
+
+- [ ] **Step 1: Add the field to the form**
+
+In `frontend/src/admin/UploadLinks.tsx`, add state beside `label` (around line 38):
+
+```typescript
+  const [email, setEmail] = useState('');
+```
+
+Add the input after the label input (around line 105), matching the surrounding style and using a deep antd import if `Input` is not already imported:
+
+```tsx
+        <Input
+          aria-label="Contributor email"
+          placeholder="Where should the link be sent?"
+          value={email}
+          onChange={(e) => setEmail(e.target.value)}
+        />
+```
+
+Send it (around line 68):
+
+```typescript
+        label,
+        email,
+        maxSubmissions: unlimited ? null : Number(cap)
+```
+
+Disable the button until both are filled (around line 120):
+
+```tsx
+disabled={label.trim() === '' || email.trim() === ''}
+```
+
+Reset it in the create handler, beside the existing `setLabel('')`:
+
+```typescript
+    setIssued(created.url);
+    setLabel('');
+    setEmail('');
+    setCap(DEFAULT_CAP);
+```
+
+Add the column to the table (around line 153):
+
+```tsx
+          { title: 'Label', dataIndex: 'label' },
+          { title: 'Sent to', dataIndex: 'contact_email' },
+```
+
+- [ ] **Step 2: Tell the admin when the mail did not go**
+
+The component holds the created link as `const [issued, setIssued] = useState<string | null>(null)` and sets it with `setIssued(created.url)` — a bare URL string, with nowhere to put a delivery outcome. Add a second piece of state beside it rather than widening `issued`, so the existing one-time-token display is untouched:
+
+```typescript
+  // Whether the link that is currently on screen was actually emailed. Separate
+  // from `issued` so the one-time display of the token keeps working exactly as
+  // it did; this only adds a note beside it.
+  const [mailed, setMailed] = useState<boolean | null>(null);
+```
+
+Extend `UploadLink` with the new column, so the table can show it:
+
+```typescript
+interface UploadLink {
+  id: number;
+  label: string;
+  contact_email: string | null;
+  revoked_at: string | null;
+```
+
+In the create handler, set it from the response alongside `setIssued`:
+
+```typescript
+    const created = await res.json();
+    setIssued(created.url);
+    setMailed(created.mail.sent);
+```
+
+And reset it wherever a new create begins, beside `setError(null)`, so a previous link's outcome cannot be read as this one's:
+
+```typescript
+    setMailed(null);
+```
+
+Then render the warning next to wherever `issued` is displayed. The link stays on screen regardless — that is the point of not rolling back:
+
+```tsx
+      {issued && mailed === false && (
+        <Alert
+          type="warning"
+          showIcon
+          message="The link was not emailed"
+          description="Copy it below and send it yourself. This is normal where no mail is configured, and in QA, where delivery is restricted to a fixed list of addresses."
+        />
+      )}
+```
+
+`Alert` is imported with a deep import if it is not already: `import Alert from 'antd/es/alert';`. Check the file's existing imports first — several antd components are already imported there.
+
+- [ ] **Step 3: Fix the two specs that create links without an address**
+
+In `frontend/tests/e2e/intake-submit.spec.ts` (line 18) and `frontend/tests/e2e/admin-draft-queue.spec.ts` (line 16), add an address to the `data` object:
+
+```typescript
+    data: { label: `Intake spec ${RUN}`, email: `intake-${RUN}@example.com` }
+```
+
+Use the label each file already builds and an address carrying the same run id, so a failure names the run that caused it. `@example.com` is reserved for exactly this and cannot reach a real person.
+
+- [ ] **Step 4: Update the upload-links spec**
+
+In `frontend/tests/e2e/admin-upload-links.spec.ts`, every test that fills `Link label` must now also fill `Contributor email` before the create button is enabled. Add to each of the three:
+
+```typescript
+    await page.getByLabel('Contributor email').fill(`${uniqueSuffix()}@example.com`);
+```
+
+And add one case asserting the requirement, inside the existing describe:
+
+```typescript
+  // The address is the point of the change: a link nobody can be sent is the
+  // thing this replaced.
+  test('will not create a link without an address', async ({ page, admin }) => {
+    await admin.open('Upload Links');
+    await page.getByLabel('Link label').fill(`Nameless ${uniqueSuffix()}`);
+
+    await expect(page.getByRole('button', { name: 'Create link' })).toBeDisabled();
+  });
+```
+
+Read the file first: the create button's accessible name may not be `Create link`, and `admin.open`'s argument must match the real tab name. Use what is there.
+
+- [ ] **Step 5: Verify the frontend**
+
+```bash
+cd frontend && npm run build && npm run lint && npm run test:unit
+```
+
+`npm run build`, not a bare `npx tsc --noEmit` — the app tsconfig excludes `tests/`.
+
+Expected: clean build, clean lint, unit tests pass.
+
+- [ ] **Step 6: Run the e2e suite**
+
+The local stack must be running. **Do not start it yourself** — ask the user.
+
+```bash
+cd frontend && npx playwright test admin-upload-links intake-submit admin-draft-queue --project=chromium
+```
+
+Then the whole suite, which is what finds anything else that created a link with only a label:
+
+```bash
+cd frontend && npx playwright test --project=chromium
+```
+
+Expected: all green.
+
+- [ ] **Step 7: Commit**
+
+```bash
+git add frontend/src/admin/UploadLinks.tsx frontend/tests/e2e/admin-upload-links.spec.ts frontend/tests/e2e/intake-submit.spec.ts frontend/tests/e2e/admin-draft-queue.spec.ts
+git commit -F- <<'EOF'
+feat(admin): ask for the contributor's address when creating a link (#260)
+
+The address is now a required field beside the label, the links table shows where each link was sent, and the admin is told plainly when the mail did not go — with the link still on screen to copy, which is the case that matters in QA and in local development where there is no mail at all.
+
+Two specs in unrelated features created links with only a label and the route now refuses that, so they are updated here rather than left to go red on somebody else's branch. That is the cost of making the address required, and it is a small one: the compiler and the suite find every call site.
+
+Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
+EOF
+```
+
+---
+
+## After the plan
+
+- The branch is `feature/260-email-the-upload-link`. **Do not push** — the user pushes and merges.
+- The PR closes #260 and should say plainly that the mail carries the working token, and why that trade was judged acceptable: an upload link grants only "submit into a queue a person must approve", bounded by `max_submissions` and #227's ceiling.
+- Per standing practice, follow with a separate SonarQube cleanup issue and PR — never folded into this branch.
+- **Testing this in QA needs care.** `MAIL_ALLOWLIST` is hardcoded to a single address in `docker-compose.qa.yml` and is deliberately not a stack variable, because it is the entire safety property stopping a QA run emailing real people. Use that address or a `+suffix` variant of it. Any other address will report `skipped-blocked`, which is the feature working correctly.

From 8a35c213fc1129a66f6fb173ac9f0da4f8eff7db Mon Sep 17 00:00:00 2001
From: Thom Lamb <thomlamb@gmail.com>
Date: Thu, 3 Sep 2026 17:39:59 -0500
Subject: [PATCH 03/13] feat(mail): have sendMail say what it actually did
 (#260)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

It returned Promise<void> and returned early in two cases that were indistinguishable from success at the call site: no SMTP credentials, and a recipient outside MAIL_ALLOWLIST. A caller could therefore report that it had emailed somebody a message nobody would ever receive, and in QA — which restricts delivery deliberately, as its entire safety property — that is the normal case rather than an edge one.

It now returns a MailOutcome saying which of the three happened. No existing caller changes: there are seven and every one ignores the result, so this is additive. Re-deriving the answer at a second site would have duplicated isAllowedRecipient and the SMTP check, which is exactly the drift the guard-in-one-place comment above them exists to prevent.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 backend/src/mailer.ts                  | 18 +++++++--
 backend/tests/unit/mailOutcome.test.ts | 54 ++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 3 deletions(-)
 create mode 100644 backend/tests/unit/mailOutcome.test.ts

diff --git a/backend/src/mailer.ts b/backend/src/mailer.ts
index d40bd29..bfa1237 100755
--- a/backend/src/mailer.ts
+++ b/backend/src/mailer.ts
@@ -85,10 +85,21 @@ export function isAllowedRecipient(to: string, allowlist: string | undefined): b
   });
 }
 
-export async function sendMail(to: string, subject: string, html: string): Promise<void> {
+/**
+ * What a send attempt actually did.
+ *
+ * `sendMail` returns early in two cases that used to be indistinguishable from
+ * success — no SMTP credentials, and a recipient outside MAIL_ALLOWLIST — which
+ * meant a caller could report "emailed" for a message nobody would ever
+ * receive. QA restricts delivery by design, so that was not a hypothetical: it
+ * is the normal case there. See #260.
+ */
+export type MailOutcome = 'sent' | 'skipped-unconfigured' | 'skipped-blocked';
+
+export async function sendMail(to: string, subject: string, html: string): Promise<MailOutcome> {
   if (!process.env.SMTP_USER || !process.env.SMTP_PASSWORD) {
     console.warn(`SMTP not configured — skipping email to ${to}: "${subject}"`);
-    return;
+    return 'skipped-unconfigured';
   }
 
   // Guarded here rather than at the four call sites, so every sender is covered
@@ -101,7 +112,7 @@ export async function sendMail(to: string, subject: string, html: string): Promi
   // is the part that was missing when QA was simply muted.
   if (!isAllowedRecipient(to, process.env.MAIL_ALLOWLIST)) {
     console.warn(`[mail-blocked] ${to} is not on MAIL_ALLOWLIST — skipping "${subject}"`);
-    return;
+    return 'skipped-blocked';
   }
 
   await transporter.sendMail({
@@ -110,4 +121,5 @@ export async function sendMail(to: string, subject: string, html: string): Promi
     subject,
     html
   });
+  return 'sent';
 }
diff --git a/backend/tests/unit/mailOutcome.test.ts b/backend/tests/unit/mailOutcome.test.ts
new file mode 100644
index 0000000..920a882
--- /dev/null
+++ b/backend/tests/unit/mailOutcome.test.ts
@@ -0,0 +1,54 @@
+import { sendMail } from '../../src/mailer';
+
+/**
+ * What sendMail says it did.
+ *
+ * It returns early in two cases that are indistinguishable from success at the
+ * call site — SMTP unconfigured, and the recipient not on MAIL_ALLOWLIST — and
+ * #260 needs to tell them apart so the admin is not told a link was emailed
+ * when it was not.
+ *
+ * Only the two skip paths are covered. A real send needs an SMTP server, which
+ * a unit test has no business starting; the integration test in Task 4 covers
+ * the route's behaviour instead.
+ */
+describe('what sendMail reports', () => {
+  const original = { ...process.env };
+
+  afterEach(() => {
+    process.env = { ...original };
+  });
+
+  it('says so when SMTP is not configured', async () => {
+    delete process.env.SMTP_USER;
+    delete process.env.SMTP_PASSWORD;
+
+    await expect(sendMail('someone@example.com', 'subject', '<p>body</p>')).resolves.toBe(
+      'skipped-unconfigured'
+    );
+  });
+
+  // The case that matters most: QA restricts delivery, and a blocked address
+  // previously returned exactly as though it had sent.
+  it('says so when the recipient is not on the allowlist', async () => {
+    process.env.SMTP_USER = 'user';
+    process.env.SMTP_PASSWORD = 'password';
+    process.env.MAIL_ALLOWLIST = 'allowed@example.com';
+
+    await expect(sendMail('someone-else@example.com', 'subject', '<p>body</p>')).resolves.toBe(
+      'skipped-blocked'
+    );
+  });
+
+  it('does not report blocked for an address that is on the allowlist', async () => {
+    process.env.SMTP_USER = 'user';
+    process.env.SMTP_PASSWORD = 'password';
+    process.env.MAIL_ALLOWLIST = 'allowed@example.com';
+
+    // Not asserting 'sent': that would need a live SMTP server. Asserting only
+    // that the allowlist did not refuse it, which is this test's subject.
+    await expect(
+      sendMail('allowed@example.com', 'subject', '<p>body</p>').catch(() => 'threw')
+    ).resolves.not.toBe('skipped-blocked');
+  });
+});

From 823796b92a066ee81910add1803fe87a3a7e26aa Mon Sep 17 00:00:00 2001
From: Thom Lamb <thomlamb@gmail.com>
Date: Thu, 3 Sep 2026 17:46:29 -0500
Subject: [PATCH 04/13] test(mail): stop the allowlist test from reaching real
 Gmail (#260)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Review of the Task 1 commit approved the implementation but flagged the third test in mailOutcome.test.ts as a defect carried over from the brief: it set SMTP_USER, SMTP_PASSWORD, and an allowlist the recipient satisfied, so sendMail fell through both early-return guards and reached the real transporter, opening a live TLS connection to smtp.gmail.com:465. The .catch(() => 'threw') wrapper hid a fast auth rejection, a slow timeout, or an accidental real send equally, and on a restricted CI runner it would hang to the Jest timeout rather than fail fast.

The fix deletes that test rather than replacing it. What it was trying to prove — that an allowlisted recipient is not blocked — is already covered hermetically by backend/tests/unit/mailAllowlist.test.ts, which exercises isAllowedRecipient directly across exact matches, plus-suffixes, domains, and refusals. The other two tests in mailOutcome.test.ts are untouched; they cover the two paths that return early, which is the entire point of the change, and neither one reaches the transporter.

The file's top doc comment is updated to match: it now says only the two skip paths are covered here, names mailAllowlist.test.ts as where the allowlist's own behaviour is tested, and spells out why a third test that reaches the transporter does not belong in this file, so nobody adds one back.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 backend/tests/unit/mailOutcome.test.ts | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/backend/tests/unit/mailOutcome.test.ts b/backend/tests/unit/mailOutcome.test.ts
index 920a882..8fb4a78 100644
--- a/backend/tests/unit/mailOutcome.test.ts
+++ b/backend/tests/unit/mailOutcome.test.ts
@@ -8,9 +8,17 @@ import { sendMail } from '../../src/mailer';
  * #260 needs to tell them apart so the admin is not told a link was emailed
  * when it was not.
  *
- * Only the two skip paths are covered. A real send needs an SMTP server, which
- * a unit test has no business starting; the integration test in Task 4 covers
- * the route's behaviour instead.
+ * Only the two skip paths are covered here. A real send needs an SMTP server,
+ * which a unit test has no business starting; the integration test in Task 4
+ * covers the route's behaviour instead.
+ *
+ * The allowlist's own behaviour — exact matches, plus-suffixes, domains,
+ * refusals — is covered directly and hermetically in
+ * backend/tests/unit/mailAllowlist.test.ts, against isAllowedRecipient itself.
+ * There is deliberately no third test here that sets SMTP_USER/SMTP_PASSWORD
+ * and an allowed recipient: that combination falls through both guards and
+ * reaches the real transporter, which opens a live TLS connection to
+ * smtp.gmail.com:465. Do not add one back.
  */
 describe('what sendMail reports', () => {
   const original = { ...process.env };
@@ -39,16 +47,4 @@ describe('what sendMail reports', () => {
       'skipped-blocked'
     );
   });
-
-  it('does not report blocked for an address that is on the allowlist', async () => {
-    process.env.SMTP_USER = 'user';
-    process.env.SMTP_PASSWORD = 'password';
-    process.env.MAIL_ALLOWLIST = 'allowed@example.com';
-
-    // Not asserting 'sent': that would need a live SMTP server. Asserting only
-    // that the allowlist did not refuse it, which is this test's subject.
-    await expect(
-      sendMail('allowed@example.com', 'subject', '<p>body</p>').catch(() => 'threw')
-    ).resolves.not.toBe('skipped-blocked');
-  });
 });

From eaecc4337972d9e126ceb82a4c034ab15a92b464 Mon Sep 17 00:00:00 2001
From: Thom Lamb <thomlamb@gmail.com>
Date: Thu, 3 Sep 2026 17:50:11 -0500
Subject: [PATCH 05/13] feat(intake): record where an upload link was sent, and
 how to say it (#260)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Adds upload_links.contact_email and the uploadLink mail template.

The column is nullable on purpose. Links already exist in QA and a migration cannot invent addresses for them, so they are grandfathered rather than backfilled with something untrue; the requirement belongs in the create route, which is where new links are actually made.

The template requires submitUrl, the same guard verification has on verifyUrl. An email inviting somebody to send in photos, with no way for them to do it, sends perfectly happily and looks fine in the log — it is the one failure here worth making impossible, and a test asserts the default body satisfies the guard it declares.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 ...700000000_add-upload-link-contact-email.js | 17 +++++++
 backend/src/db-drizzle/schema.ts              |  1 +
 backend/src/emailTemplates.ts                 | 18 ++++++-
 backend/tests/unit/uploadLinkTemplate.test.ts | 48 +++++++++++++++++++
 4 files changed, 83 insertions(+), 1 deletion(-)
 create mode 100644 backend/migrations/1787700000000_add-upload-link-contact-email.js
 create mode 100644 backend/tests/unit/uploadLinkTemplate.test.ts

diff --git a/backend/migrations/1787700000000_add-upload-link-contact-email.js b/backend/migrations/1787700000000_add-upload-link-contact-email.js
new file mode 100644
index 0000000..a73e359
--- /dev/null
+++ b/backend/migrations/1787700000000_add-upload-link-contact-email.js
@@ -0,0 +1,17 @@
+exports.up = (pgm) => {
+  pgm.sql(`
+    -- Where the link was sent (#260). Nullable, and deliberately so: links
+    -- already exist in QA and a migration cannot invent an address for them, so
+    -- they are grandfathered rather than backfilled with something untrue.
+    --
+    -- The requirement lives in the create route instead, which is where new
+    -- links are actually made. A NOT NULL column would have forced a choice
+    -- between inventing data and refusing to migrate.
+    ALTER TABLE upload_links
+      ADD COLUMN IF NOT EXISTS contact_email TEXT;
+  `);
+};
+
+exports.down = (pgm) => {
+  pgm.sql(`ALTER TABLE upload_links DROP COLUMN IF EXISTS contact_email;`);
+};
diff --git a/backend/src/db-drizzle/schema.ts b/backend/src/db-drizzle/schema.ts
index 21f5c07..a7bf63f 100644
--- a/backend/src/db-drizzle/schema.ts
+++ b/backend/src/db-drizzle/schema.ts
@@ -280,6 +280,7 @@ export const uploadLinks = pgTable("upload_links", {
 	maxSubmissions: integer("max_submissions"),
 	lastUsedAt: timestamp("last_used_at", { withTimezone: true, mode: 'string' }),
 	createdAt: timestamp("created_at", { withTimezone: true, mode: 'string' }).defaultNow().notNull(),
+	contactEmail: text("contact_email"),
 }, (table) => [
 	unique("upload_links_token_hash_key").on(table.tokenHash),
 ]);
diff --git a/backend/src/emailTemplates.ts b/backend/src/emailTemplates.ts
index 9d86e4c..60c1a8c 100644
--- a/backend/src/emailTemplates.ts
+++ b/backend/src/emailTemplates.ts
@@ -18,7 +18,8 @@ export type TemplateKey =
   | 'favoriteWithdrawn'
   | 'cartReminder'
   | 'emailChanged'
-  | 'intakeDraft';
+  | 'intakeDraft'
+  | 'uploadLink';
 
 export interface TemplateDefinition {
   /** Shown in the admin so a card is identifiable without reading its body. */
@@ -153,6 +154,21 @@ export const TEMPLATES: Record<TemplateKey, TemplateDefinition> = {
       'Nothing is listed until you publish it from that screen, and the price ' +
       'above is a suggestion rather than a decision.\n\n' +
       '[Ask for another draft]({{regenerateUrl}}) - [Discard it]({{discardUrl}})'
+  },
+
+  uploadLink: {
+    label: 'Upload link for a contributor',
+    // The link itself, for the same reason verification requires verifyUrl: an
+    // email inviting somebody to send in photos, with no way to do it, sends
+    // perfectly happily and wastes everyone's time.
+    required: ['submitUrl'],
+    available: ['submitUrl', 'label', 'submissionsAllowed'],
+    defaultSubject: 'Send us your items',
+    defaultBody:
+      'You can send us photos of items you would like us to sell.\n\n' +
+      '[Send in an item]({{submitUrl}})\n\n' +
+      'You can send {{submissionsAllowed}}. Photograph one item at a time, and tell us anything you know about it — where it came from, what it is made of, any damage. A photo cannot show any of that.\n\n' +
+      'Keep this link to yourself: anyone who has it can send us items in your name.'
   }
 };
 
diff --git a/backend/tests/unit/uploadLinkTemplate.test.ts b/backend/tests/unit/uploadLinkTemplate.test.ts
new file mode 100644
index 0000000..767f2bb
--- /dev/null
+++ b/backend/tests/unit/uploadLinkTemplate.test.ts
@@ -0,0 +1,48 @@
+import { TEMPLATES, missingPlaceholders, renderTemplate } from '../../src/emailTemplates';
+
+/**
+ * The template that carries an upload link (#260).
+ *
+ * The one failure worth making impossible is a link email with no link in it:
+ * it sends, it looks fine in the log, and it is useless to the person who gets
+ * it. That is the same guard `verification` has on `verifyUrl`.
+ */
+describe('the upload link template', () => {
+  it('is offered in the admin like the others', () => {
+    expect(TEMPLATES.uploadLink.label).toBeTruthy();
+  });
+
+  it('requires the submit link', () => {
+    expect(TEMPLATES.uploadLink.required).toContain('submitUrl');
+  });
+
+  it('refuses a body that has no link in it', () => {
+    expect(missingPlaceholders('uploadLink', 'Hello, a link is on its way.')).toEqual(['submitUrl']);
+  });
+
+  it('accepts a body that has one', () => {
+    expect(missingPlaceholders('uploadLink', 'Send your items: {{submitUrl}}')).toEqual([]);
+  });
+
+  it('offers the label and the allowance as placeholders', () => {
+    expect(TEMPLATES.uploadLink.available).toEqual(
+      expect.arrayContaining(['submitUrl', 'label', 'submissionsAllowed'])
+    );
+  });
+
+  // The default body has to satisfy the guard it declares, or the feature ships
+  // unable to send its own default.
+  it('has a default body that satisfies its own requirement', () => {
+    expect(missingPlaceholders('uploadLink', TEMPLATES.uploadLink.defaultBody)).toEqual([]);
+  });
+
+  it('renders the link into the body', () => {
+    const rendered = renderTemplate(
+      'uploadLink',
+      { subject: 'Send us your items', body: 'Here: {{submitUrl}}' },
+      { submitUrl: 'https://example.com/submit/abc', label: 'Sarah', submissionsAllowed: '25 items' }
+    );
+
+    expect(rendered.html).toContain('https://example.com/submit/abc');
+  });
+});

From b18b3e3a3d8820568d7b4943665b14d8a40176ec Mon Sep 17 00:00:00 2001
From: Thom Lamb <thomlamb@gmail.com>
Date: Thu, 3 Sep 2026 18:00:15 -0500
Subject: [PATCH 06/13] feat(intake): require an address for an upload link and
 send the link to it (#260)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Creating a link now requires a valid email address and mails the link to it, which is the whole point: getting a link to a contributor was previously a copy-and-paste into whatever the admin happened to use.

The send is awaited and its outcome reported, unlike every other sender in this codebase, which fires and forgets because nobody is waiting on the answer. Here somebody is. The admin is looking at the screen, and whether they now have to send the link by hand is exactly the thing they need to know — and QA blocks delivery to any address outside MAIL_ALLOWLIST by design, so a link that was never emailed would otherwise look precisely like one that was.

A send that could not happen does not roll the link back. The token is displayed exactly once, so a rollback would leave the admin retrying and holding a different link, discarding work that had succeeded. They end up with a usable link and an honest statement about delivery instead.

One inaccuracy left deliberately: an SMTP rejection is reported as skipped-unconfigured rather than a fourth outcome of its own. The distinction is real but nothing consumes it, and the admin's next action is identical either way.

Also updates the other integration tests that created a link with only a label, since an address is now required, and adds the uploadLink template key that GET /api/admin/email-templates was missing from its list — an omission left by the template's addition in the prior commit on this branch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 backend/src/routes/adminUploadLinks.ts        |  72 +++++++++--
 .../emailTemplates.integration.test.ts        |   1 +
 .../integration/intake.integration.test.ts    |   2 +-
 .../intakeCeiling.integration.test.ts         |   4 +-
 .../uploadLinks.integration.test.ts           | 112 ++++++++++++++++--
 5 files changed, 175 insertions(+), 16 deletions(-)

diff --git a/backend/src/routes/adminUploadLinks.ts b/backend/src/routes/adminUploadLinks.ts
index 71edd0c..d1f5b7c 100644
--- a/backend/src/routes/adminUploadLinks.ts
+++ b/backend/src/routes/adminUploadLinks.ts
@@ -2,7 +2,10 @@ import { Router, Request, Response } from 'express';
 import { pool, requireRow } from '../db';
 import { asyncRoute } from '../asyncRoute';
 import { generateToken, hashToken } from '../uploadLinks';
-import { trimTrailingSlashes } from '../utils';
+import { trimTrailingSlashes, isValidEmail } from '../utils';
+import { sendMail, MailOutcome } from '../mailer';
+import { renderTemplate } from '../emailTemplates';
+import { loadStoredTemplate } from './adminEmailTemplates';
 
 const router = Router();
 
@@ -27,7 +30,7 @@ const router = Router();
  * `token_hash` into every listing the moment somebody added a convenience.
  */
 const LINK_SELECT = `
-  SELECT id, label, revoked_at, submission_count, max_submissions, last_used_at, created_at
+  SELECT id, label, contact_email, revoked_at, submission_count, max_submissions, last_used_at, created_at
   FROM upload_links
 `;
 
@@ -44,6 +47,7 @@ const DEFAULT_MAX_SUBMISSIONS = 25;
 interface UploadLinkRow {
   id: number;
   label: string;
+  contact_email: string | null;
   revoked_at: string | null;
   submission_count: number;
   max_submissions: number | null;
@@ -51,6 +55,19 @@ interface UploadLinkRow {
   created_at: string;
 }
 
+/**
+ * What the mail tells the recipient about how much they may send.
+ *
+ * Words rather than a bare number for an uncapped link, so the sentence reads
+ * as a sentence instead of showing an empty space where a figure should be.
+ * An uncapped link is a deliberate choice the admin already had to make, so it
+ * is emailable like any other.
+ */
+function submissionsAllowed(maxSubmissions: number | null): string {
+  if (maxSubmissions === null) return 'as many items as you like';
+  return maxSubmissions === 1 ? '1 item' : `${maxSubmissions} items`;
+}
+
 router.get('/', asyncRoute(async (_req: Request, res: Response) => {
   const { rows } = await pool.query<UploadLinkRow>(`${LINK_SELECT} ORDER BY created_at DESC`);
   res.json(rows);
@@ -62,6 +79,11 @@ router.post('/', asyncRoute(async (req: Request, res: Response) => {
     return res.status(400).json({ error: 'a label is required' });
   }
 
+  const email = typeof req.body?.email === 'string' ? req.body.email.trim() : '';
+  if (email === '' || !isValidEmail(email)) {
+    return res.status(400).json({ error: 'a valid email address is required' });
+  }
+
   // Three cases, deliberately distinct. Absent means nobody decided, which
   // gets the bounded default. An explicit null means unlimited — a decision
   // someone made, visible in the request. A number is itself. Reading absent
@@ -80,10 +102,10 @@ router.post('/', asyncRoute(async (req: Request, res: Response) => {
 
   const token = generateToken();
   const { rows } = await pool.query<UploadLinkRow>(
-    `INSERT INTO upload_links (label, token_hash, max_submissions)
-     VALUES ($1, $2, $3)
-     RETURNING id, label, revoked_at, submission_count, max_submissions, last_used_at, created_at`,
-    [label, hashToken(token), maxSubmissions]
+    `INSERT INTO upload_links (label, token_hash, max_submissions, contact_email)
+     VALUES ($1, $2, $3, $4)
+     RETURNING id, label, contact_email, revoked_at, submission_count, max_submissions, last_used_at, created_at`,
+    [label, hashToken(token), maxSubmissions, email]
   );
   const link = requireRow(rows, 'the upload_links INSERT');
 
@@ -91,7 +113,43 @@ router.post('/', asyncRoute(async (req: Request, res: Response) => {
   // outbound link is built from. Absent in local development, which yields a
   // relative URL the admin screen can still show and copy usefully.
   const base = trimTrailingSlashes(process.env.PUBLIC_URL ?? '');
-  res.status(201).json({ ...link, token, url: `${base}/submit/${token}` });
+  const url = `${base}/submit/${token}`;
+
+  // Awaited, and its outcome reported rather than swallowed. Every other sender
+  // in this codebase fires and forgets because nobody is waiting on the answer;
+  // here somebody is — the admin is looking at the screen, and whether they
+  // now have to send the link by hand is the thing they need to know.
+  //
+  // A failure does not roll the link back. The token is shown exactly once, so
+  // a rollback would leave the admin retrying and holding a different link,
+  // discarding work that succeeded for the sake of tidiness.
+  let outcome: MailOutcome = 'skipped-unconfigured';
+  try {
+    const template = renderTemplate('uploadLink', await loadStoredTemplate('uploadLink'), {
+      submitUrl: url,
+      label: link.label,
+      submissionsAllowed: submissionsAllowed(link.max_submissions)
+    });
+    outcome = await sendMail(email, template.subject, template.html);
+  } catch (err) {
+    // Reported, not thrown. The link exists and is usable; the admin needs to
+    // be told the mail did not go, not handed a 500 for a link that was made.
+    //
+    // An SMTP rejection also lands here and is reported the same way as an
+    // unconfigured environment, which is not strictly accurate — a fourth
+    // outcome would be a real distinction, but nothing consumes it and the
+    // admin's next action is identical either way: copy the link and send it
+    // by hand.
+    console.error(`[upload-links] could not email ${email}:`, err);
+    outcome = 'skipped-unconfigured';
+  }
+
+  res.status(201).json({
+    ...link,
+    token,
+    url,
+    mail: { sent: outcome === 'sent', outcome }
+  });
 }));
 
 /**
diff --git a/backend/tests/integration/emailTemplates.integration.test.ts b/backend/tests/integration/emailTemplates.integration.test.ts
index bd07bce..a665464 100644
--- a/backend/tests/integration/emailTemplates.integration.test.ts
+++ b/backend/tests/integration/emailTemplates.integration.test.ts
@@ -30,6 +30,7 @@ describe('GET /api/admin/email-templates', () => {
       'favoriteWithdrawn',
       'intakeDraft',
       'passwordReset',
+      'uploadLink',
       'verification'
     ]);
 
diff --git a/backend/tests/integration/intake.integration.test.ts b/backend/tests/integration/intake.integration.test.ts
index bc212f4..3b21868 100644
--- a/backend/tests/integration/intake.integration.test.ts
+++ b/backend/tests/integration/intake.integration.test.ts
@@ -34,7 +34,7 @@ async function storedFiles(): Promise<string[]> {
 async function issueLink(label = 'Sarah', maxSubmissions?: number | null): Promise<string> {
   const res = await request(app)
     .post('/api/admin/upload-links')
-    .send({ label, ...(maxSubmissions === undefined ? {} : { maxSubmissions }) });
+    .send({ label, email: 'sarah@example.com', ...(maxSubmissions === undefined ? {} : { maxSubmissions }) });
   expect(res.status).toBe(201);
   return res.body.token as string;
 }
diff --git a/backend/tests/integration/intakeCeiling.integration.test.ts b/backend/tests/integration/intakeCeiling.integration.test.ts
index 3c06cf5..6e125dc 100644
--- a/backend/tests/integration/intakeCeiling.integration.test.ts
+++ b/backend/tests/integration/intakeCeiling.integration.test.ts
@@ -20,7 +20,9 @@ afterAll(async () => {
 });
 
 async function makeLink(): Promise<string> {
-  const res = await request(app).post('/api/admin/upload-links').send({ label: 'ceiling spec' });
+  const res = await request(app)
+    .post('/api/admin/upload-links')
+    .send({ label: 'ceiling spec', email: 'sarah@example.com' });
   expect(res.status).toBe(201);
   return res.body.token;
 }
diff --git a/backend/tests/integration/uploadLinks.integration.test.ts b/backend/tests/integration/uploadLinks.integration.test.ts
index 6b7c31b..112cb5f 100644
--- a/backend/tests/integration/uploadLinks.integration.test.ts
+++ b/backend/tests/integration/uploadLinks.integration.test.ts
@@ -16,7 +16,7 @@ describe('issuing an upload link', () => {
   it('returns the token exactly once, at creation', async () => {
     const created = await request(app)
       .post('/api/admin/upload-links')
-      .send({ label: 'Sarah' });
+      .send({ label: 'Sarah', email: 'sarah@example.com' });
 
     expect(created.status).toBe(201);
     expect(created.body.label).toBe('Sarah');
@@ -34,7 +34,7 @@ describe('issuing an upload link', () => {
   it('stores the digest rather than the token', async () => {
     const created = await request(app)
       .post('/api/admin/upload-links')
-      .send({ label: 'Estate sale box 3' });
+      .send({ label: 'Estate sale box 3', email: 'sarah@example.com' });
 
     const { rows } = await pool.query<{ token_hash: string }>(
       `SELECT token_hash FROM upload_links`
@@ -51,7 +51,7 @@ describe('issuing an upload link', () => {
   it('refuses a non-positive submission cap', async () => {
     const res = await request(app)
       .post('/api/admin/upload-links')
-      .send({ label: 'Bad cap', maxSubmissions: 0 });
+      .send({ label: 'Bad cap', email: 'sarah@example.com', maxSubmissions: 0 });
     expect(res.status).toBe(400);
   });
 
@@ -59,7 +59,9 @@ describe('issuing an upload link', () => {
   // safe. An unbounded link should be something asked for, not something that
   // happens when nobody thought about it.
   it('bounds a link that was created without a cap', async () => {
-    const res = await request(app).post('/api/admin/upload-links').send({ label: 'Sarah' });
+    const res = await request(app)
+      .post('/api/admin/upload-links')
+      .send({ label: 'Sarah', email: 'sarah@example.com' });
 
     expect(res.status).toBe(201);
     expect(res.body.max_submissions).toBe(25);
@@ -68,7 +70,7 @@ describe('issuing an upload link', () => {
   it('allows unlimited when it is asked for explicitly', async () => {
     const res = await request(app)
       .post('/api/admin/upload-links')
-      .send({ label: 'Always on', maxSubmissions: null });
+      .send({ label: 'Always on', email: 'sarah@example.com', maxSubmissions: null });
 
     expect(res.status).toBe(201);
     expect(res.body.max_submissions).toBeNull();
@@ -79,7 +81,7 @@ describe('revoking an upload link', () => {
   it('stamps revoked_at and reports it in the listing', async () => {
     const created = await request(app)
       .post('/api/admin/upload-links')
-      .send({ label: 'Temporary' });
+      .send({ label: 'Temporary', email: 'sarah@example.com' });
 
     const revoked = await request(app)
       .post(`/api/admin/upload-links/${created.body.id}/revoke`);
@@ -94,7 +96,7 @@ describe('revoking an upload link', () => {
   it('is idempotent, keeping the original timestamp', async () => {
     const created = await request(app)
       .post('/api/admin/upload-links')
-      .send({ label: 'Temporary' });
+      .send({ label: 'Temporary', email: 'sarah@example.com' });
 
     const first = await request(app).post(`/api/admin/upload-links/${created.body.id}/revoke`);
     const second = await request(app).post(`/api/admin/upload-links/${created.body.id}/revoke`);
@@ -108,3 +110,99 @@ describe('revoking an upload link', () => {
     expect(res.status).toBe(404);
   });
 });
+
+describe('emailing the link to its recipient', () => {
+  const original = { ...process.env };
+
+  afterEach(() => {
+    process.env = { ...original };
+  });
+
+  it('refuses to create a link with no address', async () => {
+    const res = await request(app)
+      .post('/api/admin/upload-links')
+      .send({ label: 'Sarah' });
+
+    expect(res.status).toBe(400);
+    expect(res.body.error).toMatch(/email/i);
+  });
+
+  it('refuses an address that is not one', async () => {
+    const res = await request(app)
+      .post('/api/admin/upload-links')
+      .send({ label: 'Sarah', email: 'not-an-address' });
+
+    expect(res.status).toBe(400);
+    expect(res.body.error).toMatch(/email/i);
+  });
+
+  it('stores the address on the link', async () => {
+    const res = await request(app)
+      .post('/api/admin/upload-links')
+      .send({ label: 'Sarah', email: 'sarah@example.com' });
+
+    expect(res.status).toBe(201);
+
+    const { rows } = await pool.query<{ contact_email: string }>(
+      `SELECT contact_email FROM upload_links WHERE id = $1`,
+      [res.body.id]
+    );
+    expect(rows[0]?.contact_email).toBe('sarah@example.com');
+  });
+
+  // The point of the whole change: QA blocks delivery by design, so a link that
+  // was not actually emailed must not be reported as though it was.
+  it('says the mail was not sent when SMTP is not configured', async () => {
+    delete process.env.SMTP_USER;
+    delete process.env.SMTP_PASSWORD;
+
+    const res = await request(app)
+      .post('/api/admin/upload-links')
+      .send({ label: 'Sarah', email: 'sarah@example.com' });
+
+    expect(res.status).toBe(201);
+    expect(res.body.mail).toEqual({ sent: false, outcome: 'skipped-unconfigured' });
+  });
+
+  it('says the mail was not sent when the address is not allowlisted', async () => {
+    process.env.SMTP_USER = 'user';
+    process.env.SMTP_PASSWORD = 'password';
+    process.env.MAIL_ALLOWLIST = 'someone@example.com';
+
+    const res = await request(app)
+      .post('/api/admin/upload-links')
+      .send({ label: 'Sarah', email: 'sarah@example.com' });
+
+    expect(res.status).toBe(201);
+    expect(res.body.mail).toEqual({ sent: false, outcome: 'skipped-blocked' });
+  });
+
+  // A send that could not happen must never cost the admin the link, because
+  // the token is shown exactly once and a rollback would hand them a different
+  // one on the retry.
+  it('still returns a usable link when the mail did not go', async () => {
+    delete process.env.SMTP_USER;
+
+    const res = await request(app)
+      .post('/api/admin/upload-links')
+      .send({ label: 'Sarah', email: 'sarah@example.com' });
+
+    expect(res.body.token).toBeTruthy();
+    expect(res.body.url).toContain(res.body.token);
+    expect(res.body.mail.sent).toBe(false);
+  });
+
+  it('lists the address, and tolerates a link that has none', async () => {
+    await pool.query(`INSERT INTO upload_links (label, token_hash) VALUES ('Older link', 'digest')`);
+    await request(app)
+      .post('/api/admin/upload-links')
+      .send({ label: 'Newer link', email: 'sarah@example.com' });
+
+    const res = await request(app).get('/api/admin/upload-links');
+
+    const older = res.body.find((row: { label: string }) => row.label === 'Older link');
+    const newer = res.body.find((row: { label: string }) => row.label === 'Newer link');
+    expect(older.contact_email).toBeNull();
+    expect(newer.contact_email).toBe('sarah@example.com');
+  });
+});

From 9c5935e8e0db0f5aeba5e63ae0b972bc30917458 Mon Sep 17 00:00:00 2001
From: Thom Lamb <thomlamb@gmail.com>
Date: Thu, 3 Sep 2026 18:07:37 -0500
Subject: [PATCH 07/13] feat(admin): ask for the contributor's address when
 creating a link (#260)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The address is now a required field beside the label, the links table shows where each link was sent, and the admin is told plainly when the mail did not go — with the link still on screen to copy, which is the case that matters in QA and in local development where there is no mail at all.

Two specs in unrelated features created links with only a label and the route now refuses that, so they are updated here rather than left to go red on somebody else's branch. That is the cost of making the address required, and it is a small one: the compiler and the suite find every call site.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 frontend/src/admin/UploadLinks.tsx            | 34 ++++++++++++++++++-
 frontend/tests/e2e/admin-draft-queue.spec.ts  |  2 +-
 frontend/tests/e2e/admin-upload-links.spec.ts | 13 +++++++
 frontend/tests/e2e/intake-submit.spec.ts      |  2 +-
 4 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/frontend/src/admin/UploadLinks.tsx b/frontend/src/admin/UploadLinks.tsx
index 30ecd5d..06efb82 100644
--- a/frontend/src/admin/UploadLinks.tsx
+++ b/frontend/src/admin/UploadLinks.tsx
@@ -14,6 +14,7 @@ const { Paragraph, Text } = Typography;
 interface UploadLink {
   id: number;
   label: string;
+  contact_email: string | null;
   revoked_at: string | null;
   submission_count: number;
   max_submissions: number | null;
@@ -36,11 +37,16 @@ const DEFAULT_CAP = '25';
 export default function UploadLinks() {
   const [links, setLinks] = useState<UploadLink[]>([]);
   const [label, setLabel] = useState('');
+  const [email, setEmail] = useState('');
   const [cap, setCap] = useState(DEFAULT_CAP);
   const [unlimited, setUnlimited] = useState(false);
   // Held only in component state and shown once. A refresh loses it, which is
   // the honest behaviour: the server genuinely cannot produce it again.
   const [issued, setIssued] = useState<string | null>(null);
+  // Whether the link that is currently on screen was actually emailed. Separate
+  // from `issued` so the one-time display of the token keeps working exactly as
+  // it did; this only adds a note beside it.
+  const [mailed, setMailed] = useState<boolean | null>(null);
   const [error, setError] = useState<string | null>(null);
   const [creating, setCreating] = useState(false);
 
@@ -57,6 +63,7 @@ export default function UploadLinks() {
   async function create() {
     setCreating(true);
     setError(null);
+    setMailed(null);
 
     const res = await fetch('/api/admin/upload-links', {
       method: 'POST',
@@ -66,6 +73,7 @@ export default function UploadLinks() {
       // that are not this screen.
       body: JSON.stringify({
         label,
+        email,
         maxSubmissions: unlimited ? null : Number(cap)
       })
     });
@@ -80,7 +88,9 @@ export default function UploadLinks() {
 
     const created = await res.json();
     setIssued(created.url);
+    setMailed(created.mail.sent);
     setLabel('');
+    setEmail('');
     setCap(DEFAULT_CAP);
     setUnlimited(false);
     await load();
@@ -106,6 +116,13 @@ export default function UploadLinks() {
           onChange={(e) => setLabel(e.target.value)}
           style={{ width: 260 }}
         />
+        <Input
+          aria-label="Contributor email"
+          placeholder="Where should the link be sent?"
+          value={email}
+          onChange={(e) => setEmail(e.target.value)}
+          style={{ width: 260 }}
+        />
         <Input
           placeholder="Max uses"
           aria-label="Maximum uses"
@@ -117,13 +134,27 @@ export default function UploadLinks() {
         <Checkbox checked={unlimited} onChange={(e) => setUnlimited(e.target.checked)}>
           No limit
         </Checkbox>
-        <Button type="primary" onClick={create} loading={creating} disabled={label.trim() === ''}>
+        <Button
+          type="primary"
+          onClick={create}
+          loading={creating}
+          disabled={label.trim() === '' || email.trim() === ''}
+        >
           Create link
         </Button>
       </Space>
 
       {error && <Alert type="error" message={error} showIcon />}
 
+      {issued && mailed === false && (
+        <Alert
+          type="warning"
+          showIcon
+          message="The link was not emailed"
+          description="Copy it below and send it yourself. This is normal where no mail is configured, and in QA, where delivery is restricted to a fixed list of addresses."
+        />
+      )}
+
       {issued && (
         <Alert
           type="success"
@@ -151,6 +182,7 @@ export default function UploadLinks() {
         pagination={false}
         columns={[
           { title: 'Label', dataIndex: 'label' },
+          { title: 'Sent to', dataIndex: 'contact_email' },
           {
             title: 'Used',
             render: (_, row) =>
diff --git a/frontend/tests/e2e/admin-draft-queue.spec.ts b/frontend/tests/e2e/admin-draft-queue.spec.ts
index 5fe3720..6cf6c67 100644
--- a/frontend/tests/e2e/admin-draft-queue.spec.ts
+++ b/frontend/tests/e2e/admin-draft-queue.spec.ts
@@ -14,7 +14,7 @@ let token: string;
 test.beforeAll(async ({ playwright }) => {
   const api = await createAdminContext(playwright);
   const res = await api.post('/api/admin/upload-links', {
-    data: { label: `Review queue spec ${RUN}` }
+    data: { label: `Review queue spec ${RUN}`, email: `draft-queue-${RUN}@example.com` }
   });
   expect(res.status(), 'creating the upload link').toBe(201);
   token = (await res.json()).token;
diff --git a/frontend/tests/e2e/admin-upload-links.spec.ts b/frontend/tests/e2e/admin-upload-links.spec.ts
index 3317821..e80b8d3 100644
--- a/frontend/tests/e2e/admin-upload-links.spec.ts
+++ b/frontend/tests/e2e/admin-upload-links.spec.ts
@@ -17,6 +17,7 @@ test.describe('Managing upload links', () => {
     await page.getByRole('tab', { name: 'Upload links' }).click();
 
     await page.getByLabel('Link label').fill(label);
+    await page.getByLabel('Contributor email').fill(`${uniqueSuffix()}@example.com`);
     await page.getByRole('button', { name: 'Create link' }).click();
 
     // Shown exactly once. The server keeps only a digest, so there is no
@@ -38,6 +39,7 @@ test.describe('Managing upload links', () => {
     await admin.goto();
     await page.getByRole('tab', { name: 'Upload links' }).click();
     await page.getByLabel('Link label').fill(label);
+    await page.getByLabel('Contributor email').fill(`${uniqueSuffix()}@example.com`);
     await page.getByRole('button', { name: 'Create link' }).click();
 
     const row = page.getByRole('row', { name: new RegExp(label) });
@@ -61,6 +63,7 @@ test.describe('Managing upload links', () => {
     await admin.goto();
     await page.getByRole('tab', { name: 'Upload links' }).click();
     await page.getByLabel('Link label').fill(label);
+    await page.getByLabel('Contributor email').fill(`${uniqueSuffix()}@example.com`);
     await page.getByText('No limit').click();
     await page.getByRole('button', { name: 'Create link' }).click();
 
@@ -69,4 +72,14 @@ test.describe('Managing upload links', () => {
     // A bare count rather than "0 of N".
     await expect(row.getByText('0 of', { exact: false })).toHaveCount(0);
   });
+
+  // The address is the point of the change: a link nobody can be sent is the
+  // thing this replaced.
+  test('will not create a link without an address', async ({ page, admin }) => {
+    await admin.goto();
+    await page.getByRole('tab', { name: 'Upload links' }).click();
+    await page.getByLabel('Link label').fill(`Nameless ${uniqueSuffix()}`);
+
+    await expect(page.getByRole('button', { name: 'Create link' })).toBeDisabled();
+  });
 });
diff --git a/frontend/tests/e2e/intake-submit.spec.ts b/frontend/tests/e2e/intake-submit.spec.ts
index 507131f..9ab45a4 100644
--- a/frontend/tests/e2e/intake-submit.spec.ts
+++ b/frontend/tests/e2e/intake-submit.spec.ts
@@ -16,7 +16,7 @@ test.beforeAll(async ({ playwright }) => {
   const api = await createAdminContext(playwright);
 
   const res = await api.post('/api/admin/upload-links', {
-    data: { label: `Intake spec ${RUN}` }
+    data: { label: `Intake spec ${RUN}`, email: `intake-${RUN}@example.com` }
   });
   expect(res.status(), 'creating the upload link').toBe(201);
   token = (await res.json()).token;

From 5a9022d8d14591c11a8966cfbc5a355df2f02721 Mon Sep 17 00:00:00 2001
From: Thom Lamb <thomlamb@gmail.com>
Date: Thu, 3 Sep 2026 18:30:35 -0500
Subject: [PATCH 08/13] test(integration): mock the mailer everywhere an upload
 link is issued (#260)

intake.integration.test.ts, intakeCeiling.integration.test.ts and uploadLinks.integration.test.ts all create upload links, and since #260 that now sends real mail. env.setup.ts never clears SMTP_USER, SMTP_PASSWORD or MAIL_ALLOWLIST, so with those inherited from a developer's shell these three files opened live TLS connections to smtp.gmail.com:465 and, with no allowlist set, actually delivered to sarah@example.com. This is the exact hazard the "Do not add one back" comment in tests/unit/mailOutcome.test.ts already warns about, reintroduced at the integration layer.

All three now mock ../../src/mailer the same way accountDetails.integration.test.ts, favorites.integration.test.ts and resendVerification.integration.test.ts already do. uploadLinks.integration.test.ts is the one place that needs to see specific MailOutcome values come back through the route, so its two outcome tests were restructured to drive the mock's return value directly (sentMail.mockResolvedValueOnce(...)) instead of threading SMTP_USER/MAIL_ALLOWLIST through the real sendMail. That is a cleaner test anyway: it isolates the route's job (reporting whatever outcome sendMail returns) from sendMail's own skip logic, which is already covered hermetically by mailOutcome.test.ts and mailAllowlist.test.ts.

Also addresses the related minor finding that nothing asserted the mail actually carried the working link: required: ['submitUrl'] on the template only guards that the placeholder is present in the body, not that the route supplied a correct value for it. A new test in uploadLinks.integration.test.ts inspects the mock's captured call and asserts the html contains the created token's /submit/ URL, and covers the three submissionsAllowed phrasings (a numeric cap, a cap of exactly one, and uncapped).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 .../integration/intake.integration.test.ts    |   8 ++
 .../intakeCeiling.integration.test.ts         |   8 ++
 .../uploadLinks.integration.test.ts           | 107 +++++++++++++-----
 3 files changed, 93 insertions(+), 30 deletions(-)

diff --git a/backend/tests/integration/intake.integration.test.ts b/backend/tests/integration/intake.integration.test.ts
index 3b21868..04b7a1e 100644
--- a/backend/tests/integration/intake.integration.test.ts
+++ b/backend/tests/integration/intake.integration.test.ts
@@ -4,6 +4,14 @@ import app from '../../src/app';
 import { pool } from '../../src/db';
 import { resetDb, closeDb } from './setup/testDb';
 
+// Every submission-cap test here issues a link, which now emails it. Mocked
+// so the suite never opens a real connection to smtp.gmail.com — see the
+// "Do not add one back" warning in tests/unit/mailOutcome.test.ts, which this
+// mirrors at the integration layer.
+jest.mock('../../src/mailer', () => ({
+  sendMail: jest.fn().mockResolvedValue('sent')
+}));
+
 const UPLOADS_DIR = process.env.UPLOADS_DIR as string;
 
 // The same 1x1 PNG the upload validation suite uses, so the accepted case
diff --git a/backend/tests/integration/intakeCeiling.integration.test.ts b/backend/tests/integration/intakeCeiling.integration.test.ts
index 6e125dc..f060b2e 100644
--- a/backend/tests/integration/intakeCeiling.integration.test.ts
+++ b/backend/tests/integration/intakeCeiling.integration.test.ts
@@ -4,6 +4,14 @@ import { pool } from '../../src/db';
 import { resetDb, closeDb } from './setup/testDb';
 import { resetAlertThrottleForTests } from '../../src/intake/abuseAlert';
 
+// makeLink() issues a link on every test, which now emails it. Mocked so the
+// suite never opens a real connection to smtp.gmail.com — see the "Do not add
+// one back" warning in tests/unit/mailOutcome.test.ts, which this mirrors at
+// the integration layer.
+jest.mock('../../src/mailer', () => ({
+  sendMail: jest.fn().mockResolvedValue('sent')
+}));
+
 const PNG = Buffer.from(
   'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==',
   'base64'
diff --git a/backend/tests/integration/uploadLinks.integration.test.ts b/backend/tests/integration/uploadLinks.integration.test.ts
index 112cb5f..d50419b 100644
--- a/backend/tests/integration/uploadLinks.integration.test.ts
+++ b/backend/tests/integration/uploadLinks.integration.test.ts
@@ -3,8 +3,33 @@ import app from '../../src/app';
 import { pool } from '../../src/db';
 import { resetDb, closeDb } from './setup/testDb';
 
+// Every test in this file issues an upload link, which now emails it.
+// Mocked exactly like the other integration suites that touch mail (see
+// accountDetails.integration.test.ts, favorites.integration.test.ts,
+// resendVerification.integration.test.ts) so the suite never opens a real
+// connection to smtp.gmail.com — see the "Do not add one back" warning in
+// tests/unit/mailOutcome.test.ts, which this mirrors at the integration
+// layer.
+//
+// Unlike those three, the "emailing the link to its recipient" describe
+// block below needs to see specific MailOutcome values come back through the
+// route rather than a single fixed one. Threading real SMTP_USER /
+// MAIL_ALLOWLIST env vars through the real sendMail was the previous
+// approach and is exactly the hazard being removed here, so those tests are
+// restructured to drive the mock's return value directly instead — that also
+// makes them a cleaner test of the route's outcome-reporting (its job),
+// separate from sendMail's own skip logic (already covered hermetically by
+// mailOutcome.test.ts and mailAllowlist.test.ts).
+jest.mock('../../src/mailer', () => ({
+  sendMail: jest.fn().mockResolvedValue('sent')
+}));
+import { sendMail, MailOutcome } from '../../src/mailer';
+const sentMail = sendMail as jest.MockedFunction<typeof sendMail>;
+
 beforeEach(async () => {
   await resetDb();
+  sentMail.mockReset();
+  sentMail.mockResolvedValue('sent');
 });
 
 afterAll(async () => {
@@ -112,12 +137,6 @@ describe('revoking an upload link', () => {
 });
 
 describe('emailing the link to its recipient', () => {
-  const original = { ...process.env };
-
-  afterEach(() => {
-    process.env = { ...original };
-  });
-
   it('refuses to create a link with no address', async () => {
     const res = await request(app)
       .post('/api/admin/upload-links')
@@ -151,37 +170,29 @@ describe('emailing the link to its recipient', () => {
   });
 
   // The point of the whole change: QA blocks delivery by design, so a link that
-  // was not actually emailed must not be reported as though it was.
-  it('says the mail was not sent when SMTP is not configured', async () => {
-    delete process.env.SMTP_USER;
-    delete process.env.SMTP_PASSWORD;
+  // was not actually emailed must not be reported as though it was. Driven
+  // through the mock's return value rather than the SMTP_USER /
+  // MAIL_ALLOWLIST env vars sendMail itself would branch on — see the file
+  // header comment for why.
+  it.each<MailOutcome>(['skipped-unconfigured', 'skipped-blocked'])(
+    'reports a %s outcome from sendMail rather than as though it sent',
+    async (outcome) => {
+      sentMail.mockResolvedValueOnce(outcome);
 
-    const res = await request(app)
-      .post('/api/admin/upload-links')
-      .send({ label: 'Sarah', email: 'sarah@example.com' });
+      const res = await request(app)
+        .post('/api/admin/upload-links')
+        .send({ label: 'Sarah', email: 'sarah@example.com' });
 
-    expect(res.status).toBe(201);
-    expect(res.body.mail).toEqual({ sent: false, outcome: 'skipped-unconfigured' });
-  });
-
-  it('says the mail was not sent when the address is not allowlisted', async () => {
-    process.env.SMTP_USER = 'user';
-    process.env.SMTP_PASSWORD = 'password';
-    process.env.MAIL_ALLOWLIST = 'someone@example.com';
-
-    const res = await request(app)
-      .post('/api/admin/upload-links')
-      .send({ label: 'Sarah', email: 'sarah@example.com' });
-
-    expect(res.status).toBe(201);
-    expect(res.body.mail).toEqual({ sent: false, outcome: 'skipped-blocked' });
-  });
+      expect(res.status).toBe(201);
+      expect(res.body.mail).toEqual({ sent: false, outcome });
+    }
+  );
 
   // A send that could not happen must never cost the admin the link, because
   // the token is shown exactly once and a rollback would hand them a different
   // one on the retry.
   it('still returns a usable link when the mail did not go', async () => {
-    delete process.env.SMTP_USER;
+    sentMail.mockResolvedValueOnce('skipped-unconfigured');
 
     const res = await request(app)
       .post('/api/admin/upload-links')
@@ -205,4 +216,40 @@ describe('emailing the link to its recipient', () => {
     expect(older.contact_email).toBeNull();
     expect(newer.contact_email).toBe('sarah@example.com');
   });
+
+  // required: ['submitUrl'] on the template is a guard that the placeholder is
+  // present in the body, not that the route supplied a working value for it.
+  // A route that passed the bare base URL, or dropped the token, would leave
+  // every other test here green — so this asserts the actual captured html,
+  // and covers the submissionsAllowed wording for all three cases (a numeric
+  // cap, a cap of exactly one, and uncapped) at the same time, since all three
+  // are the same kind of claim: what the mail says versus what was created.
+  it('emails a link that actually contains the created token, and states how many items may be sent', async () => {
+    const capped = await request(app)
+      .post('/api/admin/upload-links')
+      .send({ label: 'Sarah', email: 'sarah@example.com', maxSubmissions: 25 });
+    expect(capped.status).toBe(201);
+    expect(sentMail).toHaveBeenCalledTimes(1);
+    const [cappedTo, , cappedHtml] = sentMail.mock.calls[0]!;
+    expect(cappedTo).toBe('sarah@example.com');
+    expect(cappedHtml).toContain(`/submit/${capped.body.token}`);
+    expect(cappedHtml).toContain('25 items');
+
+    const single = await request(app)
+      .post('/api/admin/upload-links')
+      .send({ label: 'Sarah', email: 'sarah@example.com', maxSubmissions: 1 });
+    expect(single.status).toBe(201);
+    const [, , singleHtml] = sentMail.mock.calls[1]!;
+    expect(singleHtml).toContain(`/submit/${single.body.token}`);
+    expect(singleHtml).toContain('1 item');
+    expect(singleHtml).not.toContain('1 items');
+
+    const uncapped = await request(app)
+      .post('/api/admin/upload-links')
+      .send({ label: 'Sarah', email: 'sarah@example.com', maxSubmissions: null });
+    expect(uncapped.status).toBe(201);
+    const [, , uncappedHtml] = sentMail.mock.calls[2]!;
+    expect(uncappedHtml).toContain(`/submit/${uncapped.body.token}`);
+    expect(uncappedHtml).toContain('as many items as you like');
+  });
 });

From 0dd4ac36ff21492ad78896c564f18b9ce295c5d5 Mon Sep 17 00:00:00 2001
From: Thom Lamb <thomlamb@gmail.com>
Date: Thu, 3 Sep 2026 18:30:49 -0500
Subject: [PATCH 09/13] fix(mail): bound the transporter's timeouts, and fix a
 now-stale comment (#260)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

adminUploadLinks.ts awaits sendMail on the admin's request path, the first awaited send on a user-facing request in this codebase, but the transporter in mailer.ts set no connectionTimeout, greetingTimeout or socketTimeout. nodemailer's defaults then apply: two minutes to connect, ten minutes on the socket. If the SMTP host is unreachable in a way that drops packets rather than refusing, the link row and its token are already committed by the time sendMail is called, the response hangs for up to two minutes, the browser or reverse proxy gives up first, and the token — shown exactly once and unrecoverable — is never rendered. That is the link being lost in exactly the way this feature's central invariant forbids.

All three timeouts are now set to 5000ms, with a comment explaining why a send on a request path has to fail fast rather than inherit nodemailer's fire-and-forget defaults. Five seconds is generous for a reachable host and short enough that a dead one fails while the admin is still willing to wait, leaving them the "not emailed" warning and a link they can still copy instead of a stuck spinner and a token nobody ever saw.

Also corrects the comment directly above the skipped-blocked return, which said "returning as though it sent" — true before #260, and precisely backwards now that the outcome is reported through MailOutcome rather than swallowed. Reworded to describe what the code actually does today, keeping the explanation of why it skips rather than throws.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 backend/src/mailer.ts | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/backend/src/mailer.ts b/backend/src/mailer.ts
index bfa1237..ff1fe00 100755
--- a/backend/src/mailer.ts
+++ b/backend/src/mailer.ts
@@ -4,6 +4,16 @@ import nodemailer from 'nodemailer';
 // Brevo — has to set host, port and SMTP_SECURE explicitly rather than
 // inheriting these, and getting that wrong fails at send time rather than at
 // boot. See #64 on validating this at startup instead.
+// #260 put the first awaited send on a user-facing request path (the admin
+// creating an upload link). nodemailer's defaults are two minutes to connect
+// and ten minutes on the socket, which is fine for a fire-and-forget send but
+// is not a bound anyone waiting on a response can live with: the link row and
+// its token are already committed by the time sendMail is called, the token
+// is shown exactly once, and a request that hangs long enough for the browser
+// or reverse proxy to give up first loses it for good. Five seconds each is
+// long enough for a reachable host and short enough that a dead one fails
+// fast, leaving the admin with the "not emailed" warning and a link they can
+// still copy, instead of a stuck spinner and a token nobody ever saw.
 const transporter = nodemailer.createTransport({
   host: process.env.SMTP_HOST || 'smtp.gmail.com',
   port: parseInt(process.env.SMTP_PORT || '465', 10),
@@ -11,7 +21,10 @@ const transporter = nodemailer.createTransport({
   auth: {
     user: process.env.SMTP_USER,
     pass: process.env.SMTP_PASSWORD
-  }
+  },
+  connectionTimeout: 5000,
+  greetingTimeout: 5000,
+  socketTimeout: 5000
 });
 
 interface ParsedAddress {
@@ -105,11 +118,12 @@ export async function sendMail(to: string, subject: string, html: string): Promi
   // Guarded here rather than at the four call sites, so every sender is covered
   // by construction and a fifth added later cannot bypass it by forgetting.
   //
-  // Skipping rather than throwing, and returning as though it sent: three of
-  // the callers already swallow send failures into a log, so throwing would
-  // mostly be caught and logged anyway while risking a 500 on the signup path.
-  // The flow under test finishes, and the log says why no mail arrived — which
-  // is the part that was missing when QA was simply muted.
+  // Skipping rather than throwing, and reporting the skip through MailOutcome
+  // rather than pretending nothing happened: three of the callers already
+  // swallow send failures into a log, so throwing would mostly be caught and
+  // logged anyway while risking a 500 on the signup path. The flow under test
+  // finishes, and both the log and the returned outcome say why no mail
+  // arrived — which is the part that was missing when QA was simply muted.
   if (!isAllowedRecipient(to, process.env.MAIL_ALLOWLIST)) {
     console.warn(`[mail-blocked] ${to} is not on MAIL_ALLOWLIST — skipping "${subject}"`);
     return 'skipped-blocked';

From ba26ee99f0f5f0b5eb7429d64dd354e50cce6620 Mon Sep 17 00:00:00 2001
From: Thom Lamb <thomlamb@gmail.com>
Date: Thu, 3 Sep 2026 18:31:03 -0500
Subject: [PATCH 10/13] fix(mail-templates): add sample values for the
 upload-link placeholders (#260)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

submitUrl, label and submissionsAllowed were added to the uploadLink template but SAMPLE_VALUES had no entries for any of them, so an admin opening Email templates, Upload link for a contributor, Preview saw literal {{submitUrl}} in the body — the preview being the only way to check an edit before saving.

The unit test meant to catch exactly this, in emailTemplates.test.ts, iterated a hardcoded KEYS array that predated intakeDraft and uploadLink, so it never checked either template's samples. KEYS is now Object.keys(TEMPLATES) as TemplateKey[], so the guard covers every template automatically and cannot go stale the same way again. intakeDraft already had samples for all its placeholders and passes once included, as expected.

One other test in the same file, "every template can address the customer", asserts that available contains greeting/firstName/lastName — a real invariant of the six customer-facing templates, but not of intakeDraft or uploadLink, which notify the shop and a contributor rather than a customer with a name on file. Switching that test to the new all-templates KEYS would have made it fail for both, so it now uses its own explicit CUSTOMER_FACING_KEYS list instead. That is a deliberate, commented exception: a hardcoded list is correct there because the claim itself does not extend to every template, whereas the SAMPLE_VALUES guard's claim does.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 backend/src/emailTemplates.ts             |  5 +++-
 backend/tests/unit/emailTemplates.test.ts | 30 ++++++++++++++++-------
 2 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/backend/src/emailTemplates.ts b/backend/src/emailTemplates.ts
index 60c1a8c..68594d0 100644
--- a/backend/src/emailTemplates.ts
+++ b/backend/src/emailTemplates.ts
@@ -281,7 +281,10 @@ export const SAMPLE_VALUES: Record<string, string> = {
   regenerateUrl: 'https://example.com/api/intake-actions/1/regenerate?expires=0&sig=sample',
   discardUrl: 'https://example.com/api/intake-actions/1/discard?expires=0&sig=sample',
   expiresIn: 'one hour',
-  holdDuration: '24 hours'
+  holdDuration: '24 hours',
+  submitUrl: 'https://example.com/submit/sample-token',
+  label: 'Autumn drop-off',
+  submissionsAllowed: '25 items'
 };
 
 export interface StoredTemplate {
diff --git a/backend/tests/unit/emailTemplates.test.ts b/backend/tests/unit/emailTemplates.test.ts
index 302c533..0d9935a 100644
--- a/backend/tests/unit/emailTemplates.test.ts
+++ b/backend/tests/unit/emailTemplates.test.ts
@@ -8,14 +8,11 @@ import {
   SAMPLE_VALUES
 } from '../../src/emailTemplates';
 
-const KEYS: TemplateKey[] = [
-  'verification',
-  'passwordReset',
-  'favoriteSold',
-  'favoriteWithdrawn',
-  'cartReminder',
-  'emailChanged'
-];
+// Derived from TEMPLATES rather than hardcoded, so a new template is covered
+// by every it.each below the moment it is added. A hardcoded list silently
+// stops covering anything added after it was written — which is exactly how
+// intakeDraft and uploadLink went untested by the SAMPLE_VALUES guard below.
+const KEYS = Object.keys(TEMPLATES) as TemplateKey[];
 
 describe('the built-in templates', () => {
   it.each(KEYS)('%s has a default subject and body', (key) => {
@@ -268,7 +265,22 @@ describe('greeting, built from the configured format', () => {
 });
 
 describe('every template can address the customer', () => {
-  it.each(KEYS)('%s offers greeting, firstName and lastName', (key) => {
+  // Not KEYS: this is an invariant of the six customer-facing templates only.
+  // intakeDraft and uploadLink notify the shop and a contributor respectively,
+  // not a customer with a name on file, so they are deliberately not held to
+  // it — a hardcoded list is correct here rather than a staleness risk,
+  // because the set of templates this claim applies to does not grow just
+  // because TEMPLATES does.
+  const CUSTOMER_FACING_KEYS: TemplateKey[] = [
+    'verification',
+    'passwordReset',
+    'favoriteSold',
+    'favoriteWithdrawn',
+    'cartReminder',
+    'emailChanged'
+  ];
+
+  it.each(CUSTOMER_FACING_KEYS)('%s offers greeting, firstName and lastName', (key) => {
     expect(TEMPLATES[key].available).toEqual(
       expect.arrayContaining(['greeting', 'firstName', 'lastName'])
     );

From c12d40dc02f06a887d6e45a8ebe79655f6f83372 Mon Sep 17 00:00:00 2001
From: Thom Lamb <thomlamb@gmail.com>
Date: Thu, 3 Sep 2026 18:31:16 -0500
Subject: [PATCH 11/13] fix(admin): stop the upload links table claiming
 delivery it never recorded (#260)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The table headed its address column "Sent to", but contact_email records only the intent an admin typed in, never whether delivery happened — that outcome is shown once, at creation, and is not persisted. In QA, where every send is blocked by design, every row read "Sent to ..." for links that were never emailed, and the honest warning that appears at creation is guarded on `issued`, so it vanishes on refresh, leaving the false heading as the only surviving statement. Renamed to "Email", which is true of what the column actually stores.

Separately, create() called setMailed(null) before every request, including one that would go on to 400. If an admin creates link A while mail is down (warning shown, token A still on screen) and then mistypes an address on a second attempt, the 400 path returned early — but the reset had already run, so the warning for link A disappeared while token A was still displayed on the same screen. setMailed is now only called after a successful create, alongside setIssued, so a request that never produces a new link can no longer clear a warning that belongs to the one still shown.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 frontend/src/admin/UploadLinks.tsx | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/frontend/src/admin/UploadLinks.tsx b/frontend/src/admin/UploadLinks.tsx
index 06efb82..6e3ea9f 100644
--- a/frontend/src/admin/UploadLinks.tsx
+++ b/frontend/src/admin/UploadLinks.tsx
@@ -63,7 +63,6 @@ export default function UploadLinks() {
   async function create() {
     setCreating(true);
     setError(null);
-    setMailed(null);
 
     const res = await fetch('/api/admin/upload-links', {
       method: 'POST',
@@ -87,8 +86,12 @@ export default function UploadLinks() {
     }
 
     const created = await res.json();
-    setIssued(created.url);
+    // Reset here, not before the request: a 400 returns before this point, so
+    // rejecting a second link (say, a mistyped address) can no longer clear
+    // the warning that belongs to a still-displayed token from an earlier,
+    // successful create.
     setMailed(created.mail.sent);
+    setIssued(created.url);
     setLabel('');
     setEmail('');
     setCap(DEFAULT_CAP);
@@ -182,7 +185,12 @@ export default function UploadLinks() {
         pagination={false}
         columns={[
           { title: 'Label', dataIndex: 'label' },
-          { title: 'Sent to', dataIndex: 'contact_email' },
+          // Not "Sent to": contact_email records only the address the admin
+          // gave when the link was created, never whether delivery actually
+          // happened — that outcome is shown once, at creation, and is not
+          // persisted. In QA, where every send is blocked by design, "Sent
+          // to" would be false for every row on the page.
+          { title: 'Email', dataIndex: 'contact_email' },
           {
             title: 'Used',
             render: (_, row) =>

From 4e7f2557829ce43a20aae4251e58d40f43cf1b43 Mon Sep 17 00:00:00 2001
From: Thom Lamb <thomlamb@gmail.com>
Date: Thu, 3 Sep 2026 18:31:25 -0500
Subject: [PATCH 12/13] fix(admin): return contact_email from the revoke query
 (#260)

The revoke route's RETURNING clause omitted contact_email while its result was typed as UploadLinkRow, which declares the field as present. No user-visible effect since the response was never checked for it, but the type asserted something the query did not actually return. Added the column so the two agree.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 backend/src/routes/adminUploadLinks.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/backend/src/routes/adminUploadLinks.ts b/backend/src/routes/adminUploadLinks.ts
index d1f5b7c..72ef918 100644
--- a/backend/src/routes/adminUploadLinks.ts
+++ b/backend/src/routes/adminUploadLinks.ts
@@ -182,7 +182,7 @@ router.post('/:id/revoke', asyncRoute(async (req: Request, res: Response) => {
   const { rows } = await pool.query<UploadLinkRow>(
     `UPDATE upload_links SET revoked_at = COALESCE(revoked_at, now())
      WHERE id = $1
-     RETURNING id, label, revoked_at, submission_count, max_submissions, last_used_at, created_at`,
+     RETURNING id, label, contact_email, revoked_at, submission_count, max_submissions, last_used_at, created_at`,
     [req.params.id]
   );
 

From 046937cdceae621fa31a7871deaa4fef8cb65794 Mon Sep 17 00:00:00 2001
From: Thom Lamb <thomlamb@gmail.com>
Date: Thu, 3 Sep 2026 18:31:35 -0500
Subject: [PATCH 13/13] test(e2e): assert a created link's address shows in the
 table (#260)

The spec's E2E section requires that a created link shows its address in the table, but the test that creates one filled a throwaway address and never asserted the cell. The address is now kept in a variable and asserted on the row after creation, using it to locate the row's own cell rather than any other row's.

Not run: the local stack is down and Playwright was explicitly out of bounds for this pass, per the task instructions. This is written and verified by inspection and lint only.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 frontend/tests/e2e/admin-upload-links.spec.ts | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/frontend/tests/e2e/admin-upload-links.spec.ts b/frontend/tests/e2e/admin-upload-links.spec.ts
index e80b8d3..81be907 100644
--- a/frontend/tests/e2e/admin-upload-links.spec.ts
+++ b/frontend/tests/e2e/admin-upload-links.spec.ts
@@ -12,12 +12,13 @@ import { test, expect, uniqueSuffix } from './fixtures';
 test.describe('Managing upload links', () => {
   test('issues a link, shows its token once, and lists it bounded', async ({ page, admin }) => {
     const label = `Sarah ${uniqueSuffix()}`;
+    const email = `${uniqueSuffix()}@example.com`;
 
     await admin.goto();
     await page.getByRole('tab', { name: 'Upload links' }).click();
 
     await page.getByLabel('Link label').fill(label);
-    await page.getByLabel('Contributor email').fill(`${uniqueSuffix()}@example.com`);
+    await page.getByLabel('Contributor email').fill(email);
     await page.getByRole('button', { name: 'Create link' }).click();
 
     // Shown exactly once. The server keeps only a digest, so there is no
@@ -28,6 +29,8 @@ test.describe('Managing upload links', () => {
 
     const row = page.getByRole('row', { name: new RegExp(label) });
     await expect(row).toBeVisible();
+    // A created link shows its address in the table.
+    await expect(row.getByText(email)).toBeVisible();
     // The default cap, not unlimited. An unbounded link should be asked for.
     await expect(row.getByText('0 of 25')).toBeVisible();
     await expect(row.getByText('Active')).toBeVisible();