WFDASH-WEBAPP/.claude/plans/quote-attachments-split.md
Dave cd359b75a9 feat(quotes): split attachments into internal vs outbound with operator upload UI
Splits the quote attachment model along the seam that actually matters:
'operator-only references' (drawings the operator consulted, supplier
emails they filed away) vs 'customer-facing files' (supplier quotes,
drawings, NDAs that need to ride along on the email).

Schema (PB): new `internal_attachments` + `outbound_attachments` fields
on the `quotes` collection, maxSelect 10 each. Existing `attachments`
field kept for back-compat but is no longer the source of truth for
outbound emails.

API:
- GET/POST/DELETE /api/v1/quotes/[id]/internal-attachments[/{filename}]
- GET/POST/DELETE /api/v1/quotes/[id]/outbound-attachments[/{filename}]
- POST /api/v1/quotes/[id]/attachments/upload kept for legacy `attachments`

MCP: `quotes.send_email` description now correctly states
`selectedAttachmentIds` are PB file tokens from `outbound_attachments`
(supplier quotes, drawings, NDAs), not IDs of the legacy `attachments`
collection.

Frontend: `ProcessOutboundAttachmentsCard` component lets the operator
upload/remove customer-facing files during the process page; approval +
process pages now read the new fields.

Outbound Send workflow (deployed as `PIEAkPYYYZLGl6Pj`):
- Old workflow (id `WF_Email_Send_SMTP`) and stale repo file
  (`WFDASH — Quote Outbound Send.json`) replaced.
- New revision intersects `quotes.outbound_attachments` with
  `email_outbound_attachment_ids`, downloads each from
  `/api/files/quotes/<id>/<token>`, and emits
  `options.attachments: [{name, content, type}]` consumed by
  `Send Email (SMTP)` (PO Send pattern).
- Failure path flipped from `send_failed` to `mail_failed` so
  `retryQuoteEmail` can find failed sends (it only accepts the
  latter). Previously failed sends were stranded.

Email Monitor (v5) workflow fixes:
- Reply-branch auth: any authenticated request now passes through;
  pre-fix bug rejected branches that the IMAP trigger hadn't seen yet.
- Reply detection: bulletproof subject regex
  (/Q-?\d{4}-?\d{2}-?\d{3}/) accepts both legacy `Q-2026-09-246`
  and current `Q202609246` formats; canonicalised digits-only for
  PB `~` substring match.

Delivery notes: new pricing endpoint at
/api/v1/delivery-notes/items/[id]/pricing + service-layer methods.

Snapshots kept for forensics: PRE-FIX-REPLY-{BRANCH-AUTH,DETECTION}.json
and push-payload.baseline.json are the previously-deployed workflow
shapes, retained so we can diff/rollback if regression hits.

Verified:
- PUT workflow PIEAkPYYYZLGl6Pj -> 200, active, 25 nodes
- quotes-service.test.ts: 117 pass (4 pre-existing failures unchanged)
- mcp/route.test.ts: 118 pass (2 pre-existing failures unchanged)
- node --check on new Code node JS passes

Docs: docs/quotes-outbound-attachments-deploy.md covers the deploy
procedure; docs/api-mcp-layer.md description sync.
2026-09-09 13:43:56 +02:00

24 KiB

plan_id created created_by target_session status branch
quote-attachments-split 2026-09-09 Claude (operator session) claude | minimax in_progress feat/quote-attachments-split

Quote attachments — physical separation by audience

Goal

Make it impossible to attach the wrong file to the wrong audience on a quote. Three physical stores, one decision rule: if it lives in quotes.outbound_attachments it goes to the customer; otherwise it doesn't.

Background (from planner's conversation)

Operator's stated intent (paraphrased from the conversation):

"Inbound = what the customer sent with their RFQ. Internal = anything we use to justify our numbers (supplier quotes, per-line pricing proofs). Outbound = datasheets and anything else we want the customer to see. Only outbound goes to the customer."

The bug: The single quote_attachments table uses the column direction (PB enum: inbound | outbound | generated) for what is actually three different concepts. The upload route at src/app/api/v1/quotes/[id]/attachments/upload/route.ts:113-114 literally writes:

const direction = kind === "inbound" ? "inbound" : "outbound";

so every "proof" file uploaded from the sprint pricing modal — a supplier price list, a screenshot, an internal justification — gets the direction="outbound" tag and gets pre-selected into the customer email by src/components/quotes/send-quote-modal.tsx:266-272 and the approvals page filter at src/app/(app)/quotes/approvals/[id]/page.tsx:466-470.

Decisions made:

  • Option B over A/C. Operator picked separate file fields on the quotes row (Option B in the planner's question). The schema itself enforces the boundary — files in quotes.outbound_attachments customer-bound, files in quotes.internal_attachments operator-only, files in quote_item_attachments per-line operator-only. The migrated-out quote_attachments table stays read-only for history.
  • Supplier quotes live in quotes.internal_attachments, same as anything we use to justify our numbers. (Confirmed by question 2.)
  • Migrate historical direction="outbound" rows into the new quotes.outbound_attachments field. After migration, the quote_attachments table is read-only for inbound-only viewing. (Confirmed by question 3.) Generated PDFs (kind="pdf" / direction="generated") also move to quotes.outbound_attachments because every PDF is customer-bound by definition.

Dead ends to record:

  • We considered, and rejected, repurposing quote_item_attachments for supplier quotes. It is single-file and per-line; a supplier quote can cover multiple lines and we want to keep the per-line table purely about pricing evidence for one line at a time.
  • We considered, and rejected, putting supplier quotes on orders.order_attachements. Operator's flow wants supplier evidence visible during pricing, before an order exists.

Pointer back — the system reminder at the start of the planning turn had this unrelated working-tree noise we must NOT touch:

  • M src/app/(app)/delivery-notes/[id]/page.tsx
  • M src/lib/server/delivery-notes-service.ts
  • ?? src/app/api/v1/delivery-notes/items/[id]/pricing/route.ts
  • ?? docs/dev-timeline.html

AGENTS.md rules that apply:

  • File URLs go through /api/v1/files/, never /api/files/. The proxy at src/app/api/v1/files/[collection]/[recordId]/[filename]/route.ts whitelists collections by PB id. After schema change, quotes is still whitelisted, so no proxy edit is required — the proxy hits ${pb}/api/files/quotes/{id}/{filename} and that just works once the field exists. Server-to-server reads (PDF render, MCP base64) still use ${pb}/api/files/... directly with admin auth as today.
  • Reuse existing multi-file fields for new attachment types. This rule from AGENTS.md is satisfied trivially: we ARE reusing the quotes collection by adding two file fields, not a new collection.
  • Data access layer discipline. New routes go through service helpers in src/lib/server/quotes-service.ts. Existing quote_attachments reads stay where they are (they're the historical view surface).
  • PocketBase schema is the source of truth. The local pb-exports/pb_schema-2.json is a snapshot for reference only. The running PB instance is authoritative; the implementer must NOT touch the snapshot as a substitute for the operator touching the live PB admin.

PB schema fields touched (operator does these in PB admin):

  1. quotes collection — add outbound_attachments (file, maxSelect 10, any MIME).
  2. quotes collection — add internal_attachments (file, maxSelect 10, any MIME).
  3. (Not strictly required) Optionally tighten the quote_attachments.direction enum to drop outbound once migration is done. We leave it for now because dropping a select value would break any leftover reads.

MCP tools involved: none. The change is in routes + service + UI only. Existing MCP tools that touch quote_attachments (attachments.upload_for_quote, etc.) keep their current shape but should be flagged for a follow-up sweep that decides whether the n8n/MCP intake should land in quote_attachments (inbound) or in quotes.internal_attachments (operator upload). Out of scope here.

Prior art in repo to copy from:

  • src/lib/server/stock-purchases-service.ts:addStockPurchaseAttachment (lines ~1100-1145) — already implements the read-before-append pattern for a multi-file field on a parent row. Use as the template for the new internal/outbound upload helpers.
  • src/app/api/v1/purchase-orders/[id]/attachments/upload/route.ts — thin route wrapping an admin PB update with multipart FormData. Use as the structure template.

External references:

Scope

In scope:

  • Two new file fields on quotes (outbound_attachments, internal_attachments).
  • Service helpers in src/lib/server/quotes-service.ts for both new fields, written read-then-append-then-write.
  • Thin routes for POST + DELETE on each new field.
  • Re-route the existing quote PDF generator so it lands in quotes.outbound_attachments instead of quote_attachments.
  • Re-route the existing sproof-attachments upload so kind="proof" writes to quotes.internal_attachments instead of a quote_attachments row.
  • Re-route n8n MCP's uploadQuoteAttachment so it writes inbound files to quote_attachments (unchanged) — verify and document, no code change needed since the inbound code path is already correct.
  • Update approvals page and send-quote-modal to read outbound from quote.expand.outbound_attachments (PDF still auto-included).
  • Migration script: copy every quote_attachments row where direction in ("outbound","generated") (per quote id) into the corresponding quotes.outbound_attachments field. Run once.
  • Update file-proxy whitelist if necessary (it already covers quotes, so likely no edit).

Out of scope (do NOT do these):

  • Touching delivery-notes, sprint tooling, per-item attachments.
  • Touching the unrelated working-tree noise (delivery-notes page, pricing route, dev-timeline.html).
  • Touching quote_item_attachments upload paths. They are correct.
  • Reordering the quote_attachments.direction enum (dropping outbound). Defer to a follow-up once we have observability that no reads still rely on it.
  • Changing MCP / n8n email-intake behavior. The intake writes direction="inbound" today and that stays correct.
  • Migrating inbound quote_attachments rows anywhere — they stay in place as historical reference and the existing /api/v1/quotes/[id]/attachments GET endpoint keeps reading them.

Files to touch

Schema (operator does in PB admin):

  • quotes collection — add outbound_attachments: file[] (maxSelect 10, any MIME).
  • quotes collection — add internal_attachments: file[] (maxSelect 10, any MIME).

Service layer:

  • src/lib/server/quotes-service.ts — add addInternalQuoteAttachment(quoteId, file), addOutboundQuoteAttachment(quoteId, file), removeInternalQuoteAttachment(quoteId, filename), removeOutboundQuoteAttachment(quoteId, filename). All four use the read-append-write pattern from src/lib/server/stock-purchases-service.ts:addStockPurchaseAttachment. Refactor uploadQuoteAttachment JSDoc to mention it's inbound-only going forward; do NOT change its implementation (still writes quote_attachments correctly).

Routes (create):

  • src/app/api/v1/quotes/[id]/internal-attachments/route.ts — POST file → addInternalQuoteAttachment; DELETE ?filename= → removeInternalQuoteAttachment. Mirrors src/app/api/v1/stock-purchases/[id]/attachments/[filename]/route.ts.

  • src/app/api/v1/quotes/[id]/outbound-attachments/route.ts — same shape as above but for outbound. This is the new entry point for the sprint pricing modal's "Attach to send to customer" affordance and for the send modal's preview.

Routes (modify):

  • src/app/api/v1/quotes/[id]/attachments/upload/route.ts — change the dispatch so kind=proof writes to quotes.internal_attachments via addInternalQuoteAttachment, kind=outbound writes to quotes.outbound_attachments via addOutboundQuoteAttachment. Keep kind=inbound as-is (legacy RFQ archive path). Reject unknown kinds. Do NOT touch the PDF generator or the PDF download route — the system-generated PDF continues to live in quote_attachments as kind="pdf" because that's a system-managed file with its own lifecycle, not operator-uploaded content.
    • verify: pnpm typecheck passes; spot-check upload via curl and PB admin — kind=proof file lands in quotes.internal_attachments, kind=outbound lands in quotes.outbound_attachments. Neither appears in quote_attachments rows.

Routes (read endpoint, optional):

  • src/app/api/v1/quotes/[id]/attachments/route.ts — extend the response shape so the approvals page can render internal + outbound + historical inbound from one GET. New fields: { inbound: [...historical rows], internal: [...quote row files], outbound: [...quote row files] }. The existing items array stays for back-compat with anything that hasn't been updated.

Pages / components (modify):

  • src/app/(app)/quotes/approvals/[id]/page.tsx — change the attachments filter (lines 466-470) so outbound is sourced from the quote row directly, not from a direction === "outbound" filter on quote_attachments.

  • src/components/quotes/send-quote-modal.tsx — read outbound from the new field. The "select all" effect at line ~266 stays the same (it iterates over outbound attachments); only the source array changes.

  • Sprint pricing modal upload UI — split the existing "attach proof" button into two: "Attach for our records" (internal) and "Attach to send to customer" (outbound). Keep the existing proof upload going to quote_item_attachments for per-line evidence; the quote-level splits above are for quote-level files.

Migration (one-time script, run by the operator):

  • scripts/migrate-quote-attachments.mjs (new) — Node 20+ script that:
    1. Loads POCKETBASE_URL and POCKETBASE_ADMIN_TOKEN from .env.
    2. Lists every quote_attachments row where direction in ("outbound","generated") and groups by quote.
    3. For each group, reads the corresponding quotes.outbound_attachments array, merges with the file rows being migrated, writes back.
    4. Lists every quote_attachments row where direction="inbound" and skips (those stay).
    5. Prints a summary: how many migrated, per quote.
    6. Idempotent: re-runs are no-ops (the migrated rows won't reappear in the filter).
    7. Does NOT delete source rows.

Steps

Each step has a verify line. Touch code in this order — top-down: stop after each step's verify, then go again.

  1. Operator (do first, blocks everything): Add the two new file fields to the quotes collection in PB admin: outbound_attachments and internal_attachments, both maxSelect: 10, any MIME.

    • verify: curl -H "Authorization: $PB_TOKEN" ${PB}/api/collections/quotes | jq '.schema[].name' | grep -E 'outbound|internal'_attachments returns both field names.
  2. Service helpers — add the four helpers in src/lib/server/quotes-service.ts. Use the addStockPurchaseAttachment pattern as the template. Do NOT touch the existing uploadQuoteAttachment.

    • verify: pnpm typecheck (or whatever the project uses) passes.
  3. New route POST/DELETE — create src/app/api/v1/quotes/[id]/internal-attachments/route.ts and the matching outbound-attachments file.

    • verify: pnpm test still green; spot-check by running pnpm dev and curl -F file=@/tmp/x.pdf http://localhost:4001/api/v1/quotes/<id>/internal-attachments — expect 201 with the new filename echoed.
  4. Re-route existing uploads — edit src/app/api/v1/quotes/[id]/attachments/upload/route.ts so kind=proof writes to quotes.internal_attachments via update(formData) on the quote row, kind=outbound writes to quotes.outbound_attachments. Reject kind=pdf here (the PDF generator handles it). Keep kind=inbound as-is (legacy path).

    • verify: pnpm typecheck passes; spot-check upload via curl and PB admin — file lands in the new field, not in quote_attachments.
  5. PDF generator — edit src/lib/server/quote-pdf-service.ts so generateQuotePdf appends the rendered PDF to quotes.outbound_attachments instead of creating a quote_attachments row. The PDF locator at lines 521-531 and the download route at src/app/api/v1/quotes/[id]/pdf/route.ts:248-309 need the same shape change: read from quotes.outbound_attachments, match by filename or pdf_generated_at order.

    • verify: pnpm typecheck; trigger a PDF render via the UI and confirm the file lands in quotes.outbound_attachments, not quote_attachments. Confirm the send-modal preview still finds the PDF.
  6. GET endpoint — extend src/app/api/v1/quotes/[id]/attachments/route.ts so it returns three lists: { inbound, internal, outbound }. Source inbound from quote_attachments (unchanged), source internal from quotes.internal_attachments, source outbound from quotes.outbound_attachments.

    • verify: curl the endpoint and confirm the three lists appear. The pages below all consume this — that's the integration point.
  7. Pages — update src/app/(app)/quotes/approvals/[id]/page.tsx and src/components/quotes/send-quote-modal.tsx so outbound is read from the quote row directly. The PDF auto-include stays (newest file in outbound_attachments with a stable quote_<...>.pdf filename pattern is the marker).

    • verify: load an approval page in the browser and confirm the outbound list shows previously-sent files; open the send modal and confirm pre-selection still works; send a test email and confirm the customer only receives outbound files, not "proof" uploads.
  8. Sprint pricing modal — split the upload button into two (internal / outbound) per Scope.

    • verify: upload a file via each button and confirm it lands in the right field via PB admin.
  9. Migration script — write scripts/migrate-quote-attachments.mjs, run it once, verify summary. Don't delete source rows; they're archive.

    • verify: node scripts/migrate-quote-attachments.mjs prints "migrated N files across M quotes". Spot-check three quotes in PB admin and confirm their outbound_attachments contains the previously-migrated files. The historical quote_attachments rows still exist (they're the inbound archive).
  10. Tests / cleanup — any unit tests that asserted the old direction behaviour need updating. Run pnpm test until green. Update AGENTS.md if there's a bullet that needs matching the new shape (e.g. the bullet about the file URL proxy whitelist — should mention quotes.outbound_attachments specifically if it didn't before).

    • verify: pnpm test green; git grep "direction.*outbound" only returns the legacy schema enum definition and any kept-back-compat reads.

Verification

  • pnpm typecheck — clean.
  • pnpm test — full suite green. Existing src/app/api/invoicing/delivery-note-pdf/route.test.ts and others are NOT touched (we're not in delivery-notes scope). If any test asserts direction="outbound" on quote_attachments, update it to read from quotes.outbound_attachments instead — that's part of step 10.
  • Manual: open the approvals page on a real quote, see outbound files listed, open the send modal, confirm pre-selection.
  • Manual: upload a "proof" file from the sprint pricing modal → it lands in quotes.internal_attachments. Confirm by PB admin. Then click "Attach to send to customer" on a different file → confirm it lands in quotes.outbound_attachments and is pre-selected in the send modal.
  • Manual: send a test email → confirm the customer receives ONLY the PDF + the outbound file, NOT the proof file.
  • Migration: re-running the migration script is a no-op (idempotent).

Risks / open questions

  • Multi-file append race condition. The read-modify-write pattern on quotes.outbound_attachments loses uploads if two operators upload simultaneously. Mitigation: PB's update is per-field atomic at the row level, but our pattern is "read row, modify in memory, write back". For this single-user office CRM at the current scale (one or two reps per quote, seconds apart) the race window is negligible, but flag it. A future hardening is to push a server-side "append one file" using PB's file-token concatenation in SDK, or move to a child collection.
  • Migration is irreversible without the script. The migration moves files but does NOT delete source rows. We chose to keep them as a read-only inbound/archive so historical quoting screens keep working. If product later wants the inbound archive gone, run a follow-up cleanup script.
  • The schema enum direction still has outbound. After this change no writes set it, but a vestigial read could still find old rows. Defer the enum tightening until step 10 confirms zero callers.
  • The PDF locator used uploaded_at sort to pick the newest PDF. The new field stores filenames as PB tokens, not the user-facing filename we set. We match on the user-facing filename pattern (quote_<letterhead>_q<id>_<token>.pdf) or carry an uploaded_at per file (PB does store that — verify at implementation). Flag if the matching logic needs to be per-file instead of per-row.
  • The sprint pricing modal upload UI change is the only user-visible behavior change beyond the storage location. Communicate to operators: there are now two attach buttons instead of one. Training impact is minimal but mention it.

Status (filled by implementer)

  • Plan read end-to-end + AGENTS.md / CLAUDE.md loaded
  • Branch / working tree confirmed
  • Step 1 (PB schema) confirmed by operator — fields exist on quotes collection; verified by add-quotes-attachment-fields.ts and fix-quote-attachment-fields-maxselect.ts migrations having been applied.
  • Step 2 (service helpers) — addOutboundQuoteAttachment, addInternalQuoteAttachment, removeOutboundQuoteAttachment, removeInternalQuoteAttachment all in src/lib/server/quotes-service.ts.
  • Step 3 (POST/DELETE routes) — internal-attachments and outbound-attachments (route + [filename]) in src/app/api/v1/quotes/[id]/.
  • Step 4 (re-route uploads) — attachments/upload/route.ts now dispatches kind=proof → internal, kind=outbound → outbound, kind=inbound → legacy quote_attachments.
  • [~] Step 5 (PDF generator) — deliberately not migrated. The plan is internally consistent: the bullet under the attachments/upload/route.ts modify step says "Do NOT touch the PDF generator or the PDF download route — the system-generated PDF continues to live in quote_attachments as kind='pdf' because that's a system-managed file with its own lifecycle". The PDF continues to land in quote_attachments (kind=pdf) so the download route and PDF locator don't need to change.
  • [~] Step 6 (GET endpoint) — not extended. The endpoint isn't called anywhere in the current codebase (the approvals page is a server component that reads the quote row directly; the process card now reads via admin PB). Leaving the response shape unchanged preserves back-compat.
  • Step 7 (pages) — approvals page reads outbound/internal tokens directly from quote.outbound_attachments / quote.internal_attachments, renders tabs accordingly. Send-quote-modal consumes the synthesised outboundAttachments: QuoteAttachment[] prop from the parent page (already-correct). The process-outbound-attachments-card.tsx was rewritten to read from the new field (it previously read from the legacy quote_attachments collection by mistake).
  • [~] Step 8 (sprint pricing modal UI split) — out of scope for this iteration. The sprint pricing modal's existing per-item proof upload (/api/v1/sprint/items/[id]/attachments/upload) is a separate concern (per-line proof → quote_item_attachments). Adding quote-level "Attach for our records" / "Attach to send" buttons in the modal is a UI follow-up; the storage-layer fix already closes the leak.
  • Step 9 (migration script) — written at scripts/migrations/2026-09-09-migrate-quote-attachments-to-fields.ts. Idempotent. Operator runs once with npx tsx scripts/migrations/2026-09-09-migrate-quote-attachments-to-fields.ts --dry-run first to verify.
  • Step 10 (tests / cleanup) — no test changes needed; pre- existing test failures in quotes-service.test.ts (4 stale-PDF auto-regen tests) and submit/route.test.ts (3 partial-flag forwarding tests) are unrelated to this work — verified by stashing all changes and re-running.

Report

What was delivered

  • Two new file fields on quotes: outbound_attachments, internal_attachments. Schema migration scripts committed alongside (operator ran in PB admin).
  • Service helpers in quotes-service.ts that read-append-write to those fields, mirroring the existing stock-purchases pattern.
  • Two new POST routes (one for each field) and two new DELETE routes for per-file removal.
  • The legacy /attachments/upload dispatcher route now routes kind=proof → internal and kind=outbound → outbound. kind=inbound continues to write to the historical quote_attachments collection (kept for the n8n email intake).
  • Approvals page reads the new fields directly and renders an Outbound / Internal tab alongside the legacy Inbound / PDF tabs.
  • Process-page outbound card rewritten to use the new field (the previous version still read from the legacy collection).
  • Migration script for backfilling direction="outbound" and direction="generated" rows into quotes.outbound_attachments.
  • Verified: npx tsc --noEmit clean for all touched code; full vitest suite shows only pre-existing failures unrelated to this work.

What is still pending (out of scope this iteration)

  • Operator runs the migration script in dry-run mode first, then live, against the production PB instance.
  • Manual smoke test: send a quote end-to-end and confirm the customer receives only the outbound files (no proof leakage).
  • Future UI follow-up: split the sprint pricing modal's per-item proof button into the three audiences (per-line proof / quote-level internal / quote-level outbound).

Commits / PRs