docs(tickets): pod状態のTUI同期完了
This commit is contained in:
parent
f2dda9097e
commit
dcd22d4399
1
TODO.md
1
TODO.md
|
|
@ -11,7 +11,6 @@
|
|||
- ストリーム途中失敗時の継続 → [tickets/llm-worker-stream-continuation.md](tickets/llm-worker-stream-continuation.md)
|
||||
- ネイティブ GUI クライアント MVP → [tickets/native-gui-mvp.md](tickets/native-gui-mvp.md)
|
||||
- TUI 拡充
|
||||
- Run/notice 前後の Pod 状態同期と Ctrl 系操作の安定化 → [tickets/tui-pod-status-sync.md](tickets/tui-pod-status-sync.md)
|
||||
- Run 中の入力キューイング → [tickets/tui-input-queue.md](tickets/tui-input-queue.md)
|
||||
- ユーザーマニフェストのモデル設定 wizard → [tickets/tui-user-model-setup.md](tickets/tui-user-model-setup.md)
|
||||
- spawn 失敗時に Pod の stderr が TUI に表示されない → [tickets/tui-spawn-error-surface.md](tickets/tui-spawn-error-surface.md)
|
||||
|
|
|
|||
|
|
@ -1,46 +0,0 @@
|
|||
# TUI: Pod 状態同期と Ctrl 系操作の安定化
|
||||
|
||||
## 背景
|
||||
|
||||
TUI 利用中、notice / alert / compact 表示が出た前後で `Ctrl-C` / `Ctrl-X` が効かない、または意図と違う動作をしているように見えるケースがある。
|
||||
|
||||
調査上、notice 表示そのものがキー入力を壊している実装は見当たらない。一方で、TUI が持つ `running` / `paused` などのローカル状態と、Pod controller の実状態がズレる経路が複数ある。
|
||||
|
||||
代表例:
|
||||
|
||||
- `Event::RunEnd` 後、TUI は idle 扱いになるが、Pod controller 側では memory extract / consolidate / post-run compact などの post-run 処理がまだ続いている場合がある。
|
||||
- この区間で notice / alert / compact event が表示されると、ユーザーからは「notice が出て idle に見えるのに Ctrl 操作が効かない」ように見える。
|
||||
- TUI attach 時は `GetHistory` で履歴を復元するが、Pod の現在 status（Running / Paused / Idle）は復元されない。
|
||||
- 実際には Pod が Running / Paused でも、TUI 側は初期値の idle として扱い、`Ctrl-C` / `Ctrl-X` の分岐が実状態と合わない可能性がある。
|
||||
- TUI の terminal event polling は Pod event が頻繁な時に key input が遅延・取りこぼしに見えやすい構造になっている疑いがある。
|
||||
|
||||
## 要件
|
||||
|
||||
- TUI が表示・操作に使う Pod 状態は、接続直後および実行中を通じて Pod controller の実状態と一致する。
|
||||
- attach / resume 直後でも Running / Paused / Idle が正しく表示される。
|
||||
- `Ctrl-C` / `Ctrl-X` の分岐が TUI の古い推測状態に依存して誤動作しない。
|
||||
- `RunEnd` と post-run 処理中の扱いを整理する。
|
||||
- Worker turn が終わった状態と、Pod controller が次の method を即時処理できる状態を混同しない。
|
||||
- post-run compact / memory 系処理中に notice が出ても、TUI 上の status と操作可能性が矛盾しない。
|
||||
- TUI の input polling を見直し、Pod event / notice が多い状況でも key input が starvation しない。
|
||||
- `Ctrl-C` / `Ctrl-X` の UX を明確化する。
|
||||
- Running / Paused / Idle / post-run busy それぞれで、Pause / Cancel / Shutdown / TUI exit のどれを送るかを明示する。
|
||||
|
||||
## 完了条件
|
||||
|
||||
- Running / Paused な Pod に後から TUI attach しても、status 表示と `Ctrl-C` / `Ctrl-X` の挙動が実状態に合っている。
|
||||
- `RunEnd` 後の post-run 処理中に alert / compact notice が発生しても、TUI が idle と誤表示して操作不能に見える状態にならない。
|
||||
- Pod event / notice が連続しても、`Ctrl-C` / `Ctrl-X` が遅延・取りこぼしに見えない。
|
||||
- 上記の状態遷移について、少なくともユニットテストまたは再現手順で確認できる。
|
||||
|
||||
## 範囲外
|
||||
|
||||
- TUI の入力キューイング（`tickets/tui-input-queue.md`）。本チケットは状態同期と Ctrl 系操作の安定化に限定する。
|
||||
- native GUI 側の状態管理。
|
||||
- notice / alert の文言や見た目の全面 redesign。
|
||||
|
||||
## Review
|
||||
|
||||
- 状態: Approve with follow-up
|
||||
- レビュー詳細: [./tui-pod-status-sync.review.md](./tui-pod-status-sync.review.md)
|
||||
- 日付: 2026-05-04
|
||||
|
|
@ -1,80 +0,0 @@
|
|||
# Review: TUI Pod 状態同期と Ctrl 系操作の安定化
|
||||
|
||||
レビュー対象: `develop` の作業ツリー（staged + unstaged）。22 files / +641 -311。
|
||||
|
||||
## 前提・要件の確認
|
||||
|
||||
### 状態同期 (要件 1)
|
||||
- **達成**。`PodStatus` を `protocol` クレートに昇格させ、`Event::Status` と `Event::History.status` を新設（`crates/protocol/src/lib.rs:317-328, 437-447`）。
|
||||
- GetHistory 応答に現在 status を埋め込み (`crates/pod/src/ipc/server.rs:162-168`)。
|
||||
- controller の status 遷移ごとに `Event::Status` を broadcast する単一経路を `set_controller_status` に集約 (`crates/pod/src/controller.rs:68-77`)。
|
||||
- TUI 側は `App.pod_status` を `Event::Status` / `Event::History.status` から駆動し、`set_pod_status` が `running` / `paused` / `busy` を一括更新する (`crates/tui/src/app.rs:110-118, 669-679`)。
|
||||
- attach 時の history 取得時点での status 同期は `attach_history_includes_current_status` テストでカバー (`crates/pod/tests/controller_test.rs:217-238`)。
|
||||
|
||||
### post-run 処理の扱い (要件 2)
|
||||
- **達成**。`PodStatus::Busy` を新設し、`PodRunResult::Finished` / `LimitReached` で controller が一旦 Busy に入ってから post-run jobs (extract / consolidate / compact) を実行、完了後に Idle へ遷移する設計 (`crates/pod/src/controller.rs:79-135, 786-836`)。
|
||||
- Busy 中の `Event::Status::Busy` broadcast はテスト `run_end_enters_busy_until_post_run_finishes_and_broadcasts_status` で確認 (`crates/pod/tests/controller_test.rs:175-214`)。
|
||||
- TUI status line に `busy — finishing post-run work` 表示 (`crates/tui/src/ui.rs:880-890`)、Ctrl-X は Busy 中はガード (`crates/tui/src/main.rs:438-442`)。
|
||||
- `submit_input` も Busy 中は早期リターンし、誤って新しい turn を送信しない (`crates/tui/src/app.rs:298-302`)。
|
||||
|
||||
### Ctrl-C / Ctrl-X の分岐 (要件 4)
|
||||
- **達成**。`crates/tui/src/main.rs:434-442, 581-595` で `app.pod_status` の値ごとに明示的に分岐:
|
||||
- Ctrl-C: Running → Pause / それ以外 → 2-tap quit (TUI exit のみ、Pod は維持)。
|
||||
- Ctrl-X: Running → Cancel / Paused, Idle → Shutdown / Busy → ガード (押し戻し)。
|
||||
- 古い `running` / `paused` ローカルフラグも `pod_status` から派生するため二重情報源にはなっていない (`crates/tui/src/app.rs:110-118`)。
|
||||
|
||||
### Input polling (要件 3)
|
||||
- **達成**。`run_loop` を3段階に再構成 (`crates/tui/src/main.rs:281-338`):
|
||||
1. Pod event を最大 32 件まで非ブロッキングでドレイン (バウンド付きで starvation 防止)。
|
||||
2. `event::poll(ZERO)` で溜まった key event を全て処理、何かあれば再描画して即 `continue`。
|
||||
3. (1)(2) 共に空ならばはじめて `select!` で blocking poll または `client.next_event` を待つ。
|
||||
- これにより Pod event 大量受信時でも各イテレーションで key 入力チェックが必ず走るため、Ctrl-C/Ctrl-X 取りこぼしは構造的に解消されている。
|
||||
- `client.try_next_event` (新設) はバウンドドレイン用 (`crates/tui/src/client.rs:38-40`)。
|
||||
|
||||
### テスト (要件 5)
|
||||
- **達成**。`controller_test.rs` に 3本の状態遷移テストを追加:
|
||||
- `run_end_enters_busy_until_post_run_finishes_and_broadcasts_status` (要件 2)。
|
||||
- `attach_history_includes_current_status` (要件 1)。
|
||||
- `pause_while_busy_is_idempotent_not_not_running` (Pause が Busy 中に NotRunning エラーを出さないこと)。
|
||||
|
||||
### CLAUDE.md コンテキスト加工原則 (観点 6)
|
||||
- **違反なし**。`PodEvent` / `Notify` の系統は従来どおり `pod.push_notify(...)` で history append → 次の `pre_llm_request` で context へ流れる経路を踏襲 (`crates/pod/src/controller.rs:537, 683, 863, 883`)。
|
||||
- 状態 (Running / Busy 等) は context には差し込まれず、`Event::Status` / runtime_dir/status.json と Pod handle の `shared_state` のみで配信。
|
||||
|
||||
## アーキテクチャ・スコープ
|
||||
|
||||
### 良い点
|
||||
- `PodStatus` が「Pod 内部状態」から「protocol レベルの値」へ昇格したのは妥当。`Event::Status` を明示すれば外側の TUI/GUI/CLI が同じ source of truth を読める。
|
||||
- post-run jobs の実行を `run_post_run_jobs` ヘルパに集約 (`crates/pod/src/controller.rs:79-108`)、各 method 分岐 (Run / Notify / Resume / PodEvent) に重複していたロジックを `finish_controller_run` 1本にまとめている。本チケット前は同じ post-run 三段階が4箇所に重複していたため、純粋なリファクタとしても健全。
|
||||
- Busy → Idle の遷移を `finish_controller_run` 内で完結させ、controller の outer loop は `run_with_cancel_support` の戻り値だけ見れば良い構造。
|
||||
- TUI input polling のリファクタは "drain bounded → poll terminal → fall back to select" の3段で、よくある正攻法。
|
||||
|
||||
### 範囲外との切り分け (観点 7)
|
||||
- 入力キューイング (`tui-input-queue.md`) には踏み込んでいない。`tui-input-queue.md` は **"in-flight 中に submit された run の保留"** が主題で、本チケットは状態同期と入力 polling 構造のみ。サブミット中の Run は依然として `submit_input` の Busy ガード以外は controller 側 `AlreadyRunning` で拒否される (`crates/pod/src/controller.rs:464-470, 851-856`)。OK。
|
||||
|
||||
### 巻き込みの整理 (観点 8, 9)
|
||||
- 確認した周辺ファイル (`crates/manifest/src/scope.rs`, `crates/memory/src/consolidate/{input,lock,tidy}.rs`, `crates/tools/src/task.rs`, `crates/session-metrics/src/lib.rs`, `crates/pod/src/compact/prune.rs`, `crates/pod/src/ipc/interceptor.rs`, `crates/pod/tests/{consolidation_test,pod_comm_tools_test,session_metrics_test}.rs`) はすべて **rustfmt 由来の表面整形のみ** で、セマンティクスは変わっていない。`git diff HEAD` で確認済み。
|
||||
- これらは本チケットでは不要だが、`cargo fmt` の副産物として一緒に乗ってしまったタイプ。レビュー的には "別 PR ならクリーン" だが、blocking ではない。
|
||||
- `crates/pod/src/lib.rs` の `pub use` 変更は `PodStatus` が `protocol` 側に移動したことに伴う必須の追従。
|
||||
- `crates/pod/src/runtime/dir.rs` の変更も同上 (test の import パスを `protocol::PodStatus` へ）。
|
||||
- `crates/pod/tests/consolidation_test.rs` 等の `+/-` も大半が rustfmt とインポートパス更新。
|
||||
|
||||
## 指摘事項
|
||||
|
||||
### Blocking
|
||||
なし。
|
||||
|
||||
### Non-blocking / Follow-up
|
||||
- **エラー経路の Status::Busy broadcast 抜け** — `run_with_cancel_support` の `Err(e)` 分岐 (`crates/pod/src/controller.rs:820-835`) は `(PodStatus::Busy, ...)` を返すが、成功側 (`crates/pod/src/controller.rs:794-800`) と違って `Event::Status::Busy` を明示的に broadcast していない。post-run jobs はエラー時にも走るので、その間 TUI 側の `pod_status` は最後に受信した `Running` のままで、Busy バッジは出ない。Idle 遷移は `finish_controller_run` で必ず broadcast されるので致命的ではないが、UX としては "エラー後の数秒間 status が Running のまま見える" 可能性がある。`finish_controller_run` の冒頭 (Busy だった場合) にもう一度 Status::Busy を打つ等の整合化を検討してよい。
|
||||
- **`runtime_dir.write_status` が Busy では呼ばれない** — 成功分岐 (`crates/pod/src/controller.rs:794-800`) は `shared_state.set_status` と event broadcast はやるが `runtime_dir.write_status` は呼ばない。disk 上の `status.json` を外部プロセスが読む用途では Busy が見えない。`Event::Status` を購読していない CLI / 監視スクリプトには影響しうる。本チケットの完了条件としては Event 経路が満たせていれば足りるので blocking ではない。
|
||||
- **rustfmt の巻き込み** — `crates/manifest/`, `crates/memory/consolidate/`, `crates/tools/task.rs`, `crates/session-metrics/`, `crates/pod/src/compact/prune.rs`, `crates/pod/src/ipc/interceptor.rs` 等の整形変更は本チケットの目的外。次回以降は事前に `cargo fmt` 単独でコミットするか、関係ないファイルへのフォーマット差分は除外することを推奨。今回は意味的に害はないので blocking ではない。
|
||||
- **attach 直後の初期 status は `Idle`** — `App::new` で `pod_status: PodStatus::Idle` を初期値に置く (`crates/tui/src/app.rs:89-108`)。`GetHistory` 応答が届く前 (数十ms) に Ctrl-C を押すと "Idle 扱い" の 2-tap quit 経路に流れる。実害は小さく要件 1 に違反しないが、コメントに記すか `History` 受信前は Ctrl 系を抑制する手もある。Follow-up 候補。
|
||||
- **post-run 中は method_rx が pump されない** — `finish_controller_run` 内で `run_post_run_jobs` を逐次 await しているため、その間 `Method::Run` / `Notify` / `Pause` / `Shutdown` は mpsc キューで待たされる。Pause/Shutdown を Busy 中に押した場合、後続の Idle 遷移後に処理される (Pause は idempotent no-op、Shutdown は遅れて反映)。要件は満たすが「Ctrl-X (Shutdown) 押下から数秒遅延しうる」点はドキュメント or follow-up 検討の余地。
|
||||
|
||||
### Nits
|
||||
- `crates/pod/src/controller.rs:117-135` `finish_controller_run` の `new_status == PodStatus::Busy` 比較は2回出る。enum 拡張時の保守性のため、ローカル変数 `is_busy` で1回にまとめてもよい。
|
||||
- `crates/pod/src/ipc/interceptor.rs:18-22` の `tracing::warn` import 順は `cargo fmt` がやった整形だが、本チケットの差分から外せれば望ましい。
|
||||
|
||||
## 判断
|
||||
|
||||
**Approve with follow-up** — チケットの完了条件 (Running/Paused 同期、Busy の表示、starvation 解消、ユニットテスト) はすべて満たされており、設計の方向性 (PodStatus を protocol の一級型へ昇格、post-run jobs の集約) は健全。コードベースを歪めるような複雑化は見られず、範囲外の入力キューイングにも踏み込んでいない。エラー経路の Busy 表示と rustfmt 巻き込みは follow-up でフォローすればよく、blocking ではない。
|
||||
Loading…
Reference in New Issue
Block a user