fix(sqlite-bun): read affected-row counts from run() result#52
Open
danfry1 wants to merge 3 commits into
Open
Conversation
bun:sqlite exposes change counts on the run() result object, not on Database.changes (which is undefined). The bun adapter read the latter, so heartbeatRun, updateRunStatus, updateClaimedRunStatus, and the claimNextRun double-claim guard never reflected real row counts — heartbeat always reported the lease lost, and the claim guard never engaged (risking double-claims on Bun). Read the count from the statement result instead. Adds a Bun-native smoke test (bun run test:bun) wired into CI, since the Node-based Vitest suite cannot load bun:sqlite and so never covered this adapter.
The coverage gate runs under Bun, where node:sqlite cannot load, so the sqlite-node-builtin suite skips and the file is loaded-but-uncovered — dragging the branch threshold down for any code added to it. It is fully exercised by the dedicated node-sqlite CI job (on Node), so exclude it from the Bun coverage run, exactly as sqlite-bun.ts already is (covered by the bun smoke test).
The pull_request trigger filtered on base branches: [main], so a PR targeting a not-yet-merged branch (a stacked PR) never ran CI. Drop the base filter so every PR is checked.
This was referenced Jun 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a latent correctness bug in the
reflow-ts/sqlite-bunadapter.bun:sqlitereports affected-row counts on therun()result object ({ changes, lastInsertRowid }), not onDatabase.changes(which isundefinedon Bun). The adapter readthis.db.changes, so several lease/durability guards never saw real row counts:heartbeatRunalways returnedfalse→ an active worker would appear to have lost its lease on every heartbeat.updateRunStatus/updateClaimedRunStatusreturnedfalseeven on success.claimNextRundouble-claim guard (if (changes === 0) return null) never engaged → two workers could claim the same run under contention.The fix reads the count from the statement result in all four methods. No behavioural change on the Node adapters.
Why it went unnoticed
The Node-based Vitest suite cannot load
bun:sqlite, so the Bun adapter had zero automated coverage. This PR adds a Bun-native smoke test (scripts/smoke-bun-adapter.ts,bun run test:bun) that exercises the change-count paths, wired into the CItestjob (which already runs under Bun). The smoke test fails on the pre-fix code and passes after.Scope
Foundational fix on
main. The in-flight feature PRs that touch the Bun adapter (#49requeueRun, #51sleepRun) are rebased on top of this so they inherit correct change detection rather than duplicating the fix.Verified:
typecheck,test(360),build,lint:deps, andtest:bunall pass.