docs(#148): add SKILL.md and dev-loop-spec.md for dispatch redesign #149
@@ -8,15 +8,15 @@ It lives in the repo so changes are version-controlled alongside the code.
|
||||
Dispatch is a **pure shell script** — no model reasoning.
|
||||
|
||||
```
|
||||
Cron (agentTurn, toolsAllow: [exec, sessions_spawn])
|
||||
Cron (agentTurn, toolsAllow: [exec, sessions_spawn, read])
|
||||
|
|
||||
→ runs dispatch script
|
||||
→ reads output for SPAWN or HANDOFF lines
|
||||
sonnet-review-bot
commented
[NIT] SKILL.md states the dispatch script 'exits after one SPAWN action per pass', but docs/dev-loop-spec.md (Rule 10) clarifies that multiple HANDOFFs can be emitted in one pass and the script does NOT exit after HANDOFF. The architecture diagram in SKILL.md is slightly misleading — it implies the script always exits after one action, when in reality it continues processing PRs after HANDOFF emissions. **[NIT]** SKILL.md states the dispatch script 'exits after one SPAWN action per pass', but docs/dev-loop-spec.md (Rule 10) clarifies that multiple HANDOFFs can be emitted in one pass and the script does NOT exit after HANDOFF. The architecture diagram in SKILL.md is slightly misleading — it implies the script always exits after one action, when in reality it continues processing PRs after HANDOFF emissions.
|
||||
→ spawns worker if instructed
|
||||
|
||||
Dispatch script (~/.openclaw/workspace/scripts/dev-loop-dispatch.sh)
|
||||
→ pure bash, all decisions are curl API calls + branches
|
||||
→ exits after one SPAWN action per pass
|
||||
→ may emit multiple HANDOFFs in one pass
|
||||
→ exits after emitting one SPAWN line (at most one worker per run)
|
||||
|
sonnet-review-bot
commented
[NIT] The architecture diagram in SKILL.md says the dispatch script 'exits after emitting one SPAWN line', but the spec (docs/dev-loop-spec.md §4) says HANDOFF lines continue without exiting, and the diagram itself also says 'emits HANDOFF for each qualifying PR (does not exit after HANDOFF)'. The 'exits' phrasing is slightly misleading — it only exits early for SPAWN, not HANDOFF. This is not wrong but could be clarified to 'exits after emitting one SPAWN line (but continues for HANDOFF)' or similar. **[NIT]** The architecture diagram in SKILL.md says the dispatch script 'exits after emitting one SPAWN line', but the spec (docs/dev-loop-spec.md §4) says HANDOFF lines continue without exiting, and the diagram itself also says 'emits HANDOFF for each qualifying PR (does not exit after HANDOFF)'. The 'exits' phrasing is slightly misleading — it only exits early for SPAWN, not HANDOFF. This is not wrong but could be clarified to 'exits after emitting one SPAWN line (but continues for HANDOFF)' or similar.
gpt-review-bot
commented
[MAJOR] SKILL.md states the dispatch script exits after emitting one SPAWN line, which conflicts with docs/dev-loop-spec.md that indicates the script continues evaluating remaining PRs and can emit HANDOFFs even after a SPAWN. Clarify whether the script should exit after the first SPAWN or continue scanning to emit HANDOFFs, and make both documents consistent. **[MAJOR]** SKILL.md states the dispatch script exits after emitting one SPAWN line, which conflicts with docs/dev-loop-spec.md that indicates the script continues evaluating remaining PRs and can emit HANDOFFs even after a SPAWN. Clarify whether the script should exit after the first SPAWN or continue scanning to emit HANDOFFs, and make both documents consistent.
|
||||
→ emits HANDOFF for each qualifying PR (does not exit after HANDOFF)
|
||||
|
||||
Workers (Opus, spawned by cron model)
|
||||
→ receive precise task description
|
||||
@@ -34,7 +34,7 @@ The model **never** assesses project state or makes dispatch decisions.
|
||||
3. **WIP mutex** — one active worker per repo; WIP label gates new issue pickup
|
||||
4. **One SPAWN per run** — script emits at most one SPAWN line per execution
|
||||
5. **set -euo pipefail** — any curl failure aborts immediately, no partial actions
|
||||
6. **Workers reply NO_REPLY** — no announcements, no handoffs, no side effects
|
||||
6. **Workers reply NO_REPLY** — no dispatch-level side effects (workers may push changes and manage labels as part of their task)
|
||||
|
gpt-review-bot
commented
[MINOR] Safety invariant says "Workers reply NO_REPLY — no announcements, no handoffs, no side effects". However, worker templates include pushing code and removing WIP labels, which are side effects. Suggest rephrasing to "no dispatch-level side effects (no announcements/handoffs); workers may push changes and manage labels as part of their task". **[MINOR]** Safety invariant says "Workers reply NO_REPLY — no announcements, no handoffs, no side effects". However, worker templates include pushing code and removing WIP labels, which are side effects. Suggest rephrasing to "no dispatch-level side effects (no announcements/handoffs); workers may push changes and manage labels as part of their task".
|
||||
|
||||
## Dispatch Rules (in order)
|
||||
|
sonnet-review-bot
commented
[NIT] The dispatch rules table omits Rule 1 (jumps from Rule 0/0b directly to Rule 2). If Rule 1 was intentionally removed or never existed, a brief note would prevent reader confusion about the numbering gap. **[NIT]** The dispatch rules table omits Rule 1 (jumps from Rule 0/0b directly to Rule 2). If Rule 1 was intentionally removed or never existed, a brief note would prevent reader confusion about the numbering gap.
|
||||
|
||||
|
sonnet-review-bot
commented
[NIT] The dispatch rules table in SKILL.md says Rule 10 condition is 'All checks pass' but the corresponding rule in docs/dev-loop-spec.md §5 (Rule 10) also requires verifying all bot reviews are current. The SKILL.md table is a summary so this simplification is acceptable, but a note like 'All checks pass + bot reviews current' would be more precise. **[NIT]** The dispatch rules table in SKILL.md says Rule 10 condition is 'All checks pass' but the corresponding rule in docs/dev-loop-spec.md §5 (Rule 10) also requires verifying all bot reviews are current. The SKILL.md table is a summary so this simplification is acceptable, but a note like 'All checks pass + bot reviews current' would be more precise.
|
||||
@@ -42,6 +42,7 @@ The model **never** assesses project state or makes dispatch decisions.
|
||||
|------|-----------|--------|
|
||||
| 0 | WIP label > 1hr old | Remove stale WIP, continue |
|
||||
| 0b | WIP label ≤ 1hr old | Mark ACTIVE_WIP=1, continue (only gates Rule 10) |
|
||||
| _(1)_ | _(reserved — intentionally unused)_ | — |
|
||||
| 2 | Any reviewer has REQUEST_CHANGES | SPAWN:findings |
|
||||
| 3 | PR not mergeable | SPAWN:rebase |
|
||||
| 4 | CI failure, no fix plan | SPAWN:ci-fix |
|
||||
@@ -99,6 +100,7 @@ Key fields:
|
||||
If no SPAWN line in output, reply NO_REPLY.
|
||||
|
||||
|
gpt-review-bot
commented
[MINOR] Cron config references a workspace path to SKILL.md ("~/.openclaw/workspace/skills/dev-loop/SKILL.md") but this PR adds SKILL.md at the repo root. Clarify how the repo doc is synchronized into the workspace path or update the reference to avoid confusion. **[MINOR]** Cron config references a workspace path to SKILL.md ("~/.openclaw/workspace/skills/dev-loop/SKILL.md") but this PR adds SKILL.md at the repo root. Clarify how the repo doc is synchronized into the workspace path or update the reference to avoid confusion.
|
||||
See ~/.openclaw/workspace/skills/dev-loop/SKILL.md for full instructions.
|
||||
(This repo's SKILL.md is deployed to that workspace path.)
|
||||
model: hai-anthropic/anthropic--claude-4.5-haiku
|
||||
toolsAllow: [exec, sessions_spawn, read]
|
||||
```
|
||||
|
||||
@@ -204,7 +204,7 @@ If all current:
|
||||
- Emit `HANDOFF:<pr_num>`
|
||||
- Continue evaluating remaining PRs (do NOT exit)
|
||||
|
||||
If already assigned to `aweiker`: skip.
|
||||
If already assigned to `aweiker`: skip (assume handoff was already performed; continue to next PR without emitting another HANDOFF).
|
||||
|
||||
### Rule 11: New Issue Pickup
|
||||
|
||||
@@ -219,7 +219,7 @@ Claim the issue (assign to bot user to prevent double-pick), then:
|
||||
|
||||
## 6. Safety Invariants
|
||||
|
||||
These are statically checked by `test/check-invariants.sh` and enforced in all changes:
|
||||
These are statically checked by `~/.openclaw/workspace/scripts/test/check-invariants.sh` and enforced in all changes:
|
||||
|
gpt-review-bot
commented
[MINOR] Path to invariants checker is inconsistent across the doc. Here it references **[MINOR]** Path to invariants checker is inconsistent across the doc. Here it references `test/check-invariants.sh`, while later (line 271) it references `scripts/test/check-invariants.sh`. Standardize these references (e.g., `~/.openclaw/workspace/scripts/test/check-invariants.sh`) for consistency.
|
||||
|
||||
| ID | Invariant |
|
||||
|----|-----------|
|
||||
@@ -268,7 +268,7 @@ Workers **always** remove the WIP label on completion and reply `NO_REPLY`.
|
||||
## 9. Fixes for Issues #144 and #145
|
||||
|
||||
**Issue #144** (autonomous merge):
|
||||
The dispatch script contains no merge API calls anywhere. The `scripts/test/check-invariants.sh`
|
||||
The dispatch script contains no merge API calls anywhere. The `~/.openclaw/workspace/scripts/test/check-invariants.sh`
|
||||
|
gpt-review-bot
commented
[NIT] Invariants checker path differs from earlier mention (line 222). Unify the path formatting to avoid ambiguity for readers. **[NIT]** Invariants checker path differs from earlier mention (line 222). Unify the path formatting to avoid ambiguity for readers.
|
||||
invariant `S1` verifies this. Workers do not receive merge instructions.
|
||||
|
||||
**Issue #145** (merged despite REQUEST_CHANGES):
|
||||
|
||||
[NIT] Architecture snippet shows toolsAllow: [exec, sessions_spawn] while the Cron Config later includes [exec, sessions_spawn, read]. Consider aligning these for consistency.
[NIT] Architecture snippet lists toolsAllow as [exec, sessions_spawn], while the Cron config farther below includes 'read' as well. Consider aligning the lists or adding a note that 'read' is also required.