diff --git a/docs/superpowers/plans/2026-10-09-dn-deletion-flags.md b/docs/superpowers/plans/2026-10-09-dn-deletion-flags.md new file mode 100644 index 0000000..d525aff --- /dev/null +++ b/docs/superpowers/plans/2026-10-09-dn-deletion-flags.md @@ -0,0 +1,197 @@ +# Deletion flags a person can act on — Implementation Plan + +> **For agentic workers:** implement task by task, test first. Steps use checkbox (`- [ ]`) syntax. + +**Goal:** Agent Foreman's deletion flags stop being a footnote at the bottom of the inbox. They become amber cards at the top. Each card opens a page that says in plain words what the record is, why it was flagged, and what it affects, with **Delete** and **Keep** buttons for the right person. Nobody needs PocketBase admin. + +**Architecture:** `src/lib/purchasing/deletion-decisions.ts` holds all the rules: describe, block, permission, resolve. Callers: +- a POST route performs a decision; +- a server page shows a flag; +- the inbox shows the strip. + +Flags already live as `mcp_operations` rows (`tool:"request_deletion"`, `status:"committed"`, `result:{collection,recordId,reason,summary}`). See `src/lib/purchasing/deletion-requests.ts`. A decision journals a `deletion_decision` row with a full copy of the record, marks the flag rows `status:"resolved"`, and only then deletes (for Delete). + +**Spec:** `docs/superpowers/specs/2026-10-09-dn-explain-or-ask-design.md` §6 ("Flagged for deletion" decision) and §7. Approved by David on 2026-10-09. The requester is always shown as **"Agent Foreman"**. + +## Global Constraints + +- This is the **only** place app code may call `.delete(`, and only inside `resolveFlag` after the journal row is written. Agents (MCP tools) can't call it: there is no MCP tool for decisions. +- Evidence is never deleted, from here or anywhere else: `delivery_notes`, `delivery_note_items`, `dn_batches`, `dn_batch_notes`, `dn_batch_pages`. +- No PocketBase schema changes. +- Every decision needs a reason (min 3 characters) and is recorded with who and when. +- Verify each task: `npx vitest run src/lib/purchasing src/app/dashboard/financial/delivery-notes src/app/api`, `./node_modules/.bin/tsc --noEmit --project tsconfig.json`, `npx eslint `. Check that each new test fails without its change; don't use `git stash` (shared). + +--- + +### Task 1: The rules (`deletion-decisions.ts`) + +**Files:** create `src/lib/purchasing/deletion-decisions.ts` and `src/lib/purchasing/deletion-decisions.test.ts`. Modify `src/lib/purchasing/deletion-requests.ts` (the requester label, and the open flag rows for a record). + +**Interfaces (produced):** + +```ts +import type PocketBase from 'pocketbase'; +import type {AppUser} from '@/lib/auth/access'; +export const EVIDENCE=['delivery_notes','delivery_note_items','dn_batches','dn_batch_notes','dn_batch_pages'] as const; +export type FlagLink={label:string;href:string}; +export type FlagDetail={collection:string;recordId:string;kind:string;headline:string;fields:Array<{label:string;value:string}>;affects:FlagLink[];reasons:string[];requestedBy:'Agent Foreman'}; +export async function describeFlag(pb:PocketBase,collection:string,recordId:string):Promise; // null when there is no open flag or the record is gone +export async function deletionBlock(pb:PocketBase,collection:string,record:Record):Promise; // why Delete is not allowed, or null +export async function canDecide(pb:PocketBase,user:AppUser,collection:string,record:Record):Promise; +export async function resolveFlag(pb:PocketBase,user:AppUser,input:{collection:string;recordId:string;action:'delete'|'keep';reason:string}):Promise<{action:'delete'|'keep';journalId:string}>; +export class FlagDecisionError extends Error{constructor(readonly code:string,readonly status:number,message:string){super(message);}} +``` + +**Rules:** + +`kind` and `headline`, in plain words: + +| Collection | kind | headline | +|---|---|---| +| `supplier_document_aliases` | `Supplier alias` | `"" on documents is read as ` | +| `suppliers` | `Supplier` | `` | +| `purchase_orders` | `Purchase order` | ` · ` | +| `purchase_order_items` | `PO line` | ` on ` | +| `delivery_notes` | `Booked receipt` | `` | +| `dn_batch_notes` | `Delivery document` | `` | +| `dn_batches` | `Upload` | `` | +| `dn_batch_pages` | `Photo page` | `page ` | +| `delivery_note_items` | `Receipt line` | ` on ` | + +`fields`: 3–6 label/value pairs a person recognises (supplier, site, PO number, numbers, dates). Never the raw JSON. + +`affects`: links (max 10). +- **alias:** the supplier, plus delivery documents whose `supplier_name_raw` normalises (`normaliseSupplierKey` from `@/lib/dn-pipeline/resolve-entities`) to the alias's `normalised_key` → `/dashboard/financial/delivery-notes/document/` labelled ` ()`. +- **supplier:** counts as text links to `/dashboard/financial/purchase-orders?supplier=` (label `N purchase orders`), plus the documents. +- **PO:** its page `/dashboard/financial/purchase-orders/` (check this route exists: `ls src/app/dashboard/financial/purchase-orders`; use whatever detail route exists). +- **evidence:** its document page or batch page (`/dashboard/financial/delivery-notes/inbox/`). + +`reasons`: every flag reason for that record. `requestedBy`: always `'Agent Foreman'`. + +`deletionBlock` (Delete is allowed only when this returns null): +- `supplier_document_aliases` → `null`. +- `suppliers` → count `purchase_orders` (`supplier_ref`), `dn_batch_notes` (`supplier`), `supplier_document_aliases` (`supplier`). Any > 0 → `Still used by purchase orders, delivery documents and aliases. Merge it into the right supplier instead.` +- `purchase_order_items` → any `delivery_note_items` with this `purchase_order_item` → ` receipts were booked against this PO line.` +- `purchase_orders` → any `delivery_notes` (`purchase_order`), `dn_batch_notes` (`purchase_order`) or `purchase_order_items` → `This PO has deliveries, linked documents and lines.` +- `EVIDENCE` → `This is delivery evidence and is never deleted. Keep it, or set it aside or move the receipt from its document page.` + +`canDecide`: +- `user.role==='admin'` → true; +- `clock_only` → false; +- otherwise the record's site decides: + - site-bound records via `canAccessSite(user, site)`, where site comes from: `record.site` (purchase_orders, delivery_notes, dn_batches), `dn_batches.site` via `record.batch` (dn_batch_notes, dn_batch_pages), the PO's site (purchase_order_items), the delivery note's site (delivery_note_items); + - `suppliers` / `supplier_document_aliases` (supplier = `record.id` / `record.supplier`) → true when the user can access the site of at least one `purchase_orders` row with that `supplier_ref`. + +`resolveFlag`: +1. Load the record (missing → `FlagDecisionError('record_not_found',404,'This record no longer exists.')`). +2. Find the open flag rows: `mcp_operations` with `tool="request_deletion" && status="committed"` and `result.collection/recordId` matching. Filter in JS too, as `listDeletionRequests` does. None → `FlagDecisionError('no_open_flag',409,'This record is not flagged for deletion.')`. +3. `canDecide` false → `FlagDecisionError('not_allowed',403,'You can only decide flags for your own sites.')`. +4. Reason shorter than 3 characters after trim → `FlagDecisionError('reason_required',400,'Give a short reason.')`. +5. `delete` and `deletionBlock` returns a message → `FlagDecisionError('delete_blocked',409,)`. +6. Create the journal row in `mcp_operations`: + + ```ts + {id: operationRecordId(user.id,'deletion_decision',crypto.randomUUID()), + credential:user.id, tool:'deletion_decision', operation_id:crypto.randomUUID(), + payload_hash:operationPayloadHash('deletion_decision',{collection,recordId,action,reason}), + status:'committed', + result:{collection,recordId,action,reason,decidedBy:user.id,decidedByName:user.name??user.email??user.id,at:new Date().toISOString(),snapshot:record}} + ``` + + `operationRecordId` and `operationPayloadHash` come from `./operations`. Check the `AppUser` fields for a display name and use what exists. +7. For `delete`: `await pb.collection(collection).delete(recordId)`. If it throws, update the journal row `status:'failed'` with the error message and rethrow `FlagDecisionError('delete_failed',409,…)`. +8. Update every open flag row: `status:'resolved'`, `result:{...result,resolution:{action,reason,by:user.id,at}}`. +9. Return `{action,journalId}`. + +`deletion-requests.ts`: set `requestedBy` to `'Agent Foreman'` for every flag. Keep `credential` available as `requestedByKey` if you need it. `listDeletionRequests` already only lists `status:"committed"` rows, so resolved flags drop off by themselves. Confirm this with a test. + +- [ ] **Step 1: Tests** (`memoryDatabase()` from `./test-support`; users are plain objects shaped like `AppUser`: `{id, role, siteIds, ...}`; read the type in `src/lib/auth/access.ts`): + 1. Admin deletes a flagged alias: + - the alias is gone; + - one `deletion_decision` row exists, its `result.snapshot.raw_value` is the alias text, and its `action` is `delete`; + - the flag row is `status:'resolved'`; + - `listDeletionRequests` is empty. + 2. Admin keeps it: the alias is still there, the flag is resolved, and the list is empty. + 3. A supplier still used by a PO → `delete_blocked` (message mentions `1 purchase orders`). Keep is still allowed. + 4. Evidence (a flagged `dn_batch_notes` row) → `delete_blocked` with the evidence message. Keep is allowed. + 5. A Site manager whose `siteIds` include the site of a PO for the alias's supplier → allowed. A Site manager of another site → `not_allowed`. A Clock user → `not_allowed`. + 6. No reason → `reason_required`. Not flagged → `no_open_flag`. + 7. `describeFlag` for the alias: `kind 'Supplier alias'`, a headline containing the raw value and the supplier name, `requestedBy 'Agent Foreman'`, and `affects` including the delivery document whose `supplier_name_raw` matches. +- [ ] **Step 2:** run and see them fail. **Step 3:** implement. **Step 4:** verify. **Step 5:** commit `feat(dn): rules for deciding deletion flags (delete with a saved copy, keep, never evidence)`. + +--- + +### Task 2: The decision route + +**Files:** create `src/app/api/deletion-flags/[collection]/[recordId]/route.ts` and `route.test.ts`. + +`POST` body `{action:'delete'|'keep', reason:string}`: +- `requireApiUser('financial')` (return the Response if it is one); +- validate `recordId` against `/^[a-z0-9]{15}$/i`, the collection against the allowed list in `deletion-requests.ts` (`RequestDeletionSchema` enum), and `action` (→ 400 `invalid_request`); +- `resolveFlag(await pbAdmin(), user, {...})` → `200 {ok:true,action}`; +- `FlagDecisionError` → `{error:code,detail:message}` with its status; +- anything else → `409 {error:'decision_not_saved',detail:message}`. + +Follow `src/app/api/dn-batches/[id]/notes/[noteId]/site-decision/route.ts` for style and the test mocks. + +Tests: 401 passes through; 400 for a bad action; maps `delete_blocked` to 409 with detail; success returns `{ok:true,action:'keep'}` (mock `resolveFlag`). + +Commit: `feat(dn): route to delete or keep a flagged record`. + +--- + +### Task 3: The flag page + +**Files:** +- create `src/app/dashboard/financial/delivery-notes/flags/[collection]/[recordId]/page.tsx` (server); +- create `flag-decision.tsx` (client), with `flag-decision.test.tsx`. + +Page: +- `requirePageUser('financial')`; +- `describeFlag`; if null, show "This flag has already been decided" with a link back to the inbox; +- load the record, `deletionBlock` and `canDecide`; +- render, in this order: + 1. **Header:** `` badge + ``; + 2. **"Why it was flagged":** each reason, then `— Agent Foreman`; + 3. **"What it is":** the `fields` as a definition list; + 4. **"What it affects":** the `affects` links; + 5. ``. + +Use the `Card` and `Badge` components and the amber colour (`border-2 border-amber-500`) like the review cards. + +`FlagDecision({collection,recordId,canDecide,deleteBlock}:{collection:string;recordId:string;canDecide:boolean;deleteBlock:string|null})`: +- a reason `