From 4b8aee909bcad75dad4b744ceb0ecd6967d47dbc Mon Sep 17 00:00:00 2001 From: Hare Date: Wed, 20 May 2026 04:53:47 +0900 Subject: [PATCH] =?UTF-8?q?ticket:=20entry-hash-abolish=20=E5=AE=8C?= =?UTF-8?q?=E4=BA=86?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- TODO.md | 1 - tickets/entry-hash-abolish.md | 48 ------------------------ tickets/entry-hash-abolish.review.md | 56 ---------------------------- 3 files changed, 105 deletions(-) delete mode 100644 tickets/entry-hash-abolish.md delete mode 100644 tickets/entry-hash-abolish.review.md diff --git a/TODO.md b/TODO.md index 6388aff1..f90e6612 100644 --- a/TODO.md +++ b/TODO.md @@ -8,7 +8,6 @@ - Pod: 任意ターンからの Fork(複数ターン巻き戻しを汎用化) → [tickets/pod-session-fork.md](tickets/pod-session-fork.md) - Pod: Inbound PodEvent ハンドリングの重複を統合 → [tickets/pod-inbound-pod-event-dedup.md](tickets/pod-inbound-pod-event-dedup.md) - 永続化層整理 (Storage) - - Entry hash chain 廃止 → [tickets/entry-hash-abolish.md](tickets/entry-hash-abolish.md) - SessionId → SegmentId リネーム → [tickets/segment-rename.md](tickets/segment-rename.md) - Session (Segment 群の grouping) 導入 → [tickets/session-grouping-introduce.md](tickets/session-grouping-introduce.md) - live auto-fork の marker 形式確定 → [tickets/live-fork-marker.md](tickets/live-fork-marker.md) diff --git a/tickets/entry-hash-abolish.md b/tickets/entry-hash-abolish.md deleted file mode 100644 index cb1ba59d..00000000 --- a/tickets/entry-hash-abolish.md +++ /dev/null @@ -1,48 +0,0 @@ -# session-store: Entry hash chain の廃止 - -## 背景 - -session-store の各 entry は SHA-256 hash chain (`prev_hash` → `hash`) で連結されており、`HashedEntry` として JSONL に 1 行ずつ書かれる。実際に効いている用途は以下の 2 つだけ: - -1. `ensure_head_or_fork` (`crates/pod/src/pod.rs:1591`) — store 末尾と Pod 保持の `head_hash` を比較して auto-fork。**末尾識別子があれば良い**。 -2. `fork_at(source_id, at_hash)` (`crates/session-store/src/session.rs:425`) — 過去 entry からの fork。`pod-session-fork` の入口仕様は turn 番号であり、entry hash は内部 pointer に過ぎず turn boundary (TurnEnd entry の index) で代替可能。 - -宣伝されている改竄検知 (tamper-evident chain) は walk して verify するルートがコード上に存在せず、削除しても regression にならない。 - -write 経路は既に sync 化済み (`6e5b148`)。前提足場は揃っている。 - -## 要件 - -- `HashedEntry` 廃止、JSONL は 1 行 1 `LogEntry`。 -- `compute_hash` / `build_chain` / `EntryHash` の撤去(外部に公開している場合は呼び出し元を含めて)。 -- `SessionOrigin.at_hash` → `at_turn_index` (TurnEnd entry 由来の turn 番号) に置換。 -- `ensure_head_or_fork` の検知ロジックを末尾 seq 比較ベースに置換。形式は実装時に決める(terminal marker entry / 末尾 seq の何れか)。 -- **`session_head` mutex の撤去**。hash chain が無くなる結果として "head_hash を直前 entry から取得して次へ渡す" という serialize 必須の依存が消える。1 行 < `PIPE_BUF` (Linux 4KB) の `O_APPEND` write は kernel が atomic に直列化するため user space で mutex 不要。 -- 既存 JSONL の読み込み互換は不要(プロジェクト方針として後方互換性は作らない)。 - -## 完了条件 - -- `HashedEntry` / `prev_hash` / `compute_hash` / `build_chain` / `EntryHash` がコードベースから消えている。 -- `SessionOrigin` が `at_turn_index` を保持し、`fork_at` も同 API になっている。 -- `session_head` mutex への参照が無く、`SessionLogWriter` 系は writer ハンドルを `Arc` で持つだけになっている。 -- `cargo check --workspace` および `cargo test -p session-store -p pod` が通る。 -- 既存 session を新規再生成して JSONL が 1 行 1 `LogEntry` になっていることを目視確認できる。 - -## 範囲外 - -- `SessionId` → `SegmentId` のリネーム(別チケット `segment-rename`)。 -- 新 `SessionId` (Segment 群の grouping) 導入(別チケット `session-grouping-introduce`)。 -- live auto-fork の marker 形式の最終決定(別チケット `live-fork-marker`、ここでは末尾 seq 比較相当の最小実装で繋ぐ)。 - -## 関連 - -- `crates/session-store/src/session_log.rs` -- `crates/session-store/src/session.rs` -- `crates/pod/src/pod.rs:1591` `ensure_head_or_fork` 経路 -- `tickets/segment-rename.md` (後続) - -## Review - -- 状態: Approve -- レビュー詳細: [./entry-hash-abolish.review.md](./entry-hash-abolish.review.md) -- 日付: 2026-05-20 diff --git a/tickets/entry-hash-abolish.review.md b/tickets/entry-hash-abolish.review.md deleted file mode 100644 index e234957c..00000000 --- a/tickets/entry-hash-abolish.review.md +++ /dev/null @@ -1,56 +0,0 @@ -# Review: session-store: Entry hash chain の廃止 - -対象コミット: `d5dff6d update: entry hash chain と session_head mutex を撤廃` -ベース: `develop` - -## 前提・要件の確認 - -- `HashedEntry` 廃止 / JSONL = 1 行 1 `LogEntry`: 達成。 - - `crates/session-store/src/session_log.rs` から型・関連 impl が削除。 - - `FsStore::append` / `read_all` / `create_session` 全部 `&LogEntry` / `Vec` に書き換え (`crates/session-store/src/fs_store.rs:68-110`)。 -- `compute_hash` / `build_chain` / `EntryHash` 撤去: 達成。`grep` でコードベース上に残存なし。`crates/session-store/src/lib.rs:48` の re-export からも消えている。 -- `SessionOrigin.at_hash → at_turn_index`: 達成 (`crates/session-store/src/session_log.rs:167-177`)。doc コメントで「`TurnEnd` の `turn_count` 由来。`0` は SessionStart 直後」と意味も明文化済み。 -- `ensure_head_or_fork` を末尾比較ベースに置換: 達成。entry count 比較に統一 (`crates/session-store/src/session.rs:99-118`、Pod 側は `crates/pod/src/pod.rs:1619-1666`)。`Store::read_head_hash` も `read_entry_count` に改名されインタフェースとして整合 (`crates/session-store/src/store.rs:58-64`)。 -- `session_head` mutex 撤去 / writer ハンドル = `Arc`: 達成。`Arc>` を `Arc` (`ArcSwap` + `AtomicUsize`) に置換し (`crates/pod/src/pod.rs:45-86`)、`parking_lot` 依存も削除 (`crates/pod/Cargo.toml:33`)。`LogWriterHandle` は store + state + sink の薄い 3 タプル。 -- 既存 JSONL の読み込み互換不要: 仕様通り。新フォーマットは「素の `LogEntry` (`tag = "kind"`)」で、旧 `HashedEntry` ラッパー行は deserialize できないが、ticket の明示的な範囲外。 - -## 完了条件の対応 - -- `HashedEntry / prev_hash / compute_hash / build_chain / EntryHash` 残存無し: 確認 (`grep -r` で hit 無し、コメント/テスト中の文字列残存のみ — 後述 nit)。 -- `SessionOrigin.at_turn_index` & `fork_at(source_id, at_turn_index)`: 確認 (`session.rs:373-409`)。 -- `session_head` mutex 参照消失 / Writer は `Arc` のみ: 確認 (`grep -rn parking_lot|SyncMutex|SessionHead crates/pod/src/ crates/session-store/src/` で 0 件)。 -- `cargo check --workspace` / `cargo test -p session-store -p pod`: 通る。 - - `pod` 側で `double_run_returns_error` がたまに失敗するが、`KNOWN_ISSUES.md` に既知 flakiness として登録済み (本チケット起因ではない、再実行で pass)。 -- 既存 session を新規再生成して 1 行 1 `LogEntry`: 形式上は `FsStore::append` が `serde_json::to_string(&LogEntry)` を 1 行で書き出すだけになっており、目視確認できる構造である。 - -## アーキテクチャ・スコープ - -- スコープ厳守: 範囲外と宣言した「`SessionId → SegmentId` rename / Session grouping / live-fork marker 形式の最終決定」のいずれにも踏み込んでいない。`fork_at` の API は `at_turn_index: usize` という最小限の置換で止まっており、後続チケットの設計余地を温存している。 -- レイヤ境界: `session-store` は依然として低レベル persistence、Pod が状態保持と fork 判定を行うという責務分割が維持されている。`LogWriterHandle` / `SessionState` は pod クレート内で完結し、外部に新しい抽象を生やしていない。 -- 不要な後方互換無し: 旧 JSONL の読み出しを試みる shim も無く、設計方針 (CLAUDE.md「変更量を最小にするために設計を歪めたり、設計問題に対して不必要な後方互換性を作らない」) に合致。 -- 依存追加 `arc-swap = "1.9.1"`: `cargo add` で入った形跡 (pinned patch version)。`manifest` クレートで既に同 crate を使っており、新規導入ではない。プレフィックス無し短い名前ポリシーと無関係 (third-party)。問題なし。 - -## 指摘事項 - -### Blocking - -なし。 - -### Non-blocking / Follow-up - -- **mirror と disk の順序乖離リスク** — `crates/pod/src/pod.rs:622-636` (`commit_entry`) と `crates/pod/src/pod.rs:108-117` (`LogWriterHandle::append_entry`) は、`store.append` (kernel が直列化) と `sink.publish` (mirror mutex 内で直列化) を別ロックで行う。現状の Pod runtime では実質的に append は単一タスク (worker.run 駆動経路) に閉じているため race は発生しない。ただし将来「Pod の独立タスクから `LogWriterHandle::append_entry` を発火する」変更が入ると、kernel 順序と mirror 順序が一致しない可能性がある (disk に B が後着でも mirror の `subscribe_with_snapshot` プレフィックスでは A の前に B が並ぶ等)。ticket は「`O_APPEND` は kernel が atomic に直列化」までしか論証しておらず、mirror 側の不変条件は明示されていない。後続 `live-fork-marker` で marker entry 形式を確定する際に併せて「mirror の順序保証は append 経路の単一性に依存する」旨を pod 側 doc に書き残しておくと事故防止になる。 -- **`FsStore::read_entry_count` の O(n) 読み込み** — `fs_store.rs:121-122` は `fs::read_to_string` でファイル全体を読んで行数を数える。ターンの度に `ensure_head_or_fork` の中で呼ばれるため、長期セッション (数千 entry) では毎ターン無駄が出る。代替: ファイルサイズ + 末尾シーク + line counter、もしくは entry 数が膨らんできた段階で別途 metadata ファイル化。`live-fork-marker` チケットで marker entry を導入する際の見直しポイント。 -- **stale doc / test コメント** — 以下に旧用語 (`head_hash` / `SessionHead`) が残っており、ticket 完了後にも誤読を招く: - - `crates/session-store/src/lib.rs:22` doc example の `let (session_id, head_hash) = create_session(...)` (API は既に `SessionId` 単独を返す)。 - - `crates/pod/tests/restore_test.rs:66` のコメント「`collect_state` returns `head_hash = None`」。 - - `crates/pod/tests/compact_events_test.rs:523-598` の `SessionHead` / `head_hash` を前提とした説明文。テスト自体は session_id 比較で正しく動くが、説明が現実装と乖離している。 - 本チケット範囲を厳密に解釈すれば追従不要だが、近くを触る次のチケット (`segment-rename` 等) で同時に潰しておくのが望ましい。 - -### Nits - -- `crates/pod/Cargo.toml:33` は `arc-swap = "1.9.1"` と patch まで pin されている。`manifest` 側は `"1"`。整合の必要はないが、workspace.dependencies に上げる選択肢もある (将来 1.x bump 時に一箇所で動く)。 -- `pod.rs:2148-2165` 圧縮経路で `let old_session_id = self.session_state.session_id();` の直後に `let w = self.worker.as_ref().unwrap();` を 2 回行っており (1 度目は turn_count 用、2 度目は entry 構築用)、まとめて 1 つの `let w = ...` で済む。可読性のみの話。 - -## 判断 - -**Approve** — チケットの要件・完了条件は全て充足され、削除と置換が clean に行われている。`session_state` の lock-free 化はチケットの論拠 (single-appender + `O_APPEND` 原子性) の範囲内では正しく、`ensure_head_or_fork` の entry-count 比較は hash 比較と検出力上等価 (writer の自前 tally と on-disk 行数の乖離 = 他プロセス侵入)。指摘した non-blocking 項目はいずれも本チケットのスコープ外、または後続 `live-fork-marker` / `segment-rename` のタイミングで吸収できる内容。