summaryrefslogtreecommitdiff
path: root/docs/Review.md
diff options
context:
space:
mode:
authortanaka takashi <isazaka@gmail.com>2026-07-04 18:44:17 +0900
committertanaka takashi <isazaka@gmail.com>2026-07-04 18:44:17 +0900
commit751999a82b8854ef5231e0ded5fe022b1ea5d8ff (patch)
treed09653d2955f0f5354e9ef76f9bc550fb8f0732a /docs/Review.md
parent21dda921887da2515d1eba0d3d26e2036bf9f7f7 (diff)
Harden backup import and restore coverage
Diffstat (limited to 'docs/Review.md')
-rw-r--r--docs/Review.md217
1 files changed, 217 insertions, 0 deletions
diff --git a/docs/Review.md b/docs/Review.md
new file mode 100644
index 0000000..b4d351a
--- /dev/null
+++ b/docs/Review.md
@@ -0,0 +1,217 @@
+# Critical Review and Next Tasks
+
+## 1. 結論
+
+現行実装は append-only log、durability flush、CRC/sequence 検証、snapshot fallback、単一 writer lock、更新 revision、検索説明性という基盤がよく整理されている。明白な恒久メモリリークは静的確認と実行可能だった ASan test では確認されなかった。
+
+レビューで指摘したR1~R7は対応済みである。protocol E2Eを復旧し、importの完全decode、厳密整数、
+bounded line streaming、atomic export、memory estimate、セキュリティ運用記載を追加した。
+
+## 2. 診断条件
+
+- 比較基準: commit `1efecaef1f867cb218366e2b4ab4b901fdd2835f` 以降
+- 対象: domain、storage、MCP tools、CLI、system abstraction、tests、docs
+- 実行: Windows VS2026 Debug と ASan の build/CTest
+- 対応後結果: Debug、Release、ASanはいずれも17/17 pass
+
+復旧したprotocol workflowは各構成で検索1,000回、MCP lifecycle、rebuild、restartまで成功した。
+
+Release import/export benchmark(2026-07-04、同一開発機)の結果は次のとおり。
+
+| records | import | export | round-trip import |
+|---:|---:|---:|---:|
+| 10,000 | 0.277 s | 0.165 s | 0.284 s |
+| 50,000 | 4.668 s | 4.226 s | 4.794 s |
+| 100,000 | 19.725 s | 18.465 s | 19.644 s |
+
+既存のMCP workload計測では10,000件が18,960,384 bytes、50,000件が51,240,960 bytesの
+peak working setだった。100,000件はCLI上限とround-tripを確認し、実プロセスの継続監視には
+`get_system_health.memory_estimate`とOS working setを併用する。
+
+## 3. Findings
+
+以下は発見時の記録である。R1~R4、R7は実装と回帰試験で解消し、R5はhealth内訳と運用上限を
+追加、R6はthreat modelとACL推奨を文書化した。disk-backed化、retention、組込み暗号化は
+引き続きv1の対象外である。
+
+### R1 — High: 長い JSON content の import が内容を変える
+
+`src/cli/import.c` の `import_parse_message_record` は content を固定 4096-byte buffer で decode する。収まらない場合は JSON token の quote 内部を raw copy するため、`\\n`、`\\"`、`\\\\`、`\\uXXXX` が元の文字へ decode されない。
+
+影響:
+
+- export → import round-trip で Message content が変化する
+- backup restore の完全性を保証できない
+- raw escape が検索結果や host prompt に異なる意味で現れる
+
+修正:
+
+1. `mjson_get_string` の必要長を使って動的 buffer を確保する helper を用意する
+2. session、summary、tag 等を含む全 JSON string を同じ helper に統一する
+3. decode 後の byte length と UTF-8 を検証する
+
+完了条件:
+
+- 4 KiB未満・超、1 MiB境界で escape 4種を含む export/import byte-exact round-trip
+- invalid escape、decoded overflow、embedded NUL を拒否
+- ASan/Debug/Release success
+
+### R2 — High: protocol E2E test が削除されたまま CTest に登録されている
+
+commit `2861118` で `tests/mcp_protocol_test.py` が削除されたが、`tests/CMakeLists.txt` は引き続き登録している。実装状況文書もこの test が検索1000回、context/session、delete/rebuild/restart を検証すると記載していた。
+
+影響:
+
+- clean checkout で test suite が必ず failure
+- MCP schema、workflow、response budget、restart regression が未検証
+- ASan の memory safety 評価範囲が縮小
+
+修正:
+
+- test を履歴から復旧し、現行 tool description/contract に合わせる
+- 一時的に CTest 登録だけ消して green に見せない
+- test source の存在を configure 時に検証する
+
+完了条件:
+
+- clean checkout の Debug/Release/ASan が全件成功
+- write → update → history/context/sessions → repeated search → delete → rebuild → restart を通す
+
+### R3 — Medium: import size guard が約1 TiBで全体を一括確保する
+
+`IMPORT_MAX_LINE * 1024 * 1024` は約1 TiBとなる。実際には `ftell` と address space が先に制限するが、入力ファイル全体を `malloc` し、さらに parse 済み Message を保持する。
+
+影響:
+
+- 大きな import による process memory exhaustion
+- 通常 corpus でも raw JSONL と parsed records の同時保持による高い peak memory
+
+修正:
+
+- manifest と record を行単位で streaming parse する
+- 1行上限、総 record 数、総 decoded bytes の明示上限を設ける
+- transaction 要件は temporary store/log を使って維持する
+
+完了条件:
+
+- 上限超過を allocation 前に拒否
+- 50,000 record import の peak memory を計測・記録
+- truncated line と極端な長行を安全に拒否
+
+### R4 — Medium: JSON number を `double` から直接整数変換している
+
+timestamp、updated_at、revision、importance は `mjson_get_number` の `double` を直接整数へ cast する。finite、整数性、変換前範囲を十分に検証していない。
+
+影響:
+
+- 2^53超で精度を失う
+- fractional importance/revision を暗黙に切り捨てる
+- NaN、Infinity、整数型範囲外で未定義または処理系依存の変換になり得る
+
+修正:
+
+- JSON token を整数文字列として厳密 parse する、または finite/range/floor 検証後に cast する
+- timestamp、revision、importance ごとに許容範囲を定義する
+
+完了条件:
+
+- min/max、max±1、fraction、exponent、NaN相当、Infinity相当の table-driven test
+- export した全有効値が exact round-trip
+
+### R5 — Medium: 正常なデータ増加がプロセスメモリを増やし続ける
+
+これは解放忘れではなく設計上の resident growth である。live Message content に加えて、history、search posting、relation index をメモリに保持する。retention と tier lifecycle は未実装である。
+
+影響:
+
+- 長期運用では live corpus に比例して working set が増える
+- compaction は dead record を減らすが live Message は減らさない
+- 1 MiB content が多数ある場合に早く顕在化する
+
+修正候補:
+
+- 想定最大 corpus と memory budget を先に定義する
+- Message body の disk-backed/lazy load、index圧縮、retention のいずれかを選択する
+- health に index/body memory estimate を追加する
+
+完了条件:
+
+- 10k/50k/100k件と large-content corpus の peak/steady memory benchmark
+- rebuild と検索反復後に基準へ戻ることを確認
+
+### R6 — Medium: data at rest の機密性・改変耐性は OS ACL 任せ
+
+log、snapshot、JSONL backup は平文である。CRC32 は偶発破損検出であり、悪意ある改変の認証ではない。stdio host は接続後、全 tool 権限を持つ。
+
+修正:
+
+- threat model と data directory ACL の推奨設定を README に記載する
+- export file を同等の機密データとして扱うことを明記する
+- 暗号化が必要な deployment は上位暗号化 filesystem または別設計とする
+- 将来暗号化する場合は鍵管理、nonce、version migration を先に設計する
+
+### R7 — Low: export の close/flush error を成功扱いし得る
+
+record 書込み中の `fprintf` failure は検出するが、最終 `fclose` の failure を確認していない。遅延 write error や媒体容量不足が close 時に判明した場合、成功 exit となり得る。
+
+修正:
+
+- `fflush` と `fclose` の戻り値を検査する
+- backup file を temporary path に書き、成功後に replace する option を検討する
+
+## 4. 設計適合性
+
+現行の authoritative contract に対する適合度は概ね高い。
+
+適合している主な点:
+
+- MCP/domain/storage/sys の責務分離
+- append-only PUT/UPDATE/DEL と optimistic revision
+- 成功応答前 durability flush
+- CRC、sequence、length、version の fail-closed validation
+- incomplete tail の限定的 recovery
+- snapshot generation/offset validation と full replay fallback
+- exclusive data-directory lock
+- read 系 0 write / 0 flush
+- response size budget と安定 cursor
+
+検証不足:
+
+- valid snapshot + tail replay の専用 test
+- compaction の log replace と snapshot replace 間の crash
+- replace/flush failure matrix
+- protocol E2E と長時間 ASan workflow
+
+## 5. フィードバック反映度
+
+優先7項目は5件達成、2件部分達成で、およそ80%を反映している。
+
+- 達成: update、kind、context、sessions、metadata filter/update
+- 部分達成: backupは実装済みだがR1あり、2段階保存は表現契約のみ
+- 未実装の発展要望: 自動保存、自動要約・固定化、access reinforcement、decay
+
+構造化と検索経路は大きく改善した一方、「利用と時間によって状態が変わる生物的記憶」は意図的に host policy のままである。
+
+## 6. 推奨実装順序
+
+| 順位 | Task | 依存 | 完了判定 |
+|---:|---|---|---|
+| 1 | R2 protocol E2E test 復旧 | なし | clean checkout 全構成 green |
+| 2 | R1 JSON string decode 修正 | 1 | 1 MiB escape round-trip |
+| 3 | R4 strict integer parser | 2 | 境界値 test green |
+| 4 | R3 streaming import | 2,3 | 上限・rollback・peak memory 合格 |
+| 5 | snapshot/compaction crash matrix | 1 | 全停止点から一貫して復旧 |
+| 6 | R7 export completion検査 | なし | close failure test |
+| 7 | R5 memory budget設計 | benchmark後 | 最大 corpus と方式を決定 |
+| 8 | R6 security運用文書 | なし | threat model/ACL/backup記載 |
+
+## 7. Release gate
+
+次を満たすまで backup/import を安定版として扱わない。
+
+- Debug、Release、ASan の CTest が clean checkout で全件成功
+- 1 MiB content の export/import exact round-trip
+- strict numeric boundary test 成功
+- import 上限超過時に bounded memory で安全に失敗
+- valid snapshot tail replay と compaction crash recovery 成功
+- 50,000 record benchmark に I/O、時間、peak memory を記録