feat(opencode): @pm owns the TODO commit (ADR-23)

The orchestrator was running `git add ./TODO/` and `git commit -m
chore(todo): ...` itself in Phase 9, baking filesystem-tracker
specifics into commands/workflow.md. The point of @pm as an
abstraction is that it should be swappable — a Linear-backed @pm or a
Notion-backed @pm should drop in without touching the workflow
command. With API-backed trackers, "commit the TODO updates" is a
no-op and `git add ./TODO/` is wrong.

Push persistence shape behind the @pm boundary:

- New @pm capability `Commit pending changes` accepts a commit message
  and returns {ok, sha, message}. Filesystem @pm runs `git add ./TODO/`
  + `git commit -m <msg>` and returns the SHA. Tracker-backed
  implementations no-op and return sha: null.
- @pm gains tightly-scoped bash access: `git add ./TODO/*`,
  `git commit -m *`, `git status --porcelain ./TODO/*` only. Push,
  reset, rebase, checkout, branch, tag are explicit denies. Everything
  else falls through to the default deny.
- Phase 9 "Commit TODO Changes" replaces orchestrator-side git with a
  @pm dispatch; orchestrator constructs the message from run context
  and captures the returned SHA for the summary.
- Failure Handler gains a step 5 (commit pending after the failure
  comment add). Today the comment is left uncommitted in the working
  tree and gets discarded with the throwaway worktree (ADR-14) —
  forensic loss. With this change the failure note lands as its own
  commit on the failed branch.
- Routing matrix Phase 9 rows updated; ADR-22's superseded wording
  about orchestrator-side staging removed.

Stub-pass / body-pass / wip code commits remain orchestrator-owned —
those are code, not tracker-specific.

Refs: config/opencode/workflow-design.md ADR-23
This commit is contained in:
Harald Hoyer 2026-05-08 14:04:47 +02:00
parent a3e0de6d04
commit 56713cd7b8
3 changed files with 59 additions and 25 deletions

View file

@ -181,8 +181,10 @@ Every observed `(phase, signal) → action`. Empty cells are gaps. Walking this
| 9 | Orchestrator | Unresolved review-loop blocker | File sub-issue via `@pm` (label: `followup`) |
| 9 | `@test` (Phase 6) | NOT_TESTABLE future-seam note | File sub-issue via `@pm` (label: `tech-debt`) |
| 9 | Orchestrator | `@simplify` advisory not acted on | Record in summary; do NOT file (records, not work) |
| 9 | Orchestrator | All parent AC checked off | Set issue status to `Done`; sync README/parent; commit `chore(todo): …` |
| 9 | Orchestrator | Some parent AC remain unchecked AND sub-issues exist | Leave issue at `In Progress`; commit `chore(todo): …` |
| 9 | Orchestrator | All parent AC checked off | Dispatch `@pm` to set status `Done` and propagate to README/parent; then dispatch `@pm` (`Commit pending changes`) with `chore(todo): update <issue-id> status, file follow-ups` (ADR-23) |
| 9 | Orchestrator | Some parent AC remain unchecked AND sub-issues exist | Dispatch `@pm` to leave status `In Progress` and update AC checkboxes; then dispatch `@pm` (`Commit pending changes`) with the same message scheme |
| 9 | `@pm` (`Commit pending changes`) | `ok: true, sha: <hex>` | Capture SHA for run summary's "final commit SHA(s)" field |
| 9 | `@pm` (`Commit pending changes`) | `ok: true, sha: null` | Tracker-backed implementation, persistence already happened via API; record "no commit" in summary |
| Run-level | Failure Handler | Workflow is non-resumable (ADR-14) | Document the cleanup procedure: `git worktree remove`, delete branch, re-create from base, retry |
---
@ -358,9 +360,16 @@ The model carries five sub-decisions:
### ADR-22 (2026-05-08) — TODO path resolution lives with `@pm`; orchestrator never constructs TODO paths
**Context:** in early runs of the one-task-per-run workflow, the orchestrator sometimes did `@pm`'s job itself — reading `./TODO/$ISSUE_ID.md` directly to inspect the issue, instead of dispatching `@pm`. The text-level "anti-patterns" warning (workflow.md §Roles & Dispatch) wasn't enough to prevent it: once the workflow document told the orchestrator that issue files lived at `./TODO/<ID>.md`, the recipe was discoverable and tempting. Phase 1's sanity check (former steps 3 + 9 — TODO-tracker existence and `depends-on` enforcement) was the most blatant offender, since it required the orchestrator to read TODO files directly.
**Decision:** the orchestrator does not read, write, or construct any path under `TODO/` at any phase. All TODO operations — including prerequisite validation that used to live in Phase 1 — go through `@pm` dispatches. `@pm`'s response always includes the absolute file path of every issue file it touched (or read); the orchestrator captures these paths and uses them downstream (Phase 9 staging, Failure Handler comments, etc.) instead of constructing them. Phase 1 keeps only git/worktree-shaped checks; Phase 2 expands `@pm`'s existing dispatch into a "Validate run prerequisites" operation that returns either `{ok: true, issue_file_path, issue: {...}}` or a structured error.
**Alternatives:** (a) permission-deny `TODO/**` for the orchestrator — would force-fail orchestrator self-help but adds a permission layer the user prefers to avoid; (b) leave the doc warnings in place and hope the orchestrator complies — already shown to be insufficient; (c) keep Phase 1's TODO checks and just discipline the orchestrator harder — same problem as (b).
**Consequences:** discoverability of the path layout disappears from `commands/workflow.md` — the orchestrator literally never sees a `TODO/<ID>.md` template to imitate. The schema and path layout live in `agents/pm.md`, which the orchestrator does not load. `@pm`'s capabilities table grows by one ("Validate run prerequisites") and every existing capability now mandates including the absolute file path in the response. The orchestrator's Phase 9 staging step changes from constructing paths to using `@pm`-returned paths (or, equivalently, `git add ./TODO/` since the working tree was clean at Phase 1 and only `@pm` writes to TODO during a run).
**Decision:** the orchestrator does not read, write, or construct any path under `TODO/` at any phase, *and* `@pm`'s structured responses do not expose paths either — every reference to an issue is by ID. All TODO operations go through `@pm` dispatches; `@pm` resolves paths internally and never surfaces them to the orchestrator's structured input. Phase 1 keeps only git/worktree-shaped checks; Phase 2 expands `@pm`'s existing dispatch into a "Validate run prerequisites" operation that returns either `{ok: true, issue: {...}}` or a structured error. Phase 9 stages and commits TODO changes through `@pm`'s `Commit pending changes` capability (per ADR-23) — the orchestrator never runs `git add` or `git commit` on TODO files itself.
**Alternatives:** (a) permission-deny `TODO/**` for the orchestrator — would force-fail orchestrator self-help but adds a permission layer the user prefers to avoid; (b) leave the doc warnings in place and hope the orchestrator complies — already shown to be insufficient; (c) return paths in `@pm`'s response so the orchestrator can stage by file — leaks the path layout the orchestrator otherwise wouldn't see, and the path is unused for any other purpose since the orchestrator already addresses issues by ID.
**Consequences:** discoverability of the path layout disappears from `commands/workflow.md` *and* from `@pm`'s structured outputs — the orchestrator literally never sees a `TODO/<ID>.md` template to imitate, in any phase. The schema and path layout live in `agents/pm.md`, which the orchestrator does not load. `@pm`'s capabilities table grows by one ("Validate run prerequisites"). Path-construction temptation is eliminated by absence: there is no path field for the orchestrator to copy.
### ADR-23 (2026-05-08) — `@pm` owns persistence (including the TODO commit)
**Context:** the orchestrator was running `git add ./TODO/` and `git commit -m "chore(todo): ..."` itself in Phase 9 to commit `@pm`'s TODO updates, and the Failure Handler was leaving `@pm`'s failure-note comment uncommitted in the working tree. Both behaviors are correct for a *filesystem-backed* `@pm`, but they bake filesystem-specific persistence into the orchestrator. The design intent is that `@pm` is swappable — a Linear-backed implementation, a Notion-backed one, or any other issue-tracker adapter should drop in without touching `commands/workflow.md`. With API-backed trackers, "commit the TODO updates" is a no-op (the API call already persisted) and `git add ./TODO/` is wrong (no files to stage).
**Decision:** persistence shape lives behind the `@pm` boundary. `@pm` gains a new capability — `Commit pending changes` — that takes a commit message and returns a structured `{ok, sha, message}` response. The filesystem-backed `@pm` implements it by running `git add ./TODO/` + `git commit -m <msg>` and returning the new SHA. Tracker-backed `@pm` implementations no-op and return `sha: null`. The orchestrator constructs the commit message from run context (it has the issue ID, what was done, whether follow-ups were filed) and dispatches `@pm` for the actual commit at end of Phase 9 and at the Failure Handler. The orchestrator never runs `git add` or `git commit` on TODO content itself.
**Alternatives:** (a) keep orchestrator-side commit and accept that swapping `@pm` requires also touching workflow.md — defeats the swap-ability; (b) `@pm` constructs the commit message from semantic intent ("status update", "follow-ups filed") — moves run-context marshaling into `@pm` for no benefit; (c) leave failure-note comments uncommitted — current behavior, but they get lost when the user discards the throwaway worktree (ADR-14), which is silently dropping forensic data.
**Consequences:** `@pm` gains tightly-scoped bash access — only `git add ./TODO/*`, `git add ./TODO/`, `git commit -m *`, and `git status --porcelain ./TODO/*`/`/.../`; everything else is denied (no push, reset, rebase, checkout, branch, tag). Failure-note comments now land as their own commit on the failed branch, surviving the `git worktree remove` recovery step until the user explicitly discards the branch. Stub-pass and body-pass code commits remain the orchestrator's responsibility (those are code, not tracker-specific). Run summary's "final commit SHA(s)" field captures the SHA `@pm` returned, which may be `null` for non-filesystem trackers.
---