hardening: fault-inject SQL binding, migrations, and transaction recovery #54
Description
Goal
Run a deeper, contract-driven edge-case pass for 0.5.0. The cases below are probes, not claims that defects are currently present.
Probe targets
Exercise placeholder ordering across nested queries, null/empty/binary/large values, identifier rejection, constraint violations, migration failure halfway through, rollback failure, and connection cleanup. Compare PostgreSQL and SQLite behavior for the same supported contract, and prove no partial schema/data state survives failed transactional operations.
Required outcome
- Inspect current implementation and tests before adding cases; extend existing coverage instead of duplicating it.
- Exercise relevant malformed/boundary inputs, ordering and repeated operations, failure/recovery, cancellation/cleanup, and cross-runtime or cross-package parity.
- For each confirmed bug, keep a minimal reproducer and regression test with the fix. If the fix is too large or needs a product decision, file/link a separate bug issue with impact and release disposition.
- Refactor only where the probes expose a fragile boundary, repeated work, or unclear ownership. The 0.5.0 API contraction is not a substitute for behavioral hardening.
- Record the scenarios exercised, outcomes, changed tests/fixes, coverage evidence for the affected branches, and the repository's relevant validation commands. If the probe finds no bug, record the evidence and tested matrix.
A broad checklist without executed tests or source-backed findings does not complete this issue.
Initial invariant and test-gap assessment (2026-10-02)
Baseline: static inventory of test paths, scripts, and named cases on origin/develop. No suites were run and not every assertion body was inspected. Named tests are evidence signals, not proof of coverage; candidate gaps must be checked against assertions before being called missing.
Invariants and evidence
- Batch statements respect PostgreSQL parameter ceilings and transaction failures preserve database state. Existing tests cover ceilings, heterogeneous batches/joins, PostgreSQL rollback, and terminated connections.
- SQLite rollback and migration behavior matches its documented dialect contract. tests/sqlite.test.ts covers rollback failure, nested/four-level savepoints, shared-file serialization, and failed non-transactional migrations; status: broader evidence than the initial inventory suggested.
Candidate gaps to verify
Probe connection loss during rollback and lock-release/recovery after a failed or interrupted SQLite migration; assert persisted state and subsequent connection usability. Do not treat SQLite rollback parity as untested.
Closure evidence: inspect and run the relevant assertions, then mark each invariant covered, partial, not evidenced, or not run. Add a minimal regression reproducer for every confirmed defect; record the exact command and outcome for a no-defect probe.
Activity
smiggleworth commentedon Oct 12, 2026
Completed the scoped review with no remaining reproducible defect; the candidate gaps are already covered in the current source tree, so no duplicate tests or source changes were needed.
Executed on Node 24.21.0 / npm 12.0.1:
npm run test:coveragepassed 12 files and 70 tests (30 skipped because the disposable PostgreSQL service is not available in this local run). The executed suite includes SQLite rollback failure at root/nested/close phases with lock release, a second adapter's subsequent reads/writes and reopen persistence; failed multi-statement migration rollback with clean schema/ledger and retry; and cancellation before migration execution with lock release and later recovery.The PostgreSQL integration suite covers connection termination during rollback while preserving the callback error and obtaining a fresh backend, plus migration self-disconnect with no partial DDL/ledger and successful retry. Those real-database cases passed on PostgreSQL 16, 17, and 18 in merged PR #57's hosted checks. The PR head and current develop merge commit have the same source tree (
7d6a31d935be51f45fbc46d16be6148ae2ad8032); the current merge commit's push CI also passed: https://github.com/askrjs/askr-orm/actions/runs/37999024741.