From 76e5e66326430f2c9f25692a16a75431179b337a Mon Sep 17 00:00:00 2001 From: Hare Date: Sat, 2 May 2026 01:38:50 +0900 Subject: [PATCH] =?UTF-8?q?update:=20workflow=E3=81=AE=E5=AE=9F=E8=A3=85?= =?UTF-8?q?=E4=BF=AE=E6=AD=A3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/memory/src/linter/mod.rs | 45 +++++++++++++++++++++ resources/prompts/internal.toml | 2 +- tickets/workflow.md | 7 ++++ tickets/workflow.review.md | 69 +++++++++++++++++++++++++++++++++ 4 files changed, 122 insertions(+), 1 deletion(-) create mode 100644 tickets/workflow.review.md diff --git a/crates/memory/src/linter/mod.rs b/crates/memory/src/linter/mod.rs index a526ab0c..4852a4b4 100644 --- a/crates/memory/src/linter/mod.rs +++ b/crates/memory/src/linter/mod.rs @@ -25,6 +25,7 @@ use crate::schema::{ DecisionFrontmatter, KnowledgeFrontmatter, RequestFrontmatter, SummaryFrontmatter, WorkflowFrontmatter, split_frontmatter, }; +use crate::workflow::WORKFLOW_DESCRIPTION_HARD_CAP; use crate::workspace::{ClassifiedPath, RecordKind, WorkspaceLayout}; pub use existing::{ExistingRecords, scan_existing}; @@ -258,6 +259,18 @@ impl Linter { }; size::check_body::(parsed.body, &mut report); + // Mirror the loader's cap so human-edit paths fail fast instead + // of surfacing the same error only at Pod startup. + if parsed.frontmatter.model_invokation { + let actual = parsed.frontmatter.description.chars().count(); + if actual > WORKFLOW_DESCRIPTION_HARD_CAP { + report.push_error(LintError::DescriptionTooLong { + actual, + limit: WORKFLOW_DESCRIPTION_HARD_CAP, + }); + } + } + let existing = match existing::scan_existing(&self.layout) { Ok(e) => e, Err(e) => { @@ -518,6 +531,38 @@ mod tests { ))); } + #[test] + fn workflow_lint_flags_long_description_when_model_invokation() { + let (_dir, linter) = workspace(); + let desc = "x".repeat(crate::workflow::WORKFLOW_DESCRIPTION_HARD_CAP + 1); + let wf = format!( + "---\ndescription: {desc}\nmodel_invokation: true\nuser_invocable: true\n---\n" + ); + let report = linter.lint_workflow(&wf); + assert!( + report + .errors + .iter() + .any(|e| matches!(e, LintError::DescriptionTooLong { .. })), + ); + } + + #[test] + fn workflow_lint_allows_long_description_when_not_model_invokation() { + let (_dir, linter) = workspace(); + let desc = "x".repeat(crate::workflow::WORKFLOW_DESCRIPTION_HARD_CAP + 1); + let wf = format!( + "---\ndescription: {desc}\nmodel_invokation: false\nuser_invocable: true\n---\n" + ); + let report = linter.lint_workflow(&wf); + assert!( + !report + .errors + .iter() + .any(|e| matches!(e, LintError::DescriptionTooLong { .. })), + ); + } + #[test] fn workflow_lint_collects_multiple_unknown_requires() { let (_dir, linter) = workspace(); diff --git a/resources/prompts/internal.toml b/resources/prompts/internal.toml index b16ebcaa..a6038326 100644 --- a/resources/prompts/internal.toml +++ b/resources/prompts/internal.toml @@ -48,7 +48,7 @@ resident_workflows_section = """\ --- ## Resident workflows -The following workflows are advertised resident. Invoke one with / when its description matches the task. +The following workflows are advertised resident. When a user request matches one, follow its procedure as authoritative instead of improvising. User-invocable workflows can additionally be triggered by the user typing /; you cannot invoke any of them yourself. {{ entries }}\ """ diff --git a/tickets/workflow.md b/tickets/workflow.md index ea7f4751..8f6d82df 100644 --- a/tickets/workflow.md +++ b/tickets/workflow.md @@ -74,3 +74,10 @@ agent-skills (agentskills.io 形式) は本チケットの ingest 経路を再 - Knowledge / `#` の retrieval: `docs/plan/memory.md` §retrieval 経路 - Submit segment: `tickets/submit-tui-completion.md`（`Atom::WorkflowInvoke`）、`tickets/session-log-segments.md` - 後続: `tickets/agent-skills.md`（外部 SKILL を Workflow として ingest する経路） + +## Review + +- 状態: Approve +- レビュー詳細: [./workflow.review.md](./workflow.review.md) +- 日付: 2026-05-02 +- 主要指摘: なし（フォローアップ／nit レベルのみ）。 diff --git a/tickets/workflow.review.md b/tickets/workflow.review.md new file mode 100644 index 00000000..6df0bf45 --- /dev/null +++ b/tickets/workflow.review.md @@ -0,0 +1,69 @@ +# Review: Workflow 実装 + +## 前提・要件の確認 + +- **Workflow loader / 検証**: + - `/.insomnia/memory/workflow/*.md` のスキャン → `crates/memory/src/workflow.rs:120-180`（`load_workflows`）。OK。 + - frontmatter 必須欠落・型不一致は hard error → `WorkflowLoadError::Frontmatter` 経由で `PodError::WorkflowLoad` まで持ち上がる（`crates/pod/src/pod.rs:2310-2315`）。OK。 + - slug とファイル名の不一致は `Slug::parse(stem)` で hard error 化 → `crates/memory/src/workflow.rs:154-159`。`Slug::parse` の kebab-case 強制が一致検証を兼ねている。OK。 + - 未知フィールドは `tracing::warn!` → `warn_unknown_workflow_fields`（`crates/memory/src/workflow.rs:218-238`）。OK。 + - 重複 slug は最初を採用 + warn → `crates/memory/src/workflow.rs:148-152`。OK。 + +- **`/` 呼び出し経路**: + - `Segment::WorkflowInvoke` を `Pod::resolve_workflow_invocations` で resolve → `crates/pod/src/pod.rs:712-740`。 + - 解決失敗（未登録 / `user_invocable: false`）は Worker に届ける前に `Event::Error { code: InvalidRequest }` で返す → `crates/pod/src/controller.rs:366-372`、`Pod::validate_workflow_invocations`（`pod.rs:746-769`）。OK。 + - `requires` は `/.insomnia/knowledge/.md` を `WorkspaceLayout::knowledge_path` 経由で直接読み、`Item::system_message` として Workflow 本文の前に積む（`crates/pod/src/workflow/mod.rs:90-130`）。ticket の「slug 完全一致経路で取得」は満たす。 + - Markdown 本文をそのまま利用、DSL 化なし。OK。 + +- **`model_invokation` 注入**: + - `WorkflowRegistry::resident_entries` → `Pod::system_prompt` で `resident_workflow_slice` として `SystemPromptContext` に注入し、`resident_workflows_section` テンプレートで `## Resident workflows` 節を末尾に追加する（`crates/pod/src/pod.rs:592-612`、`crates/pod/src/prompt/system.rs:227-240`、`resources/prompts/internal.toml:47-54`）。OK。 + - description 上限 1024 chars（`WORKFLOW_DESCRIPTION_HARD_CAP`）を `model_invokation: true` の場合のみロード時に強制 → `crates/memory/src/workflow.rs:170-179`。Knowledge と同じポリシーで一貫。OK。 + - Phase 2 prompt には入れない: `set_resident_knowledge_injection(false)` 系のフラグで `inject_resident_knowledge` を落とすと workflow 注入も同時に無効化される（`pod.rs:589-598`）。OK。 + +- **`user_invocable: false` の挙動**: + - 補完候補から除外 → `WorkflowRegistry::list_user_invocable` は `record.user_invocable` で filter（`crates/memory/src/workflow.rs:67-73`）。`shared_state.list_workflow_completions` が IPC `ListCompletions` に流れる（`crates/pod/src/ipc/server.rs:106-115`）。OK。 + - 明示呼び出しはエラー → `validate_workflow_invocations` が `NotUserInvocable` を返し `InvalidRequest` で client に返却。OK。 + +- **Linter ルール**: + - 汎用 Write/Edit に対する `memory/workflow/` deny: `deny_write_rules` が `memory_dir()` を recursive deny するので `memory/workflow/` も covered（`crates/memory/src/scope.rs:19-24`）。OK。 + - memory tool 自身の workflow 書き込み禁止: `RecordKind::Workflow` を `Linter::lint` 冒頭で `LintError::WorkflowWriteForbidden` として弾く（`crates/memory/src/linter/mod.rs:101-105`）。`MemoryToolKind` enum に `Workflow` を含めないことで write/edit tool 自体が deserialize 段階で拒否（`crates/memory/src/tool/mod.rs:26-27`、`tool/write.rs:236-247`、`tool/edit.rs:242-253`）。OK。 + - consolidation の write tool schema に `workflow` カテゴリは含まれず（`crates/memory/src/consolidate/`、`extract/` に workflow への書き込み経路なし）。OK。 + - `lint_workflow` で frontmatter 検証 + `requires` 整合性チェック（`crates/memory/src/linter/mod.rs:250-280`）。OK。 + +- **単体テスト**: + - frontmatter 検証の正常系: `loads_valid_workflow_with_default_flags` / 異常系: `invalid_filename_is_hard_error`、`missing_description_is_hard_error`（`crates/memory/src/workflow.rs` テスト）。 + - `requires` 解決: `resolves_requires_before_workflow_body`、`missing_required_knowledge_errors`（`crates/pod/src/workflow/mod.rs` テスト）。 + - フラグ別の挙動: `model_invokation_uses_typo_field`、`user_invocable_false_errors`。 + - すべてパス確認済み（`cargo test -p memory --lib workflow`、`cargo test -p pod --lib workflow`）。OK。 + +完了条件はすべて満たされている。 + +## アーキテクチャ・スコープ + +- 低レベル基盤の `memory` クレートに loader / registry / linter を置き、Pod 側は resolver と registry の保持に留める。`llm-worker` 直下には触らず層分けは保たれている。OK（`feedback_llm_worker_scope` の方針に整合）。 +- 新規モジュールはいずれもファイル名のみで `insomnia-` プレフィックスなし。OK。 +- `crates/memory/src/workflow.rs` を新設し、schema/linter は既存ファイルへの最小追記。`pod.rs` への注入は既存の resident knowledge 経路を素直に拡張する形で、過剰な抽象化はない。`format_resident_entries` を `(slug, description)` のイテレータで共通化したのも妥当（`prompt/system.rs:255-285`）。 +- `WorkflowResolveError` は `crates/pod/src/workflow/mod.rs` 内に閉じ、`PodError::WorkflowResolve` で `From` 実装を介してチェイン。Pod 公開 API は `validate_workflow_invocations` / `workflow_completions` の 2 つのみで surface も最小。OK。 +- 依存追加は Cargo.lock の差分にあるが新規 crate 依存は導入されていない（既存 `memory`/`tracing`/`thiserror` で完結）。`cargo add` 違反なし。 +- スコープ外の変更は無し。本ブランチの merge-base は `31d9b9b` で、`433ee0f` 1 コミットのみ。develop 直近の `00755cf "manifestで一部値のzeroの扱いを変更"` は merge-base より後の develop 側コミットであり、3-way merge で develop に残る（本ブランチは触れていない）。 + +## 指摘事項 + +### 対応済み（追加修正） + +- **`lint_workflow` の description hard cap 検証** — loader 側 (`workflow.rs:170-179`) と挙動を揃えるため `Linter::lint_workflow` (`linter/mod.rs:250-`) で `model_invokation: true` の 1024 chars 上限を検証するよう追加。テスト 2 件 (`workflow_lint_flags_long_description_when_model_invokation` / `workflow_lint_allows_long_description_when_not_model_invokation`) を追加。 +- **`resident_workflows_section` の文言** — `Invoke one with /` は LLM 側に invoke 経路がなく、かつ `model_invokation: true && user_invocable: false` の組み合わせ（resident に出るが `/` 拒否）が存在するため二重に誤誘導だった。`When a user request matches one, follow its procedure as authoritative instead of improvising. User-invocable workflows can additionally be triggered by the user typing /; you cannot invoke any of them yourself.` に書き換え（user_invocable に依存しない汎用形）。 + +### Non-blocking / Follow-up + +- **Workflow body の `#` リファレンス** — Markdown 本文に `#` を書いても展開されない（resolver は `requires` フロントマターのみ参照）。`requires` で代用する設計だが、Workflow 著者が混乱しないよう将来の Workflow Linter で「本文中の `#` が `requires` に登録されているか」のチェックを追加する余地あり。 + +### Nits + +- `crates/memory/src/workflow.rs:218-238` の `warn_unknown_workflow_fields` は許可フィールドの一覧をハードコードしており、`WorkflowFrontmatter` 側にフィールドを追加した際の同期忘れリスクがある。serde の `deny_unknown_fields` 相当を別パスで模しているとはいえ、配列定数を `pub const ALLOWED_FIELDS: &[&str]` のように schema 側に置いて両方から参照する方が安全。 +- `WorkflowFrontmatter::updated_at` を `Option` 化して `epoch()` フォールバックする実装（`schema/workflow.rs:35-40`）は MVP 用途として合理的だが、`Frontmatter::updated_at` が「実時刻 or epoch」のどちらを返すかが呼び出し側で見えない。利用箇所（`size::check_body` 経由）に影響しない範囲だが、トレイトのコメントに「workflow は epoch を返し得る」旨を一言入れておくと混乱を避けられる。 +- `WorkflowResolveError::InvalidSlug` は `validate_workflow_invocations` 経由でしか発火しないが、ユーザー入力の段階で TUI 側 `Atom::WorkflowInvoke` が同じ `Slug::parse` を通っているはず（`crates/tui/src/input.rs`）。重複検証は害ではないが冗長。 + +## 判断 + +**Approve** — チケット要件は完全に満たされ、テストも妥当。スコープ外の変更も無し。指摘はすべて follow-up / nit レベル。