refactor(apns): move registrations to shared SQLite#108543
Conversation
openclaw-barnacle
Bot
added
docs
b37d306 to
fef6063
Compare
|
Codex review: needs real behavior proof before merge. Reviewed July 15, 2026, 10:35 PM ET / July 16, 2026, 02:35 UTC. Summary PR surface: Source +1144, Tests +545, Docs +6, Generated +13, Other 0. Total +1708 across 19 files. Reproducibility: yes. from source and documented workflow: replace the package through the supported manual npm, pnpm, or Bun path, leave Review metrics: 2 noteworthy metrics.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Copy recommended automerge instructionNext step before merge
Security Review findings
Review detailsBest possible solution: Make every supported upgrade converge APNs state before Gateway readiness: run the migration in managed upgrade finalization and fail startup with actionable Do we have a high-confidence way to reproduce the issue? Yes from source and documented workflow: replace the package through the supported manual npm, pnpm, or Bun path, leave Is this the best way to solve the issue? No. The database-first store and Doctor importer are appropriate, but retiring the runtime reader is safe only after every supported upgrade path migrates first or startup fails closed with an actionable repair path. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 154d1277e12e. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +1144, Tests +545, Docs +6, Generated +13, Other 0. Total +1708 across 19 files. View PR surface stats
Acceptance criteria:
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
4b91c26 to
22edde7
Compare
|
Merged via squash.
|
The canonical schema gate rejects non-STRICT tables, failing every gateway-suite shard since #108543 added the tombstones table without STRICT. Align it with every sibling table and regenerate the schema mirror.
…nsistent SQLite-only path - Remove sqliteReadReady flag and markSqliteReadReady() — SQLite reads and writes are now unified: when stateDb is available, both use SQLite - Remove useSqliteForReads getter — replaced with direct this.stateDb check - createStore() now only catches the specific 'SQLite support unavailable' error (permanent Node version limitation); other errors propagate - Doctor migration no longer calls markSqliteReadReady() — unnecessary when reads and writes are consistent Follows the established APNs SQLite migration pattern (PR openclaw#108543): - SQLite-only runtime when state DB is available - Doctor-only legacy file import (no runtime fallback to filesystem)
* refactor(apns): move registrations to SQLite * fix(apns): satisfy strict migration typing * chore(apns): remove obsolete lint suppression * chore: prune APNs max-lines baseline * refactor(apns): keep store internals private * fix(apns): preserve stored relay origins * fix(apns): advance versions across deletion * fix(apns): harden legacy state import * fix(apns): redact legacy import errors * fix(apns): reserve version successor * fix(apns): bound legacy timestamp imports * refactor(apns): extract payload builders
The canonical schema gate rejects non-STRICT tables, failing every gateway-suite shard since openclaw#108543 added the tombstones table without STRICT. Align it with every sibling table and regenerate the schema mirror.
1 participant
Closes #108535
What Problem This Solves
Existing APNs registrations live in a JSON sidecar even though they are durable runtime state. Whole-file rewrites have only process-local serialization, and simply deleting or continuing to read that store would either lose shipped iOS push registrations or leave two competing authorities that can resurrect invalidated devices.
Why This Change Was Made
APNs registration reads, writes, batch lookup, and compare-and-delete now use the shared Kysely-backed SQLite state database. A strict, exclusive Doctor migration streams the retired JSON store without a whole-file memory limit, preserves canonical SQLite rows, honors deletion tombstones, records and verifies a transactional receipt, and removes the legacy source only after the import commits.
The APNs delivery/authentication module remains separate from the new storage owner. Runtime code has no JSON fallback, dual-write, or lazy import path; legacy handling exists only under
openclaw doctor --fix.User Impact
Gateways keep direct and relay APNs registrations across upgrade and restart while moving to one transactional state authority. Invalidated registrations stay deleted even if an old or interrupted JSON source reappears. Existing installations run
openclaw doctor --fixto import the shipped file before the SQLite-only runtime uses those registrations.Evidence
Implementation and review used AI assistance.