diff options
Diffstat (limited to 'docs/Review.md')
| -rw-r--r-- | docs/Review.md | 222 |
1 files changed, 0 insertions, 222 deletions
diff --git a/docs/Review.md b/docs/Review.md deleted file mode 100644 index ed9f60c..0000000 --- a/docs/Review.md +++ /dev/null @@ -1,222 +0,0 @@ -# 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はいずれも20/20 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 | 0.854 s / 12,500,992 bytes peak | 4.327 s | 0.875 s / 11,616,256 bytes peak | -| 100,000 | 1.605 s / 16,285,696 bytes peak | 19.107 s | 1.774 s / 16,203,776 bytes peak | - -既存の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で全体を一括確保する - -対応済み。2-pass readerは最大1行とcompact ID indexだけを保持し、第2 passで選択recordを -`active.import.tmp`へ直接serializeする。flush後のatomic replaceまでは旧logを変更しない。 -100,001件、decoded上限±1、allocation全点、open/read/write/flush/replace失敗、dangling参照、 -truncated/長行を専用testで検証した。 - -`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 を記録 |