diff --git a/config/opencode/agents/pm.md b/config/opencode/agents/pm.md index ee682f3..a0d50ce 100644 --- a/config/opencode/agents/pm.md +++ b/config/opencode/agents/pm.md @@ -1,5 +1,5 @@ --- -description: Project management agent that manages a Linear-style TODO/ folder (one file per issue plus a README.md index) +description: Project management agent that manages a Linear-style TODO/ folder (one file per issue plus a README.md index). Owns persistence, including the git commit of TODO changes (ADR-23). mode: subagent tools: read: true @@ -7,7 +7,26 @@ tools: grep: true write: true edit: true - bash: false + bash: true +permission: + # Tightly-scoped git access for the `Commit pending changes` capability. + # @pm owns persistence shape (filesystem commit vs. API call vs. other), + # so the bash sandbox is opened just enough to commit TODO/ updates and + # nothing else. See ADR-23. + bash: + "*": deny + "git add ./TODO/*": allow + "git add ./TODO/": allow + "git commit -m *": allow + "git status --porcelain ./TODO/*": allow + "git status --porcelain ./TODO/": allow + # Explicit denials for safety + "git push*": deny + "git reset*": deny + "git rebase*": deny + "git checkout*": deny + "git branch*": deny + "git tag*": deny --- You are a project management assistant. Your sole responsibility is reading and updating files inside a `TODO/` directory. You do **not** modify any file outside that directory under any circumstances. @@ -115,18 +134,25 @@ Statuses: `Todo`, `In Progress`, `Done`. You can: - **Validate run prerequisites** — given an issue ID, verify the TODO tracker is well-formed in this worktree (directory + `README.md` present), locate the issue file, and confirm every entry in its `depends-on:` frontmatter resolves to a `Done` issue. Used by `/workflow`'s Phase 2 (per ADR-22) so the orchestrator never constructs a TODO path itself. Returns a structured success or failure response (see "Run-Prerequisite Output" below). -- **View** an issue by ID — read `/.md` and return its fields structured. **Always include the resolved absolute file path** in the response (`issue_file_path` field). +- **View** an issue by ID — read `/.md` and return its fields structured. - **List** issues, optionally filtered by status / parent / label. Walk `/*.md` (excluding `README.md`), parse frontmatter. -- **Create** an issue. Generate the next ID by scanning existing IDs with the same prefix and incrementing. Default `status: Todo`. Write `/.md`. If the issue is top-level (`parent: null`), update `README.md` to add it under the caller-specified category. If the issue is a sub-issue (`parent: `), update the parent file's `## Sub-issues` section. **Return the absolute path of the new issue file** (`new_issue_path`) and the absolute paths of every dependent index updated (`updated_paths`). +- **Create** an issue. Generate the next ID by scanning existing IDs with the same prefix and incrementing. Default `status: Todo`. Write `/.md`. If the issue is top-level (`parent: null`), update `README.md` to add it under the caller-specified category. If the issue is a sub-issue (`parent: `), update the parent file's `## Sub-issues` section. Return the new issue's `id`. - **Update status** in frontmatter. When status changes to/from `Done`, propagate the checkbox flip to: - `README.md` if the issue is top-level (`parent: null`), **or** - the parent issue's `## Sub-issues` line if it has a parent. - Return the list of all paths modified by the operation. -- **Add a comment** — append `- YYYY-MM-DD — ` to the issue's `## Comments` section (create the section if missing, just before EOF). Return the modified path. -- **Check off acceptance criteria** by index or matching text — flip `- [ ]` to `- [x]` under `## Acceptance criteria`. Return the modified path. -- **Edit** description or other body sections when explicitly requested. Return the modified path. +- **Add a comment** — append `- YYYY-MM-DD — ` to the issue's `## Comments` section (create the section if missing, just before EOF). +- **Check off acceptance criteria** by index or matching text — flip `- [ ]` to `- [x]` under `## Acceptance criteria`. +- **Edit** description or other body sections when explicitly requested. +- **Commit pending changes** — given a commit message, stage every modification you've made under `/` in this dispatch chain and create one git commit. Used by `/workflow`'s Phase 9 (and Failure Handler) so the orchestrator stays tracker-agnostic — see ADR-23. **Filesystem-backed `@pm` (this agent):** + 1. Run `git status --porcelain ./TODO/` to confirm there are changes to commit. If empty, return `{ok: true, sha: null, message: "no changes to commit"}` — do not error. + 2. `git add ./TODO/`. + 3. `git commit -m ""`. + 4. Capture the resulting SHA (`git rev-parse HEAD`). + 5. Return `{ok: true, sha: "", message: "committed N files"}`. -**Path-return rule:** every operation that modifies the filesystem must include the absolute path(s) of every file it touched in its response (`modified_paths` array, or named fields like `new_issue_path` / `updated_paths` for create). Read-only operations (View, List) include `issue_file_path` for the issue they read. The caller (`/workflow`'s orchestrator) deliberately does not construct TODO paths from issue IDs — it relies on these returned paths for staging, commenting, and follow-on dispatches. + Other backends (Linear, Notion, REST, …) implement this capability as a no-op or whatever their persistence model requires — the API call already persisted the data, so they return `{ok: true, sha: null, message: "no commit needed; persistence is via API"}`. + +**No-paths-in-response rule (ADR-22):** the caller (`/workflow`'s orchestrator) deliberately operates without knowing the TODO path layout. Your responses identify issues by `id`, never by absolute file path. Error messages may mention paths in prose for human readability, but the structured response shape exposes no path field. The orchestrator stages nothing — `Commit pending changes` is the only path through which `TODO/` changes become git history. ## Run-Prerequisite Output @@ -136,7 +162,6 @@ The `Validate run prerequisites` capability returns one of two JSON shapes: ```json { "ok": true, - "issue_file_path": "/abs/path/to/TODO/.md", "issue": { "id": "...", "title": "...", @@ -182,7 +207,6 @@ Single-issue schema: ```json { - "issue_file_path": "/abs/path/to/TODO/GAL-39.md", "id": "GAL-39", "title": "Implement a special stage type", "status": "Done", @@ -203,7 +227,7 @@ Single-issue schema: } ``` -`issue_file_path` is **always included** for any operation that reads or writes a single issue file (per the path-return rule above). Omit fields whose corresponding sections are absent (`null` is fine for `parent`, drop `depends_on`/`sub_issues`/`acceptance_criteria`/`integration_test_hints`/`comments` entirely if the section/field isn't in the file). +Omit fields whose corresponding sections are absent (`null` is fine for `parent`, drop `depends_on`/`sub_issues`/`acceptance_criteria`/`integration_test_hints`/`comments` entirely if the section/field isn't in the file). No path field — the caller does not need it (per the No-paths-in-response rule above). For list output, return an array of `{id, title, status, parent, labels}` objects. diff --git a/config/opencode/commands/workflow.md b/config/opencode/commands/workflow.md index 67a3208..e5379ae 100644 --- a/config/opencode/commands/workflow.md +++ b/config/opencode/commands/workflow.md @@ -103,7 +103,6 @@ Dispatch `@pm` with the issue ID `$ISSUE_ID`, `$WORKTREE_PATH`, and `Validate ru ```json { "ok": true, - "issue_file_path": "", "issue": { "id": "...", "title": "...", @@ -129,7 +128,7 @@ Dispatch `@pm` with the issue ID `$ISSUE_ID`, `$WORKTREE_PATH`, and `Validate ru On failure, stop the workflow with `@pm`'s `message` verbatim. Do **not** attempt to inspect or repair the TODO tree from the orchestrator — that belongs to `@pm`. -On success, capture `ISSUE_FILE_PATH` from the response. **Use this captured path verbatim everywhere downstream** (Phase 9 staging, Failure Handler comments, etc.) — never construct a TODO path from `$ISSUE_ID` directly. +On success, the orchestrator works exclusively from the structured `issue` object. **Every subsequent TODO operation re-dispatches `@pm` by issue ID** — the orchestrator never holds or passes around a file path. If `issue.status == "Todo"`, dispatch `@pm` again to flip it to `In Progress` (operation: `Update status`, target: the same issue ID; `@pm` propagates to README.md / parent's `## Sub-issues` line). The status edit will be staged alongside other TODO updates in Phase 9. @@ -562,8 +561,6 @@ Dispatch `@pm` with the issue ID `$ISSUE_ID` and the following operations (a sin `@pm` propagates status flips to the dependent index (the top-level README or the parent's `## Sub-issues` line) on its own — that's its job, not the orchestrator's. The orchestrator passes high-level intent ("set status to Done") and trusts `@pm` to update every dependent file. -`@pm`'s response includes the list of files it modified (absolute paths). Capture this list as `MODIFIED_TODO_PATHS` for the staging step below. - ### 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`). @@ -587,11 +584,14 @@ Tracked-worthy unresolved items must become real TODO issues; otherwise they van ### Commit TODO Changes -After both the TODO Update and File Follow-ups steps, stage every path returned by `@pm` in this run (the union of `MODIFIED_TODO_PATHS` and `NEW_SUBISSUE_PATHS` collected from each `@pm` dispatch). Commit them in a single atomic commit: `chore(todo): update status, file follow-ups`. +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: -Equivalently — and more robustly, since the orchestrator can't have edited TODO files directly (Phase 1 verified the working tree was clean and the orchestrator never writes there) — stage the entire `TODO/` directory: `git add ./TODO/`. Anything staged under `TODO/` came from `@pm` during this run. +- If follow-ups were filed: `chore(todo): update status, file follow-ups`. +- Otherwise: `chore(todo): update status and progress`. -If no follow-ups were filed, the commit message simplifies to `chore(todo): update 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. + +Capture the returned `sha` (may be `null` for non-filesystem trackers) for the run summary's "final commit SHA(s)" field. ### Run Summary - Write `$RUN_DIR/summary.md` with: @@ -616,7 +616,8 @@ At any phase, if an unrecoverable error occurs (or a routing rule explicitly abo 2. If any code was written, commit it with message `wip: incomplete workflow run for `. 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. Stop execution. +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. +6. Stop execution. ### Recovery procedure (workflow is non-resumable, ADR-14) diff --git a/config/opencode/workflow-design.md b/config/opencode/workflow-design.md index 53bf3c8..df13d83 100644 --- a/config/opencode/workflow-design.md +++ b/config/opencode/workflow-design.md @@ -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 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: ` | 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/.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/.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/.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 ` 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. ---