Skip to content

store を node:sqlite に刷新(差し替え可能なストア層の背後で)#78

Merged
kosako merged 2 commits into
mainfrom
feature/74-sqlite-store
Jun 15, 2026
Merged

store を node:sqlite に刷新(差し替え可能なストア層の背後で)#78
kosako merged 2 commits into
mainfrom
feature/74-sqlite-store

Conversation

@kosako

@kosako kosako commented Jun 14, 2026

Copy link
Copy Markdown
Owner

Closes #74

路線 2 の最後。事前に方式を相談し、node:sqlite + 差し替え可能なストア層で合意。

設計

  • 永続化を server/store.js の小さな非同期インターフェースの背後に隔離: init / insert / get / list / update / delete / count / close。receiver はこの契約だけを呼ぶ
  • 実装は createSqliteStorenode:sqliteDatabaseSync)。createStore({ backend }) ファクトリ経由で、将来 createMysqlStore 等を同契約で足せば receiver は無変更。インターフェースは async なのでネットワーク系ドライバもそのまま乗る(要件: 後で MySQL 等に差し替え可能)
  • スキーマは可搬性重視の最小構成: id + フィルタ用 project_id / demo_id / status / received_at 列 + 全文を JSON 列 data。payload にフィールドが増えてもスキーマ変更不要、他の SQL エンジンへも素直にマップ
  • feedback.json は起動時に一度だけ sqlite へ移行し feedback.json.migrated-<ts> に退避。壊れていれば feedback.json.corrupt-<ts> に退避して空起動(feedback.json の破損で全データが黙って消える(非アトミック書き込み) #47 のクラッシュセーフ挙動を踏襲)
  • node:sqlite の ExperimentalWarning はその 1 行だけ抑止
  • env: FEEDBACK_DB_PATH(db パス)/ FEEDBACK_STORE_PATH(移行元 JSON)

外から見た振る舞いは不変

widget の payload、GET /feedback.json の応答、inbox の見た目は変わりません。全書き換え→INSERT/UPDATE/DELETE、メモリ全走査→WHERE で索引、になっただけです。

検証

  • npm run check 全パス(テスト 61 件。旧 corrupt-store テストを「起動時に corrupt legacy を退避」に書き換え、「valid legacy → sqlite 移行 + archive」テストを追加。既存の post/import/status/github/delete/filter は sqlite 実装に対して green)
  • 手動 e2e: legacy json の移行 → 元ファイル archive / 新規 post が newest-first / プロセス再起動後もデータ保持(再移行は起きない)/ db ファイル生成 を確認

レビュー観点(依頼するなら)

  • async 契約が MySQL 差し替えに十分か(トランザクション境界・接続管理の余地)
  • JSON 列 + 抽出列という持ち方の是非

The receiver kept all feedback in a single JSON array and rewrote the
whole file on every change, with queries and deletes scanning the
array in memory — fine for a prototype, but it degrades by the
hundreds of items. Persistence now lives in server/store.js behind a
small async interface (init/insert/get/list/update/delete/count) with
a node:sqlite implementation; a future backend like MySQL implements
the same contract without touching the receiver. The interface is
async so a network-backed driver fits the same shape.

A minimal portable schema (id + project/demo/status/received_at
columns for filtering, the full object in a JSON column) keeps payload
growth schema-free and maps cleanly to other SQL engines. On startup a
legacy feedback.json is migrated once and archived; a corrupt one is
set aside, preserving the old crash-safe behavior.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@kosako

kosako commented Jun 14, 2026

Copy link
Copy Markdown
Owner Author

🔍 レビュー依頼(→ Codex)

  • 観点: store を node:sqlite に刷新し、差し替え可能なストア層(server/store.js)の背後に隔離する変更。重点は (1) 差し替え要件 — async 契約が MySQL 等の別バックエンドに十分か(トランザクション境界・接続管理・並行アクセスの前提)、(2) 挙動の同一性 — 全書き換え→sqlite で post/import/status/github/delete/filter の観測挙動が変わっていないか、(3) 移行の安全性 — 旧 feedback.json の一度きり移行・corrupt 退避・newest-first 順序の保持、(4) sqlite 使用上の落とし穴(SQL インジェクション、prepared statement、INSERT OR REPLACE の是非、JSON 列 + 抽出列の整合)
  • ランク: 🔴 must(merge 前必須)/ 🟡 should(推奨・対応は依頼元判断)/ ⚪ nit(任意)
  • 結果はこの PR にコメントで返します

- Reject duplicate ids with 409 instead of INSERT OR REPLACE silently
  overwriting an existing row
- Make update/delete atomic by reading synchronously (no await between
  read and write), closing the lost-update window
- Treat empty query filter values as "no filter" so NULL-column rows
  still match, matching the inbox "all" option
- Wrap the legacy migration in a transaction so an interruption rolls
  back and the next startup retries instead of stranding rows
- Restore process.emitWarning right after requiring node:sqlite

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@kosako

kosako commented Jun 14, 2026

Copy link
Copy Markdown
Owner Author

📋 レビュー結果(by Codex / GPT-5.5)

判定: ⛔ 要対応(must 2 件)→ 対応済み、再検証パス

Codex が must 2・should 3 を指摘。いずれも今回導入したコードの正当な指摘で、全件対応しました。

🔴 must(対応済み)

  1. server/store.js INSERT OR REPLACE が既存行を上書き(同一 id で silent data loss) → plain INSERT に変更し、重複 id は 409 で拒否(ids は構造上ユニーク。上書きも二重行も避け、loud に失敗させる)。移行時は INSERT OR IGNORE で先勝ちにし、移行全体は止めない。
  2. update/delete が read→write 間の await で TOCTOU(並行更新で lost update) → 内部の await this.get() を同期 SELECT に置換し、read-modify-write を await 無しで実行。単一スレッド上で原子的になる(ネットワーク系バックエンドではトランザクションで包む旨をコメント明記)。

🟡 should(対応済み)

  1. 空文字フィルタ(?projectId=)が NULL 行にマッチしなくなる → 空クエリ値は「フィルタなし(all)」扱いに統一(inbox の "all" 選択と一致)。
  2. 移行がトランザクション外で中断すると残りが永久に取り込まれない → 移行を BEGIN/COMMIT で all-or-nothing 化。失敗時は ROLLBACK+legacy ファイル温存で次回再試行。
  3. process.emitWarning 上書きがプロセス全体に永続 → require 直後に元のハンドラへ復元し、上書きを require 呼び出しの間だけに限定。

⚪ nit

なし

検証

  • npm run check 全パス(テスト 62 件。重複 id → 409 のテストを追加。移行・corrupt 退避・filter は既存テストで green)

修正コミットをこの後 push します。

@kosako kosako marked this pull request as ready for review June 15, 2026 01:32
@kosako kosako merged commit bd8de74 into main Jun 15, 2026
@kosako kosako deleted the feature/74-sqlite-store branch June 15, 2026 01:32
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.

store を単一 JSON 全書き換えから卒業する(NDJSON or node:sqlite)

1 participant