From 1094facb1e33d2bb3a62cd3ce5d66f5f465d926c Mon Sep 17 00:00:00 2001 From: Harald Hoyer Date: Fri, 8 May 2026 15:39:22 +0200 Subject: [PATCH] refactor(opencode): use $1 substitution, drop BASE_BRANCH and arg parsing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The workflow command was asking the model to parse $ARGUMENTS into positional tokens (issue ID, optional base branch). Opencode supports $1, $2, $3, ... for direct positional substitution at template-load time — the model never needs to see or parse a joined argument string. Two simplifications: 1. Replace $ARGUMENTS-parsing with $1. Every reference to the issue ID in commands/workflow.md is now $1, which opencode substitutes literally before the prompt loads. Eliminates a class of parsing errors (whitespace edge cases, mis-splits, hallucinated extra args) and removes the orchestrator's need to "remember" an ISSUE_ID variable across phases. 2. Drop BASE_BRANCH entirely. It was used in three places: - Phase 1 "branch != base" check — actual concern is "don't run on a protected branch." Replace with refusal on main/master/develop/ any matching protected name. - Phase 8 `git diff "$BASE_BRANCH"...HEAD` — anchor the diff to START_SHA captured at Phase 1 instead. With one-task-per-run (ADR-21), the run produces a small bounded diff from a known starting point; START_SHA is more accurate than diffing against a separate branch tip that may have moved. - Failure Handler recovery procedure — user-facing instructions; name "your usual integration branch" instead of $BASE_BRANCH. The command signature collapses from `/workflow [base-branch]` to just `/workflow ` — single positional, zero parsing. Routing matrix Phase 1 row updated for the protected-branch refusal; ADR-14's recovery-procedure paragraph no longer names BASE_BRANCH. Refs: config/opencode/workflow-design.md ADR-14, ADR-21 --- config/opencode/commands/workflow.md | 52 +++++++++++++--------------- config/opencode/workflow-design.md | 4 +-- 2 files changed, 26 insertions(+), 30 deletions(-) diff --git a/config/opencode/commands/workflow.md b/config/opencode/commands/workflow.md index e5379ae..b2edbaa 100644 --- a/config/opencode/commands/workflow.md +++ b/config/opencode/commands/workflow.md @@ -10,11 +10,11 @@ You are executing the multi-agent workflow inside the worktree this opencode ses - `opencode` was launched from the root of that worktree - A `TODO/` directory is committed to the repo with a per-issue tracker (schema in `agents/pm.md`) and a `TODO/README.md` index. The orchestrator does not read or construct per-issue paths — `@pm` is the only agent that touches issue files (ADR-22). -**Task reference:** $ARGUMENTS +**Issue ID for this run:** `$1` -If `$ARGUMENTS` is empty, stop immediately: "Usage: `/workflow [base-branch]` (e.g. `/workflow ABC-1`). The ID must already be tracked under `TODO/` (`@pm` validates existence at Phase 2). Base branch defaults to `main` (then `master`)." +If `$1` is empty, stop immediately: "Usage: `/workflow ` (e.g. `/workflow ABC-1`). The ID must already be tracked under `TODO/` (`@pm` validates existence at Phase 2)." -Parse `$ARGUMENTS`: the first whitespace-separated token is the issue ID, an optional second token overrides the base branch. Store as `ISSUE_ID`. +Opencode substitutes `$1` literally everywhere in this file before the prompt loads, so every reference below is the actual issue ID — the orchestrator does not parse the argument. --- @@ -56,7 +56,7 @@ The orchestrator writes plan and task-spec artifacts to a per-run directory in t └── summary.md # Phase 9 output (the run summary) ``` -Define `RUN_DIR="$WORKTREE_PATH/.workflow/run-$ISSUE_ID"` once in Phase 1 and reference it everywhere downstream. Create the directory in Phase 3 (`mkdir -p "$RUN_DIR"`). +Define `RUN_DIR="$WORKTREE_PATH/.workflow/run-$1"` once in Phase 1 and reference it everywhere downstream. Create the directory in Phase 3 (`mkdir -p "$RUN_DIR"`). **Authoring rules:** - Files are written by the orchestrator, never by subagents. @@ -78,24 +78,20 @@ This phase covers **only** git/worktree-shaped sanity. **TODO-tracker validation 2. Capture the worktree path: `WORKTREE_PATH="$(pwd)"`. 3. Verify HEAD is not detached: `git symbolic-ref --short HEAD` must succeed. If it fails, stop: "Cannot run on a detached HEAD. Check out a feature branch first." 4. Capture the current branch: `BRANCH_NAME="$(git symbolic-ref --short HEAD)"`. -5. Resolve the base branch (`BASE_BRANCH`): - - If `$ARGUMENTS` provided a second token, use it. - - Else if `git rev-parse --verify --quiet main` succeeds, use `main`. - - Else if `git rev-parse --verify --quiet master` succeeds, use `master`. - - Else stop: "Could not determine base branch (no `main` or `master`). Pass it as the second argument: `/workflow `." -6. Verify the current branch is not the base branch: if `BRANCH_NAME == BASE_BRANCH`, stop: "Cannot run workflow on the base branch (`$BASE_BRANCH`). Switch to a feature branch first." -7. **Verify the working tree is clean** (ADR-20): `git status --porcelain` must return empty. If not, stop: "Working tree must be clean. Commit or stash uncommitted changes before running the workflow." -8. Set the run-artifacts directory: `RUN_DIR="$WORKTREE_PATH/.workflow/run-$ISSUE_ID"`. Phase 3 will `mkdir -p "$RUN_DIR"` before writing the first artifact. +5. **Refuse to run on a protected branch.** If `BRANCH_NAME` matches `main`, `master`, `develop`, or any other branch the user treats as a long-lived integration branch, stop: "Cannot run workflow on `$BRANCH_NAME`. Switch to a feature branch first." The intent is to prevent accidentally committing on top of an integration branch — adjust the protected-branch list if your repo uses a different convention. +6. **Verify the working tree is clean** (ADR-20): `git status --porcelain` must return empty. If not, stop: "Working tree must be clean. Commit or stash uncommitted changes before running the workflow." +7. **Capture the run's anchor SHA:** `START_SHA="$(git rev-parse HEAD)"`. Phase 8 uses this to compute the run's diff (`git diff "$START_SHA"..HEAD`) — anchored to where this run started, not to a separate base-branch tip that may have moved. +8. Set the run-artifacts directory: `RUN_DIR="$WORKTREE_PATH/.workflow/run-$1"`. Phase 3 will `mkdir -p "$RUN_DIR"` before writing the first artifact. 9. Initialize the run-level rework counter: `PLAN_REWORK_REMAINING=1` (per ADR-13). Decrement on every P5.5-BLOCK→P4, P7-escalation-exhaustion→P3, and P8-plan-level→P3 transition. When the counter is `0` and another such transition fires, abort to the Failure Handler instead of re-entering. --- ## Phase 2: Issue Context -Dispatch `@pm` with the issue ID `$ISSUE_ID`, `$WORKTREE_PATH`, and `Validate run prerequisites` as the operation. **The orchestrator does not assume any path under the worktree's `TODO/` tree exists** — it asks `@pm` to: +Dispatch `@pm` with the issue ID `$1`, `$WORKTREE_PATH`, and `Validate run prerequisites` as the operation. **The orchestrator does not assume any path under the worktree's `TODO/` tree exists** — it asks `@pm` to: 1. Verify the TODO tracker is well-formed in this worktree (directory + index file present). -2. Locate the issue file for `$ISSUE_ID`. +2. Locate the issue file for `$1`. 3. Verify all `depends-on:` entries in the issue's frontmatter resolve to issues with `status: Done` (ADR-21 / ADR-22). 4. Return one of two structured responses: @@ -361,7 +357,7 @@ The dispatch prompt names: ### File sibling tasks as sub-issues (when N > 1) -After Phase 5.5 returns ACCEPTABLE, dispatch `@pm` to file each of `task-2.md` through `task-N.md` as a TODO sub-issue with `parent: $ISSUE_ID`. **Only task-1 continues into Phase 6.** Each filed sub-issue gets a rich seed body (ADR-21) so its eventual `/workflow` run can plan and implement without seeing siblings or the original `plan.md`. +After Phase 5.5 returns ACCEPTABLE, dispatch `@pm` to file each of `task-2.md` through `task-N.md` as a TODO sub-issue with `parent: $1`. **Only task-1 continues into Phase 6.** Each filed sub-issue gets a rich seed body (ADR-21) so its eventual `/workflow` run can plan and implement without seeing siblings or the original `plan.md`. For each task `$N` in 2…N, dispatch `@pm` with the following body content (assembled by the orchestrator from `task-.md` and the relevant slice of `plan.md`): @@ -385,13 +381,13 @@ For each task `$N` in 2…N, dispatch `@pm` with the following body content (ass .md or plan.md if present> --- -Discovered during run on `$BRANCH_NAME` for parent issue `$ISSUE_ID`. +Discovered during run on `$BRANCH_NAME` for parent issue `$1`. ``` `@pm` invocation per sub-issue: - Title — derived from `task-.md`'s task description (short imperative). - Status — `Todo`. -- Parent — `$ISSUE_ID`. +- Parent — `$1`. - Labels — propagate relevant labels from the parent (e.g. `gameplay`); add `split-from-run` to mark the provenance. - `depends-on:` — sibling sub-issue IDs that this task requires to be `Done` first. The orchestrator determines the dependency graph from the integration contracts captured in Phase 5.5 question 4. @@ -506,7 +502,7 @@ The Failure Handler's recovery procedure (ADR-14: discard worktree, delete branc Concretely on `split_needed`: 1. Write a Failure Handler summary noting `@check`'s diagnosis verbatim and the Phase 5 split that was attempted. -2. Dispatch `@pm` (operation: `Add comment`, issue ID: `$ISSUE_ID`) with the comment text: `- YYYY-MM-DD — split_needed at Phase 7 task-1; . Re-run after re-creating the worktree.` `@pm` resolves the issue file path itself; the orchestrator never constructs it. +2. Dispatch `@pm` (operation: `Add comment`, issue ID: `$1`) with the comment text: `- YYYY-MM-DD — split_needed at Phase 7 task-1; . Re-run after re-creating the worktree.` `@pm` resolves the issue file path itself; the orchestrator never constructs it. 3. Stop execution. Do not commit code, do not file new sub-issues, do not stage anything under `.workflow/`. --- @@ -518,7 +514,7 @@ Apply **Dispatch Hygiene** to each reviewer prompt before sending. Dispatch `@ch Provide reviewers with: - The absolute path to `$RUN_DIR/plan.md` (the same file Phase 4 reviewed; mid-loop revisions will have updated it in place). - The absolute path to `$RUN_DIR/task-1.md` (the spec the implementation actually targeted). -- The full diff (`git diff "$BASE_BRANCH"...HEAD`). +- The full diff for this run (`git diff "$START_SHA"..HEAD`) — anchored at the SHA captured at Phase 1, so the diff is exactly what this run produced. - Any decisions or deviations from the plan, captured inline in the dispatch prompt. - **NOT_TESTABLE manifest (ADR-18):** if task-1 went `NOT_TESTABLE` at Phase 6, the dispatch prompt includes a "Tasks completed without tests (NOT_TESTABLE)" section listing the `@test` justification and the `@check` sign-off rationale. Reviewers explicitly evaluate "does the justification still hold given the final diff?" and may BLOCK if it doesn't. If task-1 had tests (the common case), this section reads "None — task-1 has tests." @@ -550,7 +546,7 @@ The workflow is forge-agnostic. It commits locally and stops. **Do not push, and ### TODO Update -Dispatch `@pm` with the issue ID `$ISSUE_ID` and the following operations (a single dispatch can carry all of them — see `agents/pm.md` for the request shape): +Dispatch `@pm` with the issue ID `$1` and the following operations (a single dispatch can carry all of them — see `agents/pm.md` for the request shape): 1. **Check off the AC the run satisfied.** Pass the list of AC indices or texts (from the `acceptance_criteria` array `@pm` returned at Phase 2) that the implemented work fulfilled. The orchestrator decides which AC are satisfied by inspecting task-1's spec and verification output. `@pm` flips the corresponding `- [ ]` to `- [x]`. 2. **Set the issue's `status` based on AC completion** (ADR-21, AC-driven): @@ -563,7 +559,7 @@ Dispatch `@pm` with the issue ID `$ISSUE_ID` and the following operations (a sin ### File Follow-ups -Tracked-worthy unresolved items must become real TODO issues; otherwise they vanish into the per-run `summary.md` and the user (who has walked away) never sees them. Before writing the summary, scan the run for items in these categories and dispatch `@pm` to file each as a **sub-issue of the current issue** (`parent: $ISSUE_ID`). +Tracked-worthy unresolved items must become real TODO issues; otherwise they vanish into the per-run `summary.md` and the user (who has walked away) never sees them. Before writing the summary, scan the run for items in these categories and dispatch `@pm` to file each as a **sub-issue of the current issue** (`parent: $1`). | Source | New issue label | Title style | |---|---|---| @@ -577,7 +573,7 @@ Tracked-worthy unresolved items must become real TODO issues; otherwise they van - Anything already covered by an existing TODO issue (`@pm` lists existing issues; check the title/description before filing a duplicate). **Routing rules:** -- Each new issue is a sub-issue (`parent: $ISSUE_ID`). `@pm` will add it to the parent's `## Sub-issues` list automatically. The user can promote it to top-level later if it deserves its own slot. +- Each new issue is a sub-issue (`parent: $1`). `@pm` will add it to the parent's `## Sub-issues` list automatically. The user can promote it to top-level later if it deserves its own slot. - Issue body must include a "Discovered during" paragraph naming the run's branch and (where relevant) commit SHA, plus enough context for the user to triage it later without having to re-read the run. - Status: `Todo`. Default labels per the table; the orchestrator may add additional labels inferred from the parent (e.g. propagate `gameplay` from GAL-39 to a gameplay-relevant follow-up). - The Run Summary (next subsection) lists each filed follow-up by ID so the user has one place to see them. @@ -586,8 +582,8 @@ Tracked-worthy unresolved items must become real TODO issues; otherwise they van After both the TODO Update and File Follow-ups steps, dispatch `@pm` with operation `Commit pending changes` and the commit message constructed from the run context: -- If follow-ups were filed: `chore(todo): update status, file follow-ups`. -- Otherwise: `chore(todo): update status and progress`. +- If follow-ups were filed: `chore(todo): update $1 status, file follow-ups`. +- Otherwise: `chore(todo): update $1 status and progress`. `@pm` is responsible for persistence — the orchestrator does **not** run `git add` or `git commit` on TODO changes itself (per ADR-23). For the filesystem-backed `@pm`, the dispatch results in a single atomic commit on the feature branch; for tracker-backed `@pm` implementations (e.g. Linear), the dispatch is a no-op because the API calls already persisted the data. @@ -613,10 +609,10 @@ Capture the returned `sha` (may be `null` for non-filesystem trackers) for the r At any phase, if an unrecoverable error occurs (or a routing rule explicitly aborts to the Failure Handler — `PLAN_REWORK_REMAINING` exhausted, `split_needed` at Phase 7, etc.): 1. Write `$RUN_DIR/summary.md` (creating `$RUN_DIR` first if it doesn't exist) with what was completed and what failed. Do **not** stage or commit anything under `.workflow/`. -2. If any code was written, commit it with message `wip: incomplete workflow run for `. Stage code only — exclude `.workflow/` and `TODO/`. +2. If any code was written, commit it with message `wip: incomplete workflow run for $1`. Stage code only — exclude `.workflow/` and `TODO/`. 3. Leave the branch and worktree intact for the user to inspect — do not push, do not delete. -4. Dispatch `@pm` (operation: `Add comment`, issue ID: `$ISSUE_ID`) summarising what failed and naming the abort reason if it was a routing-rule abort (e.g. `split_needed at Phase 7 task-1`, `plan_rework_remaining exhausted at Phase 8`). The orchestrator never constructs the issue file path — `@pm` resolves it. -5. Dispatch `@pm` (operation: `Commit pending changes`, message: `chore(todo): record failure on `) so the failure note lands on the branch as a commit (per ADR-23). For tracker-backed `@pm` implementations this is a no-op. For filesystem `@pm`, the failure comment survives on the branch for the user to review before discarding the worktree. +4. Dispatch `@pm` (operation: `Add comment`, issue ID: `$1`) summarising what failed and naming the abort reason if it was a routing-rule abort (e.g. `split_needed at Phase 7 task-1`, `plan_rework_remaining exhausted at Phase 8`). The orchestrator never constructs the issue file path — `@pm` resolves it. +5. Dispatch `@pm` (operation: `Commit pending changes`, message: `chore(todo): record failure on $1`) so the failure note lands on the branch as a commit (per ADR-23). For tracker-backed `@pm` implementations this is a no-op. For filesystem `@pm`, the failure comment survives on the branch for the user to review before discarding the worktree. 6. Stop execution. ### Recovery procedure (workflow is non-resumable, ADR-14) @@ -625,7 +621,7 @@ The workflow is **non-resumable**. There is no `--resume` mode and no idempotent 1. `git worktree remove ` — discard the failed worktree. 2. Delete the feature branch: `git branch -D `. The Failure Handler's `wip:` commit (if any) is discarded with the branch. -3. Re-create the worktree from `$BASE_BRANCH`: `git worktree add -b `. +3. Re-create the worktree from your usual integration branch (e.g. `main`): `git worktree add -b `. 4. Re-run `/workflow ` from the fresh worktree. The throwaway-worktree model is the recovery story. Re-running on the same worktree without this cleanup risks committing partial state or appending duplicate `@pm` comments. diff --git a/config/opencode/workflow-design.md b/config/opencode/workflow-design.md index df13d83..0d3ce18 100644 --- a/config/opencode/workflow-design.md +++ b/config/opencode/workflow-design.md @@ -135,7 +135,7 @@ Every observed `(phase, signal) → action`. Empty cells are gaps. Walking this | Phase | Signal source | Signal | Action | |---|---|---|---| -| 1 | Sanity checks | Bare repo / detached HEAD / branch == base | Stop with error | +| 1 | Sanity checks | Bare repo / detached HEAD / branch is a protected name (`main`/`master`/`develop`/...) | Stop with error | | 1 | Sanity checks | Working tree dirty (`git status --porcelain` non-empty) | Stop with error (ADR-20) | | 2 | `@pm` (Validate run prerequisites) | `ok: true` | Capture `issue_file_path` and full issue context; proceed | | 2 | `@pm` (Validate run prerequisites) | `error_code: tracker_missing` | Stop with error using `@pm`'s message verbatim (ADR-22) | @@ -289,7 +289,7 @@ ADR-flavoured. New decisions append at the end. If a decision is later reversed ### ADR-14 (2026-05-08) — Workflow is non-resumable **Context:** Phase 9 has multiple sub-steps (code commit → `@pm` status update → file follow-ups → TODO commit → summary). Crashing between any two sub-steps leaves the worktree in a state that earlier docs called "partial." The original Failure Handler did not flip status back, did not recognize partial-Phase-9 separately from earlier-phase crashes, and re-running `/workflow` after a crash could append new comments and re-do work indefinitely. -**Decision:** declare the workflow non-resumable. On any failure (Failure Handler invocation), the recovery procedure is: `git worktree remove` the failed worktree, delete the feature branch, re-create the worktree from `$BASE_BRANCH`, then re-run `/workflow`. Document this explicitly in the Failure Handler section. The throwaway-worktree model means there is no in-place resume state to corrupt — the user discards the worktree and starts fresh. +**Decision:** declare the workflow non-resumable. On any failure (Failure Handler invocation), the recovery procedure is: `git worktree remove` the failed worktree, delete the feature branch, re-create the worktree from the user's usual integration branch (typically `main`), then re-run `/workflow`. Document this explicitly in the Failure Handler section. The throwaway-worktree model means there is no in-place resume state to corrupt — the user discards the worktree and starts fresh. **Alternatives:** (a) smarter Failure Handler that cleans up partial state idempotently; (b) transactional Phase 9 via a state file; (c) idempotent sub-steps so re-runs auto-resume. **Consequences:** simplest possible recovery model. Phase 9 sub-step ordering doesn't need to be defended against partial failures — partial state is acceptable because the recovery is "discard everything and re-run." User-initiated cancellation (Ctrl-C) follows the same procedure.