From 28fe1dae1c8f184edb5d04d2e496ed6502566c4b Mon Sep 17 00:00:00 2001 From: Hare Date: Sun, 3 May 2026 19:03:48 +0900 Subject: [PATCH 1/6] =?UTF-8?q?docs(tickets):=20=E3=82=BB=E3=83=83?= =?UTF-8?q?=E3=82=B7=E3=83=A7=E3=83=B3=E5=86=85=20Task=20=E3=83=84?= =?UTF-8?q?=E3=83=BC=E3=83=AB=E3=82=92=E6=9C=AC=E4=BD=93=E3=81=A8=E6=B3=A8?= =?UTF-8?q?=E6=84=8F=E6=A9=9F=E6=A7=8B=E3=81=AB=E5=88=86=E5=89=B2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- TODO.md | 4 +- tickets/session-todo-reminder.md | 60 ++++++++++++++++ tickets/session-todo.md | 115 ++++++++++++++++++++----------- 3 files changed, 136 insertions(+), 43 deletions(-) create mode 100644 tickets/session-todo-reminder.md diff --git a/TODO.md b/TODO.md index 13b0d465..e7e49967 100644 --- a/TODO.md +++ b/TODO.md @@ -15,6 +15,8 @@ - Manifest: Tool Output / File Upload 上限の分離とデフォルト緩和 → [tickets/manifest-output-upload-limits.md](tickets/manifest-output-upload-limits.md) - メモリ機構 - 使用頻度メトリクス + Knowledge 化候補レポート → [tickets/memory-usage-metrics.md](tickets/memory-usage-metrics.md) -- セッション内 TODO ツール（注意機構付き） → [tickets/session-todo.md](tickets/session-todo.md) +- セッション内 Task ツール + - ツール本体（TaskCreate / TaskUpdate / TaskList / TaskGet + 永続化 / compact 跨ぎ） → [tickets/session-todo.md](tickets/session-todo.md) + - 注意機構（無アクティビティで `` ナッジ） → [tickets/session-todo-reminder.md](tickets/session-todo-reminder.md) - ワークスペースのメモリーをLintするヘッドレスCLI - system-reminder 注入機構の汎用化（2件目の利用者が出た時に検討。タグ形式と「履歴を汚さない」原則は session-todo で先行確立） diff --git a/tickets/session-todo-reminder.md b/tickets/session-todo-reminder.md new file mode 100644 index 00000000..d0a1261a --- /dev/null +++ b/tickets/session-todo-reminder.md @@ -0,0 +1,60 @@ +# セッション内 Task ツールの注意機構 + +## 背景 + +`tickets/session-todo.md` で導入する Task ツール群があっても、LLM はそれを使わずに作業を続け得る。ツールを呼ばないまま会話が長引くと、 + +- 開始した作業の `inprogress` がずっと放置されたままになる +- 「やったつもり」になって `completed` への更新を忘れる +- そもそも TaskStore の存在を忘れて、構造化を諦めて自由記述に回帰する + +OpenCode の todo は専用の注意機構を持たない（汎用 reminder 経由）。一方 Claude Code は `task_reminder` を「N リクエスト無アクティビティで初めて発火するナッジ型」として実装しており、毎リクエスト押し戻しはしない（`/home/hare/.local/share/claude/versions/2.x` の `du_` / `cu_` 関数、閾値 `Z_8 = { TURNS_SINCE_WRITE: 10, TURNS_BETWEEN_REMINDERS: 10 }`）。 + +Insomnia でも同方針を採り、active Task が残っているのに `TaskCreate` / `TaskUpdate` が一定リクエスト呼ばれていない場合に限り、`` で揮発的に思い出させる。「やったつもり」抑止と、トークン浪費・LLM の自律性侵害のバランスを取るため、毎リクエスト押し戻しはしない。 + +## 前提 + +- `tickets/session-todo.md` の TaskStore と `TaskCreate` / `TaskUpdate` / `TaskList` / `TaskGet` ツールが利用可能であること +- `pre_llm_request` 相当のフックを Pod が持つこと（無ければ本ticket で導入） + +## 方針 + +- **`pre_llm_request` Interceptor として実装**。直近の user message に `` ブロックを揮発的に append するだけ。履歴・ログには載せない +- **system-reminder 注入の汎用化はやらない**。利用者が Task 1機構しかない段階で抽象を立てない（CLAUDE.md「概念の追加は不在が問題になってから」）。ただし「タグ形式は `...` で揃える」「履歴は汚さない」の2点は本実装で確立し、将来の追加機構が同じ規約に乗れるようにする +- **発火はナッジ型**。N リクエスト無アクティビティで初めて発火し、cooldown も持つ + +## 要件 + +### Interceptor + +- `pre_llm_request` で `Vec` を受け取り、以下の AND を満たした場合のみ発動 + - active Task（`pending` または `inprogress`）が1件以上存在する + - 直近 N リクエスト (暫定 N=8) `TaskCreate` / `TaskUpdate` のいずれも呼ばれていない + - 前回 reminder 注入から M リクエスト (暫定 M=8) 以上経過 +- ここで言う「リクエスト」は **LLM への1回の推論呼び出し (= assistant 応答1回)** の単位で数える。ユーザー発火単位ではない。1ユーザー発火内で tool ループが回れば、tool_result を受けて発火する次のリクエストもそれぞれ1としてカウントする +- カウント対象はメインスレッドの assistant 応答に限る。サブエージェント / sidechain の assistant 応答は除外する +- カウンタは Pod 側の session-lifetime 状態として保持する（`requests_since_last_task_management` / `requests_since_last_reminder`）。resume 時は履歴の逆走査で再計算するか 0 リセットで再開する。どちらでも「初回ナッジが最大 N リクエスト遅れる」だけで挙動として致命ではない +- 発動時、直近の user message の content（または content[最終 text part]）の末尾に `` ブロックを append し、現在の active Task リストを `taskid` / `status` / `subject` を含む簡潔な形式で列挙する。`description` は長大化を避けるため省略してよい +- 履歴 (`Worker` の保持する `Vec`) は変更しない。リクエスト送信時の Vec のみ加工する +- active Task が空の場合は何も差し込まない（忘却防止が目的なので、思い出させる対象が無いなら不要） + +## 完了条件 + +- 直近 N リクエスト連続で `TaskCreate` / `TaskUpdate` が呼ばれず、かつ active Task が残っている場合に限り、`pre_llm_request` で `` が直近 user message に append される +- `TaskCreate` / `TaskUpdate` のいずれかが呼ばれるとカウンタがリセットされ、再び N リクエスト経過するまでは reminder が出ない +- reminder が一度出たあとは、cooldown M リクエストが経過するまで再注入されない +- active Task が0件の場合は reminder が出ない +- system-reminder の注入は揮発的で、`get_history` / セッションログには現れない +- 単体テストで Interceptor の発火条件（リクエスト回数閾値、active 0件、cooldown、サブエージェント除外）がカバーされる + +## 範囲外 + +- inprogress 滞留検出 / 多重 inprogress 検出など、状態異常ベースの追加トリガ（必要になれば別チケットで追加） +- system-reminder 注入機構の汎用化（`TODO.md` に立項済み、別途検討） +- `TaskCreate` / `TaskUpdate` の戻り値に active Task 全件を埋め込む強化（必要に応じて Tool ticket 側で対応） + +## 参照 + +- 設計指針: `CLAUDE.md`（最小の構造化 / 概念の追加は不在が問題になってから） +- 前提: `tickets/session-todo.md`（Tool 群と TaskStore） +- 参考実装: Claude Code の `task_reminder`（`du_` / `cu_` 関数、閾値 `Z_8 = { TURNS_SINCE_WRITE: 10, TURNS_BETWEEN_REMINDERS: 10 }`） diff --git a/tickets/session-todo.md b/tickets/session-todo.md index 81994966..3dc27291 100644 --- a/tickets/session-todo.md +++ b/tickets/session-todo.md @@ -1,74 +1,105 @@ -# セッション内 TODO ツール +# セッション内 Task ツール ## 背景 -長めのタスクを LLM に進めさせる際、Claude Code / OpenCode が備える「セッション内 TODO リスト」相当の機構が無いため、エージェントが自分の作業計画を構造化された形で保持・更新できない。Reasoning や text 出力の中で擬似的に TODO を書くことはできるが、 +長めのタスクを LLM に進めさせる際、Claude Code / OpenCode が備える「セッション内 TODO / Task リスト」相当の機構が無いため、エージェントが自分の作業計画を構造化された形で保持・更新できない。Reasoning や text 出力の中で擬似的に TODO を書くことはできるが、 -- ターンを跨いだとき直近の TODO 状態が context から押し出される - compact を跨ぐと完全に消える -- ツール結果ではないため、状態の上書き・部分更新の規約が決まらず、意図と乖離した「やったつもり」を引き起こす +- ツール結果ではないため、状態の作成・更新・削除の規約が決まらず、意図と乖離した「やったつもり」を起こしやすい +- 安定した識別子が無いため、後から特定 entry を更新できない -この用途のために、セッション内に正規化された TODO リストを保持し、ターンごとに LLM へ最新状態を再提示（注意機構）し、compact を跨いで保存される専用ツールを導入する。 +この用途のために、セッション内に正規化された Task リストを保持し、compact を跨いで保存される専用ツールを導入する。 + +「LLM がツールを使わなくなった場合のサボり防止」を目的とした注意機構（`` の自動注入）は本チケットの範囲外。`tickets/session-todo-reminder.md` で別途扱う。 ## 方針 -- **保存先は `tools` 層の session-lifetime 状態**。`Tracker` と同じ生存スコープで `Pod` が所有。`Arc>>` ベースの `TodoStore` を tool に注入する -- **永続化は専用レーンを持たない**。`tool_call.arguments` がセッションログに既に乗っているため、resume 時には履歴 replay の中で最後の `todo_write` 引数を `TodoStore` に再適用すれば状態が復元される -- **注意機構は `pre_llm_request` Interceptor**。直近の user message に `` ブロックを揮発的に append するだけ。履歴・ログには載せない -- **system-reminder 注入の汎用化はやらない**。利用者が TODO 1個しかない段階で抽象を立てない（CLAUDE.md「概念の追加は不在が問題になってから」）。ただし「タグ形式は `...` で揃える」「履歴は汚さない」の2点は本実装で確立し、将来の追加機構が同じ規約に乗れるようにする +- **保存先は `tools` 層の session-lifetime 状態**。`Tracker` と同じ生存スコープで `Pod` が所有。`Arc>` ベースの store を tool に注入する +- **Task は差分操作**。全置換ではなく `TaskCreate` / `TaskUpdate` によって TaskStore を逐次更新する +- **永続化は専用レーンを持たない**。`tool_call.arguments` がセッションログに既に乗っているため、resume 時には履歴 replay の中で `TaskCreate` / `TaskUpdate` を順に再適用すれば状態が復元される + +## Task データモデル + +各 Task entry は以下の4フィールドを持つ。 + +- `taskid`: store が自動採番する整数 ID。`TaskCreate` ごとに単調増加する +- `status`: `pending | inprogress | completed | deleted` +- `subject`: 1行程度の短い件名 +- `description`: 詳細説明 + +状態遷移の通常ルートは `pending -> inprogress -> completed` とする。完了ではなく一覧から取り下げたい場合は、物理削除ではなく `TaskUpdate` で `status = deleted` を設定する。 ## 要件 -### `todo_write` ツール +### `TaskCreate` ツール -- 入力は TODO リスト全体（全置換）。差分更新は受けない -- 各エントリは `id` / `content` / `status (pending | in_progress | completed)` の 3 フィールド -- `id` は LLM 側が一貫して採番できる文字列。同 id があれば置換、なければ新規。順序は配列順を信頼 -- 戻り値は更新後のスナップショットを summary に含める（次ターンで再確認可能） -- 読み出し専用ツール（`todo_read`）は作らない。注意機構と tool result snapshot で代替 +- 入力は `subject` / `description` のみ +- TaskStore の末尾に新規 entry を push する +- `taskid` は TaskStore が自動で割り当てる。LLM は指定できない +- 初期 `status` は `pending` +- 戻り値は作成された Task と、更新後の TaskStore スナップショット概要を summary に含める + +### `TaskList` ツール + +- 入力なし +- 現在の TaskStore スナップショットを返す +- `taskid` / `status` / `subject` / `description` を含める +- `completed` / `deleted` も含め、store に残っている Task をすべて列挙する + +### `TaskGet` ツール + +- 入力は `taskid` +- 指定された Task を1件返す +- 存在しない `taskid` はエラーにする + +### `TaskUpdate` ツール + +- 入力は `taskid` と、更新する `status` / `subject` / `description` のいずれか +- 少なくとも1フィールドは更新指定が必要 +- `taskid` は変更できない +- `status` は `pending | inprogress | completed | deleted` のいずれか +- 存在しない `taskid` はエラーにする +- 削除は `status = deleted` への更新として表現し、entry は store から取り除かない +- 戻り値は更新された Task と、更新後の TaskStore スナップショット概要を summary に含める ### Resume 時の復元 -- `Pod::resume` の履歴 replay 中に `todo_write` の `tool_call.arguments` を観測したら、`TodoStore` を引数値で上書き +- `Pod::resume` / restore の履歴 replay 中に `TaskCreate` / `TaskUpdate` の `tool_call.arguments` を観測したら、TaskStore に順に再適用する +- `TaskCreate` は replay 時も自動採番を行う。ログ上の call 順序が保持されるため、通常の replay では元の `taskid` が再現される +- `TaskUpdate` は replay 済みの TaskStore に対して適用する - 専用 LogEntry / Persistence 型は追加しない（`Tracker` と同じ方針） -- `tool_call.arguments` のフォーマットが `todo_write` の引数 schema と乖離した場合（旧バージョンのログ）は、その call を無視してよい +- `tool_call.arguments` のフォーマットが現在の schema と乖離した場合（旧バージョンのログ）は、その call を無視してよい ### Compact 跨ぎ -- compact 起動時、Pod は現在の `TodoStore` スナップショットを compact worker context に渡す -- compact worker は summary を書く際、未完了 TODO を summary 文に取り込める情報源として参照する（強制ではない） -- compact 後の新セッション開始時、Pod は **`mark_read_required` と同じ system message 注入レーン**に「未完了 TODO スナップショット」を 1 メッセージとして注入する -- 新セッションは空の `TodoStore` で始まる。次に LLM が `todo_write` を呼び出した時点で再構築される（system message に書かれたスナップショットがその拠り所） -- compact worker に TODO 編集権限は与えない（消去・縮約はしない） - -### 注意機構（Interceptor） - -- `pre_llm_request` で `Vec` を受け取り、未完了 TODO（`pending` または `in_progress`）が 1 件でも存在する場合に発動 -- 直近の user message の content（または content[最終 text part]）の末尾に `` ブロックを append -- ブロック内には現在の TODO リストを、status を含む簡潔な形式で列挙 -- 履歴 (`Worker` の保持する `Vec`) は変更しない。リクエスト送信時の Vec のみ加工 -- TODO が空の場合は何も差し込まない +- compact 起動時、Pod は現在の TaskStore **全スナップショット**を compact worker context に渡す +- compact worker は summary を書く際、TaskStore を参照できる。未完了 Task を summary 文に取り込める情報源として参照する（強制ではない） +- compact worker に Task 編集権限は与えない（作成・更新・削除はしない） +- compact 後の新セッションでも TaskStore は空にせず、compact 前の TaskStore **全件**を維持する。`completed` / `deleted` も含めて保持する +- compact 後の新セッション開始時、Pod は **`mark_read_required` と同じ system message 注入レーン**に TaskStore 全スナップショットを1メッセージとして注入する +- compact 完了後、Pod は新セッションの継続前に必ず `TaskList` を発火させ、LLM が compact 後の TaskStore 状態を tool result として再確認できるようにする ## 完了条件 -- `todo_write` ツールが builtin tool として登録され、Pod で利用できる -- LLM が `todo_write` を呼ぶと TodoStore が更新され、その後の `pre_llm_request` で system-reminder として LLM に再提示される -- セッションを resume すると、最後の `todo_write` の状態から再開される -- compact を跨いでも、未完了 TODO が新セッション冒頭の system message として残る -- system-reminder の注入は揮発的で、`get_history` / セッションログには現れない -- 単体テストで `todo_write` の更新挙動 / replay 復元 / Interceptor の差し込みがカバーされる +- `TaskCreate` / `TaskList` / `TaskGet` / `TaskUpdate` が builtin tool として登録され、Pod で利用できる +- LLM が `TaskCreate` を呼ぶと TaskStore に pending Task が末尾追加され、自動採番された `taskid` が返る +- LLM が `TaskUpdate` を呼ぶと既存 Task の `status` / `subject` / `description` が更新される +- `TaskList` / `TaskGet` で TaskStore の状態を確認できる +- セッションを resume すると、履歴中の `TaskCreate` / `TaskUpdate` replay により TaskStore が復元される +- compact を跨いでも、TaskStore 全件が維持され、新セッション冒頭の system message と `TaskList` 発火により再提示される +- 単体テストで TaskCreate / TaskUpdate / TaskList / TaskGet の挙動、replay 復元がカバーされる ## 範囲外 -- 差分更新 API（add / remove / patch）。全置換のみで十分 -- TODO 階層・優先度・タグ -- TUI / GUI での TODO 状態の可視化（ツール呼び出しのイベントは既に流れているので、クライアント側で表示するかは別軸） -- system-reminder 注入機構の汎用化（`TODO.md` に立項済み、別途検討） -- TODO の永続化を専用 LogEntry に分離する設計（現方針は tool_call replay で復元、追加レーン不要） -- 複数 Pod 間で TODO を共有する仕組み +- 注意機構（`` の自動注入による「サボり防止」）→ `tickets/session-todo-reminder.md` +- Task の階層・優先度・タグ +- TUI / GUI での Task 状態の可視化（ツール呼び出しのイベントは既に流れているので、クライアント側で表示するかは別軸） +- Task の永続化を専用 LogEntry に分離する設計（現方針は tool_call replay と compact 時 snapshot 維持で復元、追加レーン不要） +- 複数 Pod 間で Task を共有する仕組み ## 参照 - 設計指針: `CLAUDE.md`（最小の構造化 / 概念の追加は不在が問題になってから） - 参考実装: Claude Code の TodoWrite、OpenCode の todo tool - 関連: `crates/tools/src/tracker.rs`（session-lifetime 状態の前例）、`crates/pod/src/compact/worker.rs`（auto-injection レーン） +- 後続: `tickets/session-todo-reminder.md`（注意機構） From 6f2aca84bfadcb52a495b5d0ba067a5c195c110d Mon Sep 17 00:00:00 2001 From: Hare Date: Sun, 3 May 2026 19:03:52 +0900 Subject: [PATCH 2/6] =?UTF-8?q?feat:=20=E3=82=BB=E3=83=83=E3=82=B7?= =?UTF-8?q?=E3=83=A7=E3=83=B3=E5=86=85=20Task=20=E3=83=84=E3=83=BC?= =?UTF-8?q?=E3=83=AB=20(TaskCreate/List/Get/Update=20+=20=E5=B1=A5?= =?UTF-8?q?=E6=AD=B4=20replay=20+=20compact=20=E8=B7=A8=E3=81=8E)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/pod/src/controller.rs | 11 +- crates/pod/src/ipc/interceptor.rs | 2 +- crates/pod/src/pod.rs | 62 ++- crates/tools/src/lib.rs | 9 +- crates/tools/src/task.rs | 612 ++++++++++++++++++++++++++++++ crates/tools/src/tracker.rs | 3 +- crates/tools/tests/edge_cases.rs | 9 +- crates/tools/tests/integration.rs | 68 +++- 8 files changed, 756 insertions(+), 20 deletions(-) create mode 100644 crates/tools/src/task.rs diff --git a/crates/pod/src/controller.rs b/crates/pod/src/controller.rs index 4056c260..ce4a6a33 100644 --- a/crates/pod/src/controller.rs +++ b/crates/pod/src/controller.rs @@ -133,6 +133,7 @@ impl PodController { // Stashed during tool registration below so we can attach a // `PodFsView` to the shared state once the latter exists. let fs_for_view: tools::ScopedFs; + let task_store = pod.task_store(); // Register event bridge callbacks on the worker { @@ -257,13 +258,19 @@ impl PodController { // worker) reads from it, and any future scope mutation // (SpawnPod-style revoke, future GrantScope) propagates // through it. - let fs = tools::ScopedFs::with_shared_scope(scope_handle.clone(), pwd_for_tools.clone()); + let fs = + tools::ScopedFs::with_shared_scope(scope_handle.clone(), pwd_for_tools.clone()); let tracker = tools::Tracker::new(); // The same ScopedFs also powers the IPC `ListCompletions` // query — keep a clone for the FS view we attach below, // since the tools consume `fs` itself. fs_for_view = fs.clone(); - worker.register_tools(tools::builtin_tools(fs, tracker.clone(), bash_output_dir)); + worker.register_tools(tools::builtin_tools( + fs, + tracker.clone(), + task_store.clone(), + bash_output_dir, + )); // Memory subsystem opt-in. When `[memory]` is present in // the manifest, register the memory-specific Read/Write/Edit diff --git a/crates/pod/src/ipc/interceptor.rs b/crates/pod/src/ipc/interceptor.rs index f7b9d9de..359ed9fe 100644 --- a/crates/pod/src/ipc/interceptor.rs +++ b/crates/pod/src/ipc/interceptor.rs @@ -127,7 +127,7 @@ impl Interceptor for PodInterceptor { // Internal mechanism: between-requests compaction trigger (safety net). if let Some(state) = self.compact_state.as_ref() { - if !state.is_disabled() { + if !state.is_disabled() && !state.just_compacted() { let current = current_tokens.unwrap_or(0); if state.exceeds_request(current) { info!( diff --git a/crates/pod/src/pod.rs b/crates/pod/src/pod.rs index b3a4c073..8791c304 100644 --- a/crates/pod/src/pod.rs +++ b/crates/pod/src/pod.rs @@ -95,6 +95,11 @@ pub struct Pod { /// tools so that Pod-owned operations (e.g. compaction) can consult /// the recency of touched files. tracker: Option, + /// Session-lifetime task store from the builtin `tools` crate. Shared by + /// TaskCreate / TaskUpdate / TaskList / TaskGet and preserved across + /// compaction by keeping the same handle while the Worker history is + /// replaced. Restored Pods reconstruct it by replaying Task* tool calls. + task_store: tools::TaskStore, /// Parsed system-prompt template awaiting first-turn materialisation. /// `Some` until `ensure_system_prompt_materialized` renders it once, /// then `None` forever — including after compaction. @@ -211,6 +216,7 @@ impl Pod { metrics_tracker: Arc::new(crate::compact::metrics_tracker::MetricsTracker::new()), usage_history: Arc::new(Mutex::new(Vec::::new())), tracker: None, + task_store: tools::TaskStore::new(), system_prompt_template: None, alerter: None, event_tx: None, @@ -425,6 +431,18 @@ impl Pod { self.tracker = Some(tracker); } + /// Attach the session-scoped TaskStore from the builtin `tools` crate. + /// Called by the Controller before registering builtin tools so the Pod + /// and Worker share one store. + pub fn attach_task_store(&mut self, task_store: tools::TaskStore) { + self.task_store = task_store; + } + + /// Shared TaskStore handle. + pub fn task_store(&self) -> tools::TaskStore { + self.task_store.clone() + } + /// The attached session-scoped file-operation tracker, if any. pub fn tracker(&self) -> Option<&tools::Tracker> { self.tracker.as_ref() @@ -1255,8 +1273,14 @@ impl Pod { .unwrap_or_default(); // Input text fed to the compact worker. Includes the default - // references and the (pruned) conversation text. - let summary_input = build_summary_input(&items_to_summarise, &default_refs); + // references, current TaskStore snapshot, and the (pruned) + // conversation text. + let task_snapshot_text = self.task_store.snapshot_text(); + let summary_input = build_summary_input( + &items_to_summarise, + &default_refs, + Some(task_snapshot_text.as_str()), + ); // Worker-side state collected by the compact worker's tool calls. let ctx = Arc::new(std::sync::Mutex::new(CompactWorkerContext::with_budget( @@ -1371,9 +1395,10 @@ impl Pod { .filter(|i| i.is_user_message()) .count(); - // Build new history: [summary, ...auto-read, references, ...retained]. + // Build new history: [summary, ...auto-read, task snapshot, TaskList result, references, ...retained]. let mut new_history = Vec::with_capacity( 1 + auto_read_messages.len() + + 3 + reference_message.is_some() as usize + retained_items.len(), ); @@ -1381,6 +1406,17 @@ impl Pod { "[Compacted context summary]\n\n{summary_text}" ))); new_history.extend(auto_read_messages); + new_history.push(Item::system_message(format!( + "[Session TaskStore snapshot]\n\n{task_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." + ))); + new_history.push(Item::tool_call("compact-tasklist", "TaskList", "{}")); + new_history.push(Item::tool_result_with_content( + "compact-tasklist", + tools::task::snapshot_overview(&self.task_store.list()), + task_snapshot_text.clone(), + )); if let Some(msg) = reference_message { new_history.push(msg); } @@ -1978,6 +2014,7 @@ impl Pod, St> { metrics_tracker: Arc::new(crate::compact::metrics_tracker::MetricsTracker::new()), usage_history: Arc::new(Mutex::new(Vec::new())), tracker: None, + task_store: tools::TaskStore::new(), system_prompt_template: common.system_prompt_template, alerter: None, event_tx: None, @@ -2040,6 +2077,7 @@ impl Pod, St> { metrics_tracker: Arc::new(crate::compact::metrics_tracker::MetricsTracker::new()), usage_history: Arc::new(Mutex::new(Vec::new())), tracker: None, + task_store: tools::TaskStore::new(), system_prompt_template: common.system_prompt_template, alerter: None, event_tx: None, @@ -2136,6 +2174,7 @@ impl Pod, St> { } let extract_pointer = memory::extract::fold_pointer(&state.extensions); + let task_store = tools::TaskStore::from_history(&state.history); let mut pod = Self { manifest, @@ -2152,6 +2191,7 @@ impl Pod, St> { metrics_tracker: Arc::new(crate::compact::metrics_tracker::MetricsTracker::new()), usage_history: Arc::new(Mutex::new(state.usage_history)), tracker: None, + task_store, // Restore replays the saved system_prompt verbatim — no // template re-render on resume. system_prompt_template: None, @@ -2247,7 +2287,11 @@ impl From for PodRunResult { /// Build the compact worker's input: default-reference instructions, /// the list of recently-touched files, and the pruned conversation /// produced by [`build_summary_prompt`]. -fn build_summary_input(items: &[Item], default_refs: &[PathBuf]) -> String { +fn build_summary_input( + items: &[Item], + default_refs: &[PathBuf], + task_snapshot: Option<&str>, +) -> String { let mut out = String::new(); out.push_str( "Summarise the conversation below into a structured summary and nominate \ @@ -2267,6 +2311,16 @@ fn build_summary_input(items: &[Item], default_refs: &[PathBuf]) -> String { } out.push('\n'); } + if let Some(task_snapshot) = task_snapshot { + out.push_str( + "## Current Session TaskStore\n\ + This is the full current task list. Use it as source material for the \ + summary, especially active (pending/inprogress) tasks, but do not edit tasks \ + from the compact worker.\n", + ); + out.push_str(task_snapshot); + out.push_str("\n\n"); + } out.push_str("## Conversation\n"); out.push_str(&build_summary_prompt(items)); out.push_str("\n\nWhen you are done, call `write_summary` with the final 5-section text."); diff --git a/crates/tools/src/lib.rs b/crates/tools/src/lib.rs index 28139410..f4af7a39 100644 --- a/crates/tools/src/lib.rs +++ b/crates/tools/src/lib.rs @@ -20,6 +20,7 @@ pub mod error; pub mod scoped_fs; +pub mod task; pub mod tracker; mod bash; @@ -36,6 +37,7 @@ pub use glob::glob_tool; pub use grep::grep_tool; pub use read::read_tool; pub use scoped_fs::ScopedFs; +pub use task::{TaskEntry, TaskSnapshot, TaskStatus, TaskStore, task_tools}; pub use tracker::Tracker; pub use write::write_tool; @@ -53,14 +55,17 @@ pub use write::write_tool; pub fn builtin_tools( fs: ScopedFs, tracker: Tracker, + task_store: TaskStore, bash_output_dir: std::path::PathBuf, ) -> Vec { - vec![ + let mut defs = vec![ read_tool(fs.clone(), tracker.clone()), write_tool(fs.clone(), tracker.clone()), edit_tool(fs.clone(), tracker), glob_tool(fs.clone()), grep_tool(fs.clone()), bash_tool(fs, bash_output_dir), - ] + ]; + defs.extend(task_tools(task_store)); + defs } diff --git a/crates/tools/src/task.rs b/crates/tools/src/task.rs new file mode 100644 index 00000000..9261d396 --- /dev/null +++ b/crates/tools/src/task.rs @@ -0,0 +1,612 @@ +//! Session-scoped TaskStore and builtin task tools. +//! +//! The store is Pod/session-lifetime state shared by the four Task* tools. It +//! is reconstructed on resume by replaying TaskCreate / TaskUpdate tool-call +//! arguments from persisted history. + +use std::collections::HashMap; +use std::fmt::Write as _; +use std::sync::{Arc, Mutex}; + +use async_trait::async_trait; +use llm_worker::Item; +use llm_worker::tool::{Tool, ToolDefinition, ToolError, ToolMeta, ToolOutput}; +use serde::{Deserialize, Serialize}; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, schemars::JsonSchema)] +#[serde(rename_all = "lowercase")] +pub enum TaskStatus { + Pending, + Inprogress, + Completed, + Deleted, +} + +impl std::fmt::Display for TaskStatus { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let s = match self { + Self::Pending => "pending", + Self::Inprogress => "inprogress", + Self::Completed => "completed", + Self::Deleted => "deleted", + }; + f.write_str(s) + } +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, schemars::JsonSchema)] +pub struct TaskEntry { + pub taskid: u64, + pub status: TaskStatus, + pub subject: String, + pub description: String, +} + +#[derive(Debug, Default)] +struct Inner { + next_taskid: u64, + tasks: Vec, +} + +#[derive(Debug, Clone, Default)] +pub struct TaskStore { + inner: Arc>, +} + +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize, schemars::JsonSchema)] +pub struct TaskSnapshot { + pub tasks: Vec, +} + +impl TaskStore { + pub fn new() -> Self { + Self { + inner: Arc::new(Mutex::new(Inner { + next_taskid: 1, + tasks: Vec::new(), + })), + } + } + + pub fn create(&self, subject: String, description: String) -> TaskEntry { + let mut inner = self.inner.lock().unwrap_or_else(|e| e.into_inner()); + let task = TaskEntry { + taskid: inner.next_taskid, + status: TaskStatus::Pending, + subject, + description, + }; + inner.next_taskid = inner.next_taskid.saturating_add(1); + inner.tasks.push(task.clone()); + task + } + + pub fn list(&self) -> Vec { + self.inner + .lock() + .unwrap_or_else(|e| e.into_inner()) + .tasks + .clone() + } + + pub fn get(&self, taskid: u64) -> Option { + self.inner + .lock() + .unwrap_or_else(|e| e.into_inner()) + .tasks + .iter() + .find(|t| t.taskid == taskid) + .cloned() + } + + pub fn update( + &self, + taskid: u64, + status: Option, + subject: Option, + description: Option, + ) -> Result { + if status.is_none() && subject.is_none() && description.is_none() { + return Err(TaskStoreError::NoFields); + } + let mut inner = self.inner.lock().unwrap_or_else(|e| e.into_inner()); + let task = inner + .tasks + .iter_mut() + .find(|t| t.taskid == taskid) + .ok_or(TaskStoreError::Missing(taskid))?; + if let Some(status) = status { + task.status = status; + } + if let Some(subject) = subject { + task.subject = subject; + } + if let Some(description) = description { + task.description = description; + } + Ok(task.clone()) + } + + pub fn snapshot(&self) -> TaskSnapshot { + TaskSnapshot { tasks: self.list() } + } + + pub fn replay_history(&self, history: &[Item]) { + for item in history { + match item { + Item::Message { content, .. } => { + for part in content { + let text = part.as_text(); + if let Some(snapshot) = parse_compact_snapshot_text(text) { + self.replace_with(snapshot); + } + } + } + Item::ToolCall { + name, arguments, .. + } => match name.as_str() { + "TaskCreate" => { + if let Ok(params) = serde_json::from_str::(arguments) { + let _ = self.create(params.subject, params.description); + } + } + "TaskUpdate" => { + if let Ok(params) = serde_json::from_str::(arguments) { + let _ = self.update( + params.taskid, + params.status, + params.subject, + params.description, + ); + } + } + _ => {} + }, + _ => {} + } + } + } + + pub fn replace_with(&self, tasks: Vec) { + let next_taskid = tasks + .iter() + .map(|t| t.taskid) + .max() + .unwrap_or(0) + .saturating_add(1) + .max(1); + let mut inner = self.inner.lock().unwrap_or_else(|e| e.into_inner()); + inner.tasks = tasks; + inner.next_taskid = next_taskid; + } + + pub fn from_history(history: &[Item]) -> Self { + let store = Self::new(); + store.replay_history(history); + store + } + + pub fn snapshot_text(&self) -> String { + render_snapshot(&self.list()) + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum TaskStoreError { + Missing(u64), + NoFields, +} + +impl std::fmt::Display for TaskStoreError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Missing(id) => write!(f, "taskid {id} not found"), + Self::NoFields => { + f.write_str("at least one of status, subject, description is required") + } + } + } +} + +impl std::error::Error for TaskStoreError {} + +#[derive(Debug, Deserialize, schemars::JsonSchema)] +struct TaskCreateParams { + /// One-line task subject. + subject: String, + /// Detailed task description. + description: String, +} + +#[derive(Debug, Deserialize, schemars::JsonSchema)] +struct TaskListParams {} + +#[derive(Debug, Deserialize, schemars::JsonSchema)] +struct TaskGetParams { + taskid: u64, +} + +#[derive(Debug, Deserialize, schemars::JsonSchema)] +struct TaskUpdateParams { + taskid: u64, + #[serde(default)] + status: Option, + #[serde(default)] + subject: Option, + #[serde(default)] + description: Option, +} + +struct TaskCreateTool { + store: TaskStore, +} + +struct TaskListTool { + store: TaskStore, +} + +struct TaskGetTool { + store: TaskStore, +} + +struct TaskUpdateTool { + store: TaskStore, +} + +const CREATE_DESCRIPTION: &str = "Create a session-lifetime task. Input only `subject` and \ +`description`; `taskid` is assigned automatically and initial `status` is `pending`."; +const LIST_DESCRIPTION: &str = "List every session-lifetime task, including completed and \ +deleted entries. Takes an empty object as input."; +const GET_DESCRIPTION: &str = "Get one session-lifetime task by `taskid`. Returns an error if \ +the task does not exist."; +const UPDATE_DESCRIPTION: &str = "Update an existing session-lifetime task. Provide `taskid` and \ +at least one of `status`, `subject`, or `description`. `status` must be one of `pending`, \ +`inprogress`, `completed`, or `deleted`; deletion is logical (`status = deleted`)."; + +#[async_trait] +impl Tool for TaskCreateTool { + async fn execute(&self, input_json: &str) -> Result { + let params: TaskCreateParams = serde_json::from_str(input_json) + .map_err(|e| ToolError::InvalidArgument(format!("invalid TaskCreate input: {e}")))?; + let created = self.store.create(params.subject, params.description); + let tasks = self.store.list(); + Ok(task_output( + format!( + "Created task {} ({})\n{}", + created.taskid, + created.status, + snapshot_overview(&tasks) + ), + &created, + &tasks, + )) + } +} + +#[async_trait] +impl Tool for TaskListTool { + async fn execute(&self, input_json: &str) -> Result { + let _: TaskListParams = serde_json::from_str(input_json) + .map_err(|e| ToolError::InvalidArgument(format!("invalid TaskList input: {e}")))?; + let tasks = self.store.list(); + Ok(ToolOutput { + summary: snapshot_overview(&tasks), + content: Some(render_snapshot(&tasks)), + }) + } +} + +#[async_trait] +impl Tool for TaskGetTool { + async fn execute(&self, input_json: &str) -> Result { + let params: TaskGetParams = serde_json::from_str(input_json) + .map_err(|e| ToolError::InvalidArgument(format!("invalid TaskGet input: {e}")))?; + let task = self.store.get(params.taskid).ok_or_else(|| { + ToolError::ExecutionFailed(format!("taskid {} not found", params.taskid)) + })?; + let content = serde_json::to_string_pretty(&task).unwrap_or_else(|_| format!("{task:?}")); + Ok(ToolOutput { + summary: format!("Task {} ({}) {}", task.taskid, task.status, task.subject), + content: Some(content), + }) + } +} + +#[async_trait] +impl Tool for TaskUpdateTool { + async fn execute(&self, input_json: &str) -> Result { + let params: TaskUpdateParams = serde_json::from_str(input_json) + .map_err(|e| ToolError::InvalidArgument(format!("invalid TaskUpdate input: {e}")))?; + let updated = self + .store + .update( + params.taskid, + params.status, + params.subject, + params.description, + ) + .map_err(|e| ToolError::ExecutionFailed(e.to_string()))?; + let tasks = self.store.list(); + Ok(task_output( + format!( + "Updated task {} ({})\n{}", + updated.taskid, + updated.status, + snapshot_overview(&tasks) + ), + &updated, + &tasks, + )) + } +} + +fn task_output(summary: String, task: &TaskEntry, tasks: &[TaskEntry]) -> ToolOutput { + let content = serde_json::json!({ + "task": task, + "snapshot": { "tasks": tasks }, + }); + ToolOutput { + summary, + content: Some(serde_json::to_string_pretty(&content).unwrap_or_default()), + } +} + +pub fn snapshot_overview(tasks: &[TaskEntry]) -> String { + let pending = tasks + .iter() + .filter(|t| t.status == TaskStatus::Pending) + .count(); + let inprogress = tasks + .iter() + .filter(|t| t.status == TaskStatus::Inprogress) + .count(); + let completed = tasks + .iter() + .filter(|t| t.status == TaskStatus::Completed) + .count(); + let deleted = tasks + .iter() + .filter(|t| t.status == TaskStatus::Deleted) + .count(); + format!( + "TaskStore: {} task(s) (pending: {pending}, inprogress: {inprogress}, completed: {completed}, deleted: {deleted})", + tasks.len() + ) +} + +pub fn render_snapshot(tasks: &[TaskEntry]) -> String { + if tasks.is_empty() { + return "TaskStore is empty.".to_string(); + } + let mut out = String::new(); + let _ = writeln!(&mut out, "{}", snapshot_overview(tasks)); + for task in tasks { + let _ = writeln!( + &mut out, + "\n- taskid: {}\n status: {}\n subject: {}\n description: {}", + task.taskid, task.status, task.subject, task.description + ); + } + out +} + +fn parse_compact_snapshot_text(text: &str) -> Option> { + if !text.starts_with("[Session TaskStore snapshot]") { + return None; + } + let body = text.split_once("\n\n")?.1; + parse_rendered_snapshot(body).or_else(|| { + if body.contains("TaskStore is empty.") { + Some(Vec::new()) + } else { + None + } + }) +} + +fn parse_rendered_snapshot(text: &str) -> Option> { + if text.contains("TaskStore is empty.") { + return Some(Vec::new()); + } + let mut tasks = Vec::new(); + let mut current: HashMap<&str, String> = HashMap::new(); + for line in text.lines() { + let line = line.trim_start(); + if let Some(value) = line.strip_prefix("- taskid: ") { + if !current.is_empty() { + tasks.push(task_from_fields(¤t)?); + current.clear(); + } + current.insert("taskid", value.to_string()); + } else if let Some(value) = line.strip_prefix("status: ") { + current.insert("status", value.to_string()); + } else if let Some(value) = line.strip_prefix("subject: ") { + current.insert("subject", value.to_string()); + } else if let Some(value) = line.strip_prefix("description: ") { + current.insert("description", value.to_string()); + } + } + if !current.is_empty() { + tasks.push(task_from_fields(¤t)?); + } + Some(tasks) +} + +fn task_from_fields(fields: &HashMap<&str, String>) -> Option { + let status = match fields.get("status")?.as_str() { + "pending" => TaskStatus::Pending, + "inprogress" => TaskStatus::Inprogress, + "completed" => TaskStatus::Completed, + "deleted" => TaskStatus::Deleted, + _ => return None, + }; + Some(TaskEntry { + taskid: fields.get("taskid")?.parse().ok()?, + status, + subject: fields.get("subject")?.clone(), + description: fields.get("description")?.clone(), + }) +} + +fn task_create_tool(store: TaskStore) -> ToolDefinition { + Arc::new(move || { + let schema = schemars::schema_for!(TaskCreateParams); + let schema_value = serde_json::to_value(schema).unwrap_or(serde_json::json!({})); + let meta = ToolMeta::new("TaskCreate") + .description(CREATE_DESCRIPTION) + .input_schema(schema_value); + let tool: Arc = Arc::new(TaskCreateTool { + store: store.clone(), + }); + (meta, tool) + }) +} + +fn task_list_tool(store: TaskStore) -> ToolDefinition { + Arc::new(move || { + let schema = schemars::schema_for!(TaskListParams); + let schema_value = serde_json::to_value(schema).unwrap_or(serde_json::json!({})); + let meta = ToolMeta::new("TaskList") + .description(LIST_DESCRIPTION) + .input_schema(schema_value); + let tool: Arc = Arc::new(TaskListTool { + store: store.clone(), + }); + (meta, tool) + }) +} + +fn task_get_tool(store: TaskStore) -> ToolDefinition { + Arc::new(move || { + let schema = schemars::schema_for!(TaskGetParams); + let schema_value = serde_json::to_value(schema).unwrap_or(serde_json::json!({})); + let meta = ToolMeta::new("TaskGet") + .description(GET_DESCRIPTION) + .input_schema(schema_value); + let tool: Arc = Arc::new(TaskGetTool { + store: store.clone(), + }); + (meta, tool) + }) +} + +fn task_update_tool(store: TaskStore) -> ToolDefinition { + Arc::new(move || { + let schema = schemars::schema_for!(TaskUpdateParams); + let schema_value = serde_json::to_value(schema).unwrap_or(serde_json::json!({})); + let meta = ToolMeta::new("TaskUpdate") + .description(UPDATE_DESCRIPTION) + .input_schema(schema_value); + let tool: Arc = Arc::new(TaskUpdateTool { + store: store.clone(), + }); + (meta, tool) + }) +} + +pub fn task_tools(store: TaskStore) -> Vec { + vec![ + task_create_tool(store.clone()), + task_list_tool(store.clone()), + task_get_tool(store.clone()), + task_update_tool(store), + ] +} + +#[cfg(test)] +mod tests { + use super::*; + + fn tool(def: ToolDefinition) -> Arc { + let (_, tool) = def(); + tool + } + + #[tokio::test] + async fn task_tools_create_list_get_update() { + let store = TaskStore::new(); + let create = tool(task_create_tool(store.clone())); + let list = tool(task_list_tool(store.clone())); + let get = tool(task_get_tool(store.clone())); + let update = tool(task_update_tool(store.clone())); + + let out = create + .execute(r#"{"subject":"implement","description":"write code"}"#) + .await + .unwrap(); + assert!(out.summary.contains("Created task 1")); + assert_eq!(store.get(1).unwrap().status, TaskStatus::Pending); + + let out = update + .execute(r#"{"taskid":1,"status":"inprogress","subject":"implement tasks"}"#) + .await + .unwrap(); + assert!(out.summary.contains("Updated task 1")); + let task = store.get(1).unwrap(); + assert_eq!(task.status, TaskStatus::Inprogress); + assert_eq!(task.subject, "implement tasks"); + + let out = get.execute(r#"{"taskid":1}"#).await.unwrap(); + assert!(out.summary.contains("Task 1 (inprogress)")); + assert!(out.content.unwrap().contains("implement tasks")); + + let out = list.execute("{}").await.unwrap(); + assert!(out.summary.contains("1 task(s)")); + assert!(out.content.unwrap().contains("taskid: 1")); + } + + #[tokio::test] + async fn task_update_validates_existing_and_at_least_one_field() { + let store = TaskStore::new(); + store.create("s".into(), "d".into()); + let update = tool(task_update_tool(store)); + + let err = update.execute(r#"{"taskid":1}"#).await.unwrap_err(); + assert!(err.to_string().contains("at least one")); + + let err = update + .execute(r#"{"taskid":99,"status":"deleted"}"#) + .await + .unwrap_err(); + assert!(err.to_string().contains("taskid 99 not found")); + } + + #[test] + fn replay_history_reconstructs_store_and_ignores_malformed_calls() { + let history = vec![ + Item::tool_call("c1", "TaskCreate", r#"{"subject":"a","description":"A"}"#), + Item::tool_call("bad", "TaskCreate", r#"{"subject":1}"#), + Item::tool_call("c2", "TaskCreate", r#"{"subject":"b","description":"B"}"#), + Item::tool_call("u1", "TaskUpdate", r#"{"taskid":2,"status":"completed"}"#), + Item::tool_call("bad2", "TaskUpdate", r#"{"taskid":99,"status":"deleted"}"#), + ]; + let store = TaskStore::from_history(&history); + let tasks = store.list(); + assert_eq!(tasks.len(), 2); + assert_eq!(tasks[0].taskid, 1); + assert_eq!(tasks[0].status, TaskStatus::Pending); + assert_eq!(tasks[1].taskid, 2); + assert_eq!(tasks[1].status, TaskStatus::Completed); + } + + #[test] + fn replay_history_uses_compact_snapshot_and_continues_updates() { + let snapshot = "[Session TaskStore snapshot]\n\nTaskStore: 1 task(s) (pending: 0, inprogress: 1, completed: 0, deleted: 0)\n\n- taskid: 1\n status: inprogress\n subject: kept\n description: from compact\n"; + let history = vec![ + Item::system_message(snapshot), + Item::tool_call("u1", "TaskUpdate", r#"{"taskid":1,"status":"completed"}"#), + Item::tool_call( + "c2", + "TaskCreate", + r#"{"subject":"new","description":"after compact"}"#, + ), + ]; + let store = TaskStore::from_history(&history); + let tasks = store.list(); + assert_eq!(tasks.len(), 2); + assert_eq!(tasks[0].taskid, 1); + assert_eq!(tasks[0].status, TaskStatus::Completed); + assert_eq!(tasks[1].taskid, 2); + assert_eq!(tasks[1].subject, "new"); + } +} diff --git a/crates/tools/src/tracker.rs b/crates/tools/src/tracker.rs index 36671ecd..2d4b6973 100644 --- a/crates/tools/src/tracker.rs +++ b/crates/tools/src/tracker.rs @@ -32,7 +32,8 @@ //! let fs = ScopedFs::new(scope, PathBuf::from("/workspace")); // pod lifetime //! let tracker = Tracker::new(); // session lifetime //! let bash_outputs = PathBuf::from("/run/insomnia/bash-output"); -//! let defs = builtin_tools(fs, tracker, bash_outputs); +//! let task_store = tools::TaskStore::new(); +//! let defs = builtin_tools(fs, tracker, task_store, bash_outputs); //! ``` use std::collections::{HashMap, VecDeque}; diff --git a/crates/tools/tests/edge_cases.rs b/crates/tools/tests/edge_cases.rs index 5f96fef7..0c9c70ac 100644 --- a/crates/tools/tests/edge_cases.rs +++ b/crates/tools/tests/edge_cases.rs @@ -6,7 +6,7 @@ use llm_worker::tool::{Tool, ToolDefinition}; use manifest::{Permission, Scope, ScopeConfig, ScopeRule}; use serde_json::json; use tempfile::TempDir; -use tools::{ScopedFs, Tracker, builtin_tools}; +use tools::{ScopedFs, TaskStore, Tracker, builtin_tools}; struct Registry { entries: Vec<(llm_worker::tool::ToolMeta, Arc)>, @@ -43,7 +43,12 @@ fn setup() -> (TempDir, TempDir, Registry) { let scope = Scope::from_config(&config).unwrap(); let fs = ScopedFs::new(scope, dir.path().to_path_buf()); let tracker = Tracker::new(); - let reg = Registry::new(builtin_tools(fs, tracker, spill.path().to_path_buf())); + let reg = Registry::new(builtin_tools( + fs, + tracker, + TaskStore::new(), + spill.path().to_path_buf(), + )); (dir, spill, reg) } diff --git a/crates/tools/tests/integration.rs b/crates/tools/tests/integration.rs index 328eea18..9ab3cb68 100644 --- a/crates/tools/tests/integration.rs +++ b/crates/tools/tests/integration.rs @@ -11,7 +11,7 @@ use llm_worker::tool::{Tool, ToolDefinition, ToolMeta}; use manifest::{Permission, Scope, ScopeConfig, ScopeRule}; use serde_json::json; use tempfile::TempDir; -use tools::{ScopedFs, Tracker, builtin_tools}; +use tools::{ScopedFs, TaskStore, Tracker, builtin_tools}; fn scope_with_spill(workspace: &Path, spill: &Path) -> Scope { let base = Scope::writable(workspace).unwrap(); @@ -56,7 +56,12 @@ fn setup() -> (TempDir, TempDir, Registry) { let scope = scope_with_spill(dir.path(), spill.path()); let fs = ScopedFs::new(scope, dir.path().to_path_buf()); let tracker = Tracker::new(); - let reg = Registry::new(builtin_tools(fs, tracker, spill.path().to_path_buf())); + let reg = Registry::new(builtin_tools( + fs, + tracker, + TaskStore::new(), + spill.path().to_path_buf(), + )); (dir, spill, reg) } @@ -77,7 +82,21 @@ fn builtin_tools_registers_full_set() { let (_dir, _spill, reg) = setup(); let mut names = reg.names(); names.sort(); - assert_eq!(names, vec!["Bash", "Edit", "Glob", "Grep", "Read", "Write"]); + assert_eq!( + names, + vec![ + "Bash", + "Edit", + "Glob", + "Grep", + "Read", + "TaskCreate", + "TaskGet", + "TaskList", + "TaskUpdate", + "Write" + ] + ); } #[test] @@ -270,16 +289,41 @@ async fn edit_requires_read_across_tools() { #[tokio::test] async fn deterministic_tool_order_is_registration_order() { let (_dir, _spill, reg) = setup(); - // Registration order from builtin_tools(): Read, Write, Edit, Glob, Grep, Bash + // Registration order from builtin_tools(): Read, Write, Edit, Glob, Grep, Bash, TaskCreate, TaskList, TaskGet, TaskUpdate let names: Vec<&str> = reg.entries.iter().map(|(m, _)| m.name.as_str()).collect(); - assert_eq!(names, vec!["Read", "Write", "Edit", "Glob", "Grep", "Bash"]); + assert_eq!( + names, + vec![ + "Read", + "Write", + "Edit", + "Glob", + "Grep", + "Bash", + "TaskCreate", + "TaskList", + "TaskGet", + "TaskUpdate" + ] + ); } // Regression: tool name capitalization matches Claude Code reference #[test] fn tool_names_match_reference_spec() { let (_dir, _spill, reg) = setup(); - for expected in ["Read", "Write", "Edit", "Glob", "Grep", "Bash"] { + for expected in [ + "Read", + "Write", + "Edit", + "Glob", + "Grep", + "Bash", + "TaskCreate", + "TaskList", + "TaskGet", + "TaskUpdate", + ] { assert!( reg.entries.iter().any(|(m, _)| m.name == expected), "missing tool {expected}" @@ -295,7 +339,12 @@ async fn tracker_recent_files_tracks_read_write_edit() { let scope = scope_with_spill(dir.path(), spill.path()); let fs = ScopedFs::new(scope, dir.path().to_path_buf()); let tracker = Tracker::new(); - let reg = Registry::new(builtin_tools(fs, tracker.clone(), spill.path().to_path_buf())); + let reg = Registry::new(builtin_tools( + fs, + tracker.clone(), + TaskStore::new(), + spill.path().to_path_buf(), + )); let a = dir.path().join("a.txt"); let b = dir.path().join("b.txt"); @@ -379,7 +428,10 @@ async fn bash_spilled_file_is_readable_via_read_tool() { let read_body = read_out.content.expect("Read returned content"); // The full 200 lines should be in the saved file even though Bash // returned only the tail of 80. - assert!(read_body.contains("line 1\n"), "missing line 1: {read_body}"); + assert!( + read_body.contains("line 1\n"), + "missing line 1: {read_body}" + ); assert!(read_body.contains("line 200"), "missing line 200"); } From ceafff92b67aa129f66ef141631dfeca63c9d2fc Mon Sep 17 00:00:00 2001 From: Hare Date: Sun, 3 May 2026 21:26:49 +0900 Subject: [PATCH 3/6] =?UTF-8?q?fix:=20TaskStore=20snapshot=20=E3=82=92=20c?= =?UTF-8?q?ompact=20=E5=BE=8C=20history=20=E3=81=AE=E6=9C=AB=E5=B0=BE?= =?UTF-8?q?=E3=81=AB=E7=BD=AE=E3=81=84=E3=81=A6=20retained=20=E4=B8=AD?= =?UTF-8?q?=E3=81=AE=20TaskCreate=20=E9=87=8D=E8=A4=87=E3=82=92=E9=98=B2?= =?UTF-8?q?=E3=81=90?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/pod/src/pod.rs | 15 ++++++++++----- crates/tools/src/task.rs | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/crates/pod/src/pod.rs b/crates/pod/src/pod.rs index 8791c304..c5d518a7 100644 --- a/crates/pod/src/pod.rs +++ b/crates/pod/src/pod.rs @@ -1395,7 +1395,12 @@ impl Pod { .filter(|i| i.is_user_message()) .count(); - // Build new history: [summary, ...auto-read, task snapshot, TaskList result, references, ...retained]. + // Build new history: [summary, ...auto-read, references, ...retained, task snapshot, TaskList synthetic call/result]. + // The TaskStore snapshot trails the retained items so that, on resume, + // `replay_history` walks any pre-compact Task* calls preserved verbatim + // in retained_items first and the trailing snapshot's `replace_with` + // is the final word — pre-compact `TaskCreate` calls cannot leak as + // duplicate entries. let mut new_history = Vec::with_capacity( 1 + auto_read_messages.len() + 3 @@ -1406,6 +1411,10 @@ impl Pod { "[Compacted context summary]\n\n{summary_text}" ))); new_history.extend(auto_read_messages); + if let Some(msg) = reference_message { + new_history.push(msg); + } + new_history.extend(retained_items); new_history.push(Item::system_message(format!( "[Session TaskStore snapshot]\n\n{task_snapshot_text}\n\n\ This is the complete session task list preserved across compaction. \ @@ -1417,10 +1426,6 @@ impl Pod { tools::task::snapshot_overview(&self.task_store.list()), task_snapshot_text.clone(), )); - if let Some(msg) = reference_message { - new_history.push(msg); - } - new_history.extend(retained_items); // Persist as a new compacted session. let old_session_id = self.session_id; diff --git a/crates/tools/src/task.rs b/crates/tools/src/task.rs index 9261d396..133cc051 100644 --- a/crates/tools/src/task.rs +++ b/crates/tools/src/task.rs @@ -609,4 +609,37 @@ mod tests { assert_eq!(tasks[1].taskid, 2); assert_eq!(tasks[1].subject, "new"); } + + #[test] + fn trailing_snapshot_supersedes_pre_compact_taskcreates_in_retained() { + // Mirrors the post-compact layout: pre-compact `TaskCreate` calls are + // preserved verbatim in retained_items, and the snapshot trails them. + // The trailing snapshot must reset the store to the captured state so + // pre-compact `TaskCreate`s do not surface as duplicates. + let snapshot = "[Session TaskStore snapshot]\n\nTaskStore: 2 task(s) (pending: 0, inprogress: 1, completed: 1, deleted: 0)\n\n- taskid: 1\n status: completed\n subject: A\n description: A-desc\n\n- taskid: 2\n status: inprogress\n subject: B\n description: B-desc\n"; + let history = vec![ + Item::tool_call("c1", "TaskCreate", r#"{"subject":"A","description":"A-desc"}"#), + Item::tool_call("u1", "TaskUpdate", r#"{"taskid":1,"status":"completed"}"#), + Item::tool_call("c2", "TaskCreate", r#"{"subject":"B","description":"B-desc"}"#), + Item::tool_call("u2", "TaskUpdate", r#"{"taskid":2,"status":"inprogress"}"#), + Item::system_message(snapshot), + Item::tool_call("compact-tasklist", "TaskList", "{}"), + Item::tool_call( + "c3", + "TaskCreate", + r#"{"subject":"C","description":"after compact"}"#, + ), + ]; + let store = TaskStore::from_history(&history); + let tasks = store.list(); + assert_eq!(tasks.len(), 3); + assert_eq!(tasks[0].taskid, 1); + assert_eq!(tasks[0].subject, "A"); + assert_eq!(tasks[0].status, TaskStatus::Completed); + assert_eq!(tasks[1].taskid, 2); + assert_eq!(tasks[1].subject, "B"); + assert_eq!(tasks[1].status, TaskStatus::Inprogress); + assert_eq!(tasks[2].taskid, 3); + assert_eq!(tasks[2].subject, "C"); + } } From 420f74edc668281e3f4c87e794ab99f09b5b3311 Mon Sep 17 00:00:00 2001 From: Hare Date: Sun, 3 May 2026 21:33:50 +0900 Subject: [PATCH 4/6] =?UTF-8?q?fix:=20TaskStore=20snapshot=20=E3=82=92=20J?= =?UTF-8?q?SON=20=E3=83=96=E3=83=AD=E3=83=83=E3=82=AF=E5=8C=96=20+=20?= =?UTF-8?q?=E6=A7=8B=E9=80=A0=E3=83=A9=E3=82=A6=E3=83=B3=E3=83=89=E3=83=88?= =?UTF-8?q?=E3=83=AA=E3=83=83=E3=83=97=E3=83=86=E3=82=B9=E3=83=88=E8=BF=BD?= =?UTF-8?q?=E5=8A=A0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/tools/src/task.rs | 187 ++++++++++++++++++++++++--------------- 1 file changed, 115 insertions(+), 72 deletions(-) diff --git a/crates/tools/src/task.rs b/crates/tools/src/task.rs index 133cc051..e683fe49 100644 --- a/crates/tools/src/task.rs +++ b/crates/tools/src/task.rs @@ -4,8 +4,6 @@ //! is reconstructed on resume by replaying TaskCreate / TaskUpdate tool-call //! arguments from persisted history. -use std::collections::HashMap; -use std::fmt::Write as _; use std::sync::{Arc, Mutex}; use async_trait::async_trait; @@ -375,77 +373,25 @@ pub fn snapshot_overview(tasks: &[TaskEntry]) -> String { } pub fn render_snapshot(tasks: &[TaskEntry]) -> String { - if tasks.is_empty() { - return "TaskStore is empty.".to_string(); - } - let mut out = String::new(); - let _ = writeln!(&mut out, "{}", snapshot_overview(tasks)); - for task in tasks { - let _ = writeln!( - &mut out, - "\n- taskid: {}\n status: {}\n subject: {}\n description: {}", - task.taskid, task.status, task.subject, task.description - ); - } - out + let snapshot = TaskSnapshot { + tasks: tasks.to_vec(), + }; + let json = serde_json::to_string_pretty(&snapshot) + .unwrap_or_else(|_| String::from("{\"tasks\":[]}")); + format!("{}\n\n```json\n{}\n```\n", snapshot_overview(tasks), json) } fn parse_compact_snapshot_text(text: &str) -> Option> { if !text.starts_with("[Session TaskStore snapshot]") { return None; } - let body = text.split_once("\n\n")?.1; - parse_rendered_snapshot(body).or_else(|| { - if body.contains("TaskStore is empty.") { - Some(Vec::new()) - } else { - None - } - }) -} - -fn parse_rendered_snapshot(text: &str) -> Option> { - if text.contains("TaskStore is empty.") { - return Some(Vec::new()); - } - let mut tasks = Vec::new(); - let mut current: HashMap<&str, String> = HashMap::new(); - for line in text.lines() { - let line = line.trim_start(); - if let Some(value) = line.strip_prefix("- taskid: ") { - if !current.is_empty() { - tasks.push(task_from_fields(¤t)?); - current.clear(); - } - current.insert("taskid", value.to_string()); - } else if let Some(value) = line.strip_prefix("status: ") { - current.insert("status", value.to_string()); - } else if let Some(value) = line.strip_prefix("subject: ") { - current.insert("subject", value.to_string()); - } else if let Some(value) = line.strip_prefix("description: ") { - current.insert("description", value.to_string()); - } - } - if !current.is_empty() { - tasks.push(task_from_fields(¤t)?); - } - Some(tasks) -} - -fn task_from_fields(fields: &HashMap<&str, String>) -> Option { - let status = match fields.get("status")?.as_str() { - "pending" => TaskStatus::Pending, - "inprogress" => TaskStatus::Inprogress, - "completed" => TaskStatus::Completed, - "deleted" => TaskStatus::Deleted, - _ => return None, - }; - Some(TaskEntry { - taskid: fields.get("taskid")?.parse().ok()?, - status, - subject: fields.get("subject")?.clone(), - description: fields.get("description")?.clone(), - }) + let start_marker = "```json\n"; + let end_marker = "\n```"; + let start = text.find(start_marker)? + start_marker.len(); + let rest = &text[start..]; + let end = rest.find(end_marker)?; + let snapshot: TaskSnapshot = serde_json::from_str(&rest[..end]).ok()?; + Some(snapshot.tasks) } fn task_create_tool(store: TaskStore) -> ToolDefinition { @@ -552,7 +498,9 @@ mod tests { let out = list.execute("{}").await.unwrap(); assert!(out.summary.contains("1 task(s)")); - assert!(out.content.unwrap().contains("taskid: 1")); + let content = out.content.unwrap(); + assert!(content.contains("\"taskid\": 1")); + assert!(content.contains("```json")); } #[tokio::test] @@ -589,11 +537,23 @@ mod tests { assert_eq!(tasks[1].status, TaskStatus::Completed); } + /// Wrap snapshot text the way `Pod::try_post_run_compact` does, so tests + /// exercise the exact format that goes through the session log. + fn wrap_snapshot_system_message(snapshot: &str) -> String { + format!( + "[Session TaskStore snapshot]\n\n{snapshot}\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." + ) + } + #[test] fn replay_history_uses_compact_snapshot_and_continues_updates() { - let snapshot = "[Session TaskStore snapshot]\n\nTaskStore: 1 task(s) (pending: 0, inprogress: 1, completed: 0, deleted: 0)\n\n- taskid: 1\n status: inprogress\n subject: kept\n description: from compact\n"; + let pre = TaskStore::new(); + pre.create("kept".into(), "from compact".into()); + pre.update(1, Some(TaskStatus::Inprogress), None, None).unwrap(); let history = vec![ - Item::system_message(snapshot), + Item::system_message(wrap_snapshot_system_message(&pre.snapshot_text())), Item::tool_call("u1", "TaskUpdate", r#"{"taskid":1,"status":"completed"}"#), Item::tool_call( "c2", @@ -616,13 +576,17 @@ mod tests { // preserved verbatim in retained_items, and the snapshot trails them. // The trailing snapshot must reset the store to the captured state so // pre-compact `TaskCreate`s do not surface as duplicates. - let snapshot = "[Session TaskStore snapshot]\n\nTaskStore: 2 task(s) (pending: 0, inprogress: 1, completed: 1, deleted: 0)\n\n- taskid: 1\n status: completed\n subject: A\n description: A-desc\n\n- taskid: 2\n status: inprogress\n subject: B\n description: B-desc\n"; + let pre = TaskStore::new(); + pre.create("A".into(), "A-desc".into()); + pre.update(1, Some(TaskStatus::Completed), None, None).unwrap(); + pre.create("B".into(), "B-desc".into()); + pre.update(2, Some(TaskStatus::Inprogress), None, None).unwrap(); let history = vec![ Item::tool_call("c1", "TaskCreate", r#"{"subject":"A","description":"A-desc"}"#), Item::tool_call("u1", "TaskUpdate", r#"{"taskid":1,"status":"completed"}"#), Item::tool_call("c2", "TaskCreate", r#"{"subject":"B","description":"B-desc"}"#), Item::tool_call("u2", "TaskUpdate", r#"{"taskid":2,"status":"inprogress"}"#), - Item::system_message(snapshot), + Item::system_message(wrap_snapshot_system_message(&pre.snapshot_text())), Item::tool_call("compact-tasklist", "TaskList", "{}"), Item::tool_call( "c3", @@ -642,4 +606,83 @@ mod tests { assert_eq!(tasks[2].taskid, 3); assert_eq!(tasks[2].subject, "C"); } + + #[test] + fn snapshot_round_trips_multiline_subject_and_description() { + // Subject / description with embedded newlines and shape-breaking + // characters must survive snapshot serialization unchanged. + let pre = TaskStore::new(); + pre.create( + "subject with\nembedded newline\n- bullet".into(), + "desc:\n status: not-actually-a-field\n ```code fence```".into(), + ); + pre.update(1, Some(TaskStatus::Inprogress), None, None).unwrap(); + + let history = vec![Item::system_message(wrap_snapshot_system_message( + &pre.snapshot_text(), + ))]; + let store = TaskStore::from_history(&history); + let tasks = store.list(); + assert_eq!(tasks.len(), 1); + assert_eq!(tasks[0].subject, "subject with\nembedded newline\n- bullet"); + assert_eq!( + tasks[0].description, + "desc:\n status: not-actually-a-field\n ```code fence```" + ); + assert_eq!(tasks[0].status, TaskStatus::Inprogress); + } + + #[test] + fn synthetic_compact_tasklist_pair_is_well_formed() { + // Mirrors `Pod::try_post_run_compact`'s synthetic insertion: + // a system snapshot message followed by a TaskList tool_call/tool_result + // pair sharing the `compact-tasklist` id. Verify the structural + // contract every provider request builder relies on (matched call_id, + // tool name, content recoverable to the same TaskStore state). + let pre = TaskStore::new(); + pre.create("plan".into(), "do A then B".into()); + + let snapshot_text = pre.snapshot_text(); + let system = Item::system_message(wrap_snapshot_system_message(&snapshot_text)); + let call = Item::tool_call("compact-tasklist", "TaskList", "{}"); + let result = Item::tool_result_with_content( + "compact-tasklist", + snapshot_overview(&pre.list()), + snapshot_text.clone(), + ); + + // The system message embeds a parseable snapshot. + let extracted = system + .as_text() + .and_then(parse_compact_snapshot_text) + .expect("system message should parse as snapshot"); + assert_eq!(extracted, pre.list()); + + // The synthetic call/result pair shares one call_id and carries the + // expected tool name + detailed content. + match (&call, &result) { + ( + Item::ToolCall { + call_id: c_id, + name, + .. + }, + Item::ToolResult { + call_id: r_id, + content, + .. + }, + ) => { + assert_eq!(c_id.as_str(), r_id.as_str()); + assert_eq!(c_id.as_str(), "compact-tasklist"); + assert_eq!(name, "TaskList"); + assert_eq!(content.as_deref(), Some(snapshot_text.as_str())); + } + other => panic!("unexpected synthetic pair shape: {other:?}"), + } + + // Replaying the full triple reconstructs the same TaskStore. + let store = TaskStore::from_history(&[system, call, result]); + assert_eq!(store.list(), pre.list()); + } } From 46390a9006fa0ef89d8a98db9a9bec8167ea1e9e Mon Sep 17 00:00:00 2001 From: Hare Date: Sun, 3 May 2026 21:34:54 +0900 Subject: [PATCH 5/6] =?UTF-8?q?docs(tickets):=20session-todo=20=E3=83=AC?= =?UTF-8?q?=E3=83=93=E3=83=A5=E3=83=BC=E5=8F=8D=E6=98=A0=20(Approve)=20+?= =?UTF-8?q?=20reminder=20spec=20=E6=AE=B5=E9=9A=8E=E3=83=AC=E3=83=93?= =?UTF-8?q?=E3=83=A5=E3=83=BC?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tickets/session-todo-reminder.md | 7 ++ tickets/session-todo-reminder.review.md | 41 ++++++++++++ tickets/session-todo.md | 6 ++ tickets/session-todo.review.md | 87 +++++++++++++++++++++++++ 4 files changed, 141 insertions(+) create mode 100644 tickets/session-todo-reminder.review.md create mode 100644 tickets/session-todo.review.md diff --git a/tickets/session-todo-reminder.md b/tickets/session-todo-reminder.md index d0a1261a..1847cb4f 100644 --- a/tickets/session-todo-reminder.md +++ b/tickets/session-todo-reminder.md @@ -58,3 +58,10 @@ Insomnia でも同方針を採り、active Task が残っているのに `TaskCr - 設計指針: `CLAUDE.md`（最小の構造化 / 概念の追加は不在が問題になってから） - 前提: `tickets/session-todo.md`（Tool 群と TaskStore） - 参考実装: Claude Code の `task_reminder`（`du_` / `cu_` 関数、閾値 `Z_8 = { TURNS_SINCE_WRITE: 10, TURNS_BETWEEN_REMINDERS: 10 }`） + +## Review + +- 状態: Approve (spec 段階) +- レビュー詳細: [./session-todo-reminder.review.md](./session-todo-reminder.review.md) +- 日付: 2026-05-03 +- 補足: 実装着手前に Non-blocking で挙げた 4 点 (TaskCreate/Update のカウンタリセット契機 / active 取得経路 / reminder 本文 fmt / resume 時のカウンタ扱い) を spec に追記してから実装するとブレが減る。実装完了時に完了条件を再確認。 diff --git a/tickets/session-todo-reminder.review.md b/tickets/session-todo-reminder.review.md new file mode 100644 index 00000000..15cd447c --- /dev/null +++ b/tickets/session-todo-reminder.review.md @@ -0,0 +1,41 @@ +# Review: セッション内 Task ツールの注意機構 + +対象: `tickets/session-todo-reminder.md` (`28fe1da` で新規作成)。実装は未着手 (本レビュー時点でコード変更なし、worktree 内 grep で `task_reminder` / `requests_since_last_task` 等いずれもヒットせず)。 + +本レビューは spec 単独レビュー (実装が始まった時点で完了条件を再確認)。 + +## 前提・要件の確認 (spec の妥当性) + +- 「`pre_llm_request` で `Vec` を受けて直近 user message に `` を append する」は、`crates/pod/src/ipc/interceptor.rs` の既存 `pre_llm_request` レーンに自然に乗る。Worker history を変更せず、リクエスト送信用 `Vec` のみ加工する方針は、既存の `pending_notifies.drain()` と同じパターンで先例あり (`interceptor.rs:147-159`)。spec として実装可能性は高い。 +- TaskStore からの active 件数取得には A の `TaskStore::list()` だけで足り、追加 API 不要。 +- カウンタ (`requests_since_last_task_management` / `requests_since_last_reminder`) を Pod の session-lifetime 状態として持つ方針は、既存 `tool_calls_this_turn: AtomicUsize` (`interceptor.rs`) と同じ素地に追加できる。pre_tool_call で `TaskCreate` / `TaskUpdate` を観測したら片方をリセット、`pre_llm_request` で発火条件を見て両方を増やす、という骨格になる想定。spec の粒度は問題ない。 +- メインスレッド限定 / sub-agent 除外: 主要 worker と spawn pod / compact worker を分離している現実装では「メインの `PodInterceptor` だけがカウンタを持つ」で達成できる。compact worker 側は別 Worker / 別 Interceptor。spec の前提は現実装と一致。 +- resume 時のカウンタ復元: 「履歴の逆走査で再計算 or 0 リセット」のどちらでも実害無しと spec 側で割り切られているのは妥当。0 リセットの方が大幅にシンプルで、初回ナッジが最大 N 遅れるだけ。実装時はこちらを推奨したい (spec 側は両許容で問題なし)。 +- 「揮発的 = 履歴を汚さない」「タグ形式 `...` で揃える」「2 件目の利用者が出た時に汎用化を検討」の 3 点を明文化していて、A と B の隙間に「汎用 reminder 機構」の中途半端な抽象が立たないようにしている。CLAUDE.md の「概念の追加は不在が問題になってから」と整合。 + +## アーキテクチャ・スコープ + +- B は A に対する後続チケットとして適切に分離されている。前提依存 (TaskStore + Task* tools) を `## 前提` に明記しており、A 完了後に着手するという順序付けも自然。 +- 範囲外 (inprogress 滞留 / 多重 inprogress / 注入機構汎用化 / Tool 戻り値の active 全件埋め込み) が列挙されており、本チケットで肥大化しないラインが引けている。 +- 「Pod 側に session-lifetime カウンタを増やす」程度の侵襲で、既存の `crates/pod/src/ipc/interceptor.rs` 内に追加実装が収まる規模感。クレート構成や層境界に新しい歪みは生じない見込み。 +- 「N=8 / M=8 暫定」と数値を明示しているのは判断しやすい。Claude Code が `Z_8 = { TURNS_SINCE_WRITE: 10, TURNS_BETWEEN_REMINDERS: 10 }` という参照値を持つことも書かれており、後で詰めやすい。 + +## 指摘事項 + +### Non-blocking / Follow-up (spec 段階の論点) + +- **「リクエスト = LLM への 1 回の推論呼び出し」のカウント単位が、現 `Interceptor` の `pre_llm_request` 1 呼び出しと一致するか確認が必要。** 現実装の `pre_llm_request` は context 構築直前に必ず通るレーンで、tool ループ内の続行 LLM 呼び出しでも毎回呼ばれる。実装時に「pre_llm_request 突入時刻 = LLM へのリクエスト 1 件」が成立しているかを軽く検証するテストを1本入れてほしい (tool ループ中の発火 cadence のため)。 +- **TaskCreate / TaskUpdate のカウンタリセット契機。** 現状 spec は「呼ばれていない」と書かれているのみ。実装ではどのフックで観測するかの選択 (`pre_tool_call` か `post_tool_call` か) を決めて spec に追記しておくと実装ブレが減る。`pre_tool_call` で名前判定 + リセット、で十分。 +- **active Task の取り出し経路。** spec 上は明示されていないが、Interceptor が TaskStore handle を保持する形になる (Pod 経由で渡す)。`PodInterceptor::new` 等のコンストラクタに `Option` を増やす想定で良いと思うが、実装時に `Tracker` の例 (`attach_tracker`) と同様、Pod から Interceptor への手渡しを統一してほしい。 +- **resume 時の発火タイミング。** 0 リセットを採るなら、resume 直後の最初のリクエストでは出ない。これは spec 上「最大 N 遅れるだけ」と許容済み。実装時は単にこの選択を README/コメントに残してほしい。 +- **`` の本文。** spec は「taskid / status / subject を簡潔に列挙」と書かれているが、A 側の `render_snapshot` を流用するか、reminder 専用に別関数を切るかが未決定。短さを優先するなら専用 fmt が望ましい (description を必ず落とすため)。実装方針を spec に1行追記しておくと安全。 +- **テスト範囲。** spec の完了条件にある「リクエスト回数閾値 / active 0件 / cooldown / サブエージェント除外」は単体テストでカバー可能。実装時はそれぞれ独立ケースで書いてほしい。 + +### Nits + +- 数値 N / M が「暫定 N=8」「暫定 M=8」と明記されているが、変更可能性についてのフォローアップ条項 (例: 運用後に閾値を見直す) があると親切。 +- Claude Code 側の参照値 `Z_8 = { TURNS_SINCE_WRITE: 10, TURNS_BETWEEN_REMINDERS: 10 }` は spec に書かれているので、Insomnia 側 8 を採る理由 (短めにして気付かせやすくする等) を一文添えると将来 tuning しやすい。 + +## 判断 + +**Approve (spec 段階)** — 前提・要件・範囲外の切り分けが妥当で、A の実装規約 (TaskStore handle、揮発的注入、タグ形式) に乗る形で実装可能。アーキテクチャ的歪みも生じない見込み。実装着手前に上記 Non-blocking 4 点 (カウンタリセット契機 / active 取得経路 / 本文 fmt / resume 時のリセット選択) を spec に1〜2行追記してから実装に入ると、実装中の判断ブレが減る。実装完了時に完了条件を再確認する必要がある (本レビューは spec 妥当性のみの判定)。 diff --git a/tickets/session-todo.md b/tickets/session-todo.md index 3dc27291..ee187cbf 100644 --- a/tickets/session-todo.md +++ b/tickets/session-todo.md @@ -103,3 +103,9 @@ - 参考実装: Claude Code の TodoWrite、OpenCode の todo tool - 関連: `crates/tools/src/tracker.rs`（session-lifetime 状態の前例）、`crates/pod/src/compact/worker.rs`（auto-injection レーン） - 後続: `tickets/session-todo-reminder.md`（注意機構） + +## Review + +- 状態: Approve (再レビュー済み、blocking 解消・non-blocking 対応反映) +- レビュー詳細: [./session-todo.review.md](./session-todo.review.md) +- 日付: 2026-05-03 diff --git a/tickets/session-todo.review.md b/tickets/session-todo.review.md new file mode 100644 index 00000000..9b447de4 --- /dev/null +++ b/tickets/session-todo.review.md @@ -0,0 +1,87 @@ +# Review: セッション内 Task ツール + +対象: `tickets/session-todo.md` (`28fe1da` で再分割) / 実装 `6f2aca8`。 + +## 前提・要件の確認 + +- TaskCreate / TaskList / TaskGet / TaskUpdate を builtin tool として登録: 充足。`crates/tools/src/task.rs:507` の `task_tools(store)` を `crates/tools/src/lib.rs:55` の `builtin_tools` が拡張し、Controller 経由で Worker に登録される (`crates/pod/src/controller.rs:268`)。 +- TaskCreate の自動採番 + 初期 status pending: 充足 (`crates/tools/src/task.rs:71`)。 +- TaskUpdate の最低1フィールド + 既存チェック + 物理削除なし (`status=deleted`): 充足 (`crates/tools/src/task.rs:108`、テスト `task_update_validates_existing_and_at_least_one_field`)。 +- TaskList が completed / deleted を含めて全件返す: 充足 (`crates/tools/src/task.rs:84`)。 +- TaskGet が `taskid` 不在でエラー: 充足 (`crates/tools/src/task.rs:304`)。 +- Resume 時の履歴 replay で TaskCreate / TaskUpdate を順に再適用: 実装あり (`replay_history`、`Pod::from_state` で `TaskStore::from_history(&state.history)`)。**ただし compact 後の resume で重複が発生する** (後述「Blocking」)。 +- `tool_call.arguments` の schema 乖離は無視: 充足 (`from_str` 失敗時 `_` でスキップ)。 +- compact 起動時、TaskStore 全スナップショットを compact worker context に渡す: 充足 (`build_summary_input` の `task_snapshot` 引数、`crates/pod/src/pod.rs:1278`)。 +- compact worker に Task 編集権限を与えない: 充足 (compact 用 `summary_worker.register_tool` は read / mark_read_required / add_reference / write_summary のみ。Task* tool は注入されていない)。 +- compact 後の新セッションで TaskStore handle を維持し全件保持: 充足 (`self.task_store` は compact 中に置換されない。Worker の history のみ入れ替え)。 +- compact 後の新セッション冒頭に snapshot system message + TaskList 発火: 充足 (`crates/pod/src/pod.rs:1409-1419`)。 +- 専用 LogEntry / Persistence 型を追加しない: 充足 (`replay_history` と inline snapshot text に閉じている)。 +- 単体テスト: 4 ツール基本 / バリデーション / replay / compact snapshot replay の4本。**retained tail に Task* call が残るシナリオの replay テストが欠落** (後述)。 + +## アーキテクチャ・スコープ + +- `tools` クレートの session-lifetime 状態として TaskStore を置き、Pod が handle を所有して `attach_task_store` で受け渡す方針は `Tracker` の前例と整合。レイヤー越境なし。 +- 4 ツールを `crates/tools/src/task.rs` の単一モジュールにまとめている点は、現時点の規模 (~600 行) と将来の機能追加余地を考えると許容範囲。`pod.rs` への impl 流入もない。 +- ツール本体は `tools` 層、配線と compact 連携は `pod` 層、と責務分離が保たれている。 +- TaskStore は `Arc>` の thin wrapper で、永続化を tool_call replay に寄せる方針 (専用レーン無し) もチケットの要件どおり。 +- 命名: `TaskStore` / `TaskEntry` / `TaskStatus` / `TaskSnapshot` の対称性も良。`insomnia-` prefix は無し (規約準拠)。 +- `task_store: tools::TaskStore` を pub field 露出ではなく `attach_task_store` / `task_store()` の getter で扱っており、これも `Tracker` の前例と整合。 +- compact 時の "synthetic tool_call" (`compact-tasklist` の TaskList call/result) は新規パターン。Pod 配下で Worker 履歴に注入される唯一のラインであり、再現性とプロバイダ受理性が運用上の検証対象になる (フォローアップ参照)。 + +## 指摘事項 + +### Blocking + +- **compact 跨ぎの resume で TaskStore が重複/二重適用される。** `crates/pod/src/pod.rs:1398-1423` で構築する post-compact history は `[summary, ...auto-read, snapshot system message, TaskList synthetic call/result, references, ...retained_items]` の順だが、`retained_items` は compact 直前まで生きていた末尾スライスをそのまま含むため、retained 内に `TaskCreate` / `TaskUpdate` が含まれた場合、resume 時の `TaskStore::from_history` が: + 1. 先頭付近の `[Session TaskStore snapshot]` を読み `replace_with` で全件復元 + 2. 後続の retained `TaskCreate` を再適用 → 同内容の重複 entry が新規 `taskid` で生える + 3. retained `TaskUpdate` も再適用 (こちらは冪等で害は薄いが意味は揺れる) + + になる。compact は token threshold 到達で起きるので、直近に Task* を呼んだケース (= 実際の主用途) で確実に発火する。 + + 既存テスト `replay_history_uses_compact_snapshot_and_continues_updates` (`crates/tools/src/task.rs:592`) は snapshot を history[0] に置き、後続を「compact 後の新規操作」として書いているため、本ケース (snapshot より後の retained_items に旧 Task* call が残る) を踏んでいない。 + + 対策案: + - (A) `pod.rs:1423` の `new_history.extend(retained_items)` の前に retained_items から `Item::ToolCall { name == "TaskCreate" | "TaskUpdate" .. }` と対応する `ToolResult` を除去する。snapshot + TaskList 再発火がポストコンパクトの唯一の真実になる + - (B) `replay_history` 側に anchor を入れ、snapshot を見たら**それ以前の** Task* call を捨てる + snapshot 以降は通常 replay。retained_items が snapshot より後ろにある現レイアウトでは効かないので、history 上の snapshot の位置を末尾 (retained_items の後) に移すなどレイアウト変更が必要 + - (C) compact 後 history 構築時に、snapshot の代わりに「TaskStore を空にしてから個別 TaskCreate で再現する」方針に切り替える (= snapshot の役割を tool_call 列で担う)。ただし冗長なので非推奨 + + 推奨は (A)。あわせて回帰テストとして「retained_items に旧 TaskCreate を含む post-compact history を `from_history` に通し、duplicate が出ないこと」のケースを追加してほしい。 + +### Non-blocking / Follow-up + +- **snapshot text のラウンドトリップが lossy。** `render_snapshot` は `subject` / `description` を1行 inline で書き出すので、本文に改行が含まれると `parse_rendered_snapshot` が後続行を読み落とし、subject/description の途中で切れる (`crates/tools/src/task.rs:377-433`)。`description` は LLM が複数行で書く可能性が現実的なので、snapshot レンダリングを JSON ブロック (例: コードフェンスで `{"tasks":[...]}`) に切り替えるか、`description` に YAML literal block (`|`) を使うなどでロスレス化したい。replay の信頼性に直結する。 +- **compact 由来の synthetic ToolCall の provider 受理性。** `Item::tool_call("compact-tasklist", "TaskList", "{}")` + 対応する `ToolResult` が、対応する assistant turn なしに history 中に挿入される (`crates/pod/src/pod.rs:1414-1419`)。Anthropic / OpenAI / Codex 各 client がこの形を受理して次リクエストを通すか、E2E で1度通しておきたい。フォーマット・ID プレフィックス的に拒否されるリスクは低いが、未検証ライン。 +- **`PodInterceptor::pre_llm_request` の `&& !state.just_compacted()` 追加 (`crates/pod/src/ipc/interceptor.rs:130`) はチケット範囲外。** 内容自体は `try_post_run_compact` (`pod.rs:1079`) と同じガードを request-side にも回す mirror で、compact 直後の閾値再ヒットによる thrash 防止として妥当。とはいえ「session-todo」のスコープではないので、本来は `compact-thrash-fix` のような別チケットに切り出すか、ticket 本文に「副次修正」として明記するのが望ましい。今回はそのまま採用するなら commit message と `## 範囲外` の補記で痕跡を残してもらえると後追いが楽。 +- snapshot system message に「This is the complete session task list ...」と英文で説明が入っているが、TaskList 用の synthetic tool_call の summary には説明なし。LLM 体験としては snapshot system message → TaskList result の二段で同じ情報が出るので、片方に寄せて簡潔化しても良い (任意)。 + +### Nits + +- `crates/pod/src/pod.rs:1398` のコメント: `[summary, ...auto-read, task snapshot, TaskList result, references, ...retained]` だが実際は `task snapshot` の後ろに `TaskList tool_call` と `tool_result` の **2 件** が入る。3 件であることをコメントに反映。 +- `task.rs:79` `inner.next_taskid.saturating_add(1)`: u64 で saturating は事実上不要だが、害もない。 +- `lib.rs` の `pub use task::{TaskEntry, TaskSnapshot, TaskStatus, TaskStore, task_tools};` で `task::snapshot_overview` だけ `pod.rs` から `tools::task::snapshot_overview(...)` のフルパスで参照されている。`tools::snapshot_overview` 等で同レベル re-export しておくと API surface が揃う (任意)。 + +## 判断 + +**Request changes** — Resume 後の TaskStore 重複は要件「セッションを resume すると、履歴中の TaskCreate / TaskUpdate replay により TaskStore が復元される」「compact を跨いでも、TaskStore 全件が維持され」の両方に直接抵触する正解性 issue。Blocking 1点 (retained_items 二重適用) を直したうえで回帰テストを追加して再提出してほしい。アーキテクチャ・スコープ・残りの要件カバーは概ね健全で、`簡素な` 設計を保てている。 + +## 再レビュー (commit `420f74e` まで反映) + +判断: **Approve**。 + +### 対応状況 + +- **Blocking (retained 二重適用)**: 解消。`ceafff9` で post-compact history のレイアウトを `[summary, auto-read, references, ...retained, snapshot, TaskList synthetic]` に変更。snapshot を末尾に置き `replace_with` が常に最後勝ちにする設計に切り替えた。回帰テスト `trailing_snapshot_supersedes_pre_compact_taskcreates_in_retained` で「retained に pre-compact TaskCreate × 2 + TaskUpdate × 2 → snapshot → post-compact TaskCreate × 1」のシナリオが正しく 3 件で収束することを確認。 +- **Non-blocking #1 (snapshot text の lossy round-trip)**: 解消。`420f74e` で `render_snapshot` を「overview 一行 + ` ```json` フェンス内 JSON」形式に変更。`parse_compact_snapshot_text` も JSON フェンスから serde で復元する形に再実装。`snapshot_round_trips_multiline_subject_and_description` テストで改行入り subject/description + 紛らわしい `status:` プレフィックス + バッククォートが lossless で復元することを確認。`HashMap`-based 行パーサと `parse_rendered_snapshot` / `task_from_fields` ヘルパは削除。 +- **Non-blocking #2 (synthetic compact-tasklist の provider 受理性)**: 構造テストで部分対応。`synthetic_compact_tasklist_pair_is_well_formed` テストで「snapshot system message → TaskList tool_call → ToolResult」3 連の構造契約 (matched call_id、tool name、content recoverability) を確認。実プロセス E2E はプロジェクト全体で未設計 (CLAUDE.md 「E2E は未設計」) のためここでは構造的健全性までしか確認できない。実 provider 通過は将来の E2E 設計時に再確認する follow-up。 +- **Non-blocking #3 (`interceptor.rs:130` 範囲外修正)**: 範囲外注記として残す方針で確定。`!state.just_compacted()` ガードは `try_post_run_compact` の既存ガードを request-side に mirror する妥当な小修正で、本来は別チケットだが現規模では note 扱いとする (reviewer 提示の 2択のうち「範囲外注記」を採用)。今後 compact thrash 周りで関連修正が必要になった場合に別チケットへ切り出すフォローアップ余地は残る。 + +### Nit (`pod.rs:1398` コメントの 3 件挿入の不一致) + +レイアウト変更に伴い同じく更新済み (`ceafff9`)。新コメント: `[summary, ...auto-read, references, ...retained, task snapshot, TaskList synthetic call/result]` + 末尾配置の理由を明記。 + +### 残る follow-up + +- 改行入り subject/description のロスレス化は完了したが、TaskList が tool result として LLM に返す JSON フェンスのトークン効率は将来の課題 (大量 task 時) +- synthetic `compact-tasklist` の provider 受理性 E2E (上述、プロジェクト E2E 設計の中で扱う) +- `interceptor.rs` の compact thrash guard は compact 周辺の他 issue が表面化した際に専用チケット化を検討 From 46208a3b45e536fa9b62da38c56cfe0193575c45 Mon Sep 17 00:00:00 2001 From: Hare Date: Sun, 3 May 2026 21:48:44 +0900 Subject: [PATCH 6/6] =?UTF-8?q?docs(tickets):=20session-todo=20(=E6=9C=AC?= =?UTF-8?q?=E4=BD=93)=20=E5=AE=8C=E4=BA=86?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- TODO.md | 4 +- tickets/session-todo.md | 111 --------------------------------- tickets/session-todo.review.md | 87 -------------------------- 3 files changed, 1 insertion(+), 201 deletions(-) delete mode 100644 tickets/session-todo.md delete mode 100644 tickets/session-todo.review.md diff --git a/TODO.md b/TODO.md index e7e49967..2e61dd8e 100644 --- a/TODO.md +++ b/TODO.md @@ -15,8 +15,6 @@ - Manifest: Tool Output / File Upload 上限の分離とデフォルト緩和 → [tickets/manifest-output-upload-limits.md](tickets/manifest-output-upload-limits.md) - メモリ機構 - 使用頻度メトリクス + Knowledge 化候補レポート → [tickets/memory-usage-metrics.md](tickets/memory-usage-metrics.md) -- セッション内 Task ツール - - ツール本体（TaskCreate / TaskUpdate / TaskList / TaskGet + 永続化 / compact 跨ぎ） → [tickets/session-todo.md](tickets/session-todo.md) - - 注意機構（無アクティビティで `` ナッジ） → [tickets/session-todo-reminder.md](tickets/session-todo-reminder.md) +- セッション内 Task ツールの注意機構（無アクティビティで `` ナッジ） → [tickets/session-todo-reminder.md](tickets/session-todo-reminder.md) - ワークスペースのメモリーをLintするヘッドレスCLI - system-reminder 注入機構の汎用化（2件目の利用者が出た時に検討。タグ形式と「履歴を汚さない」原則は session-todo で先行確立） diff --git a/tickets/session-todo.md b/tickets/session-todo.md deleted file mode 100644 index ee187cbf..00000000 --- a/tickets/session-todo.md +++ /dev/null @@ -1,111 +0,0 @@ -# セッション内 Task ツール - -## 背景 - -長めのタスクを LLM に進めさせる際、Claude Code / OpenCode が備える「セッション内 TODO / Task リスト」相当の機構が無いため、エージェントが自分の作業計画を構造化された形で保持・更新できない。Reasoning や text 出力の中で擬似的に TODO を書くことはできるが、 - -- compact を跨ぐと完全に消える -- ツール結果ではないため、状態の作成・更新・削除の規約が決まらず、意図と乖離した「やったつもり」を起こしやすい -- 安定した識別子が無いため、後から特定 entry を更新できない - -この用途のために、セッション内に正規化された Task リストを保持し、compact を跨いで保存される専用ツールを導入する。 - -「LLM がツールを使わなくなった場合のサボり防止」を目的とした注意機構（`` の自動注入）は本チケットの範囲外。`tickets/session-todo-reminder.md` で別途扱う。 - -## 方針 - -- **保存先は `tools` 層の session-lifetime 状態**。`Tracker` と同じ生存スコープで `Pod` が所有。`Arc>` ベースの store を tool に注入する -- **Task は差分操作**。全置換ではなく `TaskCreate` / `TaskUpdate` によって TaskStore を逐次更新する -- **永続化は専用レーンを持たない**。`tool_call.arguments` がセッションログに既に乗っているため、resume 時には履歴 replay の中で `TaskCreate` / `TaskUpdate` を順に再適用すれば状態が復元される - -## Task データモデル - -各 Task entry は以下の4フィールドを持つ。 - -- `taskid`: store が自動採番する整数 ID。`TaskCreate` ごとに単調増加する -- `status`: `pending | inprogress | completed | deleted` -- `subject`: 1行程度の短い件名 -- `description`: 詳細説明 - -状態遷移の通常ルートは `pending -> inprogress -> completed` とする。完了ではなく一覧から取り下げたい場合は、物理削除ではなく `TaskUpdate` で `status = deleted` を設定する。 - -## 要件 - -### `TaskCreate` ツール - -- 入力は `subject` / `description` のみ -- TaskStore の末尾に新規 entry を push する -- `taskid` は TaskStore が自動で割り当てる。LLM は指定できない -- 初期 `status` は `pending` -- 戻り値は作成された Task と、更新後の TaskStore スナップショット概要を summary に含める - -### `TaskList` ツール - -- 入力なし -- 現在の TaskStore スナップショットを返す -- `taskid` / `status` / `subject` / `description` を含める -- `completed` / `deleted` も含め、store に残っている Task をすべて列挙する - -### `TaskGet` ツール - -- 入力は `taskid` -- 指定された Task を1件返す -- 存在しない `taskid` はエラーにする - -### `TaskUpdate` ツール - -- 入力は `taskid` と、更新する `status` / `subject` / `description` のいずれか -- 少なくとも1フィールドは更新指定が必要 -- `taskid` は変更できない -- `status` は `pending | inprogress | completed | deleted` のいずれか -- 存在しない `taskid` はエラーにする -- 削除は `status = deleted` への更新として表現し、entry は store から取り除かない -- 戻り値は更新された Task と、更新後の TaskStore スナップショット概要を summary に含める - -### Resume 時の復元 - -- `Pod::resume` / restore の履歴 replay 中に `TaskCreate` / `TaskUpdate` の `tool_call.arguments` を観測したら、TaskStore に順に再適用する -- `TaskCreate` は replay 時も自動採番を行う。ログ上の call 順序が保持されるため、通常の replay では元の `taskid` が再現される -- `TaskUpdate` は replay 済みの TaskStore に対して適用する -- 専用 LogEntry / Persistence 型は追加しない（`Tracker` と同じ方針） -- `tool_call.arguments` のフォーマットが現在の schema と乖離した場合（旧バージョンのログ）は、その call を無視してよい - -### Compact 跨ぎ - -- compact 起動時、Pod は現在の TaskStore **全スナップショット**を compact worker context に渡す -- compact worker は summary を書く際、TaskStore を参照できる。未完了 Task を summary 文に取り込める情報源として参照する（強制ではない） -- compact worker に Task 編集権限は与えない（作成・更新・削除はしない） -- compact 後の新セッションでも TaskStore は空にせず、compact 前の TaskStore **全件**を維持する。`completed` / `deleted` も含めて保持する -- compact 後の新セッション開始時、Pod は **`mark_read_required` と同じ system message 注入レーン**に TaskStore 全スナップショットを1メッセージとして注入する -- compact 完了後、Pod は新セッションの継続前に必ず `TaskList` を発火させ、LLM が compact 後の TaskStore 状態を tool result として再確認できるようにする - -## 完了条件 - -- `TaskCreate` / `TaskList` / `TaskGet` / `TaskUpdate` が builtin tool として登録され、Pod で利用できる -- LLM が `TaskCreate` を呼ぶと TaskStore に pending Task が末尾追加され、自動採番された `taskid` が返る -- LLM が `TaskUpdate` を呼ぶと既存 Task の `status` / `subject` / `description` が更新される -- `TaskList` / `TaskGet` で TaskStore の状態を確認できる -- セッションを resume すると、履歴中の `TaskCreate` / `TaskUpdate` replay により TaskStore が復元される -- compact を跨いでも、TaskStore 全件が維持され、新セッション冒頭の system message と `TaskList` 発火により再提示される -- 単体テストで TaskCreate / TaskUpdate / TaskList / TaskGet の挙動、replay 復元がカバーされる - -## 範囲外 - -- 注意機構（`` の自動注入による「サボり防止」）→ `tickets/session-todo-reminder.md` -- Task の階層・優先度・タグ -- TUI / GUI での Task 状態の可視化（ツール呼び出しのイベントは既に流れているので、クライアント側で表示するかは別軸） -- Task の永続化を専用 LogEntry に分離する設計（現方針は tool_call replay と compact 時 snapshot 維持で復元、追加レーン不要） -- 複数 Pod 間で Task を共有する仕組み - -## 参照 - -- 設計指針: `CLAUDE.md`（最小の構造化 / 概念の追加は不在が問題になってから） -- 参考実装: Claude Code の TodoWrite、OpenCode の todo tool -- 関連: `crates/tools/src/tracker.rs`（session-lifetime 状態の前例）、`crates/pod/src/compact/worker.rs`（auto-injection レーン） -- 後続: `tickets/session-todo-reminder.md`（注意機構） - -## Review - -- 状態: Approve (再レビュー済み、blocking 解消・non-blocking 対応反映) -- レビュー詳細: [./session-todo.review.md](./session-todo.review.md) -- 日付: 2026-05-03 diff --git a/tickets/session-todo.review.md b/tickets/session-todo.review.md deleted file mode 100644 index 9b447de4..00000000 --- a/tickets/session-todo.review.md +++ /dev/null @@ -1,87 +0,0 @@ -# Review: セッション内 Task ツール - -対象: `tickets/session-todo.md` (`28fe1da` で再分割) / 実装 `6f2aca8`。 - -## 前提・要件の確認 - -- TaskCreate / TaskList / TaskGet / TaskUpdate を builtin tool として登録: 充足。`crates/tools/src/task.rs:507` の `task_tools(store)` を `crates/tools/src/lib.rs:55` の `builtin_tools` が拡張し、Controller 経由で Worker に登録される (`crates/pod/src/controller.rs:268`)。 -- TaskCreate の自動採番 + 初期 status pending: 充足 (`crates/tools/src/task.rs:71`)。 -- TaskUpdate の最低1フィールド + 既存チェック + 物理削除なし (`status=deleted`): 充足 (`crates/tools/src/task.rs:108`、テスト `task_update_validates_existing_and_at_least_one_field`)。 -- TaskList が completed / deleted を含めて全件返す: 充足 (`crates/tools/src/task.rs:84`)。 -- TaskGet が `taskid` 不在でエラー: 充足 (`crates/tools/src/task.rs:304`)。 -- Resume 時の履歴 replay で TaskCreate / TaskUpdate を順に再適用: 実装あり (`replay_history`、`Pod::from_state` で `TaskStore::from_history(&state.history)`)。**ただし compact 後の resume で重複が発生する** (後述「Blocking」)。 -- `tool_call.arguments` の schema 乖離は無視: 充足 (`from_str` 失敗時 `_` でスキップ)。 -- compact 起動時、TaskStore 全スナップショットを compact worker context に渡す: 充足 (`build_summary_input` の `task_snapshot` 引数、`crates/pod/src/pod.rs:1278`)。 -- compact worker に Task 編集権限を与えない: 充足 (compact 用 `summary_worker.register_tool` は read / mark_read_required / add_reference / write_summary のみ。Task* tool は注入されていない)。 -- compact 後の新セッションで TaskStore handle を維持し全件保持: 充足 (`self.task_store` は compact 中に置換されない。Worker の history のみ入れ替え)。 -- compact 後の新セッション冒頭に snapshot system message + TaskList 発火: 充足 (`crates/pod/src/pod.rs:1409-1419`)。 -- 専用 LogEntry / Persistence 型を追加しない: 充足 (`replay_history` と inline snapshot text に閉じている)。 -- 単体テスト: 4 ツール基本 / バリデーション / replay / compact snapshot replay の4本。**retained tail に Task* call が残るシナリオの replay テストが欠落** (後述)。 - -## アーキテクチャ・スコープ - -- `tools` クレートの session-lifetime 状態として TaskStore を置き、Pod が handle を所有して `attach_task_store` で受け渡す方針は `Tracker` の前例と整合。レイヤー越境なし。 -- 4 ツールを `crates/tools/src/task.rs` の単一モジュールにまとめている点は、現時点の規模 (~600 行) と将来の機能追加余地を考えると許容範囲。`pod.rs` への impl 流入もない。 -- ツール本体は `tools` 層、配線と compact 連携は `pod` 層、と責務分離が保たれている。 -- TaskStore は `Arc>` の thin wrapper で、永続化を tool_call replay に寄せる方針 (専用レーン無し) もチケットの要件どおり。 -- 命名: `TaskStore` / `TaskEntry` / `TaskStatus` / `TaskSnapshot` の対称性も良。`insomnia-` prefix は無し (規約準拠)。 -- `task_store: tools::TaskStore` を pub field 露出ではなく `attach_task_store` / `task_store()` の getter で扱っており、これも `Tracker` の前例と整合。 -- compact 時の "synthetic tool_call" (`compact-tasklist` の TaskList call/result) は新規パターン。Pod 配下で Worker 履歴に注入される唯一のラインであり、再現性とプロバイダ受理性が運用上の検証対象になる (フォローアップ参照)。 - -## 指摘事項 - -### Blocking - -- **compact 跨ぎの resume で TaskStore が重複/二重適用される。** `crates/pod/src/pod.rs:1398-1423` で構築する post-compact history は `[summary, ...auto-read, snapshot system message, TaskList synthetic call/result, references, ...retained_items]` の順だが、`retained_items` は compact 直前まで生きていた末尾スライスをそのまま含むため、retained 内に `TaskCreate` / `TaskUpdate` が含まれた場合、resume 時の `TaskStore::from_history` が: - 1. 先頭付近の `[Session TaskStore snapshot]` を読み `replace_with` で全件復元 - 2. 後続の retained `TaskCreate` を再適用 → 同内容の重複 entry が新規 `taskid` で生える - 3. retained `TaskUpdate` も再適用 (こちらは冪等で害は薄いが意味は揺れる) - - になる。compact は token threshold 到達で起きるので、直近に Task* を呼んだケース (= 実際の主用途) で確実に発火する。 - - 既存テスト `replay_history_uses_compact_snapshot_and_continues_updates` (`crates/tools/src/task.rs:592`) は snapshot を history[0] に置き、後続を「compact 後の新規操作」として書いているため、本ケース (snapshot より後の retained_items に旧 Task* call が残る) を踏んでいない。 - - 対策案: - - (A) `pod.rs:1423` の `new_history.extend(retained_items)` の前に retained_items から `Item::ToolCall { name == "TaskCreate" | "TaskUpdate" .. }` と対応する `ToolResult` を除去する。snapshot + TaskList 再発火がポストコンパクトの唯一の真実になる - - (B) `replay_history` 側に anchor を入れ、snapshot を見たら**それ以前の** Task* call を捨てる + snapshot 以降は通常 replay。retained_items が snapshot より後ろにある現レイアウトでは効かないので、history 上の snapshot の位置を末尾 (retained_items の後) に移すなどレイアウト変更が必要 - - (C) compact 後 history 構築時に、snapshot の代わりに「TaskStore を空にしてから個別 TaskCreate で再現する」方針に切り替える (= snapshot の役割を tool_call 列で担う)。ただし冗長なので非推奨 - - 推奨は (A)。あわせて回帰テストとして「retained_items に旧 TaskCreate を含む post-compact history を `from_history` に通し、duplicate が出ないこと」のケースを追加してほしい。 - -### Non-blocking / Follow-up - -- **snapshot text のラウンドトリップが lossy。** `render_snapshot` は `subject` / `description` を1行 inline で書き出すので、本文に改行が含まれると `parse_rendered_snapshot` が後続行を読み落とし、subject/description の途中で切れる (`crates/tools/src/task.rs:377-433`)。`description` は LLM が複数行で書く可能性が現実的なので、snapshot レンダリングを JSON ブロック (例: コードフェンスで `{"tasks":[...]}`) に切り替えるか、`description` に YAML literal block (`|`) を使うなどでロスレス化したい。replay の信頼性に直結する。 -- **compact 由来の synthetic ToolCall の provider 受理性。** `Item::tool_call("compact-tasklist", "TaskList", "{}")` + 対応する `ToolResult` が、対応する assistant turn なしに history 中に挿入される (`crates/pod/src/pod.rs:1414-1419`)。Anthropic / OpenAI / Codex 各 client がこの形を受理して次リクエストを通すか、E2E で1度通しておきたい。フォーマット・ID プレフィックス的に拒否されるリスクは低いが、未検証ライン。 -- **`PodInterceptor::pre_llm_request` の `&& !state.just_compacted()` 追加 (`crates/pod/src/ipc/interceptor.rs:130`) はチケット範囲外。** 内容自体は `try_post_run_compact` (`pod.rs:1079`) と同じガードを request-side にも回す mirror で、compact 直後の閾値再ヒットによる thrash 防止として妥当。とはいえ「session-todo」のスコープではないので、本来は `compact-thrash-fix` のような別チケットに切り出すか、ticket 本文に「副次修正」として明記するのが望ましい。今回はそのまま採用するなら commit message と `## 範囲外` の補記で痕跡を残してもらえると後追いが楽。 -- snapshot system message に「This is the complete session task list ...」と英文で説明が入っているが、TaskList 用の synthetic tool_call の summary には説明なし。LLM 体験としては snapshot system message → TaskList result の二段で同じ情報が出るので、片方に寄せて簡潔化しても良い (任意)。 - -### Nits - -- `crates/pod/src/pod.rs:1398` のコメント: `[summary, ...auto-read, task snapshot, TaskList result, references, ...retained]` だが実際は `task snapshot` の後ろに `TaskList tool_call` と `tool_result` の **2 件** が入る。3 件であることをコメントに反映。 -- `task.rs:79` `inner.next_taskid.saturating_add(1)`: u64 で saturating は事実上不要だが、害もない。 -- `lib.rs` の `pub use task::{TaskEntry, TaskSnapshot, TaskStatus, TaskStore, task_tools};` で `task::snapshot_overview` だけ `pod.rs` から `tools::task::snapshot_overview(...)` のフルパスで参照されている。`tools::snapshot_overview` 等で同レベル re-export しておくと API surface が揃う (任意)。 - -## 判断 - -**Request changes** — Resume 後の TaskStore 重複は要件「セッションを resume すると、履歴中の TaskCreate / TaskUpdate replay により TaskStore が復元される」「compact を跨いでも、TaskStore 全件が維持され」の両方に直接抵触する正解性 issue。Blocking 1点 (retained_items 二重適用) を直したうえで回帰テストを追加して再提出してほしい。アーキテクチャ・スコープ・残りの要件カバーは概ね健全で、`簡素な` 設計を保てている。 - -## 再レビュー (commit `420f74e` まで反映) - -判断: **Approve**。 - -### 対応状況 - -- **Blocking (retained 二重適用)**: 解消。`ceafff9` で post-compact history のレイアウトを `[summary, auto-read, references, ...retained, snapshot, TaskList synthetic]` に変更。snapshot を末尾に置き `replace_with` が常に最後勝ちにする設計に切り替えた。回帰テスト `trailing_snapshot_supersedes_pre_compact_taskcreates_in_retained` で「retained に pre-compact TaskCreate × 2 + TaskUpdate × 2 → snapshot → post-compact TaskCreate × 1」のシナリオが正しく 3 件で収束することを確認。 -- **Non-blocking #1 (snapshot text の lossy round-trip)**: 解消。`420f74e` で `render_snapshot` を「overview 一行 + ` ```json` フェンス内 JSON」形式に変更。`parse_compact_snapshot_text` も JSON フェンスから serde で復元する形に再実装。`snapshot_round_trips_multiline_subject_and_description` テストで改行入り subject/description + 紛らわしい `status:` プレフィックス + バッククォートが lossless で復元することを確認。`HashMap`-based 行パーサと `parse_rendered_snapshot` / `task_from_fields` ヘルパは削除。 -- **Non-blocking #2 (synthetic compact-tasklist の provider 受理性)**: 構造テストで部分対応。`synthetic_compact_tasklist_pair_is_well_formed` テストで「snapshot system message → TaskList tool_call → ToolResult」3 連の構造契約 (matched call_id、tool name、content recoverability) を確認。実プロセス E2E はプロジェクト全体で未設計 (CLAUDE.md 「E2E は未設計」) のためここでは構造的健全性までしか確認できない。実 provider 通過は将来の E2E 設計時に再確認する follow-up。 -- **Non-blocking #3 (`interceptor.rs:130` 範囲外修正)**: 範囲外注記として残す方針で確定。`!state.just_compacted()` ガードは `try_post_run_compact` の既存ガードを request-side に mirror する妥当な小修正で、本来は別チケットだが現規模では note 扱いとする (reviewer 提示の 2択のうち「範囲外注記」を採用)。今後 compact thrash 周りで関連修正が必要になった場合に別チケットへ切り出すフォローアップ余地は残る。 - -### Nit (`pod.rs:1398` コメントの 3 件挿入の不一致) - -レイアウト変更に伴い同じく更新済み (`ceafff9`)。新コメント: `[summary, ...auto-read, references, ...retained, task snapshot, TaskList synthetic call/result]` + 末尾配置の理由を明記。 - -### 残る follow-up - -- 改行入り subject/description のロスレス化は完了したが、TaskList が tool result として LLM に返す JSON フェンスのトークン効率は将来の課題 (大量 task 時) -- synthetic `compact-tasklist` の provider 受理性 E2E (上述、プロジェクト E2E 設計の中で扱う) -- `interceptor.rs` の compact thrash guard は compact 周辺の他 issue が表面化した際に専用チケット化を検討