From 62bd5af576cabf31113f03b3cfd67040465ef029 Mon Sep 17 00:00:00 2001 From: DEls Date: Fri, 9 Oct 2026 02:01:47 +0200 Subject: [PATCH] docs(dn): stage 1 implementation plan Co-Authored-By: Claude Opus 5.5 --- .../2026-10-09-dn-agent-tools-stage-1.md | 277 ++++++++++++++++++ 1 file changed, 277 insertions(+) create mode 100644 docs/superpowers/plans/2026-10-09-dn-agent-tools-stage-1.md diff --git a/docs/superpowers/plans/2026-10-09-dn-agent-tools-stage-1.md b/docs/superpowers/plans/2026-10-09-dn-agent-tools-stage-1.md new file mode 100644 index 0000000..b76d2c6 --- /dev/null +++ b/docs/superpowers/plans/2026-10-09-dn-agent-tools-stage-1.md @@ -0,0 +1,277 @@ +# Delivery-note agent tools — Stage 1 Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Purchasing MCP errors explain themselves, and a second delivery on the same supplier order is no longer blocked as a duplicate at posting time. + +**Architecture:** A `describePurchasingError` helper turns any thrown error into `{code, message, hint}`; the MCP tool wrapper returns it and `runOperation` journals it in `mcp_operations.result`. A pure `receiptIdentityConflict` function replaces the raw reference comparison in `postLineWithinLease`: alternate-to-alternate matches between documents with different primary numbers are no longer identity. + +**Tech Stack:** Next.js 16 (App Router), TypeScript, PocketBase JS SDK, Zod, Vitest. + +**Spec:** `docs/superpowers/specs/2026-10-09-delivery-agent-tools-design.md` (Part 1 §1.1, §1.2) + +## Global Constraints + +- No deletes: `runOperation` must still throw `business_delete_forbidden` for any `delete` mutation. +- No PocketBase schema changes in this stage (`mcp_operations` keeps its fields; the message goes into `result`). +- Existing duplicate protection stays for: same primary number, and an invoice citing a delivery-note number that is already a receipt's primary. +- Run tests with `npx vitest run `; typecheck with `./node_modules/.bin/tsc --noEmit --project tsconfig.json`. + +--- + +### Task 1: Explain purchasing errors + +**Files:** +- Modify: `src/lib/purchasing/errors.ts` +- Create: `src/lib/purchasing/errors.test.ts` +- Modify: `src/mcp/tool-contract.ts:16` +- Modify: `src/lib/purchasing/operations.ts:61-62` (failure branch of `runOperation`) +- Test: `src/lib/purchasing/operations.test.ts` (append one test) + +**Interfaces:** +- Produces: `describePurchasingError(error: unknown): {code: string; message: string; hint?: string}` from `@/lib/purchasing/errors`. `safePurchasingError` is removed (only caller is `tool-contract.ts`). + +- [ ] **Step 1: Write the failing tests** + +`src/lib/purchasing/errors.test.ts`: +```ts +import {expect,it} from 'vitest'; +import {PurchasingError,describePurchasingError} from './errors'; +it('keeps the code of a PurchasingError and adds its message',()=>{ + expect(describePurchasingError(new PurchasingError('resource_locked',409))).toEqual({code:'resource_locked',message:'resource_locked',hint:'Another change is in progress on this batch or order. Retry in a few seconds.'}); +}); +it('exposes the real message of an ordinary error with a generic code',()=>{ + expect(describePurchasingError(Error('This supplier document already has a receipt, possibly against a different PO. Resolve its identity before posting'))) + .toEqual({code:'operation_failed',message:'This supplier document already has a receipt, possibly against a different PO. Resolve its identity before posting',hint:'A posted receipt shares a document number with this slip. Compare both documents; if they are different deliveries, correct the reference numbers.'}); +}); +it('hints at reprocessing when the preview is stale',()=>{ + expect(describePurchasingError(Error('Processing result is stale; process this note again')).hint).toBe('Run process_delivery_candidate again, then post with the new snapshotHash.'); +}); +it('describes non-errors without throwing',()=>{ + expect(describePurchasingError('boom')).toEqual({code:'operation_failed',message:'boom'}); +}); +``` + +Append to `src/lib/purchasing/operations.test.ts`: +```ts +it('journals the real failure message in the operation result',async()=>{ + const {pb,store}=memoryDatabase();const ctx={pb,principal:{id:'agent',permissions:new Set<'read'|'write'>(['read','write'])}}; + await expect(runOperation(ctx,'post_delivery_line',{operationId:'5a2f0c0e-8a4e-4b7e-9d3f-6c1b2a3d4e5f'},async()=>{throw Error('Processing result is stale; process this note again');})).rejects.toThrow(/stale/); + const row=[...store('mcp_operations').values()][0] as Record; + expect(row).toMatchObject({status:'failed',error_code:'operation_failed',result:{error:{code:'operation_failed',message:'Processing result is stale; process this note again'}}}); +}); +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `npx vitest run src/lib/purchasing/errors.test.ts src/lib/purchasing/operations.test.ts` +Expected: FAIL — `describePurchasingError` is not exported; journal row has no `result.error`. + +- [ ] **Step 3: Implement** + +`src/lib/purchasing/errors.ts`: +```ts +export class PurchasingError extends Error { + constructor(public code:string,public status=400){super(code);} +} +export type PurchasingErrorDescription={code:string;message:string;hint?:string}; +const HINTS:Array<[RegExp,string]>=[ + [/^resource_locked$|locked/i,'Another change is in progress on this batch or order. Retry in a few seconds.'], + [/stale|context changed/i,'Run process_delivery_candidate again, then post with the new snapshotHash.'], + [/already has a receipt/i,'A posted receipt shares a document number with this slip. Compare both documents; if they are different deliveries, correct the reference numbers.'], + [/identity or PO first|candidate_entities_unresolved/i,'Link the supplier and purchase order with resolve_delivery_entities first.'], + [/grouping/i,'Confirm the page grouping (set_delivery_grouping) before processing or posting.'], + [/review reason is required/i,'Put a reason inside confirmation.reason when changing quantity, unit, PO item or value.'], + [/allocation changed|Unknown allocation/i,'Re-read the preview; allocations move after each post. Use the current index for these sourceLineIds.'], +]; +export function describePurchasingError(error:unknown):PurchasingErrorDescription { + const code=error instanceof PurchasingError?error.code:'operation_failed'; + const message=error instanceof Error?error.message:String(error); + const hint=HINTS.find(([pattern])=>pattern.test(message))?.[1]; + return hint?{code,message,hint}:{code,message}; +} +``` + +`src/mcp/tool-contract.ts` — replace the import and the catch: +```ts +import {describePurchasingError} from '@/lib/purchasing/errors'; +``` +```ts + }catch(error){return {isError:true,content:[{type:'text' as const,text:JSON.stringify(describePurchasingError(error))}]};} +``` + +`src/lib/purchasing/operations.ts` — in the failure branch, keep `error_code` and add the description to `result`: +```ts + const described=describePurchasingError(error); + await lease.update('mcp_operations',id,{status:submitted&&![400,401,403,409,422].includes(Number((error as {status?:number})?.status))?'uncertain':'failed',error_code:described.code,result:{error:described}}).catch(()=>undefined); +``` +and add `import {describePurchasingError} from './errors';` (keep the existing `PurchasingError` import). + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `npx vitest run src/lib/purchasing src/mcp` +Expected: PASS (all purchasing and MCP tests). + +- [ ] **Step 5: Commit** + +```bash +git add src/lib/purchasing/errors.ts src/lib/purchasing/errors.test.ts src/lib/purchasing/operations.ts src/lib/purchasing/operations.test.ts src/mcp/tool-contract.ts +git commit -m "fix(mcp): return and journal the real purchasing error with a hint" +``` + +--- + +### Task 2: Receipt identity rule + +**Files:** +- Create: `src/lib/dn-pipeline/receipt-identity.ts` +- Create: `src/lib/dn-pipeline/receipt-identity.test.ts` + +**Interfaces:** +- Produces: `receiptIdentityConflict(candidate: {primary: string; alternates: readonly string[]}, receipt: {primary: string; references: readonly string[]}): string | null` — returns the matching number, or `null` when there is no identity conflict. + +Rule: normalise every number (`[^a-z0-9]` removed, lower-case). Conflict when (a) candidate primary equals receipt primary; (b) candidate primary is among receipt references; (c) receipt primary is among candidate alternates. Alternate-to-alternate overlap counts **only** when either primary is empty. + +- [ ] **Step 1: Write the failing tests** + +```ts +import {expect,it} from 'vitest'; +import {receiptIdentityConflict} from './receipt-identity'; +it('does not treat a shared Builders order number as the same document',()=>{ + expect(receiptIdentityConflict({primary:'12362463',alternates:['2025505214','505031683']},{primary:'12062458',references:['12062458','2025505214','505032051']})).toBeNull(); +}); +it('does not treat a shared Infraset sales order as the same document',()=>{ + expect(receiptIdentityConflict({primary:'DEL84831',alternates:['INV85568','SO62169']},{primary:'DEL84513',references:['DEL84513','SO62169']})).toBeNull(); +}); +it('still blocks the same document number',()=>{ + expect(receiptIdentityConflict({primary:'VDN1',alternates:[]},{primary:'vdn-1',references:[]})).toBe('vdn1'); +}); +it('blocks an invoice that cites an already posted delivery note',()=>{ + expect(receiptIdentityConflict({primary:'IN019100',alternates:['IO019234']},{primary:'IO019234',references:['IO019234']})).toBe('io019234'); +}); +it('blocks a delivery note already cited by a posted invoice',()=>{ + expect(receiptIdentityConflict({primary:'IO019234',alternates:[]},{primary:'IN019100',references:['IN019100','IO019234']})).toBe('io019234'); +}); +it('keeps alternate overlap as a conflict when a primary is missing',()=>{ + expect(receiptIdentityConflict({primary:'',alternates:['SO62169']},{primary:'DEL84513',references:['SO62169']})).toBe('so62169'); +}); +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `npx vitest run src/lib/dn-pipeline/receipt-identity.test.ts` +Expected: FAIL — module not found. + +- [ ] **Step 3: Implement** + +```ts +/** + * Does a posted receipt already hold this delivery document? + * + * Suppliers print order and account numbers beside the document number + * (Builders "12362463 / 2025505214 / 505031683"; Infraset sales order + * SO62169). Every delivery against that order repeats them, so matching + * any number against any number blocked the second delivery of every + * order. Identity is decided by primary numbers: a primary matching the + * other side's primary or references, or the other side's primary among + * our alternates (an invoice citing its delivery note). Two documents + * with different primary numbers that only share an alternate are two + * documents on one order. When a primary is missing we cannot tell, and + * any shared number still blocks. + */ +const norm=(value:unknown)=>typeof value==='string'?value.replace(/[^a-z0-9]/gi,'').toLowerCase():''; + +export function receiptIdentityConflict( + candidate:{primary:string;alternates:readonly string[]}, + receipt:{primary:string;references:readonly string[]}, +):string|null{ + const ours=norm(candidate.primary),theirs=norm(receipt.primary); + const ourAlts=new Set(candidate.alternates.map(norm).filter(Boolean)); + const theirRefs=new Set(receipt.references.map(norm).filter(Boolean)); + if(ours&&ours===theirs)return ours; + if(ours&&theirRefs.has(ours))return ours; + if(theirs&&ourAlts.has(theirs))return theirs; + if(!ours||!theirs){for(const value of ourAlts)if(theirRefs.has(value))return value;} + return null; +} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `npx vitest run src/lib/dn-pipeline/receipt-identity.test.ts` +Expected: PASS (6 tests). + +- [ ] **Step 5: Commit** + +```bash +git add src/lib/dn-pipeline/receipt-identity.ts src/lib/dn-pipeline/receipt-identity.test.ts +git commit -m "feat(dn): decide receipt identity by primary document numbers" +``` + +--- + +### Task 3: Use the identity rule when posting + +**Files:** +- Modify: `src/lib/dn-pipeline/post-line.ts:27-38` +- Test: `src/lib/dn-pipeline/post-line.test.ts` (append two tests) + +**Interfaces:** +- Consumes: `receiptIdentityConflict` from Task 2. + +- [ ] **Step 1: Write the failing tests** + +Append to `src/lib/dn-pipeline/post-line.test.ts` (fixture note: `doc_number: 'VDN1'`, supplier `supplierabcdefg`): +```ts +it('posts a second delivery that only shares the supplier order number with a posted receipt',async()=>{ + c.note.doc_number_alts=['ORDER77']; + identities.mockResolvedValue([{id:'other',delivery_note_number:'VDN0',raw_ocr_data:{documentReferences:['VDN0','ORDER77']},expand:{purchase_order:{supplier_ref:'supplierabcdefg'}}}]); + await run();expect(batch).toHaveBeenCalledTimes(1); +}); +it('still blocks a receipt whose primary number this slip cites',async()=>{ + c.note.doc_number_alts=['VDN0']; + identities.mockResolvedValue([{id:'other',delivery_note_number:'VDN0',raw_ocr_data:{documentReferences:['VDN0']},expand:{purchase_order:{supplier_ref:'supplierabcdefg'}}}]); + await expect(run()).rejects.toThrow(/already has a receipt/);expect(batch).not.toHaveBeenCalled(); +}); +``` + +- [ ] **Step 2: Run tests to verify the first fails** + +Run: `npx vitest run src/lib/dn-pipeline/post-line.test.ts` +Expected: the "shares the supplier order number" test FAILS with "already has a receipt"; the other passes. + +- [ ] **Step 3: Implement** + +In `src/lib/dn-pipeline/post-line.ts` add `import {receiptIdentityConflict} from './receipt-identity';` and replace the loop over `identities` (current lines 31-38) with: +```ts + const alternates=Array.isArray(context.note.doc_number_alts)?context.note.doc_number_alts.map(String):[]; + const identities=await pb.collection('delivery_notes').getFullList({fields:'id,delivery_note_number,raw_ocr_data,expand.purchase_order.supplier_ref',expand:'purchase_order'}); + for(const receipt of identities){ + if(receipt.id===state?.deliveryNoteId)continue; + const receiptReferences=Array.isArray(receipt.raw_ocr_data?.documentReferences)?receipt.raw_ocr_data.documentReferences.map(String):[]; + if(!receiptIdentityConflict({primary:String(context.note.doc_number||context.note.delivery_note_number||''),alternates},{primary:String(receipt.delivery_note_number??''),references:receiptReferences}))continue; + const supplier=receipt.expand?.purchase_order?.supplier_ref || receipt.raw_ocr_data?.supplier; + if(!supplier||supplier===context.supplier.id)throw Error('This supplier document already has a receipt, possibly against a different PO. Resolve its identity before posting'); + } +``` +Keep `reference`, `references` and `receivingIdentityKey` as they are (still used for the receipt record). + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `npx vitest run src/lib/dn-pipeline` then `./node_modules/.bin/tsc --noEmit --project tsconfig.json` +Expected: all dn-pipeline tests PASS; no type errors. + +- [ ] **Step 5: Commit** + +```bash +git add src/lib/dn-pipeline/post-line.ts src/lib/dn-pipeline/post-line.test.ts +git commit -m "fix(dn): stop blocking second deliveries that share a supplier order number" +``` + +--- + +### Task 4: Verify and release + +- [ ] **Step 1:** Run `npx vitest run src/lib/purchasing src/lib/dn-pipeline src/mcp` and the typecheck. Expected: PASS. +- [ ] **Step 2:** Ask David to push `main` (Coolify deploys from it). Do not push without his go-ahead. +- [ ] **Step 3:** After deploy, through the purchasing MCP: `process_delivery_candidate` then `post_delivery_line` for Builders 12362463 (batch `kz5o0926odhoka9`, note `ngq70y25ydd8xwm`) and Infraset DEL84831 (batch `nsevw5u7qhmlmen`, note `cydrtes9uk9jou1`; kerbs take Fig 7 to 1,740 of 1,700 — post with `acceptExcess` and a reason). Expected: both post; any failure now shows a readable message.