docs(dn): stage 1 implementation plan

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-09 02:01:47 +02:00 · 2026-10-09 02:01:47 +02:00 · 62bd5af576
commit 62bd5af576
parent 6c6f767cfb
1 changed files with 277 additions and 0 deletions
--- 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
@ -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 <path>`; 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<string,unknown>;
+ 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.