diff --git a/Cargo.lock b/Cargo.lock index befb590e..89de8352 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3620,6 +3620,7 @@ dependencies = [ "session-store", "tokio", "toml", + "tools", "unicode-width", "uuid", ] diff --git a/TODO.md b/TODO.md index f7c8e155..6722a7fb 100644 --- a/TODO.md +++ b/TODO.md @@ -6,6 +6,7 @@ - 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: 子→親の TurnEnded/Errored callback を親由来ターンのみに絞る → [tickets/pod-parent-turn-callback.md](tickets/pod-parent-turn-callback.md) +- Pod: セッションログをバックエンドにした Pod 単位の永続化 → [tickets/pod-persistent-state.md](tickets/pod-persistent-state.md) - llm-worker のエラー耐性 - ストリーム途中失敗時の継続 → [tickets/llm-worker-stream-continuation.md](tickets/llm-worker-stream-continuation.md) - ネイティブ GUI クライアント MVP → [tickets/native-gui-mvp.md](tickets/native-gui-mvp.md) diff --git a/crates/tui/Cargo.toml b/crates/tui/Cargo.toml index b49a2b60..eb89635b 100644 --- a/crates/tui/Cargo.toml +++ b/crates/tui/Cargo.toml @@ -17,3 +17,6 @@ manifest = { workspace = true } session-store = { workspace = true } pod-registry = { workspace = true } serde = { workspace = true, features = ["derive"] } + +[dev-dependencies] +tools = { workspace = true } diff --git a/crates/tui/src/task.rs b/crates/tui/src/task.rs index 0c9bdb7d..bb3980a2 100644 --- a/crates/tui/src/task.rs +++ b/crates/tui/src/task.rs @@ -315,3 +315,116 @@ mod tests { ); } } + +/// Cross-crate contract tests. The TUI deliberately re-implements a +/// stripped-down mirror of `tools::TaskStore` instead of depending on +/// the real one (see `tickets/tui-task-display.md`). That decoupling +/// means a format change on the tools side — a renamed field on +/// `TaskEntry`, a different fence syntax in `render_snapshot`, a new +/// JSON wrapper — would silently leave the TUI parsing nothing instead +/// of failing loudly. +/// +/// These tests pull `tools` in as a dev-dependency so the contract is +/// exercised at CI time. If they fail, either the format genuinely +/// changed (update both sides) or the TUI mirror has drifted (re-sync +/// it). +#[cfg(test)] +mod cross_format_contract { + use super::*; + use tools::task::{TaskStatus as ToolsTaskStatus, TaskStore as ToolsTaskStore}; + + /// Mirrors the envelope `Pod::try_pre_run_compact` wraps the raw + /// snapshot text in. Hand-rolled here so the test fails loudly if + /// the prose around the JSON fence ever shifts. + fn wrap_pod_style(snapshot_text: &str) -> String { + format!( + "[Session TaskStore snapshot]\n\n{snapshot_text}\n\n\ + This is the complete session task list preserved across compaction. \ + The following TaskList tool result presents the same state through the tool lane." + ) + } + + fn tools_status_label(s: ToolsTaskStatus) -> &'static str { + match s { + ToolsTaskStatus::Pending => "pending", + ToolsTaskStatus::Inprogress => "inprogress", + ToolsTaskStatus::Completed => "completed", + ToolsTaskStatus::Deleted => "deleted", + } + } + + fn tui_status_label(s: TaskStatus) -> &'static str { + match s { + TaskStatus::Pending => "pending", + TaskStatus::Inprogress => "inprogress", + TaskStatus::Completed => "completed", + TaskStatus::Deleted => "deleted", + } + } + + #[test] + fn tools_snapshot_text_round_trips_into_tui_store() { + let upstream = ToolsTaskStore::new(); + upstream.create("first".into(), "first desc".into()); + upstream.create("second".into(), "second desc with\nnewline".into()); + upstream + .update(1, Some(ToolsTaskStatus::Inprogress), None, None) + .expect("update 1"); + upstream + .update(2, Some(ToolsTaskStatus::Completed), None, None) + .expect("update 2"); + + let envelope = wrap_pod_style(&upstream.snapshot_text()); + + let mut downstream = TaskStore::new(); + downstream.apply_system_message_text(&envelope); + + let upstream_tasks = upstream.list(); + let downstream_tasks = downstream.tasks(); + assert_eq!( + downstream_tasks.len(), + upstream_tasks.len(), + "TUI parsed wrong number of tasks — `tools::render_snapshot` shape may have shifted" + ); + for (u, d) in upstream_tasks.iter().zip(downstream_tasks.iter()) { + assert_eq!(d.taskid, u.taskid); + assert_eq!(d.subject, u.subject); + assert_eq!(d.description, u.description); + assert_eq!(tui_status_label(d.status), tools_status_label(u.status)); + } + } + + #[test] + fn tools_taskentry_field_shape_deserializes_into_tui_taskentry() { + // A single `tools::TaskEntry` round-tripped through JSON. Field + // renames like `taskid` → `task_id` or status case changes on + // the tools side would surface here as a serde failure or a + // wrong-status assertion. + let upstream = ToolsTaskStore::new(); + let created = upstream.create("subj".into(), "desc".into()); + let json = serde_json::to_string(&created).expect("serialize tools::TaskEntry"); + let parsed: TaskEntry = + serde_json::from_str(&json).expect("deserialize into tui::task::TaskEntry"); + assert_eq!(parsed.taskid, created.taskid); + assert_eq!(parsed.subject, created.subject); + assert_eq!(parsed.description, created.description); + assert_eq!(tui_status_label(parsed.status), "pending"); + } + + #[test] + fn empty_tools_store_snapshot_is_recognised_by_tui() { + // Edge case: a freshly initialised TaskStore still produces a + // valid snapshot envelope. The TUI must parse it as "zero + // tasks", not silently fall through to no-op. + let upstream = ToolsTaskStore::new(); + let envelope = wrap_pod_style(&upstream.snapshot_text()); + + // Seed the TUI store with stale state to confirm replacement. + let mut downstream = TaskStore::new(); + downstream.apply_tool_call("TaskCreate", r#"{"subject":"stale","description":""}"#); + assert_eq!(downstream.tasks().len(), 1); + + downstream.apply_system_message_text(&envelope); + assert!(downstream.is_empty()); + } +} diff --git a/crates/tui/src/ui.rs b/crates/tui/src/ui.rs index d22f2bd2..38205862 100644 --- a/crates/tui/src/ui.rs +++ b/crates/tui/src/ui.rs @@ -66,25 +66,30 @@ pub fn draw(frame: &mut Frame, app: &mut App) { let input_render = app.input.render(input_content_width); let input_height = input_area_height(&input_render, area.height); let mini_view_h = task_mini_view_height(&app.task_store); + // One blank row separates the history tail from the mini-view so + // the latest message doesn't visually crash into the task summary. + // Folds away with the mini-view when there are no tasks. + let mini_view_gap = if mini_view_h > 0 { 1 } else { 0 }; let chunks = Layout::vertical([ - Constraint::Min(0), // history view - Constraint::Length(mini_view_h), // task mini-view (0 when empty) - Constraint::Length(1), // separator - Constraint::Length(1), // status - Constraint::Length(input_height), // input area + Constraint::Min(0), // history view + Constraint::Length(mini_view_gap), // gap above mini-view + Constraint::Length(mini_view_h), // task mini-view (0 when empty) + Constraint::Length(1), // separator + Constraint::Length(1), // status + Constraint::Length(input_height), // input area ]) .split(area); draw_history(frame, app, chunks[0]); if mini_view_h > 0 { - draw_task_mini_view(frame, &app.task_store, chunks[1]); + draw_task_mini_view(frame, &app.task_store, chunks[2]); } - draw_separator(frame, chunks[2]); - draw_status(frame, app, chunks[3]); - draw_input(frame, &input_render, chunks[4]); + draw_separator(frame, chunks[3]); + draw_status(frame, app, chunks[4]); + draw_input(frame, &input_render, chunks[5]); if let Some(state) = app.completion.as_ref().filter(|c| c.is_active()) { - draw_completion_popup(frame, state, chunks[4]); + draw_completion_popup(frame, state, chunks[5]); } } diff --git a/tickets/pod-persistent-state.md b/tickets/pod-persistent-state.md new file mode 100644 index 00000000..ccf7f895 --- /dev/null +++ b/tickets/pod-persistent-state.md @@ -0,0 +1,84 @@ +# Pod: セッションログをバックエンドにした Pod 単位の永続化 + +## 背景 + +現在の永続化の主軸は session-store の append-only JSONL ログで、`SessionId` 単位に会話履歴・設定・scope snapshot・usage・拡張 payload を復元できる。一方で Pod 単位のランタイム状態は `/{pod_name}/` 配下の `status.json` / `history.json` / `spawned_pods.json` などに write-through されているが、runtime dir は再起動で消えてよい領域であり、Pod プロセスの寿命を超える復元ソースとしては扱えない。 + +特に spawned Pod の管理情報は `SpawnedPodRegistry` のコメントにもある通り、現状は runtime dir への write-through のみで、再起動した spawner が子 Pod 一覧を rebuild する future work になっている。 + +このチケットでは、既存の session-store を物理バックエンドとして利用しつつ、Pod 名をキーにした永続状態を追加し、Pod 単位で「最後にどの session を保持していたか」「spawned children をどう復元するか」を扱えるようにする。 + +## 方針 + +- session log は引き続き会話状態の唯一の復元ソースにする。 + - `history.json` や runtime dir の snapshot を永続正本にはしない。 + - LLM context に載せる新規 input は、既存方針通り先に worker history / session log に commit されている必要がある。 +- Pod 単位の永続化は「Pod identity → session / child registry などへの参照」を保存する薄いメタデータ層として設計する。 + - 会話本文を二重保存しない。 + - active session だけでなく、compaction / fork / resume によってその Pod が辿ってきた過去 session を順序付きで保持する。これは UI の履歴表示、直近以前への復元、active session 変更の監査に使う。 + - session-store の `Store` trait を拡張するか、隣接 trait / module を追加して、FsStore 以外の backend でも同じ形で実装できるようにする。 +- FsStore のデフォルト layout は `/pods/` 配下など、`sessions/` と同じ data_dir 管理下に置く。 + - runtime dir (`/{pod_name}/`) は引き続き socket / pid / status など一時状態専用。 +- Pod lifecycle 上の write point を明確にする。 + - Pod 作成時: pod name と allocated session id を記録。 + - first run で `SessionStart` が materialize された後: active session / head を更新できる状態にする。 + - compaction / fork / resume で active session が変わる場合: Pod state も同時に更新。 + - `SpawnPod` / callback / `StopPod` による child registry 変更時: runtime dir だけでなく persistent Pod state にも write-through。 +- 復元時は Pod state から active session を解決し、その session log を `restore_from_manifest` 相当の経路で復元する。 + - session id を明示した resume は既存通り session を直接指定できる。 + - Pod 名 resume は Pod state → active session → session restore の順に解決する。 + - live writer 衝突は既存の pod-registry / session_id collision check を維持する。 + +## データ粒度の考え方 + +- ユーザー視点の会話継続単位と、内部の append-only log 単位を分けて扱う。 + - ユーザー視点: Pod / thread / conversation のような安定 ID。compaction しても同じ会話として継続する。 + - 内部 log 視点: session segment / revision / epoch のような履歴再構築単位。compaction や fork で新しい log root が必要なら新 ID になる。 +- 現状の `SessionId` は内部 log 単位の性質が強い。compaction は履歴を要約済み prefix に置き換えて新しい append-only chain を始めるため、低レベルには「新 session」として扱うのは自然。ただし UX / データモデル上は「同じ Pod conversation の新 revision」と見せる。 +- 将来 DB backend を追加する場合も、`Conversation/PodState` と `SessionSegment` を分ける形に寄せる。 + - `pod_state.active_session_id` は現在 append 先の segment を指す。 + - `pod_state.session_history[]` は Pod 視点で active だった segment の順序付き履歴。 + - compaction / fork の構造的 lineage は session log の `SessionOrigin` または DB の relation として保持し、Pod state は「この Pod がどれを active にしたか」の操作履歴に留める。 + +## 要件 + +- Pod 名をキーに、少なくとも以下を永続化できること: + - active `SessionId` + - ordered session history: その Pod が active として保持してきた `SessionId` の時系列リスト + - 各 entry には最低限 `session_id` と遷移理由（new / resume / compact / fork など）を持たせる + - compaction / fork の構造的な出自は session log の `SessionOrigin` を正本とし、Pod state 側は Pod 視点の active session 遷移履歴として扱う + - Pod manifest / scope 復元に必要な参照または snapshot の扱い（既存 session log の `pod.scope` snapshot と責務を重複させない） + - spawned children の registry（pod name, socket path, delegated scope, callback address, child session id が必要なら含める） +- `SpawnedPodRegistry` が runtime dir の `spawned_pods.json` だけでなく、Pod 永続状態から初期化できること。 +- `ListPods` / `SendToPod` / `ReadPodOutput` / `StopPod` は、復元後の spawner でも永続化された child registry を基に動作できること。 + - ただし `ReadPodOutput` の read cursor は session-lifetime / in-memory のままでよい。永続化対象にしない。 +- Pod の compaction により active session id が変わった場合、Pod 永続状態と pod-registry の session id が整合すること。 +- 既存の `--session ` resume は壊さない。 +- 新しい Pod 名単位 resume / attach の入口を決めること。 + - 例: `pod --pod-state ` ではなく、既存 `pod.name` と manifest cascade から同名 Pod state を探す形など。 + - CLI / TUI の最小導線を本チケット内で確定する。 + +## 完了条件 + +- `session-store` に Pod 単位メタデータを扱う backend API と FsStore 実装がある。 +- Pod state が active session と ordered session history を保持し、new / resume / compaction / fork の遷移が順序付きで記録される。 +- 新規 Pod 起動、resume、compaction、spawn / stop の各タイミングで Pod 永続状態が更新される。 +- Pod プロセス再起動後、Pod 名から active session を復元し、会話を継続できる。 +- spawner Pod の再起動後、永続化された spawned children 一覧から `ListPods` が復元され、到達可能な child に対して comm tools が使える。 +- runtime dir は引き続き一時状態として扱われ、永続正本に依存しない。 +- live writer の二重起動は既存 pod-registry / session lock と同等以上に防止される。 + +## 範囲外 + +- 会話履歴そのものの保存形式変更。 +- session log の DB 化や remote backend 実装。 +- Pod state の自動 GC / retention policy。 +- TUI 上の高度な Pod 一覧 UI。最小限の resume / attach 導線を超える UX は別チケット。 +- `ReadPodOutput` cursor の永続化。 + +## 関連 + +- `crates/session-store/`: 既存の session append-only backend。 +- `crates/pod/src/runtime/dir.rs`: runtime dir の `history.json` / `spawned_pods.json`。 +- `crates/pod/src/spawn/registry.rs`: spawned children registry。現状は write-through のみで復元未実装。 +- `tickets/pod-session-fork.md`: active session 切り替え設計との整合が必要。 diff --git a/tickets/tui-task-display.md b/tickets/tui-task-display.md index 074906c7..4751551d 100644 --- a/tickets/tui-task-display.md +++ b/tickets/tui-task-display.md @@ -78,3 +78,8 @@ - TUI 側受信パス: `crates/tui/src/app.rs`（`Event::ToolCallDone` / `Event::SystemMessage` / `Event::History`）、`crates/tui/src/ui.rs`、`crates/tui/src/block.rs`（`ToolCallBlock` / `SystemMessage`） - 設計指針: `AGENTS.md`「LLM コンテキストの加工原則」（history からの決定的再構成は許容変換） - 関連チケット: `tickets/session-todo.md`（Task ツール本体）、`tickets/session-todo-reminder.md`（LLM 側ナッジ） + +## Review +- 状態: Approve +- レビュー詳細: [./tui-task-display.review.md](./tui-task-display.review.md) +- 日付: 2026-05-04 diff --git a/tickets/tui-task-display.review.md b/tickets/tui-task-display.review.md new file mode 100644 index 00000000..2cb5d29e --- /dev/null +++ b/tickets/tui-task-display.review.md @@ -0,0 +1,56 @@ +# Review: TUI に Task ストア状態を表示する + +## 前提・要件の確認 + +### TUI 側の TaskStore 取り回し +- `App` に最新 `TaskStore` を持つ: `crates/tui/src/app.rs:84` `pub task_store: TaskStore` で保持。型は TUI 内 mirror（`crates/tui/src/task.rs`）で `tools` 依存なし。要件通り。 +- `ToolCallDone` で `TaskCreate` / `TaskUpdate` を反映: `crates/tui/src/app.rs:569-586`、name は当該ブロックから取得し `task_store.apply_tool_call(name, &arguments)` を呼ぶ。完了タイミングのみで部分引数は混入しない。要件通り。 +- 部分引数（streaming 中）を適用しない: `Event::ToolCallArgsDelta`（`app.rs:561-568`）は `args_stream.push_str` のみで `task_store` は触らない。要件通り。 +- `SystemMessage` 観測時、`[Session TaskStore snapshot]` 始まりなら snapshot を適用: `crates/tui/src/app.rs:383-386`（`push_history_item` の "system" ブランチ）から `task_store.apply_system_message_text(&text)` を呼ぶ。`Event::SystemMessage` は `app.rs:494-497` で `push_history_item` を経由するため live 経路もカバー。要件通り。 +- 初回 history replay でも同じ経路: `restore_history` が `task_store = TaskStore::new()` でリセット後（`app.rs:857-858`）、`push_history_item` を順に通すため自動復元される。要件通り。 +- 不正 JSON や未知フィールドを黙って無視: `apply_tool_call` の各 match arm で `if let Ok(...)` ガード、`apply_system_message_text` も `parse_snapshot_text` が `Option` を返す。`crates/tui/src/task.rs:213-220` のテスト `malformed_arguments_are_silently_ignored` で挙動確認済み。要件通り。 + +### ミニビュー +- 配置 `history / mini-view / separator / status / input`: `crates/tui/src/ui.rs:70-77` の `Layout::vertical` で確認。要件通り。 +- 0 件時は領域ごと畳む: `task_mini_view_height` が `is_empty()` で 0 を返し、`Constraint::Length(0)` で行ごと消える（`ui.rs:99-106`）。要件通り。 +- 最大 3 件 active を表示 + サマリ 1 行: `MINI_VIEW_MAX_ACTIVE = 3`（`ui.rs:94`）と `mini_view_summary_line` で実装。subject の改行は `entry.subject.lines().next()` で先頭行のみ（`ui.rs:141`）。要件通り。 +- 件数サマリ: pending / inprogress / completed / deleted の内訳を1行で表示（`ui.rs:153-167`）。要件通り。 + +### サイドペイン +- トグルキー `Ctrl-T`: `crates/tui/src/main.rs:436-439`。要件通り。 +- 横分割は history 領域内のみ: `draw_history` 内で `Layout::horizontal` 分割し、status / input / separator は外側 `chunks` の全幅を維持（`ui.rs:326-334`）。要件通り。 +- 全件（pending / inprogress / completed / deleted）+ description を含めて列挙: `draw_task_side_pane`（`ui.rs:386-460`）。各エントリは `taskid` + status mark + subject + description を表示。要件通り。 +- スクロール対応: `task_pane_scroll` を `App` に保持、`PageUp` / `PageDown` がペイン open 時に優先（`main.rs:471-489`）。最大値で clamp（`ui.rs:449-452`）。要件通り。 + +### レイアウトと一貫性 +- ミニビューとサイドペインは同じ `App::task_store` を参照する単一情報源（`ui.rs:81, 399`）。要件通り。 +- 既存の completion popup は `chunks[4]` (input rect) を使うため、ミニビューが挟まっても座標計算に影響なし（`ui.rs:86-88, 191-246`）。 +- スクロール / status line は `chunks[0]` の `inner` を使うため、サイドペインが開いてもスクロール計算は分割後の history rect に追従（`ui.rs:347-350`）。 +- 副作用無し。 + +### 完了条件 +すべて満たしている。`cargo test -p tui` 71 件合格、`cargo build --workspace` クリーン。 + +## アーキテクチャ・スコープ + +- **`tools` 依存を持ち込まない方針** が一貫して守られている。TUI 内 `task.rs` mirror は `tools::TaskStore` 全機能ではなく TUI が必要とする最小サブセット（`apply_tool_call` / `apply_system_message_text` / `tasks()` / `counts()`）に絞っている。`feedback_llm_worker_scope` / 「TUI は表示専門レイヤ」の方針に整合。 +- `serde` は `cargo add` で追加（`crates/tui/Cargo.toml:19`）。memory の `feedback_cargo_add` 遵守。 +- protocol 表面は無変更。`Event::SystemMessage` / `Event::ToolCallDone` / `Event::History` の既存経路にすべて乗っている。チケットの「protocol 表面を増やさない」目標を達成。 +- LLM コンテキスト加工原則: history からの純粋な再構成変換であり context への割り込みではない。整合済み。 +- ファイル分割: 新規 `task.rs` モジュール（180行 + テスト）として切り出されており、`app.rs` / `ui.rs` への変更も既存パターン（cache.rs / scroll.rs と同じ位置付け）に整合。コードベースを歪めていない。 + +## 指摘事項 + +### Non-blocking / Follow-up +- **snapshot text フォーマット契約の自動検出機構が無い**: `crates/tui/src/task.rs:168-179` `parse_snapshot_text` は `crates/tools/src/task.rs:393-404` `parse_compact_snapshot_text` と byte-for-byte 同一の手書きクローン。`tools::render_snapshot` のフォーマット（フェンス記号・改行配置・ヘッダ文字列）が変わると、TUI は無言で snapshot 検出を失敗するだけで CI からは見えない。Doc コメント（`task.rs:8-17`）と TUI 側ユニットテストの手書き fixture（`wrap_snapshot` ヘルパ）で「契約に乗っているつもり」を表明しているが、`tools::render_snapshot` の出力をそのまま食わせる意味でのリンク確認は無い。 + - 落としどころ案: `crates/tools` を `dev-dependencies` だけに入れ、`#[cfg(test)]` で `tools::render_snapshot(...)` の実出力を `tui::task::TaskStore::apply_system_message_text` に流すクロステストを 1 本足す。本番ビルドの依存は増やさず、契約破綻だけ拾える。今のスコープ外でも構わない。 +- **History replay 経路に「retained TaskCreate → 末尾 snapshot で上書き」シナリオの App テストが無い**: `tools` 側には `trailing_snapshot_supersedes_pre_compact_taskcreates_in_retained`（`crates/tools/src/task.rs:584-628`）があるが、TUI の `App::handle_pod_event(Event::History { ... })` 経由で同じ並びを通す App レベルテストは未追加。実装上は `push_history_item` を順に呼ぶだけなので動くはずで、`history_replay_reconstructs_task_store`（`app.rs:1404-1448`）と `snapshot_text_replaces_state_and_advances_next_id`（`task.rs:251-284`）の組み合わせで間接的にカバーはされている。リグレッション保護として 1 本欲しい程度の話。 +- **狭い端末でのトグル無反応**: `task_side_pane_width`（`ui.rs:373-384`）は `area_width < 60` なら 0 を返すため、`task_pane_open == true` でも視覚的に何も起きない。Ctrl-T が "効いていない" ように見えるリスクがある。レアケースなので非ブロッキング。 + +### Nits +- `crates/tui/src/task.rs:165-166` の `TaskSnapshot` は `tasks` フィールドのみだが、本家 `tools::TaskSnapshot` には他フィールドが追加される可能性がある。`#[serde(deny_unknown_fields)]` を付けないことで forward-compat を保っている（適切）が、その意図を 1 行コメントしておくと将来読み手に親切。 +- `task_status_mark`（`ui.rs:172-184`）の戻り値タプルに型エイリアスを入れると、サイドペイン側でも mini-view 側でも使われている共有意図がコードから読みやすい。今のままで実害は無い。 + +## 判断 + +**Approve** — チケットの前提・方針・要件・完了条件をすべて満たしており、設計の意図（`tools` 非依存・protocol 不変・history からの純粋再構成）も忠実に実装されている。tools 側との snapshot フォーマット契約はテキスト一致でしか担保されていないが、これはチケットが意識的に選んだトレードオフであり、必要であれば dev-dependency でクロステストを 1 本足す程度のフォローアップで十分。コードベースを歪める箇所は無い。