13 KiB
Task 8 report: hardened promotion and duplicate identity
Status: complete
Code commit
e4f5d6fd06f2e903658cb57b3517c8cdba023935 — fix(dn): harden candidate promotion identity
RED evidence
Before implementation, the new safety tests failed for the intended missing behaviours:
npx vitest run src/lib/dn-pipeline/promote.test.ts src/lib/dn-pipeline/checks.test.ts
6 failed, 51 passed
- documentIdentity is not a function (2 checks tests)
- image candidate was forced to application/pdf
- delivery_notes.batch_note was not written
- partial line failure produced no promotion_failed review event
- posted candidate was mutated instead of rejected
The pre-existing stale-delete expectation was also revised to the required review-visible failure semantics: a failed reconciliation delete must not be followed by new financial line writes.
GREEN evidence
npx vitest run src/lib/dn-pipeline/promote.test.ts src/lib/dn-pipeline/checks.test.ts src/lib/dn-pipeline/run.test.ts
3 files passed; 95 tests passed
npx vitest run src/lib/dn-pipeline
15 files passed; 199 tests passed
npm run test:run
93 files passed; 1088 tests passed
npm run typecheck
passed
python3 scripts/pb-add-dn-staged-ingestion.py --dry-run
passed; offline preview only, no schema changes written
git diff --check
passed
Files
src/lib/dn-pipeline/promote.ts- Uses
batchNoteIdas the permanentdelivery_notes.batch_notekey and recovers by querying that relation with a bound PocketBase filter. - Preserves candidate MIME and filename, including image candidates.
- Reconciles only unposted partial rows; item deletes are no longer best-effort because writing after a failed delete could duplicate payable rows.
- Records
dn_review_eventson partial promotion failure, leaves the candidate inneeds_review, and posts only after every line write succeeds. - Rejects posted/promoted candidates unless an explicit correction flag is supplied.
- Uses
src/lib/dn-pipeline/promote.test.ts- Covers candidate keying, image MIME/name, partial failure plus audit and retry, unmatched zero quantity, idempotent item replacement, and posted immutability.
src/lib/dn-pipeline/checks.ts- Adds
normaliseDocumentNumberanddocumentIdentity. - Uses supplier-scoped duplicate identities; unknown-supplier collisions are review-visible and do not globally collide with known suppliers.
- Adds
src/lib/dn-pipeline/checks.test.ts- Covers same-supplier blocking, cross-supplier reuse, and unknown-supplier review behaviour.
src/lib/dn-pipeline/run.ts- Passes supplier identity and document identity sets into checks, feeds promoted identities back within a batch, preserves source MIME/extension on candidate files, and binds touched PocketBase lookups safely.
Decisions and self-review
- The candidate relation is linked to the delivery row at create/update time,
while the candidate is marked
stage=posted,status=promoted, and givenposted_atonly after all item writes complete. - A retry removes only rows belonging to the same candidate's delivery row, then recreates the complete line set. A partial line failure therefore converges to one complete set rather than accumulating duplicates.
- Unknown supplier identity is
unknown:<normalised-number>for review matching only. It never blocks a known supplier with the same printed number. - Existing month-end receipt/invoice payable semantics, staged feature-flag routing, Clock/photo/image-quality/nearest-site behaviour, and production PocketBase writes were left unchanged.
- No migration was applied; the required dry-run ran without credentials and reported the intended additive schema only.
Concerns
- Explicit correction is represented by
explicitCorrectionon the promotion input; the normal automatic path rejects posted candidates. A future manual correction workflow should supply its own audit action and actor. - Legacy delivery-note rows without supplier provenance are placed in the unknown identity namespace and therefore require review when duplicated.
Fix Round 1
Status: complete.
RED evidence
Before the production fixes, the focused regression suite recorded the
expected failures: npx vitest run src/lib/dn-pipeline/promote.test.ts src/lib/dn-pipeline/run.test.ts reported 7 failed, 56 passed. The
failures covered posted legacy reruns, duplicate-key create races, MIME/name
propagation, swallowed review/audit persistence errors, and a posted-race
failure handler rewriting a candidate.
GREEN evidence
- Focused promotion/runner tests: 64 passed (
promote.test.ts,run.test.ts). - Full delivery-note pipeline: 208 passed across 15 files.
- Full repository suite: 1,097 passed across 93 files.
npm run typecheck: passed.python3 scripts/pb-add-dn-staged-ingestion.py --dry-run: passed without production credentials or writes.git diff --check: passed.
Fixes and regression coverage
upsertCandidatenow returns a posted legacy candidate unchanged, and the runner counts it as reconciled/promoted without invoking promotion or rewriting candidate fields/files.- A lost
batch_notecreate race now re-queries with a bound PocketBase filter. A posted winner is returned as the existing immutable result; an unposted/in-progress winner raises retryablepromotion_in_progresswithout touching its lines or candidate. - Runner extraction passes authoritative MIME to promotion. Content signatures override incorrect declarations, and candidate/output names retain the correct extension for PDFs, common raster formats, extensionless files, and uncommon image MIME types.
- Review-marker and audit-event persistence now run independently with bounded retries. At least one durable marker/event is required for the original failure to be surfaced; if both fail, a combined persistence error reaches the runner/batch. Failure handling never relabels a candidate observed as posted.
- Added integration coverage for posted reruns, realistic duplicate-key races, runner MIME/name propagation, partial writes with audit visibility and retry convergence, marker/event failure combinations, and posted-race protection.
Fix Round self-review
- Code commit:
7c7f0a717eec941df1efd1740a4e841803dc2ac6(fix(dn): close promotion race and rerun gaps). - Supplier-scoped duplicate identity, zero quantity for unmatched lines, post-after-all-line-writes ordering, staged feature-flag routing, and month-end/receipt/invoice payable semantics remain unchanged.
- Clock/photo/image-quality/nearest-site behaviour and migration scope remain unchanged. No production PocketBase writes were performed.
Fix Round 2
Status: complete.
RED evidence
The new regression tests were run before the production changes. The focused promotion suite reported 3 failed, 23 passed: candidate read failures were treated as absence, concurrent promoters both proceeded, and a second promoter could enter while the first was persisting a failure marker.
GREEN evidence
- Focused promotion/runner tests: 67 passed (
promote.test.ts,run.test.ts). - Full delivery-note pipeline: 211 passed across 15 files.
- Isolated Poppler split test: 6 passed.
- Full repository suite, serial workers: 1,100 passed across 93 files.
npm run typecheck: passed.python3 scripts/pb-add-dn-staged-ingestion.py --dry-run: passed without credentials or writes.git diff --check: passed.
Fixes and regression coverage
promoteNotenow acquires the shared acquisition-specific batch lease before reading the candidate. Candidate read errors are fail-closed and cannot be interpreted as candidate absence; no delivery row or item mutation occurs.- The complete promotion attempt—terminal candidate check, bound delivery-note
lookup/create/reconciliation, item replacement, posted marker, and durable
failure persistence—runs under the same
withBatchLease(pb, batchId, ...). Lease losers receive retryablepromotion_in_progressand perform no writes. - The runner no longer performs an unprotected candidate
needs_reviewupdate after promotion failure. Promotion owns failure marking while holding the lease, and the runner records the batch/check failure only. - Added interleaving tests proving a second promoter cannot post while the first owns the lease during delivery creation or failure-marker persistence, plus fail-closed candidate-read coverage.
Fix Round 2 self-review
- Code commit:
c37821d232259be314fb78928f2d71e21f71c0f4(fix(dn): serialize promotion under batch lease). - Create-race recovery, supplier-scoped duplicate identity, unknown-supplier review semantics, zero unmatched quantity, authoritative MIME/name handling, post-after-all-line-writes ordering, and posted immutability remain intact.
- Parallel full-suite runs consistently hit the pre-existing Poppler split fixture's 5-second timeout under load (1,099 passed, 1 timeout); the isolated and serial full-suite runs pass. No production PocketBase writes were made.
Fix Round 3
Status: complete.
RED evidence
The new lock/run regressions were executed before implementation. The focused
lock and legacy-run run reported 4 failed, 45 passed: missing lock schema
was not classified, callbacks had no ownership context/heartbeat, and a
concurrent legacy run proceeded instead of returning batch_locked.
GREEN evidence
- Focused lock, promotion, legacy-run, staged-run, tick, and assembly tests: 131 passed.
- Full delivery-note pipeline: 215 passed across 15 files.
- Full repository suite, serial workers: 1,104 passed across 93 files.
- Migration self-test: passed (
index replacement and idempotence). - Migration dry-run: passed; no credentials and no schema writes.
npm run typecheck: passed.git diff --check: passed.
Fixes and regression coverage
withBatchLeasenow fails closed whendn_pipeline_locksis absent or missing required methods, with actionabledn_pipeline_schema_required; there is no no-lock compatibility fallback.- Legacy
runBatchnow owns the entire execution under one shared lease, including page work, candidate reads/upserts, checks, and financial promotion. It callspromoteNoteWithinLeaseto avoid nested acquisition; direct callers continue using publicpromoteNoteand its own lease. - Lease callbacks receive
assertOwned()/renew()context. A heartbeat renews before expiry, renewal/read ownership loss fails closed, and acquisition IDs remain unique so an old owner cannot release or write a reclaimed replacement. - Promotion asserts ownership before delivery writes, line deletes/creates, posted marking, and review-marker/audit persistence. Staged ingestion and manual assembly likewise guard every later mutation.
- Added tests for >TTL heartbeat renewal, renewal failure and replacement protection, schema-required errors, concurrent legacy-run exclusion, and ownership-safe staged/manual paths.
Fix Round 3 self-review
- Code commit:
2f798ba58622f95b7badc422bd38d0b70c3bd67f(fix(dn): lease legacy runs and renew ownership). - Prior create-race recovery, candidate immutability, MIME/name preservation, supplier-scoped duplicate identity, unmatched zero quantity, and post-after-all-line-writes ordering remain unchanged.
- The additive staged-ingestion migration remains a release prerequisite; it must be applied before deployment. No production apply or PocketBase writes were performed.
Fix Round 4
Status: complete.
RED evidence
The new fencing regressions recorded 17 intended failures before the implementation: protected business mutations could occur outside the lease transaction, allowing a reclaimed owner to write after ownership loss.
GREEN evidence
- Focused fencing, lock, page, promotion, legacy-run, staged-run, tick, and assembly suites: 164 passed across 9 files.
npm run typecheck: passed.git diff --check: passed.- No production PocketBase writes were performed.
Fixes and files
src/lib/dn-pipeline/batch-mutations.tsandtest-batch.tsprovide the PocketBase batch mutation adapter and test fixture.src/lib/dn-pipeline/lock.tsexposes the fenced lease mutation helpers and rejects SDKs without transactional batch support.src/lib/dn-pipeline/page-store.ts,promote.ts,run.ts, andrun-staged.tsroute protected writes through the lease fence.src/app/api/dn-batches/tick/route.tsandsrc/app/api/dn-batches/[id]/assembly/route.tsuse the same protected path.- Regression coverage is in
batch-mutations.test.ts,fencing.test.ts, and the focused lock/page/promotion/run/staged/tick/assembly test updates.
Decision
Each business mutation shares a PocketBase batch transaction with the current lease-record update. If the lease fence is stale or reclaimed, PocketBase rejects the transaction and rolls back the business mutation.
Code commit
b5ba219d10cb2760c4776fe6751b8800e67684c6 —
fix(dn): transactionally fence protected writes