diff --git a/TODO.md b/TODO.md index 47487142..68c2d29a 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: Paused→Run の interrupt 前処理を Pod 内部に閉じる → [tickets/pod-interrupt-prep-internalize.md](tickets/pod-interrupt-prep-internalize.md) -- Pod: Run 入力の事前 validate を Pod 内部に閉じる → [tickets/pod-input-validate-internalize.md](tickets/pod-input-validate-internalize.md) - Pod: Inbound PodEvent ハンドリングの重複を統合 → [tickets/pod-inbound-pod-event-dedup.md](tickets/pod-inbound-pod-event-dedup.md) - Pod: セッションログをバックエンドにした Pod 単位の永続化 → [tickets/pod-persistent-state.md](tickets/pod-persistent-state.md) - 永続化層のセマンティック整理 → [tickets/persistence-semantics.md](tickets/persistence-semantics.md) diff --git a/tickets/pod-input-validate-internalize.md b/tickets/pod-input-validate-internalize.md deleted file mode 100644 index ccf9b59f..00000000 --- a/tickets/pod-input-validate-internalize.md +++ /dev/null @@ -1,55 +0,0 @@ -# Run 入力の事前 validate を Pod 内部に閉じる - -## 背景 - -`controller_loop` は `Method::Run { input }` を受けた直後、`drive_turn` に流す前に `pod.validate_workflow_invocations(&input)` を呼んで invalid なら `ErrorCode::InvalidRequest` を返している (`crates/pod/src/controller.rs:613`): - -```rust -if let Err(e) = pod.validate_workflow_invocations(&input) { - let _ = event_tx.send(Event::Error { - code: ErrorCode::InvalidRequest, - message: e.to_string(), - }); - continue; -} -``` - -これは Pod 内部の `workflow_registry` を覗いてバリデーション判定する責務漏れになっている。`pod.validate_workflow_invocations` は read-only 関数で、`Pod::run` の冒頭でも当然呼べる。 - -なぜ controller が肩代わりしているかというと、`Pod::run` (`crates/pod/src/pod.rs:1138`) は冒頭で: - -```rust -self.commit_entry(LogEntry::UserInput { ts: ..., segments: input.clone() })?; -self.user_segments.push(input.clone()); -// ... -attachments.extend(self.resolve_workflow_invocations(&input)?); -``` - -の順で動く。`resolve_workflow_invocations` が失敗を返すころには既に UserInput が session log に commit されてしまっており、「invalid な input が history に残る」状態になる。それを避けるために controller 側で先打ち validate している。 - -[[pod-interrupt-prep-internalize]] と同じパターン: Pod 内部の整合性を保つ前処理が Controller 層に染み出している。 - -## 要件 - -- `Pod::run` の冒頭、`commit_entry(LogEntry::UserInput { .. })` よりも前に `validate_workflow_invocations` を呼び、エラー時は session log に何も commit せず `PodError` で early return する。 -- `controller_loop` の `Method::Run` ハンドリングから事前 validate 呼び出しを削除する。 -- `Pod::run` から返る `PodError`（新規に追加する workflow validation 失敗のバリアントを含む）を `worker_error_code` 等の controller 側エラーコードマッピングで `ErrorCode::InvalidRequest` にマップする。 - - `PodError` に既存の InvalidRequest 相当バリアントがあればそれを使う。無ければ `WorkflowResolveError` をラップする最小限のバリアントを追加する。 -- `Pod::validate_workflow_invocations` メソッドの可視性は `pub` のままでよい（IPC `ListCompletions` 経路や他テストから参照される可能性があるので外向き API を狭めない）。 - -## 完了条件 - -- `controller.rs` の `Method::Run` ブランチに `pod.validate_workflow_invocations` の呼び出しが残っていない。 -- 不正な workflow slug を含む `Method::Run` を投げると、UserInput が session log に commit されないまま `Event::Error { code: InvalidRequest, .. }` が flow する（既存の挙動と同等）。 -- 既存の workflow invocation 関連テスト（成功 / NotFound / NotUserInvocable / InvalidSlug）が通る。 - -## 範囲外 - -- `resolve_workflow_invocations` 側のロジック変更。 -- `validate_workflow_invocations` の判定基準変更（user_invocable / slug parse 等）。 -- `Method::Run` 以外の経路（Resume / RunForNotification）からの入力検証。Resume は入力を取らず、RunForNotification の入力は notify buffer drain で system message として入るため workflow invocation の経路に乗らない。 - -## Review -- 状態: Approve with follow-up -- レビュー詳細: [./pod-input-validate-internalize.review.md](./pod-input-validate-internalize.review.md) -- 日付: 2026-05-15 diff --git a/tickets/pod-input-validate-internalize.review.md b/tickets/pod-input-validate-internalize.review.md deleted file mode 100644 index e01aa2ee..00000000 --- a/tickets/pod-input-validate-internalize.review.md +++ /dev/null @@ -1,36 +0,0 @@ -# Review: Run 入力の事前 validate を Pod 内部に閉じる - -## 前提・要件の確認 - -- 要件1「`Pod::run` の冒頭、`commit_entry(LogEntry::UserInput { .. })` よりも前に `validate_workflow_invocations` を呼び、エラー時は session log に何も commit せず `PodError` で early return」: 満たされている。`crates/pod/src/pod.rs:1138-1145` で `self.validate_workflow_invocations(&input)?;` が `prepare_for_run` よりも前、すなわち `ensure_session_head` / pre-run compact / `commit_entry(UserInput)` のいずれよりも手前にある。`?` で `WorkflowResolveError` が `PodError::WorkflowResolve` に lift され early return される。 -- 要件2「`controller_loop` の `Method::Run` ハンドリングから事前 validate 呼び出しを削除」: 満たされている。`crates/pod/src/controller.rs:600-630` の `Method::Run` ブランチに validate 呼び出しは存在しない。 -- 要件3「`PodError` を `worker_error_code` で `ErrorCode::InvalidRequest` にマップ」: 満たされている。`crates/pod/src/controller.rs:944` に `PodError::WorkflowResolve(_) => ErrorCode::InvalidRequest` 分岐が追加され、ticket が示した「既存バリアントの再利用」方針通り `WorkflowResolveError` を新規ラップせずに既存 `PodError::WorkflowResolve(#[from] WorkflowResolveError)` (`pod.rs:3177-3178`) を流用している。 -- 要件4「`Pod::validate_workflow_invocations` の可視性は `pub` のまま」: 満たされている (`pod.rs:1407`)。docstring は controller 経路ではなく `Pod::run` / `Pod::interrupt_and_run` 入口で呼ぶ旨に更新済 (`pod.rs:1402-1406`)。 - -## 完了条件の確認 - -- 「`controller.rs` の `Method::Run` ブランチに `pod.validate_workflow_invocations` の呼び出しが残っていない」: OK (`grep` で残存呼び出しなし、Pod::run/interrupt_and_run のみ参照)。 -- 「不正な workflow slug を含む `Method::Run` を投げると、UserInput が session log に commit されないまま `Event::Error { code: InvalidRequest, .. }` が flow する」: OK。`Pod::run` 入口の `validate_workflow_invocations?` が `prepare_for_run` / `commit_entry(UserInput)` よりも手前で抜けるため session log には何も書かれない。`drive_turn` の `Err(e)` ブランチが `worker_error_code` 経由で `InvalidRequest` を載せた `Event::Error` を broadcast する。 -- 「既存テスト通過」: `cargo test -p pod --lib` 173/173 通過 (再現確認済)。`workflow::tests::{resolves_requires_before_workflow_body, user_invocable_false_errors, missing_required_knowledge_errors}` は `resolve_workflow_invocation` の関数単体テストで本 refactor の影響を受けない。 - -## アーキテクチャ・スコープ - -- 責務漏れの是正方向は妥当: controller が Pod 内部の `workflow_registry` を覗いて条件分岐していたのを Pod の入口に集約。read-only 関数を本来の所有者側で呼ぶだけの最小変更。 -- 範囲外項目 (resolve ロジック / 判定基準 / Resume / RunForNotification) には踏み込んでいない。`run_for_notification` / `resume` は user 入力 segments を受け取らないので validate 経路に乗らない点もコードと整合。 -- `interrupt_and_run` 側にも validate を入れた判断は妥当。チケットには明示されていないが、ticket 背景にある同じ責務漏れパターン (`pod-interrupt-prep-internalize`) と整合させるため必要。validate 抜きで `interrupt_and_run` を呼ぶと orphan tool_result closure / interrupt system note が worker history に書き込まれた後で `Pod::run` 入口の validate に弾かれ、half-applied 状態が残る。`interrupt_and_run.rs:31-37` のコメントが `pod-interrupt-prep-internalize` 統合時に自然に重複解消する旨を明示しており、将来の畳み込みに対する負債もない。 -- `worker_error_code` の新規 `WorkflowResolve` 分岐は副作用として post-commit な resolve エラー (`KnowledgeNotFound` / `KnowledgeRead` / `KnowledgeFrontmatter`) も `InvalidRequest` にマップする。これらは以前 `_ => Internal` に落ちていたため挙動変化だが、意味論的にはより適切方向であり、validate / resolve でバリアントを分けるのは本チケットの範囲を超える。許容範囲。 - -## 指摘事項 - -### Non-blocking / Follow-up - -- `Event::UserMessage` 早期 broadcast 起因の UX 差分: 旧実装では validate 失敗時、`Event::UserMessage` も `Event::Status { Running }` も飛ばなかった。新実装では bad slug の場合でも UserMessage → Status(Running) → Error → Status(Idle) が並ぶ。TUI (`crates/tui/src/app.rs:478-485`) は `UserMessage` 受信で `turn_index += 1` し TurnHeader + UserMessage block を push するため、エラーが返るだけのリクエストでも turn 番号が前進し、ターンヘッダと user line がエラーブロックの上に残る。チケットの完了条件 (session log への UserInput commit 抑止) には抵触しないが、`bad slug → ghost turn header が残る` 表示挙動は旧実装と異なる。フォローアップで controller 側の `Method::Run` ハンドリングを「validate を controller でも呼ばずに Pod 側に丸投げするのは維持しつつ、UserMessage broadcast は drive_turn の中で投げる」「Pod::run 内で resolve 成功直後に Event::UserMessage 相当を内側から飛ばす」等の動線整理を検討する余地がある。本チケットの責務漏れ修正としては完了範囲外と判断。 -- `worker_error_code` の `WorkflowResolve` 分岐コメント: validate-stage 失敗 (InvalidSlug / NotFound / NotUserInvocable) と resolve-stage 失敗 (KnowledgeNotFound / KnowledgeRead / KnowledgeFrontmatter) が同じ `InvalidRequest` に丸まる旨を一行コメントで明示しておくと将来の差分判断が楽になる。Nit に近い。 - -### Nits - -- なし。 - -## 判断 - -Approve with follow-up — チケットの前提・要件・完了条件は全て満たされ、責務漏れの修正方向も妥当。`Event::UserMessage` 早期 broadcast による turn 表示差分は許容範囲のトレードオフ (実装者ノート通り) で、本チケットの完了範囲外として別途整理する余地のみ残る。