From dbe6a0cf8f01351b7d87e9d5925459cd1b44db59 Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Sat, 29 Aug 2026 07:43:47 -0500 Subject: [PATCH 1/5] docs(intake): design the upload-link, AI-draft and review-queue pipeline (#220) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A named, revocable link lets someone without an account send in photos of one item plus a note; a background worker drafts the listing; the admin is emailed and publishes it deliberately from a review queue. Most of the lifecycle already exists and is reused rather than rebuilt: `pending` has been the unpublished state since #90 and is already excluded from every public query, the upload path already validates magic bytes against a three-type allowlist, mail already has editable templates and an allowlist guard, and node-cron is already the background-work pattern. What is new is a way in for someone with no admin account, the first LLM integration in this codebase, and somewhere to review a draft. The design turns on one invariant: nothing reaches the storefront at a price a model guessed. The suggested price lives on the draft and never on the item, the email carries no publish button, and the publish path refuses an item with no price. That is also why `price_cents` becomes nullable rather than defaulting to zero — a sentinel that formats as "$0.00" is the same class of quiet failure as `DEMO_MODE` once being "demo unless the value is exactly false", and nullability makes the compiler enumerate all fifteen call sites instead. Ref #220 --- .../2026-08-29-intake-pipeline-design.md | 178 ++++++++++++++++++ 1 file changed, 178 insertions(+) create mode 100644 docs/superpowers/specs/2026-08-29-intake-pipeline-design.md diff --git a/docs/superpowers/specs/2026-08-29-intake-pipeline-design.md b/docs/superpowers/specs/2026-08-29-intake-pipeline-design.md new file mode 100644 index 0000000..672d272 --- /dev/null +++ b/docs/superpowers/specs/2026-08-29-intake-pipeline-design.md @@ -0,0 +1,178 @@ +# Intake Pipeline — Design + +**Issue:** [#220 — Shared upload links, AI-drafted listings, and an admin review queue](https://gitea.bermudalamb.synology.me/bermudalamb/redefined-designs/issues/220) +**Date:** 2026-08-29 +**Status:** Draft + +## Goal + +Someone who is not the admin can send in photos of one item through a link they were given. A draft listing is written for it automatically. The admin is told by email, and the item sits unpublished until the admin has read the copy, typed a price and published it deliberately. + +## Where this starts from + +Most of the lifecycle this feature needs is already built, which is why the design below adds a pipeline in front of it rather than a parallel one beside it. + +- **`pending` is already the unpublished state.** `items.status` defaults to `'pending'` since #90, `NON_PUBLIC_STATUSES` keeps it out of every public and storefront query, and `POST /admin/items/:id/mark-available` and `/unpublish` already move an item across that line. Nothing here needs a new visibility concept, and inventing one would mean a second thing to keep correct. +- **Uploads are already hardened.** Multer writes to `UPLOADS_DIR`, `uploadTypes.ts` checks the declared type against the file's leading bytes, the allowlist is three image types, and `uploads.ts` refuses to serve anything it does not recognise. A new upload path that did not go through those is a new hole. +- **Mail already exists,** including admin-editable markdown bodies with placeholder validation, the `MAIL_ALLOWLIST` guard that stops non-production environments mailing real people, and `PUBLIC_URL` for building links. +- **Background work has a pattern.** `cron.schedule('0 9 * * *', ...)` in `server.ts` drives the cart reminders. This is a single-instance deployment, which is what makes in-process scheduling and the in-memory rate-limit store tenable. + +Three things do not exist: a way in for someone without an admin account, any LLM integration at all, and anywhere to review a draft. + +## Decisions + +Settled in conversation before this was written, and recorded on the issue. + +| Question | Decision | +| --- | --- | +| What the "shared folder" is | A web upload page, not a watched NAS share or a mailbox | +| Who can upload | Anyone holding a named, revocable link — no account | +| What the uploader supplies | Photos of one item, plus one free-text note | +| What the AI drafts | Name, marketing description, category and tags from the existing taxonomy, and a suggested price | +| Where a submission lives pre-approval | An `items` row at `status='pending'`, not a separate submissions table | +| How the admin acts on it | Signed one-click links in the email, plus a review queue in the admin | +| Whether one click can publish | No. Publishing always requires a typed price | +| Model | Sonnet 5, in an env var | + +## The invariant + +**Nothing reaches the storefront at a price a model guessed.** Everything below follows from that: the suggested price lives on the draft and never on the item, the email has no publish button, and the publish path refuses an item with no price. + +## Architecture + +### Schema + +Three migrations. + +```sql +CREATE TABLE upload_links ( + id SERIAL PRIMARY KEY, + label TEXT NOT NULL, + -- The token is never stored. A leaked database is not also a leaked set of + -- working upload links, and the admin screen can show a token exactly once, + -- at creation, for the same reason a password reset link is not re-readable. + token_hash TEXT NOT NULL UNIQUE, + revoked_at TIMESTAMPTZ, + submission_count INTEGER NOT NULL DEFAULT 0, + -- Null means no cap. A link handed to a regular contributor is open-ended; + -- one handed out for a single box of stock is not. + max_submissions INTEGER, + last_used_at TIMESTAMPTZ, + created_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +CREATE TABLE item_drafts ( + id SERIAL PRIMARY KEY, + item_id INTEGER NOT NULL UNIQUE REFERENCES items(id) ON DELETE CASCADE, + -- SET NULL rather than CASCADE: deleting a link must not delete the items + -- that came in through it. Provenance is lost; the goods are not. + upload_link_id INTEGER REFERENCES upload_links(id) ON DELETE SET NULL, + submitter_note TEXT, + state TEXT NOT NULL DEFAULT 'queued', -- queued | drafting | ready | failed | discarded + attempts INTEGER NOT NULL DEFAULT 0, + model TEXT, + ai_name TEXT, + ai_description TEXT, + ai_category_id INTEGER REFERENCES categories(id) ON DELETE SET NULL, + ai_tag_names TEXT[], + -- Deliberately not on items. See the invariant. + ai_suggested_price_cents INTEGER, + ai_error TEXT, + input_tokens INTEGER, + output_tokens INTEGER, + cost_micros INTEGER, + drafted_at TIMESTAMPTZ, + reviewed_at TIMESTAMPTZ, + created_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +ALTER TABLE items ALTER COLUMN price_cents DROP NOT NULL; +``` + +**On making `price_cents` nullable.** This is the most invasive line in the design: `price_cents` is referenced across fifteen files including the cart and the checkout. The alternative is to insert a submission at `0` and guard publishing on `price_cents > 0`, which changes no types and no other file. + +It is still the wrong choice. Zero is a real number that formats as a real price, and a sentinel that renders as "$0.00" in a shop is precisely the failure this codebase already has a scar from — `DEMO_MODE` meaning "demo unless the value is exactly false" quietly stopped the shop charging anyone. Null cannot be formatted by accident. And the blast radius is not hidden work but the opposite: making the column nullable makes the TypeScript type `number | null`, and the compiler then names every one of those fifteen sites and refuses to build until each has said what it does with an unpriced item. A sentinel gets the same reach with none of the enumeration. + +The publish guard belongs in `mark-available` alongside the existing status checks, and is worth an integration test of its own. + +### Ingestion + +`POST /api/intake/:token` — public, unauthenticated, `uploadImages` for the files. + +1. Hash the token, look up a matching `upload_links` row that is not revoked. No match, revoked, or over its cap: `404`. Not `403` — whether a link exists is not something a stranger needs to be able to distinguish, which is the reasoning `uploads.ts` already applies. +2. Validate the photos through the existing `uploadTypes` checks. Caps: at most 10 photos, at most 10 MB each. +3. In one transaction: insert the `items` row (`status='pending'`, `price_cents=NULL`, a placeholder name — the submission timestamp — replaced by the draft or the admin), insert `item_images`, insert `item_drafts` at `state='queued'`, bump the link's counter. +4. Respond immediately. The uploader is told it arrived and that a person will look at it. **The AI is not called here** — a slow or failing API call must not turn into a failed upload for someone who did nothing wrong. + +Rate limited by IP through a new limiter in `rateLimit.ts`. `keyByCallerAndEmail` does not fit — there is no email — so this needs its own key function over `ipKeyGenerator` alone, and the existing comment about a bare `ip:` bucket being a shared allowance applies and is accepted here: the link itself is the per-caller identity, and its counter is the per-caller cap. + +### The drafting worker + +A new `backend/src/intakeDrafting.ts`, driven both by a call at the end of a successful submission and by a `node-cron` sweeper that picks up anything left `queued` or stuck in `drafting`. The sweeper is what makes a restart mid-draft recoverable rather than a permanently stalled row. + +The request, via `@anthropic-ai/sdk` and `client.messages.parse()` with `zodOutputFormat`, so the shape is validated rather than parsed out of prose: + +- Every photo as a base64 `image` block. +- The submitter's note verbatim, clearly framed as **the only trustworthy factual claims available**. The system prompt says plainly: describe what is visible, use the note for anything not visible, and never state a material, age, maker or provenance that is in neither. On a one-of-a-kind item, an invented "1930s hand-thrown stoneware" is not a cosmetic error — it is a false claim on a storefront. +- The existing categories and tags as the closed set to choose from, so the draft lands inside the taxonomy the filters already work on. +- The suggested price framed as a rough starting point. + +The note is untrusted input from an unauthenticated stranger. It is passed as data, and nothing the model returns is executed, interpolated into SQL, or rendered as HTML — the drafted description goes through the same markdown-with-`html: false` treatment as every other stored body, which is what makes "a model wrote this" and "a person wrote this" equally safe to render. + +Result: `state='ready'`, the fields populated, token counts and computed cost recorded. Failure: `attempts` incremented, `ai_error` stored, and after three attempts `state='failed'` — which still leaves a perfectly good pending item with photos in the review queue, just with no draft copy. A failed AI call must never lose someone's submission. + +**Spend ceiling.** A monthly cap (`INTAKE_MONTHLY_BUDGET_USD`) summed from `cost_micros`. Past it, submissions still save and still notify; only the API call is skipped, with `state='failed'` and an error saying so. The ceiling protects the bill, and it must not be the thing that loses inventory. + +### Notification and signed links + +Reuses `emailTemplates.ts` with a new `intakeDraft` key, so the copy is editable from the settings screen like every other template, with `reviewUrl` required. + +Action links are HMAC-signed with a new `INTAKE_ACTION_SECRET` over `(draftId, action, expiry)`, verified with a timing-safe comparison, good for 30 days. + +- **Regenerate** — back to `state='queued'`. +- **Discard** — `state='discarded'` and the item unpublished; recoverable from the queue, because a one-click destructive action reachable from an inbox should not be final. +- **Review & publish** — an ordinary deep link into the admin queue, behind authentik like the rest of `/admin`. It carries no signature and grants nothing. + +The two signed actions are deliberately the ones whose worst case is a wasted API call or a recoverable hide. Nothing a single click can do puts an item on the storefront. + +### The review queue + +A new admin screen listing drafts by state: photos, the submitter's note, which link it came from, the drafted copy in editable fields, the suggested price shown as a suggestion beside an empty price input, and Publish / Regenerate / Discard. Publish writes the edited values onto the item and calls the existing `mark-available`. + +Reuses the existing admin item components where they fit rather than growing a second item editor. + +### Configuration + +New variables, added to `envValidation.ts` and — per #107, enforced by `composeEnvironment.test.ts` — to both compose files if they go on the required list. + +| Variable | Requirement | +| --- | --- | +| `ANTHROPIC_API_KEY` | Required when intake is enabled | +| `INTAKE_MODEL` | Optional, defaults to `claude-sonnet-5` | +| `INTAKE_MONTHLY_BUDGET_USD` | Optional, defaults to `20` | +| `INTAKE_ACTION_SECRET` | Required when intake is enabled | + +Intake is feature-flagged off by default, so none of these become required for an environment that does not run it. `PUBLIC_URL` is already required alongside SMTP and is what the review links are built from. + +## Error handling + +The through-line: **a submission is the only irreplaceable thing here.** Photos of a one-of-a-kind object may not be retakeable — the item may not be in the sender's hands any more. Every failure mode below is arranged so the photos survive it. + +| Failure | Behaviour | +| --- | --- | +| API call fails or times out | Retry to three attempts, then `state='failed'`. Item and photos intact, reviewable with no draft | +| Budget exhausted | Submission saved, draft skipped, admin still notified | +| Malformed model output | Rejected by the schema, counts as a failed attempt | +| SMTP down | Draft still `ready` and visible in the queue; the queue, not the email, is the source of truth | +| Link revoked mid-upload | `404`. Already-submitted items are unaffected | +| Restart mid-draft | The cron sweeper re-queues anything stuck in `drafting` | + +## Testing + +- **Unit** — token hashing and constant-time verification, HMAC signing and expiry, the budget calculation, the prompt builder's handling of an empty note, and the publish guard's price rule. All pure, in the style of `keyByCallerAndEmail` and `isAllowedRecipient` being exported specifically to be tested directly. +- **Integration** — submission through a valid link creates a pending item with images and a queued draft; a revoked link gets `404`; publishing without a price is refused; a signed action link works once and an expired or tampered one does not. The Anthropic client is stubbed; no test spends money. +- **E2E** — the submission page, and the queue round trip from draft to published. Assertions scoped to the item under test rather than the whole grid, since the dev database never truncates. + +## Out of scope + +Contributor accounts, a watched NAS folder, email-in submission, multi-item submissions, image editing or cropping, and regenerating with a steer ("try again, warmer"). All are additive later; none change the schema above. -- 2.54.0 From 722bade3831dc0e56bbdf9e2bb48832afeaed1fb Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Sat, 29 Aug 2026 07:52:09 -0500 Subject: [PATCH 2/5] docs(intake): price arriving items rather than leaving them unpriced (#220) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `price_cents` stays NOT NULL and gains a default of 80.00. Where the model suggests a price the worker writes it onto the item; where it does not, the default stands. This is cheaper to build than the nullable design it replaces — the column's type is unchanged, so the fifteen files that read `price_cents`, the cart and the checkout among them, keep working untouched, and the migration adds a default and nothing else. It also gives up a guarantee. The storefront can now be reached by a price the admin never chose, so the protection moves out of the schema and into the review queue, where it is weaker. 80.00 is a plausible number rather than an obvious sentinel, so a default left unnoticed sells the item instead of announcing itself the way "$0.00" would. `price_source` is added to make that legible: the queue labels a price as coming from the model, the default, or the admin, and marks anything unconfirmed as such at the point of publishing. Publishing unconfirmed remains allowed — that is the decision taken — but it is stated rather than silent. Publishing still happens only from the queue, and the notification email still carries no publish button. Ref #220 --- .../2026-08-29-intake-pipeline-design.md | 58 ++++++++++++++----- 1 file changed, 43 insertions(+), 15 deletions(-) diff --git a/docs/superpowers/specs/2026-08-29-intake-pipeline-design.md b/docs/superpowers/specs/2026-08-29-intake-pipeline-design.md index 672d272..60a3f16 100644 --- a/docs/superpowers/specs/2026-08-29-intake-pipeline-design.md +++ b/docs/superpowers/specs/2026-08-29-intake-pipeline-design.md @@ -6,7 +6,7 @@ ## Goal -Someone who is not the admin can send in photos of one item through a link they were given. A draft listing is written for it automatically. The admin is told by email, and the item sits unpublished until the admin has read the copy, typed a price and published it deliberately. +Someone who is not the admin can send in photos of one item through a link they were given. A draft listing is written for it automatically. The admin is told by email, and the item sits unpublished until the admin has read the copy, checked the price and published it deliberately. ## Where this starts from @@ -31,12 +31,22 @@ Settled in conversation before this was written, and recorded on the issue. | What the AI drafts | Name, marketing description, category and tags from the existing taxonomy, and a suggested price | | Where a submission lives pre-approval | An `items` row at `status='pending'`, not a separate submissions table | | How the admin acts on it | Signed one-click links in the email, plus a review queue in the admin | -| Whether one click can publish | No. Publishing always requires a typed price | +| Whether one click can publish | No. Publishing always happens from the review queue | +| What an item is priced at on arrival | The AI's suggestion, or 80.00 when it has none | | Model | Sonnet 5, in an env var | ## The invariant -**Nothing reaches the storefront at a price a model guessed.** Everything below follows from that: the suggested price lives on the draft and never on the item, the email has no publish button, and the publish path refuses an item with no price. +**Nothing reaches the storefront without the admin publishing it from the queue.** The item arrives `pending`, which is already invisible to every public query, and only `mark-available` moves it. The email carries no publish button, so the admin has necessarily seen the item and its price before anything ships. + +The price is the deliberate exception, and it is worth being precise about what was traded. An arriving item is priced immediately — the model's suggestion if it made one, otherwise 80.00 — so the review queue's price field is pre-filled rather than blank. The admin can change it, but is not forced to, and an unchanged field publishes at a number the admin did not choose. + +That is a real risk and it is accepted knowingly. It is bounded by the queue: publishing is a deliberate act on a screen showing the price, not something that can happen from an inbox or by a timeout. What is given up is the stronger guarantee that a machine-guessed price could never reach the storefront at all. + +Two consequences follow, and both are load-bearing: + +- **80.00 is a plausible price, not an obvious sentinel.** `0` would render as "$0.00" and read as a bug to anyone who saw it; 80.00 renders as a decision. A default that went unnoticed therefore sells the item rather than announcing itself. This is the argument for surfacing it loudly in the queue rather than for choosing a different number. +- **The queue must make the price's provenance visible.** A price field is not enough. The queue shows whether the number came from the model, from the 80.00 default, or from the admin, so "nobody has looked at this price" is legible at a glance instead of being indistinguishable from a considered one. ## Architecture @@ -75,8 +85,17 @@ CREATE TABLE item_drafts ( ai_description TEXT, ai_category_id INTEGER REFERENCES categories(id) ON DELETE SET NULL, ai_tag_names TEXT[], - -- Deliberately not on items. See the invariant. + -- Kept even though the suggestion is also copied onto the item, so what the + -- model proposed stays readable after the admin has edited the item's price. + -- Without it there is no way to ask later whether the model's numbers were + -- any good. ai_suggested_price_cents INTEGER, + -- ai | default | admin. What the item's current price actually came from. + -- Derivable by comparing three numbers, but only fragilely: a model that + -- happens to suggest exactly 8000, or an admin who deliberately types the + -- model's number, both collapse the comparison. Recorded rather than + -- inferred, because the queue uses it to say "nobody has chosen this price". + price_source TEXT NOT NULL DEFAULT 'default', ai_error TEXT, input_tokens INTEGER, output_tokens INTEGER, @@ -86,14 +105,19 @@ CREATE TABLE item_drafts ( created_at TIMESTAMPTZ NOT NULL DEFAULT now() ); -ALTER TABLE items ALTER COLUMN price_cents DROP NOT NULL; +-- An arriving item is always priced, so the column stays NOT NULL and only +-- gains a fallback. 80.00 applies when the model declined to suggest anything; +-- a suggestion, when there is one, is written over it by the worker. +ALTER TABLE items ALTER COLUMN price_cents SET DEFAULT 8000; ``` -**On making `price_cents` nullable.** This is the most invasive line in the design: `price_cents` is referenced across fifteen files including the cart and the checkout. The alternative is to insert a submission at `0` and guard publishing on `price_cents > 0`, which changes no types and no other file. +**On `price_cents` staying NOT NULL.** The alternative considered was making it nullable so that "no price yet" was expressible, with the publish path refusing an unpriced item. That was rejected in favour of always having a price. -It is still the wrong choice. Zero is a real number that formats as a real price, and a sentinel that renders as "$0.00" in a shop is precisely the failure this codebase already has a scar from — `DEMO_MODE` meaning "demo unless the value is exactly false" quietly stopped the shop charging anyone. Null cannot be formatted by accident. And the blast radius is not hidden work but the opposite: making the column nullable makes the TypeScript type `number | null`, and the compiler then names every one of those fifteen sites and refuses to build until each has said what it does with an unpriced item. A sentinel gets the same reach with none of the enumeration. +The practical effect is that this design costs far less to build. `price_cents` is referenced across fifteen files including the cart and the checkout, and every one of them keeps working untouched — the column's type does not change, so nothing downstream has to learn about an item that has no price. The migration adds a default and nothing else. -The publish guard belongs in `mark-available` alongside the existing status checks, and is worth an integration test of its own. +What it costs is covered under the invariant above: the storefront can now be reached by a price the admin never chose, so the protection moves from the schema into the queue's presentation, where it is weaker. `price_source` exists to make that presentation possible. + +The number lives in the migration rather than in configuration. Changing a default price is a rare, deliberate act with a record attached, which is the right amount of friction; an env var would let it drift silently between environments, and a wrong default is not visible anywhere until something has already sold at it. ### Ingestion @@ -101,7 +125,7 @@ The publish guard belongs in `mark-available` alongside the existing status chec 1. Hash the token, look up a matching `upload_links` row that is not revoked. No match, revoked, or over its cap: `404`. Not `403` — whether a link exists is not something a stranger needs to be able to distinguish, which is the reasoning `uploads.ts` already applies. 2. Validate the photos through the existing `uploadTypes` checks. Caps: at most 10 photos, at most 10 MB each. -3. In one transaction: insert the `items` row (`status='pending'`, `price_cents=NULL`, a placeholder name — the submission timestamp — replaced by the draft or the admin), insert `item_images`, insert `item_drafts` at `state='queued'`, bump the link's counter. +3. In one transaction: insert the `items` row (`status='pending'`, no `price_cents` given so the 8000 default applies, a placeholder name — the submission timestamp — replaced by the draft or the admin), insert `item_images`, insert `item_drafts` at `state='queued'` and `price_source='default'`, bump the link's counter. 4. Respond immediately. The uploader is told it arrived and that a person will look at it. **The AI is not called here** — a slow or failing API call must not turn into a failed upload for someone who did nothing wrong. Rate limited by IP through a new limiter in `rateLimit.ts`. `keyByCallerAndEmail` does not fit — there is no email — so this needs its own key function over `ipKeyGenerator` alone, and the existing comment about a bare `ip:` bucket being a shared allowance applies and is accepted here: the link itself is the per-caller identity, and its counter is the per-caller cap. @@ -119,7 +143,9 @@ The request, via `@anthropic-ai/sdk` and `client.messages.parse()` with `zodOutp The note is untrusted input from an unauthenticated stranger. It is passed as data, and nothing the model returns is executed, interpolated into SQL, or rendered as HTML — the drafted description goes through the same markdown-with-`html: false` treatment as every other stored body, which is what makes "a model wrote this" and "a person wrote this" equally safe to render. -Result: `state='ready'`, the fields populated, token counts and computed cost recorded. Failure: `attempts` incremented, `ai_error` stored, and after three attempts `state='failed'` — which still leaves a perfectly good pending item with photos in the review queue, just with no draft copy. A failed AI call must never lose someone's submission. +Result: `state='ready'`, the fields populated, token counts and computed cost recorded. Where the model returned a price, it is written onto the item and `price_source='ai'`; where it did not, the item keeps the 8000 default and `price_source` stays `'default'`. The suggested price is recorded on the draft either way. + +Failure: `attempts` incremented, `ai_error` stored, and after three attempts `state='failed'` — which still leaves a perfectly good pending item with photos in the review queue, just with no draft copy and priced at the default. A failed AI call must never lose someone's submission. **Spend ceiling.** A monthly cap (`INTAKE_MONTHLY_BUDGET_USD`) summed from `cost_micros`. Past it, submissions still save and still notify; only the API call is skipped, with `state='failed'` and an error saying so. The ceiling protects the bill, and it must not be the thing that loses inventory. @@ -137,7 +163,9 @@ The two signed actions are deliberately the ones whose worst case is a wasted AP ### The review queue -A new admin screen listing drafts by state: photos, the submitter's note, which link it came from, the drafted copy in editable fields, the suggested price shown as a suggestion beside an empty price input, and Publish / Regenerate / Discard. Publish writes the edited values onto the item and calls the existing `mark-available`. +A new admin screen listing drafts by state: photos, the submitter's note, which link it came from, the drafted copy in editable fields, the price, and Publish / Regenerate / Discard. Publish writes the edited values onto the item and calls the existing `mark-available`. + +The price field carries the weight the schema no longer does, so it is not an ordinary input. It is pre-filled, and it is labelled with where the number came from — the model, or the 80.00 default, or the admin — with anything that is not `price_source='admin'` marked visibly as unconfirmed. Editing it sets `price_source='admin'`. Publishing something still marked unconfirmed is allowed, because that is the decision taken, but it says so plainly at the point of publishing rather than after. Reuses the existing admin item components where they fit rather than growing a second item editor. @@ -160,8 +188,8 @@ The through-line: **a submission is the only irreplaceable thing here.** Photos | Failure | Behaviour | | --- | --- | -| API call fails or times out | Retry to three attempts, then `state='failed'`. Item and photos intact, reviewable with no draft | -| Budget exhausted | Submission saved, draft skipped, admin still notified | +| API call fails or times out | Retry to three attempts, then `state='failed'`. Item and photos intact, reviewable with no draft, priced at the default | +| Budget exhausted | Submission saved, draft skipped, admin still notified, item priced at the default | | Malformed model output | Rejected by the schema, counts as a failed attempt | | SMTP down | Draft still `ready` and visible in the queue; the queue, not the email, is the source of truth | | Link revoked mid-upload | `404`. Already-submitted items are unaffected | @@ -169,8 +197,8 @@ The through-line: **a submission is the only irreplaceable thing here.** Photos ## Testing -- **Unit** — token hashing and constant-time verification, HMAC signing and expiry, the budget calculation, the prompt builder's handling of an empty note, and the publish guard's price rule. All pure, in the style of `keyByCallerAndEmail` and `isAllowedRecipient` being exported specifically to be tested directly. -- **Integration** — submission through a valid link creates a pending item with images and a queued draft; a revoked link gets `404`; publishing without a price is refused; a signed action link works once and an expired or tampered one does not. The Anthropic client is stubbed; no test spends money. +- **Unit** — token hashing and constant-time verification, HMAC signing and expiry, the budget calculation, the prompt builder's handling of an empty note, and the `price_source` transitions. All pure, in the style of `keyByCallerAndEmail` and `isAllowedRecipient` being exported specifically to be tested directly. +- **Integration** — submission through a valid link creates a pending item with images, a queued draft and the 8000 default; a model suggestion overwrites that price and sets `price_source='ai'`; a failed or budget-skipped draft leaves the default in place; a revoked link gets `404`; a signed action link works once and an expired or tampered one does not. The Anthropic client is stubbed; no test spends money. - **E2E** — the submission page, and the queue round trip from draft to published. Assertions scoped to the item under test rather than the whole grid, since the dev database never truncates. ## Out of scope -- 2.54.0 From 783d10abc4af017f04cce2295ee6c87512446d3a Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Sat, 29 Aug 2026 08:09:39 -0500 Subject: [PATCH 3/5] docs(intake): plan the upload-link and submission-page slice (#220) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first of four slices from the intake design, and the only one that is worth planning in detail yet — the later slices' shape depends on what this one actually produces. Seven tasks: the schema, extracting the validated image-upload pipeline out of routes/admin.ts so the public endpoint reuses it rather than growing a near-copy of it, token generation and hashing, the admin API for issuing and revoking links, the public submission endpoint, the submission page, and the admin screen. Three things the plan settles that the design left open or got wrong. The image caps become the constants already in the codebase rather than the 10-photo and 10 MB figures the design invented, because two different caps on one pipeline is a defect waiting to happen. The feature flag is dropped from this slice: nothing is reachable until a link exists, and the flag earns its keep in slice 2 where a paid API call appears. And the link is resolved before multer runs, so a stranger holding a bad token cannot cause a byte to be written to the uploads volume — cleanup afterwards would leave an unauthenticated caller in control of disk churn, and leans on an unlink that a crash between write and delete would skip. That ordering is asserted by a test, so a later reordering fails loudly rather than silently. Ref #220 --- .../plans/2026-08-29-intake-upload-links.md | 1514 +++++++++++++++++ 1 file changed, 1514 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-29-intake-upload-links.md diff --git a/docs/superpowers/plans/2026-08-29-intake-upload-links.md b/docs/superpowers/plans/2026-08-29-intake-upload-links.md new file mode 100644 index 0000000..cbb7d74 --- /dev/null +++ b/docs/superpowers/plans/2026-08-29-intake-upload-links.md @@ -0,0 +1,1514 @@ +# Intake — Upload Links and Submission Page 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:** Someone holding a named, revocable link can send in photos of one item plus a note, and it arrives in the admin inventory as a pending item. + +**Architecture:** A new `upload_links` table holds hashed tokens the admin issues and revokes. A public, rate-limited endpoint accepts a multipart submission through the *existing* validated image-upload pipeline — extracted from `routes/admin.ts` into a shared module rather than duplicated — and writes an `items` row at `status='pending'` with its images and an `item_drafts` row carrying the note and provenance. No AI and no email in this slice: the submission lands in the inventory screen that already exists. + +**Tech Stack:** Express 4 + TypeScript, Postgres via `pg`, `node-pg-migrate`, multer, `express-rate-limit`, Jest + supertest (backend), React + antd + Vite, Vitest + Playwright (frontend). + +**Spec:** `docs/superpowers/specs/2026-08-29-intake-pipeline-design.md` + +## Global Constraints + +- **Node 20+ required** for any `npm run migrate:*` command. `node-pg-migrate` pulls an `lru-cache` calling `diagnostics_channel.tracingChannel()`, absent before Node 19.9; on Node 18 it dies inside minified library code. Use `scripts/start-local.ps1`, which handles the version switch. +- **Every route handler must be wrapped in `asyncRoute`.** `tests/unit/routesAreWrapped.test.ts` enforces this by scanning source and will fail the build otherwise. Express 4 does not forward rejected promises; an unwrapped async handler hangs the request forever. +- **antd imports are deep ESM paths** — `import Button from 'antd/es/button'`. Never `import { Button } from 'antd'`. Note this repo uses `antd/es/`, not `antd/lib/`. +- **Uploaded files must go through the existing validation** — allowlist of `image/jpeg`, `image/png`, `image/webp`; magic-byte check after write; stored filename from `randomUUID()` plus an extension derived from the validated type, never from `originalname`. +- **Image caps are the existing constants**, not new ones: `MAX_IMAGES_PER_REQUEST = 6`, `MAX_IMAGE_BYTES = 8_000_000`. (The spec proposed 10 and 10 MB; those were invented numbers and are superseded by the values already in the codebase. One set of caps, not two.) +- **SQL uses bound parameters.** Never format a caller-supplied value into a query string. +- **Commit style:** Conventional Commits, subject ending `(#NNN)` with the sub-issue number, no hard wrapping in bodies. + +## Deviations from the spec, and why + +| Spec says | This plan does | Why | +| --- | --- | --- | +| Caps of 10 photos / 10 MB | Reuses existing 6 / 8 MB constants | The codebase already defines these for the admin upload path. Two different caps for the same pipeline is a bug waiting to happen | +| "Three migrations" | One migration file | All three DDL changes ship together and a partial application is meaningless. node-pg-migrate runs a file in one transaction | +| Intake feature-flagged off | No flag in this slice | Nothing is reachable until the admin creates a link, and an absent or revoked token 404s. The flag earns its keep in slice 2, where a paid API call appears | + +## File Structure + +**Created:** +- `backend/migrations/1787500000000_add-intake-pipeline.js` — `upload_links`, `item_drafts`, and the `items.price_cents` default +- `backend/src/imageUpload.ts` — the shared multer pipeline, extracted from `routes/admin.ts` +- `backend/src/uploadLinks.ts` — token generation and hashing, pure +- `backend/src/routes/adminUploadLinks.ts` — admin CRUD and revoke +- `backend/src/routes/intake.ts` — the public submission endpoint +- `backend/tests/unit/uploadLinks.test.ts` +- `backend/tests/integration/uploadLinks.integration.test.ts` +- `backend/tests/integration/intake.integration.test.ts` +- `frontend/src/intake/Submit.tsx` — the public submission page +- `frontend/src/intake/intakeApi.ts` +- `frontend/src/admin/UploadLinks.tsx` — the admin link-management screen + +**Modified:** +- `backend/src/routes/admin.ts` — the upload pipeline moves out; imports it back in +- `backend/src/rateLimit.ts` — a limiter keyed on caller alone +- `backend/src/app.ts` — mounts the two new routers +- `backend/tests/integration/setup/testDb.ts` — new tables in the truncate list +- `frontend/src/main.tsx` — the `/submit/:token` route +- `frontend/src/admin/Admin.tsx` — a tab for upload links + +--- + +### Task 1: Schema + +**Files:** +- Create: `backend/migrations/1787500000000_add-intake-pipeline.js` +- Modify: `backend/tests/integration/setup/testDb.ts:31-39` + +**Interfaces:** +- Consumes: nothing +- Produces: tables `upload_links` and `item_drafts`; `items.price_cents` defaults to `8000` + +- [ ] **Step 1: Write the migration** + +Create `backend/migrations/1787500000000_add-intake-pipeline.js`: + +```js +exports.up = (pgm) => { + pgm.sql(` + CREATE TABLE IF NOT EXISTS upload_links ( + id SERIAL PRIMARY KEY, + label TEXT NOT NULL, + -- The token itself is never stored, only its digest. A leaked database + -- is then not also a leaked set of working upload links, and the admin + -- screen can show a token exactly once — at creation — for the same + -- reason a password reset link is not re-readable. + token_hash TEXT NOT NULL UNIQUE, + revoked_at TIMESTAMPTZ, + submission_count INTEGER NOT NULL DEFAULT 0, + -- Null means no cap. A link handed to a regular contributor is + -- open-ended; one handed out for a single box of stock is not. + max_submissions INTEGER, + last_used_at TIMESTAMPTZ, + created_at TIMESTAMPTZ NOT NULL DEFAULT now() + ); + + CREATE TABLE IF NOT EXISTS item_drafts ( + id SERIAL PRIMARY KEY, + item_id INTEGER NOT NULL UNIQUE REFERENCES items(id) ON DELETE CASCADE, + -- SET NULL rather than CASCADE: deleting a link must not delete the + -- items that arrived through it. Provenance is lost; the goods are not. + upload_link_id INTEGER REFERENCES upload_links(id) ON DELETE SET NULL, + submitter_note TEXT, + state TEXT NOT NULL DEFAULT 'queued', + attempts INTEGER NOT NULL DEFAULT 0, + model TEXT, + ai_name TEXT, + ai_description TEXT, + ai_category_id INTEGER REFERENCES categories(id) ON DELETE SET NULL, + ai_tag_names TEXT[], + ai_suggested_price_cents INTEGER, + -- ai | default | admin. Where the item's current price came from. + -- Recorded rather than inferred: a model that happens to suggest exactly + -- 8000, or an admin who deliberately types the model's number, both + -- collapse any comparison-based guess. + price_source TEXT NOT NULL DEFAULT 'default', + ai_error TEXT, + input_tokens INTEGER, + output_tokens INTEGER, + cost_micros INTEGER, + drafted_at TIMESTAMPTZ, + reviewed_at TIMESTAMPTZ, + created_at TIMESTAMPTZ NOT NULL DEFAULT now() + ); + + -- The queue reads by state, and everything else reads by item. + CREATE INDEX IF NOT EXISTS item_drafts_state_idx ON item_drafts (state); + + -- A submitted item is priced on arrival rather than left unpriced, so the + -- column keeps NOT NULL and only gains a fallback. The AI columns above + -- stay null until the drafting worker exists. + ALTER TABLE items ALTER COLUMN price_cents SET DEFAULT 8000; + `); +}; + +exports.down = (pgm) => { + pgm.sql(` + ALTER TABLE items ALTER COLUMN price_cents DROP DEFAULT; + DROP TABLE IF EXISTS item_drafts; + DROP TABLE IF EXISTS upload_links; + `); +}; +``` + +- [ ] **Step 2: Add the new tables to the integration truncate list** + +In `backend/tests/integration/setup/testDb.ts`, the `TRUNCATE` in `resetDb()` becomes — note `item_drafts` and `upload_links` lead, since `item_drafts` references `items`: + +```ts + await testPool.query(` + TRUNCATE TABLE item_drafts, upload_links, orders, checkout_items, checkouts, shipping_addresses, + cart_items, carts, customer_tokens, customer_sessions, favorites, customers, item_tags, + item_images, items, tags, categories + RESTART IDENTITY CASCADE + `); +``` + +- [ ] **Step 3: Run the migration up and back down** + +```bash +cd backend +npm run migrate:up +npm run migrate:down +npm run migrate:up +``` + +Expected: three clean runs. The down must succeed — an irreversible migration is a migration that cannot be tested. + +- [ ] **Step 4: Confirm the existing suite still passes** + +```bash +cd backend && npm run test:integration +``` + +Expected: PASS. The price default is additive, so nothing existing should shift. + +- [ ] **Step 5: Commit** + +```bash +git add backend/migrations/1787500000000_add-intake-pipeline.js backend/tests/integration/setup/testDb.ts +git commit -m "feat(intake): add upload_links and item_drafts, and default an item's price (#NNN)" +``` + +--- + +### Task 2: Extract the shared image-upload pipeline + +This is a pure refactor. No behaviour changes; the existing tests are the safety net. + +**Files:** +- Create: `backend/src/imageUpload.ts` +- Modify: `backend/src/routes/admin.ts:1-300` + +**Interfaces:** +- Consumes: `uploadTypes.ts` (`ALLOWED_IMAGE_TYPES`, `SIGNATURE_BYTES`, `extensionFor`, `isAllowedImageType`, `signatureMatches`) +- Produces: + - `uploadImages: (req, res, next) => void` — multer middleware, field name `images` + - `verifyUploadedImages(req: Request): Promise` — refusal message, or null + - `insertItemImages(client: PoolClient, itemId: number, files: Express.Multer.File[], firstSortOrder: number): Promise` + - `MAX_IMAGES_PER_REQUEST: number`, `MAX_IMAGE_BYTES: number` + +- [ ] **Step 1: Run the upload tests first, to know they pass before you touch anything** + +```bash +cd backend && npx jest -c jest.integration.config.js --runInBand uploadValidation adminInventory +``` + +Expected: PASS. Record this — it is the comparison for Step 4. + +- [ ] **Step 2: Move the code** + +Create `backend/src/imageUpload.ts` and move into it, unchanged, from `routes/admin.ts`: `UnsupportedImageTypeError`, `UPLOADS_DIR`, `MAX_IMAGES_PER_REQUEST`, `MAX_IMAGE_BYTES`, `MAX_TEXT_FIELDS`, `MAX_TEXT_FIELD_BYTES`, `storage`, `upload`, `readHead`, `discardUploads`, `discardUnlessAccepted`, `verifyUploadedImages`, `uploadImages`, and `insertItemImages`. + +Keep every existing comment verbatim. They record why the code is shaped as it is (#95, #103, #180) and are the most valuable thing being moved. + +Export `uploadImages`, `verifyUploadedImages`, `insertItemImages`, `MAX_IMAGES_PER_REQUEST`, `MAX_IMAGE_BYTES`. Everything else stays module-private. + +Add this at the top of the new file: + +```ts +/** + * The one validated path from a multipart request to files on the uploads + * volume. + * + * Extracted from routes/admin.ts when a second caller appeared (#220's public + * intake endpoint). It is deliberately one module rather than two similar ones: + * every property that makes uploads safe here — the type allowlist, the + * magic-byte check after the write, names from a CSPRNG rather than from + * `originalname`, and the cleanup of anything a failed request left behind — + * is a property a second implementation would have to reproduce exactly. A + * near-copy that drifted would be precisely the gap #95 and #103 exist to + * close. + */ +``` + +- [ ] **Step 3: Import them back into `routes/admin.ts`** + +```ts +import { + uploadImages, + verifyUploadedImages, + insertItemImages, + MAX_IMAGES_PER_REQUEST +} from '../imageUpload'; +``` + +Remove the now-unused imports from `../uploadTypes`, `multer`, `fs`, and `randomUUID` from `routes/admin.ts` — but only those genuinely no longer referenced. Let `npm run lint` tell you which. + +- [ ] **Step 4: Run the same tests and the whole suite** + +```bash +cd backend +npm run lint +npm run build +npx jest -c jest.integration.config.js --runInBand uploadValidation adminInventory +npm run test:unit +``` + +Expected: identical results to Step 1, plus a clean lint and build. `routesAreWrapped` must still pass. + +- [ ] **Step 5: Commit** + +```bash +git add backend/src/imageUpload.ts backend/src/routes/admin.ts +git commit -m "refactor(uploads): extract the validated image pipeline for a second caller (#NNN)" +``` + +--- + +### Task 3: Token generation and hashing + +**Files:** +- Create: `backend/src/uploadLinks.ts` +- Test: `backend/tests/unit/uploadLinks.test.ts` + +**Interfaces:** +- Consumes: nothing +- Produces: + - `generateToken(): string` — 43-character base64url, 256 bits of CSPRNG entropy + - `hashToken(token: string): string` — 64-character lowercase hex SHA-256 + +- [ ] **Step 1: Write the failing test** + +Create `backend/tests/unit/uploadLinks.test.ts`: + +```ts +import { generateToken, hashToken } from '../../src/uploadLinks'; + +describe('generateToken', () => { + it('produces a URL-safe token with no padding', () => { + expect(generateToken()).toMatch(/^[A-Za-z0-9_-]{43}$/); + }); + + // The token is the entire access control on the intake endpoint. If two + // calls could collide, one person's link would open another's. + it('does not repeat', () => { + const seen = new Set(Array.from({ length: 1000 }, () => generateToken())); + expect(seen.size).toBe(1000); + }); +}); + +describe('hashToken', () => { + it('is a lowercase hex sha256 digest', () => { + expect(hashToken('abc')).toBe( + 'ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad' + ); + }); + + it('is stable across calls, so a stored digest keeps matching', () => { + const token = generateToken(); + expect(hashToken(token)).toBe(hashToken(token)); + }); + + it('gives different tokens different digests', () => { + expect(hashToken(generateToken())).not.toBe(hashToken(generateToken())); + }); +}); +``` + +- [ ] **Step 2: Run it to verify it fails** + +```bash +cd backend && npx jest -c jest.unit.config.js uploadLinks +``` + +Expected: FAIL — `Cannot find module '../../src/uploadLinks'`. + +- [ ] **Step 3: Write the implementation** + +Create `backend/src/uploadLinks.ts`: + +```ts +import crypto from 'crypto'; + +/** + * Issuing and recognising the tokens that open the public intake endpoint. + * + * Kept apart from the routes so the rules are pure and testable directly, the + * same reasoning as `uploadTypes.ts` and `keyByCallerAndEmail`. + */ + +// 32 bytes — 256 bits. base64url so the value survives being pasted into a URL, +// a chat message and a QR code without escaping. +const TOKEN_BYTES = 32; + +export function generateToken(): string { + return crypto.randomBytes(TOKEN_BYTES).toString('base64url'); +} + +/** + * The digest stored against a link. + * + * SHA-256 rather than bcrypt, deliberately. A password hash is slow on purpose + * because a human password has little entropy and must survive an offline + * dictionary attack. This is 256 bits from a CSPRNG: there is no dictionary, + * and guessing is not a threat that slowing the hash addresses. Meanwhile the + * digest is computed on every submission request, so a deliberately slow hash + * would be a denial-of-service surface on an unauthenticated endpoint. + * + * No timing-safe comparison is needed here because the lookup is an indexed + * equality match on the digest, not a byte-by-byte compare of the secret — and + * an attacker who could mount a timing attack against a 256-bit random value + * would still need the value. + */ +export function hashToken(token: string): string { + return crypto.createHash('sha256').update(token).digest('hex'); +} +``` + +- [ ] **Step 4: Run the test to verify it passes** + +```bash +cd backend && npx jest -c jest.unit.config.js uploadLinks +``` + +Expected: PASS, 5 tests. + +- [ ] **Step 5: Commit** + +```bash +git add backend/src/uploadLinks.ts backend/tests/unit/uploadLinks.test.ts +git commit -m "feat(intake): generate and hash upload link tokens (#NNN)" +``` + +--- + +### Task 4: Admin API for issuing and revoking links + +**Files:** +- Create: `backend/src/routes/adminUploadLinks.ts` +- Modify: `backend/src/app.ts:74-80` +- Test: `backend/tests/integration/uploadLinks.integration.test.ts` + +**Interfaces:** +- Consumes: `generateToken`, `hashToken` from `src/uploadLinks`; `pool`, `requireRow` from `src/db`; `asyncRoute` +- Produces: + - `GET /api/admin/upload-links` → `UploadLinkRow[]`, never including a token + - `POST /api/admin/upload-links` `{label, maxSubmissions?}` → `201` with `{...row, token, url}` — the only time the token is ever returned + - `POST /api/admin/upload-links/:id/revoke` → `200` with the updated row + +- [ ] **Step 1: Write the failing test** + +Create `backend/tests/integration/uploadLinks.integration.test.ts`: + +```ts +import request from 'supertest'; +import app from '../../src/app'; +import { pool } from '../../src/db'; +import { resetDb, closeDb } from './setup/testDb'; + +beforeEach(async () => { + await resetDb(); +}); + +afterAll(async () => { + await pool.end(); + await closeDb(); +}); + +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' }); + + expect(created.status).toBe(201); + expect(created.body.label).toBe('Sarah'); + expect(created.body.token).toMatch(/^[A-Za-z0-9_-]{43}$/); + expect(created.body.url).toContain(`/submit/${created.body.token}`); + + const listed = await request(app).get('/api/admin/upload-links'); + expect(listed.status).toBe(200); + expect(listed.body).toHaveLength(1); + // The whole point of storing a digest: the listing cannot leak it back. + expect(listed.body[0].token).toBeUndefined(); + expect(listed.body[0].token_hash).toBeUndefined(); + }); + + 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' }); + + const { rows } = await pool.query(`SELECT token_hash FROM upload_links`); + expect(rows[0].token_hash).not.toBe(created.body.token); + expect(rows[0].token_hash).toMatch(/^[a-f0-9]{64}$/); + }); + + it('refuses a link with no label', async () => { + const res = await request(app).post('/api/admin/upload-links').send({ label: ' ' }); + expect(res.status).toBe(400); + }); + + it('refuses a non-positive submission cap', async () => { + const res = await request(app) + .post('/api/admin/upload-links') + .send({ label: 'Bad cap', maxSubmissions: 0 }); + expect(res.status).toBe(400); + }); +}); + +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' }); + + const revoked = await request(app) + .post(`/api/admin/upload-links/${created.body.id}/revoke`); + + expect(revoked.status).toBe(200); + expect(revoked.body.revoked_at).not.toBeNull(); + }); + + it('is idempotent, so a second click is not an error', async () => { + const created = await request(app) + .post('/api/admin/upload-links') + .send({ label: 'Temporary' }); + + 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`); + + expect(second.status).toBe(200); + }); + + it('404s for a link that does not exist', async () => { + const res = await request(app).post('/api/admin/upload-links/9999/revoke'); + expect(res.status).toBe(404); + }); +}); +``` + +- [ ] **Step 2: Run it to verify it fails** + +```bash +cd backend && npx jest -c jest.integration.config.js --runInBand uploadLinks.integration +``` + +Expected: FAIL — 404s, because the router is not mounted. + +- [ ] **Step 3: Write the router** + +Create `backend/src/routes/adminUploadLinks.ts`: + +```ts +import { Router, Request, Response } from 'express'; +import { pool, requireRow } from '../db'; +import { asyncRoute } from '../asyncRoute'; +import { generateToken, hashToken } from '../uploadLinks'; + +const router = Router(); + +/** + * Issuing and retiring the links that open the public intake endpoint. + * + * A link is named because provenance matters more than convenience here: when + * one leaks, the question is which one, and the answer has to come from + * somewhere. The token is returned by exactly one response in this file and is + * unrecoverable afterwards, which is why the admin screen has to present it as + * a one-time reveal rather than a field to come back to. + */ + +/** Shaped so a `SELECT *` can never leak the digest into a response. */ +const LINK_SELECT = ` + SELECT id, label, revoked_at, submission_count, max_submissions, last_used_at, created_at + FROM upload_links +`; + +interface UploadLinkRow { + id: number; + label: string; + revoked_at: string | null; + submission_count: number; + max_submissions: number | null; + last_used_at: string | null; + created_at: string; +} + +router.get('/', asyncRoute(async (_req: Request, res: Response) => { + const { rows } = await pool.query(`${LINK_SELECT} ORDER BY created_at DESC`); + res.json(rows); +})); + +router.post('/', asyncRoute(async (req: Request, res: Response) => { + const label = typeof req.body?.label === 'string' ? req.body.label.trim() : ''; + if (label === '') { + return res.status(400).json({ error: 'a label is required' }); + } + + // Absent means no cap, which is a different thing from a cap of zero — a + // link that can never be used is a mistake rather than an intent. + const rawCap = req.body?.maxSubmissions; + let maxSubmissions: number | null = null; + if (rawCap !== undefined && rawCap !== null && rawCap !== '') { + const parsed = Number(rawCap); + if (!Number.isInteger(parsed) || parsed < 1) { + return res.status(400).json({ error: 'maxSubmissions must be a positive whole number' }); + } + maxSubmissions = parsed; + } + + const token = generateToken(); + const { rows } = await pool.query( + `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] + ); + const link = requireRow(rows, 'the upload_links INSERT'); + + // PUBLIC_URL is already required alongside SMTP and is what every other + // outbound link is built from. Empty in a local environment, which yields a + // relative URL the admin screen can still render usefully. + const base = (process.env.PUBLIC_URL ?? '').replace(/\/+$/, ''); + res.status(201).json({ ...link, token, url: `${base}/submit/${token}` }); +})); + +router.post('/:id/revoke', asyncRoute(async (req: Request, res: Response) => { + // COALESCE so revoking twice keeps the original timestamp: the useful fact is + // when access ended, and a second click should not rewrite that. + const { rows } = await pool.query( + `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`, + [req.params.id] + ); + + if (rows.length === 0) { + return res.status(404).json({ error: 'not found' }); + } + res.json(rows[0]); +})); + +export default router; +``` + +- [ ] **Step 4: Mount it** + +In `backend/src/app.ts`, beside the other admin routers and **above** the catch-all `app.use('/api/admin', ...)`: + +```ts +import adminUploadLinksRouter from './routes/adminUploadLinks'; +``` + +```ts +app.use('/api/admin/upload-links', requireAdminGate, adminUploadLinksRouter); +``` + +Order matters: `/api/admin` is mounted last and would otherwise swallow the path. `requireAdminGate` goes on the router itself, per the reasoning in `middleware/adminGate.ts`. + +- [ ] **Step 5: Run the tests** + +```bash +cd backend +npx jest -c jest.integration.config.js --runInBand uploadLinks.integration +npm run test:unit +npm run lint && npm run build +``` + +Expected: PASS, 7 integration tests. `routesAreWrapped` must pass — every handler above is wrapped. + +- [ ] **Step 6: Commit** + +```bash +git add backend/src/routes/adminUploadLinks.ts backend/src/app.ts backend/tests/integration/uploadLinks.integration.test.ts +git commit -m "feat(intake): issue and revoke named upload links (#NNN)" +``` + +--- + +### Task 5: The public submission endpoint + +**Files:** +- Create: `backend/src/routes/intake.ts` +- Modify: `backend/src/rateLimit.ts` (append), `backend/src/app.ts` +- Test: `backend/tests/integration/intake.integration.test.ts` + +**Interfaces:** +- Consumes: `uploadImages`, `verifyUploadedImages`, `insertItemImages` from `src/imageUpload`; `hashToken` from `src/uploadLinks`; `intakeLimiter` from `src/rateLimit` +- Produces: + - `GET /api/intake/:token` → `{label}` for a usable link, else `404` + - `POST /api/intake/:token` (multipart: `images[]`, `note`) → `201 {ok: true}`, else `404` / `400` + - `keyByCaller(req: Request): string` exported from `rateLimit.ts` + +- [ ] **Step 1: Write the failing test** + +Create `backend/tests/integration/intake.integration.test.ts`: + +```ts +import request from 'supertest'; +import { promises as fs } from 'fs'; +import app from '../../src/app'; +import { pool } from '../../src/db'; +import { resetDb, closeDb } from './setup/testDb'; + +const UPLOADS_DIR = process.env.UPLOADS_DIR as string; + +// The same 1x1 PNG the upload validation suite uses, so the accepted case +// exercises the whole path rather than a buffer that merely starts right. +const PNG = Buffer.from( + 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==', + 'base64' +); + +// multer.diskStorage does not create its destination. +beforeAll(async () => { + await fs.mkdir(UPLOADS_DIR, { recursive: true }); +}); + +beforeEach(async () => { + await resetDb(); +}); + +afterAll(async () => { + await pool.end(); + await closeDb(); +}); + +async function storedFiles(): Promise { + return fs.readdir(UPLOADS_DIR); +} + +async function issueLink(label = 'Sarah', maxSubmissions?: number): Promise { + const res = await request(app) + .post('/api/admin/upload-links') + .send({ label, ...(maxSubmissions === undefined ? {} : { maxSubmissions }) }); + expect(res.status).toBe(201); + return res.body.token; +} + +describe('checking a link before showing the form', () => { + it('names the link so the page can greet the sender', async () => { + const token = await issueLink('Sarah'); + const res = await request(app).get(`/api/intake/${token}`); + + expect(res.status).toBe(200); + expect(res.body.label).toBe('Sarah'); + }); + + // 404 rather than 403 throughout: whether a link exists is not something a + // stranger needs to be able to distinguish. Same reasoning as uploads.ts. + it('404s an unknown token', async () => { + const res = await request(app).get('/api/intake/not-a-real-token'); + expect(res.status).toBe(404); + }); + + it('404s a revoked link', async () => { + const token = await issueLink(); + const { rows } = await pool.query(`SELECT id FROM upload_links`); + await request(app).post(`/api/admin/upload-links/${rows[0].id}/revoke`); + + const res = await request(app).get(`/api/intake/${token}`); + expect(res.status).toBe(404); + }); +}); + +describe('submitting an item', () => { + it('creates a pending item with its images, note and provenance', async () => { + const token = await issueLink('Sarah'); + + const res = await request(app) + .post(`/api/intake/${token}`) + .field('note', 'Hand-thrown stoneware, chip on the base') + .attach('images', PNG, 'front.png') + .attach('images', PNG, 'back.png'); + + expect(res.status).toBe(201); + expect(res.body.ok).toBe(true); + + const { rows: items } = await pool.query( + `SELECT id, status, price_cents FROM items` + ); + expect(items).toHaveLength(1); + expect(items[0].status).toBe('pending'); + // The default from Task 1, not a price anyone chose. + expect(items[0].price_cents).toBe(8000); + + const { rows: images } = await pool.query( + `SELECT image_path FROM item_images WHERE item_id = $1 ORDER BY sort_order`, + [items[0].id] + ); + expect(images).toHaveLength(2); + expect(images[0].image_path).toMatch(/^\/uploads\/[a-f0-9-]+\.png$/); + + const { rows: drafts } = await pool.query( + `SELECT submitter_note, state, price_source, upload_link_id FROM item_drafts WHERE item_id = $1`, + [items[0].id] + ); + expect(drafts[0].submitter_note).toBe('Hand-thrown stoneware, chip on the base'); + expect(drafts[0].state).toBe('queued'); + expect(drafts[0].price_source).toBe('default'); + expect(drafts[0].upload_link_id).not.toBeNull(); + }); + + it('counts the submission against the link', async () => { + const token = await issueLink(); + await request(app).post(`/api/intake/${token}`).attach('images', PNG, 'a.png'); + + const { rows } = await pool.query(`SELECT submission_count, last_used_at FROM upload_links`); + expect(rows[0].submission_count).toBe(1); + expect(rows[0].last_used_at).not.toBeNull(); + }); + + it('refuses a submission with no photos', async () => { + const token = await issueLink(); + const res = await request(app).post(`/api/intake/${token}`).field('note', 'nothing attached'); + + expect(res.status).toBe(400); + const { rows } = await pool.query(`SELECT id FROM items`); + expect(rows).toHaveLength(0); + }); + + // The file is named .png and declared image/png, but the bytes are not. + // This is the check that cannot happen before the write. + it('refuses a file whose bytes disagree with its type', async () => { + const token = await issueLink(); + const res = await request(app) + .post(`/api/intake/${token}`) + .attach('images', Buffer.from('not an image'), { + filename: 'evil.png', + contentType: 'image/png' + }); + + expect(res.status).toBe(400); + const { rows } = await pool.query(`SELECT id FROM items`); + expect(rows).toHaveLength(0); + }); + + it('404s a revoked link without creating anything', async () => { + const token = await issueLink(); + const { rows: links } = await pool.query(`SELECT id FROM upload_links`); + await request(app).post(`/api/admin/upload-links/${links[0].id}/revoke`); + + const res = await request(app).post(`/api/intake/${token}`).attach('images', PNG, 'a.png'); + + expect(res.status).toBe(404); + const { rows } = await pool.query(`SELECT id FROM items`); + expect(rows).toHaveLength(0); + }); + + // The reason `requireUsableLink` is ordered ahead of `uploadImages`. Without + // that ordering this still returns 404 and still creates no item — the bytes + // just reach the disk first and are deleted afterwards. This asserts they + // never arrive, so a future reordering of the middleware fails here rather + // than quietly handing an unauthenticated caller control of disk churn. + it('writes nothing to the uploads volume for a token that does not work', async () => { + const before = await storedFiles(); + + const res = await request(app) + .post('/api/intake/not-a-real-token') + .attach('images', PNG, 'a.png'); + + expect(res.status).toBe(404); + expect(await storedFiles()).toEqual(before); + }); + + it('stops accepting once the link hits its cap', async () => { + const token = await issueLink('One shot', 1); + + const first = await request(app).post(`/api/intake/${token}`).attach('images', PNG, 'a.png'); + expect(first.status).toBe(201); + + const second = await request(app).post(`/api/intake/${token}`).attach('images', PNG, 'b.png'); + expect(second.status).toBe(404); + + const { rows } = await pool.query(`SELECT id FROM items`); + expect(rows).toHaveLength(1); + }); +}); + +describe('a submitted item does not reach the storefront', () => { + it('is absent from the public catalogue', async () => { + const token = await issueLink(); + await request(app).post(`/api/intake/${token}`).attach('images', PNG, 'a.png'); + + const res = await request(app).get('/api/items'); + expect(res.status).toBe(200); + expect(res.body).toHaveLength(0); + }); +}); +``` + +- [ ] **Step 2: Run it to verify it fails** + +```bash +cd backend && npx jest -c jest.integration.config.js --runInBand intake.integration +``` + +Expected: FAIL — 404s on every intake path, since the router is not mounted. + +- [ ] **Step 3: Add the rate limiter** + +Append to `backend/src/rateLimit.ts`: + +```ts +/** + * Keyed on the caller alone, because an intake submission carries no email. + * + * The comment on `keyByCallerAndEmail` warns that a bare `ip:` bucket is a + * shared allowance rather than a per-caller one, and that is accepted here: the + * link is the per-caller identity, and its `submission_count` and + * `max_submissions` are the per-caller cap. This limiter exists for a different + * job — bounding what a single address can throw at an unauthenticated endpoint + * that writes files to disk. + */ +export function keyByCaller(req: Request): string { + return ipKeyGenerator(req.ip ?? ''); +} + +// Deliberately looser than the password-reset allowance. Someone photographing +// a box of stock legitimately submits several items in a row, and the cost of +// refusing them is a lost consignment. +export const intakeLimiter = rateLimit({ + windowMs: 15 * 60 * 1000, + max: 20, + keyGenerator: keyByCaller, + standardHeaders: true, + legacyHeaders: false, + message: { error: 'too many submissions — please try again later' } +}); +``` + +- [ ] **Step 4: Write the router** + +Create `backend/src/routes/intake.ts`: + +```ts +import { Router, Request, Response, NextFunction } from 'express'; +import { pool, requireRow } from '../db'; +import { asyncRoute } from '../asyncRoute'; +import { hashToken } from '../uploadLinks'; +import { uploadImages, verifyUploadedImages, insertItemImages } from '../imageUpload'; +import { intakeLimiter } from '../rateLimit'; + +const router = Router(); + +/** + * The public way in: photos of one item, from someone with no account. + * + * Everything here is reachable by a stranger holding a URL, so the shape of + * every refusal matters. Refusals are 404 rather than 403 throughout — + * unknown, revoked and exhausted links are indistinguishable from the outside, + * because whether a link exists is not something a stranger needs to be able + * to learn. That is the same reasoning `uploads.ts` applies to files. + * + * The AI is deliberately not called here. A slow or failing model request must + * not turn into a failed upload for someone who did nothing wrong, and the + * photos may be the only copy — the item is often no longer in the sender's + * hands. The row is left at `state='queued'` for the worker (#220, slice 2). + */ + +interface LinkRow { + id: number; + label: string; +} + +/** The link resolved by `requireUsableLink`, carried to the handler. */ +interface IntakeRequest extends Request { + uploadLink?: LinkRow; +} + +/** + * The link a token opens, or null. + * + * The cap is applied in SQL rather than in a later branch so that "usable" is + * one concept with one definition, used identically by the GET and the POST. + */ +async function usableLink(token: string): Promise { + const { rows } = await pool.query( + `SELECT id, label FROM upload_links + WHERE token_hash = $1 + AND revoked_at IS NULL + AND (max_submissions IS NULL OR submission_count < max_submissions)`, + [hashToken(token)] + ); + return rows[0] ?? null; +} + +router.get('/:token', intakeLimiter, asyncRoute(async (req: Request, res: Response) => { + const link = await usableLink(req.params.token); + if (!link) { + return res.status(404).json({ error: 'not found' }); + } + // The label only. Nothing about the catalogue, the admin, or other links. + res.json({ label: link.label }); +})); + +/** + * Resolves the link *before* multer runs, so a stranger holding a bad token + * cannot cause a single byte to be written to the uploads volume. + * + * `discardUnlessAccepted` would delete those files afterwards, but "written + * then deleted" is a materially worse position than "never written" on an + * endpoint the whole internet can reach: it is disk churn an unauthenticated + * caller controls, and it leans on a cleanup that a crash between the write + * and the unlink would skip. Ordering this middleware ahead of `uploadImages` + * is the whole mitigation. + */ +const requireUsableLink = asyncRoute(async (req: Request, res: Response, next: NextFunction) => { + const link = await usableLink(req.params.token); + if (!link) { + res.status(404).json({ error: 'not found' }); + return; + } + (req as IntakeRequest).uploadLink = link; + next(); +}); + +router.post('/:token', intakeLimiter, requireUsableLink, uploadImages, asyncRoute(async (req: Request, res: Response) => { + // Set by requireUsableLink above. Re-checked rather than asserted non-null, + // so a future reordering of the middleware fails as a 404 rather than as a + // crash on undefined. + const link = (req as IntakeRequest).uploadLink; + if (!link) { + return res.status(404).json({ error: 'not found' }); + } + + const files = (req.files as Express.Multer.File[]) || []; + if (files.length === 0) { + return res.status(400).json({ error: 'at least one photo is required' }); + } + + const refusal = await verifyUploadedImages(req); + if (refusal) { + return res.status(400).json({ error: refusal }); + } + + const note = typeof req.body?.note === 'string' ? req.body.note.trim() : ''; + + const client = await pool.connect(); + try { + await client.query('BEGIN'); + + // A placeholder name. `items.name` is NOT NULL and nobody has named this + // yet — the drafting worker or the admin replaces it. A timestamp is used + // rather than "Untitled" so several waiting submissions are still tellable + // apart in the inventory list. + const { rows } = await client.query<{ id: number }>( + `INSERT INTO items (name, description, status) + VALUES ($1, $2, 'pending') + RETURNING id`, + [`Submission ${new Date().toISOString()}`, null] + ); + const itemId = requireRow(rows, 'the intake item INSERT').id; + + await insertItemImages(client, itemId, files, 0); + + await client.query( + `INSERT INTO item_drafts (item_id, upload_link_id, submitter_note) + VALUES ($1, $2, $3)`, + [itemId, link.id, note === '' ? null : note] + ); + + // Counted inside the transaction and guarded on the same conditions as the + // lookup, so two submissions racing on the last slot of a capped link + // cannot both succeed. + const counted = await client.query( + `UPDATE upload_links + SET submission_count = submission_count + 1, last_used_at = now() + WHERE id = $1 + AND revoked_at IS NULL + AND (max_submissions IS NULL OR submission_count < max_submissions)`, + [link.id] + ); + if (counted.rowCount === 0) { + await client.query('ROLLBACK'); + return res.status(404).json({ error: 'not found' }); + } + + await client.query('COMMIT'); + // No item id in the response: the sender has no business knowing about + // the catalogue, and nothing they can do with it. + res.status(201).json({ ok: true }); + } catch (err) { + await client.query('ROLLBACK'); + console.error(err); + res.status(500).json({ error: 'internal error' }); + } finally { + client.release(); + } +})); + +export default router; +``` + +- [ ] **Step 5: Mount it** + +In `backend/src/app.ts`, with the other public routers — no `requireAdminGate`: + +```ts +import intakeRouter from './routes/intake'; +``` + +```ts +app.use('/api/intake', intakeRouter); +``` + +- [ ] **Step 6: Run the tests** + +```bash +cd backend +npx jest -c jest.integration.config.js --runInBand intake.integration +npm run test:unit +npm run lint && npm run build +``` + +Expected: PASS, 11 integration tests, and the full unit suite including `routesAreWrapped`. + +- [ ] **Step 7: Run the whole backend suite before committing** + +```bash +cd backend && npm run test:integration +``` + +Expected: PASS. This is the point where a mistake in Task 2's extraction would surface in the admin inventory tests. + +- [ ] **Step 8: Commit** + +```bash +git add backend/src/routes/intake.ts backend/src/rateLimit.ts backend/src/app.ts backend/tests/integration/intake.integration.test.ts +git commit -m "feat(intake): accept photo submissions through a shared link (#NNN)" +``` + +--- + +### Task 6: The public submission page + +**Files:** +- Create: `frontend/src/intake/intakeApi.ts`, `frontend/src/intake/Submit.tsx` +- Modify: `frontend/src/main.tsx:101-110` + +**Interfaces:** +- Consumes: `GET/POST /api/intake/:token` +- Produces: route `/submit/:token` + +- [ ] **Step 1: Write the API client** + +Create `frontend/src/intake/intakeApi.ts`: + +```ts +export interface IntakeLink { + label: string; +} + +export async function fetchIntakeLink(token: string): Promise { + const res = await fetch(`/api/intake/${encodeURIComponent(token)}`); + // Every refusal is a 404 by design, so there is one "this link does not + // work" state rather than several the page would have to explain. + if (!res.ok) return null; + return res.json(); +} + +export async function submitItem( + token: string, + files: File[], + note: string +): Promise<{ ok: true } | { ok: false; error: string }> { + const body = new FormData(); + for (const file of files) body.append('images', file); + body.append('note', note); + + const res = await fetch(`/api/intake/${encodeURIComponent(token)}`, { method: 'POST', body }); + if (res.ok) return { ok: true }; + + const payload = await res.json().catch(() => ({})); + return { ok: false, error: payload.error ?? 'Something went wrong. Please try again.' }; +} +``` + +- [ ] **Step 2: Write the page** + +Create `frontend/src/intake/Submit.tsx`: + +```tsx +import React, { useEffect, useState } from 'react'; +import { useParams } from 'react-router-dom'; +import Typography from 'antd/es/typography'; +import Card from 'antd/es/card'; +import Upload from 'antd/es/upload'; +import ButtonAntd from 'antd/es/button'; +import Input from 'antd/es/input'; +import Alert from 'antd/es/alert'; +import Spin from 'antd/es/spin'; +import Space from 'antd/es/space'; +import type { UploadFile } from 'antd/es/upload/interface'; +import { fetchIntakeLink, submitItem } from './intakeApi'; + +const { Title, Paragraph } = Typography; +const { TextArea } = Input; + +// The three types the server will accept. Listed here so the file picker +// offers exactly those; the server checks the bytes regardless. +const ACCEPT = 'image/jpeg,image/png,image/webp'; +const MAX_IMAGES = 6; + +export default function Submit() { + const { token = '' } = useParams(); + const [label, setLabel] = useState(null); + const [checking, setChecking] = useState(true); + const [files, setFiles] = useState([]); + const [note, setNote] = useState(''); + const [sending, setSending] = useState(false); + const [sent, setSent] = useState(false); + const [error, setError] = useState(null); + + useEffect(() => { + let cancelled = false; + void fetchIntakeLink(token).then((link) => { + if (cancelled) return; + setLabel(link?.label ?? null); + setChecking(false); + }); + return () => { + cancelled = true; + }; + }, [token]); + + async function send() { + setSending(true); + setError(null); + const result = await submitItem( + token, + files.map((f) => f.originFileObj as File).filter(Boolean), + note + ); + setSending(false); + if (result.ok) { + setSent(true); + return; + } + setError(result.error); + } + + if (checking) return ; + + // One state for every refusal, matching the server's single 404. + if (label === null) { + return ( + + This link is not active + + It may have been turned off, or already used as many times as it was meant for. Ask + whoever sent it to you for a new one. + + + ); + } + + if (sent) { + return ( + + Thank you — it arrived + + Somebody will look at your photos and write it up. Nothing is listed for sale until + they have. + + { + setFiles([]); + setNote(''); + setSent(false); + }} + > + Send another item + + + ); + } + + return ( + + Send in an item + + Photos of one item, and anything you know about it. Send each item separately. + + + + false} + onChange={({ fileList }) => setFiles(fileList)} + > + Choose photos + + +