Skip to content

fix(push): release SQLite queue when transactions cannot start - #22208

Closed
OrcaWin wants to merge 1 commit into
mainfrom
codex/memory-push-sqlite-queue-0922
Closed

OrcaWin wants to merge 1 commit into
mainfrom
codex/memory-push-sqlite-queue-0922

Conversation

@OrcaWin

@OrcaWin OrcaWin commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator
Files Added Deleted Net
Test 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​35 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​35
Prod 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​5 $\color{#cf222e}{\Huge{\mathbf{−}}}$​3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​2

ELI5

A failed SQLite write lock could leave push registration, delivery bookkeeping, and shutdown waiting forever. Later operations now proceed when the lock becomes available.

What Changed

Move BEGIN IMMEDIATE inside the existing cleanup boundary so the transaction always releases its queue slot. Only roll back a transaction that actually started.

Why

The previous failure path left an unresolved promise retaining every later queued operation. Reusing the existing finally preserves serialization and avoids new retry or database policies.

Linked Issue

N/A — found during the requested local memory audit; its tracker stays local.

Visual Proof

N/A — resource lifetime and queue admission changes have no visual change. Regression tests provide before/after evidence.

Testing

  • A regression using two real SQLite connections fails by timeout on the original code and passes after the fix. It repeats lock failure and recovery ten times, then closes the database.
  • Push suite: 143 tests passed; four PostgreSQL integration cases skipped. Push TypeScript check passed.
  • Executed on macOS with ORCA_BACKGROUND_LAUNCH=1; PostgreSQL code unchanged.

AI Disclosure

Implemented and reviewed with Codex.

Review

Self-reviewed ownership, failure paths, and retained-resource bounds.

Agent skill upstream boundary

  • Not applicable.

Notes

No wire format, platform-specific process handling, Git-provider behavior, or workspace-kind change.

Checklist

  • Small and focused; explanation covers behavior, mechanism, and approach.
  • Automated regressions added and run.
  • Cross-platform and SSH/remote impact considered.
  • Full repository lint, typecheck, test, and build (focused checks passed; remaining checks left to CI).

@OrcaWin

OrcaWin commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator Author

Closing as fully superseded on current main (1a723194c3294298c8db106105d1be226823ae9c). I read the complete two-file patch, including the real-SQLite lock failure/recovery regression.

Push now delegates openPushDatabase() to @orca-cloud/cloud-database (shared in #33714). Its SqliteDatabase.transaction() places BEGIN IMMEDIATE inside the outer try/finally that releases the queue promise. The inner operation/COMMIT try/catch starts only after BEGIN succeeds, so BEGIN failure never runs ROLLBACK and always releases the queue. Queries and close continue to await the serialized tail. The current cloud/apps/push/src/sqlite-transaction-queue.test.ts also covers lock refusal followed by usable work and close.

The production paths are unchanged between the audited checkout and the current main commit above. The queue-release and conditional-rollback behavior this PR repairs is already implemented; there is no remaining distinct production fix to carry forward. This was a read-only source comparison; no tests or service launches were run.

@OrcaWin OrcaWin closed this Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant