diff --git a/docs/api-mcp-layer.md b/docs/api-mcp-layer.md index 77e16c6..1ab122a 100644 --- a/docs/api-mcp-layer.md +++ b/docs/api-mcp-layer.md @@ -519,7 +519,7 @@ Grouped by resource family. The full description is in the table column; the des | `quotes.save_as_template` | Snapshot an existing quote as a reusable template. The original quote is not modified; a new `is_template=true` row is created. | | `quotes.split` | Split a quote into multiple quotes by grouping items. Body is `{ source_quote_id, groups: [{ name, item_ids: string[] }] }`. Useful for partial wins where the customer accepts some lines but not others. | | `quotes.partial_approve` | Mark a subset of quote items as `approved` (or `rejected`) on a quote. Body: `{ quote_id, decisions: [{ item_id, decision }] }`. Useful when the customer accepts some lines but not others. | -| `quotes.submit` | Flip a `new` quote to `approval_pending` once every line is priced. Idempotent — re-submitting an already-`approval_pending` quote is a no-op (returns `statusChanged: false`). Gated: every line on the quote must have `selling_price > 0`;... | +| `quotes.submit` | Flip a `new` quote to `approval_pending` once every line is priced. Idempotent — re-submitting an already-`approval_pending` or `partial_approval_pending` quote is a no-op (returns `statusChanged: false`). Gated: every line on the quote ... | | `quotes.re_price` | Re-run pricing on every quote_item under a quote. Whitelist of patchable fields is the same as `bulk_update_quote_items`. Returns the per-row result list. | | `quotes.next_number` | Reserve the next quote number (`Q{yy}{mm}{NNN}`). Atomic counter; gaps are possible if the caller never persists the resulting draft. | | `quotes.lookup` | Free-text lookup over quotes for the order page's quote-picker dropdown. Substring match across quote_number, quote_reference, rfq_number, customer name, company name. Excludes templates by default. | diff --git a/src/app/api/mcp/FEATURE-REQUESTS.md b/src/app/api/mcp/FEATURE-REQUESTS.md index 8c0dc2a..e3a4365 100644 --- a/src/app/api/mcp/FEATURE-REQUESTS.md +++ b/src/app/api/mcp/FEATURE-REQUESTS.md @@ -149,6 +149,13 @@ These came up but need a human answer before code changes: - 2026-08-15 — **`quotes.submit` shipped.** New REST + MCP tool that flips `status: new → approval_pending` once every line is priced. Closes the walk-in pipeline gap — previously the agent had to call `quotes.update({status: "approval_pending"})` directly, which had no "all items priced" gate and bypassed the audit trail. Service: `submitQuote(id, actor?)` in `quotes-service.ts` (typed `SubmitQuoteError` with `code: "quote_not_found" | "invalid_status" | "unpriced_items"`); REST: `POST /api/v1/quotes/[id]/submit` (200 + `{ quote, statusChanged }` on success, 422 with `code` + `offendingItemIds` on the gated failure); MCP: `quotes.submit({id, actor?})` entry + dispatch. Idempotent — re-submitting an already-`approval_pending` quote is a no-op (`statusChanged: false`, no PB write). The `unpriced_items` failure surfaces the offending item ids so the agent can price the gaps and retry. Optional `actor: { id, name }` lands on `quotes.submitted_by` + `quotes.submitted_at` (both are optional fields on the Quote type; PB silently drops them until ops adds the columns to the schema — see the new “submit audit fields” pending request below). 8 new unit tests for `submitQuote` (happy path, idempotent re-submit, all 6 wrong-status types, unpriced with mixed lines + moved-to-sibling exclusion, 404, empty/whitespace id, no actor, empty actor name) + 9 new REST integration tests (200/200-idempotent/200-no-actor/200-malformed-actor/404/422-invalid_status/422-unpriced_items/500/discriminator). Recipe 7 in `docs/agents/workflows.md` updated to include `submit` between `re_price` and `send_email`. `hermes-skill.md` status enum now documents the `new → approval_pending` transition via `quotes.submit` and the `quotes.send_email` rejection (`code: "invalid_status"`) when called on a `new` quote. `docs/api-mcp-layer.md` regenerated — tool count 126 → 127. +- 2026-08-15 — **Phase G (continued): Gielie's 4-Fail matrix closed.** Hermes re-ran the `quotes.submit` / `quotes.send_email` test matrix and reported 4 FAILs in the response payload and pre-send validation paths. All four are now fixed in the same commit batch: + - **Test 3 — `quotes.submit` response echo.** The 200 response was a bare `{ quote, statusChanged }` so the agent had no way to confirm what target state it landed on. `submitQuote` now returns `{ quote, statusChanged, previousStatus, submittedAt, targetStatus, submittedBy? }` on every path (including idempotent re-submit + `partial_approval_pending`). The new `targetStatus` field discriminates `approval_pending` (default) vs `partial_approval_pending` (when the `partial: true` flag is passed); `previousStatus` is always populated so the agent can build a state delta without a follow-up `quotes.get` round-trip. `submittedAt` is a fresh ISO timestamp from the server, not echoed from the request. REST route signature unchanged (extra fields are additive). + - **Test 4 — `quotes.send_email` no-PDF failure surfaces readiness gaps.** Previously the pipeline returned the opaque `no_pdf` code on any "PDF not rendered yet" path, including the cases where the quote was missing items or had unpriced / supplierless lines. The operator wanted the agent to *tell* them what to fix rather than auto-render partial PDFs. New `diagnosePdfReadiness(pb, quoteId, quote)` helper in `src/lib/server/quotes-service.ts` runs the same three gates as `quotes/[id]/pdf/route.ts` (placeholder customer, no items, items-not-ready) BEFORE the PDF-presence check, and emits a structured `SendQuoteEmailFailure` with `details.issues: [{ itemId, description, missing[] }]`. The MCP `failure` envelope now carries the per-line gaps so the agent can present them verbatim: `Read 'Black industrial bags' — missing: cost price, selling price. Read 'Hex bolt M16' — missing: no supplier.` The `no_pdf` code is preserved as a fallback when the readiness gates all pass but no PDF is on the quote. Updated `TOOL_HINTS` for `quotes.send_email` to document the new failure codes. + - **Test 5 — `quote_items.create` auto-derive fires for real.** The `deriveSellingPriceIfOmitted` helper was already called from `quote_items.update`, `bulk_update_quote_items`, and `quotes.re_price` (Phase F), but `createQuoteItem` was reading `input.selling_price` directly — agents who passed `cost_price` + `markup` without `selling_price` got a row with `selling_price = 0`, which then tripped the `unpriced_items` gate on `quotes.submit`. The helper is now invoked in `createQuoteItem` and the persisted value comes from `derived.selling_price`, matching the REST PATCH / MCP `quote_items.update` paths. The same `deriveSellingPriceIfOmitted` is exported (10 unit tests) so callers can dry-run the derivation client-side. + - **Test 6 — `quotes.submit` partial-approval flag.** A "partial quote" — some lines approved, others left blank because the customer only wants a subset — used to require a render-then-partial chain because the `unpriced_items` gate was unconditional. New `partial: true` flag on `quotes.submit` flips the target status to `partial_approval_pending` AND bypasses the unpriced gate. The MCP `quotes.submit` schema + description + REST route now document the flag; the tool description summarises the new target status. Idempotent re-submit of an already-`partial_approval_pending` quote is a no-op (same `statusChanged: false` semantics as the `approval_pending` path). The full `previousStatus` / `submittedAt` / `submittedBy?` / `targetStatus` echo from Test 3 carries through to the partial path. + - **Verification:** 83/83 `quotes-service.test.ts` + 9/9 `mcp-upstream-fetch.test.ts` + 16/16 `quote-items-service.test.ts` + 110/113 `route.test.ts` (the 3 pre-existing failures on `main` are unrelated: one stale-tools-list count, two `purchase_orders.record_diff_report` 400-vs-404 mocks). `pytest scripts/mcp-doc-gen.py --check` clean (127 tools, up to date). `docs/api-mcp-layer.md` regenerated — only the `quotes.submit` description line changed (added "or `partial_approval_pending`" to the idempotency clause). No PB schema changes; no new tools added. + ### Customers: case-insensitive email match - **Surfaced:** 2026-08-15 — searching by email fails when the case doesn't match (PB `=` is case-sensitive on stored values) - **Workaround:** use substring `~` instead of `=` (the customers service does this in the search branch but not the exact branch) diff --git a/src/app/api/mcp/route.ts b/src/app/api/mcp/route.ts index bdd1359..92e751b 100644 --- a/src/app/api/mcp/route.ts +++ b/src/app/api/mcp/route.ts @@ -317,10 +317,10 @@ function isAlreadyPriced(item: { // actually tests today. Add entries as new write tools land. const TOOL_HINTS: Record = { "quotes.submit": { - hint: "Submission can fail with 422 (unpriced_items, invalid_status) or 404 (quote_not_found). All deterministic — do NOT retry.", + hint: "Submission can fail with 422 (unpriced_items, invalid_status) or 404 (quote_not_found). All deterministic — do NOT retry. If the quote has unpriced items but the operator wants the partial path, re-send with `partial: true` (bypasses the unpriced gate, flips to `partial_approval_pending`).", nextActions: [ - "Read data.upstreamBody.code — unpriced_items lists offendingItemIds; price the gaps and resend", - "invalid_status means the quote is not in `new` or `approval_pending`; use quotes.get to inspect", + "Read data.upstreamBody.code — unpriced_items lists offendingItemIds; either price the gaps and resend, OR resend with `partial: true` to enter the partial-approval workflow", + "invalid_status means the quote is not in `new`; use quotes.get to inspect the current status (already-pending returns statusChanged: false, no error)", "quote_not_found means the id is wrong; re-resolve via quotes.list", ], }, @@ -339,11 +339,13 @@ const TOOL_HINTS: Record = { ], }, "quotes.send_email": { - hint: "Send can fail with 422 (invalid_status, no recipient, needs attachment) or 500 (PDF render).", + hint: "Send can fail with 422 (invalid_status, no_recipient, invalid_recipient, placeholder_customer, no_items, items_not_ready, no_pdf) or 404 (quote_not_found). When the PDF is missing, the failure carries `details.issues` listing every line that's missing required fields — present those to the operator verbatim, don't auto-render.", nextActions: [ - "If 422 invalid_status: the quote is not in approval_pending/partial_approved — call quotes.submit first", - "If no recipient resolved: set customer.email / alternative_email or pass `to` explicitly", - "If 500 with 'Failed to update record.': PB schema is missing a select value; ask ops to add it", + "If 422 invalid_status: the quote is not in a resendable state — call quotes.submit first (or use quotes.retry_email if it was mail_failed)", + "If no_recipient / invalid_recipient: set customer.email or pass `to` explicitly", + "If placeholder_customer: the quote is still on Unknown Sender / unnamed — pick a real customer via the process page", + "If items_not_ready: read `failure.details.issues` — that's the per-line missing-field list; ask the operator to price + supplier the gaps, or render the PDF themselves", + "If 500 with 'Failed to update record.': PB schema is missing a select value; ask ops to add it via the PB admin", ], }, "quotes.partial_approve": { @@ -943,7 +945,8 @@ const tools = [ name: "quotes.send_email", description: "Front-door for the outbound email send pipeline. Validates and flips the quote to `sending`; the actual SMTP send runs asynchronously in n8n. `selectedAttachmentIds` is a list of `quote_attachments` IDs to include (the PDF is always attached separately). `attachAuthLetters` toggles the per-letterhead supplier authorization letter.\n\n" + - "Only `id` is required. The service walks four sources for the recipient: `to` → `customer.email` → `customer.alternative_email` → `quote.email_sender`. `cc` / `bcc` default to empty. `subject` defaults to the quote's stored `email_subject` if set, else a templated \"Quote {number} — …\" fallback. `message` defaults to the quote's stored `email_body` if set, else empty.", + "Only `id` is required. The service walks four sources for the recipient: `to` → `customer.email` → `customer.alternative_email` → `quote.email_sender`. `cc` / `bcc` default to empty. `subject` defaults to the quote's stored `email_subject` if set, else a templated \"Quote {number} — …\" fallback. `message` defaults to the quote's stored `email_body` if set, else empty.\n\n" + + "The server does NOT auto-render the PDF when one is missing — instead it surfaces the readiness gaps (placeholder customer, no items, items-not-ready) in `failure.details.issues` so the agent can present them to the operator verbatim. Operator fixes the gaps or pre-renders the PDF themselves, then resends.", inputSchema: { type: "object", required: ["id"], @@ -1049,7 +1052,7 @@ const tools = [ { name: "quotes.submit", description: - "Flip a `new` quote to `approval_pending` once every line is priced. Idempotent — re-submitting an already-`approval_pending` quote is a no-op (returns `statusChanged: false`). Gated: every line on the quote must have `selling_price > 0`; unpriced lines are returned as `offendingItemIds` in the 422 so the operator can fix them. Use this after walking through `quotes.create` (walk-in) or `quotes.create_from_email` (email) to push the quote into the approval queue. Body: `{ id, actor?: { id, name } }`. The `actor` is optional and lands on `quotes.submitted_by` when the PB admin has added the column. Returns: `{ quote, statusChanged: boolean }` on success; `code: \"invalid_status\" | \"unpriced_items\" | \"quote_not_found\"` on failure.", + "Flip a `new` quote to `approval_pending` once every line is priced. Idempotent — re-submitting an already-`approval_pending` or `partial_approval_pending` quote is a no-op (returns `statusChanged: false`). Gated: every line on the quote must have `selling_price > 0` unless `partial: true` is set; unpriced lines are returned as `offendingItemIds` in the 422 so the operator can fix them. Pass `partial: true` to allow unpriced items and flip the quote to `partial_approval_pending` instead of `approval_pending` — this is the missing state-machine transition that enables the partial-approve + split workflow (closed 2026-08-15). Use this after walking through `quotes.create` (walk-in) or `quotes.create_from_email` (email) to push the quote into the approval queue. Body: `{ id, actor?: { id, name }, partial?: boolean }`. The `actor` is optional; it lands on `quotes.submitted_by` when the PB admin has added the column. Returns: `{ quote, statusChanged, previousStatus, submittedAt, submittedBy?, targetStatus }` on success; `code: \"invalid_status\" | \"unpriced_items\" | \"quote_not_found\"` on failure. `previousStatus` is the status before the call (echoed even on the no-op path); `targetStatus` is the status the call aimed for (`approval_pending` or `partial_approval_pending`).", inputSchema: { type: "object", required: ["id"], @@ -1058,12 +1061,17 @@ const tools = [ actor: { type: "object", description: - "Optional agent / operator identity stamp. Mirrors the `actor` shape used by `order_items.confirm_collected` and `purchase_orders.record_diff_report`.", + "Optional agent / operator identity stamp. Mirrors the `actor` shape used by `order_items.confirm_collected` and `purchase_orders.record_diff_report`. Echoed in the response as `submittedBy` and stamped on `quotes.submitted_by` when the PB admin has added the column.", properties: { id: { type: "string" }, name: { type: "string" }, }, }, + partial: { + type: "boolean", + description: + "When true, the quote flips to `partial_approval_pending` instead of `approval_pending`, and the unpriced-items gate is bypassed (lines with `selling_price = 0` are allowed through). Use this to enter the partial-approve + split workflow when not every item is priced yet. Defaults to false (full-approval path, strict gate).", + }, }, }, }, @@ -4231,6 +4239,9 @@ export async function POST(req: NextRequest) { throw new Error("Missing id for quotes.submit."); } const hintEntry = TOOL_HINTS["quotes.submit"]; + const submitBody: Record = {}; + if (args.actor) submitBody.actor = args.actor; + if (args.partial === true) submitBody.partial = true; const data = await fetchUpstreamJson( `${upstreamOrigin(req)}/api/v1/quotes/${args.id}/submit`, { @@ -4238,7 +4249,7 @@ export async function POST(req: NextRequest) { headers: { "Content-Type": "application/json", }, - body: JSON.stringify(args.actor ? { actor: args.actor } : {}), + body: JSON.stringify(submitBody), }, "quotes.submit", { hint: hintEntry?.hint, nextActions: hintEntry?.nextActions }, diff --git a/src/app/api/v1/quotes/[id]/submit/route.ts b/src/app/api/v1/quotes/[id]/submit/route.ts index 590787e..8de70ff 100644 --- a/src/app/api/v1/quotes/[id]/submit/route.ts +++ b/src/app/api/v1/quotes/[id]/submit/route.ts @@ -40,24 +40,34 @@ export async function POST( ) { const { id } = await params; - // Optional actor stamp. Default to the agent identity so the - // audit trail always has a name; fall back to a sentinel if the - // caller sent nothing. + // Optional actor stamp + partial flag. The `actor` lands on + // `quotes.submitted_by` (when the PB admin adds the column); the + // `partial` flag flips the quote to `partial_approval_pending` + // instead of `approval_pending` and lets unpriced items through + // (closes the state-machine gap surfaced 2026-08-15 — the previous + // path required a render-then-partial chain). let actor: { id: string; name: string } | undefined; + let partial = false; try { const body = await req.json().catch(() => null); - if (body && typeof body === "object" && body.actor && typeof body.actor === "object") { - const a = body.actor as Record; - if (typeof a.id === "string" && typeof a.name === "string") { - actor = { id: a.id, name: a.name }; + if (body && typeof body === "object") { + if (body.actor && typeof body.actor === "object") { + const a = body.actor as Record; + if (typeof a.id === "string" && typeof a.name === "string") { + actor = { id: a.id, name: a.name }; + } + } + if (body.partial === true) { + partial = true; } } } catch { - // Body is optional — bubble up as no actor if it's malformed. + // Body is optional — bubble up as no actor / no partial if it's + // malformed. } try { - const result = await submitQuote(id, actor); + const result = await submitQuote(id, actor, { partial }); return NextResponse.json(result); } catch (err) { if (err instanceof SubmitQuoteError) { diff --git a/src/lib/server/quote-items-service.ts b/src/lib/server/quote-items-service.ts index 69f4cbe..2c81be1 100644 --- a/src/lib/server/quote-items-service.ts +++ b/src/lib/server/quote-items-service.ts @@ -138,6 +138,20 @@ export async function createQuoteItem( quantity = q; } + // F.1: when caller supplies both cost_price and markup but omits + // selling_price, derive it here. Without this the created row lands + // with selling_price = 0, the unpriced gate on `quotes.submit` trips, + // and the operator has to manually PATCH each line back to life. The + // REST PATCH path (quote_items/[id]) and the MCP `quote_items.update` + // path both already invoke this helper; create was the outlier + // (caught 2026-08-15 via the live test matrix — Hermes confirmed + // `selling_price = 0` on three separate create+update sequences). + const derived = deriveSellingPriceIfOmitted({ + cost_price: input.cost_price, + markup: input.markup, + selling_price: input.selling_price, + }); + const pb = await getAdminPocketBase(); // Confirm the parent quote exists and grab its quote_number for @@ -190,7 +204,10 @@ export async function createQuoteItem( description, quantity, cost_price: Number(input.cost_price) || 0, - selling_price: Number(input.selling_price) || 0, + // Read selling_price through the derive result so the F.1 + // auto-derive (cost × (1 + markup/100)) actually fires on + // create. See derived computation above. + selling_price: Number(derived.selling_price) || 0, markup: Number(input.markup) || 0, sku: typeof input.sku === "string" ? input.sku : "", unit: typeof input.unit === "string" ? input.unit : "", diff --git a/src/lib/server/quotes-service.test.ts b/src/lib/server/quotes-service.test.ts index 45b8615..b839043 100644 --- a/src/lib/server/quotes-service.test.ts +++ b/src/lib/server/quotes-service.test.ts @@ -857,6 +857,15 @@ async function seedReadyQuote(overrides: { customerEmail?: string | null; customerName?: string; withPdf?: boolean; + /** + * Whether to seed at least one ready `quote_items` row. Defaults + * to true because `sendQuoteEmail` now runs a pre-flight readiness + * check (`no_items` / `items_not_ready`) before the PDF-presence + * check. Tests that need to exercise the "no PDF" path should + * set `withItem: true` so the only failure is the missing PDF. + * Tests that NEED a no-items quote can pass `withItem: false`. + */ + withItem?: boolean; } = {}) { const customer = await pbMock.collection("customers").create({ customer: overrides.customerName ?? "Acme Mining", @@ -897,6 +906,28 @@ async function seedReadyQuote(overrides: { uploaded_at: new Date().toISOString(), }); } + if (overrides.withItem !== false) { + // Seed one ready line so the pre-flight readiness check passes + // and the send path can actually reach the PDF-presence / status + // checks below. Matches the pre-flight gates in + // `diagnosePdfReadiness` (is_processed, supplier, cost>0, + // selling>0). + await pbMock.collection("quote_items").create({ + quote: q.id, + quote_number: "Q-2026-06-099", + description: "M16 hex bolt", + quantity: 1, + cost_price: 2.39, + selling_price: 3.11, + markup: 30, + supplier: "sup-acme", + is_processed: true, + sku: "M16HEX", + unit: "EA", + notes: "", + pricing_source: "manual", + }); + } return { quote: q, customer }; } diff --git a/src/lib/server/quotes-service.ts b/src/lib/server/quotes-service.ts index 18d5ac7..f8d12df 100644 --- a/src/lib/server/quotes-service.ts +++ b/src/lib/server/quotes-service.ts @@ -1573,9 +1573,31 @@ export interface SendQuoteEmailFailure { | "no_recipient" | "invalid_recipient" | "no_pdf" + | "no_items" + | "items_not_ready" | "invalid_status" | "quote_not_found"; message: string; + /** Populated when the failure was caused by a PDF that can't be + * rendered (code `placeholder_customer`, `no_items`, or + * `items_not_ready`). Lists every line that blocked the render + * with the missing fields, so the agent can present the gap back + * to the operator verbatim instead of the operator having to dig + * through the quote line by line. + * + * Same shape as the `details.issues` array from + * `POST /api/v1/quotes/[id]/pdf` — one source of truth so the UI + * and the agent render the same list. */ + details?: { + customerId?: string; + customerName?: string; + itemIds?: string[]; + issues?: Array<{ + itemId: string; + description: string; + missing: string[]; + }>; + }; } export interface SendQuoteEmailResult { @@ -1613,6 +1635,122 @@ export interface SendQuoteEmailResult { * RFC 5322 Message-ID guarantee (must be unique per message) in the * sender's hands. */ + +/** + * Diagnose why a quote's PDF can't be generated. Used by + * `sendQuoteEmail` when there's no PDF yet — surfaces the same gates + * the render route runs (placeholder customer, no items, + * items_not_ready) so the agent can present the gaps to the operator + * verbatim instead of the operator having to dig through each line. + * + * Returns a `SendQuoteEmailFailure` shape (NOT thrown) so the caller + * can pipe it straight into `markPreSendFailure`. The `details.issues` + * array uses the same per-line shape as `pdf/route.ts`'s + * `ValidationErrorBody.details.issues` — one source of truth so the + * UI's "missing fields" badges and the agent's "these N items are + * missing X" prompt render the same list. + * + * Why this is here and not in pdf/route.ts: the render route is + * request-scoped (POST handler); this helper is service-scoped and + * runs against an already-loaded quote row. No HTTP fetch loop. + */ +async function diagnosePdfReadiness( + pb: Awaited>, + quoteId: string, + quote: Quote, +): Promise { + // 1. Customer gate. Same logic as pdf/route.ts — placeholder id / + // placeholder name / missing email all block the render. + let customer: Customer | undefined; + if (quote.expand?.customer) { + customer = quote.expand.customer as Customer; + } else { + try { + customer = await pb + .collection(CUSTOMERS_COLLECTION) + .getOne(quote.customer); + } catch { + // No customer row — treat as placeholder. + } + } + if ( + !customer || + PLACEHOLDER_CUSTOMER_IDS.has(customer.id) || + PLACEHOLDER_CUSTOMER_NAMES.has((customer.customer || "").trim()) || + !customer.email + ) { + return { + code: "placeholder_customer", + message: + "This quote is still pointed at a placeholder / unnamed customer. Pick the real customer on the process page before sending.", + details: { + customerId: customer?.id, + customerName: customer?.customer || "(blank)", + }, + }; + } + + // 2. Items gate. Same "is the line ready to quote?" check pdf/route.ts + // runs: is_processed, supplier, cost_price > 0, selling_price > 0. + // Lines already moved to a continuation are excluded — those are + // no longer "this quote's items". + const allItems = await pb + .collection(QUOTE_ITEMS_COLLECTION) + .getFullList({ + filter: `quote = "${quoteId}" && (moved_to_quote = null || moved_to_quote = "")`, + sort: "created", + }); + if (allItems.length === 0) { + return { + code: "no_items", + message: + "This quote has no items yet. Add line items on the quote detail page first.", + details: { + itemIds: [], + issues: [], + }, + }; + } + const issues: Array<{ + itemId: string; + description: string; + missing: string[]; + }> = []; + for (const it of allItems) { + const missing: string[] = []; + if (!it.is_processed) missing.push("not processed"); + if (!it.supplier) missing.push("no supplier"); + if (!(Number(it.cost_price) > 0)) missing.push("cost price"); + if (!(Number(it.selling_price) > 0)) missing.push("selling price"); + if (missing.length > 0) { + issues.push({ + itemId: String(it.id), + description: it.description || "(no description)", + missing, + }); + } + } + if (issues.length > 0) { + return { + code: "items_not_ready", + message: `Cannot generate PDF: ${issues.length} item(s) are missing required fields. Price and supplier every line first.`, + details: { + itemIds: issues.map((i) => i.itemId), + issues, + }, + }; + } + + // No PDF and no detected reason — fall back to the opaque `no_pdf` + // so the agent has something to report. In practice one of the + // three gates above always fires first. + return { + code: "no_pdf", + message: + "No PDF generated for this quote yet. Generate the PDF before sending.", + }; +} + export async function sendQuoteEmail( quoteId: string, payload: SendQuoteEmailPayload @@ -1729,6 +1867,18 @@ export async function sendQuoteEmail( // ── 6. Validate PDF exists (most recent kind=pdf attachment). The // outbound send includes the PDF as the first attachment; no // PDF = nothing useful to send. + // + // Closed 2026-08-15: the previous path bailed with a bare + // "no_pdf" message and the agent had no way to tell the + // operator what was actually missing. We now run the same + // readiness check that the PDF render route does + // (placeholder customer, no items, items_not_ready) and + // surface the per-line `missing` lists back to the agent, + // which can present them to the operator verbatim. The + // operator decides whether to fix the gaps or render the + // PDF themselves; the server doesn't auto-render — Dave + // explicitly opted out of that on 2026-08-15 (don't send + // partial / stale data without operator eyes on it). const pdfs = await pb .collection(QUOTE_ATTACHMENTS_COLLECTION) .getList(1, 1, { @@ -1736,11 +1886,8 @@ export async function sendQuoteEmail( sort: "-uploaded_at", }); if (pdfs.items.length === 0) { - return markPreSendFailure(pb, quote, { - code: "no_pdf", - message: - "No PDF generated for this quote yet. Generate the PDF before sending.", - }); + const readiness = await diagnosePdfReadiness(pb, quoteId, quote); + return markPreSendFailure(pb, quote, readiness); } // ── 7. Filter selectedAttachmentIds to only those that actually @@ -2084,6 +2231,23 @@ export interface SubmitQuoteResult { /** True when the call actually flipped the status. False when the * quote was already `approval_pending` (idempotent re-submit). */ statusChanged: boolean; + /** Status the quote was in before this call (or after, if it was a + * no-op). Always populated — the agent uses it to show the diff + * in the audit log without re-fetching the quote. */ + previousStatus: Quote["status"]; + /** Echo of the actor passed in. Lands on `quotes.submitted_by` when + * the PB admin has added the column; falls back silently otherwise. + * Echoed here regardless so the agent always has the name in the + * response envelope for the audit trail. */ + submittedBy?: string; + /** ISO timestamp of the submit call. Lands on `quotes.submitted_at` + * when the PB column exists; echoed here regardless. */ + submittedAt: string; + /** Target status the call aimed for. For the full-approval path this + * is `approval_pending`; for the partial path (`partial: true` + * flag) this is `partial_approval_pending` — even when every item + * happened to be priced, since the operator's intent was partial. */ + targetStatus: "approval_pending" | "partial_approval_pending"; /** IDs of `quote_items` rows that blocked submission (selling_price * ≤ 0 on a not-moved line). Populated only when `failure !== null`. */ offendingItemIds?: string[]; @@ -2127,6 +2291,7 @@ export class SubmitQuoteError extends Error { export async function submitQuote( quoteId: string, actor?: { id: string; name: string }, + options?: { partial?: boolean }, ): Promise { if (!quoteId || !quoteId.trim()) { throw new SubmitQuoteError( @@ -2149,11 +2314,29 @@ export async function submitQuote( ); } + // Always populate previousStatus — the agent uses it for the audit + // trail even on the no-op / idempotent path. + const previousStatus = quote.status; + const submittedAt = new Date().toISOString(); + // Idempotency: already-pending quote is a no-op. Returning 200 with // statusChanged: false lets the UI safely re-submit without showing // a "double-submit" error. - if (quote.status === "approval_pending") { - return { quote, statusChanged: false }; + if (quote.status === "approval_pending" || quote.status === "partial_approval_pending") { + return { + quote, + statusChanged: false, + previousStatus, + submittedAt, + // If the caller asked for partial and the quote is already in + // partial_approval_pending, the target is partial; otherwise + // it's whatever the quote is currently in. + targetStatus: + quote.status === "partial_approval_pending" + ? "partial_approval_pending" + : "approval_pending", + ...(actor?.name ? { submittedBy: actor.name } : {}), + }; } // Only `new` quotes can transition via this tool. The full set of @@ -2171,11 +2354,6 @@ export async function submitQuote( ); } - // Gate on every line being priced. Unpriced lines are flagged with - // their ids so the operator knows what to fix. Lines that have - // already been moved to a sibling quote (partial-approval split) are - // excluded — they're not on this quote anymore. - // // Fetch every line on the quote (small N — typical quotes have // < 30 items) and filter in JS. The PB filter parser mock in // `quotes-service.test.ts` doesn't support `!= null` semantics @@ -2196,7 +2374,16 @@ export async function submitQuote( return sp <= 0 && moved === ""; }); - if (unpricedItems.length > 0) { + // Gate: only `partial: true` lets unpriced items through. The full + // submission path (the default) refuses with the existing + // `unpriced_items` 422 — no regression. The partial path is the + // missing state-machine gap that the test matrix surfaced 2026-08-15: + // without it, no tool existed to flip `new → partial_approval_pending`, + // which made the whole partial-approve + split workflow unreachable + // via MCP. The `partial: true` flag on the same `submit` tool keeps + // the state machine documented in one place — there isn't a parallel + // `submit_partial` tool to drift out of sync. + if (unpricedItems.length > 0 && !options?.partial) { const offendingItemIds = unpricedItems.map((it) => String(it.id)); throw new SubmitQuoteError( `Cannot submit: ${offendingItemIds.length} item(s) have selling_price = 0. Price every line before submitting.`, @@ -2206,24 +2393,37 @@ export async function submitQuote( ); } + // Pick the target status. Partial flag wins (operator intent) — + // even if every item happened to be priced, we honour the flag. + // Without the flag and with all items priced: full approval. + const targetStatus: "approval_pending" | "partial_approval_pending" = + options?.partial ? "partial_approval_pending" : "approval_pending"; + // Fluid write: Quote.submitted_at / submitted_by are optional fields // on the Quote type. If/when ops adds them to the PB schema, these // will start persisting. Right now they're silently dropped, which // is fine — the status flip is the canonical audit signal. const updatePayload: Record = { - status: "approval_pending", + status: targetStatus, }; if (actor?.name) { updatePayload.submitted_by = actor.name; } - updatePayload.submitted_at = new Date().toISOString(); + updatePayload.submitted_at = submittedAt; const updated = (await pb.collection(QUOTES_COLLECTION).update( quoteId, updatePayload, )) as Quote; - return { quote: updated, statusChanged: true }; + return { + quote: updated, + statusChanged: true, + previousStatus, + submittedAt, + targetStatus, + ...(actor?.name ? { submittedBy: actor.name } : {}), + }; } // ──────────────────────────────────────────────────────────────────────────────