From a9340a8817f349d25a77a589ee2c60b863b4ac4f Mon Sep 17 00:00:00 2001 From: Hare Date: Wed, 20 May 2026 06:42:09 +0900 Subject: [PATCH 1/5] =?UTF-8?q?feat:=20live=20auto-fork=20=E3=81=AE=20mark?= =?UTF-8?q?er=20=E5=BD=A2=E5=BC=8F=E3=82=92=E7=A2=BA=E5=AE=9A=EF=BC=88seq?= =?UTF-8?q?=20=E6=AF=94=E8=BC=83=20+=20forked=5Ffrom=20=E8=A8=98=E9=8C=B2?= =?UTF-8?q?=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 方針: 末尾 entry-count 比較で検知し、元 Segment は immutable のまま （terminal marker を書き戻さない）。fork lineage は新 Segment の SegmentStart.forked_from に前向きに記録するため、log だけから辿れる。 過去 fork と対称で、nested fork も marker 位置の調停が不要。 - session-store ensure_head_or_fork に at_turn_index 引数を追加し 新 Segment へ forked_from を記録 - pod ensure_segment_head の auto-fork も同様に forked_from を記録 （at_turn_index = writer の現 turn_count） - fork_at の doc に「元 Segment を mutate しない」invariant を明記 - test: nested past-fork が祖先を不変に保つ / Pod 並行 writer drift で auto-fork し forked_from を記録 / 元 Segment に marker が書かれない --- crates/pod/src/pod.rs | 15 +++- crates/pod/tests/compact_events_test.rs | 85 +++++++++++++++++++++- crates/session-store/src/segment.rs | 41 ++++++++++- crates/session-store/tests/session_test.rs | 74 ++++++++++++++++++- 4 files changed, 206 insertions(+), 9 deletions(-) diff --git a/crates/pod/src/pod.rs b/crates/pod/src/pod.rs index def8d557..c88e2276 100644 --- a/crates/pod/src/pod.rs +++ b/crates/pod/src/pod.rs @@ -1672,9 +1672,13 @@ impl Pod { return Ok(()); } // Auto-fork within the same Session: mint a fresh Segment and - // switch to it. The new SegmentStart entry replaces the mirror - // and is broadcast through the sink so existing subscribers - // reset their view. + // switch to it. The source segment is left immutable (no terminal + // marker is written back); the fork relationship is recorded + // forward on the new segment's `forked_from`, with `at_turn_index` + // = the writer's current turn (its in-memory history reflects + // state up to that turn). The new SegmentStart replaces the mirror + // and is broadcast through the sink so existing subscribers reset + // their view. let fork_segment_id = session_store::new_segment_id(); let entry = LogEntry::SegmentStart { ts: segment_log::now_millis(), @@ -1682,7 +1686,10 @@ impl Pod { system_prompt: w.get_system_prompt().map(String::from), config: w.request_config().clone(), history: to_logged(w.history()), - forked_from: None, + forked_from: Some(session_store::SegmentOrigin { + segment_id: loc.segment_id, + at_turn_index: w.turn_count(), + }), compacted_from: None, }; self.store diff --git a/crates/pod/tests/compact_events_test.rs b/crates/pod/tests/compact_events_test.rs index 3c6d13de..1376b85c 100644 --- a/crates/pod/tests/compact_events_test.rs +++ b/crates/pod/tests/compact_events_test.rs @@ -17,7 +17,7 @@ use llm_worker::llm_client::event::{Event as LlmEvent, ResponseStatus, StatusEve use llm_worker::llm_client::types::Item; use llm_worker::llm_client::{ClientError, LlmClient, Request}; use protocol::Event; -use session_store::FsStore; +use session_store::{FsStore, LogEntry, Store}; use tokio::sync::broadcast; use pod::Pod; @@ -213,6 +213,89 @@ fn system_texts_in_sink_session_start( Vec::new() } +/// Live auto-fork: when another writer extends the segment behind the +/// Pod's back, the next run's `ensure_segment_head` detects the +/// entry-count drift and branches into a fresh segment **within the same +/// Session**. The source segment is left immutable (no terminal marker +/// written back); the new segment records its parentage forward via +/// `SegmentStart.forked_from`. +#[tokio::test] +async fn concurrent_writer_drift_auto_forks_with_forked_from() { + // No compaction: keep run → run deterministic so each run consumes + // exactly one mock response and ensure_segment_head is the only fork + // trigger. + const NO_COMPACT_MANIFEST_TOML: &str = r#" +[pod] +name = "test-pod" +pwd = "./" + +[model] +scheme = "anthropic" +model_id = "test-model" + +[worker] +max_tokens = 100 + +[[scope.allow]] +target = "./" +permission = "write" +"#; + let client = MockClient::new(vec![single_text_events("first"), single_text_events("second")]); + let mut pod = make_pod_with_manifest(NO_COMPACT_MANIFEST_TOML, client).await; + + pod.run_text("first").await.unwrap(); + + let store = pod.store().clone(); + let session_id = pod.session_id(); + let source_segment_id = pod.segment_id(); + let source_len_before = store.read_all(session_id, source_segment_id).unwrap().len(); + + // Simulate a foreign writer appending to the same segment. This bumps + // the on-disk entry count past the Pod's own append tally without + // updating the Pod's `entries_written`. + store + .append( + session_id, + source_segment_id, + &LogEntry::UserInput { + ts: 9999, + segments: vec![protocol::Segment::text("interloper")], + }, + ) + .unwrap(); + + // Next run triggers ensure_segment_head, which sees the drift. + pod.run_text("second").await.unwrap(); + + // The Pod moved to a new segment in the same Session. + let new_segment_id = pod.segment_id(); + assert_ne!(new_segment_id, source_segment_id); + assert_eq!(pod.session_id(), session_id, "auto-fork stays in-Session"); + + // New segment records forked_from pointing at the source. + let new_entries = store.read_all(session_id, new_segment_id).unwrap(); + match &new_entries[0] { + LogEntry::SegmentStart { + session_id: seg_session, + forked_from: Some(origin), + .. + } => { + assert_eq!(*seg_session, session_id); + assert_eq!(origin.segment_id, source_segment_id); + } + other => panic!("expected SegmentStart with forked_from, got {other:?}"), + } + + // Source segment is unchanged except for the foreign append — the + // auto-fork wrote no terminal marker back into it. + let source_after = store.read_all(session_id, source_segment_id).unwrap(); + assert_eq!(source_after.len(), source_len_before + 1); + assert!(matches!( + source_after.last(), + Some(LogEntry::UserInput { .. }) + )); +} + #[tokio::test] async fn compact_emits_session_start_carrying_summary_and_task_snapshot() { let client = MockClient::new(vec![ diff --git a/crates/session-store/src/segment.rs b/crates/session-store/src/segment.rs index 00b62f65..503a5675 100644 --- a/crates/session-store/src/segment.rs +++ b/crates/session-store/src/segment.rs @@ -114,8 +114,30 @@ pub fn restore_by_segment( restore(store, session_id, segment_id) } -/// Check if the store's entry count still matches the writer's tally. -/// If not, auto-fork into a new segment within the same Session. +/// Live auto-fork on concurrent-writer detection. +/// +/// Checks whether the store's on-disk entry count still matches the +/// writer's own append tally. If they match, the writer still owns the +/// segment tail and nothing happens. If the store has grown behind the +/// writer's back, another process appended to the same segment — so we +/// branch into a fresh segment within the same Session. +/// +/// # Marker form +/// +/// Detection is by **tail entry-count comparison**, not by writing a +/// terminal marker into the source segment. The source segment is left +/// completely immutable — identical to the past-fork ([`fork_at`]) +/// invariant. The fork relationship is instead recorded forward on the +/// *new* segment's `SegmentStart.forked_from`, so the lineage is still +/// reconstructable from the logs alone (read each segment's +/// `SegmentStart`; follow `forked_from` / `compacted_from` backward). +/// Listing a parent's children is a cheap `list_segments(session_id)` +/// scan filtered on `forked_from.segment_id`. +/// +/// `at_turn_index` is the writer's current `turn_count`: the fork seeds +/// the new segment with the writer's in-memory history (which reflects +/// state up to that turn), so that is the divergence point relative to +/// the now-diverged source segment. /// /// Updates `segment_id` and `entries_written` in place when a fork occurs. pub fn ensure_head_or_fork( @@ -123,12 +145,14 @@ pub fn ensure_head_or_fork( session_id: SessionId, segment_id: &mut SegmentId, entries_written: &mut usize, + at_turn_index: usize, state: SegmentStartState<'_>, ) -> Result<(), StoreError> { let store_count = store.read_entry_count(session_id, *segment_id)?; if store_count == *entries_written { return Ok(()); } + let source_segment_id = *segment_id; let fork_id = crate::new_segment_id(); let entry = LogEntry::SegmentStart { ts: segment_log::now_millis(), @@ -136,7 +160,10 @@ pub fn ensure_head_or_fork( system_prompt: state.system_prompt.map(String::from), config: state.config.clone(), history: to_logged(state.history), - forked_from: None, + forked_from: Some(SegmentOrigin { + segment_id: source_segment_id, + at_turn_index, + }), compacted_from: None, }; store.create_segment(session_id, fork_id, &[entry])?; @@ -425,6 +452,14 @@ pub fn fork( /// `TurnEnd` in the source segment that the fork should branch from. /// Replay collects state up to and including that `TurnEnd`; entries /// after it are not carried into the new segment. +/// +/// # Invariant: the source segment is never mutated +/// +/// Past-fork only reads the source and seeds a brand-new segment. It +/// writes no marker back into the source — exactly like live auto-fork +/// ([`ensure_head_or_fork`]). This keeps nested past-forks simple: a +/// fork of a fork just reads its own source and branches again, with no +/// marker-position bookkeeping to reconcile across the chain. pub fn fork_at( store: &impl Store, source_session_id: SessionId, diff --git a/crates/session-store/tests/session_test.rs b/crates/session-store/tests/session_test.rs index e05ab309..a18e8ac8 100644 --- a/crates/session-store/tests/session_test.rs +++ b/crates/session-store/tests/session_test.rs @@ -459,6 +459,7 @@ async fn session_auto_forks_on_conflict() { sid, &mut segment_id, &mut entries_written, + /* at_turn_index */ 0, SegmentStartState { system_prompt: worker_a.get_system_prompt(), config: worker_a.request_config(), @@ -476,10 +477,81 @@ async fn session_auto_forks_on_conflict() { let fork_state = collect_state(&fork_entries); assert_eq!(fork_state.session_id, Some(sid), "auto-fork inherits Session"); - // Original segment should still have the interloper entry + // The new segment records its lineage forward via forked_from; the + // source segment is left immutable (no terminal marker written back). + match &fork_entries[0] { + LogEntry::SegmentStart { + forked_from: Some(origin), + .. + } => { + assert_eq!(origin.segment_id, original_segid); + assert_eq!(origin.at_turn_index, 0); + } + other => panic!("expected SegmentStart with forked_from, got {other:?}"), + } + + // Original segment should still have the interloper entry and NO + // terminal fork marker — it is byte-for-byte unchanged. let original_entries = store.read_all(sid, original_segid).unwrap(); + assert_eq!( + original_entries.len(), + 2, + "source segment holds only SegmentStart + interloper UserInput" + ); let has_interloper = original_entries .iter() .any(|e| matches!(e, LogEntry::UserInput { .. })); assert!(has_interloper); } + +/// Nested past-fork: forking a segment that is itself a fork must not +/// require touching any ancestor. Each `fork_at` only reads its direct +/// source and seeds a new segment, so a chain of forks composes cleanly. +#[tokio::test] +async fn nested_past_fork_leaves_ancestors_immutable() { + let (_dir, store) = make_store(); + let client = MockLlmClient::new(simple_text_events()); + let worker = Worker::new(client); + + let (sid, root_segid) = session_store::create_segment( + &store, + SegmentStartState { + system_prompt: worker.get_system_prompt(), + config: worker.request_config(), + history: worker.history(), + }, + ) + .unwrap(); + + let (worker, _) = run_and_persist(worker, &store, sid, root_segid, "Hello").await; + let root_before = store.read_all(sid, root_segid).unwrap(); + + // First past-fork at the completed turn. + let fork1 = session_store::fork_at(&store, sid, root_segid, worker.turn_count()).unwrap(); + // Fork the fork (turn 0 = right after its SegmentStart seed). + let fork2 = session_store::fork_at(&store, sid, fork1, 0).unwrap(); + + // All three are distinct, all in the same Session. + assert_ne!(fork1, root_segid); + assert_ne!(fork2, fork1); + for seg in [root_segid, fork1, fork2] { + assert_eq!( + collect_state(&store.read_all(sid, seg).unwrap()).session_id, + Some(sid) + ); + } + + // The root and fork1 are untouched by forking their descendants. + assert_eq!(store.read_all(sid, root_segid).unwrap().len(), root_before.len()); + let fork1_entries = store.read_all(sid, fork1).unwrap(); + assert_eq!(fork1_entries.len(), 1, "fork1 is just its SegmentStart seed"); + + // fork2's lineage points at fork1, not the root. + match &store.read_all(sid, fork2).unwrap()[0] { + LogEntry::SegmentStart { + forked_from: Some(origin), + .. + } => assert_eq!(origin.segment_id, fork1), + other => panic!("expected SegmentStart with forked_from, got {other:?}"), + } +} From ca7c5b82d7d87fc980a9ce5765501c3a97578082 Mon Sep 17 00:00:00 2001 From: Hare Date: Wed, 20 May 2026 06:44:54 +0900 Subject: [PATCH 2/5] =?UTF-8?q?ticket:=20live-fork-marker=20=E3=83=AC?= =?UTF-8?q?=E3=83=93=E3=83=A5=E3=83=BC=20(Approve)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tickets/live-fork-marker.md | 7 ++++++ tickets/live-fork-marker.review.md | 38 ++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100644 tickets/live-fork-marker.review.md diff --git a/tickets/live-fork-marker.md b/tickets/live-fork-marker.md index 9796132e..c50a330d 100644 --- a/tickets/live-fork-marker.md +++ b/tickets/live-fork-marker.md @@ -38,3 +38,10 @@ live auto-fork（concurrent writer 検知）と過去 fork（UI から turn 選 - `tickets/entry-hash-abolish.md` (前提) - `tickets/session-grouping-introduce.md` (前提、Session 単位の lineage と整合) - `tickets/pod-session-fork.md` + +## Review + +- 状態: Approve（完了可） +- レビュー詳細: [./live-fork-marker.review.md](./live-fork-marker.review.md) +- 日付: 2026-05-20 +- フォローアップ: auto-fork ロジックの二重実装（`ensure_head_or_fork` free fn と `Pod::ensure_segment_head` inline）の一本化、または KNOWN_ISSUES 登録を推奨（本チケット範囲外・両経路テスト済み）。 diff --git a/tickets/live-fork-marker.review.md b/tickets/live-fork-marker.review.md new file mode 100644 index 00000000..cd101d4c --- /dev/null +++ b/tickets/live-fork-marker.review.md @@ -0,0 +1,38 @@ +# Review: session-store live auto-fork の marker 形式確定 + +対象コミット: `ac3ee5f feat: live auto-fork の marker 形式を確定（seq 比較 + forked_from 記録）` +ベース: `develop` (`46b0e20`) + +## 前提・要件の確認 + +- **marker 形式を (a)/(b) のどちらかに確定して実装**: 達成。(b) 末尾 entry-count 比較を本決定として採用。`entry-hash-abolish` で入った最小実装 (`store_count == *entries_written` 比較) をそのまま昇格し、`segment.rs:151-153` / `pod.rs:1666-1673` で検知。terminal marker variant は追加していない。 +- **(b) の欠点「fork 関係を引くには別 metadata index が要る」の解消**: 達成かつ妥当（下記アーキ節）。新 Segment 側 `SegmentStart.forked_from` への前向き記録で log だけから lineage を辿れるようにし、別 index を不要にした (`segment.rs:163-166`, `pod.rs:1689-1692`)。 +- **過去 fork は元 Segment を touch しない invariant の明文化**: 達成。`fork_at` doc に `# Invariant: the source segment is never mutated` を追記 (`segment.rs:456-462`)。`fork_at` 本体は `read_all` → `collect_state` → `create_segment` のみで source を書かないことをコードで確認 (`segment.rs:469-499`)。 +- **`pod.rs` の auto-fork 経路を確定形式に合わせる**: 達成。本番経路 `Pod::ensure_segment_head` の inline auto-fork に `forked_from` を記録 (`pod.rs:1683-1694`)。 + +## 完了条件 + +- **live auto-fork の marker 形式が決まり実装に反映**: OK（上記）。 +- **nested 過去 fork が正しく動く（test）**: OK。`session_test::nested_past_fork_leaves_ancestors_immutable` が root→fork1→fork2 を作り、(1) root と fork1 が子の fork で不変、(2) fork2 の `forked_from.segment_id == fork1`（祖先 root ではなく直接の親）を検証。実行 pass 確認済み。 +- **並行 writer auto-fork が検知され新 Segment に分岐（test）**: OK。本番経路を通す `compact_events_test::concurrent_writer_drift_auto_forks_with_forked_from` が、別 writer の append で drift→`ensure_segment_head` が新 Segment へ分岐→`forked_from` 記録→元 Segment は foreign append 以外 byte-for-byte 不変（entry 数 +1 のみ）を検証。実行 pass 確認済み。 + +## アーキテクチャ・スコープ + +- **option (b) + forked_from 記録の設計判断**: 妥当。(a) terminal marker は元 Segment を mutate するため、過去 fork（不変）と live fork（可変）で semantics が割れ、チケット背景が懸念した「nested で marker 位置解釈が破綻」を構造的に抱える。(b) を採ったうえで欠点（lineage の引きにくさ）を新 Segment 側の前向き記録で潰したことで、`forked_from` / `compacted_from` の SegmentOrigin スキーマが既に存在する流れにも自然に乗っており、新型・新 variant を一切増やしていない。(a) を採るべき残存理由は無い。 +- **scope**: 範囲外（過去 fork の物理コピー方式、fork tree 可視化 UI）に踏み込んでいない。terminal marker variant 非追加も option(a) 不採用と整合し過不足なし。最小変更に収まっている。 +- **at_turn_index の意味論**: 揃っている。`SegmentOrigin.at_turn_index` は「直近 TurnEnd の turn_count」と定義 (`segment_log.rs:174-184`)。auto-fork は `w.turn_count()` を渡す (`pod.rs:1691`) が、`ensure_segment_head` は `prepare_for_run` 内で**新ターン実行前**に呼ばれる (`pod.rs:1175`) ため、`turn_count()` は直近完了ターンの値=分岐点を指す。これは compaction が `compacted_from.at_turn_index` に渡す値 (`pod.rs:2193 source_turn_count = worker.turn_count()`) および `TurnEnd` が記録する値 (`pod.rs:1880`) と同一ソースで、3 経路で一貫している。テストの `at_turn_index == 0`（0 ターン worker）も定義どおり。 +- **forked_from 記録による既存経路への副作用**: 無し。`collect_state` の `SegmentStart` アームは `forked_from` / `compacted_from` を `..` で破棄しており replay には一切影響しない (`segment_log.rs:251-262`)。`forked_from` の読み手は本 diff 時点で存在せず（grep 済み）、restore / compaction / collect_state は SegmentStart の他フィールドのみ使用。`#[serde(default, skip_serializing_if=...)]` 付きなので旧ログ互換も維持。 + +## 指摘事項 + +### Non-blocking / Follow-up + +- **auto-fork ロジックの二重実装** — `session_store::ensure_head_or_fork`（free fn, `segment.rs:143`、本番未使用でテスト専用）と `Pod::ensure_segment_head`（本番, inline `pod.rs:1666-1707`）に同じ fork ロジックが二重に存在し、今回両方へ `forked_from` 記録を入れた。この二重化は `entry-hash-abolish` 時点からの既存事項で、当時の review (`6e791d8:entry-hash-abolish.review.md:13`) でも両者を別々に更新したことが記録されている既知の状態。drift リスクはあるが、(1) 両者がそれぞれテストされている（free fn = `session_auto_forks_on_conflict`、inline = `concurrent_writer_drift_auto_forks_with_forked_from`）、(2) unify は本チケットのスコープ（marker 形式の確定）外、という理由で本チケットでの blocking とはしない。ただし free fn が本番 0 caller のままテスト専用で残り、本番は inline を持つ構造は、放置すると確実に drift するため、KNOWN_ISSUES への明示登録または近接チケットでの一本化（Pod が free fn を呼ぶ形へ寄せる）を推奨。 + +### Nits + +- `concurrent_writer_drift_auto_forks_with_forked_from` の inline manifest 文字列内コメント "test-pod" 等は問題なし。auto-fork が compaction と独立して発火することを保証するため `NO_COMPACT_MANIFEST_TOML` を用意した意図はコメントに明記されており妥当。 + +## 判断 + +**Approve（完了可）** — 要件・完了条件をすべて充足し、option(b)+forked_from 前向き記録という設計判断は妥当で、新型・新 variant を増やさず最小変更に収まっている。replay / restore / compaction への副作用も無い。唯一の懸念である auto-fork ロジック二重化は本チケット起源ではない既知の non-blocking 事項で、両経路ともテスト済み。フォローアップとして二重実装の一本化（または KNOWN_ISSUES 登録）のみ推奨。 From 8747cc802f578dc5eb23ec57739dde28cca38262 Mon Sep 17 00:00:00 2001 From: Hare Date: Wed, 20 May 2026 06:45:14 +0900 Subject: [PATCH 3/5] =?UTF-8?q?chore:=20auto-fork=20=E3=83=AD=E3=82=B8?= =?UTF-8?q?=E3=83=83=E3=82=AF=E4=BA=8C=E9=87=8D=E5=AE=9F=E8=A3=85=E3=82=92?= =?UTF-8?q?=20KNOWN=5FISSUES=20=E3=81=AB=E7=99=BB=E9=8C=B2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- KNOWN_ISSUES.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/KNOWN_ISSUES.md b/KNOWN_ISSUES.md index 507b3fc4..39e72e70 100644 --- a/KNOWN_ISSUES.md +++ b/KNOWN_ISSUES.md @@ -14,4 +14,5 @@ Ticket を切るほどではないが、次に近所を触るときに合わせ - `crates/tui/src/app.rs:478-485` — bad workflow slug を含む `Method::Run` 送信時、`Event::UserMessage` の早期 broadcast で `turn_index += 1` されターンヘッダだけ残る ("ghost turn header")。次に TUI のターンヘッダ / エラー表示周りを触るときに整理。→ [tickets/pod-input-validate-internalize.md] の review 由来。 - `crates/pod/src/controller.rs:944` — `worker_error_code` で `PodError::WorkflowResolve(_) => InvalidRequest` が post-commit な resolve エラー (`KnowledgeNotFound` 等) にも適用される。意味論的には妥当方向だが、resolve 系のエラー粒度を分けたくなったタイミングで再評価。 - `crates/pod/tests/controller_test.rs` — `double_run_returns_error` がたまに失敗する flakiness を観測。`pod-interrupt-prep-internalize` 以前から存在する別件。次に controller_test の Run 連投系のタイミングを触るときに併せて原因を切り分け。 -- `crates/session-store/src/fs_store.rs:117-122` — `FsStore::read_entry_count` が `fs::read_to_string` で全文ロードしてから行数カウントするため O(n)。`ensure_head_or_fork` は run-start でしか呼ばれず現状は許容範囲だが、長期セッションが普通になった時点で `\n` バイト数の cheap count か末尾 seek に置き換える。→ [tickets/entry-hash-abolish.review.md] follow-up。 +- `crates/session-store/src/fs_store.rs:117-122` — `FsStore::read_entry_count` が `fs::read_to_string` で全文ロードしてから行数カウントするため O(n)。`ensure_head_or_fork` は run-start でしか呼ばれず現状は許容範囲だが、長期セッションが普通になった時点で `\n` バイト数の cheap count か末尾 seek に置き換える。 +- `crates/session-store/src/segment.rs:121` `ensure_head_or_fork` (free fn, test 専用・本番 caller ゼロ) と `crates/pod/src/pod.rs` `Pod::ensure_segment_head` (本番 inline) に live auto-fork の検知 + forked_from 記録が二重実装されている。entry-hash-abolish 以前からの重複で、両方独立にテスト済みだが drift 必至。session-store 側を本番から呼ぶ形に寄せるか free fn を畳むかは要設計判断。Pod state / fork 周辺を次に触るときに統合を検討。 From 3057fe6c24f8b28d3b53fc88c1d09c7112b88bcd Mon Sep 17 00:00:00 2001 From: Hare Date: Wed, 20 May 2026 06:45:19 +0900 Subject: [PATCH 4/5] =?UTF-8?q?ticket:=20live-fork-marker=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/live-fork-marker.md | 47 ------------------------------ tickets/live-fork-marker.review.md | 38 ------------------------ 3 files changed, 86 deletions(-) delete mode 100644 tickets/live-fork-marker.md delete mode 100644 tickets/live-fork-marker.review.md diff --git a/TODO.md b/TODO.md index 4bf43af2..1e1b12cc 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) - - live auto-fork の marker 形式確定 → [tickets/live-fork-marker.md](tickets/live-fork-marker.md) - Pod 単位永続化 - Pod state backend と FsStore 実装 → [tickets/pod-state-backend.md](tickets/pod-state-backend.md) - Pod lifecycle 各点での write 配線 → [tickets/pod-state-write-points.md](tickets/pod-state-write-points.md) diff --git a/tickets/live-fork-marker.md b/tickets/live-fork-marker.md deleted file mode 100644 index c50a330d..00000000 --- a/tickets/live-fork-marker.md +++ /dev/null @@ -1,47 +0,0 @@ -# session-store: live auto-fork の marker 形式確定 - -## 背景 - -`entry-hash-abolish` で `ensure_head_or_fork` は末尾 seq 比較ベースに置換されたが、最小実装で繋いだだけで marker 形式の本決定は保留している。 - -live auto-fork（concurrent writer 検知）と過去 fork（UI から turn 選択）は性質が違う: - -- **live auto-fork**: 元 Segment の末尾に terminal marker (例: `Forked { to: SegmentId }`) を append する CoW semantics。以降の writer は marker を見て新 Segment に自動移動。 -- **過去 fork**: 元 Segment は無変更で、replay して新 Segment を生やすだけ。 - -両者を同じ marker で扱うと、過去 fork から更に過去で fork した場合に元 Segment への marker 位置解釈が複雑化して破綻する。**過去 fork は元 Segment に触れない方針を固定**した上で、live auto-fork 側の marker 形式を確定する。 - -## 要件 - -- live auto-fork 検知の形式を以下のどちらかに確定して実装: - - (a) **terminal marker entry**: 元 Segment 末尾に `Forked { to: SegmentId }` 等の LogEntry を 1 行 append する - - (b) **末尾 seq 比較**: 元 Segment に書き込みは行わず、writer の保持する末尾 seq と store の末尾 seq の差分のみで検知する -- 選択基準: - - (a) は読み手側が fork チェーンを log だけから辿れる利点。書き手競合時に 1 write 増えるコスト。 - - (b) は元 Segment が完全 immutable で、過去 fork との semantics 統一が綺麗。fork 関係を引くには別の metadata index が要る。 -- 過去 fork 側は引き続き元 Segment を touch しないことを invariant として明文化。 -- `pod.rs` の `ensure_head_or_fork` を確定後の形式に合わせて書き直す。 - -## 完了条件 - -- live auto-fork の marker 形式が決まり、実装に反映されている。 -- 過去 fork からの nested 過去 fork が正しく動く（test で確認）。 -- 並行 writer による live auto-fork が正しく検知され、新 Segment に分岐する（test で確認）。 - -## 範囲外 - -- 過去 fork の物理コピー方式（既に `fork_at` で `SessionStart` seed 1 回書き込みの方針で固定）。 -- Fork tree の可視化 UI。 - -## 関連 - -- `tickets/entry-hash-abolish.md` (前提) -- `tickets/session-grouping-introduce.md` (前提、Session 単位の lineage と整合) -- `tickets/pod-session-fork.md` - -## Review - -- 状態: Approve（完了可） -- レビュー詳細: [./live-fork-marker.review.md](./live-fork-marker.review.md) -- 日付: 2026-05-20 -- フォローアップ: auto-fork ロジックの二重実装（`ensure_head_or_fork` free fn と `Pod::ensure_segment_head` inline）の一本化、または KNOWN_ISSUES 登録を推奨（本チケット範囲外・両経路テスト済み）。 diff --git a/tickets/live-fork-marker.review.md b/tickets/live-fork-marker.review.md deleted file mode 100644 index cd101d4c..00000000 --- a/tickets/live-fork-marker.review.md +++ /dev/null @@ -1,38 +0,0 @@ -# Review: session-store live auto-fork の marker 形式確定 - -対象コミット: `ac3ee5f feat: live auto-fork の marker 形式を確定（seq 比較 + forked_from 記録）` -ベース: `develop` (`46b0e20`) - -## 前提・要件の確認 - -- **marker 形式を (a)/(b) のどちらかに確定して実装**: 達成。(b) 末尾 entry-count 比較を本決定として採用。`entry-hash-abolish` で入った最小実装 (`store_count == *entries_written` 比較) をそのまま昇格し、`segment.rs:151-153` / `pod.rs:1666-1673` で検知。terminal marker variant は追加していない。 -- **(b) の欠点「fork 関係を引くには別 metadata index が要る」の解消**: 達成かつ妥当（下記アーキ節）。新 Segment 側 `SegmentStart.forked_from` への前向き記録で log だけから lineage を辿れるようにし、別 index を不要にした (`segment.rs:163-166`, `pod.rs:1689-1692`)。 -- **過去 fork は元 Segment を touch しない invariant の明文化**: 達成。`fork_at` doc に `# Invariant: the source segment is never mutated` を追記 (`segment.rs:456-462`)。`fork_at` 本体は `read_all` → `collect_state` → `create_segment` のみで source を書かないことをコードで確認 (`segment.rs:469-499`)。 -- **`pod.rs` の auto-fork 経路を確定形式に合わせる**: 達成。本番経路 `Pod::ensure_segment_head` の inline auto-fork に `forked_from` を記録 (`pod.rs:1683-1694`)。 - -## 完了条件 - -- **live auto-fork の marker 形式が決まり実装に反映**: OK（上記）。 -- **nested 過去 fork が正しく動く（test）**: OK。`session_test::nested_past_fork_leaves_ancestors_immutable` が root→fork1→fork2 を作り、(1) root と fork1 が子の fork で不変、(2) fork2 の `forked_from.segment_id == fork1`（祖先 root ではなく直接の親）を検証。実行 pass 確認済み。 -- **並行 writer auto-fork が検知され新 Segment に分岐（test）**: OK。本番経路を通す `compact_events_test::concurrent_writer_drift_auto_forks_with_forked_from` が、別 writer の append で drift→`ensure_segment_head` が新 Segment へ分岐→`forked_from` 記録→元 Segment は foreign append 以外 byte-for-byte 不変（entry 数 +1 のみ）を検証。実行 pass 確認済み。 - -## アーキテクチャ・スコープ - -- **option (b) + forked_from 記録の設計判断**: 妥当。(a) terminal marker は元 Segment を mutate するため、過去 fork（不変）と live fork（可変）で semantics が割れ、チケット背景が懸念した「nested で marker 位置解釈が破綻」を構造的に抱える。(b) を採ったうえで欠点（lineage の引きにくさ）を新 Segment 側の前向き記録で潰したことで、`forked_from` / `compacted_from` の SegmentOrigin スキーマが既に存在する流れにも自然に乗っており、新型・新 variant を一切増やしていない。(a) を採るべき残存理由は無い。 -- **scope**: 範囲外（過去 fork の物理コピー方式、fork tree 可視化 UI）に踏み込んでいない。terminal marker variant 非追加も option(a) 不採用と整合し過不足なし。最小変更に収まっている。 -- **at_turn_index の意味論**: 揃っている。`SegmentOrigin.at_turn_index` は「直近 TurnEnd の turn_count」と定義 (`segment_log.rs:174-184`)。auto-fork は `w.turn_count()` を渡す (`pod.rs:1691`) が、`ensure_segment_head` は `prepare_for_run` 内で**新ターン実行前**に呼ばれる (`pod.rs:1175`) ため、`turn_count()` は直近完了ターンの値=分岐点を指す。これは compaction が `compacted_from.at_turn_index` に渡す値 (`pod.rs:2193 source_turn_count = worker.turn_count()`) および `TurnEnd` が記録する値 (`pod.rs:1880`) と同一ソースで、3 経路で一貫している。テストの `at_turn_index == 0`（0 ターン worker）も定義どおり。 -- **forked_from 記録による既存経路への副作用**: 無し。`collect_state` の `SegmentStart` アームは `forked_from` / `compacted_from` を `..` で破棄しており replay には一切影響しない (`segment_log.rs:251-262`)。`forked_from` の読み手は本 diff 時点で存在せず（grep 済み）、restore / compaction / collect_state は SegmentStart の他フィールドのみ使用。`#[serde(default, skip_serializing_if=...)]` 付きなので旧ログ互換も維持。 - -## 指摘事項 - -### Non-blocking / Follow-up - -- **auto-fork ロジックの二重実装** — `session_store::ensure_head_or_fork`（free fn, `segment.rs:143`、本番未使用でテスト専用）と `Pod::ensure_segment_head`（本番, inline `pod.rs:1666-1707`）に同じ fork ロジックが二重に存在し、今回両方へ `forked_from` 記録を入れた。この二重化は `entry-hash-abolish` 時点からの既存事項で、当時の review (`6e791d8:entry-hash-abolish.review.md:13`) でも両者を別々に更新したことが記録されている既知の状態。drift リスクはあるが、(1) 両者がそれぞれテストされている（free fn = `session_auto_forks_on_conflict`、inline = `concurrent_writer_drift_auto_forks_with_forked_from`）、(2) unify は本チケットのスコープ（marker 形式の確定）外、という理由で本チケットでの blocking とはしない。ただし free fn が本番 0 caller のままテスト専用で残り、本番は inline を持つ構造は、放置すると確実に drift するため、KNOWN_ISSUES への明示登録または近接チケットでの一本化（Pod が free fn を呼ぶ形へ寄せる）を推奨。 - -### Nits - -- `concurrent_writer_drift_auto_forks_with_forked_from` の inline manifest 文字列内コメント "test-pod" 等は問題なし。auto-fork が compaction と独立して発火することを保証するため `NO_COMPACT_MANIFEST_TOML` を用意した意図はコメントに明記されており妥当。 - -## 判断 - -**Approve（完了可）** — 要件・完了条件をすべて充足し、option(b)+forked_from 前向き記録という設計判断は妥当で、新型・新 variant を増やさず最小変更に収まっている。replay / restore / compaction への副作用も無い。唯一の懸念である auto-fork ロジック二重化は本チケット起源ではない既知の non-blocking 事項で、両経路ともテスト済み。フォローアップとして二重実装の一本化（または KNOWN_ISSUES 登録）のみ推奨。 From 8e1c5d3bdc2bda788fb2d72d9d27354b908559a6 Mon Sep 17 00:00:00 2001 From: Hare Date: Wed, 20 May 2026 06:45:43 +0900 Subject: [PATCH 5/5] =?UTF-8?q?chore:=20=E7=A9=BA=E3=81=AB=E3=81=AA?= =?UTF-8?q?=E3=81=A3=E3=81=9F=20Storage=20=E8=A6=AA=E8=A6=8B=E5=87=BA?= =?UTF-8?q?=E3=81=97=E3=82=92=20TODO=20=E3=81=8B=E3=82=89=E5=89=8A?= =?UTF-8?q?=E9=99=A4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- TODO.md | 1 - 1 file changed, 1 deletion(-) diff --git a/TODO.md b/TODO.md index 1e1b12cc..bf1838be 100644 --- a/TODO.md +++ b/TODO.md @@ -7,7 +7,6 @@ - Pod: 空応答ターン (Submit 後 AI 応答ゼロで Pause/Cancel) を自動巻き戻し → [tickets/pod-empty-turn-rollback.md](tickets/pod-empty-turn-rollback.md) - 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) - Pod 単位永続化 - Pod state backend と FsStore 実装 → [tickets/pod-state-backend.md](tickets/pod-state-backend.md) - Pod lifecycle 各点での write 配線 → [tickets/pod-state-write-points.md](tickets/pod-state-write-points.md)