From 0fa615305c5aed21f56b61784725f73440a198cd Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 10:58:40 +0000 Subject: [PATCH 1/3] test(export): positions reach the ERP with item corrections; overlong position values block approval (#46, test-first) Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1 --- tests/integration/export.test.ts | 41 ++++++++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/tests/integration/export.test.ts b/tests/integration/export.test.ts index bcdbaa0..84a24d1 100644 --- a/tests/integration/export.test.ts +++ b/tests/integration/export.test.ts @@ -13,7 +13,7 @@ import { syntheticExtractResponse } from "@/features/extraction/fixtures"; import { getActor, type Actor } from "@/features/identity"; import { QUEUES, reprocessRequest } from "@/features/jobs"; import { createRequest, getRequest, lockRequest, transitionRequest } from "@/features/requests"; -import { approveRequest, correctField, currentFieldValues } from "@/features/review"; +import { approveRequest, correctField, currentFieldValues, currentLineItemValues, ReviewRefused } from "@/features/review"; import { createTenancy, type Tenancy } from "@/features/tenancy"; import { companyWithAdmin, createStack, invitedUser, type Stack } from "./helpers/stack"; @@ -43,11 +43,19 @@ describe("export: approved requests reach the ERP exactly once", () => { if (request.signal.aborted) throw request.signal.reason; return response; }; - return { tenancy, boss, erp: createErpClient({ baseUrl: "http://web/api/erp-mock", token: TOKEN, timeoutMs, fetch: fetchImpl }), fieldValues: currentFieldValues }; + return { tenancy, boss, erp: createErpClient({ baseUrl: "http://web/api/erp-mock", token: TOKEN, timeoutMs, fetch: fetchImpl }), fieldValues: currentFieldValues, lineItemValues: currentLineItemValues }; } + const found = (value: string, segmentId: string) => ({ value, status: "found", evidence: { segmentId, quote: value }, modelStatus: "found", reason: null }) as const; + const missingValue = { value: null, status: "missing", evidence: null, modelStatus: "missing", reason: null } as const; + /** Two synthetic positions whose quotes sit in the fixture's segments. */ + const POSITIONS = [ + { index: 0, description: found("Musterbau Beispiel GmbH", "s2"), quantity: found("15.10.2026", "s4"), unit: { ...missingValue }, material: { ...missingValue }, dimensions: { ...missingValue } }, + { index: 1, description: found("Erika Beispiel", "s3"), quantity: { ...missingValue }, unit: { ...missingValue }, material: { ...missingValue }, dimensions: { ...missingValue } }, + ]; + /** A request approved through the review module, with its export job moved to the test queue. */ - async function approved(actor: Actor) { + async function approved(actor: Actor, lineItems: typeof POSITIONS = [], beforeApproval: (requestId: string) => Promise = async () => {}) { const requestId = randomUUID(); const documentId = randomUUID(); await tenancy.withTenant(actor.companyId, async (tx) => { @@ -56,10 +64,11 @@ describe("export: approved requests reach the ERP exactly once", () => { { id: documentId, requestId, filename: "anfrage.eml", contentType: "message/rfc822", kind: "eml", sizeBytes: 10, sha256: "x".repeat(64), storageKey: `${actor.companyId}/${requestId}/${documentId}` }, ]); const row = await transitionRequest(tx, (await lockRequest(tx, requestId))!, "processing.started", { attempts: 1 }); - await persistExtractionRun(tx, { requestId, jobId: randomUUID(), outcomes: [{ documentId, response: syntheticExtractResponse(documentId) }] }); + await persistExtractionRun(tx, { requestId, jobId: randomUUID(), outcomes: [{ documentId, response: syntheticExtractResponse(documentId, {}, lineItems) }] }); await transitionRequest(tx, row, "processing.succeeded"); }); await correctField(tenancy, actor, requestId, "company", "Musterbau Beispiel GmbH & Co. KG"); + await beforeApproval(requestId); await approveRequest({ tenancy, boss }, actor, requestId); await stack.database.pool.query("delete from pgboss.job where name = $1 and singleton_key = $2", [QUEUES.exportRequest, requestId]); const data = { requestId, companyId: actor.companyId }; @@ -127,6 +136,30 @@ describe("export: approved requests reach the ERP exactly once", () => { expect(sent).toMatchObject({ requestId, subject: "Anfrage Flansche DN 100", fields: { company: "Musterbau Beispiel GmbH & Co. KG", contactPerson: "Erika Beispiel" } }); expect(Date.parse((sent as { approvedAt: string }).approvedAt)).not.toBeNaN(); + // Without positions the body stays exactly what contract v1.0 sent (#46: the field is optional). + expect(sent).not.toHaveProperty("lineItems"); + }); + + it("sends the reviewed positions in document order: an item correction wins over the extraction (#46)", async () => { + const deps = depsWith(); + const { requestId, job } = await approved(clerk, POSITIONS, (id) => correctField(tenancy, clerk, id, "quantity", "1300", 0)); + let sent: unknown; + const erp = deps.erp; + await exportRequestJob({ ...deps, erp: { submit: (body) => ((sent = body), erp.submit(body)) } }, job); + + expect((sent as { lineItems: unknown }).lineItems).toEqual([ + { position: 1, description: "Musterbau Beispiel GmbH", quantity: "1300", unit: null, material: null, dimensions: null }, + { position: 2, description: "Erika Beispiel", quantity: null, unit: null, material: null, dimensions: null }, + ]); + expect((await requestOf(clerk, requestId))?.status).toBe("EXPORTED"); + expect(mock.created()).toBe(1); + }); + + it("refuses the approval while a position value would break the ERP contract (#46)", async () => { + const tooLong = [{ ...POSITIONS[0]!, description: found("Musterbau Beispiel GmbH", "s2") }]; + await expect( + approved(clerk, tooLong, (id) => correctField(tenancy, clerk, id, "description", "x".repeat(501), 0)), + ).rejects.toThrow(ReviewRefused); }); it("duplicate delivery of the same job: the row lock lets one export, the other sees EXPORTED and never calls the ERP", async () => { From 6587cab1d3452d6142ebc5b0bea04e4c66f9b73e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 11:03:20 +0000 Subject: [PATCH 2/3] feat(export): send reviewed positions to the ERP (contract 1.1.0) (#46) Adds the optional lineItems array to the ERP contract, builds it from the latest run's positions with item corrections, and refuses the approval when a position value, the position count or the body size would break the contract. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1 --- CHANGELOG.md | 3 ++ contracts/erp-export.openapi.yaml | 38 ++++++++++++++- docs/technical/api.md | 6 +++ src/features/export/contract.ts | 18 ++++++- src/features/export/erp-export.contract.ts | 13 +++++ src/features/export/export-job.ts | 6 ++- src/features/export/index.ts | 4 +- src/features/export/payload.test.ts | 30 ++++++++++++ src/features/export/payload.ts | 55 +++++++++++++++++----- src/features/review/index.ts | 1 + src/features/review/review.ts | 25 +++++++++- src/worker.ts | 4 +- tests/integration/duplicates.test.ts | 4 +- tests/integration/export.test.ts | 10 ++-- tests/integration/log-privacy.test.ts | 4 +- tests/integration/request-list.test.ts | 4 +- 16 files changed, 194 insertions(+), 31 deletions(-) create mode 100644 src/features/export/payload.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index d461102..92c224d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ This file records what changes **in the product** – process and session state ## [Unreleased] ### Added +- ERP export sends the reviewed positions (description, quantity, unit, material, dimensions) with each + approved request – ERP contract 1.1.0, additive and optional. A position value the ERP would refuse + blocks the approval so the clerk can still correct it. - Observability: structured JSON logs (pino) with IDs and codes only, correlated by the request id from web through worker to the AI service; `/api/health` also shows whether the AI service is reachable and how many jobs are waiting. diff --git a/contracts/erp-export.openapi.yaml b/contracts/erp-export.openapi.yaml index aab43d8..e5bc3b4 100644 --- a/contracts/erp-export.openapi.yaml +++ b/contracts/erp-export.openapi.yaml @@ -1,7 +1,7 @@ openapi: 3.1.0 info: title: RequestFlow ERP export - version: 1.0.0 + version: 1.1.0 description: | Port between RequestFlow and the customer's ERP (ADR-0001 D9). In the pilot the ERP mock (`/api/erp-mock`, active only with `ERP_MOCK_ENABLED=true`) implements it. @@ -9,6 +9,9 @@ info: Idempotency: `Idempotency-Key` is the RequestFlow request id. A repeated call with the same key and the same body returns the SAME `erpReference` (status 200 instead of 201) – never a second record. The same key with a different body is refused with 409. + + 1.1.0 (#46): optional `lineItems` – the reviewed positions in document order. Additive and backwards + compatible: a request without positions sends exactly the 1.0.0 body (the field is omitted). servers: - url: /api/erp-mock paths: @@ -114,6 +117,39 @@ components: description: ISO date (YYYY-MM-DD) when the value is a date, else the reviewed text. type: [string, "null"] maxLength: 500 + lineItems: + description: Reviewed positions in document order (1.1.0). Omitted when the request has none. + type: array + minItems: 1 + maxItems: 200 + items: + $ref: "#/components/schemas/LineItem" + LineItem: + type: object + additionalProperties: false + required: [position, description, quantity, unit, material, dimensions] + properties: + position: + description: 1-based position in the document order. + type: integer + minimum: 1 + description: + type: [string, "null"] + maxLength: 500 + quantity: + description: Plain decimal with a dot, no grouping ("1250", "2.5"), else the reviewed text. + type: [string, "null"] + maxLength: 500 + unit: + description: mm, cm, m, kg, t or pcs when known, else the unit as written. + type: [string, "null"] + maxLength: 500 + material: + type: [string, "null"] + maxLength: 500 + dimensions: + type: [string, "null"] + maxLength: 500 QuoteRequestReceipt: type: object additionalProperties: false diff --git a/docs/technical/api.md b/docs/technical/api.md index 56e9c15..c8dc078 100644 --- a/docs/technical/api.md +++ b/docs/technical/api.md @@ -48,6 +48,12 @@ flag or without `ERP_TOKEN` the route answers 404. `Authorization: Bearer ` (UUID, must equal `requestId` in the body), JSON body ≤ 64 KiB with a `Content-Length` (411/413 otherwise). +Contract 1.1.0 (#46) adds the optional `lineItems` array (1–200 positions: `position` ≥ 1 plus +`description`, `quantity`, `unit`, `material`, `dimensions`, each `string | null` ≤ 500 characters). It is +sent only when the request has positions, so a 1.0.0 receiver sees an unchanged body for requests without +positions. Values that would break these limits (or push the body over 64 KiB) block the approval, where +the clerk can still correct them – nothing is cut silently. + | Status | Meaning | |---|---| | 201 | stored; body `{ erpReference, requestId, receivedAt }` | diff --git a/src/features/export/contract.ts b/src/features/export/contract.ts index 39f28de..3be09db 100644 --- a/src/features/export/contract.ts +++ b/src/features/export/contract.ts @@ -6,14 +6,26 @@ import type { components } from "./erp-export.contract"; // generated types and these schemas drift apart. export type QuoteRequest = components["schemas"]["QuoteRequest"]; export type QuoteRequestReceipt = components["schemas"]["QuoteRequestReceipt"]; +export type QuoteRequestLineItem = components["schemas"]["LineItem"]; const text = (max: number) => z.string().max(max).nullable(); +export const lineItemSchema = z.strictObject({ + position: z.int().min(1), + description: text(500), + quantity: text(500), + unit: text(500), + material: text(500), + dimensions: text(500), +}); + export const quoteRequestSchema = z.strictObject({ requestId: z.uuid(), subject: text(300), approvedAt: z.iso.datetime({ offset: true }), fields: z.strictObject({ company: text(500), contactPerson: text(500), requestedDeliveryDate: text(500) }), + // 1.1.0 (#46): optional and omitted when there are no positions – older receivers see the 1.0.0 body. + lineItems: z.array(lineItemSchema).min(1).max(200).optional(), }); export const receiptSchema = z.strictObject({ @@ -27,4 +39,8 @@ export const errorSchema = z.strictObject({ error: z.strictObject({ code: z.enum // Compile-time drift guard: parsed values must be assignable to the generated contract types. type Assignable = A extends B ? true : never; -export const contractGuards: [Assignable, QuoteRequest>, Assignable, QuoteRequestReceipt>] = [true, true]; +export const contractGuards: [ + Assignable, QuoteRequest>, + Assignable, QuoteRequestReceipt>, + Assignable, QuoteRequestLineItem>, +] = [true, true, true]; diff --git a/src/features/export/erp-export.contract.ts b/src/features/export/erp-export.contract.ts index 294febd..ac5f96a 100644 --- a/src/features/export/erp-export.contract.ts +++ b/src/features/export/erp-export.contract.ts @@ -36,6 +36,19 @@ export interface components { /** @description ISO date (YYYY-MM-DD) when the value is a date, else the reviewed text. */ requestedDeliveryDate: string | null; }; + /** @description Reviewed positions in document order (1.1.0). Omitted when the request has none. */ + lineItems?: components["schemas"]["LineItem"][]; + }; + LineItem: { + /** @description 1-based position in the document order. */ + position: number; + description: string | null; + /** @description Plain decimal with a dot, no grouping ("1250", "2.5"), else the reviewed text. */ + quantity: string | null; + /** @description mm, cm, m, kg, t or pcs when known, else the unit as written. */ + unit: string | null; + material: string | null; + dimensions: string | null; }; QuoteRequestReceipt: { erpReference: string; diff --git a/src/features/export/export-job.ts b/src/features/export/export-job.ts index f628670..33ef8f4 100644 --- a/src/features/export/export-job.ts +++ b/src/features/export/export-job.ts @@ -4,7 +4,7 @@ import { logEvent } from "@/features/observability"; import { canTransition, lockRequest, recordExportRetry, transitionRequest } from "@/features/requests"; import type { Tenancy } from "@/features/tenancy"; import { ErpExportError, type ErpExporter } from "./erp-client"; -import { buildQuoteRequest, ExportNotPossible, type FieldValues } from "./payload"; +import { buildQuoteRequest, ExportNotPossible, type FieldValues, type LineItemValues } from "./payload"; import { ensureExportRecord, markExportSucceeded, recordExportAttemptFailure } from "./repository"; // Export handler (ADR-0001 D9). Exactly once from three guards together: @@ -17,6 +17,8 @@ export interface ExportDeps { tenancy: Tenancy; erp: ErpExporter; fieldValues: FieldValues; + /** Reviewed positions (#46) – required, so no caller drops them silently. */ + lineItemValues: LineItemValues; } export interface ExportDrainDeps extends ExportDeps { @@ -48,7 +50,7 @@ export async function exportRequestJob(deps: ExportDeps, job: Job): Promise<"exp return "skipped"; } await ensureExportRecord(tx, requestId); - const payload = await buildQuoteRequest(tx, request, deps.fieldValues); + const payload = await buildQuoteRequest(tx, request, deps.fieldValues, deps.lineItemValues); const { receipt, replay } = await deps.erp.submit(payload); await markExportSucceeded(tx, requestId, receipt.erpReference); await transitionRequest(tx, request, "export.succeeded", { errorStage: null, errorMessage: null, nextRetryAt: null }); diff --git a/src/features/export/index.ts b/src/features/export/index.ts index 4a0f614..e4ab7fd 100644 --- a/src/features/export/index.ts +++ b/src/features/export/index.ts @@ -1,7 +1,7 @@ // Public API of the `export` module: ERP port + REST adapter, idempotency. // Other modules import only from this file (dependency-cruiser, ADR-0001 D1). -export { errorCodes, errorSchema, quoteRequestSchema, receiptSchema, type QuoteRequest, type QuoteRequestReceipt } from "./contract"; +export { errorCodes, errorSchema, lineItemSchema, quoteRequestSchema, receiptSchema, type QuoteRequest, type QuoteRequestLineItem, type QuoteRequestReceipt } from "./contract"; export { createErpClient, ErpExportError, type ErpExporter, type ErpSettings } from "./erp-client"; export { describeExportFailure, drainExports, exportRequestJob, type ExportDeps, type ExportDrainDeps, type ExportDrainOptions, type ExportDrainResult } from "./export-job"; -export { buildQuoteRequest, ERP_LIMITS, ExportNotPossible, exportLimitViolations, type FieldValues } from "./payload"; +export { buildQuoteRequest, ERP_LIMITS, ExportNotPossible, exportLimitViolations, type FieldValues, type LineItemValues } from "./payload"; export { getExportRecord, listExportRecords, type ExportRecord } from "./repository"; diff --git a/src/features/export/payload.test.ts b/src/features/export/payload.test.ts new file mode 100644 index 0000000..df9b3f0 --- /dev/null +++ b/src/features/export/payload.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from "vitest"; +import { ERP_LIMITS, exportLimitViolations } from "./payload"; + +const header = { company: "Musterbau Beispiel GmbH", contact_person: "Erika Beispiel", requested_delivery_date: "2026-10-15" }; +const item = (description: string | null = "Flansch DN 100") => ({ description, quantity: "1250", unit: "pcs", material: null, dimensions: null }); + +describe("exportLimitViolations (ERP contract 1.1.0, #46)", () => { + it("accepts a request with and without positions inside the limits", () => { + expect(exportLimitViolations("Anfrage", header)).toEqual([]); + expect(exportLimitViolations("Anfrage", header, [item(), item("Dichtung DN 100")])).toEqual([]); + }); + + it("names an overlong position value by position and field", () => { + expect(exportLimitViolations("Anfrage", header, [item(), item("x".repeat(ERP_LIMITS.field + 1))])).toEqual(["item 2 description"]); + }); + + it("refuses more positions than the ERP accepts", () => { + const items = Array.from({ length: ERP_LIMITS.lineItems + 1 }, () => item("F")); + expect(exportLimitViolations("Anfrage", header, items)).toContain("lineItems"); + }); + + it("refuses a body the ERP would reject as too large, even when every value is within its own limit", () => { + const items = Array.from({ length: ERP_LIMITS.lineItems }, () => ({ description: "d".repeat(400), quantity: "q".repeat(400), unit: null, material: null, dimensions: null })); + expect(exportLimitViolations("Anfrage", header, items)).toEqual(["body"]); + }); + + it("still ignores header fields the ERP never receives", () => { + expect(exportLimitViolations("Anfrage", { ...header, additional_requirements: "x".repeat(ERP_LIMITS.field + 1) })).toEqual([]); + }); +}); diff --git a/src/features/export/payload.ts b/src/features/export/payload.ts index f866ae9..4fabc64 100644 --- a/src/features/export/payload.ts +++ b/src/features/export/payload.ts @@ -1,11 +1,14 @@ import { listAuditEvents } from "@/features/audit"; import type { RequestRow } from "@/features/requests"; import type { TenantTx } from "@/features/tenancy"; -import { quoteRequestSchema, type QuoteRequest } from "./contract"; +import { quoteRequestSchema, type QuoteRequest, type QuoteRequestLineItem } from "./contract"; /** Reviewed values of a request (the latest correction, else the extraction) – injected by the caller. */ export type FieldValues = (tx: TenantTx, requestId: string) => Promise>; +/** Reviewed positions in document order, one record of item fields each – injected by the caller (#46). */ +export type LineItemValues = (tx: TenantTx, requestId: string) => Promise>>; + export class ExportNotPossible extends Error { constructor(message: string) { super(message); @@ -13,35 +16,63 @@ export class ExportNotPossible extends Error { } } -/** ERP limits of the contract (maxLength of subject and fields). */ -export const ERP_LIMITS = { subject: 300, field: 500 } as const; +/** ERP limits of the contract: maxLength of subject and fields, maxItems of positions, body size. */ +export const ERP_LIMITS = { subject: 300, field: 500, lineItems: 200, bodyBytes: 64 * 1024 } as const; /** Header fields that go to the ERP (contracts/erp-export.openapi.yaml). */ const EXPORTED_FIELDS = ["company", "contact_person", "requested_delivery_date"] as const; +/** Position fields that go to the ERP (contract 1.1.0, #46). */ +const EXPORTED_ITEM_FIELDS = ["description", "quantity", "unit", "material", "dimensions"] as const; + +function toLineItems(items: Array>): QuoteRequestLineItem[] { + return items.map((item, index) => ({ + position: index + 1, + description: item.description ?? null, + quantity: item.quantity ?? null, + unit: item.unit ?? null, + material: item.material ?? null, + dimensions: item.dimensions ?? null, + })); +} /** - * Header fields whose reviewed value would break the ERP contract (too long). Checked at approval, so - * the clerk can still correct the value – after approval corrections are closed (#9 review). + * Values whose reviewed state would break the ERP contract (too long, too many positions, body too + * large). Checked at approval, so the clerk can still correct them – after approval corrections are + * closed (#9 review). Returns field keys, `item N ` for positions, `lineItems` or `body`. */ -export function exportLimitViolations(subject: string | null, values: Record): string[] { - // Only the fields the ERP receives (contract v1) – a long free text that is never exported must - // not block the approval (#22 review). - const fields = EXPORTED_FIELDS.filter((key) => { +export function exportLimitViolations(subject: string | null, values: Record, items: Array> = []): string[] { + // Only the fields the ERP receives – a long free text that is never exported must not block the + // approval (#22 review). + const violations: string[] = EXPORTED_FIELDS.filter((key) => { const value = values[key]; return value != null && value.length > ERP_LIMITS.field; }); - return subject !== null && subject.length > ERP_LIMITS.subject ? ["subject", ...fields] : fields; + if (subject !== null && subject.length > ERP_LIMITS.subject) violations.unshift("subject"); + items.forEach((item, index) => { + for (const key of EXPORTED_ITEM_FIELDS) { + const value = item[key]; + if (value != null && value.length > ERP_LIMITS.field) violations.push(`item ${index + 1} ${key}`); + } + }); + if (items.length > ERP_LIMITS.lineItems) violations.push("lineItems"); + // The ERP refuses bodies over 64 KiB before reading them; a generous estimate of the envelope + // (ids, timestamps, keys) keeps the check on the safe side. + const estimate = JSON.stringify({ subject, values: EXPORTED_FIELDS.map((key) => values[key] ?? null), items: toLineItems(items) }); + if (Buffer.byteLength(estimate, "utf8") + 1024 > ERP_LIMITS.bodyBytes) violations.push("body"); + return violations; } /** * The ERP payload. Deterministic for an approved request (values are frozen after approval, the * approval time comes from its audit event), so a retry sends the same body under the same key. + * Positions are sent only when there are any – a request without positions keeps the 1.0.0 body. * A payload outside the contract (e.g. an overlong value) is refused here – never cut silently. */ -export async function buildQuoteRequest(tx: TenantTx, request: RequestRow, fieldValues: FieldValues): Promise { +export async function buildQuoteRequest(tx: TenantTx, request: RequestRow, fieldValues: FieldValues, lineItemValues: LineItemValues): Promise { const approval = (await listAuditEvents(tx, "request", request.id)).filter((event) => event.action === "request.approved").at(-1); if (!approval) throw new ExportNotPossible("approval event missing"); const values = await fieldValues(tx, request.id); + const items = await lineItemValues(tx, request.id); const payload: QuoteRequest = { requestId: request.id, subject: request.subject ?? null, @@ -51,7 +82,9 @@ export async function buildQuoteRequest(tx: TenantTx, request: RequestRow, field contactPerson: values.contact_person ?? null, requestedDeliveryDate: values.requested_delivery_date ?? null, }, + ...(items.length > 0 ? { lineItems: toLineItems(items) } : {}), }; if (!quoteRequestSchema.safeParse(payload).success) throw new ExportNotPossible("payload outside the ERP contract"); + if (Buffer.byteLength(JSON.stringify(payload), "utf8") > ERP_LIMITS.bodyBytes) throw new ExportNotPossible("payload larger than the ERP accepts"); return payload; } diff --git a/src/features/review/index.ts b/src/features/review/index.ts index 0695b5f..ddf44fe 100644 --- a/src/features/review/index.ts +++ b/src/features/review/index.ts @@ -7,6 +7,7 @@ export { correctField, correctionHistory, currentFieldValues, + currentLineItemValues, FIELD_LABELS, loadReview, REJECTION_REASON_MAX, diff --git a/src/features/review/review.ts b/src/features/review/review.ts index 93f5a2f..eb4d25b 100644 --- a/src/features/review/review.ts +++ b/src/features/review/review.ts @@ -108,6 +108,27 @@ export async function currentFieldValues(tx: TenantTx, requestId: string): Promi ); } +/** + * The reviewed positions in document order (#46): per item field the latest correction made on the + * current run, else the extracted value. Like the review view, a position correction made before a + * newer run is not mapped onto it. Runs in the caller's tenant transaction (export under the row lock). + */ +export async function currentLineItemValues(tx: TenantTx, requestId: string): Promise>> { + const extraction = await latestRun(tx, requestId); + if (!extraction) return []; + const corrections = await currentCorrections(tx, requestId); + return extraction.lineItems.map((item) => { + const extracted = new Map(item.fields.map((field) => [field.fieldKey, field.value])); + return Object.fromEntries( + ITEM_FIELDS.map((key) => { + const correction = corrections.get(correctionKey(key, item.itemIndex)); + const current = correction && correction.createdAt >= extraction.run.createdAt ? correction : undefined; + return [key, current ? current.newValue : (extracted.get(key) ?? null)]; + }), + ); + }); +} + export async function loadReview(tenancy: Tenancy, actor: Actor, requestId: string): Promise { authorize(actor, "requests.process"); return tenancy.withTenant(actor.companyId, async (tx) => { @@ -210,7 +231,9 @@ export async function approveRequest(deps: { tenancy: Tenancy; boss: JobSender } // A possible duplicate is approved only after a clerk decided it is not one (#27) – nothing is // exported twice by accident. if (request.possibleDuplicate && request.duplicateDecision !== "distinct") throw new ReviewRefused("duplicate_undecided"); - if (exportLimitViolations(request.subject, await currentFieldValues(tx, requestId)).length > 0) throw new ReviewRefused("value_too_long"); + if (exportLimitViolations(request.subject, await currentFieldValues(tx, requestId), await currentLineItemValues(tx, requestId)).length > 0) { + throw new ReviewRefused("value_too_long"); + } await transitionRequest(tx, request, "approve"); await enqueueRequestExport(deps.boss, tx, requestId); await recordAudit(tx, { actorUserId: actor.userId, action: "request.approved", entityType: "request", entityId: requestId }); diff --git a/src/worker.ts b/src/worker.ts index d574298..8611956 100644 --- a/src/worker.ts +++ b/src/worker.ts @@ -9,7 +9,7 @@ import { createErpClient, drainExports } from "@/features/export"; import { createAiServiceClient } from "@/features/extraction"; import { assertProcessingBudget, drain } from "@/features/jobs"; import { logEvent } from "@/features/observability"; -import { currentFieldValues } from "@/features/review"; +import { currentFieldValues, currentLineItemValues } from "@/features/review"; import { S3BlobStore } from "@/features/storage"; import { createTenancy } from "@/features/tenancy"; @@ -27,7 +27,7 @@ async function main(): Promise { const tenancy = createTenancy(database.db); const deps = { tenancy, storage, ai, boss }; // The export reads the reviewed values through the review module (injected – no module cycle). - const exportDeps = { tenancy, erp, boss, fieldValues: currentFieldValues }; + const exportDeps = { tenancy, erp, boss, fieldValues: currentFieldValues, lineItemValues: currentLineItemValues }; let running = true; const stop = (signal: string) => { diff --git a/tests/integration/duplicates.test.ts b/tests/integration/duplicates.test.ts index 7f7923d..e59e037 100644 --- a/tests/integration/duplicates.test.ts +++ b/tests/integration/duplicates.test.ts @@ -13,7 +13,7 @@ import { getActor, type Actor } from "@/features/identity"; import { processRequestJob, QUEUES, reprocessRequest, ReprocessRefused } from "@/features/jobs"; import { latestRun } from "@/features/extraction"; import { createRequest, getRequest, lockRequest, transitionRequest } from "@/features/requests"; -import { approveRequest, confirmNotDuplicate, currentFieldValues, rejectAsDuplicate, ReviewRefused } from "@/features/review"; +import { approveRequest, confirmNotDuplicate, currentFieldValues, currentLineItemValues, rejectAsDuplicate, ReviewRefused } from "@/features/review"; import { createTenancy, type Tenancy } from "@/features/tenancy"; import { companyWithAdmin, createStack, invitedUser, type Stack } from "./helpers/stack"; @@ -94,7 +94,7 @@ describe("duplicate handling", () => { await expect(reprocessRequest({ tenancy, boss }, clerk, duplicateId)).rejects.toBeInstanceOf(ReprocessRefused); const mock = createErpMock({ token: "t".repeat(24), store: new MemoryMockStore() }); const erp = createErpClient({ baseUrl: "http://erp", token: "t".repeat(24), timeoutMs: 500, fetch: async (input, init) => mock.handle(new Request(input, init)) }); - expect(await exportRequestJob({ tenancy, erp, fieldValues: currentFieldValues }, { id: randomUUID(), data: { requestId: duplicateId, companyId: clerk.companyId } })).toBe("skipped"); + expect(await exportRequestJob({ tenancy, erp, fieldValues: currentFieldValues, lineItemValues: currentLineItemValues }, { id: randomUUID(), data: { requestId: duplicateId, companyId: clerk.companyId } })).toBe("skipped"); expect(mock.created()).toBe(0); expect(await exportJobs(duplicateId)).toBe(0); }); diff --git a/tests/integration/export.test.ts b/tests/integration/export.test.ts index 84a24d1..c51803f 100644 --- a/tests/integration/export.test.ts +++ b/tests/integration/export.test.ts @@ -156,10 +156,10 @@ describe("export: approved requests reach the ERP exactly once", () => { }); it("refuses the approval while a position value would break the ERP contract (#46)", async () => { - const tooLong = [{ ...POSITIONS[0]!, description: found("Musterbau Beispiel GmbH", "s2") }]; - await expect( - approved(clerk, tooLong, (id) => correctField(tenancy, clerk, id, "description", "x".repeat(501), 0)), - ).rejects.toThrow(ReviewRefused); + // Corrections are capped at 500 characters, so an overlong position value can only come from the + // extraction; the approval must refuse it while the clerk can still correct it. + const tooLong = [{ ...POSITIONS[0]!, description: { ...found("Musterbau Beispiel GmbH", "s2"), value: "x".repeat(501) } }]; + await expect(approved(clerk, tooLong)).rejects.toThrow(ReviewRefused); }); it("duplicate delivery of the same job: the row lock lets one export, the other sees EXPORTED and never calls the ERP", async () => { @@ -181,7 +181,7 @@ describe("export: approved requests reach the ERP exactly once", () => { const { requestId, job } = await approved(clerk); const values = await tenancy.withTenant(clerk.companyId, async (tx) => { const { buildQuoteRequest } = await import("@/features/export"); - return buildQuoteRequest(tx, (await getRequest(tx, requestId))!, currentFieldValues); + return buildQuoteRequest(tx, (await getRequest(tx, requestId))!, currentFieldValues, currentLineItemValues); }); const first = await deps.erp.submit(values); diff --git a/tests/integration/log-privacy.test.ts b/tests/integration/log-privacy.test.ts index a9a9581..30ff950 100644 --- a/tests/integration/log-privacy.test.ts +++ b/tests/integration/log-privacy.test.ts @@ -14,7 +14,7 @@ import { submitUpload } from "@/features/intake"; import { drain } from "@/features/jobs"; import { captureLogs } from "@/features/observability"; import { getRequest } from "@/features/requests"; -import { approveRequest, correctField, currentFieldValues } from "@/features/review"; +import { approveRequest, correctField, currentFieldValues, currentLineItemValues } from "@/features/review"; import { S3BlobStore } from "@/features/storage"; import { createTenancy, type Tenancy } from "@/features/tenancy"; import { companyWithAdmin, createStack, invitedUser, type Stack } from "./helpers/stack"; @@ -81,7 +81,7 @@ describe("logs of a full synthetic run carry no content and no personal data", ( await until(async () => (await statusOf(requestId)) === "REVIEW", () => drain({ tenancy, storage, ai, boss }, { maxMs: 1_000 })); await correctField(tenancy, clerk, requestId, "company", "Korrigierte Firma GmbH"); await approveRequest({ tenancy, boss }, clerk, requestId); - await until(async () => (await statusOf(requestId)) === "EXPORTED", () => drainExports({ tenancy, erp, boss, fieldValues: currentFieldValues }, { maxMs: 1_000 })); + await until(async () => (await statusOf(requestId)) === "EXPORTED", () => drainExports({ tenancy, erp, boss, fieldValues: currentFieldValues, lineItemValues: currentLineItemValues }, { maxMs: 1_000 })); } finally { restore(); } diff --git a/tests/integration/request-list.test.ts b/tests/integration/request-list.test.ts index b1e88e0..0161314 100644 --- a/tests/integration/request-list.test.ts +++ b/tests/integration/request-list.test.ts @@ -7,7 +7,7 @@ import { createErpMock, MemoryMockStore } from "@/features/erp-mock"; import { createErpClient, drainExports, listExportRecords } from "@/features/export"; import { persistExtractionRun } from "@/features/extraction"; import { syntheticExtractResponse } from "@/features/extraction/fixtures"; -import { approveRequest, currentFieldValues } from "@/features/review"; +import { approveRequest, currentFieldValues, currentLineItemValues } from "@/features/review"; import { getActor, type Actor } from "@/features/identity"; import { QUEUES } from "@/features/jobs"; import { createRequest, getRequest, listRequests, lockRequest, transitionRequest } from "@/features/requests"; @@ -118,7 +118,7 @@ describe("request list: errors, retries, filters and reprocessing (#26)", () => const mock = createErpMock({ token: "e".repeat(24), store: new MemoryMockStore(), faults: ["503"] }); const erp = createErpClient({ baseUrl: "http://erp", token: "e".repeat(24), timeoutMs: 1_000, fetch: async (input, init) => mock.handle(new Request(input, init)) }); - await drainExports({ tenancy, erp, boss, fieldValues: currentFieldValues }, { maxMs: 2_000, queues }); + await drainExports({ tenancy, erp, boss, fieldValues: currentFieldValues, lineItemValues: currentLineItemValues }, { maxMs: 2_000, queues }); const request = (await tenancy.withTenant(clerk.companyId, (tx) => getRequest(tx, id)))!; const record = (await tenancy.withTenant(clerk.companyId, (tx) => listExportRecords(tx, [id]))).get(id); From 3efdce3b58ab8895fde98b41c040d7ebe579614c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 11:12:50 +0000 Subject: [PATCH 3/3] fix(review): own refusal for requests too large for the ERP; tests for stale item corrections (#46 review) Too many positions or a too large body cannot be fixed by a correction, so the clerk gets export_too_large instead of value_too_long. The export test now checks the refusal codes and that the exported positions equal what the review shows after a newer run. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DJ5vaKvTYiMvdngT4d3xo1 --- src/app/requests/[id]/messages.ts | 1 + src/features/review/review.ts | 9 ++++++--- tests/integration/export.test.ts | 32 ++++++++++++++++++++++++++++--- 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/src/app/requests/[id]/messages.ts b/src/app/requests/[id]/messages.ts index db90406..91be866 100644 --- a/src/app/requests/[id]/messages.ts +++ b/src/app/requests/[id]/messages.ts @@ -13,6 +13,7 @@ export const ERROR_MESSAGES: Record = { reason_missing: "Bitte einen Grund für die Ablehnung angeben.", reason_too_long: "Der Grund ist zu lang (höchstens 1000 Zeichen).", value_too_long: "Ein Wert ist zu lang für den ERP-Export (höchstens 500 Zeichen) – bitte zuerst korrigieren.", + export_too_large: "Die Anfrage ist zu umfangreich für den ERP-Export (höchstens 200 Positionen, 64 KiB) – bitte ablehnen und direkt im ERP erfassen.", duplicate_undecided: "Mögliches Duplikat – bitte zuerst entscheiden, ob es eine eigenständige Anfrage ist.", not_a_possible_duplicate: "Für diese Anfrage steht keine Duplikat-Entscheidung an.", forbidden: "Keine Berechtigung.", diff --git a/src/features/review/review.ts b/src/features/review/review.ts index eb4d25b..a65e895 100644 --- a/src/features/review/review.ts +++ b/src/features/review/review.ts @@ -71,6 +71,7 @@ export type ReviewRefusal = | "reason_missing" | "reason_too_long" | "value_too_long" + | "export_too_large" | "duplicate_undecided" | "not_a_possible_duplicate"; @@ -231,9 +232,11 @@ export async function approveRequest(deps: { tenancy: Tenancy; boss: JobSender } // A possible duplicate is approved only after a clerk decided it is not one (#27) – nothing is // exported twice by accident. if (request.possibleDuplicate && request.duplicateDecision !== "distinct") throw new ReviewRefused("duplicate_undecided"); - if (exportLimitViolations(request.subject, await currentFieldValues(tx, requestId), await currentLineItemValues(tx, requestId)).length > 0) { - throw new ReviewRefused("value_too_long"); - } + const violations = exportLimitViolations(request.subject, await currentFieldValues(tx, requestId), await currentLineItemValues(tx, requestId)); + // A single value the clerk can shorten; too many positions or a too large body cannot be fixed by + // a correction – the clerk is told so instead of being asked to correct something (#46 review). + if (violations.some((key) => key !== "lineItems" && key !== "body")) throw new ReviewRefused("value_too_long"); + if (violations.length > 0) throw new ReviewRefused("export_too_large"); await transitionRequest(tx, request, "approve"); await enqueueRequestExport(deps.boss, tx, requestId); await recordAudit(tx, { actorUserId: actor.userId, action: "request.approved", entityType: "request", entityId: requestId }); diff --git a/tests/integration/export.test.ts b/tests/integration/export.test.ts index c51803f..a6e429d 100644 --- a/tests/integration/export.test.ts +++ b/tests/integration/export.test.ts @@ -5,7 +5,7 @@ import { loadConfig } from "@/config/env"; import { sendInTransaction } from "@/db/job-queue"; import { createJobQueue, installJobQueues } from "@/db/job-queue-client"; import { listAuditEvents } from "@/features/audit"; -import { insertDocuments } from "@/features/documents"; +import { insertDocuments, listDocuments } from "@/features/documents"; import { createErpMock, MemoryMockStore, type ErpMock, type MockFault } from "@/features/erp-mock"; import { createErpClient, drainExports, exportRequestJob, getExportRecord, type ExportDrainDeps } from "@/features/export"; import { persistExtractionRun } from "@/features/extraction"; @@ -13,7 +13,7 @@ import { syntheticExtractResponse } from "@/features/extraction/fixtures"; import { getActor, type Actor } from "@/features/identity"; import { QUEUES, reprocessRequest } from "@/features/jobs"; import { createRequest, getRequest, lockRequest, transitionRequest } from "@/features/requests"; -import { approveRequest, correctField, currentFieldValues, currentLineItemValues, ReviewRefused } from "@/features/review"; +import { approveRequest, correctField, currentFieldValues, currentLineItemValues, loadReview, ReviewRefused } from "@/features/review"; import { createTenancy, type Tenancy } from "@/features/tenancy"; import { companyWithAdmin, createStack, invitedUser, type Stack } from "./helpers/stack"; @@ -159,7 +159,33 @@ describe("export: approved requests reach the ERP exactly once", () => { // Corrections are capped at 500 characters, so an overlong position value can only come from the // extraction; the approval must refuse it while the clerk can still correct it. const tooLong = [{ ...POSITIONS[0]!, description: { ...found("Musterbau Beispiel GmbH", "s2"), value: "x".repeat(501) } }]; - await expect(approved(clerk, tooLong)).rejects.toThrow(ReviewRefused); + await expect(approved(clerk, tooLong)).rejects.toMatchObject({ code: "value_too_long" }); + }); + + it("refuses more positions than the ERP accepts with its own code – a correction cannot fix that (#46 review)", async () => { + const tooMany = Array.from({ length: 201 }, (_, index) => ({ ...POSITIONS[1]!, index })); + const refusal = approved(clerk, tooMany); + await expect(refusal).rejects.toBeInstanceOf(ReviewRefused); + await expect(refusal).rejects.toMatchObject({ code: "export_too_large" }); + }); + + it("exports exactly the positions the review shows: an item correction older than the latest run is ignored (#46 review)", async () => { + const deps = depsWith(); + const { requestId, job } = await approved(clerk, POSITIONS, async (id) => { + await correctField(tenancy, clerk, id, "quantity", "1300", 0); + // A newer run (as after reprocessing) makes the correction stale – positions may have shifted. + await tenancy.withTenant(clerk.companyId, async (tx) => { + const documentId = (await listDocuments(tx, id))[0]!.id; + await persistExtractionRun(tx, { requestId: id, jobId: randomUUID(), outcomes: [{ documentId, response: syntheticExtractResponse(documentId, {}, POSITIONS) }] }); + }); + }); + let sent: { lineItems?: Array> } | undefined; + const erp = deps.erp; + await exportRequestJob({ ...deps, erp: { submit: (body) => ((sent = body), erp.submit(body)) } }, job); + + expect(sent?.lineItems?.[0]).toMatchObject({ position: 1, quantity: "15.10.2026" }); + const shown = (await loadReview(tenancy, clerk, requestId))!.lineItems.map((item) => Object.fromEntries(item.fields.map((field) => [field.key, field.value]))); + expect(sent?.lineItems?.map(({ position: _position, ...values }) => values)).toEqual(shown); }); it("duplicate delivery of the same job: the row lock lets one export, the other sees EXPORTED and never calls the ERP", async () => {