refactor: remove workflow machinery
This commit is contained in:
@@ -25,7 +25,6 @@ When these roots differ, implementation worktrees must be created under `impleme
|
||||
- Ticket role launch prompts now emit a `Workspace routing context` section for Orchestrator launches, and for any launch that explicitly provides original/target roots.
|
||||
- `TicketRoleLaunchPlan` carries `original_workspace_root`, `implementation_worktree_root`, and `target_workspace_root` as explicit plan fields for callers/tests.
|
||||
- Orchestrator role prompt resources now say to create implementation worktrees under the original workspace root and to run merge-completion against the recorded target workspace.
|
||||
- `worktree-workflow` now describes `<original-workspace-root>/.worktree/<task-name>` and `git -C <original-workspace-root> ...` instead of assuming the Orchestrator cwd is the repository root.
|
||||
|
||||
## Follow-up boundaries
|
||||
|
||||
|
||||
@@ -1,133 +0,0 @@
|
||||
# Public builtin workflows and Yoi dogfood workflows
|
||||
|
||||
Yoi should split workflows into public builtin procedures and project-local dogfood procedures. The builtin set should teach the general Ticket/role lifecycle; this repository's Git worktree, branch, cargo, nix, merge, cleanup, and report conventions should stay project-local unless they are explicitly selected as an optional extension.
|
||||
|
||||
This document records the design/audit decision for Ticket `00001KTRKZ14C`. It does not implement the builtin workflow loader.
|
||||
|
||||
## Decision
|
||||
|
||||
- Add a typed builtin workflow source, conceptually `WorkflowSource::Builtin`, with clear provenance in any list/show/launch surface.
|
||||
- Store public builtin workflow resources under `resources/workflows/<slug>.md`, not under `resources/prompts`.
|
||||
- Workflows are user-visible procedural artifacts selected by slug and source. They need source/provenance/listing semantics distinct from prompt fragments.
|
||||
- They are still embedded runtime resources, but prompt-fragment override rules should not silently become workflow authority.
|
||||
- Source priority for workflow lookup should be:
|
||||
1. explicit path/file selector, if the CLI/config surface allows one;
|
||||
2. workspace `.yoi/workflow/<slug>.md`;
|
||||
3. user workflow directory, if/when one is added;
|
||||
4. builtin `resources/workflows/<slug>.md`;
|
||||
5. external skill/plugin workflow sources, only when explicitly enabled and named.
|
||||
- Workspace workflows override builtin workflows by slug. Overrides must show provenance so a role launch can say whether it used workspace, user, builtin, or plugin workflow text.
|
||||
- Keep role Profile policy separate from workflow prose. Profiles select role behavior/tool policy; workflows provide procedural flow; launch prompts provide concrete Ticket/action context.
|
||||
- Public resident core should remain small: intake requirements sync and orchestrator routing. Multi-agent implementation/review is builtin-available but not necessarily resident in every profile. Git worktree mechanics are optional/project-local by default.
|
||||
|
||||
## Audit of current workspace workflows
|
||||
|
||||
| Current slug | Public builtin candidate | Dogfood/project-local material | Decision |
|
||||
| --- | --- | --- | --- |
|
||||
| `ticket-intake-workflow` | Yes. The core requirement-sync flow, existing-Ticket refinement, readiness, duplicate avoidance, and Ticket materialization are product-generic. | Mentions current local storage and Yoi-specific Ticket command examples. | Move cleaned generic version to builtin core. Keep storage-command specifics either bounded and product-generic or in project-local notes. |
|
||||
| `ticket-orchestrator-routing` | Yes. The routing classifications, queued acceptance contract, relation/orchestration-plan checks, and planning-return rules are product-generic. | Contains Yoi dogfood examples, worktree/multi-agent handoff specifics, and merge-completion guidance tied to this repo's operating policy. | Move generic routing gate to builtin core. Split dogfood merge/worktree details into workspace override or separate dogfood workflow. |
|
||||
| `ticket-preflight-workflow` | Compatibility candidate only. It now describes planning/requirements sync and legacy canonical id compatibility rather than a standalone lane. | Mostly project-history wording and deprecated-preflight vocabulary. | Do not make resident core. Keep as builtin compatibility alias only if existing configs still reference it; otherwise replace references with intake/planning sync. |
|
||||
| `multi-agent-workflow` | Partly. The sibling coder/reviewer loop, intent packet, branch-local review, and merge-ready dossier are useful product concepts. | Heavy Yoi dogfood specifics: Git worktrees, commits, cargo/nix validation, branch cleanup, docs/report conventions, and parent/child policy. | Split into builtin `multi-agent-workflow` for role loop + dossier, and workspace dogfood extension for Git worktree/cargo/nix mechanics. |
|
||||
| `worktree-workflow` | Not resident public core. | Almost entirely this repository's Git worktree mechanics and `.yoi` path exclusions. | Keep workspace-local dogfood workflow. Optionally create a non-resident builtin `git-worktree-isolation` later, disabled unless explicitly selected. |
|
||||
|
||||
## Slug and workspace Ticket settings migration decision
|
||||
|
||||
Use explicit dogfood slugs for workflows whose semantics differ from the public builtin workflow. Same-slug workspace overrides are allowed for local wording/policy tweaks of the same public contract, but they should not be used to hide Yoi repository Git/worktree/cargo/nix/merge semantics behind a public slug. This avoids accidental shadowing when a user expects `multi-agent-workflow` to mean the generic builtin role loop.
|
||||
|
||||
Planned selector mapping for this repository after builtin workflow loading exists:
|
||||
|
||||
| Role/config surface | Workflow selector | Source intent | Rationale |
|
||||
| --- | --- | --- | --- |
|
||||
| `[ticket.roles.intake].workflow` | `ticket-intake-workflow` | builtin core by default; workspace same-slug override only if this repo needs a true local refinement of the same intake contract | Intake is mostly product-generic; dogfood Git/worktree policy does not belong here. |
|
||||
| `[ticket.roles.orchestrator].workflow` | `ticket-orchestrator-routing` | builtin core by default; dogfood merge/worktree instructions come from explicit launch context or a separate dogfood workflow, not a hidden same-slug override | Routing semantics should stay public/generic; repo-specific implementation mechanics should be opt-in and visible. |
|
||||
| `[ticket.roles.coder].workflow` | `yoi-dogfood-multi-agent-workflow` | workspace-local dogfood workflow | Coder work in this repository is tied to the repo worktree/branch/validation contract and must not shadow generic builtin `multi-agent-workflow`. |
|
||||
| `[ticket.roles.reviewer].workflow` | `yoi-dogfood-multi-agent-workflow` | workspace-local dogfood workflow | Reviewer evidence in this repository includes branch/worktree/cargo/nix/Ticket-doctor conventions; use an explicit dogfood slug. |
|
||||
| compatibility references | `ticket-preflight-workflow` | builtin compatibility alias or workspace compatibility file, non-resident | Do not keep it as a role default. It should point to planning/requirements sync language only. |
|
||||
| dogfood worktree helper | `yoi-dogfood-worktree-workflow` | workspace-local helper, not a role default by itself | Referenced from `yoi-dogfood-multi-agent-workflow`; keeps Git worktree mechanics out of generic builtin workflows. |
|
||||
|
||||
`multi-agent-workflow` should be reserved for the generic builtin sibling coder/reviewer loop. This repository should not keep a same-slug workspace override with dogfood semantics once the builtin exists. If a temporary same-slug override is needed during migration, it must be short-lived and the Ticket/config comment must say it shadows the builtin intentionally.
|
||||
|
||||
## Builtin workflow set
|
||||
|
||||
### Resident builtin core
|
||||
|
||||
These should be advertised by default when Ticket support is enabled:
|
||||
|
||||
- `ticket-intake-workflow`
|
||||
- generic Ticket requirements sync/materialization;
|
||||
- existing Ticket refinement with explicit user agreement;
|
||||
- readiness and duplicate handling;
|
||||
- no Yoi repository cargo/nix/worktree policy.
|
||||
- `ticket-orchestrator-routing`
|
||||
- explicit routing gate;
|
||||
- queued acceptance before implementation side effects;
|
||||
- relation/orchestration-plan/context checks;
|
||||
- implementation-ready IntentPacket shape;
|
||||
- no repository-specific merge authority or Git cleanup policy.
|
||||
|
||||
### Builtin available but optional
|
||||
|
||||
These can be embedded and selectable, but should not be injected as resident core unless the active Profile/config opts in:
|
||||
|
||||
- `multi-agent-workflow`
|
||||
- generic sibling coder/reviewer loop;
|
||||
- intent packet, reviewer evidence, blocker/fix-loop, merge-ready dossier;
|
||||
- no mandatory Git worktree, cargo, nix, or `.yoi` cleanup rules.
|
||||
- `ticket-preflight-workflow`
|
||||
- transitional alias for old configurations that still name preflight;
|
||||
- should route to planning/requirements sync language and avoid reintroducing preflight as a lifecycle state.
|
||||
|
||||
### Workspace-local dogfood workflows
|
||||
|
||||
These remain in this repository under `.yoi/workflow/` as overrides or dogfood-only slugs:
|
||||
|
||||
- `worktree-workflow`
|
||||
- mechanical Yoi project worktree setup;
|
||||
- tracked `.yoi` project records visible;
|
||||
- `.yoi/memory`, local/runtime/log/lock/secret-like paths excluded;
|
||||
- branch/worktree cleanup policy.
|
||||
- A dogfood extension for multi-agent/merge-completion, if the generic builtin `multi-agent-workflow` is cleaned:
|
||||
- concrete `cargo fmt`, `cargo test`, `target/debug/yoi ticket doctor`, validation policy;
|
||||
- merge authority boundaries for this repository;
|
||||
- docs/report conventions.
|
||||
|
||||
## Stale vocabulary cleanup
|
||||
|
||||
Builtin workflows and active workspace workflows should remove or replace the following old wording:
|
||||
|
||||
- `Action required` / `Attention required` as storage or state concepts. Use explicit Ticket state, typed relation metadata, orchestration-plan waiting notes, or Ticket thread comments instead.
|
||||
- `preflight` as a lifecycle lane/state. Use planning/requirements sync or compatibility alias language.
|
||||
- `open` / `pending` / `closed bucket` authority. Current authority is `state` plus optional `resolution.md` for closed Tickets.
|
||||
- Long-lived umbrella/progress-container Ticket terminology. Use concrete implementation Tickets and optional Objective context.
|
||||
|
||||
## Follow-up implementation boundaries
|
||||
|
||||
1. **Builtin workflow resource loader**
|
||||
- Add embedded `resources/workflows/*.md` resources.
|
||||
- Add `WorkflowSource::Builtin` / equivalent typed provenance.
|
||||
- Update workflow discovery/list/show/launch so workspace overrides builtin by slug and provenance is visible.
|
||||
- Include tests for source priority and override behavior.
|
||||
|
||||
2. **Public workflow text cleanup**
|
||||
- Create cleaned builtin versions of `ticket-intake-workflow` and `ticket-orchestrator-routing`.
|
||||
- Split generic `multi-agent-workflow` from Yoi dogfood Git/worktree/cargo/nix mechanics.
|
||||
- Keep `ticket-preflight-workflow` as compatibility-only or remove references if no longer needed.
|
||||
|
||||
3. **Dogfood workflow retention**
|
||||
- Keep or rename workspace-local dogfood workflows for this repository's implementation mechanics.
|
||||
- Ensure `.yoi/workspace.toml` `[ticket.roles.*].workflow` selectors intentionally point to builtin generic workflows or workspace dogfood overrides, with no accidental slug shadowing.
|
||||
|
||||
4. **Vocabulary migration**
|
||||
- Sweep active workflow text, prompts, docs, and Ticket UI wording for `Action required`, `Attention required`, and old preflight lane language.
|
||||
- Replace with typed state/relation/orchestration-plan terminology.
|
||||
|
||||
5. **Panel/role launch provenance display**
|
||||
- When a role launches with a workflow, include the workflow source/provenance in diagnostic/launch context so builtin vs workspace override is debuggable.
|
||||
|
||||
## Non-goals for this Ticket
|
||||
|
||||
- Implementing the builtin workflow loader.
|
||||
- Moving current workflow files into runtime resources in this branch.
|
||||
- Changing Ticket lifecycle states.
|
||||
- Changing role Profile policy.
|
||||
- Changing scheduler/queue behavior.
|
||||
@@ -1,6 +1,6 @@
|
||||
# Tickets and development workflow
|
||||
|
||||
Yoi project work is tracked through Tickets. For normal use, interact with Tickets through `yoi panel`, Ticket tools, the `yoi ticket ...` CLI, and Ticket workflows. Git history plus Ticket files remain the authoritative state-transition record behind those interfaces.
|
||||
Yoi project work is tracked through Tickets. For normal use, interact with Tickets through `yoi panel`, Ticket tools, the `yoi ticket ...` CLI, and typed role surfaces. Git history plus Ticket files remain the authoritative state-transition record behind those interfaces.
|
||||
|
||||
The current local backend stores each Ticket in the flat `.yoi/tickets/<ticket-id>/` layout. The directory name is the canonical opaque Ticket id: a fixed-width Crockford base32 Unix epoch millisecond timestamp. Slugs and frontmatter `id`/`slug` fields are not current-state authority. That storage detail matters for maintainers and backend compatibility, but it is not the primary user-facing workflow.
|
||||
|
||||
@@ -25,7 +25,7 @@ Use the highest-level interface that matches the work:
|
||||
- Use `yoi panel` for the Ticket/Intake/Orchestrator workspace Dashboard and role-launch actions.
|
||||
- Use `yoi objective ...` for lightweight medium-term Objective records and their non-blocking canonical Ticket links.
|
||||
- Inside Workers, use typed Ticket tools to create, inspect, comment, review, and close Tickets.
|
||||
- For multi-step work, follow the Ticket Intake, Orchestrator Routing, planning/requirements-sync, and Multi-agent workflows.
|
||||
- For multi-step work, follow the typed Ticket role surfaces and recorded Ticket lifecycle gates.
|
||||
|
||||
Maintainers can inspect the local `.yoi/tickets/` files directly when debugging storage, but normal user instructions should go through `yoi panel`, Ticket tools, or `yoi ticket ...`.
|
||||
|
||||
@@ -55,7 +55,7 @@ Use them when a Worker needs to materialize or update project records:
|
||||
- Reviewer records approve/request-changes review results.
|
||||
- Maintainer closes a Ticket with a resolution when merge/validation/cleanup evidence is complete.
|
||||
|
||||
Do not bypass workflow gates just because Ticket tools are available. Ticket mutation is a project-record operation and should remain auditable.
|
||||
Do not bypass Ticket lifecycle gates just because Ticket tools are available. Ticket mutation is a project-record operation and should remain auditable.
|
||||
|
||||
## Objective records
|
||||
|
||||
@@ -122,22 +122,18 @@ root = ".yoi/tickets"
|
||||
[ticket.roles.intake]
|
||||
profile = "project:intake"
|
||||
launch_prompt = "$workspace/ticket/intake/launch"
|
||||
workflow = "ticket-intake-workflow"
|
||||
|
||||
[ticket.roles.orchestrator]
|
||||
profile = "project:orchestrator"
|
||||
launch_prompt = "$workspace/ticket/orchestrator/launch"
|
||||
workflow = "ticket-orchestrator-routing"
|
||||
|
||||
[ticket.roles.coder]
|
||||
profile = "project:coder"
|
||||
launch_prompt = "$workspace/ticket/coder/launch"
|
||||
workflow = "multi-agent-workflow"
|
||||
|
||||
[ticket.roles.reviewer]
|
||||
profile = "project:reviewer"
|
||||
launch_prompt = "$workspace/ticket/reviewer/launch"
|
||||
workflow = "multi-agent-workflow"
|
||||
```
|
||||
|
||||
Fixed roles are:
|
||||
@@ -155,7 +151,7 @@ when a spike is useful, let the Orchestrator create an ordinary task-specific re
|
||||
|
||||
`launch_prompt` is a per-action first-run prompt reference for future prompt resolution. Current launcher behavior exposes the ref but does not treat it as system instruction.
|
||||
|
||||
`workflow` is the workflow the launched role should follow. State and phase-specific prompt injection are future work; any dynamic prompt content must be committed as history before it affects model context.
|
||||
Role launch prompts are plain history input. State and phase-specific prompt injection are future work; any dynamic prompt content must be committed as history before it affects model context.
|
||||
|
||||
`provider = "builtin:yoi_local"` selects Yoi's built-in local Ticket backend. `root = ".yoi/tickets"` is the canonical local storage root for this repository. Legacy `kind = "local"` is accepted only as a short transitional alias; new configs should use `provider`.
|
||||
|
||||
@@ -165,15 +161,10 @@ If `.yoi/workspace.toml` has no `[ticket]` table and no legacy fallback file exi
|
||||
- backend root: `<workspace>/.yoi/tickets`
|
||||
- all role profiles: `inherit`
|
||||
- no launch prompt refs
|
||||
- workflows:
|
||||
- intake: `ticket-intake-workflow`
|
||||
- orchestrator: `ticket-orchestrator-routing`
|
||||
- coder: `multi-agent-workflow`
|
||||
- reviewer: `multi-agent-workflow`
|
||||
|
||||
Important: top-level Ticket role launches cannot execute `profile = "inherit"` because top-level launch has no parent Profile to inherit from. Configure concrete role profiles in `.yoi/workspace.toml` under `[ticket.roles.*]` before using `yoi panel` role-launch actions.
|
||||
|
||||
## Workflow lifecycle
|
||||
## Ticket lifecycle
|
||||
|
||||
Ticket-driven development normally moves through these gates:
|
||||
|
||||
@@ -189,7 +180,7 @@ Each gate records its decision or evidence in the Ticket thread or artifacts.
|
||||
|
||||
### 1. Intake
|
||||
|
||||
Use `ticket-intake-workflow` when a user request is broad, ambiguous, or not yet a Ticket.
|
||||
Use the Intake role launch prompt when a user request is broad, ambiguous, or not yet a Ticket.
|
||||
|
||||
Intake should:
|
||||
|
||||
@@ -202,7 +193,7 @@ Intake should not schedule implementation, spawn coder/reviewer Workers, create
|
||||
|
||||
### 2. Orchestrator routing
|
||||
|
||||
Use `ticket-orchestrator-routing` to classify the next action for an existing Ticket.
|
||||
Use the Orchestrator role launch prompt to classify the next action for an existing Ticket.
|
||||
|
||||
Routing classifications include:
|
||||
|
||||
@@ -219,7 +210,7 @@ Routing decisions should be recorded with `TicketComment` using `plan` or `decis
|
||||
|
||||
### 3. Planning/requirements sync
|
||||
|
||||
Use `ticket-preflight-workflow` only as a legacy-compatible planning/requirements sync entry. Return `ready` or `queued` Tickets to `planning` only when the Orchestrator can name a concrete missing decision or information item after bounded project-context checks; risk flags and risky domains are context-lookup and reviewer-focus signals, not automatic stop gates.
|
||||
Use planning/requirements sync only as a bounded Ticket refinement step. Return `ready` or `queued` Tickets to `planning` only when the Orchestrator can name a concrete missing decision or information item after bounded project-context checks; risk flags and risky domains are context-lookup and reviewer-focus signals, not automatic stop gates.
|
||||
|
||||
Planning sync should resolve or record:
|
||||
|
||||
@@ -233,7 +224,7 @@ Do not send Tickets with unresolved concrete missing decisions/information direc
|
||||
|
||||
### 4. Implementation assignment
|
||||
|
||||
Use `multi-agent-workflow` for implementation-ready Tickets.
|
||||
Use the Coder and Reviewer role launch prompts for implementation-ready Tickets.
|
||||
|
||||
The Orchestrator should prepare an `IntentPacket` with:
|
||||
|
||||
@@ -292,19 +283,18 @@ The role-launch path is:
|
||||
User triggers a Ticket action in yoi panel
|
||||
-> Dashboard builds a TicketRoleLaunchContext
|
||||
-> client Ticket role launcher reads .yoi/workspace.toml [ticket] settings
|
||||
-> launcher selects the role Profile and workflow
|
||||
-> launcher selects the role Profile
|
||||
-> launcher spawns the role Worker
|
||||
-> launcher sends Method::Run with WorkflowInvoke + Text segments
|
||||
-> launcher sends Method::Run with Text segments
|
||||
-> launcher waits for run-acceptance evidence
|
||||
-> Dashboard reports success/failure
|
||||
```
|
||||
|
||||
The launched Worker receives dynamic Ticket/action context as its first committed run input. The Dashboard does not inject hidden context, does not write Ticket files directly, and does not construct prompt/workflow segments by hand.
|
||||
The launched Worker receives dynamic Ticket/action context as its first committed run input. The Dashboard does not inject hidden context, does not write Ticket files directly, and does not construct prompt segments by hand.
|
||||
|
||||
The first run input contains:
|
||||
|
||||
- the selected fixed role;
|
||||
- the workflow canonical id from workspace `[ticket.roles.<role>]` settings;
|
||||
- Ticket id when the action targets an existing Ticket;
|
||||
- freeform user instruction/context from the action;
|
||||
- configured `launch_prompt` reference if present, as an unresolved reference for future prompt resolution.
|
||||
@@ -324,19 +314,15 @@ root = ".yoi/tickets"
|
||||
|
||||
[ticket.roles.intake]
|
||||
profile = "project:intake"
|
||||
workflow = "ticket-intake-workflow"
|
||||
|
||||
[ticket.roles.orchestrator]
|
||||
profile = "project:orchestrator"
|
||||
workflow = "ticket-orchestrator-routing"
|
||||
|
||||
[ticket.roles.coder]
|
||||
profile = "project:coder"
|
||||
workflow = "multi-agent-workflow"
|
||||
|
||||
[ticket.roles.reviewer]
|
||||
profile = "project:reviewer"
|
||||
workflow = "multi-agent-workflow"
|
||||
```
|
||||
|
||||
If a role still uses `profile = "inherit"`, the Dashboard fails closed with a diagnostic explaining that a concrete profile is required.
|
||||
|
||||
@@ -1,42 +0,0 @@
|
||||
# Workflows and orchestration
|
||||
|
||||
Yoi development uses workflows to make multi-step agent work repeatable without hiding authority in chat state.
|
||||
|
||||
Project-authored workflows live under `.yoi/workflow/`. Generated memory lives under `.yoi/memory/`; the two should not be mixed.
|
||||
|
||||
## Workflow role
|
||||
|
||||
A workflow should define how to coordinate work. It should not become a private implementation branch, an unreviewed design decision, or a replacement for Tickets.
|
||||
|
||||
Current workflow themes include:
|
||||
|
||||
- Intake clarification before materializing user requests as Tickets
|
||||
- Orchestrator routing from Tickets to the next workflow/action
|
||||
- planning/requirements synchronization when concrete missing decisions or information block routing
|
||||
- worktree setup and cleanup
|
||||
- sibling coder/reviewer Worker orchestration
|
||||
- human-gated maintenance and merge readiness
|
||||
|
||||
## Child Workers
|
||||
|
||||
Spawned Workers are useful for scoped implementation, review, or exploration. They are not independent project authorities.
|
||||
|
||||
A parent/orchestrator must verify:
|
||||
|
||||
- child output via `ReadWorkerOutput`
|
||||
- live/restorable state via Worker tools when relevant
|
||||
- worktree state and diff
|
||||
- validation command output
|
||||
- Ticket requirements and acceptance criteria
|
||||
|
||||
Notifications are hints to inspect state. They are not proof of completion.
|
||||
|
||||
## Merge and close responsibility
|
||||
|
||||
Unless explicitly authorized otherwise, final merge, cleanup, design-boundary decisions, and ticket closure remain the orchestrator/human responsibility.
|
||||
|
||||
Child Workers may commit in delegated worktrees when the workflow allows it, but the merge-ready dossier should make the final decision auditable from repository records.
|
||||
|
||||
## Public and dogfood workflow split
|
||||
|
||||
Public builtin workflow candidates and this repository's dogfood-specific workflow mechanics are tracked in `docs/design/workflows-public-dogfood-split.md`.
|
||||
@@ -28,5 +28,4 @@
|
||||
- [ticket](ticket.md) — # テスト妥当性レビュー: ticket - 評価: 概ね良い
|
||||
- [tools](tools.md) — # テスト妥当性レビュー: tools - 評価: 混在
|
||||
- [tui](tui.md) — # テスト妥当性レビュー: tui - 評価: 混在
|
||||
- [workflow](workflow.md) — # テスト妥当性レビュー: workflow
|
||||
- [yoi](yoi.md) — # テスト妥当性レビュー: yoi - 評価: 混在
|
||||
|
||||
@@ -35,7 +35,7 @@
|
||||
- `inherit` profile は top-level launches で拒否される;
|
||||
- 解決不能な profile selectors は spawn 前に失敗する;
|
||||
- concrete role config と scaffold config が launch plans を生成する;
|
||||
- 設定された workflow/profile/launch prompt refs が表面化される;
|
||||
- 設定された profile/launch prompt refs が表面化される;
|
||||
- spawn config が pod name、profile、role、workspace root、`cwd` 境界を正しく保持する;
|
||||
- prompt override precedence がテストされている;
|
||||
- role-specific launch prompts が intake、orchestrator、coder、reviewer 向けの重要な運用ガイダンスを含む。
|
||||
|
||||
@@ -1,71 +0,0 @@
|
||||
# テスト妥当性レビュー: protocol
|
||||
|
||||
- 評価: 概ね良い
|
||||
|
||||
## 確認範囲
|
||||
|
||||
- 対象は `crates/protocol` と解釈した。
|
||||
- 読んだ範囲:
|
||||
- `crates/protocol/README.md`
|
||||
- `crates/protocol/Cargo.toml`
|
||||
- `crates/protocol/src/lib.rs`
|
||||
- `crates/protocol/src/stream.rs`
|
||||
- 実行した検証:
|
||||
- `cargo test -p protocol` — pass。unit tests 38 件、doc tests 0 件。
|
||||
|
||||
## 現在のテストがよくカバーしていること
|
||||
|
||||
`protocol` crate の主責務である「Pod client/server 間の wire protocol 型」をかなり広く検証できている。特に以下は妥当。
|
||||
|
||||
- `Method` / `Event` の serde tag 形式を、`serde_json::Value` または固定 JSON で確認している。
|
||||
- `Method::Run` と `Segment::{Text, Paste}` の roundtrip がある。
|
||||
- 未知の `Segment` kind が `Segment::Unknown` に落ちる forward compatibility 要件を直接テストしている。
|
||||
- `Notify`, `PodEvent`, completion, snapshot, status, compaction, tool result, LLM retry/continuation など、最近の運用上重要そうな wire shape が個別に押さえられている。
|
||||
- `Snapshot` の legacy default (`status`, `context_window`, `context_tokens`) を確認しており、互換性テストとして価値がある。
|
||||
- `PodEvent::should_notify_agent()` のような単なる serde ではない分類 invariant もテストされている。
|
||||
|
||||
この crate はロジックが薄く、ほぼ protocol schema の authority なので、現在のテストの中心が serde shape / roundtrip になっていること自体は適切。
|
||||
|
||||
## 不足 / 疑問のあるテスト
|
||||
|
||||
- `src/stream.rs` の `JsonLineReader` / `JsonLineWriter` にテストがない。JSONL reader/writer はこの crate の README 上の「JSONL message boundary」に近い責務なので、空行 skip、EOF、invalid JSON の `InvalidData`、writer の newline/flush 相当は最低限押さえたい。
|
||||
- `Method` / `Event` の variant coverage は完全ではない。
|
||||
- 未確認の例: `Method::Cancel`, `ListRewindTargets`, `RewindTo`, `Shutdown`; `Event::TextDone`, `ToolCallStart`, `ToolCallArgsDelta`, `ToolCallDone`, `Usage`, `Shutdown`, `MemoryWorker`, `RewindTargets`, `RewindApplied` など。
|
||||
- protocol crate は variant 追加・rename の影響が大きいため、未テスト variant が増えると wire contract の退行を見逃しやすい。
|
||||
- `Segment::flatten_to_text()` と `Method::run_text()` が未テスト。特に `flatten_to_text()` は sigil 復元・貼り付け content・unknown placeholder という明確な仕様を持つので、serde よりも壊れやすい実装ロジックとしてテスト価値が高い。
|
||||
- `ScopeRule` / `Permission` の wire shape と `recursive` default は `ScopeSubDelegated` 経由で一部触れているが、`recursive` 省略時の default や `Permission` の lowercase serde は直接確認されていない。
|
||||
- 一部テストは roundtrip のみで、期待 JSON 名や payload 値の確認が弱い。
|
||||
- 例: `pod_discovery_methods_roundtrip()` は `RestorePod { name }` / `RegisterPeer { name }` の値を assert していない。
|
||||
- roundtrip だけだと、同一型内で serialize/deserialize が揃って壊れた場合に検出できないことがある。
|
||||
- 固定 JSON 文字列との完全一致テストが一部ある。wire format の固定には有効だが、serde の field order に依存するため、意図しない脆さもある。重要な canonical shape だけに絞り、他は `serde_json::Value` で意味を assert する現在の混合方針は概ね妥当。
|
||||
|
||||
## 追加提案
|
||||
|
||||
- `stream.rs` に async unit tests を追加する。
|
||||
- writer が JSON + `\n` を出す。
|
||||
- reader が空行を skip して次の JSON を読む。
|
||||
- EOF で `Ok(None)`。
|
||||
- invalid JSON が `io::ErrorKind::InvalidData` になる。
|
||||
- writer → reader の小さな end-to-end roundtrip。
|
||||
- `Segment::flatten_to_text()` の仕様テストを追加する。
|
||||
- `Text`, `Paste`, `FileRef`, `KnowledgeRef`, `WorkflowInvoke`, `Unknown` を混ぜた順序保持。
|
||||
- `@`, `#`, `/` の sigil 復元。
|
||||
- 未カバーの control/event variant に、少なくとも代表的な pinned wire-shape テストを足す。
|
||||
- `Cancel`, `Shutdown`, `RewindTo`, `ListRewindTargets`
|
||||
- `ToolCall*`, `Usage`, `TextDone`, `RewindTargets`, `RewindApplied`, `MemoryWorker`
|
||||
- roundtrip だけのテストは、重要 payload を assert する。
|
||||
- `RestorePod.name`
|
||||
- `RegisterPeer.name`
|
||||
- `PodsListed` / `PodRestored` / `PeerRegistered` は現在 payload equality があるので良い。
|
||||
- `ScopeRule` の default と permission rename を単体で確認する。
|
||||
- `{"target": "...", "permission": "read"}` で `recursive == true`
|
||||
- `Permission::Write` が `"write"` になる。
|
||||
|
||||
## 実行したコマンド
|
||||
|
||||
```sh
|
||||
cd /home/hare/Projects/yoi && cargo test -p protocol
|
||||
```
|
||||
|
||||
結果: pass。38 unit tests passed、0 failed。doc tests は 0 件。
|
||||
|
||||
@@ -1,108 +0,0 @@
|
||||
# テスト妥当性レビュー: workflow
|
||||
- 判定: 概ね良い
|
||||
|
||||
## 確認範囲
|
||||
- Crate: `crates/workflow`
|
||||
- 確認した責務:
|
||||
- Workflow フロントマターの分割と schema defaults
|
||||
- 人間編集向け Workflow linter
|
||||
- builtin resources と `.yoi/workflow` からの Workflow discovery/loading
|
||||
- `SKILL.md` parsing と Skill-to-Workflow projection
|
||||
- Workflow registry の collision/shadowing behavior
|
||||
- Workflow directory write-deny scope helper
|
||||
- 主に読んだファイル:
|
||||
- `crates/workflow/README.md`
|
||||
- `crates/workflow/Cargo.toml`
|
||||
- `crates/workflow/src/{lib.rs,schema.rs,linter.rs,workflow.rs,skill.rs,scope.rs,error.rs}`
|
||||
- 狭い integration 参照: Workflow invocation resolution 周辺の `crates/pod/src/workflow/mod.rs` tests
|
||||
|
||||
## 現在のテストがよくカバーしていること
|
||||
- この crate には、純粋な parsing/loading 責務に対する堅実な unit-test set がある: `cargo test -p workflow` は 34 unit tests を実行し、すべて成功している。
|
||||
- Workflow loading は妥当にカバーされている:
|
||||
- valid workspace Workflow loading と default flags
|
||||
- `model_invokation` / `user_invocable` behavior
|
||||
- workspace Workflow が slug で builtin Workflow を override すること
|
||||
- 少なくとも 1 つの builtin Workflow の builtin provenance
|
||||
- invalid filename が hard error になること
|
||||
- required `description` の欠落が hard error になること
|
||||
- legacy `.yoi/memory/workflow` が無視されること
|
||||
- resident description cap enforcement
|
||||
- Registry/Skill collision behavior がカバーされている:
|
||||
- collision がない場合の insertion
|
||||
- workspace Workflow が Skill を shadow すること
|
||||
- first-fed Skill が later-fed Skill に勝つこと
|
||||
- human-readable shadow message の smoke check
|
||||
- `SKILL.md` parsing は主要な invariants をよくカバーしている:
|
||||
- minimal valid skill
|
||||
- directory/name mismatch
|
||||
- invalid slug names
|
||||
- empty description
|
||||
- description が cap ちょうど、および cap 超過の場合
|
||||
- missing frontmatter
|
||||
- `allowed-tools` を含む optional spec fields が受け入れられること
|
||||
- scan behavior: missing root、deterministic ordering、broken sibling を skip しつつ good sibling を保持すること
|
||||
- Skill-to-Workflow defaults
|
||||
- Linter tests は現在の重要な linter checks をカバーしている:
|
||||
- required Knowledge が存在する valid file
|
||||
- missing required Knowledge
|
||||
- resident description cap
|
||||
- Workflow body size limit
|
||||
- テストは temporary directories を使い、private implementation details よりも observable API behavior を主に assert しており、この crate には適切である。
|
||||
|
||||
## ギャップ / 疑問のあるテスト
|
||||
- Builtin Workflow coverage はやや弱い。`missing_directory_loads_builtin_registry` は unrelated slug が存在しないことだけを確認しており、`builtin_workflow_records_have_visible_provenance` は `multi-agent-workflow` だけを確認している。別のテストがたまたま触れない限り、1 つの builtin slug が削除または misconfigure される regression を見逃す可能性がある。
|
||||
- Loader と linter の invariants がテスト上で完全には揃っていない。linter は `WORKFLOW_BODY_LIMIT` を enforce するが、`load_workflows` は現在 enforce していない。body size が runtime/load invariant の意図なら loader test がない。意図的に lint-only なら、その境界を明示したままにすべきである。
|
||||
- `requires` validation は linter と後段の `pod` invocation resolution でのみテストされており、`load_workflows` ではテストされていない。これは意図的かもしれないが、crate tests だけでは責務分担が明確ではない。
|
||||
- linter の Knowledge existence check は filename-stem ベースである。テストは malformed Knowledge files、invalid Knowledge filenames、または「一致する `.md` file が存在する」と「valid Knowledge record が存在する」の違いをカバーしていない。意図する invariant が「valid Knowledge record」なら、現在のテストは許容しすぎている。
|
||||
- 複数の hard-error paths が十分にテストされていない:
|
||||
- Workflow files の malformed YAML frontmatter
|
||||
- Workflow files の missing frontmatter
|
||||
- `SKILL.md` の missing `name` / missing `description`
|
||||
- `.yoi/workflow` 配下の non-`.md` files と subdirectories
|
||||
- Workflow description cap と body cap の exact boundary
|
||||
- `skill::tests::invalid_slug_name_is_error` は必要以上に緩い: fixture は uppercase の directory/name が一致しているため、期待される error は `InvalidName` であるべき。`NameDirMismatch` も許容すると、validation ordering や fixture の regression を隠してしまう。
|
||||
- `skill::tests::extra_frontmatter_fields_are_kept` は名前がやや不自然である: parsed `SkillRecord` はそれらの optional fields を保持しない。このテストが実際に assert しているのは「optional/spec-compatible fields are accepted and ignored」であり、それ自体は妥当だが、より直接的に表現すべきである。
|
||||
- crate-level tests は、「workspace Workflows を load し、configured directories から Skills を load し、registry に merge し、shadowed Skills を report する」という full pipeline を exercise していない。隣接する一部 behavior は `pod` でカバーされているが、`workflow` crate 自体では pieces を独立にテストしているだけである。
|
||||
|
||||
## 追加するとよいもの
|
||||
- builtin assertions を強化する:
|
||||
- 期待されるすべての builtin slugs が存在することを assert する
|
||||
- それらの `source`, `path`, `model_invokation`, `user_invocable`, および選択した `requires` values を assert する
|
||||
- builtin resource changes が明確に fail するよう、小さな contract test として維持する
|
||||
- Workflow loader の negative/edge tests を追加する:
|
||||
- missing frontmatter
|
||||
- malformed YAML
|
||||
- `model_invokation: true` のとき description cap ちょうどが accepted になること
|
||||
- non-`.md` files が ignored されること
|
||||
- subdirectories が ignored されること
|
||||
- workspace Workflow が `requires` values を preserve すること
|
||||
- body limit が lint-only なのか load-time でもあるのかをテストで明確にする
|
||||
- linter edge tests を追加する:
|
||||
- body が `WORKFLOW_BODY_LIMIT` ちょうどなら accepted
|
||||
- resident description が cap ちょうどなら accepted
|
||||
- 複数の `requires` がすべての missing references を report すること
|
||||
- invalid `requires` slug が malformed frontmatter または slug parse error になること
|
||||
- valid Knowledge parsing が意図されている場合、malformed Knowledge record behavior
|
||||
- Skill tests を引き締める:
|
||||
- matching invalid directory/name に対して invalid slug test が `SkillParseError::InvalidName` を assert するようにする
|
||||
- missing `name` と missing `description` を追加する
|
||||
- optional-field test を、それらの fields が accepted/ignored されることを表す名前または内容にする
|
||||
- parsed Skills からの registry assembly に対する crate-level integration-style unit test を 1 つ追加する:
|
||||
- builtin/workspace registry を load する
|
||||
- 2 つの Skill directories を parse する
|
||||
- priority order で merge する
|
||||
- accepted Skill、shadowed Skill、および結果の user-invocable/resident entries を assert する
|
||||
|
||||
## 実行したコマンド
|
||||
- `cargo test -p workflow`
|
||||
- 結果: passed
|
||||
- 34 unit tests passed, 0 failed; doc-tests: 0 tests
|
||||
- `cargo test -p workflow -- --list`
|
||||
- 結果: passed
|
||||
- 34 unit tests, 0 benchmarks を確認; doc-tests: 0 tests
|
||||
- `git status --short`
|
||||
- Read-only check; この review の変更外に既存の modified paths があることを示した:
|
||||
- `.yoi/tickets/00001KTVPS6K3/item.md`
|
||||
- `.yoi/tickets/00001KTVPS6K3/thread.md`
|
||||
- `crates/tui/src/multi_pod.rs`
|
||||
|
||||
Reference in New Issue
Block a user