[文書] DTP Playbook v2: 8 Phase → 3 Phase に圧縮
miyabi-core 既存資産(7,654行)を最大活用する方針に転換。 v1 の 1,780行新規 → v2 の 1,000行新規(既存 DAG/GitHub/承認/並列を流用)。 推定時間: 1〜1.5時間 → 25〜35分。 Phase A: gate.rs + lock.rs + store.rs + protocol.rs(4ファイル新規) Phase B: CLI サブコマンド追加(既存 main.rs に追加) Phase C: GitHub Evidence + E2E テスト Codex Round 2 レビュー 3件も保存。 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
146fcafc5e
commit
39a5180b8e
6 changed files with 1137 additions and 2 deletions
245
docs/dtp/PLAYBOOK-v2.md
Normal file
245
docs/dtp/PLAYBOOK-v2.md
Normal file
|
|
@ -0,0 +1,245 @@
|
|||
# DTP Playbook v2 — miyabi-cli-standalone 統合版
|
||||
|
||||
_v2: miyabi-core の既存資産を最大活用。DTP リポのゼロ実装を廃止。_
|
||||
|
||||
---
|
||||
|
||||
## North Star
|
||||
|
||||
> LLM の揺らぎを許さない。GATE が許可し、GitHub が確定し、merge が不可逆にする。
|
||||
|
||||
---
|
||||
|
||||
## 方針転換の理由
|
||||
|
||||
miyabi-core に DAG (823行), GitHub (947行), 承認 (231行), 並列実行 (451行), ワークフロー (889行), セッション永続化 (466行), エラーポリシー (536行) が **既に Rust で実装済み**。
|
||||
|
||||
DTP リポでゼロから書いていた Phase 2 (DAG), Phase 5 (GitHub), Phase 7 (CLI) の大部分は **不要**。
|
||||
|
||||
---
|
||||
|
||||
## 既存資産マップ(miyabi-core)
|
||||
|
||||
| ファイル | 行数 | DTP で使える API |
|
||||
|---------|------|-----------------|
|
||||
| `dag.rs` | 823 | TaskGraph, topological_sort, build_levels, TaskGraphBuilder |
|
||||
| `github.rs` | 467 | GitHubClient, get_issue, get_pull_request, close_issue, create_issue |
|
||||
| `github_tools.rs` | 480 | Tool registry, execute |
|
||||
| `orchestration.rs` | 451 | Orchestrator, ParallelConfig, execute_sequential |
|
||||
| `agent/approval.rs` | 231 | ApprovalCallback trait, AutoApproveAll, RejectHighRisk |
|
||||
| `workflow.rs` | 889 | Workflow, StepResult, StepStatus, WorkflowManager |
|
||||
| `error_policy.rs` | 536 | CircuitBreaker, FallbackStrategy |
|
||||
| `session.rs` | 466 | SessionStorage (save/load/list) — tasks.json 永続化の参考 |
|
||||
| `git.rs` | 211 | find_git_root, get_current_branch, has_uncommitted_changes |
|
||||
| `openclaw.rs` | 354 | OpenClawClient, get_agents |
|
||||
| `hooks.rs` | 862 | HookEvent, HookManager, execute |
|
||||
| `tool.rs` | 1359 | execute_dag, execute_parallel |
|
||||
| `config.rs` | 525 | 設定管理 |
|
||||
| **合計** | **7654** | |
|
||||
|
||||
---
|
||||
|
||||
## 新規追加するもの(4 ファイル / 約 400行)
|
||||
|
||||
```
|
||||
crates/miyabi-core/src/
|
||||
├── gate.rs (~100行) NEW: GATE 検証関数
|
||||
├── lock.rs (~100行) NEW: ファイルロック (lease + heartbeat)
|
||||
├── protocol.rs (~150行) NEW: DeterministicExecutionProtocol
|
||||
└── store.rs (~50行) NEW: tasks.json 永続化
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Phase 構成(v1 の 8 Phase → v2 の 3 Phase に圧縮)
|
||||
|
||||
### Phase A: GATE + Lock + Store(新規 4 ファイル)
|
||||
|
||||
**やること**:
|
||||
|
||||
- [ ] `src/gate.rs` を作成
|
||||
- `ensure_issue(task) -> Result<()>`: github_issue_number > 0
|
||||
- `ensure_dependencies_done(task, done_ids) -> Result<()>`: 全依存が done
|
||||
- `ensure_impact(task, human_approved) -> Result<()>`: impact 記録済 + HIGH→承認
|
||||
- `ensure_branch_name(name) -> Result<()>`: feature/issue-N-* 形式
|
||||
- `ensure_pr_ready(task) -> Result<()>`: branch + pr_number 存在
|
||||
- `ensure_merge_commit(sha) -> Result<()>`: 40文字 hex
|
||||
- [ ] `src/lock.rs` を作成
|
||||
- `FileLockManager`: acquire_lock, release_lock, has_conflict, release_expired
|
||||
- lease_duration + last_heartbeat ベース
|
||||
- `from_tasks()` で既存タスクからロック状態を復元
|
||||
- [ ] `src/store.rs` を作成
|
||||
- `TaskStore`: with_file(path), load, save (atomic rename), add_task, update_task, get_task
|
||||
- tasks.json スキーマ: version, tasks[], file_locks, dag_levels
|
||||
- session.rs の save/load パターンを参考に実装
|
||||
- [ ] `src/protocol.rs` を作成
|
||||
- `DeterministicExecutionProtocol`: 全 GATE を統合
|
||||
- register_task → check_dependencies → record_impact → assign_and_lock → record_branch → record_pr → record_merge
|
||||
- 各メソッドで GATE 検証 → 状態遷移 → store 永続化
|
||||
- [ ] `src/lib.rs` に `pub mod gate; pub mod lock; pub mod protocol; pub mod store;` を追加
|
||||
- [ ] テスト: happy path (register → ... → done)
|
||||
- [ ] テスト: GATE 拒否 (issue なし, impact なし, 不正 SHA, ロック競合)
|
||||
- [ ] テスト: HIGH risk → 承認なし → 拒否
|
||||
- [ ] `cargo test` GREEN
|
||||
- [ ] `cargo clippy --all-targets --all-features -- -D warnings` GREEN
|
||||
|
||||
**担当**: Codex A (worktree)
|
||||
**依存**: なし
|
||||
**GATE**: test GREEN + clippy GREEN + 8 テスト以上
|
||||
|
||||
**参照ファイル**:
|
||||
- `src/dag.rs` — TaskGraph の API(DAG 操作のインターフェース確認)
|
||||
- `src/session.rs` — SessionStorage の save/load パターン
|
||||
- `src/agent/approval.rs` — ApprovalCallback の承認パターン
|
||||
- `src/error.rs` — エラー型の定義パターン
|
||||
|
||||
---
|
||||
|
||||
### Phase B: CLI サブコマンド追加
|
||||
|
||||
**やること**:
|
||||
|
||||
- [ ] `src/main.rs`(既存 CLI エントリ)に DTP サブコマンドを追加
|
||||
- `miyabi-cli gate register --issue N --title T`
|
||||
- `miyabi-cli gate status [task-id]`
|
||||
- `miyabi-cli gate assign <task-id> --agent A --node N --files F`
|
||||
- `miyabi-cli gate impact <task-id> --risk R --symbols N`
|
||||
- `miyabi-cli gate branch <task-id> <name>`
|
||||
- `miyabi-cli gate pr <task-id> <number>`
|
||||
- `miyabi-cli gate merge <task-id> <sha>`
|
||||
- `miyabi-cli gate locks`
|
||||
- `miyabi-cli gate dag`
|
||||
- `miyabi-cli gate dispatchable`
|
||||
- [ ] `--format json` 対応(全コマンド)
|
||||
- [ ] 終了コード: 0=成功, 1=GATE拒否, 2=入力不正
|
||||
- [ ] `--store-path` オプション(tasks.json のパス指定)
|
||||
- [ ] テスト: register → status で task 表示
|
||||
- [ ] `cargo test` GREEN
|
||||
- [ ] `cargo clippy` GREEN
|
||||
|
||||
**担当**: Codex B (worktree)
|
||||
**依存**: Phase A GREEN
|
||||
**GATE**: register + status が動作 + JSON 出力
|
||||
|
||||
**参照ファイル**:
|
||||
- `src/main.rs`(既存 CLI 構造)
|
||||
- 既存のサブコマンド実装パターン
|
||||
|
||||
---
|
||||
|
||||
### Phase C: GitHub Evidence + E2E
|
||||
|
||||
**やること**:
|
||||
|
||||
- [ ] `src/github.rs` の既存 `get_pull_request()` を使って merge 証跡を取得するラッパー追加
|
||||
- `fetch_merge_evidence(pr_number) -> Result<MergeEvidence>`
|
||||
- `MergeEvidence { merge_commit_sha, merged_at, state, head_ref }`
|
||||
- [ ] protocol.rs に `verify_merge()` を追加(record_merge を置換)
|
||||
- GitHub API 経由で PR merged を検証
|
||||
- 障害時は `AwaitingGithubSync` 的なエラーを返す
|
||||
- [ ] protocol.rs に escape hatch 追加
|
||||
- `force_unlock(task_id, reason)`
|
||||
- `manual_complete(task_id, reason)`
|
||||
- [ ] E2E テスト:
|
||||
1. register → check_dependencies → record_impact → assign_and_lock
|
||||
2. record_branch → record_pr → verify_merge(mock)
|
||||
3. status → Done
|
||||
- [ ] `cargo test` GREEN
|
||||
- [ ] `cargo clippy` GREEN
|
||||
- [ ] `npx miyabi bus record-run --skill dtp --result success`
|
||||
|
||||
**担当**: Claude Code (local) — GitHub API アクセスが必要
|
||||
**依存**: Phase A + B GREEN
|
||||
**GATE**: E2E テスト GREEN + clippy GREEN
|
||||
|
||||
**参照ファイル**:
|
||||
- `src/github.rs` — GitHubClient (既存)
|
||||
- `src/error_policy.rs` — CircuitBreaker (障害時フォールバック)
|
||||
- `src/hooks.rs` — HookEvent (audit hooks)
|
||||
|
||||
---
|
||||
|
||||
## Phase DAG
|
||||
|
||||
```
|
||||
Phase A (gate + lock + store + protocol)
|
||||
│
|
||||
├──→ Phase B (CLI サブコマンド)
|
||||
│ │
|
||||
└────────┤
|
||||
▼
|
||||
Phase C (GitHub Evidence + E2E)
|
||||
```
|
||||
|
||||
Phase A と B は **A 完了後に B 開始**(直列)。
|
||||
Phase C は **A + B 完了後に開始**。
|
||||
|
||||
---
|
||||
|
||||
## 時間見積もり
|
||||
|
||||
| Phase | 新規行 | テスト行 | 時間 |
|
||||
|-------|--------|---------|------|
|
||||
| A | 400 | 200 | 15分 |
|
||||
| B | 150 | 50 | 10分 |
|
||||
| C | 100 | 100 | 10分 |
|
||||
| **合計** | **650** | **350** | **35分** |
|
||||
|
||||
Codex 並列なら **A(15分) → B+C 並列(10分) = 約 25分**。
|
||||
|
||||
---
|
||||
|
||||
## v1 → v2 の差分
|
||||
|
||||
| 項目 | v1 (8 Phase) | v2 (3 Phase) |
|
||||
|------|-------------|-------------|
|
||||
| 新規コード量 | 1,780行 | 1,000行 |
|
||||
| Phase 数 | 8 | 3 |
|
||||
| DAG 実装 | 新規 | **不要**(dag.rs 823行あり) |
|
||||
| GitHub 実装 | 新規 | **不要**(github.rs 947行あり) |
|
||||
| 承認実装 | 新規 | **不要**(approval.rs 231行あり) |
|
||||
| 並列実行 | 新規 | **不要**(orchestration.rs 451行あり) |
|
||||
| CLI 構造 | 新規 | **不要**(main.rs の既存構造に追加) |
|
||||
| 推定時間 | 1〜1.5時間 | **25〜35分** |
|
||||
|
||||
---
|
||||
|
||||
## Codex レビュー反映状況
|
||||
|
||||
| 指摘 | v2 での対応 |
|
||||
|------|-----------|
|
||||
| R1-1: conditions 未評価 | Phase A: gate.rs で検証関数を独立実装、protocol.rs が唯一の窓口 |
|
||||
| R1-3: 出自検証なし | Phase C: github.rs 経由で merge 証跡を検証 |
|
||||
| R1-5: TTL 固定 | Phase A: lock.rs に lease_duration + last_heartbeat |
|
||||
| R2-1: atomic write | Phase A: store.rs で atomic rename |
|
||||
| R2-2: TOCTOU | Phase A: lock.rs の acquire で再チェック |
|
||||
| R3-1: SSOT 昇格しすぎ | tasks.json は execution ledger、GitHub が authority |
|
||||
| R3-4: merge commit 取得経路 | Phase C: github.rs の get_pull_request() で merge_commit_sha 取得 |
|
||||
| R3-5: escape hatch | Phase C: force_unlock, manual_complete |
|
||||
| Playbook R2: Phase 依存不完全 | v2 で 3 Phase に圧縮、依存が明確 |
|
||||
| Playbook R2: Phase gate 不足 | 各 Phase に GATE (test + clippy) を明記 |
|
||||
|
||||
---
|
||||
|
||||
## ロールバックポイント
|
||||
|
||||
| Phase | タグ | 戻り方 |
|
||||
|-------|------|--------|
|
||||
| A 完了 | `v0.1-dtp-phase-a` | `git checkout v0.1-dtp-phase-a` |
|
||||
| B 完了 | `v0.2-dtp-phase-b` | `git checkout v0.2-dtp-phase-b` |
|
||||
| C 完了 | `v1.0-dtp-complete` | `git checkout v1.0-dtp-complete` |
|
||||
|
||||
---
|
||||
|
||||
## 引き継ぎ
|
||||
|
||||
Phase 間の引き継ぎは `autorun/HANDOFF.md` のプロシージャに従う。
|
||||
v2 では Phase が 3 つだけなので、引き継ぎポイントは 2 箇所:
|
||||
|
||||
1. Phase A → Phase B: gate/lock/store/protocol が動く状態を確認
|
||||
2. Phase B → Phase C: CLI が動く状態を確認
|
||||
|
||||
---
|
||||
|
||||
_This is the single source of truth for DTP implementation in miyabi-cli-standalone._
|
||||
_Do not start coding until this Playbook is approved._
|
||||
276
docs/dtp/reviews/round2-code.md
Normal file
276
docs/dtp/reviews/round2-code.md
Normal file
|
|
@ -0,0 +1,276 @@
|
|||
# Round 2 Code Review: Deterministic Task Protocol Rust Codebase
|
||||
|
||||
対象依頼:
|
||||
- Read all Rust source files in `src/`: `lib.rs`, `types.rs`, `state.rs`, `dag.rs`, `lock.rs`, `store.rs`, `protocol.rs`, `gate.rs`, `main.rs`
|
||||
- Check:
|
||||
1. GATE conditions completeness/correctness
|
||||
2. State machine coverage vs Playbook
|
||||
3. DAG correctness
|
||||
4. Soundness / error handling / logic bugs
|
||||
5. `cargo test` / `cargo clippy` and test sufficiency
|
||||
|
||||
作業場所:
|
||||
- Workspace root: `/Users/shunsukehayashi/dev/ops/openclaw-workspace`
|
||||
- Relevant existing implementation found: `Miyabi/packages/task-manager/`
|
||||
- Relevant playbook found: `docs/design/deterministic-protocol-playbook.md`
|
||||
|
||||
## Executive Summary
|
||||
|
||||
現ワークスペースでは、依頼で指定された Rust codebase は見つかりませんでした。少なくともこの repo 配下には:
|
||||
|
||||
- `Cargo.toml`
|
||||
- `src/lib.rs`
|
||||
- `src/types.rs`
|
||||
- `src/state.rs`
|
||||
- `src/dag.rs`
|
||||
- `src/lock.rs`
|
||||
- `src/store.rs`
|
||||
- `src/protocol.rs`
|
||||
- `src/gate.rs`
|
||||
- `src/main.rs`
|
||||
|
||||
が存在しません。
|
||||
|
||||
代わりに存在するのは TypeScript 実装:
|
||||
|
||||
- `Miyabi/packages/task-manager/src/task-manager.ts`
|
||||
- `Miyabi/packages/task-manager/src/types/task.ts`
|
||||
- `Miyabi/packages/task-manager/src/state/task-state-machine.ts`
|
||||
|
||||
および、未実装 Playbook:
|
||||
|
||||
- `docs/design/deterministic-protocol-playbook.md`
|
||||
|
||||
です。
|
||||
|
||||
したがって、依頼された Rust コードそのものに対する (1)〜(5) の完全レビューは **実施不能** です。以下では、その理由、現存実装との乖離、実行した `cargo` コマンドの結果を記録します。
|
||||
|
||||
## What Was Actually Found
|
||||
|
||||
### Found
|
||||
|
||||
- Playbook: `docs/design/deterministic-protocol-playbook.md`
|
||||
- TypeScript task manager package: `Miyabi/packages/task-manager/src/...`
|
||||
|
||||
### Not Found
|
||||
|
||||
- Rust crate root (`Cargo.toml`)
|
||||
- Requested Rust source files under any `src/`
|
||||
- `gate.rs`, `protocol.rs`, `dag.rs`, `lock.rs`, `store.rs` as Rust modules
|
||||
|
||||
## Command Evidence
|
||||
|
||||
### Source tree inspection
|
||||
|
||||
`Miyabi/packages/task-manager/src` contains only TypeScript files/directories:
|
||||
|
||||
- `index.ts`
|
||||
- `task-manager.ts`
|
||||
- `types/*.ts`
|
||||
- `state/task-state-machine.ts`
|
||||
- `execution/*.ts`
|
||||
- `sync/*.ts`
|
||||
- `decomposition/*.ts`
|
||||
|
||||
No Rust files were present there.
|
||||
|
||||
### cargo test
|
||||
|
||||
Executed in:
|
||||
|
||||
- `/Users/shunsukehayashi/dev/ops/openclaw-workspace/Miyabi/packages/task-manager`
|
||||
|
||||
Result:
|
||||
|
||||
```text
|
||||
error: could not find `Cargo.toml` in `/Users/shunsukehayashi/dev/ops/openclaw-workspace/Miyabi/packages/task-manager` or any parent directory
|
||||
```
|
||||
|
||||
### cargo clippy
|
||||
|
||||
Executed in:
|
||||
|
||||
- `/Users/shunsukehayashi/dev/ops/openclaw-workspace/Miyabi/packages/task-manager`
|
||||
|
||||
Result:
|
||||
|
||||
```text
|
||||
error: could not find `Cargo.toml` in `/Users/shunsukehayashi/dev/ops/openclaw-workspace/Miyabi/packages/task-manager` or any parent directory
|
||||
```
|
||||
|
||||
## Review Outcome by Requested Question
|
||||
|
||||
## 1. Are the GATE conditions in `gate.rs` complete and correct?
|
||||
|
||||
**Unable to review directly** because `gate.rs` does not exist in this workspace.
|
||||
|
||||
What can be said from the Playbook:
|
||||
|
||||
- The playbook defines GATE semantics conceptually in `docs/design/deterministic-protocol-playbook.md`
|
||||
- But there is no Rust `gate.rs` implementation here to verify completeness, ordering, or enforcement
|
||||
|
||||
What can be said from the existing TypeScript implementation:
|
||||
|
||||
- There is no equivalent `gate.ts` module
|
||||
- Existing state conditions are only strings on transition rules in `task-state-machine.ts`
|
||||
- Those conditions are not evaluated by `canTransition()` or `applyTransition()`
|
||||
|
||||
Implication:
|
||||
|
||||
- In the currently present code, GATE semantics are not implemented as executable guards
|
||||
- So if the intended Rust crate exists elsewhere, it has not been checked in here
|
||||
|
||||
## 2. Does the state machine in `state.rs` cover all transitions from the Playbook?
|
||||
|
||||
**Unable to review directly** because `state.rs` does not exist.
|
||||
|
||||
However, comparing the Playbook to the existing TypeScript state machine shows a clear mismatch:
|
||||
|
||||
Playbook expects:
|
||||
|
||||
- `merged` state
|
||||
- `awaiting_github_sync` state
|
||||
- stronger gate-driven transitions around review/merge/finalization
|
||||
|
||||
Existing TypeScript state machine has only:
|
||||
|
||||
- `draft`
|
||||
- `pending`
|
||||
- `analyzing`
|
||||
- `implementing`
|
||||
- `reviewing`
|
||||
- `deploying`
|
||||
- `done`
|
||||
- `blocked`
|
||||
- `failed`
|
||||
- `cancelled`
|
||||
|
||||
Missing vs Playbook:
|
||||
|
||||
- `merged`
|
||||
- `awaiting_github_sync`
|
||||
|
||||
Also, the TypeScript state machine still includes transitions incompatible with the newer deterministic plan:
|
||||
|
||||
- `pending -> implementing` with `skip_analysis`
|
||||
- `reviewing -> done` without an explicit `merged` state
|
||||
|
||||
So if `state.rs` was meant to implement the Playbook, that implementation is not present here, and the nearest existing implementation is not yet aligned with the Playbook.
|
||||
|
||||
## 3. Is the DAG implementation in `dag.rs` correct?
|
||||
|
||||
**Unable to review directly** because `dag.rs` does not exist.
|
||||
|
||||
No Rust DAG implementation matching the requested file layout was found in this workspace.
|
||||
|
||||
There is also no TypeScript `dag` module currently present under `Miyabi/packages/task-manager/src/` matching the deterministic protocol plan. The playbook describes one, but it is still plan-only in this repo.
|
||||
|
||||
Therefore:
|
||||
|
||||
- topological sort correctness: not reviewable
|
||||
- cycle detection correctness: not reviewable
|
||||
- dispatch semantics: not reviewable
|
||||
|
||||
## 4. Are there any soundness issues, missing error handling, or logic bugs?
|
||||
|
||||
For the requested Rust codebase: **not reviewable because the codebase is absent**.
|
||||
|
||||
For the nearest existing TypeScript implementation, there are already serious soundness gaps:
|
||||
|
||||
### A. Transition conditions are declarative only, not enforced
|
||||
|
||||
In `Miyabi/packages/task-manager/src/state/task-state-machine.ts`:
|
||||
|
||||
- transition rules contain `conditions?: string[]`
|
||||
- `canTransition(from, to)` checks only `from -> to`
|
||||
- `applyTransition()` does not evaluate those conditions
|
||||
|
||||
Consequence:
|
||||
|
||||
- `review_approved`, `dependency_blocked`, `skip_analysis`, etc. are currently metadata, not executable gates
|
||||
|
||||
### B. Updated task objects are returned but not persisted by callers
|
||||
|
||||
`applyTransition()` returns `{ valid, task }`, but the calling code does not consistently write the updated task back into storage.
|
||||
|
||||
Example:
|
||||
|
||||
- `TaskManager.updateTaskState()` returns `result.valid` only
|
||||
- it does not replace the stored task with `result.task`
|
||||
|
||||
Consequence:
|
||||
|
||||
- state transition may be "accepted" but not materially stored
|
||||
|
||||
### C. No deterministic gate/protocol module exists yet
|
||||
|
||||
The playbook assumes:
|
||||
|
||||
- protocol as the only transition entry point
|
||||
- lock manager
|
||||
- store
|
||||
- gate module
|
||||
- event log + snapshot
|
||||
|
||||
None of those implementations are present in the current package.
|
||||
|
||||
### D. No Rust test surface exists for the requested deterministic protocol
|
||||
|
||||
Since the Rust crate is absent, there is no way to validate:
|
||||
|
||||
- gate edge cases
|
||||
- stale lease handling
|
||||
- lock race recovery
|
||||
- cycle detection
|
||||
- GitHub evidence verification
|
||||
|
||||
## 5. Does cargo test pass and are the tests sufficient?
|
||||
|
||||
### `cargo test`
|
||||
|
||||
No. It does not run because there is no Rust crate in the inspected location.
|
||||
|
||||
### `cargo clippy`
|
||||
|
||||
No. Same reason.
|
||||
|
||||
### Test sufficiency
|
||||
|
||||
For the requested Rust crate: **not assessable**.
|
||||
|
||||
For the existing TypeScript package:
|
||||
|
||||
- there are tests under `Miyabi/packages/task-manager/tests/`
|
||||
- but they are not Rust tests
|
||||
- and they cannot establish correctness of a nonexistent Rust deterministic protocol implementation
|
||||
|
||||
So the direct answer to the user’s question is:
|
||||
|
||||
- `cargo test` does not pass because there is no Cargo project here
|
||||
- test sufficiency for the requested Rust codebase cannot be evaluated because that codebase is absent
|
||||
|
||||
## Concrete Conclusion
|
||||
|
||||
This round cannot complete the requested Rust code review because the Rust codebase described in the prompt is not present in the workspace.
|
||||
|
||||
The strongest defensible review result is:
|
||||
|
||||
1. The requested Rust files do not exist in this repo/workspace.
|
||||
2. `cargo test` and `cargo clippy` fail immediately due to missing `Cargo.toml`.
|
||||
3. The only nearby implementation is TypeScript, and it is still pre-deterministic:
|
||||
- no gate module
|
||||
- no DAG module for this protocol
|
||||
- no lock/store/protocol modules
|
||||
- state conditions not enforced
|
||||
- state updates not reliably persisted
|
||||
4. The Playbook is a plan, not an implemented Rust system in this workspace.
|
||||
|
||||
## Recommended Next Step
|
||||
|
||||
If the intended Rust codebase lives elsewhere, provide one of:
|
||||
|
||||
- the exact crate path
|
||||
- the repository path containing `Cargo.toml`
|
||||
- the branch/worktree where `src/gate.rs`, `src/protocol.rs`, etc. exist
|
||||
|
||||
Once that path is available, the requested full code review can be performed precisely.
|
||||
418
docs/dtp/reviews/round2-openclaw.md
Normal file
418
docs/dtp/reviews/round2-openclaw.md
Normal file
|
|
@ -0,0 +1,418 @@
|
|||
# Review: OpenClaw Agent Integration for DTP
|
||||
|
||||
## Executive Summary
|
||||
|
||||
現状の `deterministic-task-protocol` は、OpenClaw `main` エージェントが自律的に `draft -> done` を回すにはまだ入口が足りません。最大の理由は 3 つです。
|
||||
|
||||
1. `dtp` バイナリに実用 CLI がまだなく、`src/main.rs` は単なる bootstrap 出力しか持っていません。[`src/main.rs:1`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/src/main.rs#L1)
|
||||
2. protocol 実装は in-memory happy path に留まっており、GitHub 証跡検証、lease heartbeat、reconcile、event log、JSON 出力、終了コード、identity 付きの運用コマンドが未実装です。[`src/protocol.rs:53`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/src/protocol.rs#L53) [`src/store.rs:47`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/src/store.rs#L47)
|
||||
3. repo 直下の `AGENTS.md` は Rust 開発ガイドとしては十分ですが、OpenClaw main がこの repo でどう振る舞うかは書かれていません。逆に `refs/AGENTS.md` と `refs/SOUL.md` は広すぎて、この repo 専用の実行契約になっていません。[`AGENTS.md:1`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/AGENTS.md#L1) [`refs/AGENTS.md:1`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/refs/AGENTS.md#L1) [`refs/SOUL.md:56`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/refs/SOUL.md#L56)
|
||||
|
||||
Playbook 側はかなり正しく、OpenClaw integration の要件もおおむね言語化できています。特に authority-aware sync、`awaiting_github_sync`、lease + heartbeat、escape hatch、CLI subcommands は妥当です。[`docs/PLAYBOOK.md:46`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/docs/PLAYBOOK.md#L46) [`docs/PLAYBOOK.md:456`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/docs/PLAYBOOK.md#L456) [`docs/PLAYBOOK.md:526`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/docs/PLAYBOOK.md#L526) [`docs/PLAYBOOK.md:747`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/docs/PLAYBOOK.md#L747) [`autorun/phase-7-cli/TASKS.md:8`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/autorun/phase-7-cli/TASKS.md#L8)
|
||||
|
||||
以下、質問ごとに整理します。
|
||||
|
||||
## 1. Can an OpenClaw agent (main) call the DTP CLI to execute the full protocol? What is missing?
|
||||
|
||||
### Short answer
|
||||
|
||||
まだできません。
|
||||
|
||||
### Why not
|
||||
|
||||
- `dtp` CLI が未実装です。`src/main.rs` は `"dtp: deterministic-task-protocol bootstrap"` を出すだけです。[`src/main.rs:1`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/src/main.rs#L1)
|
||||
- protocol の public API は Rust から直接呼ぶ前提のメソッド群だけで、OpenClaw main が shell command として安全に使える interface になっていません。[`src/protocol.rs:62`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/src/protocol.rs#L62)
|
||||
- `TaskStore::with_file()` はあるものの、protocol の `new()` は default in-memory store を使うだけで、CLI/daemon から永続ストアを注入する導線がありません。[`src/protocol.rs:54`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/src/protocol.rs#L54) [`src/store.rs:53`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/src/store.rs#L53)
|
||||
- merge 検証は `record_merge(task_id, merge_commit)` に文字列を渡せば通る設計で、GitHub API からの出自検証がありません。OpenClaw agent が勝手に SHA を渡せてしまいます。[`src/protocol.rs:183`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/src/protocol.rs#L183) [`src/gate.rs:72`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/src/gate.rs#L72)
|
||||
- lock は固定 TTL のみで heartbeat/lease renew がありません。cron から lease 維持もできません。[`src/types.rs:26`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/src/types.rs#L26) [`src/lock.rs:52`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/src/lock.rs#L52)
|
||||
- state model がまだ `merged` / `awaiting_github_sync` / `manual` 系を持っていません。OpenClaw 運用ではここが重要です。[`src/types.rs:7`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/src/types.rs#L7) [`docs/PLAYBOOK.md:211`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/docs/PLAYBOOK.md#L211)
|
||||
- output contract が未定義です。OpenClaw main が自律運転するには JSON 出力と機械判定可能な exit code が必須ですが、現状ありません。[`autorun/phase-7-cli/TASKS.md:28`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/autorun/phase-7-cli/TASKS.md#L28)
|
||||
|
||||
### Minimum missing pieces before OpenClaw can drive it
|
||||
|
||||
1. 実 CLI 実装
|
||||
2. 永続 store path の指定
|
||||
3. GitHub credentials / repo context の指定
|
||||
4. JSON 出力
|
||||
5. 明確な exit code
|
||||
6. identity-aware lock ownership
|
||||
7. heartbeat renew / stale sweep
|
||||
8. GitHub evidence fetch + reconcile
|
||||
9. manual/external completion path
|
||||
10. audit hooks
|
||||
|
||||
## 2. How should `CLAUDE.md` and `AGENTS.md` be structured for this repo so agents know the rules?
|
||||
|
||||
### Current problem
|
||||
|
||||
- repo 直下 `AGENTS.md` は開発者向けで、OpenClaw/Codex/Claude がこの repo で何を守るべきかの運用契約が不足しています。[`AGENTS.md:16`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/AGENTS.md#L16)
|
||||
- `refs/AGENTS.md` は openclaw-workspace 全体の map、`refs/SOUL.md` は OpenClaw main の人格/優先順位です。どちらも大事ですが、この repo 専用の「DTPをどう運転するか」までは書いていません。[`refs/AGENTS.md:6`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/refs/AGENTS.md#L6) [`refs/SOUL.md:58`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/refs/SOUL.md#L58)
|
||||
- 現状 `.claude/` と `.codex/` にガイドファイルがありません。これは repo-specific behavior を agents に教える機会を逃しています。
|
||||
|
||||
### Recommended structure
|
||||
|
||||
#### `AGENTS.md` at repo root
|
||||
|
||||
役割: repo-local execution contract。全エージェント共通の「この repo ではどう動くか」を簡潔に書く。
|
||||
|
||||
入れるべき内容:
|
||||
|
||||
1. Repo mission
|
||||
2. SSOT split
|
||||
3. Mandatory startup reads
|
||||
4. Allowed / forbidden transitions
|
||||
5. Required commands before and after edits
|
||||
6. DTP-specific invariants
|
||||
7. CLI-first operation rules
|
||||
8. Testing and verification contract
|
||||
9. Links to `refs/AGENTS.md` / `refs/SOUL.md` / `docs/PLAYBOOK.md`
|
||||
|
||||
サンプル見出し:
|
||||
|
||||
- `Purpose`
|
||||
- `Read First`
|
||||
- `Fact vs Execution Ledger`
|
||||
- `If You Modify Protocol`
|
||||
- `If You Run as OpenClaw Main`
|
||||
- `Required Verification`
|
||||
- `Escape Hatches`
|
||||
|
||||
特に重要な文言:
|
||||
|
||||
- `GitHub is fact SSOT; local snapshot is execution ledger.`
|
||||
- `No agent may mark a task done without GitHub evidence or an audited manual/external completion path.`
|
||||
- `All automation must prefer dtp CLI over direct JSON edits.`
|
||||
- `Heartbeat ownership must match agent@node.`
|
||||
|
||||
#### `CLAUDE.md`
|
||||
|
||||
役割: Claude Code / OpenClaw-style operator brief。対話型エージェントや main agent 向けに「どの順で読むか」「どう dispatch するか」を書く。
|
||||
|
||||
入れるべき内容:
|
||||
|
||||
1. Session startup for this repo
|
||||
2. `docs/PLAYBOOK.md` is normative for implementation order
|
||||
3. `autorun/*/TASKS.md` and `GATE.md` drive phase execution
|
||||
4. When acting as OpenClaw main, prefer orchestration and delegation, not direct code hoarding
|
||||
5. When acting on user message, direct answer still wins per `refs/SOUL.md`
|
||||
6. Exact `dtp` commands for autonomous operation
|
||||
7. Escalation points
|
||||
|
||||
推奨順序:
|
||||
|
||||
1. `refs/SOUL.md`
|
||||
2. repo `AGENTS.md`
|
||||
3. `docs/PLAYBOOK.md`
|
||||
4. relevant `autorun/phase-*/TASKS.md`
|
||||
5. relevant handoff note
|
||||
|
||||
### Concrete recommendation
|
||||
|
||||
- `AGENTS.md`: durable, cross-agent, repo-local invariant
|
||||
- `CLAUDE.md`: runtime/operator workflow for Claude/OpenClaw style agents
|
||||
- `refs/AGENTS.md` / `refs/SOUL.md`: inherited upstream context only
|
||||
|
||||
つまり、`refs/*` を読むだけでは不十分で、この repo 自身の `AGENTS.md` と `CLAUDE.md` に DTP運転ルールを持つべきです。
|
||||
|
||||
## 3. What is the minimal CLI interface needed for an agent to autonomously run draft-to-done?
|
||||
|
||||
### Truly minimal autonomous set
|
||||
|
||||
Playbook の一覧は広めですが、OpenClaw main が自律的に `draft -> done` を回すための最小集合はこれです。
|
||||
|
||||
1. `dtp register`
|
||||
2. `dtp dispatchable`
|
||||
3. `dtp impact`
|
||||
4. `dtp approve`
|
||||
5. `dtp assign`
|
||||
6. `dtp heartbeat`
|
||||
7. `dtp branch`
|
||||
8. `dtp pr`
|
||||
9. `dtp verify-merge`
|
||||
10. `dtp confirm-done`
|
||||
11. `dtp status --json`
|
||||
|
||||
### Why each is needed
|
||||
|
||||
- `register`: Issue と completion mode を固定する
|
||||
- `dispatchable`: main が次に実行すべき task を判定する
|
||||
- `impact`: GATE 3 を通す
|
||||
- `approve`: HIGH/CRITICAL の人間承認を監査付きで残す
|
||||
- `assign`: lock と owner identity を確定する
|
||||
- `heartbeat`: 実装中 lease を維持する
|
||||
- `branch`: 実在 branch を state と結びつける
|
||||
- `pr`: PR identity を task に紐づける
|
||||
- `verify-merge`: GitHub authority から merged 事実を pull する
|
||||
- `confirm-done`: completion mode に応じて最終確定する
|
||||
- `status --json`: orchestration loop が機械判定する
|
||||
|
||||
### Additional commands that are near-minimal in production
|
||||
|
||||
OpenClaw 運用まで考えると、次の 4 つも事実上必要です。
|
||||
|
||||
1. `dtp unlock --reason --operator`
|
||||
2. `dtp reconcile`
|
||||
3. `dtp sweep-leases`
|
||||
4. `dtp events --json`
|
||||
|
||||
### Output contract
|
||||
|
||||
各コマンドに最低限必要な contract:
|
||||
|
||||
- `--json` support
|
||||
- stable schema
|
||||
- deterministic exit code
|
||||
- no interactive prompt by default
|
||||
|
||||
推奨 exit code:
|
||||
|
||||
- `0`: success
|
||||
- `1`: gate rejected
|
||||
- `2`: invalid input
|
||||
- `3`: conflict / lease lost
|
||||
- `4`: GitHub unavailable / degraded
|
||||
- `5`: invariant violation / bug
|
||||
|
||||
### Missing from current implementation
|
||||
|
||||
- `src/main.rs` に clap CLI なし
|
||||
- `approve` コマンド相当なし
|
||||
- `status` / `dispatchable` / `reconcile` / `sweep-leases` なし
|
||||
- JSON output なし
|
||||
- store path / repo context / operator identity flags なし
|
||||
|
||||
## 4. How should heartbeat/lease renewal work when called from a cron job?
|
||||
|
||||
### Current gap
|
||||
|
||||
現状は TTL だけで、lease renew 機構がありません。[`src/types.rs:33`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/src/types.rs#L33) [`src/lock.rs:52`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/src/lock.rs#L52)
|
||||
|
||||
Playbook は lease 300s + heartbeat 60s + missed heartbeats 2 を提案していて、これは妥当です。[`docs/PLAYBOOK.md:40`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/docs/PLAYBOOK.md#L40) [`docs/PLAYBOOK.md:467`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/docs/PLAYBOOK.md#L467)
|
||||
|
||||
### Recommended model
|
||||
|
||||
#### Lease data
|
||||
|
||||
`TaskLock` should carry:
|
||||
|
||||
- `locked_by`
|
||||
- `locked_at`
|
||||
- `lease_duration_sec`
|
||||
- `heartbeat_interval_sec`
|
||||
- `last_heartbeat`
|
||||
- `fencing_token`
|
||||
- `affected_files`
|
||||
|
||||
### Cron interaction model
|
||||
|
||||
cron からは 2 系統に分けるべきです。
|
||||
|
||||
1. owner cron: lease renew
|
||||
2. janitor cron: stale lease sweep
|
||||
|
||||
#### Owner renew
|
||||
|
||||
コマンド例:
|
||||
|
||||
```bash
|
||||
dtp heartbeat <task-id> --agent main --node mainmini --json
|
||||
```
|
||||
|
||||
必要な gate:
|
||||
|
||||
- task exists
|
||||
- task is in `implementing` or other active state
|
||||
- lock exists
|
||||
- lock owner matches `agent@node`
|
||||
- lease is not already fenced off by newer owner
|
||||
|
||||
成功時:
|
||||
|
||||
- `last_heartbeat` 更新
|
||||
- `lock_heartbeat` event 記録
|
||||
- new `expires_at` を JSON 出力
|
||||
|
||||
失敗時:
|
||||
|
||||
- exit `3` if lock ownership lost
|
||||
- exit `1` if task not heartbeat-eligible
|
||||
- exit `4` if store or GitHub degraded path relevant
|
||||
|
||||
#### Janitor sweep
|
||||
|
||||
コマンド例:
|
||||
|
||||
```bash
|
||||
dtp sweep-leases --json
|
||||
```
|
||||
|
||||
責務:
|
||||
|
||||
- stale lease を検出
|
||||
- forced release を event として記録
|
||||
- task state を `blocked` or `pending_recovery` 相当に移す
|
||||
- owner mismatch を報告
|
||||
|
||||
### Important rule
|
||||
|
||||
cron は「とりあえず更新」してはいけません。owner identity を必ず指定し、`main@mainmini` が保持する lock を `main@mainmini` の cron だけが renew できるようにすべきです。
|
||||
|
||||
### Recommended timings
|
||||
|
||||
- lease duration: 300s
|
||||
- heartbeat interval: 60s
|
||||
- stale after: 2 missed intervals
|
||||
- janitor sweep: every 2-5 min
|
||||
|
||||
### Why cron needs both renew and sweep
|
||||
|
||||
OpenClaw main は long-running conversation とは限らず、cron や heartbeat runner に lease 維持を委譲する場面があります。そのため:
|
||||
|
||||
- active owner renews
|
||||
- janitor reclaims abandoned leases
|
||||
|
||||
の二面構成が必要です。renew だけだと orphan lock が残り、sweep だけだと稼働中タスクを誤解放します。
|
||||
|
||||
## 5. What escape hatches are needed for production operation?
|
||||
|
||||
### Required escape hatches
|
||||
|
||||
1. `force-unlock`
|
||||
2. `reconcile-from-github`
|
||||
3. `manual-complete`
|
||||
4. `external-op-complete`
|
||||
5. `reopen`
|
||||
6. `takeover-lock`
|
||||
7. `mark-awaiting-github-sync`
|
||||
8. `retry-github-sync`
|
||||
9. `abandon-task`
|
||||
10. `supersede-task`
|
||||
|
||||
### Why they are needed
|
||||
|
||||
- agent crash
|
||||
- lock orphaning
|
||||
- GitHub outage
|
||||
- merge happened externally
|
||||
- no-PR legitimate work
|
||||
- human intervention in production
|
||||
|
||||
### Minimum audited fields for every escape hatch
|
||||
|
||||
- `operator`
|
||||
- `reason`
|
||||
- `approved_by`
|
||||
- `timestamp`
|
||||
- `previous_state`
|
||||
- `new_state`
|
||||
- `correlation_id`
|
||||
|
||||
### Specific recommendations
|
||||
|
||||
#### `force-unlock`
|
||||
|
||||
必要条件:
|
||||
|
||||
- operator required
|
||||
- reason required
|
||||
- current lock holder and age recorded
|
||||
|
||||
#### `manual-complete`
|
||||
|
||||
使いどころ:
|
||||
|
||||
- docs-only
|
||||
- external ops
|
||||
- emergency human completion
|
||||
|
||||
必要条件:
|
||||
|
||||
- `completion_mode in {manual, external-op}`
|
||||
- human approval present
|
||||
- audit event recorded
|
||||
|
||||
#### `reconcile-from-github`
|
||||
|
||||
使いどころ:
|
||||
|
||||
- PR merged externally
|
||||
- issue closed externally
|
||||
- webhook missed
|
||||
|
||||
これは production では特に重要です。OpenClaw main は drift 修正のために自律的に reconcile できる必要があります。
|
||||
|
||||
#### `takeover-lock`
|
||||
|
||||
クラッシュ復旧で重要です。`force-unlock` と `assign` を別々にやるより、
|
||||
|
||||
- old owner stale confirmed
|
||||
- operator approves takeover
|
||||
- new fencing token issued
|
||||
|
||||
を一発でやる方が安全です。
|
||||
|
||||
## Additional Findings
|
||||
|
||||
### A. Current `AGENTS.md` is too generic for autonomous agents
|
||||
|
||||
repo root の `AGENTS.md` は Rust 開発ガイドとしては問題ありませんが、agent execution protocol が不足しています。OpenClaw main はこの文書を読んでも、
|
||||
|
||||
- どの docs を先に読むべきか
|
||||
- `refs/*` をどう扱うか
|
||||
- `dtp` をどう叩くか
|
||||
- いつ人間に escalate すべきか
|
||||
|
||||
が分かりません。[`AGENTS.md:63`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/AGENTS.md#L63)
|
||||
|
||||
### B. `refs/AGENTS.md` and `refs/SOUL.md` should remain references, not runtime-only truth
|
||||
|
||||
symlink 自体は良いです。upstream workspace のルールと main の人格を持ち込めます。[`refs/AGENTS.md:6`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/refs/AGENTS.md#L6) [`refs/SOUL.md:1`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/refs/SOUL.md#L1)
|
||||
|
||||
ただし、この repo の運用契約まで symlink 先に依存すべきではありません。repo local の `AGENTS.md` / `CLAUDE.md` で上書きすべきです。
|
||||
|
||||
### C. Playbook already implies the right CLI, but implementation lags far behind
|
||||
|
||||
Playbook と autorun task は、OpenClaw integration の設計書としてかなり使えます。特に:
|
||||
|
||||
- `heartbeat`
|
||||
- `verify-merge`
|
||||
- `confirm-done`
|
||||
- `unlock`
|
||||
- `reconcile`
|
||||
- `run-e2e`
|
||||
|
||||
はそのまま CLI contract にすべきです。[`docs/PLAYBOOK.md:781`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/docs/PLAYBOOK.md#L781) [`autorun/phase-7-cli/TASKS.md:10`](/Users/shunsukehayashi/dev/platform/_core/miyabi-private/deterministic-task-protocol/autorun/phase-7-cli/TASKS.md#L10)
|
||||
|
||||
## Recommended Next Moves
|
||||
|
||||
1. Implement real `dtp` CLI in `src/main.rs` with clap and JSON output first.
|
||||
2. Add repo-local `CLAUDE.md` focused on OpenClaw/Codex operation order.
|
||||
3. Expand repo-local `AGENTS.md` from generic Rust guidance to execution contract.
|
||||
4. Add persistent store path + identity flags to every mutating command.
|
||||
5. Implement `heartbeat` and `sweep-leases` before trying autonomous OpenClaw runs.
|
||||
6. Implement `verify-merge` and `reconcile` before allowing `confirm-done`.
|
||||
7. Add audited escape hatches before production use.
|
||||
|
||||
## Direct Answers
|
||||
|
||||
### (1) Can OpenClaw main call the dtp CLI for the full protocol?
|
||||
|
||||
まだ無理です。`dtp` CLI 自体が未実装で、GitHub verification、heartbeat、JSON output、persistent store、escape hatch が足りません。
|
||||
|
||||
### (2) How should `CLAUDE.md` and `AGENTS.md` be structured?
|
||||
|
||||
- `AGENTS.md`: repo-local invariant and execution contract
|
||||
- `CLAUDE.md`: runtime/operator workflow for Claude/OpenClaw style agents
|
||||
- `refs/*`: inherited upstream context only
|
||||
|
||||
### (3) Minimal CLI for autonomous draft-to-done?
|
||||
|
||||
`register`, `dispatchable`, `impact`, `approve`, `assign`, `heartbeat`, `branch`, `pr`, `verify-merge`, `confirm-done`, `status --json` が最小です。運用上は `unlock`, `reconcile`, `sweep-leases`, `events --json` もほぼ必須です。
|
||||
|
||||
### (4) How should heartbeat/lease renewal work from cron?
|
||||
|
||||
owner cron による `heartbeat` と janitor cron による `sweep-leases` の二系統。identity match と fencing token が必要です。lease 300s / heartbeat 60s / stale after 2 misses を推奨します。
|
||||
|
||||
### (5) What escape hatches are needed?
|
||||
|
||||
`force-unlock`, `reconcile-from-github`, `manual-complete`, `external-op-complete`, `reopen`, `takeover-lock`, `mark-awaiting-github-sync`, `retry-github-sync`, `abandon-task`, `supersede-task` が必要です。全て audit fields 必須です。
|
||||
196
docs/dtp/reviews/round2-playbook.md
Normal file
196
docs/dtp/reviews/round2-playbook.md
Normal file
|
|
@ -0,0 +1,196 @@
|
|||
# Auto Run Playbook Review Round 2
|
||||
|
||||
対象レビューの前提:
|
||||
- 指定された `docs/autorun/00`〜`08`, `docs/PLAYBOOK.md`, `docs/PLAN-v1.md` はこのワークスペースでは見つかりませんでした。
|
||||
- そのため、Phase 0〜8 と Codex 3-body 反映がまとまっている実質的な対象として `[deterministic-protocol-playbook.md](/Users/shunsukehayashi/dev/ops/openclaw-workspace/docs/design/deterministic-protocol-playbook.md)` を主レビュー対象にし、元草案として `/Users/shunsukehayashi/.claude/plans/snuggly-bouncing-turtle.md`、既存レビューとして `KOTOWARI/reviews/102-104` を補助参照しました。
|
||||
- 以下はその前提での detailed review です。
|
||||
|
||||
## Findings
|
||||
|
||||
### 1. Critical: Phase dependency DAG が本文と矛盾しており、Phase 4 の依存が不足しています
|
||||
|
||||
- 依存関係表では Phase 4 は Phase 3 のみ依存です (`deterministic-protocol-playbook.md:864-867`)。
|
||||
- しかし本文では Phase 4 の `FileLockManager` は `EventStore` と `SnapshotStore` に依存し (`:473-478`)、さらに Phase 1 で追加した `TaskLock` 型にも依存します (`:177-184`)。
|
||||
- これは表で `4 | 3` と書かれているのが不完全で、実質は `4 | 1,3` です。
|
||||
- 同様に Phase 5 も表では `3` のみ依存ですが (`:866`)、`GitHubEvidence` や `CompletionMode` は Phase 1 で入る型に依存しています (`:196-209`, `:223-249`)。
|
||||
|
||||
影響:
|
||||
- 自動オーケストレータが依存表だけを見て実行すると、未定義型のまま Phase 4/5 を先行開始し得ます。
|
||||
- 並列実行時の「着手可能判定」が誤るため、Playbook の DAG Integrity を損ねます。
|
||||
|
||||
推奨:
|
||||
- 依存表を以下に修正してください。
|
||||
- Phase 4 → `1,3`
|
||||
- Phase 5 → `1,3`
|
||||
- Phase 6 → `1,2,3,4,5`
|
||||
- 依存 DAG は prose ではなく machine-readable にも持つべきです。少なくとも `phase_id`, `depends_on`, `artifacts_produced`, `artifacts_required` を JSON/YAML で定義した方がよいです。
|
||||
|
||||
### 2. Critical: approval gate が「Phase 遷移」を止める設計になっておらず、個別 API gate と Playbook gate が分離しています
|
||||
|
||||
- 文書全体では「Do not start coding until Phase 0 is GREEN」とありますが (`deterministic-protocol-playbook.md:921`)、Phase 1 以降を止める機械的 gate は Phase 完了条件の文章だけです (`:163`, `:259`, `:364`, `:452`, `:522`, `:605`, `:777`, `:806`, `:844`)。
|
||||
- Phase 6 の GATE 群は task lifecycle 用であり (`:627-762`)、Playbook lifecycle 用ではありません。つまり `registerTask` や `assignAndLock` の gate はある一方、「Phase 3 が未完了なら Phase 4 の実装を開始できない」を enforce する仕組みが書かれていません。
|
||||
|
||||
影響:
|
||||
- 自律エージェントが「コードを書き始める/次 Phase へ進む」判定を自然言語で行ってしまう
|
||||
- Playbook の自律実行で premature transition が起きる
|
||||
|
||||
推奨:
|
||||
- Phase ごとに明示的な `approvalGate` を追加してください。
|
||||
- 最低限必要な gate:
|
||||
- `phase0_gate`: 全テスト GREEN と index fresh の実コマンド結果を保存済み
|
||||
- `phaseN_exit_gate`: 生成ファイル、テスト、レビュー項目、ログ記録が揃っている
|
||||
- `phaseN_to_phaseN+1_gate`: 前 Phase の exit gate artifact を検証済み
|
||||
- これを CLI でも `miyabi playbook phase-approve <phase>` のように機械可読化するのが安全です。
|
||||
|
||||
### 3. High: retry 条件が failure mode 網羅になっていません
|
||||
|
||||
- 文書に retry が明示されるのは主に GitHub API degraded mode だけです (`deterministic-protocol-playbook.md:580-590`, `:602-603`, `:775`)。
|
||||
- しかし failure mode はもっと多いです。
|
||||
|
||||
未定義または不十分な failure mode:
|
||||
- CAS conflict 発生時の retry/backoff 方針がない (`:433-437`)
|
||||
- `flock` 取得失敗・lock file 破損時の retry がない (`:428-445`, `:480-488`)
|
||||
- event append 失敗時の retry / fail-fast 条件がない (`:393-399`)
|
||||
- snapshot rebuild が壊れたときの fallback がない (`:418-420`, `:844`)
|
||||
- stale lease 解放後に元エージェントが遅延 heartbeat を送った場合の fence token がない (`:503-505`, `:692-695`)
|
||||
- GitHub evidence fetch は retry があるが、GraphQL の closingIssuesReferences 部分の partial failure 方針がない (`:557-560`)
|
||||
- `recordBranch`, `recordPR`, `verifyMerge`, `confirmDone` で idempotency が定義されていない (`:698-744`)
|
||||
|
||||
推奨:
|
||||
- 各 Phase に「retry matrix」を追加してください。
|
||||
- 各 API で少なくとも次を定義:
|
||||
- retryable / non-retryable
|
||||
- max attempts
|
||||
- backoff
|
||||
- idempotency key
|
||||
- terminal state
|
||||
- lock 系は heartbeat だけでなく fencing token を入れるべきです。lease を再取得した新オーナーより古い heartbeat は無効化できないと race が残ります。
|
||||
|
||||
### 4. High: Codex 3-body review 反映は前進しているが、未反映の運用タスクがまだあります
|
||||
|
||||
反映済み:
|
||||
- soft deps を level から外す (`deterministic-protocol-playbook.md:43-45`, `:357-360`)
|
||||
- event log + snapshot (`:45`, `:368-452`)
|
||||
- GitHub evidence (`:46-50`, `:536-560`)
|
||||
- completionMode (`:50`, `:208-209`, `:738-740`, `:759-761`)
|
||||
|
||||
未反映または不足:
|
||||
- review source の traceability がない
|
||||
- R1/R2/R3 のどのレビュー文書・誰の判断に基づくかを artifact として残していない。表 (`:34-50`) だけでは実装後の監査に弱いです。
|
||||
- single-writer fallback の実運用手順がない
|
||||
- Windows/SMB で advisory lock 非保証と書いているだけで (`:899`)、誰が writer になるか、フェイルオーバー手順、二重 writer 検出がありません。
|
||||
- queue/lock 連携の migration task がない
|
||||
- 既存 `agent-skill-bus` と `task-manager` のどちらが lock authority になるか、切替手順がないです。これは earlier review の coordination 論点に対して未着手です。
|
||||
- approval/audit hook の統合タスクがない
|
||||
- Phase 8 で `record-run` はありますが (`:833`)、Phase 単位のレビュー承認記録、human approval の永続化フロー、announce/notification は Playbook に未定義です。
|
||||
- protocol bypass hardening task がない
|
||||
- 検証項目には「Protocol 以外の経路で applyTransition を呼ぶテスト」がありますが (`:911`)、実際に `applyTransition` を private 化・export しない・store 直書き禁止 lint を入れる等の implementation task が相当 Phase にありません。
|
||||
|
||||
### 5. High: autonomous execution にはまだ曖昧さが残っています
|
||||
|
||||
曖昧な点:
|
||||
- 成果物の exact artifact が Phase ごとに固定されていない
|
||||
- 例えば Phase 2 は何ファイルが追加されれば done か曖昧です (`:270-364`)。
|
||||
- 誰が human approval を出せるか不明
|
||||
- `approvedBy` はあるが authority が未定義です (`:243-249`, `:671-675`)。
|
||||
- branch existence check の実行コンテキストが不明
|
||||
- `git branch --list` はどの repo/worktree で走るか未指定です (`:700-703`)。
|
||||
- `current HEAD` / `current file set` の基準が曖昧
|
||||
- `recordImpact()` の gate が repo root / branch / worktree のどれを指すか不明です (`:661-665`)。
|
||||
- GitHub linked issue 判定の repo 境界が不明
|
||||
- mono-repo / cross-repo PR の扱いが未定義です (`:557-560`, `:736-740`)。
|
||||
|
||||
結論:
|
||||
- 現状のままでは「理解している人間」なら進められますが、「Playbook だけ読んだ agent が曖昧さなく自律実行」はまだ厳しいです。
|
||||
|
||||
## Direct Answers
|
||||
|
||||
### 1. Are the Phase dependencies correct?
|
||||
|
||||
部分的には正しいですが、不完全です。
|
||||
|
||||
正しい点:
|
||||
- Phase 6 が 2/3/4/5 の統合に依存するのは妥当です (`deterministic-protocol-playbook.md:867`)。
|
||||
- Phase 7 → Phase 8 の流れも妥当です (`:868-869`)。
|
||||
|
||||
誤りまたは不足:
|
||||
- Phase 4 は実質 `1,3` 依存です。
|
||||
- Phase 5 は実質 `1,3` 依存です。
|
||||
- 並列可能の記述「Phase 2 は Phase 3/4 と並行可能」は、Phase 6 で統合する前提では正しいものの、型変更が Phase 2/3/4 に跨るので merge coordination task が必要です (`:888-890`)。
|
||||
|
||||
### 2. Are the approval gates sufficient to prevent premature Phase transitions?
|
||||
|
||||
いいえ。task gate はあるが phase gate は足りません。
|
||||
|
||||
不足している gate:
|
||||
- Phase start gate
|
||||
- Phase exit artifact verification
|
||||
- reviewer/human approval authority
|
||||
- protocol bypass prevention before later phases start
|
||||
- migration completion gate before switching authority from old queue/lock to new protocol
|
||||
|
||||
### 3. Are the retry conditions complete for each failure mode?
|
||||
|
||||
いいえ。不完全です。
|
||||
|
||||
特に不足:
|
||||
- CAS conflict retry
|
||||
- lock acquisition retry and fencing
|
||||
- late heartbeat handling
|
||||
- event append failure
|
||||
- snapshot corruption/rebuild fallback
|
||||
- GitHub partial evidence failure
|
||||
- idempotent re-run of `recordPR`, `verifyMerge`, `confirmDone`
|
||||
|
||||
### 4. Is any Phase missing tasks that should be there based on the Codex 3-body reviews in docs/reviews/?
|
||||
|
||||
はい。少なくとも次が抜けています。
|
||||
|
||||
- Phase 2 or 3:
|
||||
- protocol bypass hardening task
|
||||
- transition API visibility restriction
|
||||
- Phase 3 or 4:
|
||||
- single-writer / fallback writer election task
|
||||
- fence token implementation
|
||||
- Phase 5:
|
||||
- legacy authority migration plan from existing queue/lock
|
||||
- GitHub evidence partial-failure handling
|
||||
- Phase 8:
|
||||
- phase-level approval/audit recording
|
||||
- crash recovery / replay test
|
||||
- idempotent rerun test
|
||||
|
||||
補足:
|
||||
- `KOTOWARI/reviews/102-104` は直接この Playbook のレビューではありませんが、そこに共通している教訓は「SSOT 不明」「計画を止める gate が弱い」「CI/計測がないまま完了条件だけ測定可能として書かれている」です (`KOTOWARI/reviews/102-storage-layer-review.md`, `KOTOWARI/reviews/103-testing-strategy-review.md`, `KOTOWARI/reviews/104-implementation-timeline-review.md`)。この弱点は現 Playbook にも残っています。
|
||||
|
||||
### 5. Can an agent execute these Playbooks autonomously without ambiguity?
|
||||
|
||||
まだ完全には無理です。
|
||||
|
||||
理由:
|
||||
- 依存が表と実装本文でずれる
|
||||
- Phase gate が機械可読でない
|
||||
- retry matrix が不足
|
||||
- authority/operator の定義が不足
|
||||
- repo/worktree/repo-owner 境界が曖昧
|
||||
|
||||
人間の伴走ありなら実行可能ですが、完全自律化には次が必要です。
|
||||
|
||||
## Recommended Changes Before Auto Run
|
||||
|
||||
1. `playbook.yaml` を追加し、各 Phase に `depends_on`, `inputs`, `outputs`, `approval_gate`, `retry_policy`, `owner`, `commands` を持たせる。
|
||||
2. 依存表を修正する。
|
||||
- Phase 4 → `1,3`
|
||||
- Phase 5 → `1,3`
|
||||
- Phase 6 → `1,2,3,4,5`
|
||||
3. Phase gate を CLI 化する。
|
||||
- 例: `miyabi playbook check-phase 3`, `miyabi playbook approve-phase 3`
|
||||
4. retry matrix を各 API/Phase に追加する。
|
||||
5. fence token と single-writer fallback を明文化する。
|
||||
6. legacy queue/lock から新 protocol への authority cutover task を追加する。
|
||||
7. crash recovery / replay / idempotent rerun を Phase 8 の必須テストにする。
|
||||
|
||||
## Bottom Line
|
||||
|
||||
この Playbook は初回草案よりかなり改善されており、Codex 3-body review の主要論点は多く反映されています。
|
||||
ただし「自律実行できる Auto Run Playbook」として見ると、まだ task gate の設計が中心で、phase orchestration gate・retry completeness・authority migration が不足しています。現状評価は「設計としては strong、Auto Run 用ハーネスとしては未完成」です。
|
||||
Loading…
Add table
Add a link
Reference in a new issue