From 751999a82b8854ef5231e0ded5fe022b1ea5d8ff Mon Sep 17 00:00:00 2001 From: tanaka takashi Date: Sat, 4 Jul 2026 18:44:17 +0900 Subject: Harden backup import and restore coverage --- docs/Review.md | 217 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 217 insertions(+) create mode 100644 docs/Review.md (limited to 'docs/Review.md') 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 を記録 -- cgit v1.2.3