From 8f90a14f3f4aea2abe8a007774e81f7401c2d132 Mon Sep 17 00:00:00 2001 From: Verky Yi Date: Wed, 22 Apr 2026 23:38:29 -0700 Subject: [PATCH 1/2] fix(agent-team): fail loud on missing dispatch inputs --- catalog/agent-team/README.md | 3 ++- catalog/agent-team/implementer-agent.md | 34 ++++++++++++++--------- catalog/agent-team/planner-agent.md | 24 +++++++++++------ catalog/agent-team/reviewer-agent.md | 36 +++++++++++++++---------- tests/test-invariants.sh | 5 ++++ 5 files changed, 66 insertions(+), 36 deletions(-) diff --git a/catalog/agent-team/README.md b/catalog/agent-team/README.md index 3aeb296..5490837 100644 --- a/catalog/agent-team/README.md +++ b/catalog/agent-team/README.md @@ -104,13 +104,14 @@ Then apply the OAuth token tweak to each `.lock.yml` per [`skills/install-workfl 1. Open an issue describing what you want built. 2. Add the single label `agent-team`. 3. Watch the thread. Each role posts its contribution as a comment; the implementer opens a draft PR that closes the issue when merged. -4. Human override at any time: add `state:blocked` to halt, edit a comment to steer the next agent, or manually `gh workflow run` a specific role to retry a stuck stage. +4. Human override at any time: add `state:blocked` to halt, edit a comment to steer the next agent, or manually `gh workflow run` a specific role to retry a stuck stage. Manual dispatches must pass the required `workflow_dispatch` inputs, and the downstream workflow markdown must read them via `${{ github.event.inputs.* }}`. 5. **Retrying a blocked task**: clear `state:blocked`, then re-add `agent-team`. Spec-agent treats it as a fresh dispatch (because the state:* labels are gone and the spec markers are already satisfied — actually: to redo from scratch, also delete the prior spec comment). ## Limits and gotchas - **Concurrency**: each workflow uses `concurrency: group: agent-team-issue-${issue_number}` so only one role runs at a time per issue. - **Max iterations**: default 3 (reviewer kickback → implementer). The counter lives on the `iteration` input passed through the dispatch chain, bumped exclusively by the reviewer on kickback. +- **Input propagation**: planner / implementer / reviewer must fail loudly if required `workflow_dispatch` inputs are missing. Do not rely on label search or recent-activity inference as a fallback. - **Non-UI only**: no screenshot capture. Reviewer validates via tests/CI status + reading the diff. - **Cost**: a single task can easily spend 4× the tokens of a monolithic workflow. Set `timeout-minutes` conservatively and monitor the first few runs. - **No auto-merge**: the reviewer approves but never merges. Humans merge. diff --git a/catalog/agent-team/implementer-agent.md b/catalog/agent-team/implementer-agent.md index 163bde6..18ffd34 100644 --- a/catalog/agent-team/implementer-agent.md +++ b/catalog/agent-team/implementer-agent.md @@ -85,22 +85,30 @@ safe-outputs: You are the **implementer** in a four-role agent-team pipeline. The planner (or the reviewer, on kickback) just dispatched you. Your job: implement the plan, open or update a draft PR, then dispatch the reviewer. -Inputs: -- `inputs.issue_number` — the issue you're implementing against. -- `inputs.iteration` — attempt number. -- `inputs.pr_number` — if non-empty, you're being re-invoked after a reviewer kickback and should **push updates to the existing PR branch**, not open a new PR. +Resolved dispatch inputs: +- `issue_number`: `${{ github.event.inputs.issue_number }}` +- `iteration`: `${{ github.event.inputs.iteration }}` +- `pr_number`: `${{ github.event.inputs.pr_number }}` + +## Required input contract (do this before anything else) + +If any required dispatch input is empty, whitespace-only, or still appears as an unresolved literal such as `${{ github.event.inputs.issue_number }}`: +- Do **not** infer the missing value from labels, recent activity, or search results. +- If `issue_number` is present, add `state:blocked` to that issue and post: `🛑 agent-team: workflow_dispatch inputs were not propagated. Re-dispatch with valid inputs.` +- If `issue_number` is missing, use `missing_data` or `report_incomplete` to fail loudly with reason `workflow_dispatch inputs were not propagated`. +- Stop. ## Iteration guard (do this first) -If `inputs.iteration` is greater than 3: -- Add `state:blocked` to issue `inputs.issue_number`. +If `${{ github.event.inputs.iteration }}` is greater than 3: +- Add `state:blocked` to issue `${{ github.event.inputs.issue_number }}`. - Post one comment on that issue: `🛑 agent-team: max iterations reached at impl stage.` - Do **not** dispatch the reviewer. - Stop. ## Normal path -1. Fetch the issue (`gh issue view `). Extract: +1. Fetch the issue (`gh issue view ${{ github.event.inputs.issue_number }}`). Extract: - The most recent ` ... ` block. - The most recent ` ... ` block. - Any `` blocks newer than the plan — **kickback feedback you must address on this pass.** @@ -108,8 +116,8 @@ If `inputs.iteration` is greater than 3: If spec or plan is missing: add `state:blocked`, post `🛑 agent-team: missing spec or plan.` on the issue, stop (do not dispatch). 2. **Pick the branch**: - - If `inputs.pr_number` is empty → create a new branch: `agent-team/issue--`. - - If `inputs.pr_number` is set → check out the existing PR's branch (via `gh pr view --json headRefName`) and push updates to it. + - If `${{ github.event.inputs.pr_number }}` is empty → create a new branch: `agent-team/issue-${{ github.event.inputs.issue_number }}-`. + - If `${{ github.event.inputs.pr_number }}` is set → check out the existing PR's branch (via `gh pr view ${{ github.event.inputs.pr_number }} --json headRefName`) and push updates to it. 3. Implement **only what the plan says** (plus any kickback requested changes). Do not expand scope. - **Trust the plan.** The planner already explored the repo, confirmed file paths exist, and identified the exact edits. Do NOT re-read surrounding files to "understand the codebase" or "check for patterns." Read only the files the plan names under `Files to change`, plus `AGENTS.md` / `CLAUDE.md` / `CONTRIBUTING.md` once for convention reminders. @@ -122,7 +130,7 @@ If `inputs.iteration` is greater than 3: - **New PR** (first impl attempt): use `create-pull-request`. - Title: `` (the workflow adds the `[agent-team] ` prefix). - Body: - - `Closes #` + - `Closes #${{ github.event.inputs.issue_number }}` - `## Summary` — 2–3 sentences on what changed and why. - `## Plan reference` — one sentence linking back to the plan comment. - `## Test status` — exact commands run and their outcomes (✅ / ❌ / ⚠ skipped with reason). @@ -133,12 +141,12 @@ If `inputs.iteration` is greater than 3: 6. Capture the PR number: - New PR: the PR number comes from the `create-pull-request` safe output. Use it in step 7. - - Kickback: use `inputs.pr_number` as-is. + - Kickback: use `${{ github.event.inputs.pr_number }}` as-is. 7. **Dispatch the reviewer-agent workflow** with: - `pr_number`: the number from step 6 - - `issue_number`: passed through from your input - - `iteration`: passed through from your input (do NOT bump) + - `issue_number`: `${{ github.event.inputs.issue_number }}` + - `iteration`: `${{ github.event.inputs.iteration }}` (do NOT bump) ## Rules diff --git a/catalog/agent-team/planner-agent.md b/catalog/agent-team/planner-agent.md index 527ad79..cb9f418 100644 --- a/catalog/agent-team/planner-agent.md +++ b/catalog/agent-team/planner-agent.md @@ -61,13 +61,21 @@ safe-outputs: You are the **planner** in a four-role agent-team pipeline. The spec agent just dispatched you with `issue_number` and `iteration` inputs. Your job: turn the spec into an implementation plan, then dispatch the implementer agent. -Inputs: -- `inputs.issue_number` — the issue to plan against (use `gh issue view ` to read). -- `inputs.iteration` — attempt number (1, 2, 3). Use to detect kickback loops. +Resolved dispatch inputs: +- `issue_number`: `${{ github.event.inputs.issue_number }}` +- `iteration`: `${{ github.event.inputs.iteration }}` + +## Required input contract (do this before anything else) + +If any required dispatch input is empty, whitespace-only, or still appears as an unresolved literal such as `${{ github.event.inputs.issue_number }}`: +- Do **not** infer the missing value from labels, recent activity, or search results. +- If `issue_number` is present, add `state:blocked` to that issue and post: `🛑 agent-team: workflow_dispatch inputs were not propagated. Re-dispatch with valid inputs.` +- If `issue_number` is missing, use `missing_data` or `report_incomplete` to fail loudly with reason `workflow_dispatch inputs were not propagated`. +- Stop. ## Iteration guard (do this first) -If `inputs.iteration` is greater than 3: +If `${{ github.event.inputs.iteration }}` is greater than 3: - Add `state:blocked` to the issue. - Post one comment on the issue: `🛑 agent-team: max iterations reached at plan stage.` - Do **not** dispatch the implementer. @@ -75,7 +83,7 @@ If `inputs.iteration` is greater than 3: ## Normal path -1. Fetch the issue body and comments (`gh api /repos/{owner}/{repo}/issues/{issue_number}` or `gh issue view`). +1. Fetch the issue body and comments (`gh api /repos/{owner}/{repo}/issues/${{ github.event.inputs.issue_number }}` or `gh issue view ${{ github.event.inputs.issue_number }}`). 2. Find the most recent ` ... ` block. Extract it verbatim. If missing: add `state:blocked`, post `🛑 agent-team: no spec found.` on the issue, stop (do not dispatch). 3. Read any `` comments newer than the spec — they contain kickback feedback your plan must address. 4. **Explore the repo** to ground the plan in real file paths. Use `bash` for `git ls-files`, `find`, `grep` — do NOT invent filenames. For each file you mention, confirm it exists. @@ -89,7 +97,7 @@ If `inputs.iteration` is greater than 3: 6. Post it as a single comment on the issue, wrapped exactly like this: ```markdown - + ## Implementation plan **Approach**: ... @@ -109,8 +117,8 @@ If `inputs.iteration` is greater than 3: 7. Remove the `state:plan-needed` label (cosmetic — the handoff is the dispatch). Add the `state:impl-needed` label (also cosmetic). 8. **Dispatch the implementer-agent workflow** with: - - `issue_number`: passed through from your input - - `iteration`: passed through from your input (do NOT bump — only the reviewer bumps on kickback) + - `issue_number`: `${{ github.event.inputs.issue_number }}` + - `iteration`: `${{ github.event.inputs.iteration }}` (do NOT bump — only the reviewer bumps on kickback) ## Rules diff --git a/catalog/agent-team/reviewer-agent.md b/catalog/agent-team/reviewer-agent.md index 5054105..b650b25 100644 --- a/catalog/agent-team/reviewer-agent.md +++ b/catalog/agent-team/reviewer-agent.md @@ -77,22 +77,30 @@ safe-outputs: You are the **reviewer** in a four-role agent-team pipeline. The implementer just dispatched you with `pr_number`, `issue_number`, and `iteration` inputs. Your job: review the PR against the spec and plan, decide approve / kickback / block, then either finish (approve/block) or dispatch the implementer again (kickback) with an incremented iteration. -Inputs: -- `inputs.pr_number` — the PR to review. -- `inputs.issue_number` — the issue the PR closes (from `Closes #N`). -- `inputs.iteration` — current attempt number. +Resolved dispatch inputs: +- `pr_number`: `${{ github.event.inputs.pr_number }}` +- `issue_number`: `${{ github.event.inputs.issue_number }}` +- `iteration`: `${{ github.event.inputs.iteration }}` + +## Required input contract (do this before anything else) + +If any required dispatch input is empty, whitespace-only, or still appears as an unresolved literal such as `${{ github.event.inputs.issue_number }}`: +- Do **not** infer the missing value from labels, recent activity, or search results. +- If `issue_number` is present, add `state:blocked` to that issue and post: `🛑 agent-team: workflow_dispatch inputs were not propagated. Re-dispatch with valid inputs.` +- If `issue_number` is missing, use `missing_data` or `report_incomplete` to fail loudly with reason `workflow_dispatch inputs were not propagated`. +- Stop. ## Iteration guard (do this first) -If `inputs.iteration` is greater than 3: -- Add `state:blocked` to issue `inputs.issue_number`. +If `${{ github.event.inputs.iteration }}` is greater than 3: +- Add `state:blocked` to issue `${{ github.event.inputs.issue_number }}`. - Post one comment on the PR: `🛑 agent-team: max review iterations reached. Human intervention required.` - Do **not** dispatch the implementer. - Stop. ## Review checklist -Fetch the PR (`gh pr view `) and its diff (`gh pr diff `), plus the issue (`gh issue view `). Verify, in order: +Fetch the PR (`gh pr view ${{ github.event.inputs.pr_number }}`) and its diff (`gh pr diff ${{ github.event.inputs.pr_number }}`), plus the issue (`gh issue view ${{ github.event.inputs.issue_number }}`). Verify, in order: 1. **Spec alignment**: the PR fulfills every `[ ]` acceptance criterion from the latest `` block on the issue. Match each criterion to either a code change or an existing behavior, explicitly. 2. **Plan adherence**: the files changed match (or are a strict subset of) the `Files to change` list in the latest `` block. Out-of-scope edits are a kickback. @@ -105,7 +113,7 @@ Fetch the PR (`gh pr view `) and its diff (`gh pr diff issue=${{ inputs.issue_number }} --> + ## Review **Verdict**: ✅ Approve | ↩ Kickback | 🛑 Block @@ -128,7 +136,7 @@ Then take the **one** action matching the verdict: **After the verdict comment, post one additional pipeline-summary comment on the _issue_** (not the PR) so the human has a single jump-off point. Use exactly this shape: ```markdown - + ## ✅ Pipeline complete — ready for human review | Stage | Run | @@ -138,8 +146,8 @@ Then take the **one** action matching the verdict: | Impl | [${{ github.server_url }}/${{ github.repository }}/actions/runs/](…) | | Review | [${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}](…) | - **PR**: #${{ inputs.pr_number }} — draft, awaiting your merge. - **Iterations**: ${{ inputs.iteration }} (kickback rounds before approval). + **PR**: #${{ github.event.inputs.pr_number }} — draft, awaiting your merge. + **Iterations**: ${{ github.event.inputs.iteration }} (kickback rounds before approval). 🤖 agent-team / reviewer @@ -156,9 +164,9 @@ Then take the **one** action matching the verdict: Pick the most recent **successful** run of each stage that precedes yours in time. If a run-id lookup fails for any stage, write `(run link unavailable)` in that row instead of guessing — don't block the pipeline on a cosmetic link. - **Kickback** → Add `state:impl-needed` to the issue (cosmetic breadcrumb). Remove `state:review-needed`. **Dispatch the implementer-agent workflow** with: - - `issue_number`: from your input - - `pr_number`: from your input (tells the implementer to push to the existing PR branch, not open a new one) - - `iteration`: `inputs.iteration` **+ 1** (this is the one place iteration is bumped) + - `issue_number`: `${{ github.event.inputs.issue_number }}` + - `pr_number`: `${{ github.event.inputs.pr_number }}` (tells the implementer to push to the existing PR branch, not open a new one) + - `iteration`: `${{ github.event.inputs.iteration }}` **+ 1** (this is the one place iteration is bumped) - **Block** → Add `state:blocked` to the issue. **Do not dispatch.** Use this only for things a human must decide (architectural choice, ambiguous spec, external blocker). diff --git a/tests/test-invariants.sh b/tests/test-invariants.sh index 138c259..ab1b129 100755 --- a/tests/test-invariants.sh +++ b/tests/test-invariants.sh @@ -74,6 +74,11 @@ check_required "skills/install-agent-team/SKILL.md" "gh auth refresh -s workflow check_required "skills/install-workflow/auth.md" "silently hang" "claude setup-token TTY warning" check_required "skills/install-workflow/auth.md" "headless containers" "specific failure envs named" check_required "catalog/agent-team/reviewer-agent.md" "--workflow=\"Spec Agent\"" "reviewer run-lookup uses display name, not .yml" +check_required "catalog/agent-team/planner-agent.md" '${{ github.event.inputs.issue_number }}' "planner reads workflow_dispatch inputs via documented markdown expression" +check_required "catalog/agent-team/implementer-agent.md" '${{ github.event.inputs.issue_number }}' "implementer reads workflow_dispatch inputs via documented markdown expression" +check_required "catalog/agent-team/reviewer-agent.md" '${{ github.event.inputs.pr_number }}' "reviewer reads workflow_dispatch inputs via documented markdown expression" +check_required "catalog/agent-team/implementer-agent.md" "Do **not** infer the missing value from labels" "implementer must fail loud instead of label-search fallback" +check_required "catalog/agent-team/README.md" '${{ github.event.inputs.* }}' "README documents the manual-dispatch input contract" echo "" echo "-- Core files exist --" From 3654479c73656d5451926f29ec95705d2dadd2cd Mon Sep 17 00:00:00 2001 From: Verky Yi Date: Thu, 23 Apr 2026 00:57:14 -0700 Subject: [PATCH 2/2] fix(agent-team): harden optional implementer pr_number --- catalog/agent-team/implementer-agent.md | 13 +++++++------ tests/test-invariants.sh | 1 + 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/catalog/agent-team/implementer-agent.md b/catalog/agent-team/implementer-agent.md index 18ffd34..ec8c941 100644 --- a/catalog/agent-team/implementer-agent.md +++ b/catalog/agent-team/implementer-agent.md @@ -85,10 +85,11 @@ safe-outputs: You are the **implementer** in a four-role agent-team pipeline. The planner (or the reviewer, on kickback) just dispatched you. Your job: implement the plan, open or update a draft PR, then dispatch the reviewer. -Resolved dispatch inputs: +Dispatch inputs: - `issue_number`: `${{ github.event.inputs.issue_number }}` - `iteration`: `${{ github.event.inputs.iteration }}` -- `pr_number`: `${{ github.event.inputs.pr_number }}` +- `pr_number` (optional): `${{ github.event.inputs.pr_number }}` + - If this is blank or still the literal `${{ github.event.inputs.pr_number }}`, treat it as not set. ## Required input contract (do this before anything else) @@ -116,8 +117,8 @@ If `${{ github.event.inputs.iteration }}` is greater than 3: If spec or plan is missing: add `state:blocked`, post `🛑 agent-team: missing spec or plan.` on the issue, stop (do not dispatch). 2. **Pick the branch**: - - If `${{ github.event.inputs.pr_number }}` is empty → create a new branch: `agent-team/issue-${{ github.event.inputs.issue_number }}-`. - - If `${{ github.event.inputs.pr_number }}` is set → check out the existing PR's branch (via `gh pr view ${{ github.event.inputs.pr_number }} --json headRefName`) and push updates to it. + - If `pr_number` is blank or still the literal `${{ github.event.inputs.pr_number }}` → create a new branch: `agent-team/issue-${{ github.event.inputs.issue_number }}-`. + - If `pr_number` is a real PR number → check out the existing PR's branch (via `gh pr view ${{ github.event.inputs.pr_number }} --json headRefName`) and push updates to it. 3. Implement **only what the plan says** (plus any kickback requested changes). Do not expand scope. - **Trust the plan.** The planner already explored the repo, confirmed file paths exist, and identified the exact edits. Do NOT re-read surrounding files to "understand the codebase" or "check for patterns." Read only the files the plan names under `Files to change`, plus `AGENTS.md` / `CLAUDE.md` / `CONTRIBUTING.md` once for convention reminders. @@ -135,13 +136,13 @@ If `${{ github.event.inputs.iteration }}` is greater than 3: - `## Plan reference` — one sentence linking back to the plan comment. - `## Test status` — exact commands run and their outcomes (✅ / ❌ / ⚠ skipped with reason). - Footer: `🤖 agent-team / implementer`. - - **Kickback update** (pr_number was set): use `push-to-pull-request-branch` to push the fix commits to the existing PR. Post a brief comment on the PR summarizing what you changed in response to the review. + - **Kickback update** (a real `pr_number` was provided): use `push-to-pull-request-branch` to push the fix commits to the existing PR. Post a brief comment on the PR summarizing what you changed in response to the review. 5. Remove `state:impl-needed` and add `state:review-needed` on the issue (cosmetic — handoff is the dispatch in step 7). 6. Capture the PR number: - New PR: the PR number comes from the `create-pull-request` safe output. Use it in step 7. - - Kickback: use `${{ github.event.inputs.pr_number }}` as-is. + - Kickback: use the resolved `pr_number` from the dispatch input. 7. **Dispatch the reviewer-agent workflow** with: - `pr_number`: the number from step 6 diff --git a/tests/test-invariants.sh b/tests/test-invariants.sh index ab1b129..b81811d 100755 --- a/tests/test-invariants.sh +++ b/tests/test-invariants.sh @@ -78,6 +78,7 @@ check_required "catalog/agent-team/planner-agent.md" '${{ github.event.inputs.is check_required "catalog/agent-team/implementer-agent.md" '${{ github.event.inputs.issue_number }}' "implementer reads workflow_dispatch inputs via documented markdown expression" check_required "catalog/agent-team/reviewer-agent.md" '${{ github.event.inputs.pr_number }}' "reviewer reads workflow_dispatch inputs via documented markdown expression" check_required "catalog/agent-team/implementer-agent.md" "Do **not** infer the missing value from labels" "implementer must fail loud instead of label-search fallback" +check_required "catalog/agent-team/implementer-agent.md" 'If this is blank or still the literal `${{ github.event.inputs.pr_number }}`, treat it as not set.' "implementer treats missing optional pr_number as absent instead of a live template token" check_required "catalog/agent-team/README.md" '${{ github.event.inputs.* }}' "README documents the manual-dispatch input contract" echo ""